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

refactor: mv ChangedAccount #10472

Merged
merged 5 commits into from
Aug 24, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
100 changes: 100 additions & 0 deletions crates/evm/execution-types/src/execution_outcome.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,24 @@ use revm::{
};
use std::collections::HashMap;

/// Represents a changed account
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub struct ChangedAccount {
/// The address of the account.
pub address: Address,
/// Account nonce.
pub nonce: u64,
/// Account balance.
pub balance: U256,
}

impl ChangedAccount {
/// Creates a new [`ChangedAccount`] with the given address and 0 balance and nonce.
pub const fn empty(address: Address) -> Self {
Self { address, nonce: 0, balance: U256::ZERO }
}
}

/// Represents the outcome of block execution, including post-execution changes and reverts.
///
/// The `ExecutionOutcome` structure aggregates the state changes over an arbitrary number of
Expand Down Expand Up @@ -324,6 +342,17 @@ impl ExecutionOutcome {
self.requests = requests;
self
}

/// Returns an iterator over all changed accounts from the `ExecutionOutcome`.
///
/// This method filters the accounts to return only those that have undergone changes
/// and maps them into `ChangedAccount` instances, which include the address, nonce, and
/// balance.
pub fn changed_accounts(&self) -> impl Iterator<Item = ChangedAccount> + '_ {
self.accounts_iter().filter_map(|(addr, acc)| acc.map(|acc| (addr, acc))).map(
|(address, acc)| ChangedAccount { address, nonce: acc.nonce, balance: acc.balance },
)
}
}

#[cfg(test)]
Expand Down Expand Up @@ -781,4 +810,75 @@ mod tests {
// Assert that splitting at the first block number returns None for the lower outcome
assert_eq!(exec_res.clone().split_at(123), (None, exec_res));
}

