From 74dbf01bf4873cba49d3ce28e7556d8d245b79f6 Mon Sep 17 00:00:00 2001 From: Steph Sinyakov Date: Tue, 15 Oct 2024 19:59:59 +0200 Subject: [PATCH 1/3] feat: metrics for the `send_transaction` wallet method --- Cargo.lock | 2 ++ Cargo.toml | 4 ++++ crates/wallet/Cargo.toml | 3 +++ crates/wallet/src/lib.rs | 25 ++++++++++++++++++++++++- 4 files changed, 33 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index c2a73df..a333755 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4508,6 +4508,8 @@ dependencies = [ "alloy-primitives", "alloy-rpc-types", "jsonrpsee", + "metrics", + "metrics-derive", "reth-primitives", "reth-revm", "reth-rpc-eth-api", diff --git a/Cargo.toml b/Cargo.toml index 8877b33..13319b2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -195,6 +195,10 @@ reth-network = { git = "https://github.com/paradigmxyz/reth.git", rev = "75dda1c reth-network-types = { git = "https://github.com/paradigmxyz/reth.git", rev = "75dda1c" } reth-chain-state = { git = "https://github.com/paradigmxyz/reth.git", rev = "75dda1c" } +# metrics +metrics = "0.23.0" +metrics-derive = "0.1.0" + # rpc jsonrpsee = "0.24" diff --git a/crates/wallet/Cargo.toml b/crates/wallet/Cargo.toml index c7c3d99..3bd58fb 100644 --- a/crates/wallet/Cargo.toml +++ b/crates/wallet/Cargo.toml @@ -25,6 +25,9 @@ thiserror.workspace = true tracing.workspace = true tokio = { workspace = true, features = ["sync"] } +metrics.workspace = true +metrics-derive.workspace = true + [dev-dependencies] serde_json.workspace = true jsonrpsee = { workspace = true, features = ["server", "client", "macros"] } diff --git a/crates/wallet/src/lib.rs b/crates/wallet/src/lib.rs index 47d431f..1adc9b3 100644 --- a/crates/wallet/src/lib.rs +++ b/crates/wallet/src/lib.rs @@ -26,6 +26,8 @@ use jsonrpsee::{ core::{async_trait, RpcResult}, proc_macros::rpc, }; +use metrics::Counter; +use metrics_derive::Metrics; use reth_primitives::{revm_primitives::Bytecode, BlockId}; use reth_rpc_eth_api::helpers::{EthCall, EthState, EthTransactions, FullEthApi, LoadFee}; use reth_storage_api::{StateProvider, StateProviderFactory}; @@ -186,6 +188,7 @@ impl OdysseyWallet { Capabilities { delegation: DelegationCapability { addresses: valid_designations } }, )])), permit: Default::default(), + metrics: WalletMetrics::default(), }; Self { inner: Arc::new(inner) } } @@ -210,7 +213,15 @@ where trace!(target: "rpc::wallet", ?request, "Serving wallet_sendTransaction"); // validate fields common to eip-7702 and eip-1559 - validate_tx_request(&request)?; + match validate_tx_request(&request) { + Ok(_) => { + self.inner.metrics.valid_send_transaction_calls.increment(1); + } + Err(err) => { + self.inner.metrics.invalid_send_transaction_calls.increment(1); + return Err(err.into()); + } + } let valid_delegations: &[Address] = self .inner @@ -322,6 +333,8 @@ struct OdysseyWalletInner { capabilities: WalletCapabilities, /// Used to guard tx signing permit: Mutex<()>, + /// Metrics for a `wallet`. + metrics: WalletMetrics, } fn validate_tx_request(request: &TransactionRequest) -> Result<(), OdysseyWalletError> { @@ -343,6 +356,16 @@ fn validate_tx_request(request: &TransactionRequest) -> Result<(), OdysseyWallet Ok(()) } +/// Metrics for a `wallet`. +#[derive(Metrics)] +#[metrics(scope = "wallet")] +struct WalletMetrics { + /// Number of invalid calls to `wallet_sendTransaction` + invalid_send_transaction_calls: Counter, + /// Number of valid calls to `wallet_sendTransaction` + valid_send_transaction_calls: Counter, +} + #[cfg(test)] mod tests { use crate::{ From 79fa855cd351b3e0229912f072517046304ed455 Mon Sep 17 00:00:00 2001 From: Steph Sinyakov Date: Wed, 16 Oct 2024 09:44:38 +0200 Subject: [PATCH 2/3] fix: review fixes --- crates/wallet/src/lib.rs | 51 +++++++++++++++++++++++++++------------- 1 file changed, 35 insertions(+), 16 deletions(-) diff --git a/crates/wallet/src/lib.rs b/crates/wallet/src/lib.rs index 1adc9b3..05bfaab 100644 --- a/crates/wallet/src/lib.rs +++ b/crates/wallet/src/lib.rs @@ -213,14 +213,9 @@ where trace!(target: "rpc::wallet", ?request, "Serving wallet_sendTransaction"); // validate fields common to eip-7702 and eip-1559 - match validate_tx_request(&request) { - Ok(_) => { - self.inner.metrics.valid_send_transaction_calls.increment(1); - } - Err(err) => { - self.inner.metrics.invalid_send_transaction_calls.increment(1); - return Err(err.into()); - } + if let Err(err) = validate_tx_request(&request) { + self.inner.metrics.invalid_send_transaction_calls.increment(1); + return Err(err.into()); } let valid_delegations: &[Address] = self @@ -232,6 +227,7 @@ where if let Some(authorizations) = &request.authorization_list { // check that all auth items delegate to a valid address if authorizations.iter().any(|auth| !valid_delegations.contains(&auth.address)) { + self.inner.metrics.invalid_send_transaction_calls.increment(1); return Err(OdysseyWalletError::InvalidAuthorization.into()); } } @@ -241,8 +237,10 @@ where // if this is an eip-1559 tx, ensure that it is an account that delegates to a // whitelisted address (false, Some(TxKind::Call(addr))) => { - let state = - self.inner.provider.latest().map_err(|_| OdysseyWalletError::InternalError)?; + let state = self.inner.provider.latest().map_err(|_| { + self.inner.metrics.invalid_send_transaction_calls.increment(1); + OdysseyWalletError::InternalError + })?; let delegated_address = state .account_code(addr) .ok() @@ -257,13 +255,17 @@ where if delegated_address == Address::ZERO || !valid_delegations.contains(&delegated_address) { + self.inner.metrics.invalid_send_transaction_calls.increment(1); return Err(OdysseyWalletError::IllegalDestination.into()); } } // if it's an eip-7702 tx, let it through (true, _) => (), // create tx's disallowed - _ => return Err(OdysseyWalletError::IllegalDestination.into()), + _ => { + self.inner.metrics.invalid_send_transaction_calls.increment(1); + return Err(OdysseyWalletError::IllegalDestination.into()); + } } // we acquire the permit here so that all following operations are performed exclusively @@ -276,7 +278,10 @@ where Some(BlockId::pending()), ) .await - .map_err(Into::into)?; + .map_err(|err| { + self.inner.metrics.invalid_send_transaction_calls.increment(1); + err.into() + })?; request.nonce = Some(tx_count.to()); // set chain id @@ -290,14 +295,22 @@ where EthCall::estimate_gas_at(&self.inner.eth_api, request.clone(), BlockId::latest(), None), LoadFee::eip1559_fees(&self.inner.eth_api, None, None) ); - let estimate = estimate.map_err(Into::into)?; + let estimate = estimate.map_err(|err| { + self.inner.metrics.invalid_send_transaction_calls.increment(1); + err.into() + })?; + if estimate >= U256::from(350_000) { + self.inner.metrics.invalid_send_transaction_calls.increment(1); return Err(OdysseyWalletError::GasEstimateTooHigh { estimate: estimate.to() }.into()); } request.gas = Some(estimate.to()); // set gas price - let (base_fee, _) = base_fee.map_err(|_| OdysseyWalletError::InvalidTransactionRequest)?; + let (base_fee, _) = base_fee.map_err(|_| { + self.inner.metrics.invalid_send_transaction_calls.increment(1); + OdysseyWalletError::InvalidTransactionRequest + })?; let max_priority_fee_per_gas = 1_000_000_000; // 1 gwei request.max_fee_per_gas = Some(base_fee.to::() + max_priority_fee_per_gas); request.max_priority_fee_per_gas = Some(max_priority_fee_per_gas); @@ -310,7 +323,13 @@ where &self.inner.wallet, ) .await - .map_err(|_| OdysseyWalletError::InvalidTransactionRequest)?; + .map_err(|_| { + self.inner.metrics.invalid_send_transaction_calls.increment(1); + OdysseyWalletError::InvalidTransactionRequest + })?; + + // all checks passed, increment the valid calls counter + self.inner.metrics.valid_send_transaction_calls.increment(1); // this uses the internal `OpEthApi` to either forward the tx to the sequencer, or add it to // the txpool @@ -356,7 +375,7 @@ fn validate_tx_request(request: &TransactionRequest) -> Result<(), OdysseyWallet Ok(()) } -/// Metrics for a `wallet`. +/// Metrics for the `wallet_` RPC namespace. #[derive(Metrics)] #[metrics(scope = "wallet")] struct WalletMetrics { From afab1dba8ad7df6cc2a6baee8e206c17cd19eeef Mon Sep 17 00:00:00 2001 From: Oliver Date: Thu, 17 Oct 2024 11:51:54 +0200 Subject: [PATCH 3/3] docs: nit --- crates/wallet/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/wallet/src/lib.rs b/crates/wallet/src/lib.rs index 05bfaab..8132a8c 100644 --- a/crates/wallet/src/lib.rs +++ b/crates/wallet/src/lib.rs @@ -352,7 +352,7 @@ struct OdysseyWalletInner { capabilities: WalletCapabilities, /// Used to guard tx signing permit: Mutex<()>, - /// Metrics for a `wallet`. + /// Metrics for the `wallet_` RPC namespace. metrics: WalletMetrics, }