Conversation
- Add min_tx_gas field to VM config - Reject transactions whose gas_limit is below configured min_tx_gas - Floor receipt.gas_spent to at least min_tx_gas on successful exeuction
9d6c7fe to
114416c
Compare
herr-seppia
left a comment
There was a problem hiding this comment.
In addition to what's been commented, remember that the well-known config should be changed only during a release flow.
node/src/mempool.rs
Outdated
| #[error("gas price lower than minimum {0}")] | ||
| GasPriceTooLow(u64), | ||
| #[error("gas limit lower than minimum {0}")] | ||
| #[error("gas limit lower than minimum {0} LUX")] |
There was a problem hiding this comment.
Gas limit is not expressed in LUX
node/src/mempool.rs
Outdated
| let min_gas_limit = vm.min_gas_limit(); | ||
| if tx.inner.gas_limit() < min_gas_limit { | ||
| return Err(TxAcceptanceError::GasLimitTooLow(min_gas_limit)); | ||
| // Check global minimum gas limit and per-tx fee floor |
rusk/CHANGELOG.md
Outdated
|
|
||
| ### Added | ||
|
|
||
| - Add TX floor price feature, including for well-known chain ids [#3940] |
| .unwrap_or(false) | ||
| } | ||
|
|
||
| fn min_tx_gas(&self, height: u64) -> u64 { |
There was a problem hiding this comment.
I would return this as Option to be aligned with its meaning
rusk/src/lib/node/vm/config.rs
Outdated
| pub generation_timeout: Option<Duration>, | ||
|
|
||
| /// Minimum gas charged for any transaction. | ||
| pub min_tx_gas: u64, |
rusk/src/lib/node/vm/config/known.rs
Outdated
| const MAINNET_BLOB_ACTIVATION: FeatureActivation = | ||
| FeatureActivation::Height(MAINNET_AT_10_12_2025_AT_09_00_UTC); | ||
|
|
||
| const MAINNET_MIN_TX_GAS_ACTIVATION: FeatureActivation = |
There was a problem hiding this comment.
This should mention a timestamp also
rusk/src/lib/node/vm/config/known.rs
Outdated
| const TESTNET_AT_12_11_2025_AT_09_00_UTC: FeatureActivation = | ||
| FeatureActivation::Height(1_814_090); | ||
|
|
||
| const TESTNET_MIN_TX_GAS_ACTIVATION: FeatureActivation = |
vm/src/execute.rs
Outdated
| // feature min_tx_gas is enabled | ||
| use core::cmp::min; | ||
|
|
||
| let floor = min(config.min_tx_gas, receipt.gas_limit); |
There was a problem hiding this comment.
Why are you using the gas limit of the tx? Isn't enough to check if the gas spent is lower then the minimum enforced?
There was a problem hiding this comment.
Why are you using the gas limit of the tx? Isn't enough to check if the gas spent is lower then the minimum enforced?
Leftover, I can actually simplify this to the following now in that branch:
if receipt.gas_spent < config.min_tx_gas {
receipt.gas_spent = config.min_tx_gas;
}
98e9514 to
ff70264
Compare
| // Enforce gas_limit >= min_tx_gas if min_tx_gas feature is enabled | ||
| if config.min_tx_gas > 0 && tx.gas_limit() < config.min_tx_gas { | ||
| return Err(Error::Panic( | ||
| "transaction gas_limit is below the minimum required gas".into(), |
There was a problem hiding this comment.
I'd change the text of the panic message to:
"transaction gas limit is below the required minimum"
Resolves #3940