#[test]
fn test_changed_accounts() {
// Set up some sample accounts
let address1 = Address::random();
let address2 = Address::random();
let address3 = Address::random();

// Set up account info with some changes
let account_info1 =
AccountInfo { nonce: 1, balance: U256::from(100), code_hash: B256::ZERO, code: None };
let account_info2 =
AccountInfo { nonce: 2, balance: U256::from(200), code_hash: B256::ZERO, code: None };

// Set up the bundle state with these accounts
let mut bundle_state = BundleState::default();
bundle_state.state.insert(
address1,
BundleAccount {
info: Some(account_info1),
storage: Default::default(),
original_info: Default::default(),
status: Default::default(),
},
);
bundle_state.state.insert(
address2,
BundleAccount {
info: Some(account_info2),
storage: Default::default(),
original_info: Default::default(),
status: Default::default(),
},
);

// Unchanged account
bundle_state.state.insert(
address3,
BundleAccount {
info: None,
storage: Default::default(),
original_info: Default::default(),
status: Default::default(),
},
);

let execution_outcome = ExecutionOutcome {
bundle: bundle_state,
receipts: Receipts::default(),
first_block: 0,
requests: vec![],
};

// Get the changed accounts
let changed_accounts: Vec<ChangedAccount> = execution_outcome.changed_accounts().collect();

// Assert that the changed accounts match the expected ones
assert_eq!(changed_accounts.len(), 2);

assert!(changed_accounts.contains(&ChangedAccount {
address: address1,
nonce: 1,
balance: U256::from(100)
}));

assert!(changed_accounts.contains(&ChangedAccount {
address: address2,
nonce: 2,
balance: U256::from(200)
}));
}
}
1 change: 1 addition & 0 deletions crates/transaction-pool/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@
use crate::{identifier::TransactionId, pool::PoolInner};
use aquamarine as _;
use reth_eth_wire_types::HandleMempoolData;
use reth_execution_types::ChangedAccount;
use reth_primitives::{Address, BlobTransactionSidecar, PooledTransactionsElement, TxHash, U256};
use reth_storage_api::StateProviderFactory;
use std::{collections::HashSet, sync::Arc};
Expand Down
18 changes: 4 additions & 14 deletions crates/transaction-pool/src/maintain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::{
blobstore::{BlobStoreCanonTracker, BlobStoreUpdates},
error::PoolError,
metrics::MaintainPoolMetrics,
traits::{CanonicalStateUpdate, ChangedAccount, TransactionPool, TransactionPoolExt},
traits::{CanonicalStateUpdate, TransactionPool, TransactionPoolExt},
BlockInfo, PoolTransaction,
};
use futures_util::{
Expand All @@ -13,7 +13,7 @@ use futures_util::{
};
use reth_chain_state::CanonStateNotification;
use reth_chainspec::{ChainSpec, ChainSpecProvider};
use reth_execution_types::ExecutionOutcome;
use reth_execution_types::ChangedAccount;
use reth_fs_util::FsPathError;
use reth_primitives::{
Address, BlockHash, BlockNumber, BlockNumberOrTag, IntoRecoveredTransaction,
Expand Down Expand Up @@ -282,7 +282,7 @@ pub async fn maintain_transaction_pool<Client, P, St, Tasks>(

// we know all changed account in the new chain
let new_changed_accounts: HashSet<_> =
changed_accounts_iter(new_state).map(ChangedAccountEntry).collect();
new_state.changed_accounts().map(ChangedAccountEntry).collect();

// find all accounts that were changed in the old chain but _not_ in the new chain
let missing_changed_acc = old_state
Expand Down Expand Up @@ -415,7 +415,7 @@ pub async fn maintain_transaction_pool<Client, P, St, Tasks>(
}

let mut changed_accounts = Vec::with_capacity(state.state().len());
for acc in changed_accounts_iter(state) {
for acc in state.changed_accounts() {
// we can always clear the dirty flag for this account
dirty_addresses.remove(&acc.address);
changed_accounts.push(acc);
Expand Down Expand Up @@ -560,16 +560,6 @@ where
Ok(res)
}

/// Extracts all changed accounts from the `BundleState`
fn changed_accounts_iter(
execution_outcome: &ExecutionOutcome,
) -> impl Iterator<Item = ChangedAccount> + '_ {
execution_outcome
.accounts_iter()
.filter_map(|(addr, acc)| acc.map(|acc| (addr, acc)))
.map(|(address, acc)| ChangedAccount { address, nonce: acc.nonce, balance: acc.balance })
}

/// Loads transactions from a file, decodes them from the RLP format, and inserts them
/// into the transaction pool on node boot up.
/// The file is removed after the transactions have been successfully processed.
Expand Down
3 changes: 2 additions & 1 deletion crates/transaction-pool/src/pool/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,12 @@ use crate::{
PoolTransaction, PropagatedTransactions, TransactionOrigin,
},
validate::{TransactionValidationOutcome, ValidPoolTransaction},
CanonicalStateUpdate, ChangedAccount, PoolConfig, TransactionOrdering, TransactionValidator,
CanonicalStateUpdate, PoolConfig, TransactionOrdering, TransactionValidator,
};
use best::BestTransactions;
use parking_lot::{Mutex, RwLock, RwLockReadGuard};
use reth_eth_wire_types::HandleMempoolData;
use reth_execution_types::ChangedAccount;
use reth_primitives::{
Address, BlobTransaction, BlobTransactionSidecar, IntoRecoveredTransaction,
PooledTransactionsElement, TransactionSigned, TxHash, B256,
Expand Down
21 changes: 1 addition & 20 deletions crates/transaction-pool/src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use crate::{
};
use futures_util::{ready, Stream};
use reth_eth_wire_types::HandleMempoolData;
use reth_execution_types::ChangedAccount;
use reth_primitives::{
kzg::KzgSettings, transaction::TryFromRecoveredTransactionError, AccessList, Address,
BlobTransactionSidecar, BlobTransactionValidationError, PooledTransactionsElement,
Expand Down Expand Up @@ -675,26 +676,6 @@ impl fmt::Display for CanonicalStateUpdate<'_> {
}
}

/// Represents a changed account
#[derive(Clone, Copy, Debug, PartialEq, Eq, Default)]
pub struct ChangedAccount {
/// The address of the account.
pub address: Address,
/// Account nonce.
pub nonce: u64,
/// Account balance.
pub balance: U256,
}

// === impl ChangedAccount ===

impl ChangedAccount {
/// Creates a new `ChangedAccount` with the given address and 0 balance and nonce.
pub(crate) const fn empty(address: Address) -> Self {
Self { address, nonce: 0, balance: U256::ZERO }
}
}

/// An `Iterator` that only returns transactions that are ready to be executed.
///
/// This makes no assumptions about the order of the transactions, but expects that _all_
Expand Down
Loading