From 1f6bc2af1d9e233af3bd548e8a0b0415c3775ca6 Mon Sep 17 00:00:00 2001 From: Samuel Laferriere <9342524+samlaf@users.noreply.github.com> Date: Thu, 5 Nov 2026 10:56:05 -0500 Subject: [PATCH] fix(rpc): give WithdrawalAddressBound its own error code, 4005 Refs https://linear.app/seismic-systems/issue/SEI-1053 https://github.com/SeismicSystems/summit/pull/474 gave WithdrawalAddressBound code 4004, which Forbidden already had from https://github.com/SeismicSystems/summit/pull/475. Release builds enable permissioned, so both are live there, and a client can't tell "this node is bound to another withdrawal address" from "signer not authorized" by code. Move WithdrawalAddressBound to 4005. Forbidden keeps 4004, since it has been released since v0.1.2 and WithdrawalAddressBound hasn't been released yet. To stop this from happening again, each RpcError code is now its variant's discriminant, under #[repr(i32)], so the compiler rejects a duplicate code (E0081). RpcError::code() reads it back, and the From impl uses that instead of repeating each number. The variants are listed in code order. ## Possible follow-up: ungate the permissioned variants The duplicate check only covers variants compiled into the build. With the four permissioned variants behind #[cfg(feature = "permissioned")], a clash with one of them only fails an all-features build. CI runs one, so it's still caught, but a plain cargo check isn't enough. They're gated because RpcError is private to the crate, so without the feature they'd never be constructed and -D warnings would fail on dead_code. Ungating them with #[cfg_attr(not(feature = "permissioned"), allow(dead_code))] would make every build check every code, and would remove the cfgs from the From match. Left out to keep this change scoped. --- docs/staking-and-participating.md | 2 +- rpc/src/error.rs | 170 ++++++++++++++++++------------ rpc/tests/integration_test.rs | 2 +- 3 files changed, 105 insertions(+), 69 deletions(-) diff --git a/docs/staking-and-participating.md b/docs/staking-and-participating.md index d0c328c8..f80ffc41 100644 --- a/docs/staking-and-participating.md +++ b/docs/staking-and-participating.md @@ -42,7 +42,7 @@ The account's withdrawal address is fixed by its first deposit, or by genesis fo ## Binding the Withdrawal Address -By default, `getDepositSignature` signs deposit data for whatever address the request carries. With `summit run --withdrawal-address-path `, the node signs for one address only, fixed by its first request (or by genesis for a founder) and never changed. Requests for any other address are refused with error `4004`. `getWithdrawalAddress` returns the bound address. +By default, `getDepositSignature` signs deposit data for whatever address the request carries. With `summit run --withdrawal-address-path `, the node signs for one address only, fixed by its first request (or by genesis for a founder) and never changed. Requests for any other address are refused with error `4005`. `getWithdrawalAddress` returns the bound address. Note that the node binds the address only; the key is kept in custody of the operator. The node signs deposit data with its validator keys, and anyone can submit that data from any wallet. diff --git a/rpc/src/error.rs b/rpc/src/error.rs index b5f81618..9032d82e 100644 --- a/rpc/src/error.rs +++ b/rpc/src/error.rs @@ -16,132 +16,168 @@ use jsonrpsee::types::ErrorObjectOwned; /// - 5xxx: internal errors. /// /// Callers should match on the code, not the message; `data` carries details. +/// +/// Each variant's code is its discriminant, so the compiler rejects two +/// variants with the same code (E0081). Keep the variants in code order. +#[repr(i32)] pub enum RpcError { - KeyStoreError(String), - CheckpointNotFound, - FinalizedHeaderNotFound, - ValidatorNotFound, - DepositNotFound, - WithdrawalNotFound, - EpochNotFound, - InvalidPublicKey(String), - InvalidWithdrawalAddress(String), - GenesisPathError(String), - InvalidGenesis(String), - IoError(String), - InvalidKey(String), + KeyStoreError(String) = 1000, + CheckpointNotFound = 2000, + GenesisPathError(String) = 2001, + IoError(String) = 2002, + FinalizedHeaderNotFound = 2003, + EpochNotFound = 2004, + InvalidGenesis(String) = 2005, + ValidatorNotFound = 3000, + InvalidPublicKey(String) = 3001, + InvalidKey(String) = 3002, + DepositNotFound = 3003, + WithdrawalNotFound = 3004, StateProofKeyLimit { max: usize, actual: usize, - }, + } = 3005, StateProofCostLimit { max: usize, actual: usize, - }, + } = 3006, StateProofBusy { max: usize, - }, - Internal(String), - DisabledInObserverMode, - WithdrawalAddressBound(crate::WithdrawalAddress), + } = 3007, + InvalidWithdrawalAddress(String) = 3008, #[cfg(feature = "permissioned")] - InvalidAdminAddress(String), + InvalidAdminAddress(String) = 4000, #[cfg(feature = "permissioned")] - TimestampOutOfWindow, + TimestampOutOfWindow = 4001, #[cfg(feature = "permissioned")] - InvalidSignature, + InvalidSignature = 4002, + DisabledInObserverMode = 4003, /// Signature is valid and the signer is known, but its role does not /// permit the requested action. #[cfg(feature = "permissioned")] - Forbidden, + Forbidden = 4004, + WithdrawalAddressBound(crate::WithdrawalAddress) = 4005, + Internal(String) = 5000, +} + +impl RpcError { + /// The JSON-RPC error code: the variant's discriminant. + pub fn code(&self) -> i32 { + // SAFETY: `RpcError` is `#[repr(i32)]`, so every variant is laid out + // as a `#[repr(C)]` union of `#[repr(C)]` structs whose first field is + // the `i32` discriminant. Reading an `i32` through a pointer to the + // enum reads that discriminant: + // https://doc.rust-lang.org/reference/items/enumerations.html#pointer-casting + unsafe { *(self as *const Self as *const i32) } + } } impl From for ErrorObjectOwned { fn from(err: RpcError) -> Self { + let code = err.code(); match err { RpcError::KeyStoreError(msg) => { - ErrorObjectOwned::owned(1000, "Keystore error", Some(msg)) + ErrorObjectOwned::owned(code, "Keystore error", Some(msg)) } RpcError::CheckpointNotFound => { - ErrorObjectOwned::owned(2000, "Checkpoint not found", None::<()>) + ErrorObjectOwned::owned(code, "Checkpoint not found", None::<()>) } - RpcError::FinalizedHeaderNotFound => { - ErrorObjectOwned::owned(2003, "Finalized header not found", None::<()>) + RpcError::GenesisPathError(msg) => { + ErrorObjectOwned::owned(code, "Invalid genesis path", Some(msg)) } - RpcError::EpochNotFound => ErrorObjectOwned::owned(2004, "Epoch not found", None::<()>), - RpcError::ValidatorNotFound => { - ErrorObjectOwned::owned(3000, "Validator not found", None::<()>) + RpcError::IoError(msg) => ErrorObjectOwned::owned(code, "I/O error", Some(msg)), + RpcError::FinalizedHeaderNotFound => { + ErrorObjectOwned::owned(code, "Finalized header not found", None::<()>) } - RpcError::DepositNotFound => { - ErrorObjectOwned::owned(3003, "Deposit not found", None::<()>) + RpcError::EpochNotFound => ErrorObjectOwned::owned(code, "Epoch not found", None::<()>), + RpcError::InvalidGenesis(msg) => { + ErrorObjectOwned::owned(code, "Invalid genesis content", Some(msg)) } - RpcError::WithdrawalNotFound => { - ErrorObjectOwned::owned(3004, "Withdrawal not found", None::<()>) + RpcError::ValidatorNotFound => { + ErrorObjectOwned::owned(code, "Validator not found", None::<()>) } RpcError::InvalidPublicKey(msg) => { - ErrorObjectOwned::owned(3001, "Invalid public key", Some(msg)) + ErrorObjectOwned::owned(code, "Invalid public key", Some(msg)) } - RpcError::InvalidWithdrawalAddress(msg) => { - ErrorObjectOwned::owned(3008, "Invalid withdrawal address", Some(msg)) - } - RpcError::GenesisPathError(msg) => { - ErrorObjectOwned::owned(2001, "Invalid genesis path", Some(msg)) + RpcError::InvalidKey(msg) => { + ErrorObjectOwned::owned(code, "Invalid key descriptor", Some(msg)) } - RpcError::InvalidGenesis(msg) => { - ErrorObjectOwned::owned(2005, "Invalid genesis content", Some(msg)) + RpcError::DepositNotFound => { + ErrorObjectOwned::owned(code, "Deposit not found", None::<()>) } - RpcError::IoError(msg) => ErrorObjectOwned::owned(2002, "I/O error", Some(msg)), - RpcError::InvalidKey(msg) => { - ErrorObjectOwned::owned(3002, "Invalid key descriptor", Some(msg)) + RpcError::WithdrawalNotFound => { + ErrorObjectOwned::owned(code, "Withdrawal not found", None::<()>) } RpcError::StateProofKeyLimit { max, actual } => ErrorObjectOwned::owned( - 3005, + code, "State proof key limit exceeded", Some(format!("requested {actual} keys, maximum is {max}")), ), RpcError::StateProofCostLimit { max, actual } => ErrorObjectOwned::owned( - 3006, + code, "State proof cost limit exceeded", Some(format!("requested cost {actual}, maximum is {max}")), ), RpcError::StateProofBusy { max } => ErrorObjectOwned::owned( - 3007, + code, "State proof generation at capacity", Some(format!( "at the limit of {max} concurrent state proof requests; retry shortly" )), ), - RpcError::Internal(msg) => ErrorObjectOwned::owned(5000, "Internal error", Some(msg)), - RpcError::DisabledInObserverMode => ErrorObjectOwned::owned( - 4003, - "Method disabled in observer mode", - Some( - "this node runs with --observer and does not sign with the validator keystore", - ), - ), - RpcError::WithdrawalAddressBound(bound) => ErrorObjectOwned::owned( - 4004, - "Withdrawal address already bound", - Some(format!( - "this node's withdrawal address is bound to {bound}; it signs deposits only for that address" - )), - ), + RpcError::InvalidWithdrawalAddress(msg) => { + ErrorObjectOwned::owned(code, "Invalid withdrawal address", Some(msg)) + } #[cfg(feature = "permissioned")] RpcError::InvalidAdminAddress(msg) => { - ErrorObjectOwned::owned(4000, "Invalid admin address", Some(msg)) + ErrorObjectOwned::owned(code, "Invalid admin address", Some(msg)) } #[cfg(feature = "permissioned")] RpcError::TimestampOutOfWindow => { - ErrorObjectOwned::owned(4001, "Timestamp outside allowed window", None::<()>) + ErrorObjectOwned::owned(code, "Timestamp outside allowed window", None::<()>) } #[cfg(feature = "permissioned")] RpcError::InvalidSignature => { - ErrorObjectOwned::owned(4002, "Invalid signature", None::<()>) + ErrorObjectOwned::owned(code, "Invalid signature", None::<()>) } + RpcError::DisabledInObserverMode => ErrorObjectOwned::owned( + code, + "Method disabled in observer mode", + Some( + "this node runs with --observer and does not sign with the validator keystore", + ), + ), #[cfg(feature = "permissioned")] RpcError::Forbidden => { - ErrorObjectOwned::owned(4004, "Signer not authorized for this action", None::<()>) + ErrorObjectOwned::owned(code, "Signer not authorized for this action", None::<()>) } + RpcError::WithdrawalAddressBound(bound) => ErrorObjectOwned::owned( + code, + "Withdrawal address already bound", + Some(format!( + "this node's withdrawal address is bound to {bound}; it signs deposits only for that address" + )), + ), + RpcError::Internal(msg) => ErrorObjectOwned::owned(code, "Internal error", Some(msg)), } } } + +#[cfg(test)] +mod tests { + use super::*; + + /// `code()` reads the discriminant through a pointer cast; check it on + /// unit, tuple and struct variants. + #[test] + fn code_is_the_discriminant() { + assert_eq!(RpcError::CheckpointNotFound.code(), 2000); + assert_eq!(RpcError::Internal(String::from("x")).code(), 5000); + assert_eq!(RpcError::StateProofBusy { max: 7 }.code(), 3007); + let bound = "0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266" + .parse() + .unwrap(); + let err = ErrorObjectOwned::from(RpcError::WithdrawalAddressBound(bound)); + assert_eq!(err.code(), 4005); + } +} diff --git a/rpc/tests/integration_test.rs b/rpc/tests/integration_test.rs index 41d69ca8..3f9eac0f 100644 --- a/rpc/tests/integration_test.rs +++ b/rpc/tests/integration_test.rs @@ -1261,7 +1261,7 @@ fn assert_refused_for_bound( ) { match result { Err(err) => { - assert_eq!(err.code(), 4004, "expected a binding refusal, got {err:?}"); + assert_eq!(err.code(), 4005, "expected a binding refusal, got {err:?}"); let data = err.data().map(|d| d.get().to_string()).unwrap_or_default(); assert!(data.contains(bound), "error should name {bound}: {data}"); }