From 9b6fd3f58898b51b34805398efece8783f05519c Mon Sep 17 00:00:00 2001 From: Naman Garg <0708ng@gmail.com> Date: Tue, 9 Jul 2024 16:12:23 +0530 Subject: [PATCH 1/3] fix: validate tx chain id --- bolt-sidecar/src/client/rpc.rs | 5 +++++ bolt-sidecar/src/primitives/commitment.rs | 9 +++++++++ bolt-sidecar/src/state/execution.rs | 17 +++++++++++++++++ bolt-sidecar/src/state/fetcher.rs | 6 ++++++ 4 files changed, 37 insertions(+) diff --git a/bolt-sidecar/src/client/rpc.rs b/bolt-sidecar/src/client/rpc.rs index ec796167..a5981eb6 100644 --- a/bolt-sidecar/src/client/rpc.rs +++ b/bolt-sidecar/src/client/rpc.rs @@ -33,6 +33,11 @@ impl RpcClient { Self(client) } + /// Get the chain ID. + pub async fn get_chain_id(&self) -> TransportResult { + self.0.request("eth_chainId", ()).await + } + /// Get the basefee of the latest block. pub async fn get_basefee(&self, block_number: Option) -> TransportResult { let tag = block_number.map_or(BlockNumberOrTag::Latest, BlockNumberOrTag::Number); diff --git a/bolt-sidecar/src/primitives/commitment.rs b/bolt-sidecar/src/primitives/commitment.rs index c220ad37..cbe9e63b 100644 --- a/bolt-sidecar/src/primitives/commitment.rs +++ b/bolt-sidecar/src/primitives/commitment.rs @@ -42,6 +42,15 @@ impl InclusionRequest { } true } + + /// Validates the transaction chain id against the provided chain id. + /// Returns true if the chain id matches, false otherwise. + pub fn validate_chain_id(&self, chain_id: u64) -> bool { + match self.tx.chain_id() { + Some(tx_chain_id) if tx_chain_id == chain_id => true, + _ => false, + } + } } fn deserialize_tx_signed<'de, D>(deserializer: D) -> Result diff --git a/bolt-sidecar/src/state/execution.rs b/bolt-sidecar/src/state/execution.rs index 4dd74601..41a7d6ba 100644 --- a/bolt-sidecar/src/state/execution.rs +++ b/bolt-sidecar/src/state/execution.rs @@ -37,6 +37,9 @@ pub enum ValidationError { /// Could not recover signature, #[error("Could not recover signer")] RecoverSigner, + /// The transaction chain ID does not match the expected chain ID. + #[error("Chain ID mismatch")] + ChainIdMismatch, /// NOTE: this should not be exposed to the user. #[error("Internal error: {0}")] Internal(String), @@ -79,6 +82,9 @@ pub struct ExecutionState { /// proposal duties for a single lookahead. block_templates: HashMap, + /// The chain ID of the chain (constant). + chain_id: u64, + /// The state fetcher client. client: C, } @@ -93,6 +99,7 @@ impl ExecutionState { slot: 0, account_states: HashMap::new(), block_templates: HashMap::new(), + chain_id: client.get_chain_id().await?, client, }) } @@ -107,6 +114,11 @@ impl ExecutionState { &self.block_templates } + /// Returns the chain ID of the chain + pub fn chain_id(&self) -> u64 { + self.chain_id + } + /// Validates the commitment request against state (historical + intermediate). /// NOTE: This function only simulates against execution state, it does not consider /// timing or proposer slot targets. @@ -136,6 +148,11 @@ impl ExecutionState { return Err(ValidationError::BaseFeeTooLow(max_basefee as u128)); } + // Validate the chain ID + if !req.validate_chain_id(self.chain_id()) { + return Err(ValidationError::ChainIdMismatch); + } + // If we have the account state, use it here if let Some(account_state) = self.account_state(&sender) { // Validate the transaction against the account state diff --git a/bolt-sidecar/src/state/fetcher.rs b/bolt-sidecar/src/state/fetcher.rs index 58957978..2df3b150 100644 --- a/bolt-sidecar/src/state/fetcher.rs +++ b/bolt-sidecar/src/state/fetcher.rs @@ -38,6 +38,8 @@ pub trait StateFetcher { address: &Address, block_number: Option, ) -> Result; + + async fn get_chain_id(&self) -> Result; } /// A basic state fetcher that uses an RPC client to fetch state updates. @@ -177,6 +179,10 @@ impl StateFetcher for StateClient { } } } + + async fn get_chain_id(&self) -> Result { + self.client.get_chain_id().await + } } #[cfg(test)] From fd82250932ffd050d08c45df0e7c049703aa2b8f Mon Sep 17 00:00:00 2001 From: Naman Garg <0708ng@gmail.com> Date: Tue, 9 Jul 2024 19:13:56 +0530 Subject: [PATCH 2/3] chore(sidecar): validate chain id first --- bolt-sidecar/src/state/execution.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/bolt-sidecar/src/state/execution.rs b/bolt-sidecar/src/state/execution.rs index 41a7d6ba..a19e6947 100644 --- a/bolt-sidecar/src/state/execution.rs +++ b/bolt-sidecar/src/state/execution.rs @@ -130,6 +130,11 @@ impl ExecutionState { pub async fn try_commit(&mut self, request: &CommitmentRequest) -> Result<(), ValidationError> { let CommitmentRequest::Inclusion(req) = request; + // Validate the chain ID + if !req.validate_chain_id(self.chain_id()) { + return Err(ValidationError::ChainIdMismatch); + } + let sender = req.tx.recover_signer().ok_or(ValidationError::Internal( "Failed to recover signer from transaction".to_string(), ))?; @@ -148,11 +153,6 @@ impl ExecutionState { return Err(ValidationError::BaseFeeTooLow(max_basefee as u128)); } - // Validate the chain ID - if !req.validate_chain_id(self.chain_id()) { - return Err(ValidationError::ChainIdMismatch); - } - // If we have the account state, use it here if let Some(account_state) = self.account_state(&sender) { // Validate the transaction against the account state From 36463230a6cd63080fe64174c8a716e4026a215d Mon Sep 17 00:00:00 2001 From: Naman Garg <0708ng@gmail.com> Date: Tue, 9 Jul 2024 19:19:31 +0530 Subject: [PATCH 3/3] chore(sidecar): remove chain_id getter fn --- bolt-sidecar/src/state/execution.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/bolt-sidecar/src/state/execution.rs b/bolt-sidecar/src/state/execution.rs index a19e6947..ad01b143 100644 --- a/bolt-sidecar/src/state/execution.rs +++ b/bolt-sidecar/src/state/execution.rs @@ -114,11 +114,6 @@ impl ExecutionState { &self.block_templates } - /// Returns the chain ID of the chain - pub fn chain_id(&self) -> u64 { - self.chain_id - } - /// Validates the commitment request against state (historical + intermediate). /// NOTE: This function only simulates against execution state, it does not consider /// timing or proposer slot targets. @@ -131,7 +126,7 @@ impl ExecutionState { let CommitmentRequest::Inclusion(req) = request; // Validate the chain ID - if !req.validate_chain_id(self.chain_id()) { + if !req.validate_chain_id(self.chain_id) { return Err(ValidationError::ChainIdMismatch); }