NethermindEth / NethermindEth/pluto
Revisit the SSZ/JSON decode-probing fallback in the parsigex codec
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 8
- Forks
- 5
- Avg merge
- 4d 16h
- Merged PRs (30d)
- 37
Description
Summary
Partial-signature payloads carry no encoding tag — the protobuf envelope is duty + opaque bytes — so deserialize_signed_data reconstructs the type by probing: per duty type, up to 2 SSZ shapes then (if the bytes look like JSON) 2 JSON shapes. ssz_codec layers a second probe underneath (20-byte vs 12-byte versioned-attestation headers), so one Attester payload can be tried five ways; first successful parse wins. This is a faithful port of Charon's core/proto.go marshal/unmarshal.
Problems:
- Error swallowing: 13
if let Ok(...)sites discard the per-attempt errors; when everything fails the caller getsParSigExCodecError::UnsupportedDutyType— misleading, since the duty type was supported and the bytes just didn't parse. There is no way to see "SSZ failed because X, JSON failed because Y". - Ambiguity is designed in: nothing guarantees the candidate shapes are mutually exclusive; a phase0 decode can accept bytes meant as versioned and get silently re-wrapped.
- Charon has a per-test kill switch (
DisableSSZMarshallingForT); Pluto has no equivalent, so the JSON decode branch is effectively unreachable in tests for SSZ-capable types.
Constraint
The wire format must interoperate with Charon in mixed clusters. An explicit encoding tag in the envelope deviates from Charon's parsigex protocol and is only viable behind protocol negotiation or as an upstream change — the probing itself cannot simply be removed.
Proposed change
- Short term: stop swallowing errors — accumulate each attempt's failure into the returned error (and a debug log), and rename the terminal variant to reflect "no codec accepted the payload". Land decode-rejection tests with it: today a single rejection branch in
unsigneddata.rsis tested (rejects_empty_set) — the SSZ/JSON fallback failures, the length/offset guards, and the codec's per-duty-type error branches are all unasserted, so the error-accumulation change would ship blind. - Medium term: replace the hand-written probe chains with a per-duty-type table of candidate codecs declared in one place, so encode (
serialize_signed_data's downcast ladder) and decode stay symmetric. This composes with theSignedData-to-enum refactor (#639), which makes the encode side an exhaustive match.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in crates/core/src/parsigex_codec.rs at deserialize_signed_data and its SSZ/JSON probing branches, then inspect the rejection path and rejects_empty_set coverage in unsigneddata.rs. Trace the per-duty-type decode and serialize_signed_data paths before deciding how the candidate codecs should be represented. Done means failed attempts are visible in the terminal error and debug log, with tests covering fallback failures, length/offset guards, and per-duty-type branches.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- blockchain, distributed-systems, testing-qa
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100