Skip to content

Commit

Permalink
chore(mempool): convert unit to flow test (#1741)
Browse files Browse the repository at this point in the history
This test is flow-specific, hence must be tested in a flow test.
Specifically, this tests behavior when the mempool is in "validate mode"
which isn't supported yet.

Note: removed from the test the behavior where commit block decreased
the known account nonce, which had two bugs:
1. commit_block from a different leader reduced the account nonce,
this is a reorg and isnt unsupported yet, and when it is, it would
use a dedicated API test.
2. it decreased the nonce back to 0, which doesn't make sense because at
least one tx must have been accepted in a block --- also fixed a
similar issue in a separate flow.
Will add an assert in commit_block on minimum nonce being 1 in
subseuqent commits.

There will also be a subsequent commit that decreases the nonces in
`test_commit_block_includes_proposed_txs_subset`, they are too high for
readability. Also the test logic has some redundancies.

Co-Authored-By: Gilad Chase <[email protected]>
  • Loading branch information
giladchase and Gilad Chase authored Nov 3, 2024
1 parent 9b7cf84 commit 9d8f82b
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 45 deletions.
42 changes: 0 additions & 42 deletions crates/mempool/src/mempool_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -581,48 +581,6 @@ fn test_commit_block_includes_all_proposed_txs() {
expected_mempool_content.assert_eq(&mempool);
}

#[rstest]
fn test_commit_block_from_different_leader() {
// Setup.
let tx_address_0_nonce_3 = tx!(tx_hash: 1, address: "0x0", tx_nonce: 3);
let tx_address_0_nonce_5 = tx!(tx_hash: 2, address: "0x0", tx_nonce: 5);
let tx_address_0_nonce_6 = tx!(tx_hash: 3, address: "0x0", tx_nonce: 6);
let tx_address_1_nonce_2 = tx!(tx_hash: 4, address: "0x1", tx_nonce: 2);

let queued_txs = [TransactionReference::new(&tx_address_1_nonce_2)];
let pool_txs = [
tx_address_0_nonce_3,
tx_address_0_nonce_5,
tx_address_0_nonce_6.clone(),
tx_address_1_nonce_2.clone(),
];
let mut mempool = MempoolContentBuilder::new()
.with_pool(pool_txs)
.with_priority_queue(queued_txs)
.build_into_mempool();

// Test.
let nonces = [
("0x0", 5),
("0x1", 0), // A hole, missing nonce 1 for address "0x1".
("0x2", 1),
];
let tx_hashes = [
1, 2, // Hashes known to mempool.
5, 6, // Hashes unknown to mempool, from a different node.
];
commit_block(&mut mempool, nonces, tx_hashes);

// Assert.
let expected_queue_txs = [TransactionReference::new(&tx_address_0_nonce_6)];
let expected_pool_txs = [tx_address_0_nonce_6, tx_address_1_nonce_2];
let expected_mempool_content = MempoolContentBuilder::new()
.with_pool(expected_pool_txs)
.with_priority_queue(expected_queue_txs)
.build();
expected_mempool_content.assert_eq(&mempool);
}

// Fee escalation tests.

#[rstest]
Expand Down
34 changes: 31 additions & 3 deletions crates/mempool/tests/flow_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,15 +147,15 @@ fn test_commit_block_includes_proposed_txs_subset(mut mempool: Mempool) {
&[tx_address_1_nonce_1.tx.clone(), tx_address_0_nonce_3.tx],
);

// Not included in block: address "0x2" nonce 2, address "0x1" nonce 1.
let nonces = [("0x0", 3), ("0x1", 0)];
// Not included in block: address "0x2" nonce 2, address "0x1" nonce 2.
let nonces = [("0x0", 3), ("0x1", 1)];
let tx_hashes = [1, 4];
commit_block(&mut mempool, nonces, tx_hashes);

get_txs_and_assert_expected(
&mut mempool,
2,
&[tx_address_2_nonce_2.tx, tx_address_1_nonce_1.tx],
&[tx_address_2_nonce_2.tx, tx_address_1_nonce_2.tx],
);
}

Expand Down Expand Up @@ -216,3 +216,31 @@ fn test_flow_commit_block_rewinds_queued_nonce(mut mempool: Mempool) {
// Nonces 3 and 4 were re-enqueued correctly.
get_txs_and_assert_expected(&mut mempool, 2, &[tx_nonce_3.tx, tx_nonce_4.tx]);
}

#[rstest]
fn test_flow_commit_block_from_different_leader(mut mempool: Mempool) {
// Setup.
// TODO: set the mempool to `validate` mode once supported.

let tx_nonce_2 = add_tx_input!(tx_hash: 1, address: "0x0", tx_nonce: 2, account_nonce: 2);
let tx_nonce_3 = add_tx_input!(tx_hash: 2, address: "0x0", tx_nonce: 3, account_nonce: 2);
let tx_nonce_4 = add_tx_input!(tx_hash: 3, address: "0x0", tx_nonce: 4, account_nonce: 2);

for input in [&tx_nonce_2, &tx_nonce_3, &tx_nonce_4] {
add_tx(&mut mempool, input);
}

// Test.
let nonces = [("0x0", 3), ("0x1", 2)];
let tx_hashes = [
1, // Address 0: known hash accepted for nonce 2.
99, // Address 0: unknown hash accepted for nonce 3.
4, // Unknown Address 1 (with unknown hash) for nonce 2.
];
commit_block(&mut mempool, nonces, tx_hashes);

// Assert: two stale transactions were removed, one was added to a block by a different leader
// and the other "lost" to a different transaction with the same nonce that was added by the
// different leader.
get_txs_and_assert_expected(&mut mempool, 1, &[tx_nonce_4.tx]);
}

0 comments on commit 9d8f82b

Please sign in to comment.