-
Notifications
You must be signed in to change notification settings - Fork 624
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
Cap maximum gas price #4308
Cap maximum gas price #4308
Conversation
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.
Please add a test that asserts the gas price does not exceed maximum gas price even when there is congestion.
Added new tests and moved creation of congestion to a separate function |
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.
Well done!
chain/client/tests/process_blocks.rs
Outdated
@@ -77,6 +77,79 @@ pub fn create_nightshade_runtimes(genesis: &Genesis, n: usize) -> Vec<Arc<dyn Ru | |||
.collect() | |||
} | |||
|
|||
/// Create environment and set of transactions which cause congestion on the chain. | |||
fn create_env_with_congestion( |
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.
Nice
chain/client/tests/process_blocks.rs
Outdated
env.produce_block(0, i); | ||
let block = env.clients[0].chain.get_block_by_height(i).unwrap().clone(); | ||
let protocol_version = env.clients[0] | ||
.runtime_adapter | ||
.get_epoch_protocol_version(block.header().epoch_id()) | ||
.unwrap(); | ||
let min_gas_price = | ||
env.clients[0].chain.block_economics_config.min_gas_price(protocol_version); | ||
if block.header().gas_price() > 10 * min_gas_price { | ||
return true; |
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.
Please also assert that congestion actually happens. You can do this by asserting that gas usage is at least as large as gas limit.
- Actually update nightly protocol version to catch CapMaxGasPrice feature which I forgot to do here: #4308 - Fix tests for `RestoreReceiptsAfterFix` feature started failing after it. Reason: protocol feature have to be set in produced blocks explicitly like in `storage_usage_fix_tests`
Cap maximum gas price: #4302
Test plan
test_cap_max_gas_price
checks thatmax_gas_price <= 10 * min_gas_price
for the new protocol version.process_blocks.rs:cap_max_gas_price_tests
check thatgas_price
may exceed10 * min_gas_price
for the old protocol version but may not for the new one.