Skip to content

refactor(admin): key envelope payload decoding off the SPS-50 tx type - #206

Open
prajwolrg wants to merge 2 commits into
mainfrom
refactor/admin-tx-payload-format
Open

refactor(admin): key envelope payload decoding off the SPS-50 tx type#206
prajwolrg wants to merge 2 commits into
mainfrom
refactor/admin-tx-payload-format

Conversation

@prajwolrg

@prajwolrg prajwolrg commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

Description

Re-implements admin envelope payload parsing so the SPS-50 tag's tx type byte selects the concrete action type to decode, instead of SSZ-decoding the envelope into SignedPayload's nested action unions. The old format re-encoded what the tag already says: it spent redundant union selector bytes on L1, left the tag byte entirely unchecked, and tied the wire format to Rust enum ordering — the main obstacle to constructing admin txs from other languages (STR-2366). Each tx type now has a flat, self-describing SSZ schema ({seqno, action, signatures}), and parse_tx returns Option with warn-and-skip semantics, matching the bridge parser.

Security-wise the tag byte becomes load-bearing but is still not signed; that's safe because signatures cover the signing message rendered from the reconstructed action, so a mis-tagged payload either fails SSZ decode or fails threshold verification in the handler. This is noted in the code docs.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature/Enhancement (non-breaking change which adds functionality or enhances an existing one)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactor
  • New or updated tests
  • Dependency update
  • Security fix

Notes to Reviewers

Review entry point: crates/subprotocols/admin/txs/src/parser.rs — the wire container SignedActionPayload<A>, the tx-type-keyed decode dispatch, and SignedPayload::into_envelope_bytes() (encode side). Everything else is fallout: callers switching to the Option flow and encode sites switching off the removed SSZ derives.

This is a consensus wire-format break for admin txs: already-signed-but-unbroadcast payloads in the old encoding won't parse, and external tooling that builds these envelopes must adopt the new layout.

Tested with the parser unit tests (new roundtrip over arbitrary actions + malformed-payload rejection) and the full admin integration suites (asm_admin, asm_admin_to_{checkpoint,bridge,stf,ee_stf}), all green.

Checklist

  • I have performed a self-review of my code.
  • I have commented my code where necessary.
  • I have updated the documentation if needed.
  • My changes do not introduce new warnings.
  • I have added tests that prove my changes are effective or that my feature works.
  • New and existing tests pass with my changes.

Related Issues

STR-2366

@codecov

codecov Bot commented Jul 27, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.26752% with 9 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
crates/subprotocols/admin/txs/src/parser.rs 93.44% 8 Missing ⚠️
.../subprotocols/admin/subprotocol/src/subprotocol.rs 75.00% 1 Missing ⚠️
Files with missing lines Coverage Δ
.../subprotocols/admin/txs/src/actions/updates/mod.rs 98.82% <100.00%> (+6.09%) ⬆️
...rates/subprotocols/admin/txs/src/test_utils/mod.rs 90.10% <100.00%> (ø)
.../subprotocols/admin/subprotocol/src/subprotocol.rs 84.00% <75.00%> (+2.51%) ⬆️
crates/subprotocols/admin/txs/src/parser.rs 94.28% <93.44%> (+20.37%) ⬆️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Base automatically changed from refactor/subprotocol-init-configs to main July 27, 2026 05:05
Only OperatorSet and Sequencer had conversions; the tx-type-keyed
envelope parser needs every concrete update type convertible so its
decode dispatch can stay generic.
The envelope was SSZ-decoded directly into SignedPayload, whose nested
action unions re-encode what the tag's tx type byte already says. That
left the tag byte unchecked, spent redundant selector bytes on L1, and
tied the wire format to Rust enum ordering — awkward for constructing
admin txs from other languages (STR-2366).

Decode into a per-type SignedActionPayload<A> container selected by the
tag instead, giving every tx type a flat, self-describing SSZ schema.
SignedPayload stays as the in-memory handler type; it loses its SSZ
derives so the old discriminant-carrying format can't be emitted by
accident, and encoding moves to into_envelope_bytes().

parse_tx now returns Option like the bridge parser, warning and
skipping unknown or malformed txs at one shared log site.

A mis-tagged payload can't forge an action: signatures cover the
signing message rendered from the reconstructed action, so a wrong tag
either fails SSZ decode or fails threshold verification.

This is a wire-format break: already-signed-but-unbroadcast admin
payloads in the old encoding will no longer parse.
@prajwolrg
prajwolrg force-pushed the refactor/admin-tx-payload-format branch from 71fb018 to 192f9f6 Compare July 27, 2026 05:07
@prajwolrg
prajwolrg marked this pull request as ready for review July 27, 2026 05:10
@github-actions

Copy link
Copy Markdown

Commit: e2ae102
SP1 Execution Results

program cycles gas
asm-stf 136,410,021 134,124,666
moho 5,223,535 5,525,318

@prajwolrg
prajwolrg requested review from irnb and storopoli July 27, 2026 16:01

@storopoli storopoli left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ACK 192f9f6

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.

2 participants