gas param config change event #682
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
arhag
requested changes
Feb 1, 2024
arhag
previously requested changes
Feb 21, 2024
elmato
reviewed
Feb 23, 2024
| @@ -43,6 +42,11 @@ struct config_wrapper { | |||
| void set_fee_parameters(const fee_parameters& fee_params, | |||
| bool allow_any_to_be_unspecified); | |||
|
|
|||
| void update_gas_params(eosio::asset ram_price_mb, uint64_t minimum_gas_price); | |||
| void update_gas_params2(std::optional<uint64_t> gas_txnewaccount, std::optional<uint64_t> gas_newaccount, std::optional<uint64_t> gas_txcreate, std::optional<uint64_t> gas_codedeposit, std::optional<uint64_t> gas_sset, std::optional<uint64_t> minimum_gas_price); | |||
There was a problem hiding this comment.
shouldn't this function be be update_consensus_parameters since minimum_gas_price was moved away from the gas_parameter_type ?
struct gas_parameter_type {
uint64_t gas_txnewaccount = 0;
uint64_t gas_newaccount = 25000;
uint64_t gas_txcreate = 32000;
uint64_t gas_codedeposit = 200;
uint64_t gas_sset = 20000;
};
struct consensus_parameter_data_v0 {
uint64_t minimum_gas_price = 0;
gas_parameter_type gas_parameter;
};
src/config_wrapper.cpp
Outdated
| @@ -121,14 +126,29 @@ void config_wrapper::set_fee_parameters(const fee_parameters& fee_params, | |||
| bool allow_any_to_be_unspecified) | |||
| { | |||
| if (fee_params.gas_price.has_value()) { | |||
| _cached_config.gas_price = *fee_params.gas_price; | |||
| eosio::check(*fee_params.gas_price >= one_gwei, "gas_price must >= 1Gwei"); | |||
| if (_cached_config.evm_version.has_value() && _cached_config.evm_version->cached_version >= 1) { | |||
There was a problem hiding this comment.
why not doing ?
if(get_evm_version() >= 1) {...
src/config_wrapper.cpp
Outdated
| constexpr uint64_t storage_slot_bytes = 346; | ||
|
|
||
| eosio::check(gas_per_byte_f >= 0.0, "gas_per_byte must >= 0"); | ||
| eosio::check(gas_per_byte_f * contract_fixed_bytes < (double)(0x7ffffffffffull), "gas_per_byte too big"); |
There was a problem hiding this comment.
Change 0x7ffffffffffull to
constexpr uint64_t max_gas_per_byte = (1ull << 43) - 1;
src/config_wrapper.cpp
Outdated
| void config_wrapper::update_gas_params2(std::optional<uint64_t> gas_txnewaccount, std::optional<uint64_t> gas_newaccount, std::optional<uint64_t> gas_txcreate, std::optional<uint64_t> gas_codedeposit, std::optional<uint64_t> gas_sset, std::optional<uint64_t> minimum_gas_price) | ||
| { | ||
| // for simplicity, ensure last (cached) evm_version >= 1, not touching promote logic | ||
| eosio::check(_cached_config.evm_version.has_value() && _cached_config.evm_version->cached_version >= 1, |
There was a problem hiding this comment.
why not doing ?
if(get_evm_version() >= 1) {...
src/config_wrapper.cpp
Outdated
| if (gas_txcreate.has_value()) v.gas_parameter.gas_txcreate = *gas_txcreate; | ||
| if (gas_codedeposit.has_value()) v.gas_parameter.gas_codedeposit = *gas_codedeposit; | ||
| if (gas_sset.has_value()) { | ||
| eosio::check(*gas_sset >= 2900, "G_sset must >= 2900"); |
There was a problem hiding this comment.
2900 is used in many places, we should create a constant
elmato
approved these changes
Feb 23, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves #679