Skip to content

Conversation

@jkrvivian
Copy link
Contributor

@jkrvivian jkrvivian commented Oct 4, 2024

Description of change

This PR will be merged to the feature branch core-node/feat/remove-multiple-types.

Reduce amount of versioned types V2 to V1:

  • SystemParametersV1
  • IotaSystemStateInnerV1
  • VotingPowerInfoV1

This PR also includes renaming changes in accordance with the rules:

  • Add version postfix to those that will certainly be upgraded in the future.
  • No version postfix to those that won't be new version.
  • For the wrappers, we don't add Wrapper postfix.
  • For inner types, we don't add Inner, just add version postfix {structure}V1.

Structures with versioning:

  • SystemParametersV1
  • IotaSystemState (the wrapper)
  • IotaSystemStateV1
  • Validator (the wrapper)
  • ValidatorV1
  • ValidatorMetadataV1
  • ValidatorSetV1
  • ValidatorEpochInfoEventV1
  • SystemEpochInfoEventV1
  • StakingPoolV1
  • PoolTokenExchangeRateV1
  • StakedIotaV1
  • StorageFundV1
  • TimelockedStakedIotaV1
  • VotingPowerInfoV1

Structures that would not get changed, thus with no version postfix:

***** genesis *****

  • GenesisValidatorMetadata
  • GenesisChainParameters
  • TokenDistributionSchedule
  • TokenAllocation

***** validator cap *****

  • UnverifiedValidatorOperationCap
  • ValidatorOperationCap

***** validator *****

  • ValidatorJoinEvent
  • ValidatorLeaveEvent
  • StakingRequestEvent
  • UnstakingRequestEvent

Links to any relevant issues

Close #2089

Type of change

  • Enhancement
  • Breaking change

How the change has been tested

Run the build and test commands:

  • iota move build
  • iota move test

Change checklist

  • I have followed the contribution guidelines for this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have checked that new and existing unit tests pass locally with my changes

@jkrvivian jkrvivian added vm-language Issues related to the VM & Language Team node Issues related to the Core Node team labels Oct 4, 2024
@jkrvivian jkrvivian self-assigned this Oct 4, 2024
@jkrvivian jkrvivian requested a review from a team as a code owner October 4, 2024 09:34
@muXxer muXxer deleted the branch core-node/feat/remove-multiple-types October 4, 2024 10:28
@muXxer muXxer closed this Oct 4, 2024
@jkrvivian jkrvivian reopened this Oct 4, 2024
@alexsporn
Copy link
Member

Why did you add V1 suffixes to existing types?

@jkrvivian
Copy link
Contributor Author

Why did you add V1 suffixes to existing types?

Because there's V1 in the Rust side (crates/iota-types/src/iota_system_state/iota_system_state_inner_v1.rs), I thought it'd be good to align the naming to start with V1.

Still the question is, should we have V1 for all the existing structures? Or should we start without versioning {structure}, and for the next version it goes {structure}V2?

for example:

ValidatorV1{}
ValidatorV2{}
...

V.S.

Validator{}
ValidatorV2{}
...

@github-actions
Copy link
Contributor

github-actions bot commented Oct 8, 2024

✅ Vercel Preview Deployment is ready!

View Preview

Copy link
Contributor

@miker83z miker83z left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

Copy link
Member

@lzpap lzpap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we modify the framework and don't intend to release a new protocol version for this, we should update the framework-snapshot bytes for protocol version 1. If this is not done then nodes will panic on startup that they have a different framework loaded from the snapshot than what is present in the node software.

We don't have to do this once we start bumping protocol versions, then we only create a new snapshot cut when the new protocol version is introduced.

@jkrvivian
Copy link
Contributor Author

Since we modify the framework and don't intend to release a new protocol version for this, we should update the framework-snapshot bytes for protocol version 1. If this is not done then nodes will panic on startup that they have a different framework loaded from the snapshot than what is present in the node software.

We don't have to do this once we start bumping protocol versions, then we only create a new snapshot cut when the new protocol version is introduced.

