Repository navigation
Land verified mining and pool stack - #1695
Conversation
Share mining jobs and share validation across opt-in SV1 and authenticated SV2 listeners. Keep durable PPLNS accounting and external-signer reconciliation behind one ledger owner. Add the explicitly approved PoolLedger storage family without resetting existing records. Verify with 490 behavioral tests, strict feature-enabled workspace lint, and 14 architecture tests. No operator database was opened or migrated.
The template refresh interval left accepted work active. A miner could submit another winning sibling before the next refresh, causing a stale parent rejection and an avoidable SV2 disconnect. Retire work at the authoritative Applied result in both native hosts. Preserve duplicate identity for exact accepted replays and record in-flight stale-parent receipts before returning a stale share error. Prove both host paths with failing-before regression tests. The native SV1/SV2 process proof covers maturity, external signing, payout recovery and hash-based orphan accounting without custody in the node.
The registry includes the native pool methods, but the feature gate classified them as unshipped. Honor the pool build feature so the inventory agrees with dispatch in both build modes.
Resolve the current path-crate dependency graph so locked tool builds work again. Preserve existing external versions and align the two added cryptographic packages with the root lockfile.
crates/node: make the five SRI/Noise dev-deps (binary_sv2, codec_sv2, common_messages_sv2, mining_sv2, noise_sv2) optional and enable them through the existing pool-sv2 feature. Only the pool/sv2.rs host-path regression uses them, and it compiles solely under pool-sv2, so non-pool test builds no longer resolve the Noise/secp duplicate-crypto chain. docs(contracts): record the approved pool-only DEP-02 exception for the pinned noise_sv2 2.0.0 family (secp256k1 0.28.2 / secp256k1-sys 0.9.2 / bitcoin_hashes 0.13.1 exact-version skips) and the SRI pin maintenance rule. No new splits; deny still fails any other duplicate.
…edger Header-hash uniqueness: retire_job tombstones header hashes into pplns_retired_hashes_v1 instead of deleting them, so a retired hash can never be credited again under another job (ShareConflict names the original credit). share_by_hash still resolves tombstoned hashes. Secondary identity: ShareCredit gains header version and ntime, and tuple_key covers job/account/nonce/version/ntime/extranonce, so version-rolling and ntime-rolling shares credit as distinct shares instead of false ShareIdentityConflict. Table::last no longer materializes the table: single lazy pass retaining one row plus overlay merge. rows() uses iter_prefix directly (the only scan primitive this tree's storage backend exposes). PoolConfig::encode validates before encoding so an over-count or invalid change script can never be persisted or compared as bytes. LedgerError::is_permanent_share_conflict classifies JobRetired, ShareConflict, ShareIdentityConflict and BlockConflict for protocol adapters to map to rejections rather than LedgerUnavailable retries. Node/rpc ShareCredit literals updated for the new fields; the adapter mapping itself is left to the owning lane.
- stratum-v2 session: validate rejection codes before ticket removal (reject_share/reject_block), reserve event capacity before share validation, track per-connection parent delivery for SetNewPrevHash (ignore owner fresh flag, retry missing parent, force Future until a parent is known). - node sv2: handshake (15s) and idle (600s) deadlines with progress tracking, sane per-connection channel caps (8 channels, 16 jobs/channel, 16 pending acks, 32 pending events), announce via per-connection delivery state. - node ledger: reconcile outside StableRead with tip-generation recheck (idempotent retry); include ShareCredit version/ntime. - protocol regressions: malformed reject preserves ticket, event-backlog pre-validation check, fresh-independent delivery.
- lifecycle: rename pool_reward_script to final_payout_script as the one owner for pool jobs and ordinary getblocktemplate; pool reward wins when configured, else mining payout. Test helper uses the same owner. Regression covers differing configured values resolving to the pool script, keeping the RpcBindings/RpcSurface lifecycle structure. - stratum SV1: track has_id through decode (Request/DecodeFailure), emit replies only for frames carrying an id, and carry the flag into deferred share completions so notification completions still record ServerEvents without a wire reply. Adds decode regression for calls, explicit null ids, and suppressed notification failures.
- Reserve two item slots clamped to the configured bounds with only share plus block-framing bytes, so minima admit a submit on an empty outbox and one in-flight submit cannot spend the whole pool budget and stall the pool at a single share. Block bodies admit over the byte bound rather than shedding a found block. - Continue block-record delivery when the share is already durable (crash restore, post-eviction replay) or permanently refused, and report the duplicate only after the block fate is known. - Record block-candidate shares in the duplicate set even at per-job capacity, so replays report Duplicate instead of validating twice. - Normalize SV1 wire version bits to the canonical header version at the edge before the common submit owner; submit and settlement keep no wire branching and the book stays the single share predicate. - Map permanent ledger conflicts to RecordOutcome::Rejected through the SV1 adapter only: rejected shares answer Duplicate with no retry and no buffering, and refused block records drop instead of wedging the drain. SV2 paths keep their existing contract. - Regressions: minima admission, pool-wide reservation, crash-restore block delivery, permanent-refusal no-retry/no-wedge, edge version normalization, unknown-job verdict, block-at-capacity replay.
…e ABA recheck hole
Normalize SV1 outcome handling, version test typing, and lint style. Exercise pool-wide outbox buffering, permanent-refusal recovery, block-share replay, and SV2 ticket/capacity behavior with the corrected fixtures.
Delegate failure-to-wire mapping to one helper while preserving verdicts, notification suppression, and deferred completion behavior.
Move credited delivery and acceptance handling into one helper while preserving inline and deferred responses. Keep failure mapping and version normalization unchanged.
Use Copy prefixes in the nonce helper and update call sites without changing the fixtures.
Use contextual test failures instead of expect calls.
Use Debug formatting in SV1 tests, simplify tombstone checks, and express retired hash lookup with if/else. Behavior is unchanged.
Add greatest-prefix snapshot lookups with native backend implementations and fold pending overlay puts and deletes without scanning ordinary tail reads. Document pool clause proofs.
Use ok_or_else for the table-prefix overflow invariant without changing its condition or message.
Keep the verified BlockPayload submission path, SV2 stale-parent fake, and recorded payload assertions after moving the migration into the first commit.
Exercise getpoolinfo through real TCP dispatch for authorized management access, credential refusal, and public-surface refusal.
Cover directory locking, composed limits, authority files, one-pending submission, observer errors, and management authentication with executable tests. Record remaining lifecycle and evidence gaps.
Share per-output balance mutation, canonical scriptSig limits, and primitive lowercase-hex encoding without changing ledger or wire behavior.
Map permanent SV2 share conflicts to rejection, validate payout change scripts before encoding, and align architecture and embedding contracts with the final code.
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Summary
This PR lands the mining and pool stack (SV1, SV2, pool ledger, payout). The scope is significant — ~50K additions across 265 files. The overall design is well-structured with clear separation of concerns and solid test coverage for the core paths.
Blocking issues found:
-
SV2 connection livelock (
crates/node/src/pool/sv2.rs, lines 272–275): The block record permit is reserved unconditionally on every loop iteration before processingbuffered_input. When all 8MAX_PENDING_BLOCK_RECORDSslots are exhausted, the loop sleeps andcontinues — even whenbuffered_inputalready contains a fully-read frame. Every share submission, channel open, and even the Noise handshake is blocked until the ledger drains. The permit should only be acquired lazily (e.g., by passingNoneand acquiring insidehandle_eventsonly whenBlockFoundis encountered). -
SV2 connection closed on queued block record (
crates/node/src/pool/sv2.rs, lines 391–395): Whenrecord_or_enqueuereturnsQueued(transient ledger failure — block buffered for retry), the SV2 path returns a fatal error that terminates the connection without sendingSubmitShares.Success. AQueuedoutcome is not fatal — the block is durably buffered. This discards the connection for a found block under legitimate ledger load, causing the miner to lose credit visibility. The SV1 path handles an analogous scenario without disconnecting.
Previously posted comment on ledger.rs line 679: The analysis in that comment concluded there is no defect (the code is internally consistent). Please disregard it.
The ledger accounting logic (pool-ledger/src/store.rs), the coinbase construction (crates/mining/src/coinbase.rs), block candidate assembly (crates/mining/src/template.rs), and the SV1 session state machine (crates/stratum/src/sv1/session.rs) all appear correct. The BlockRecordPermit reservation accounting is balanced. The AccountResolver login parsing is consistent.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
reap_finished discarded panicked connection workers, letting shutdown succeed after a panic. Retain a service-level panic flag, set it on any reaped or joined panic, join remaining workers, and return the panic through stop_join, matching SV1 sessions_panicked and POOL-09. Add reaped-panic and join-remaining regressions.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b324d32df7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0938c6c151
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e0294bd97c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A retained payout block whose depth falls below payout_confirmations must not stay Confirmed and paid. Require sufficient confirmations on the Confirmed path and let Decision::Revert restore the reservation.
Reserve SHARE + FRAMING + MAX_BLOCK_BYTES before validation and enforce the shared byte invariant in commit_reserved, shedding with OutboxFull past capacity instead of admitting raw bodies over the bound.
An Undetermined submission released the global block reservation before retaining the full PendingBlock. Keep recovery entries charged until removed via DeferredCharge (Permit transfer for Durable, per-entry SeqCst hold sharing the compact queue slot for Queued with token-identity drain), including after accounting is durable. Linearizable capacity under one lock; snapshot-isolated ledger; at-least-once idempotent delivery.
Bounded reconcile pages materialized whole tables before offset/limit. Use bounded snapshot scans with a key cursor (page_after limit+1 seek) so each page bounds storage reads and allocations. Linearizable seeks; snapshot isolation per commit; idempotent retry from cursors.
The count cap alone admits 2,500 P2TR/P2WSH outputs (~430,000 WU) before funding inputs and signatures, exceeding the 400,000-WU standard limit. Track the serialized-output weight ((8 + varint + script) * 4) while reserving and stop before exceeding 400,000 WU minus funding headroom.
With txindex but no script index, a confirmed replacement payout aborted reconciliation: find_dead_spend errored on the original transaction after it left the mempool. Collect transaction confirmation evidence first and skip spend observations when one batch transaction conclusively confirms, so missing abandonment evidence never blocks authoritative confirmation.
|
Sweep triage: this PR's 5 failing checks (
A branch with the mechanical fixes (fmt + lockfile + the mechanical clippy hits) is pushed at |
|
@codex review |
Apply rustfmt to the branch so cargo fmt --all -- --check matches the CI formatter. No behavior changes.
Remove the remaining clippy errors under the CI feature set: backticked LevelDB in the doc, safe length conversions, a non-lazy ok_or, map_or, a moved tests module, and the redundant clone in the bundle test. No behavior change.
Add the codec result aliases, use map_or and is_multiple_of, merge the duplicate missing-body arm, scope the test-only BodyInterest import, and remove same-type conversions. No behavior change.
Move the line-count expectation to materialize, use map_or for the staged timestamp, and format the block hash directly in the offline-import test manifest. No behavior change.
Remove unreachable helpers, gate test-only seam contracts, and route the negotiated AddrV2 reply through the overlay-filtered gossip projection. Route denial now gates feeler, restart, and candidate selection. The p2p library suite still fails six inherited tests at base 9537779: addrman persistence and v8 preservation expectations, SAM CANT_REACH_PEER and TIMEOUT failure accounting, service start cancellation, and failed-start teardown. This commit does not repair that separate contract debt.
Regenerate the two standalone lockfiles for the branch dependency changes. Both pass cargo metadata --locked; the template-provider tests and clippy pass.
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
## Summary Main `a60a704c0` fails CI after #1695. This branch repairs each failure at its source: - Storage clippy: replace the one-iteration prefix loop with `iterator.next()`. - UTXO consensus: `hash_serialized_3` and the decoder commitment use numeric vout order, matching Core `std::map<uint32_t, Coin>`. Core v2 file bytes stay in coins-DB cursor order. - P2P contracts: classify AddrMan format states explicitly, preserve unmarked pre-release files without migration, stabilize the SAM command collector, and stop `release_package_info` from re-requesting riding parents. - Node tests: set the required `dns_seeds_enabled = false` in the overlay routing fixture. - RPC identity: `getpeerinfo` renders the typed anonymous handle, and `disconnectnode` accepts it through the peer table. Scan budget tests assert the actual expansion-budget error. ## Commits | Commit | Concern | |---|---| | `052ccb650` | storage clippy | | `13b421bbf` | UTXO commitment order | | `a651fc35f` | p2p contracts | | `2e27f6f01` | node fixture | | `30b9ea138` | RPC anonymous handles | | `937943b8f` | RPC contract docs | ## Verification | Gate | Result | |---|---| | `cargo fmt --all -- --check` | pass | | `scripts/ci-pr.sh clippy` | all 5 profiles pass | | Full-node clippy, `rocksdb,fjall,redb,kernel` | pass | | `test-crates` | p2p 1291, node 360, RPC no-default 728, all profiles pass | | `test-workspace` | 1535 and 737 tests pass | The full-node clippy run used a disposable libclang 22 shim because this host lacks CI's libclang 13. GitHub CI remains the authoritative full-node proof. ## Sync impact The UTXO change restores the assumeutxo `hash_serialized_3` commitment by hashing numeric vout order. It affects snapshot validation, not peer synchronization. Relevant clauses: `SYNC-01` and `SYNC-05`. Smallest proofs: - `crates/utxo/tests/snapshot_v4_golden.rs` - `core_snapshot::tests::core_file_cursor_order_and_hash_numeric_order_diverge_at_16512` The p2p changes are test contracts, package relay release, and RPC identity surfaces. They do not touch an authoritative sync, validation, or commit path. See also: #1695
Draft subsystem stack split from PR #1687.
Base: f024fc3
Tip: 86a5800
Full repository gate: native-gate-20261027T191330Z-1920985 (scripts/ci-pr.sh all, exit 0).
Scope: mining and pool (SV1, SV2, pool ledger, payout).
This is a verified stack tip. Do not rebase or reparent it
without owner approval.
No better-than-Core claim. No mainnet qualification.
See also: #1687
Do not merge until SV2 reliably sends changed parents and payout checks cannot release funds from mixed-chain observations.
Findings
Summary
Adds the native SV1/SV2 pool, durable share accounting, and external payout signing. It also changes block payload ownership, checkpoint recovery, index seeding, and portable index bundles.
Diagram
sequenceDiagram participant Worker as Pool refresh worker participant Book as Shared job book participant Socket as Existing SV2 connection participant Miner Worker->>Book: "Install job with new parent, clean_jobs=true" Worker->>Book: "Refresh unchanged job, clean_jobs=false" Socket->>Book: Read current_installed Book-->>Socket: "Latest job, clean_jobs=false" Socket->>Miner: Active job without new parent Miner->>Socket: Share hashed with old parent Socket-->>Miner: Share rejectedReviews (6) · Last reviewed commit: "Update lockfile after main integration" · Reviewed by Greptile