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(mempool_test_utils): add non mut account_with_id, rename previous implementation #2801

Open
wants to merge 2 commits into
base: yair/changing_test_scenarios
Choose a base branch
from
Open
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
24 changes: 18 additions & 6 deletions crates/mempool_test_utils/src/starknet_api_test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ pub fn executable_invoke_tx(cairo_version: CairoVersion) -> AccountTransaction {

let mut tx_generator = MultiAccountTransactionGenerator::new();
tx_generator.register_account(default_account);
tx_generator.account_with_id(0).generate_executable_invoke()
tx_generator.account_with_id_mut(0).generate_executable_invoke()
}

pub fn generate_deploy_account_with_salt(
Expand Down Expand Up @@ -162,9 +162,9 @@ type SharedNonceManager = Rc<RefCell<NonceManager>>;
/// tx_generator.register_account_for_flow_test(some_account_type.clone());
/// tx_generator.register_account_for_flow_test(some_account_type);
///
/// let account_0_tx_with_nonce_0 = tx_generator.account_with_id(0).generate_invoke_with_tip(1);
/// let account_1_tx_with_nonce_0 = tx_generator.account_with_id(1).generate_invoke_with_tip(3);
/// let account_0_tx_with_nonce_1 = tx_generator.account_with_id(0).generate_invoke_with_tip(1);
/// let account_0_tx_with_nonce_0 = tx_generator.account_with_id_mut(0).generate_invoke_with_tip(1);
/// let account_1_tx_with_nonce_0 = tx_generator.account_with_id_mut(1).generate_invoke_with_tip(3);
/// let account_0_tx_with_nonce_1 = tx_generator.account_with_id_mut(0).generate_invoke_with_tip(1);
/// ```
// Note: when moving this to starknet api crate, see if blockifier's
// [blockifier::transaction::test_utils::FaultyAccountTxCreatorArgs] can be made to use this.
Expand Down Expand Up @@ -194,7 +194,10 @@ impl MultiAccountTransactionGenerator {
default_deploy_account_tx
}

pub fn account_with_id(&mut self, account_id: AccountId) -> &mut AccountTransactionGenerator {
pub fn account_with_id_mut(
&mut self,
account_id: AccountId,
) -> &mut AccountTransactionGenerator {
self.account_tx_generators.get_mut(account_id).unwrap_or_else(|| {
panic!(
"{account_id:?} not found! This number should be an index of an account in the \
Expand All @@ -203,6 +206,15 @@ impl MultiAccountTransactionGenerator {
})
}

pub fn account_with_id(&self, account_id: AccountId) -> &AccountTransactionGenerator {
self.account_tx_generators.get(account_id).unwrap_or_else(|| {
panic!(
"{account_id:?} not found! This number should be an index of an account in the \
initialization array. "
)
})
}

// TODO(deploy_account_support): once we support deploy account in tests, remove this method and
// only use new_account_default in tests. In particular, deploy account txs must be then sent to
// the GW via the add tx endpoint just like other txs.
Expand Down Expand Up @@ -280,7 +292,7 @@ impl AccountTransactionGenerator {
rpc_invoke_tx(invoke_tx_args)
}

pub fn sender_address(&mut self) -> ContractAddress {
pub fn sender_address(&self) -> ContractAddress {
self.account.sender_address
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ async fn await_block(
.ok_or(())
}

pub async fn end_to_end_integration(mut tx_generator: MultiAccountTransactionGenerator) {
pub async fn end_to_end_integration(tx_generator: MultiAccountTransactionGenerator) {
const EXPECTED_BLOCK_NUMBER: BlockNumber = BlockNumber(15);

info!("Checking that the sequencer node executable is present.");
Expand Down
17 changes: 11 additions & 6 deletions crates/starknet_integration_tests/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,19 +153,19 @@ pub fn create_integration_test_tx_generator() -> MultiAccountTransactionGenerato
tx_generator
}

fn create_txs_for_integration_test(
pub fn create_txs_for_integration_test(
tx_generator: &mut MultiAccountTransactionGenerator,
) -> Vec<RpcTransaction> {
const ACCOUNT_ID_0: AccountId = 0;
const ACCOUNT_ID_1: AccountId = 1;

// Create RPC transactions.
let account0_invoke_nonce1 =
tx_generator.account_with_id(ACCOUNT_ID_0).generate_invoke_with_tip(2);
tx_generator.account_with_id_mut(ACCOUNT_ID_0).generate_invoke_with_tip(2);
let account0_invoke_nonce2 =
tx_generator.account_with_id(ACCOUNT_ID_0).generate_invoke_with_tip(3);
tx_generator.account_with_id_mut(ACCOUNT_ID_0).generate_invoke_with_tip(3);
let account1_invoke_nonce1 =
tx_generator.account_with_id(ACCOUNT_ID_1).generate_invoke_with_tip(4);
tx_generator.account_with_id_mut(ACCOUNT_ID_1).generate_invoke_with_tip(4);

vec![account0_invoke_nonce1, account0_invoke_nonce2, account1_invoke_nonce1]
}
Expand All @@ -176,7 +176,7 @@ fn create_account_txs(
n_txs: usize,
) -> Vec<RpcTransaction> {
(0..n_txs)
.map(|_| tx_generator.account_with_id(account_id).generate_invoke_with_tip(1))
.map(|_| tx_generator.account_with_id_mut(account_id).generate_invoke_with_tip(1))
.collect()
}

Expand All @@ -198,14 +198,19 @@ where
/// list of transaction hashes, in the order they are expected to be in the mempool.
pub async fn run_integration_test_scenario<'a, Fut>(
tx_generator: &mut MultiAccountTransactionGenerator,
create_rpc_txs_fn: impl Fn(&mut MultiAccountTransactionGenerator) -> Vec<RpcTransaction>,
send_rpc_tx_fn: &'a mut dyn FnMut(RpcTransaction) -> Fut,
test_tx_hashes_fn: impl Fn(&[TransactionHash]) -> Vec<TransactionHash>,
) -> Vec<TransactionHash>
where
Fut: Future<Output = TransactionHash> + 'a,
{
let rpc_txs = create_txs_for_integration_test(tx_generator);
let rpc_txs = create_rpc_txs_fn(tx_generator);
let tx_hashes = send_rpc_txs(rpc_txs, send_rpc_tx_fn).await;
test_tx_hashes_fn(&tx_hashes)
}

pub fn test_tx_hashes_for_integration_test(tx_hashes: &[TransactionHash]) -> Vec<TransactionHash> {
// Return the transaction hashes in the order they should be given by the mempool:
// Transactions from the same account are ordered by nonce; otherwise, higher tips are given
// priority.
Expand Down
44 changes: 38 additions & 6 deletions crates/starknet_integration_tests/tests/end_to_end_flow_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ use starknet_api::transaction::TransactionHash;
use starknet_integration_tests::flow_test_setup::{FlowTestSetup, SequencerSetup};
use starknet_integration_tests::utils::{
create_integration_test_tx_generator,
create_txs_for_integration_test,
run_integration_test_scenario,
test_tx_hashes_for_integration_test,
};
use starknet_sequencer_infra::trace_util::configure_tracing;
use starknet_types_core::felt::Felt;
Expand Down Expand Up @@ -49,6 +51,7 @@ async fn end_to_end_flow(mut tx_generator: MultiAccountTransactionGenerator) {
);

let next_height = INITIAL_HEIGHT.unchecked_next();
let n_heights = next_height.iter_up_to(LAST_HEIGHT.unchecked_next()).count();
let heights_to_build = next_height.iter_up_to(LAST_HEIGHT.unchecked_next());
let expected_content_ids = [
Felt::from_hex_unchecked(
Expand All @@ -66,15 +69,44 @@ async fn end_to_end_flow(mut tx_generator: MultiAccountTransactionGenerator) {
// We start at height 1, so we need to skip the proposer of the initial height.
expected_proposer_iter.next().unwrap();

let create_rpc_txs_scenarios =
[create_txs_for_integration_test, create_txs_for_integration_test];

let test_tx_hashes_scenarios =
[test_tx_hashes_for_integration_test, test_tx_hashes_for_integration_test];

assert_eq!(
n_heights,
expected_content_ids.len(),
"Expected the same number of heights and content ids"
);
assert_eq!(
n_heights,
create_rpc_txs_scenarios.len(),
"Expected the same number of heights and scenarios"
);
assert_eq!(
n_heights,
test_tx_hashes_scenarios.len(),
"Expected the same number of heights and scenarios"
);

// Build multiple heights to ensure heights are committed.
for (height, expected_content_id) in itertools::zip_eq(heights_to_build, expected_content_ids) {
for (height, expected_content_id, create_rpc_txs_fn, test_tx_hashes_fn) in itertools::izip!(
heights_to_build,
expected_content_ids,
create_rpc_txs_scenarios.iter(),
test_tx_hashes_scenarios.iter(),
) {
debug!("Starting height {}.", height);
// Create and send transactions.
let expected_batched_tx_hashes =
run_integration_test_scenario(&mut tx_generator, &mut |tx| {
sequencer_to_add_txs.assert_add_tx_success(tx)
})
.await;
let expected_batched_tx_hashes = run_integration_test_scenario(
&mut tx_generator,
create_rpc_txs_fn,
&mut |tx| sequencer_to_add_txs.assert_add_tx_success(tx),
test_tx_hashes_fn,
)
.await;
let expected_validator_id = expected_proposer_iter
.next()
.unwrap()
Expand Down
28 changes: 20 additions & 8 deletions crates/starknet_integration_tests/tests/mempool_p2p_flow_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,10 @@ use starknet_integration_tests::utils::{
create_gateway_config,
create_http_server_config,
create_integration_test_tx_generator,
create_txs_for_integration_test,
run_integration_test_scenario,
test_rpc_state_reader_config,
test_tx_hashes_for_integration_test,
};
use starknet_mempool_p2p::config::MempoolP2pConfig;
use starknet_mempool_p2p::MEMPOOL_TOPIC;
Expand Down Expand Up @@ -128,10 +130,15 @@ async fn test_mempool_sends_tx_to_other_peer(mut tx_generator: MultiAccountTrans
let mut expected_txs = HashSet::new();

// Create and send transactions.
let _tx_hashes = run_integration_test_scenario(&mut tx_generator, &mut |tx: RpcTransaction| {
expected_txs.insert(tx.clone()); // push the sent tx to the expected_txs list
add_tx_http_client.assert_add_tx_success(tx)
})
let _tx_hashes = run_integration_test_scenario(
&mut tx_generator,
create_txs_for_integration_test,
&mut |tx: RpcTransaction| {
expected_txs.insert(tx.clone()); // push the sent tx to the expected_txs list
add_tx_http_client.assert_add_tx_success(tx)
},
test_tx_hashes_for_integration_test,
)
.await;

while !expected_txs.is_empty() {
Expand Down Expand Up @@ -161,10 +168,15 @@ async fn test_mempool_receives_tx_from_other_peer(

let mut expected_txs = HashSet::new();

let _tx_hashes = run_integration_test_scenario(&mut tx_generator, &mut |tx: RpcTransaction| {
expected_txs.insert(tx.clone());
ready(TransactionHash::default()) // using the default value because we don't use the hash anyways.
})
let _tx_hashes = run_integration_test_scenario(
&mut tx_generator,
create_txs_for_integration_test,
&mut |tx: RpcTransaction| {
expected_txs.insert(tx.clone());
ready(TransactionHash::default()) // using the default value because we don't use the hash anyways.
},
test_tx_hashes_for_integration_test,
)
.await;
for tx in &expected_txs {
broadcast_channels
Expand Down