Fix round-skip wedge in drand write_pulse#1
Open
loom-agent wants to merge 1 commit into
Open
Conversation
`write_pulse` is an unsigned, feeless extrinsic whose only round gate was `pulse.round > last_stored_round`, with no upper bound. A single accepted pulse could therefore leap `LastStoredRound` from L to R, leaving rounds L+1..R-1 permanently un-storable (every later pulse must exceed `last`), which wedges the CR-v3 weight reveals and the metadata timelocks that reference those skipped rounds. Bound the advance at two layers: - dispatch (`write_pulse`): once a baseline is anchored, require `pulse.round == last_stored_round + 1`. The offchain worker already submits a contiguous run starting at `last + 1`, one single-round transaction at a time, so this stays liveness-safe while making a leap impossible. The first-storage anchor (last == 0 && oldest == 0) still accepts any round > 0, matching the OCW which seeds `last` to `current_round - 1` before submitting the first pulse. - mempool (`validate_unsigned`): drop any round more than `MAX_PULSES_TO_FETCH` ahead of `last`, so leap attempts are rejected before they reach dispatch. The bound equals the OCW's largest catch-up run, so legitimate catch-up is unaffected. spec_version bumped to 426 for the dispatchable behavior change. Refs RaoFoundation#2794
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hi! I am Loom Agent - an autonomous AI agent set up by my maintainer to help contribute to this repository. This pull request was prepared autonomously under human supervision. Feedback is very welcome; I will do my best to address review comments promptly.
Release Notes
write_pulsecall could advanceLastStoredRoundpast the rounds in between, permanently wedging the weight reveals and metadata timelocks that referenced the skipped rounds. The per-call round advance is now bounded to exactly one round, and leap attempts are rejected at the mempool.Description
pallets/drandwrite_pulseis an unsigned, feeless extrinsic. Its only round gate waspulse.round > last_stored_round, with no upper bound, so a single accepted pulse could advanceLastStoredRoundfromLtoR. RoundsL+1..R-1could then never be stored again (every later pulse must exceedlast), and any reveal or metadata timelock targeting those rounds fails to resolve its pulse and is dropped or locked indefinitely. This is most likely during chain lag, an offchain-worker HTTP outage, or a sync stall, whenlastfalls far behind the real drand round.Root cause: the dispatch round check in
write_pulseonly enforced a lower bound, and the mempool validatorvalidate_unsignedonly dropped rounds<= last. Neither bounded how far a single pulse could jumpLastStoredRound.Fix (two-layer bound):
write_pulserequirespulse.round == last_stored_round + 1. The offchain worker already submits a contiguous run starting atlast + 1, one single-round transaction at a time (pulses: vec![pulse]per tx, ascending rounds), so legitimate operation is unaffected and a leap becomes impossible. The first-storage anchor (no pulse stored yet) still accepts any round > 0, matching the OCW, which seedslasttocurrent_round - 1before submitting the first pulse.validate_unsignednow drops any round more thanMAX_PULSES_TO_FETCH(50) ahead oflast, so leap attempts are rejected before dispatch. The bound equals the OCW's largest single catch-up run, so legitimate catch-up stays valid.This closes the wedge at its root: a single pulse can no longer skip rounds. The reveal-sink resilience mentioned in the issue is complementary defense-in-depth and is intentionally left for a follow-up, since it is not needed once the leap is impossible.
transaction_versionis unchanged (thewrite_pulsesignature is unchanged);spec_versionis bumped 425 -> 426 for the dispatchable behavior change.Related Issue(s)
LastStoredRoundto skip rounds, permanently denying timelock reveals RaoFoundation/subtensor#2794Type of Change
Breaking Change
Non-breaking for honest operation. The offchain worker only ever submits consecutive rounds (
last + 1, last + 2, ...), so the new strict== last + 1dispatch rule and theMAX_PULSES_TO_FETCHmempool cap accept every legitimate pulse. The only pulses newly rejected are non-consecutive leaps, which cannot originate from the OCW.spec_versionis bumped (425 -> 426) so nodes upgrade in lockstep;transaction_versionis unchanged.Testing
Built and tested with the project's pinned Rust 1.89.0 toolchain (
rust-toolchain.toml), using the CI feature flags:cargo fmt --all -- --check-> cleancargo clippy -p pallet-drand --all-features --all-targets -- -D warnings-> no warningscargo test -p pallet-drand --all-features-> 25 passed, 0 failed(These are the formatting/lint/test gates that
./scripts/fix_rust.shapplies to the changed crate; the full script additionally runszepterand a workspace-wide build.)New tests in
pallets/drand/src/tests.rs:write_pulse_rejects_round_skip: a BLS-valid pulse for round 1000 is rejected whenlast = 998(a skip of 2), and storage is left unchanged.write_pulse_accepts_consecutive_round: round 1000 withlast = 999is accepted, confirming the strict rule does not reject the legitimate next round.validate_unsigned_rejects_round_too_far_ahead: a round oflast + MAX_PULSES_TO_FETCH + 1is dropped at the mempool.The two wedge tests were verified to FAIL on the pre-fix source (reverting only the pallet source, keeping the new tests) and PASS with the fix:
23 passed; 2 failed25 passed; 0 failedChecklist
./scripts/fix_rust.shto ensure my code is formatted and linted correctlyScreenshots (if applicable)
N/A (no UI change).
Additional Notes
pallets/drand(dispatch + mempool validation) plus thespec_versionbump; no other pallet calls the affected functions.spec_version(425 -> 426) may need adjustment to coordinate with other in-flight runtime PRs.