Skip to content

Prove collect-rewards against the head, not the finalized block - #152

Merged
n13 merged 2 commits into
mainfrom
n13/proof-block-head
Sep 2, 2026
Merged

Prove collect-rewards against the head, not the finalized block#152
n13 merged 2 commits into
mainfrom
n13/proof-block-head

Conversation

@n13

@n13 n13 commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Problem

collect-rewards failed on staging mainnet with:

Error: Failed to get ZK Merkle proof for leaf 1780: Error: Leaf index 1780 not found in ZK tree at block 0x4d5336...
Failed after 107.12s

The chain is QPoW, so finality trails the head by a long way — measured at ~100 blocks on staging mainnet. collect_rewards proved against chain_getFinalizedHead, so any leaf minted inside that window is not yet in the tree at the proof block.

Confirmed against the live chain: the failing block 0x4d5336… is block 1730, and leaf 1780 did not settle until block 1750. getMerkleProof(1780, 1730) returns null; getMerkleProof(1780, 1750) returns a proof. Nothing was corrupt — the leaf simply did not exist yet at the block being proved against.

It also worked on the dev chain, which finalizes immediately, so the window never opens there.

Changes

Prove against the head. Every other read path in the CLI already uses the latest block; this one was the outlier. The existing comment was right that a best-block proof can be reorged out, but on PoW the answer to that is re-running the proof, not waiting ~100 blocks. --at-block still pins a specific block.

Check settlement before generating proofs. The run above died on proof 66 of 1809 after 107 seconds. Leaves settle in index order, so provability is monotonic: one probe of the highest selected leaf covers the common case, and a binary search finds the boundary otherwise. Same outcome, about a second.

Warn and skip instead of aborting. Leaves that are still too new are reported and dropped so one recent transfer cannot hold up an entire sweep. If nothing is provable, the error now says why and what to do.

try_get_zk_merkle_proof. Returns Ok(None) for an unsettled leaf so callers can tell that apart from an RPC failure. get_zk_merkle_proof is now a thin wrapper over it, so the RPC call is not duplicated.

Testing

  • cargo build --release clean
  • cargo clippy --release --all-targets clean
  • cargo test --release --lib — 324 passed, 0 failed

Behaviour verified against the live staging chain by direct RPC (block-by-block binary search for the settlement boundary of leaf 1780) rather than by re-running the sweep.

Not included

Two related issues live in other repos and are deliberately out of scope: runtime/src/apis.rs collapses LeafIndexOutOfBounds and LeafNotYetSettled into a single None, which is why the original error could not say which had happened; and zkTree_getState takes no block argument while silently accepting one.

…zed block

QPoW finality trails the head by a long way (~100 blocks on staging mainnet), so
any leaf minted inside that window is absent from the ZK tree at the finalized
block. collect_rewards proved against chain_getFinalizedHead, which made recent
rewards unsweepable and failed with "Leaf index N not found in ZK tree at block
H" part-way through proof generation.

Prove against the head instead, matching every other read path in the CLI. A
proof invalidated by a reorg is re-run, which is far cheaper than waiting for
finality. --at-block still pins a specific block for callers who want one.

Also check settlement before generating proofs rather than discovering it on the
Nth one: leaves settle in index order, so one probe of the highest selected leaf
covers the common case and a binary search finds the boundary otherwise. Leaves
that are still too new are now reported and skipped instead of aborting the whole
sweep, and the all-unsettled case gets an error that says what to do about it.

Splits get_zk_merkle_proof into try_get_zk_merkle_proof, which returns Ok(None)
for an unsettled leaf so callers can tell that apart from an RPC failure.
@n13 n13 added the bot-review Request automated review from review-bot label Sep 2, 2026

@n13 n13 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewer model: GPT Sol

Verdict (advisory): Request changes

