feat(protocol): PCR-2 base changes #5270
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 4 Skipped Deployments
|
cyberphysic4l
changed the base branch from
core-protocol/procotol-version-4
to
protocol/pcr2-clean
| params: &AdvanceEpochParams, | ||
| ) -> Result<ProgrammableTransaction, ExecutionError> { | ||
| let call_arg_vec = vec
There was a problem hiding this comment.
I would either add a comment here, that construct_advance_epoch_pt_impl adds more parameters here in the front, so that you directly see that this matches the "advance epoch" function, or we move the code from construct_advance_epoch_pt_impl and duplicate it to _v1 and _v2, so that it is clearly visible?
There was a problem hiding this comment.
But yeah, not a blocking issue from my side, PR approved.
There was a problem hiding this comment.
Hey @cyberphysic4l , a few nitpicks and a proposal with more impact to discuss.
crates/iota-types/src/iota_system_state/iota_system_state_inner_v2.rs
Outdated
Show resolved
Hide resolved
| pub safe_mode_computation_charges: u64, | ||
| /// Amount of burned computation charges accumulated during safe mode. | ||
| #[schemars(with = "BigInt<u64>")] | ||
| #[serde_as(as = "Readable<BigInt<u64>, _>")] | ||
| pub safe_mode_computation_charges_burned: u64, |
There was a problem hiding this comment.
This breaks backwards compatibility for the JSON-RPC API (both the rename and the addition of the new field).
If we could version this as well, it would allow more flexibility in handling versioning in the API as well.
An idea would be to restructure the parent module to
iota_system_state/
v1/
v2/Another would be to split iota_system_state_summary into iota_system_state_summary::{v1,v2}.
And last we could introduce a new IotaSystemStateSummaryV2.
There was a problem hiding this comment.
Introducing a V2 would require an enum like we have for IotaSystemState as well, and changing IotaSystemStateTrait and quite a lot of other code. Is a new version the only way we can change types in the JSON-RPC API in general, just like with the move types and their rust versions?
There was a problem hiding this comment.
I am afraid so. If we want to maintain backward compatibility we need the same response type, or something that is compatible with the existing response type.
This is true for every API, application, or library that depends on the public API iota-types as well. Even switching from a struct to an enum would be a breaking change because it would require different handling in any dependent code.
So the only way to maintain backward compatibility in every component that is affected by these changes would be to use a different struct.
We could break backwards compatibility for iota-types on the premise that is likely only used in the parent workspace and internal libraries. If that is the case, an alternative would be to deal with the two versions on iota-json-rpc-types and refactor APIs dependent on iota-types. The downside is that we introduce additional representations and logic in our libraries for the same information.
There was a problem hiding this comment.
So the only way to maintain backward compatibility in every component that is affected by these changes would be to use a different
struct.
Actually this is not precise. At the very least we must break the IotaSystemStateTrait API.
Description of change
protocol_defined_base_feeand config parameterbase_gas_price.protocol_defined_base_feeenables the base fee defined bybase_gas_pricein the place of the reference gas price defined by the gas price survey in earlier protocol versions. Thisbase_gas_pricefor each transaction is burned, and any additional gas price will be passed to the validators as a tip.advance_epochfunction in the iota framework is updated to burn the base fee and pass additional fee to validators. The function now takes an additional call argument -computation_charge_burned- to allow the base fee component and tip to be distinguished.IotaSystemStateV2and associated types to include asafe_mode_computation_charges_burnedfield. Some tests are updated to deal with this new system state version. Updates were carried out following the guide provided in crates/iota-framework/packages/iota-system/sources/iota_system.moveChangeEpochV2transaction kind is introduced for the epoch change system transaction corresponding to theadvance_epochfunction changes mentioned above. The new feature flag,protocol_defined_base_feeis used to determined when the newChangeEpochV2transaction kind should be used and the additional call argument be passed to theadvance_epochfunctionSystemEpochInfoEventV2to include thetips_amountfield specifying how much of user computation fees are passed to validators.Links to any relevant issues
Fixes #3122
Should be merged along with iotaledger/iota-rust-sdk#22
Type of change
How the change has been tested
Existing tests pass. Added move tests for tips in
rewards_distribution_tests.moveChange checklist
Tick the boxes that are relevant to your changes, and delete any items that are not.