Skip to content

fix(miner): governor reputation-history read-modify-write is unguarded, unlike sibling scalar-state writes #8855

Description

@JSONbored

⚠️ Definition of Done: this issue must be completed in full, in a single PR. Do not split this
work across multiple PRs, and do not defer any Deliverable below to a follow-up issue. A PR that
satisfies only some of the Deliverables, stubs a required test, or leaves a checkbox
partially-done does NOT resolve this issue and will be closed.

Context

packages/loopover-miner/lib/governor-state.ts:382-396's loadReputationHistory/saveReputationHistory pair has no withTransaction wrapper, unlike the other three scalar-state methods in the same file (lines 292-296 explain why those three need it: "so two processes on the same file... cannot interleave a stale read with each other's write"). packages/loopover-miner/lib/loop-cli.ts:580-587 does const prior = governorState.loadReputationHistory(...); governorState.saveReputationHistory(..., { decided: prior.decided + 1, unfavorable: prior.unfavorable + (isRejectedPr ? 1 : 0) }, ...) -- a classic read-modify-write with no transaction around the pair, even though governor-state.sqlite3 is shared across fleet containers (DEPLOYMENT.md:60). Two containers finishing an attempt concurrently can both read the same counts and both +1, losing an increment in the Governor's self-reputation safety throttle.

Requirements

Wrap the load+save pair in one BEGIN IMMEDIATE transaction (matching the other three scalar-state methods' pattern in the same file), or expose a new atomic incrementReputationHistory(delta) method on GovernorState that performs the read-modify-write inside a single transaction, and update loop-cli.ts:580-587 to use it.

Deliverables

  • The reputation-history read-modify-write in governor-state.ts (or its loop-cli.ts call site) is wrapped in a single atomic transaction
  • A concurrency test simulating two overlapping increments (e.g. two saveReputationHistory calls racing against one loadReputationHistory read) asserting no increment is lost

All of the above Deliverables are required in the same PR unless the deliverable text itself states otherwise.

Test Coverage Requirements

packages/loopover-miner/** -- 99%+ patch coverage including the new concurrency test path.

Expected Outcome

Two fleet containers finishing attempts concurrently can no longer lose a reputation-history increment; the Governor's self-reputation safety throttle counts every decided attempt correctly.

Links & Resources

  • packages/loopover-miner/lib/governor-state.ts:292-296,382-396
  • packages/loopover-miner/lib/loop-cli.ts:580-587
  • packages/loopover-miner/DEPLOYMENT.md:60

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions