Add checkpoint sync SIP - #96
Conversation
4e6fd1e to
ad29671
Compare
| - Cluster member set. A ValidatorAdded event uses the `operatorIds` order to pair share bytes with operators, as defined in section 2. After parsing, the cluster membership is represented as a set and serialized in ascending operator_id order. | ||
| - Addresses. All addresses are encoded as their 20 raw bytes when hashed. Any text rendering uses lowercase hex. EIP-55 checksum casing is not used in the hashed form. | ||
| - Fee recipient fallback. If no Owner record exists for an owner, the fee recipient defaults to the owner address. | ||
| - Owner nonce. Owner records serialize `next_validator_nonce`, the nonce value to use for the next ValidatorAdded by that owner. It starts at 0. Every ValidatorAdded log for the owner advances it exactly once before validation, including a malformed or rejected ValidatorAdded. |
There was a problem hiding this comment.
Could this diverge across clients?
Line 62 says an event that "cannot be decoded as its ABI event… changes no record," but this bullet says every ValidatorAdded advances the nonce "including a malformed" one. For a log that fails ABI decode there's also no owner to attribute the advance to — the owner lives inside the decoded payload.
Since the nonce feeds the registration-signature message, two implementers diverging here reproduces the #972-class silent split. Worth pinning: only an ABI-decodable ValidatorAdded (with a known owner) advances that owner's nonce; an undecodable one moves nothing — and reconcile with lines 62 and 77.
| - Operator fee and Cluster accounting fields carried by events are contract bookkeeping, not validator client membership state. | ||
| - Cluster liquidation status is derived from ClusterLiquidated and ClusterReactivated, not from the Cluster.active field. Those events update only an included cluster record. If no cluster record is included for that owner and operator set, the event changes no canonical record. A cluster created later starts with `liquidated = false` unless a later ClusterLiquidated event changes it. | ||
|
|
||
| **Removal representation.** V1 pins removal per entity. A ValidatorRemoved event removes the validator record identified by the event owner and validator public key from `state_root` and removes the encrypted share records for that same owner and validator public key from `share_set_root`. If that was the last active validator in a cluster, the cluster record is also omitted. An OperatorRemoved event sets `removed = true` for that operator while any active cluster still references it. A removed operator with no active cluster reference is omitted from the canonical record set. Active operators are included even if they are not members of an active cluster. |
There was a problem hiding this comment.
Should we pin a single predicate here?
Operator tombstone-vs-omit gates on "active cluster" in this line but "included cluster" on line 67, and neither is defined — in particular whether a liquidated cluster counts.
Cluster omission likewise says "last active validator" here vs "no remaining validators" on line 69, though validators carry no active flag. Two clients reading these differently produce different operators/clusters lists and a different state_root.
Suggest one defined term, e.g. an operator is referenced iff any cluster record present in clusters lists it (liquidation-independent), and a cluster is omitted iff it has zero validator records — then use it everywhere.
|
|
||
| The bundle carries the full active encrypted share set because a fresh operator bootstrap needs its encrypted share and clients may not retain every ciphertext locally. The ciphertexts are public event data; plaintext shares are not part of the checkpoint. | ||
|
|
||
| `state_root` commits to the public validator client registry state. `share_set_root` commits to encrypted share records for validators active at block B. It excludes encrypted shares for validators removed before B because those shares are not needed to bootstrap the current state. |
There was a problem hiding this comment.
What does "active at block B" mean precisely?
This is a third sense of "active" (alongside "active/included cluster" around lines 67/86), and validators carry no active flag. It also interacts with the line-287 reject rule ("any active validator … exactly one share per operator_id"), so a liquidated cluster could be accepted or rejected depending on the reading.
Suggest defining share-set membership identically to validator-record membership — a validator's shares are in share_set_root iff its record is in state_root (present and not ValidatorRemoved) — and dropping "active at block B" / "removed before B".
|
|
||
| 1. Parse exactly one `CheckpointBundleV1` from the received SSZ bytes. Reject malformed SSZ, trailing bytes, or any representation that cannot materialize that single container. If checkpoint distribution includes an optional snapshot_digest, confirm it for download integrity, but do not treat it as a signed trust anchor. | ||
| 2. Confirm the certificate message network_id and ssv_contract_address match the node's configured network and contract. Reject on mismatch. Reject if canonical_spec_version is one the node does not implement, since a root is only meaningful under the canonical state definition that produced it. | ||
| 3. Confirm block B is finalized according to the node's consensus data source. Reject if finality cannot be established. A finalized block can no longer be reverted by a chain reorganization (reorg), so the state at B is permanent. Confirm block_hash matches the finalized block. |
There was a problem hiding this comment.
Do we need to bind the execution-layer log source to the finalized consensus block?
State is folded from eth_getLogs (EL), but finality and block_hash are checked against the consensus source here, and block_hash (line 309, "Hash of block B") doesn't say whether it's the EL block hash or the beacon root.
Without confirming the finalized beacon block's execution_payload.block_hash equals the folded block, "finalized" gives no guarantee about the logs actually folded. Suggest specifying block_hash as the EL block hash and adding that binding step.
| **Canonical encoding and ordering.** The following choices are pinned. | ||
|
|
||
| - Operator identity. Operators are keyed by `operator_id` only. The canonical fold must not deduplicate operators by raw event bytes, decoded RSA key bytes, PEM text, owner address, or any other key. If two OperatorAdded events assign two different operator ids but decode to the same RSA public key, v1 still has two operator records. | ||
| - Operator public key. OperatorAdded publicKey bytes have appeared in more than one layout, so v1 defines `canonical_public_key` explicitly. First, try to decode the field as the SSV operator public key wrapper, equivalent to ABI decoding one dynamic `bytes` value. If that succeeds and the decoded bytes parse as the expected base64 PEM RSA public key payload, those decoded bytes are the canonical bytes. If wrapper decoding fails, the raw event bytes are accepted only if they parse directly as the same base64 PEM RSA public key payload. Otherwise the OperatorAdded event is rejected as malformed. The canonical bytes are hashed exactly as bytes; clients must not reserialize the key into a different PEM, DER, JSON, or text layout before hashing. If a client cannot decode the key into this canonical form, it must stop or mark the event malformed according to the v1 fold, not silently skip the operator. |
There was a problem hiding this comment.
Might be worth pinning the exact hashed byte form here.
"canonical bytes are hashed exactly as bytes" plus the undefined "header and footer normalization" means base64/PEM whitespace and line-wrapping differences change state_root.
Could we specify the precise canonical byte string (base64 alphabet/padding, PEM header/footer, newline treatment) with a worked example and a vector? Also worth stating outright that a wrapper which decodes structurally but whose contents don't parse is malformed — the "Otherwise" clause implies it, but the boundary of "wrapper decoding fails" reads both ways.
|
|
||
| ```text | ||
| CheckpointCertificateMessageV1 = Container[ | ||
| domain: Bytes32, |
There was a problem hiding this comment.
Any reason schema_version has no pinned v1 value or import check?
It sits one transposed character from scheme_version in a normative SSZ field order, is never assigned a value (unlike canonical_spec_version = 1 / scheme_version = 1), and step 4 validates only scheme_version — so an importer can't reject a schema_version it doesn't understand.
Suggest pinning schema_version = 1, adding the import check, and considering a less collision-prone name.
| ] | ||
| ``` | ||
|
|
||
| `MAX_OPERATORS`, `MAX_VALIDATORS`, `MAX_CLUSTERS`, `MAX_OWNERS`, `MAX_OPERATOR_PUBLIC_KEY_BYTES`, and `MAX_CLUSTER_OPERATORS` are fixed SIP constants for `canonical_spec_version = 1`. They set checkpoint encoding limits and must be high enough to cover every state that can exist under the SSV contract. They must be identical across implementations. A client rejects a v1 state that cannot be represented within these limits. The concrete values are part of the v1 conformance vectors. |
There was a problem hiding this comment.
Worth making these constants normative in the SIP itself?
Deferring every MAX_* to the conformance-vector file leaves the SIP not self-contained for interop: they don't affect the hashed bytes (keccak over ssz_serialize, not hash_tree_root), but they do gate accept/reject ("a client rejects a v1 state that cannot be represented within these limits"), so two clients with different caps diverge on the import decision.
Suggest inlining the v1 values or pinning the vector file's hash here as normative — plus a note that MAX_* affect representability only, not the root.
| ] | ||
| ``` | ||
|
|
||
| `MAX_VECTOR_CASE_ID_BYTES`, `MAX_VECTOR_SIGNER_SETS`, `MAX_VECTOR_BUNDLE_BYTES`, and `MAX_VECTOR_SIDE_DATA_BYTES` are fixed test format constants for v1. `bundle_bytes` is the exact byte input passed to the importer. Accepted cases encode one valid `CheckpointBundleV1`. Rejection cases may contain malformed SSZ, duplicate records, records outside canonical membership, bad certificate fields, or bad signatures. `expected_result` uses these v1 codes: |
There was a problem hiding this comment.
Should reject-reason precedence be defined?
Conformance asserts the exact expected_result code, but a bundle that fails multiple ways (e.g. a bad signature and a root mismatch) has no specified winner — the import steps aren't stated to be strictly ordered or short-circuiting.
Two clients could return different codes for the same bundle, producing spurious conformance failures. Suggest declaring the steps strictly ordered with first-failure-wins and mapping that order onto the enum.
|
|
||
| For v1, `MAX_CHECKPOINT_SIGNATURE_BYTES = 256`, and every `CheckpointSignatureV1.signature` must contain exactly 256 bytes. A different length makes the signature record invalid and ignored for signer counting. | ||
|
|
||
| The signed message bytes are exactly the 32 bytes of `certificate_message_hash`. The v1 signature is RSASSA-PKCS1-v1_5 with SHA-256 over those bytes. Equivalently, an implementation signs and verifies the byte string `certificate_message_hash` with the same RSA and SHA-256 procedure used for existing SSV signed messages. No text encoding, hex encoding, EIP-191 wrapper, or additional prefix is applied. |
There was a problem hiding this comment.
Might be worth tightening RSA verification.
"the same RSA and SHA-256 procedure used for existing SSV signed messages" leaves PKCS#1 v1.5 strictness and the public exponent unpinned; a lenient verifier (non-canonical padding, small exponent) is the classic signature-forgery vector.
For a security primitive, suggest requiring strict PKCS#1 v1.5 (exact SHA-256 DigestInfo, full-length padding) and pinning the exponent (e.g. 65537).
Separately, worth stating valid_from_block/valid_to_block are EL block numbers matching block_number.
|
|
||
| **Required meaning.** A certificate attests that the signer independently materialized the canonical public state and active encrypted share set at block B, without trusting the bundle being signed, and computed the signed roots. For a child checkpoint, it also attests that the signer consumed the exact ordered event delta identified by `delta_log_set_hash`. A certificate is not an attestation that a downloaded checkpoint file parsed or that a hosting path is trustworthy. | ||
|
|
||
| **Acceptance rule.** A checkpoint is accepted only when one set of unique active signer ids signs the same certificate message and satisfies `TrustedSignerSetV1`: at least `threshold` unique signers, every required implementation quorum, and `min_controller_count` when nonzero. Duplicate signer ids count once. Unknown, retired, wrong scheme, and invalid signature records are ignored before checking the threshold rules. For v1, the counted set must include both Anchor and go-ssv signers. |
There was a problem hiding this comment.
Could we make the counting rule explicit here?
It would help to state that the threshold, every implementation quorum's min_signers, and min_controller_count are all evaluated against the same deduplicated set of valid active signer ids (pseudocode would remove all doubt).
And since implementation_id/controller_id are self-asserted by the trusted signer set, worth calling out in Security Considerations the explicit assumption that signer-set authors set them to genuinely independent parties — the format can't enforce it.
2 participants
Summary
Adds a SIP for canonical state roots and checkpoint sync.
The SIP specifies the canonical network state, checkpoint payload commitments for public state and encrypted shares, certificate meaning, import verification, conformance tests, and security considerations.
Validation
main; the diff containssips/checkpoint_sync.mdand oneall.mdindex row.