Skip to content

Conversation

@jkrvivian
Copy link
Contributor

@jkrvivian jkrvivian commented Oct 9, 2024

Description of change

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

  • Remove IotaSystemStateInnerV1, rename IotaSystemStateInnverV2 to IotaSystemStateV1
  • Align type names to Move codes
    Note: IotaSystemStateWrapper still exists and corresponds to IotaSystemState in Move. It's a thin wrapper for accessing the inner object and is rarely used in Rust. It's used only in genesis snapshots and testing, and in most cases, we use the enum IotaSystemState instead.
  • Add an enum PoolTokenExchangeRate to wraps different versions of PoolTokenExchangeRate

Changes to move codes:
This is supposed to be done in #3048 , but we forgot it.

  • Rename ValidatorWrapper in Move to Validator

Links to any relevant issues

Close #3154

Type of change

Choose a type of change, and delete any options that are not relevant.

  • Enhancement (a non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

How the change has been tested

cargo test:

  • /crates/iota-types
  • /crates/iota-indexer
  • /crates/iota-json-rpc
  • /crates/iota-graphql-rpc
  • /crates/iota-genesis-bulder
  • /crates/iota-execution

Change checklist

Tick the boxes that are relevant to your changes, and delete any items that are not.

  • I have followed the contribution guidelines for this project
  • I have performed a self-review of my own code
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that new and existing unit tests pass locally with my changes

@jkrvivian jkrvivian added the node Issues related to the Core Node team label Oct 9, 2024
@jkrvivian jkrvivian self-assigned this Oct 9, 2024
@jkrvivian jkrvivian requested review from a team, kodemartin and miker83z as code owners October 9, 2024 14:30
@muXxer
Copy link
Contributor

muXxer commented Oct 10, 2024

Please don't forget run cargo +nightly fmt and cargo clippy --fix.

@jkrvivian jkrvivian force-pushed the core-node/remove-rust-old-types branch 2 times, most recently from 5edc59b to aeb0d0e Compare October 14, 2024 07:01
@jkrvivian jkrvivian force-pushed the core-node/feat/remove-multiple-types branch from 3b0f4fd to b98d4cd Compare October 14, 2024 07:10
@jkrvivian jkrvivian requested a review from a team as a code owner October 14, 2024 07:10
@jkrvivian jkrvivian force-pushed the core-node/remove-rust-old-types branch from aeb0d0e to c376496 Compare October 14, 2024 07:11
@muXxer muXxer force-pushed the core-node/feat/remove-multiple-types branch 2 times, most recently from 6fb25d5 to 2a63305 Compare October 14, 2024 10:29
@jkrvivian jkrvivian requested a review from Thoralf-M October 14, 2024 12:47
@jkrvivian jkrvivian force-pushed the core-node/remove-rust-old-types branch 3 times, most recently from 2babbd5 to 6ccee71 Compare October 15, 2024 07:11
@miker83z miker83z requested a review from a team October 15, 2024 08:20
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.

I think this PR could cover renaming StakedIota and TimelockedStakedIota as well! I think these are the only one remaining from the list in #3048 description

@jkrvivian jkrvivian force-pushed the core-node/remove-rust-old-types branch from 10cebb6 to d660f40 Compare October 23, 2024 12:32
@jkrvivian jkrvivian force-pushed the core-node/remove-rust-old-types branch from 275ffcd to 0ed0c63 Compare October 23, 2024 14:16
@miker83z
Copy link
Contributor

miker83z commented Oct 23, 2024

I suggest these changes because I found it difficult to find a nice way to treat traits for StakedIota, TimelockedStakedIota and PoolTokenExchangeRate -> #3603

Originally I was going with the approach of having is_staked_iota in the Trait and implementing this function for each version. However, given the amount of places that re-use that StakedIota type, I thought this use of the traits could have been problematic when it comes to fixing the remaining parts of the monorepo such as APIs and typescript side.
This approach is not something necessary, but my general idea is to make this process a bit smoother and faster to finalize.

Plus, the PR fixes the BCS conversions and makes the node run.

@miker83z miker83z requested review from a team as code owners October 24, 2024 08:36
@github-actions
Copy link
Contributor

⚠️ 🦋 Changesets Warning: This PR has changes to public npm packages, but does not contain a changeset. You can create a changeset easily by running pnpm changeset in the root of the IOTA repo, and following the prompts. If your change does not need a changeset (e.g. a documentation-only change), you can ignore this message. This warning will be removed when a changeset is added to this pull request.

Learn more about Changesets.

@miker83z miker83z merged commit c29dd1f into core-node/feat/remove-multiple-types Oct 24, 2024
23 of 27 checks passed
@miker83z miker83z deleted the core-node/remove-rust-old-types branch October 24, 2024 08:48
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove multiple versions from iota-types/iota-system types and align naming to Move

7 participants