Skip to content

fix(cluster): accept charon's null/omitempty cluster-file shapes - #581

Open
varex83agent wants to merge 2 commits into
mainfrom
fix/cluster-serde-defaults
Open

fix(cluster): accept charon's null/omitempty cluster-file shapes#581
varex83agent wants to merge 2 commits into
mainfrom
fix/cluster-serde-defaults

Conversation

@varex83agent

Copy link
Copy Markdown
Collaborator

Problem

pluto's versioned cluster-definition and distributed-validator structs are missing serde defaults for several fields that charon legitimately emits as JSON null or omits entirely. As a result, serde rejects cluster files that charon writes.

Concretely, charon (v1.7.1 defaults to writing cluster-file v1.10, and also reads v1.8/v1.9) can produce:

  • "operators": null and "validators": null — Go marshals a nil slice as null, and these fields have no omitempty, so the key is always present but may be null.
  • an absent timestamp and absent name — both omitempty, so the key is dropped when empty.
  • an absent public_shares and absent partial_deposit_data on distributed validators — both omitempty.

#[serde(default)] alone handles absent keys but not an explicit null, so the null-marshaled slices additionally need DefaultOnNull.

Fix

  • operators, validators (definition v1.8 / v1.9 / v1.10): #[serde(default)] + #[serde_as(as = "DefaultOnNull")] — accepts both null and an absent key, deserializing to an empty vec. Mirrors the existing deposit_amounts handling.
  • timestamp, name (v1.8 / v1.9 / v1.10): #[serde(default)]. (v1.10 already had it for name; v1.8/v1.9 did not.)
  • public_shares, partial_deposit_data (DistValidatorV1x8orLater): #[serde(default)].

All three definition versions are fixed because charon reads v1.8/v1.9/v1.10 (cluster/version.go: supportedVersions).

Notes

  • The existing Definition deserializer enforces num_validators == validators.len(), so validators: null is only valid alongside num_validators: 0 — which is exactly what charon emits. The test reflects this.
  • Serialization is unchanged: DefaultOnNull's SerializeAs forwards to the normal serializer, so non-empty slices still serialize as arrays.

Tests

  • definition_accepts_null_slices_and_absent_omitempty_fields drives the v1.8/v1.9/v1.10 fixtures with operators/validators set to null and name/timestamp removed, asserting they parse to empty/default.
  • dist_validator_v1x8_accepts_absent_omitempty_fields strips public_shares and partial_deposit_data from a real lock fixture's validator and asserts it parses.

cargo test -p pluto-cluster (98 tests), clippy, and fmt all pass.

🤖 Generated with Claude Code

varex83agent and others added 2 commits August 3, 2026 13:19
pluto's versioned cluster-definition and distributed-validator structs
lacked serde defaults for several fields that charon legitimately emits as
JSON `null` or omits entirely, so deserialization rejected files charon
writes:

- `operators` and `validators` — charon has no `omitempty` on these, so a
  nil slice marshals as `null`. Added `#[serde(default)]` plus
  `DefaultOnNull` so both `null` and an absent key deserialize to an empty
  vec (charon v1.7.1 defaults to writing v1.10 but also reads v1.8/v1.9,
  so all three definition structs are fixed).
- `timestamp` and `name` — `omitempty`, so absent when empty. Added
  `#[serde(default)]` (v1.10 already had it for `name`; v1.8/v1.9 did not).
- `public_shares` and `partial_deposit_data` on `DistValidatorV1x8orLater`
  — `omitempty`, so absent when empty. Added `#[serde(default)]`.

Root cause is a missing `#[serde(default)]` (and `DefaultOnNull` for the
null-marshaled slices) on fields charon can omit or null out. Tests drive
each definition version and the distributed validator through the
null/absent shapes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@emlautarom1 emlautarom1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The underlying problem is not fully solved:

Checked the corpus at crates/cluster/src/examples/ using this branch:

                              main             this PR
cluster-definition-000..006   PARSES (all 7)   PARSES (all 7)
cluster-lock-000              FAILS -> invalid type: null, expected a string
cluster-lock-001              FAILS -> invalid type: null, expected a string
cluster-lock-002              FAILS -> missing field `name`
cluster-lock-003              FAILS -> invalid value: string "", expected hex bytes convertible to target type

All seven definitions passed before this PR too, so the corpus result is unchanged end to end. Charon's own TestExamples round-trips and signature-verifies every one of these files.

The default-written v1.10 shape also still fails, with this branch applied:

baseline (unmodified)                            PARSES
distributed_validators: null                     FAILS -> invalid type: null, expected a sequence
builder_registration.message.fee_recipient = ""  FAILS -> invalid value: string ""...
builder_registration.message.pubkey = ""         FAILS -> ...
builder_registration.signature = ""              FAILS -> ...
deposit_data[0].pubkey = ""                      FAILS -> ...

This is due to the following:

  1. lock.rs isn't touched. LockV1x8orLater.distributed_validators has no default, and Go marshals the nil slice as null with no omitempty, so charon's own empty-cluster lock is rejected. Same fix as the rest of this PR.

  2. A second root cause the PR doesn't cover: [u8; N] hex fields reject "" while Vec<u8> hex fields accept it, and charon's to0xHex(nil) returns exactly "". That accounts for cluster-lock-003 and all four builder-registration/deposit failures above.

  3. The pre-v1.8 structs are in scope in practice. The description says charon reads v1.8/v1.9/v1.10, but supportedVersions lists v1_0 through v1_10. cluster-lock-000/001 (v1.1.0, config_signature: null) and -002 (v1.2.0, absent name) are real files that fail today.

I suggest to mirror Charon's TestExamples criteria: parse every file in crates/cluster/src/examples/ raw, locks included (only definitions are processed today).

Feel free to fold the changes onto this PR or create follow ups.

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