-
Notifications
You must be signed in to change notification settings - Fork 201
feat(l1): enforce minimum priority-fee floor at mempool admission #6604
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
base: main
Are you sure you want to change the base?
Changes from 3 commits
e70fa48
133710f
0aaec3e
c4dd603
bb31aa0
6e36410
01de0bd
8b7e49f
922e5e4
9d66aaa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -91,6 +91,12 @@ Node options: | |
| [env: ETHREX_MEMPOOL_MAX_SIZE=] | ||
| [default: 10000] | ||
|
|
||
| --mempool.min-tip <MIN_TIP_WEI> | ||
| Minimum effective priority fee (in wei) required for a transaction to be admitted into the mempool. For typed transactions this is `max_priority_fee_per_gas`; for legacy transactions it is `gas_price - base_fee`. Set to 0 to disable the floor. | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
|
||
| [env: ETHREX_MEMPOOL_MIN_TIP=] | ||
| [default: 1] | ||
|
|
||
| --precompute-witnesses | ||
| Once synced, computes execution witnesses upon receiving newPayload messages and stores them in local storage | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| use ethrex_blockchain::Blockchain; | ||
| use ethrex_blockchain::BlockchainOptions; | ||
| use ethrex_blockchain::constants::MAX_INITCODE_SIZE; | ||
| use ethrex_blockchain::constants::{ | ||
| TX_ACCESS_LIST_ADDRESS_GAS, TX_ACCESS_LIST_STORAGE_KEY_GAS, TX_CREATE_GAS_COST, | ||
|
|
@@ -467,3 +468,147 @@ fn blobs_bundle_insert_and_remove() { | |
| vec![None] | ||
| ); | ||
| } | ||
|
|
||
| // --- min-tip floor admission tests ---------------------------------------- | ||
|
|
||
| /// Builds a blockchain configured with `min_tip_wei` as the admission floor | ||
| /// (everything else permissive for tests). | ||
| fn blockchain_with_min_tip(store: Store, min_tip_wei: u64) -> Blockchain { | ||
| let mut bc = Blockchain::default_with_store(store); | ||
| let mut opts = bc.options.clone(); | ||
| opts.min_tip_wei = min_tip_wei; | ||
| bc.options = opts; | ||
| bc | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn zero_tip_eip1559_rejected_under_default_floor() { | ||
| let (config, header) = build_basic_config_and_header(false, false); | ||
| let store = setup_storage(config, header).await.expect("Storage setup"); | ||
| let blockchain = blockchain_with_min_tip(store, 1_000_000); | ||
|
|
||
| let tx = EIP1559Transaction { | ||
| max_priority_fee_per_gas: 0, | ||
| max_fee_per_gas: 1_000_000, | ||
| gas_limit: 50_000_000, | ||
| to: TxKind::Call(Address::from_low_u64_be(1)), | ||
| ..Default::default() | ||
| }; | ||
| let tx = Transaction::EIP1559Transaction(tx); | ||
|
|
||
| let res = blockchain | ||
| .validate_transaction(&tx, Address::random()) | ||
| .await; | ||
| assert!(matches!( | ||
| res, | ||
| Err(MempoolError::TipBelowMinimum { | ||
| actual: 0, | ||
| limit: 1_000_000, | ||
| }), | ||
| )); | ||
| } | ||
|
|
||
| #[tokio::test] | ||
|
Comment on lines
+585
to
+592
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Prompt To Fix With AIThis is a comment left during a code review.
Path: test/tests/blockchain/mempool_tests.rs
Line: 504-511
Comment:
**Negative assertions mask unrelated validation failures**
`at_floor_eip1559_passes_tip_check` and `floor_of_zero_admits_zero_tip` both use `assert!(!matches!(res, Err(MempoolError::TipBelowMinimum { .. })))`. This passes when `validate_transaction` returns `Ok(_)` OR when it returns any other `Err` variant. If a future refactor reorders checks and the tip guard is accidentally skipped while another error fires, these tests would still pass, providing false confidence. The real fix is to assert `Ok(_)` or to explicitly document that a different error is acceptable here.
How can I resolve this? If you propose a fix, please make it concise.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| async fn at_floor_eip1559_passes_tip_check() { | ||
| // Tip at the floor passes the min-tip check. The tx may still fail later | ||
| // (intrinsic gas, balance, etc.) — we only assert the tip check itself | ||
| // does not fire. | ||
| let (config, header) = build_basic_config_and_header(false, false); | ||
| let store = setup_storage(config, header).await.expect("Storage setup"); | ||
| let blockchain = blockchain_with_min_tip(store, 1_000_000); | ||
|
|
||
| let tx = EIP1559Transaction { | ||
| max_priority_fee_per_gas: 1_000_000, | ||
| max_fee_per_gas: 1_000_000, | ||
| gas_limit: 50_000_000, | ||
| to: TxKind::Call(Address::from_low_u64_be(1)), | ||
| ..Default::default() | ||
| }; | ||
| let tx = Transaction::EIP1559Transaction(tx); | ||
|
|
||
| let res = blockchain | ||
| .validate_transaction(&tx, Address::random()) | ||
| .await; | ||
| assert!(!matches!(res, Err(MempoolError::TipBelowMinimum { .. }))); | ||
| } | ||
|
|
||
| #[tokio::test] | ||
|
|
||
| async fn floor_of_zero_admits_zero_tip() { | ||
| // Operators can disable the floor with --mempool.min-tip 0. | ||
| let (config, header) = build_basic_config_and_header(false, false); | ||
| let store = setup_storage(config, header).await.expect("Storage setup"); | ||
| let blockchain = blockchain_with_min_tip(store, 0); | ||
|
|
||
| let tx = EIP1559Transaction { | ||
| max_priority_fee_per_gas: 0, | ||
| max_fee_per_gas: 0, | ||
| gas_limit: 50_000_000, | ||
| to: TxKind::Call(Address::from_low_u64_be(1)), | ||
| ..Default::default() | ||
| }; | ||
| let tx = Transaction::EIP1559Transaction(tx); | ||
|
|
||
| let res = blockchain | ||
| .validate_transaction(&tx, Address::random()) | ||
| .await; | ||
| assert!(!matches!(res, Err(MempoolError::TipBelowMinimum { .. }))); | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn legacy_effective_tip_below_floor_rejected() { | ||
| // Legacy tx: effective_tip = gas_price.saturating_sub(base_fee). | ||
| // Test header has no base_fee (None → 0), so effective_tip = gas_price. | ||
| let (config, header) = build_basic_config_and_header(false, false); | ||
| let store = setup_storage(config, header).await.expect("Storage setup"); | ||
| let blockchain = blockchain_with_min_tip(store, 1_000_000); | ||
|
|
||
| let tx = ethrex_common::types::LegacyTransaction { | ||
| gas_price: U256::from(999_999u64), // 1 wei below floor | ||
| gas: 50_000_000, | ||
| to: TxKind::Call(Address::from_low_u64_be(1)), | ||
| ..Default::default() | ||
| }; | ||
| let tx = Transaction::LegacyTransaction(tx); | ||
|
|
||
| let res = blockchain | ||
| .validate_transaction(&tx, Address::random()) | ||
| .await; | ||
| assert!(matches!( | ||
| res, | ||
| Err(MempoolError::TipBelowMinimum { | ||
| actual: 999_999, | ||
| limit: 1_000_000, | ||
| }), | ||
| )); | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn options_field_is_used_in_validate_transaction() { | ||
| // Smoke test that BlockchainOptions::min_tip_wei is consulted (not | ||
| // accidentally ignored if the option-plumbing breaks). | ||
| let (config, header) = build_basic_config_and_header(false, false); | ||
| let store = setup_storage(config, header).await.expect("Storage setup"); | ||
|
|
||
| let mut bc = Blockchain::default_with_store(store); | ||
| let mut opts = BlockchainOptions::default(); | ||
| opts.min_tip_wei = 5_000_000_000; // 5 gwei | ||
| bc.options = opts; | ||
|
|
||
| let tx = EIP1559Transaction { | ||
| max_priority_fee_per_gas: 1_000_000_000, // 1 gwei (below 5 gwei) | ||
| max_fee_per_gas: 5_000_000_000, | ||
| gas_limit: 50_000_000, | ||
| to: TxKind::Call(Address::from_low_u64_be(1)), | ||
| ..Default::default() | ||
| }; | ||
| let tx = Transaction::EIP1559Transaction(tx); | ||
|
|
||
| let res = bc.validate_transaction(&tx, Address::random()).await; | ||
| assert!(matches!( | ||
| res, | ||
| Err(MempoolError::TipBelowMinimum { | ||
| actual: 1_000_000_000, | ||
| limit: 5_000_000_000, | ||
| }), | ||
| )); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
6e36410