Yes! Thanks for the reminder, we will do this update in a sole commit in the feature branch so it will not be squashed.

@lzpap
Copy link
Member

lzpap commented Oct 9, 2024

Since we modify the framework and don't intend to release a new protocol version for this, we should update the framework-snapshot bytes for protocol version 1. If this is not done then nodes will panic on startup that they have a different framework loaded from the snapshot than what is present in the node software.
We don't have to do this once we start bumping protocol versions, then we only create a new snapshot cut when the new protocol version is introduced.

Yes! Thanks for the reminder, we will do this update in a sole commit in the feature branch so it will not be squashed.

IMHO it doesn't matter right now if the commit is squashed or not, I would keep it together with the framework changes (this PR), unless you plan to modfiy other stuff in the iota-framework crate.

@jkrvivian
Copy link
Contributor Author

Since we modify the framework and don't intend to release a new protocol version for this, we should update the framework-snapshot bytes for protocol version 1. If this is not done then nodes will panic on startup that they have a different framework loaded from the snapshot than what is present in the node software.
We don't have to do this once we start bumping protocol versions, then we only create a new snapshot cut when the new protocol version is introduced.

Yes! Thanks for the reminder, we will do this update in a sole commit in the feature branch so it will not be squashed.

IMHO it doesn't matter right now if the commit is squashed or not, I would keep it together with the framework changes (this PR), unless you plan to modfiy other stuff in the iota-framework crate.

I talked to @miker83z , and he mentioned there will be additional changes for deny list type. So, it'd be good to do it later. 💪

@miker83z
Copy link
Contributor

miker83z commented Oct 9, 2024

@lzpap

Since we modify the framework and don't intend to release a new protocol version for this, we should update the framework-snapshot bytes for protocol version 1. If this is not done then nodes will panic on startup that they have a different framework loaded from the snapshot than what is present in the node software.
We don't have to do this once we start bumping protocol versions, then we only create a new snapshot cut when the new protocol version is introduced.

Yes! Thanks for the reminder, we will do this update in a sole commit in the feature branch so it will not be squashed.

IMHO it doesn't matter right now if the commit is squashed or not, I would keep it together with the framework changes (this PR), unless you plan to modfiy other stuff in the iota-framework crate.

I suggested to create the snapshot after the changes to the framework are finalized in the feature branch core-node/feat/remove-multiple-types given that there can be maybe other changes -> #3095

id: UID,
/// A self-custodial object holding the staked IOTA tokens.
staked_iota: StakedIota,
staked_iota: StakedIotaV1,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TimelockedStakedIota and StakedIota are part of the public API, so it will have ramifications in the TS SDK, wallet and explorer too. FYI @begonaalvarezd

