From c3ee901e78526c03543bbe398d3b80bfbc9d48b6 Mon Sep 17 00:00:00 2001 From: Elin Tulchinsky Date: Tue, 15 Oct 2024 13:28:29 +0300 Subject: [PATCH] chore(mempool): add logs to mempool logic --- Cargo.lock | 1 + crates/mempool/Cargo.toml | 1 + crates/mempool/src/mempool.rs | 48 +++++++++++++++++++++-- crates/mempool_types/src/mempool_types.rs | 7 ++++ crates/starknet_api/src/core.rs | 1 + crates/starknet_api/src/transaction.rs | 1 + 6 files changed, 56 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 83e57f621d..6286e1d59f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10335,6 +10335,7 @@ dependencies = [ "starknet_mempool_p2p_types", "starknet_mempool_types", "tokio", + "tracing", ] [[package]] diff --git a/crates/mempool/Cargo.toml b/crates/mempool/Cargo.toml index 4901a5a6fb..d07170b013 100644 --- a/crates/mempool/Cargo.toml +++ b/crates/mempool/Cargo.toml @@ -20,6 +20,7 @@ starknet_mempool_infra.workspace = true starknet_mempool_p2p_types.workspace = true starknet_mempool_types.workspace = true tokio.workspace = true +tracing.workspace = true [dev-dependencies] assert_matches.workspace = true diff --git a/crates/mempool/src/mempool.rs b/crates/mempool/src/mempool.rs index 1fcb05d78b..e162f3323a 100644 --- a/crates/mempool/src/mempool.rs +++ b/crates/mempool/src/mempool.rs @@ -11,6 +11,7 @@ use starknet_mempool_types::mempool_types::{ CommitBlockArgs, MempoolResult, }; +use tracing; use crate::transaction_pool::TransactionPool; use crate::transaction_queue::TransactionQueue; @@ -52,6 +53,7 @@ impl Mempool { /// Transactions are guaranteed to be unique across calls until the block in-progress is /// created. // TODO: Consider renaming to `pop_txs` to be more consistent with the standard library. + #[tracing::instrument(skip(self), err)] pub fn get_txs(&mut self, n_txs: usize) -> MempoolResult> { let mut eligible_tx_references: Vec = Vec::with_capacity(n_txs); let mut n_remaining_txs = n_txs; @@ -68,6 +70,11 @@ impl Mempool { self.mempool_state.insert(tx_ref.address, tx_ref.nonce); } + tracing::info!( + "Returned {} out of {n_txs} transactions, ready for sequencing.", + eligible_tx_references.len() + ); + Ok(eligible_tx_references .iter() .map(|tx_ref| { @@ -80,6 +87,16 @@ impl Mempool { } /// Adds a new transaction to the mempool. + #[tracing::instrument( + skip(self, args), + fields( // Log subset of (informative) fields. + tx_nonce = %args.tx.nonce(), + tx_hash = %args.tx.tx_hash(), + account_state = %args.account_state + ), + ret, + err + )] pub fn add_tx(&mut self, args: AddTransactionArgs) -> MempoolResult<()> { let AddTransactionArgs { tx, account_state } = args; self.validate_incoming_nonce(tx.nonce(), account_state)?; @@ -101,17 +118,22 @@ impl Mempool { /// Update the mempool's internal state according to the committed block (resolves nonce gaps, /// updates account balances). + #[tracing::instrument(skip(self, args), ret, err)] pub fn commit_block(&mut self, args: CommitBlockArgs) -> MempoolResult<()> { + let CommitBlockArgs { nonces, tx_hashes } = args; + tracing::debug!("Committing block with {} transactions to mempool.", tx_hashes.len()); + // Align mempool data to committed nonces. - for (&address, &nonce) in &args.nonces { + for (&address, &nonce) in &nonces { let next_nonce = nonce.try_increment().map_err(|_| MempoolError::FeltOutOfRange)?; let account_state = AccountState { address, nonce: next_nonce }; self.align_to_account_state(account_state); } + tracing::debug!("Aligned mempool to committed nonces."); // Rewind nonces of addresses that were not included in block. let known_addresses_not_included_in_block = - self.mempool_state.keys().filter(|&key| !args.nonces.contains_key(key)); + self.mempool_state.keys().filter(|&key| !nonces.contains_key(key)); for address in known_addresses_not_included_in_block { // Account nonce is the minimal nonce of this address: it was proposed but not included. let tx_reference = self @@ -123,7 +145,7 @@ impl Mempool { } // Hard-delete: finally, remove committed transactions from the mempool. - for tx_hash in args.tx_hashes { + for tx_hash in tx_hashes { let Ok(_tx) = self.tx_pool.remove(tx_hash) else { continue; // Transaction hash unknown to mempool, from a different node. }; @@ -131,10 +153,13 @@ impl Mempool { // TODO(clean_account_nonces): remove address from nonce table after a block cycle / // TTL. } + tracing::debug!("Removed committed transactions known to mempool."); // Commit: clear block creation staged state. self.mempool_state.clear(); + tracing::debug!("Successfully committed block to mempool."); + Ok(()) } @@ -219,10 +244,16 @@ impl Mempool { }; if !self.should_replace_tx(existing_tx_ref, &incoming_tx_ref) { + tracing::info!( + "{existing_tx_ref} was not replaced by {incoming_tx_ref} due to insufficient + fee escalation." + ); // TODO(Elin): consider adding a more specific error type / message. return Err(MempoolError::DuplicateNonce { address, nonce }); } + tracing::info!("{existing_tx_ref} will be replaced by {incoming_tx_ref}."); + self.tx_queue.remove(address); self.tx_pool .remove(existing_tx_ref.tx_hash) @@ -274,6 +305,17 @@ pub struct TransactionReference { pub max_l2_gas_price: GasPrice, } +impl std::fmt::Display for TransactionReference { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let TransactionReference { address, nonce, tx_hash, tip, max_l2_gas_price } = self; + write!( + f, + "TransactionReference {{ address: {address}, nonce: {nonce}, tx_hash: {tx_hash}, + tip: {tip}, max_l2_gas_price: {max_l2_gas_price} }}" + ) + } +} + impl TransactionReference { pub fn new(tx: &Transaction) -> Self { TransactionReference { diff --git a/crates/mempool_types/src/mempool_types.rs b/crates/mempool_types/src/mempool_types.rs index 4a46adbf79..01362eeed4 100644 --- a/crates/mempool_types/src/mempool_types.rs +++ b/crates/mempool_types/src/mempool_types.rs @@ -14,6 +14,13 @@ pub struct AccountState { pub nonce: Nonce, } +impl std::fmt::Display for AccountState { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let AccountState { address, nonce } = self; + write!(f, "AccountState {{ address: {address}, nonce: {nonce} }}") + } +} + #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] pub struct AddTransactionArgs { pub tx: Transaction, diff --git a/crates/starknet_api/src/core.rs b/crates/starknet_api/src/core.rs index e48e8917d2..142cfa15d0 100644 --- a/crates/starknet_api/src/core.rs +++ b/crates/starknet_api/src/core.rs @@ -203,6 +203,7 @@ pub struct CompiledClassHash(pub StarkHash); #[derive( Debug, Default, + Display, Copy, Clone, Eq, diff --git a/crates/starknet_api/src/transaction.rs b/crates/starknet_api/src/transaction.rs index 172b179349..0322323849 100644 --- a/crates/starknet_api/src/transaction.rs +++ b/crates/starknet_api/src/transaction.rs @@ -969,6 +969,7 @@ pub struct EventIndexInTransactionOutput(pub usize); PartialOrd, Serialize, derive_more::Deref, + derive_more::Display, )] #[serde(from = "PrefixedBytesAsHex<8_usize>", into = "PrefixedBytesAsHex<8_usize>")] pub struct Tip(pub u64);