Amend GasSchedule Updator CLI for easy scaling of minimum Gas unit price. #292
Conversation
supra-yoga
changed the title
isaacdoidge
left a comment
isaacdoidge
left a comment
There was a problem hiding this comment.
Since we need to update the code anyway, wouldn't it be simpler to just stick with the current way of doing things? It seems that if we use the new approach then we might forget to update the code.
|
|
||
| /// Returns true if the change set contains any additions or deletions. | ||
| /// This indicates that a feature version bump is required. | ||
| /// A mutation alone does not require a feature version bump. |
There was a problem hiding this comment.
Why do we not need to increase the version when mutation occurs? I think we do need to do this.
There was a problem hiding this comment.
From reading the ver.rs file, I was able to infer that we should increase the gas version only if we modify the gas calculation algorithm, e.g.:
- Add a new instruction or opcode with a new cost behavior.
- Change the internal formula for how storage or IO gas is computed.
- Upgrade Move VM gas model
In our case, adjusting min_gas_unit_price will only affect the Mempool acceptance, not computation. Therefore, it would not require a version bump, whereas if we want to add a new parameter for SupraEVM, then we are adding a new entry to the GasSchedule altogether, which mandates a version bump.
Do correct me if my inference of the ver.rs file is wrong :)
| /// This script can be submitted as a governance proposal to update the on-chain gas schedule. | ||
| /// The script includes a comment section listing the contents of the gas schedule in a human readable format. | ||
| #[derive(Parser, Debug)] | ||
| pub enum GasScheduleGenerator { |
There was a problem hiding this comment.
It would be good to also mention that the schedule defined in the Rust code should also be updated whenever the on-chain schedule is updated, including LATEST_GAS_FEATURE_VERSION and the documentation in ver.rs.
The idea behind this PR was to formalize a way to update the on-chain GasSchedule with the desired changes, though I agree that the current method of updating the constants and generating a new schedule is simpler. I haven't tried adding a new parameter to the hard-coded values of gas params here: https://github.com/Entropy-Foundation/aptos-core/blob/dev/aptos-move/aptos-gas-schedule/src/gas_schedule/transaction.rs#L60 Since we will be adding a new parameter for SupraEVM, I'll go ahead and test this scenario. |
Description
Tooling for https://github.com/Entropy-Foundation/foundation-multisig-tools/issues/176.
Key Areas to Review
Need to review places where
GasScheduleV2is being serialized and deserialized, as changes have been made to the ser/de behavior.Type of Change
Which Components or Systems Does This Change Impact?