@jkrvivian jkrvivian merged commit 3b0f4fd into core-node/feat/remove-multiple-types Oct 9, 2024
17 of 21 checks passed
@jkrvivian jkrvivian deleted the core-node/remove-move-old-types branch October 9, 2024 09:30
jkrvivian added a commit that referenced this pull request Oct 14, 2024
…he iota-system types (#3048)

* refactor(iota-framework/iota-system): Remove deprecated structures

* refactor(iota-framework/iota-system): Rename ValidatorSet to ValidatorSetV1

* refactor(iota-framework/iota-system): Rename Validator to ValidatorV1

* refactor(iota-framework/iota-system): Rename ValidatorMetadata to ValidatorMetadataV1

* refactor(iota-framework/iota-system): Rename IotaSystemStateInnerV1 to IotaSystemStateV1

* refactor(iota-framework/iota-system): Rename SystemEpochInfoEvent to SystemEpochInfoEventV1

* refactor(iota-framework/iota-system): Rename StakingPool to StakingPoolV1

* refactor(iota-framework/iota-system): Rename PoolTokenExchangeRate to PoolTokenExchangeRateV1

* refactor(iota-framework/iota-system): Rename StakedIota to StakedIotaV1

* refactor(iota-framework/iota-system): Rename StorageFund to StorageFundV1

* refactor(iota-framework/iota-system): Rename TimelockedStakedIota to TimelockedStakedIotaV1

* refactor(iota-framework/iota-system): Rename IotaSystemStateInner to IotaSystemState in comments
muXxer pushed a commit that referenced this pull request Oct 14, 2024
…he iota-system types (#3048)

* refactor(iota-framework/iota-system): Remove deprecated structures

* refactor(iota-framework/iota-system): Rename ValidatorSet to ValidatorSetV1

* refactor(iota-framework/iota-system): Rename Validator to ValidatorV1

* refactor(iota-framework/iota-system): Rename ValidatorMetadata to ValidatorMetadataV1

* refactor(iota-framework/iota-system): Rename IotaSystemStateInnerV1 to IotaSystemStateV1

* refactor(iota-framework/iota-system): Rename SystemEpochInfoEvent to SystemEpochInfoEventV1

* refactor(iota-framework/iota-system): Rename StakingPool to StakingPoolV1

* refactor(iota-framework/iota-system): Rename PoolTokenExchangeRate to PoolTokenExchangeRateV1

* refactor(iota-framework/iota-system): Rename StakedIota to StakedIotaV1

* refactor(iota-framework/iota-system): Rename StorageFund to StorageFundV1

* refactor(iota-framework/iota-system): Rename TimelockedStakedIota to TimelockedStakedIotaV1

* refactor(iota-framework/iota-system): Rename IotaSystemStateInner to IotaSystemState in comments
muXxer pushed a commit that referenced this pull request Oct 14, 2024
…he iota-system types (#3048)

* refactor(iota-framework/iota-system): Remove deprecated structures

* refactor(iota-framework/iota-system): Rename ValidatorSet to ValidatorSetV1

* refactor(iota-framework/iota-system): Rename Validator to ValidatorV1

* refactor(iota-framework/iota-system): Rename ValidatorMetadata to ValidatorMetadataV1

* refactor(iota-framework/iota-system): Rename IotaSystemStateInnerV1 to IotaSystemStateV1

* refactor(iota-framework/iota-system): Rename SystemEpochInfoEvent to SystemEpochInfoEventV1

* refactor(iota-framework/iota-system): Rename StakingPool to StakingPoolV1

* refactor(iota-framework/iota-system): Rename PoolTokenExchangeRate to PoolTokenExchangeRateV1

* refactor(iota-framework/iota-system): Rename StakedIota to StakedIotaV1

* refactor(iota-framework/iota-system): Rename StorageFund to StorageFundV1

* refactor(iota-framework/iota-system): Rename TimelockedStakedIota to TimelockedStakedIotaV1

* refactor(iota-framework/iota-system): Rename IotaSystemStateInner to IotaSystemState in comments
muXxer pushed a commit that referenced this pull request Oct 21, 2024
…he iota-system types (#3048)

* refactor(iota-framework/iota-system): Remove deprecated structures

* refactor(iota-framework/iota-system): Rename ValidatorSet to ValidatorSetV1

* refactor(iota-framework/iota-system): Rename Validator to ValidatorV1

* refactor(iota-framework/iota-system): Rename ValidatorMetadata to ValidatorMetadataV1

* refactor(iota-framework/iota-system): Rename IotaSystemStateInnerV1 to IotaSystemStateV1

* refactor(iota-framework/iota-system): Rename SystemEpochInfoEvent to SystemEpochInfoEventV1

* refactor(iota-framework/iota-system): Rename StakingPool to StakingPoolV1

* refactor(iota-framework/iota-system): Rename PoolTokenExchangeRate to PoolTokenExchangeRateV1

* refactor(iota-framework/iota-system): Rename StakedIota to StakedIotaV1

* refactor(iota-framework/iota-system): Rename StorageFund to StorageFundV1

* refactor(iota-framework/iota-system): Rename TimelockedStakedIota to TimelockedStakedIotaV1

* refactor(iota-framework/iota-system): Rename IotaSystemStateInner to IotaSystemState in comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

node Issues related to the Core Node team vm-language Issues related to the VM & Language Team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Task (VM-Language)]: Remove multiple versions from the iota-system types

7 participants