NethermindEth / NethermindEth/pluto

Revisit the SSZ/JSON decode-probing fallback in the parsigex codec

Open
#640 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

enhancement rust
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 gets ParSigExCodecError::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.rs is 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 the SignedData-to-enum refactor (#639), which makes the encode side an exhaustive match.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.