Blocking findings:

  1. src/collect_rewards_lib.rs:434 filters unsettled leaves only after the --amount selection at lines 351-359 has stopped and discarded the remaining candidates. If a high-value recent leaf is selected first, this can now error with no provable transfers even though an omitted settled transfer could satisfy the request; with a mixed selection it can submit less than the requested amount and still report success. For example, a requested amount of 50 with an unsettled 100 transfer and an older settled 60 transfer selects only the 100 transfer, removes it here, and errors instead of withdrawing the available 60. Filter all unspent transfers to the provable prefix before amount selection, or backfill from settled unselected transfers and revalidate the requested amount after filtering.

  2. The exact head is not formatted with the repository-pinned toolchain. cargo +nightly-2026-08-31 fmt --all -- --check reports diffs in both changed files (src/cli/wormhole.rs:204, src/collect_rewards_lib.rs:59, and src/collect_rewards_lib.rs:369). The live Fast Checks job fails on the same diffs, so every downstream CI job is skipped. Please apply the pinned formatter.

Validation on exact head 471536571d3af6866a2bd447c542cdae428f73c1:

  • git diff --check — passed.
  • taplo format --check --config taplo.toml — passed.
  • cargo +nightly-2026-08-31 fmt --all -- --check — failed as described above.
  • SKIP_CIRCUIT_BUILD=1 cargo test --release --locked --lib collect_rewards_lib::tests — 15 passed.
  • SKIP_CIRCUIT_BUILD=1 cargo clippy --release --all-targets --locked -- -D warnings — passed.
  • Full release library run compiled and passed 323 tests; its sole failure was the pre-existing artifact-dependent verifier test because the isolated review run intentionally skipped circuit-bin generation.

No test in this patch exercises settlement filtering together with --amount; please add coverage for backfilling/amount preservation when the initially selected high-value leaf is unsettled.

@n13 n13 removed the bot-review Request automated review from review-bot label Sep 2, 2026
The settlement filter ran after the --amount selection, so a large
not-yet-settled leaf could be selected, then dropped, leaving the sweep
short of the requested amount or failing outright while an older settled
transfer could have covered it.

Establish the settlement boundary before selecting: resolve the proof
block, probe the boundary over all unspent transfers, then run the
largest-first selection over the settled ones only. The requested amount
is validated against the provable total, and the shortfall error says how
many transfers were held back as too new.

Also format with the pinned nightly rustfmt.
@n13 n13 added the bot-review Request automated review from review-bot label Sep 2, 2026

@n13 n13 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewer model: GPT Sol

Verdict (advisory): Approve

No blocking findings.

The follow-up commit resolves both earlier blockers. Settlement is now determined across all unspent leaf indices before largest-first amount selection, so a recent unsettled leaf cannot crowd an older provable transfer out of the withdrawal set. The requested amount is rechecked against settled funds, with focused coverage for backfilling and an insufficient settled balance. The pinned formatting failure is also resolved.

I traced the fixed-block proof path into the node/runtime implementation as well: zkTree_getMerkleProof honors the supplied block hash, and the runtime exposes a monotonic processed-leaf prefix, which supports the boundary search used here. The existing public get_zk_merkle_proof error behavior remains intact through the new optional wrapper.

Validation on exact head 2932cd1c47289711a6cbd049ca8419be422c133f:

  • git diff --check - passed.
  • taplo format --check --config taplo.toml - passed.
  • cargo +nightly-2026-08-31 fmt --all -- --check - passed.
  • cargo metadata --locked --no-deps --format-version 1 - passed.
  • SKIP_CIRCUIT_BUILD=1 cargo test --release --locked --lib collect_rewards_lib::tests - 19 passed.
  • SKIP_CIRCUIT_BUILD=1 cargo clippy --release --all-targets --locked -- -D warnings - passed.
  • Hosted CI - all current checks passed, including Ubuntu/macOS builds and tests, examples, strict analysis/docs, formatting, security audit, and dependency cooldown.

@n13 n13 removed the bot-review Request automated review from review-bot label Sep 2, 2026
@n13
n13 merged commit 0083796 into main Sep 2, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant