Skip to content

Commit

Permalink
test(mempool): add fee escalation unit test (#1338)
Browse files Browse the repository at this point in the history
  • Loading branch information
elintul authored Oct 13, 2024
1 parent c161cca commit 57822fd
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 11 deletions.
2 changes: 1 addition & 1 deletion crates/mempool/src/mempool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ pub struct Mempool {
mempool_state: HashMap<ContractAddress, Nonce>,
// The most recent account nonces received, for all account in the pool.
account_nonces: AccountToNonce,
// TODO(Elin): make configurable.
// TODO(Elin): make configurable; should be bounded?
// Percentage increase for tip and max gas price to enable transaction replacement.
fee_escalation_percentage: u8, // E.g., 10 for a 10% increase.
}
Expand Down
72 changes: 69 additions & 3 deletions crates/mempool/src/mempool_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ use crate::{add_tx_input, tx};
struct MempoolContent {
tx_pool: Option<TransactionPool>,
tx_queue_content: Option<TransactionQueueContent>,
fee_escalation_percentage: u8,
}

impl MempoolContent {
Expand All @@ -42,17 +43,17 @@ impl MempoolContent {

impl From<MempoolContent> for Mempool {
fn from(mempool_content: MempoolContent) -> Mempool {
let MempoolContent { tx_pool, tx_queue_content } = mempool_content;
let MempoolContent { tx_pool, tx_queue_content, fee_escalation_percentage } =
mempool_content;
Mempool {
tx_pool: tx_pool.unwrap_or_default(),
tx_queue: tx_queue_content
.map(|content| content.complete_to_tx_queue())
.unwrap_or_default(),
fee_escalation_percentage,
// TODO: Add implementation when needed.
mempool_state: Default::default(),
account_nonces: Default::default(),
// TODO(Elin): add field to builder, together with tests.
fee_escalation_percentage: 0,
}
}
}
Expand All @@ -61,6 +62,7 @@ impl From<MempoolContent> for Mempool {
struct MempoolContentBuilder {
tx_pool: Option<TransactionPool>,
tx_queue_content_builder: TransactionQueueContentBuilder,
fee_escalation_percentage: u8,
}

impl MempoolContentBuilder {
Expand Down Expand Up @@ -92,10 +94,16 @@ impl MempoolContentBuilder {
self
}

fn with_fee_escalation_percentage(mut self, fee_escalation_percentage: u8) -> Self {
self.fee_escalation_percentage = fee_escalation_percentage;
self
}

fn build(self) -> MempoolContent {
MempoolContent {
tx_pool: self.tx_pool,
tx_queue_content: self.tx_queue_content_builder.build(),
fee_escalation_percentage: self.fee_escalation_percentage,
}
}

Expand Down Expand Up @@ -586,6 +594,64 @@ fn test_add_tx_fills_nonce_gap(mut mempool: Mempool) {
expected_mempool_content.assert_eq(&mempool);
}

#[rstest]
fn test_fee_escalation_valid_replacement() {
let increased_values = [
99, // Exactly increase percentage.
100, // More than increase percentage,
180, // More than 100% increase, to check percentage calculation.
];
for increased_value in increased_values {
// Setup.
let tx = tx!(tx_hash: 1, tip: 90, max_l2_gas_price: 90, tx_nonce: 1, sender_address: "0x0");
let mut mempool = MempoolContentBuilder::new()
.with_pool([tx])
.with_fee_escalation_percentage(10)
.build_into_mempool();

let valid_replacement_input = add_tx_input!(tx_hash: 2, tip: increased_value,
max_l2_gas_price: u128::from(increased_value), tx_nonce: 1, sender_address: "0x0");

// Test and assert.
add_tx(&mut mempool, &valid_replacement_input);

// Verify transaction was replaced.
let expected_mempool_content =
MempoolContentBuilder::new().with_pool([valid_replacement_input.tx]).build();
expected_mempool_content.assert_eq(&mempool);
}
}

#[rstest]
fn test_fee_escalation_invalid_replacement() {
// Setup.
let tx = tx!(tx_hash: 1, tip: 100, max_l2_gas_price: 100, tx_nonce: 1, sender_address: "0x0");
let mut mempool = MempoolContentBuilder::new()
.with_pool([tx.clone()])
.with_fee_escalation_percentage(10)
.build_into_mempool();

let input_not_enough_tip = add_tx_input!(tx_hash: 3, tip: 109, max_l2_gas_price: 110,
tx_nonce: 1, sender_address: "0x0");
let input_not_enough_gas_price = add_tx_input!(tx_hash: 4, tip: 110, max_l2_gas_price: 109,
tx_nonce: 1, sender_address: "0x0");
let input_not_enough_both = add_tx_input!(tx_hash: 5, tip: 109, max_l2_gas_price: 109,
tx_nonce: 1, sender_address: "0x0");

// Test and assert.
for input in [&input_not_enough_tip, &input_not_enough_gas_price, &input_not_enough_both] {
add_tx_expect_error(
&mut mempool,
input,
MempoolError::DuplicateNonce { address: contract_address!("0x0"), nonce: nonce!(1) },
);
}

// Verify transaction was not replaced.
let expected_mempool_content = MempoolContentBuilder::new().with_pool([tx]).build();
expected_mempool_content.assert_eq(&mempool);
}

// `commit_block` tests.

#[rstest]
Expand Down
23 changes: 16 additions & 7 deletions crates/mempool/src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ use crate::mempool::Mempool;
macro_rules! tx {
(tip: $tip:expr, tx_hash: $tx_hash:expr, sender_address: $sender_address:expr,
tx_nonce: $tx_nonce:expr, max_l2_gas_price: $max_l2_gas_price:expr) => {{
use starknet_api::block::GasPrice;
use starknet_api::core::{ContractAddress, PatriciaKey};
use starknet_api::executable_transaction::Transaction;
use starknet_api::hash::StarkHash;
use starknet_api::test_utils::invoke::executable_invoke_tx;
Expand All @@ -26,16 +28,15 @@ macro_rules! tx {
TransactionHash,
ValidResourceBounds,
};
use mempool_test_utils::starknet_api_test_utils::VALID_L2_GAS_MAX_PRICE_PER_UNIT;


let resource_bounds = ValidResourceBounds::AllResources(AllResourceBounds {
l2_gas: ResourceBounds {
max_price_per_unit: $max_l2_gas_price.into(),
max_price_per_unit: GasPrice($max_l2_gas_price),
..Default::default()
},
..Default::default()
});

Transaction::Invoke(executable_invoke_tx(invoke_tx_args!{
sender_address: contract_address!($sender_address),
tx_hash: TransactionHash(StarkHash::from($tx_hash)),
Expand All @@ -44,9 +45,10 @@ macro_rules! tx {
resource_bounds,
}))
}};
(tip: $tip:expr, tx_hash: $tx_hash:expr, sender_address: $sender_address:expr, tx_nonce: $tx_nonce:expr) => {
(tip: $tip:expr, tx_hash: $tx_hash:expr, sender_address: $sender_address:expr, tx_nonce: $tx_nonce:expr) => {{
use mempool_test_utils::starknet_api_test_utils::VALID_L2_GAS_MAX_PRICE_PER_UNIT;
tx!(tip: $tip, tx_hash: $tx_hash, sender_address: $sender_address, tx_nonce: $tx_nonce, max_l2_gas_price: VALID_L2_GAS_MAX_PRICE_PER_UNIT)
};
}};
(tx_hash: $tx_hash:expr, sender_address: $sender_address:expr, tx_nonce: $tx_nonce:expr) => {
tx!(tip: 0, tx_hash: $tx_hash, sender_address: $sender_address, tx_nonce: $tx_nonce)
};
Expand All @@ -65,6 +67,9 @@ macro_rules! tx {
(tx_nonce: $tx_nonce:expr) => {
tx!(tip: 0, tx_hash: 0, sender_address: "0x0", tx_nonce: $tx_nonce)
};
(tx_hash: $tx_hash:expr, tip: $tip:expr, max_l2_gas_price: $max_l2_gas_price:expr, tx_nonce: $tx_nonce:expr, sender_address: $sender_address:expr) => {
tx!(tip: $tip, tx_hash: $tx_hash, sender_address: $sender_address, tx_nonce: $tx_nonce, max_l2_gas_price: $max_l2_gas_price)
};
}

/// Creates an input for `add_tx` with the given field subset (the rest receive default values).
Expand All @@ -78,13 +83,14 @@ macro_rules! add_tx_input {
let tx = $crate::tx!(tip: $tip, tx_hash: $tx_hash, sender_address: $sender_address, tx_nonce: $tx_nonce, max_l2_gas_price: $max_l2_gas_price);
let address = contract_address!($sender_address);
let account_nonce = nonce!($account_nonce);
let account_state = AccountState {address, nonce: account_nonce};
let account_state = AccountState { address, nonce: account_nonce };

AddTransactionArgs { tx, account_state }
}};
(tip: $tip:expr, tx_hash: $tx_hash:expr, sender_address: $sender_address:expr,
tx_nonce: $tx_nonce:expr, account_nonce: $account_nonce:expr) => {{
add_tx_input!(tip: $tip, tx_hash: $tx_hash, sender_address: $sender_address, tx_nonce: $tx_nonce, account_nonce: $account_nonce, max_l2_gas_price: VALID_L2_GAS_MAX_PRICE_PER_UNIT)
use mempool_test_utils::starknet_api_test_utils::VALID_L2_GAS_MAX_PRICE_PER_UNIT;
add_tx_input!(tip: $tip, tx_hash: $tx_hash, sender_address: $sender_address, tx_nonce: $tx_nonce, account_nonce: $account_nonce, max_l2_gas_price: VALID_L2_GAS_MAX_PRICE_PER_UNIT)
}};
(tx_hash: $tx_hash:expr, sender_address: $sender_address:expr, tx_nonce: $tx_nonce:expr, account_nonce: $account_nonce:expr) => {
add_tx_input!(tip: 0, tx_hash: $tx_hash, sender_address: $sender_address, tx_nonce: $tx_nonce, account_nonce: $account_nonce)
Expand All @@ -107,6 +113,9 @@ macro_rules! add_tx_input {
(tx_hash: $tx_hash:expr, tx_nonce: $tx_nonce:expr) => {
add_tx_input!(tip: 0, tx_hash: $tx_hash, sender_address: "0x0", tx_nonce: $tx_nonce, account_nonce: 0)
};
(tx_hash: $tx_hash:expr, tip: $tip:expr, max_l2_gas_price: $max_l2_gas_price:expr, tx_nonce: $tx_nonce:expr, sender_address: $sender_address:expr) => {
add_tx_input!(tip: $tip, tx_hash: $tx_hash, sender_address: $sender_address, tx_nonce: $tx_nonce, account_nonce: 0, max_l2_gas_price: $max_l2_gas_price)
};
}

#[track_caller]
Expand Down

0 comments on commit 57822fd

Please sign in to comment.