Skip to content

Raptorcast: fix commitment - #3248

Open
xinyuan-dev wants to merge 2 commits into
masterfrom
xinyuan/fix-raptorcast-commitment
Open

xinyuan-dev wants to merge 2 commits into
masterfrom
xinyuan/fix-raptorcast-commitment

Conversation

@xinyuan-dev

@xinyuan-dev xinyuan-dev commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

This PR implements the validation of secp256k1 signature and restrict deterministic raptorcast commitment to a subset of headers that's relevant to encoding scheme, excluding the signature itself.

Copilot AI lite review requested due to automatic review settings September 8, 2026 11:51
@xinyuan-dev
xinyuan-dev requested a review from a team September 8, 2026 11:52

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

A few changes are needed to avoid avoidable panics and to correct/clarify guarantees (notably signature “unique byte representation” wording and use of constants in tests).

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR tightens signature canonicalization by rejecting high-S secp256k1 signatures at deserialization time, and adjusts deterministic raptorcast commitment logic to commit only to a deterministic subset of chunk header fields (including a coarse timestamp bucket) relevant to the encoding scheme.

Changes:

  • Enforce low-S secp256k1 signatures during SecpSignature::deserialize, updating tests accordingly.
  • Make raptorcast commitments deterministic by committing to (round, global_merkle_root, encoding_scheme_variant, app_message_len, coarse_ts) rather than signature bytes.
  • Introduce EncodingScheme::variant() and a shared timestamp bucketing helper (TIMESTAMP_BUCKET_MS / coarse_ts) for deterministic encoding inputs.
File summaries
File Description
monad-secp/src/secp.rs Reject high-S signatures at deserialization; update malleability test to fail at decode time.
monad-raptorcast/src/util.rs Add EncodingScheme::variant() helper to expose the header variant byte.
monad-raptorcast/src/round_info.rs Redefine commitment claim to a deterministic header subset and expand commitment conflict tests.
monad-raptorcast/src/packet/deterministic.rs Add timestamp bucketing constant/function and use it in seed derivation.
Review details

Suppressed comments (1)

monad-raptorcast/src/round_info.rs:540

  • This test hard-codes the next timestamp bucket boundary (2048). Prefer referencing deterministic::TIMESTAMP_BUCKET_MS so the test remains correct if the bucket size changes.
            .try_commit(&dummy_chunk(10, &MERKLE_A, LEN_A, 2048))
  • Files reviewed: 4/4 changed files
  • Comments generated: 3
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread monad-raptorcast/src/round_info.rs
Comment thread monad-raptorcast/src/round_info.rs Outdated
Comment thread monad-secp/src/secp.rs Outdated
@xinyuan-dev
xinyuan-dev force-pushed the xinyuan/fix-raptorcast-commitment branch from 1d339ee to 2026c88 Compare September 8, 2026 12:04
marcograss
marcograss previously approved these changes Sep 8, 2026
Comment thread monad-raptorcast/src/round_info.rs Outdated
@xinyuan-dev
xinyuan-dev marked this pull request as draft September 9, 2026 07:51
@xinyuan-dev
xinyuan-dev force-pushed the xinyuan/fix-raptorcast-commitment branch from 2026c88 to bc5d30e Compare September 9, 2026 16:40
@xinyuan-dev
xinyuan-dev marked this pull request as ready for review September 9, 2026 16:40
Comment thread monad-secp/src/secp.rs
Comment thread monad-raptorcast/src/util.rs
Comment thread monad-raptorcast/src/round_info.rs
Comment thread monad-raptorcast/src/packet/deterministic.rs
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.

4 participants