Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: metrics for the send_transaction wallet method #45

Merged
merged 3 commits into from
Oct 17, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
3 changes: 3 additions & 0 deletions crates/wallet/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
Expand Down
58 changes: 50 additions & 8 deletions crates/wallet/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -186,6 +188,7 @@ impl<Provider, Eth> OdysseyWallet<Provider, Eth> {
Capabilities { delegation: DelegationCapability { addresses: valid_designations } },
)])),
permit: Default::default(),
metrics: WalletMetrics::default(),
};
Self { inner: Arc::new(inner) }
}
Expand All @@ -210,7 +213,10 @@ where
trace!(target: "rpc::wallet", ?request, "Serving wallet_sendTransaction");

// validate fields common to eip-7702 and eip-1559
validate_tx_request(&request)?;
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
.inner
Expand All @@ -221,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());
}
}
Expand All @@ -230,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()
Expand All @@ -246,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
Expand All @@ -265,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
Expand All @@ -279,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::<u128>() + max_priority_fee_per_gas);
request.max_priority_fee_per_gas = Some(max_priority_fee_per_gas);
Expand All @@ -299,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
Expand All @@ -322,6 +352,8 @@ struct OdysseyWalletInner<Provider, Eth> {
capabilities: WalletCapabilities,
/// Used to guard tx signing
permit: Mutex<()>,
/// Metrics for a `wallet`.
onbjerg marked this conversation as resolved.
Show resolved Hide resolved
metrics: WalletMetrics,
}

fn validate_tx_request(request: &TransactionRequest) -> Result<(), OdysseyWalletError> {
Expand All @@ -343,6 +375,16 @@ fn validate_tx_request(request: &TransactionRequest) -> Result<(), OdysseyWallet
Ok(())
}

/// Metrics for the `wallet_` RPC namespace.
#[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::{
Expand Down