Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make some improvements to the address swap map usage #4376

Conversation

DaughterOfMars
Copy link
Contributor

Description of change

  1. Moved the address swap map definition site to the genesis migration crate, as that is the only place it is used.
  2. Reconfigured the types and methods of the map for better usability and removed RefCell usage.

Copy link
Contributor

github-actions bot commented Dec 5, 2024

This pull request has been deployed to Vercel.

Latest commit: 77b372d

✅ Preview: https://iota-rebased-explorer-092603f1de8088243ccb8b861b23e-cbhlbabfc.vercel.app

Copy link
Contributor

github-actions bot commented Dec 5, 2024

This pull request has been deployed to Vercel.

Latest commit: 77b372d

✅ Preview: https://apps-ui-jgwuibn65-iota1.vercel.app

Copy link
Contributor

github-actions bot commented Dec 5, 2024

This pull request has been deployed to Vercel.

Latest commit: 77b372d

✅ Preview: https://apps-backend-29a719c9ccbdb9b80c97b599a599d8953354da-1xmvse1tr.vercel.app

Copy link
Contributor

github-actions bot commented Dec 5, 2024

This pull request has been deployed to Vercel.

Latest commit: 77b372d

✅ Preview: https://wallet-dashboard-a225fcfa1e2f852205a61a7cd1d5d93e92-hstb04lbo.vercel.app

Copy link
Contributor

@Dkwcs Dkwcs left a comment

Choose a reason for hiding this comment

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

The solution looks more elegant.
However, haven’t we gone back to using the ref mut construction, which we were trying to avoid?
I also agree with the previous point that it’s unnecessary, and if we can avoid it, we should. Using RefCell doesn’t seem like an overkill solution here, does it?

@DaughterOfMars
Copy link
Contributor Author

The solution looks more elegant. However, haven’t we gone back to using the ref mut construction, which we were trying to avoid? I also agree with the previous point that it’s unnecessary, and if we can avoid it, we should. Using RefCell doesn’t seem like an overkill solution here, does it?

The problem wasn't the mutable refs, it was that validation specifically did not need mutable refs.

…-Map-from-a-CSV-file' into dev-tools/improve-swap-map
Copy link
Contributor

github-actions bot commented Dec 5, 2024

This pull request has been deployed to Vercel.

Latest commit: c3865de

✅ Preview: https://apps-ui-5aho8mx0e-iota1.vercel.app

Copy link
Contributor

github-actions bot commented Dec 5, 2024

This pull request has been deployed to Vercel.

Latest commit: c3865de

✅ Preview: https://iota-rebased-explorer-092603f1de8088243ccb8b861b23e-2jz39u0xq.vercel.app

Copy link
Contributor

github-actions bot commented Dec 5, 2024

This pull request has been deployed to Vercel.

Latest commit: c3865de

✅ Preview: https://wallet-dashboard-a225fcfa1e2f852205a61a7cd1d5d93e92-pvfs9llp3.vercel.app

Copy link
Contributor

github-actions bot commented Dec 5, 2024

This pull request has been deployed to Vercel.

Latest commit: c3865de

✅ Preview: https://apps-backend-29a719c9ccbdb9b80c97b599a599d8953354da-oy0a8fxmx.vercel.app

Copy link
Contributor

github-actions bot commented Dec 5, 2024

This pull request has been deployed to Vercel.

Latest commit: ac2bf6e

✅ Preview: https://apps-ui-cnfvufq1e-iota1.vercel.app

Copy link
Contributor

github-actions bot commented Dec 5, 2024

This pull request has been deployed to Vercel.

Latest commit: ac2bf6e

✅ Preview: https://apps-backend-29a719c9ccbdb9b80c97b599a599d8953354da-qtkscjjaq.vercel.app

Copy link
Contributor

github-actions bot commented Dec 5, 2024

This pull request has been deployed to Vercel.

Latest commit: ac2bf6e

✅ Preview: https://iota-rebased-explorer-092603f1de8088243ccb8b861b23e-kja7b1wqf.vercel.app

Copy link
Contributor

github-actions bot commented Dec 5, 2024

This pull request has been deployed to Vercel.

Latest commit: ac2bf6e

✅ Preview: https://wallet-dashboard-a225fcfa1e2f852205a61a7cd1d5d93e92-43mx49a23.vercel.app

@DaughterOfMars DaughterOfMars requested a review from Dkwcs December 6, 2024 13:49
@Dkwcs Dkwcs merged commit d21f2b5 into sc-platform/Read-an-Address-Swap-Map-from-a-CSV-file Dec 9, 2024
40 checks passed
@Dkwcs Dkwcs deleted the dev-tools/improve-swap-map branch December 9, 2024 09:58
miker83z added a commit that referenced this pull request Dec 20, 2024
…swap in migration test (#4389)

* feat(iota-genesis-builder): add address swap map for swaping origin addresses to destination during the migration process

* fix(iota-genesis-builder): relative path supporting, remove mut modifier

* fix(iota-genesis-builder): address_swap_map become field of Migration struct, minor renaming refactoring

* fix(iota-genesis-builder): add additional hashmap that track if address has been swapped at least once
Create dedicated function to convert and swap addresses

* fix(iota-genesis-builder): review's cmments fix

* fix(iota-genesis-builder): anyhow error has been added instead of panic

* cargo clippy

* typo spell fix

* fix(iota-genesis-builder): add comments to the public functions

* fix(iota-genesis-builder): remove mut modificator, remove extra hashmap

* fix(iota-types): parse first address from bech32 format, csv headers support

* fix(iota-types): example of CSV file in comments above initialization function

* Minor fix

* refactor(iota-genesis-builder): move output processing out of main

* feat(iota-genesis-builder): create a dedicated run migration function for iota snapshot

* feat(iota-e2e-tests): include snapshot generation and address swap in migration test

* Make some improvements to the address swap map usage (#4376)

* Make some improvements to the address swap map usage

* fix(iota-e2e-tests): update import

* fix typo

---------

Co-authored-by: Dkwcs <pavlo.botnar@gmail.com>
Co-authored-by: DaughterOfMars <chloedaughterofmars@gmail.com>
miker83z added a commit that referenced this pull request Dec 20, 2024
…swap in migration test (#4389)

* feat(iota-genesis-builder): add address swap map for swaping origin addresses to destination during the migration process

* fix(iota-genesis-builder): relative path supporting, remove mut modifier

* fix(iota-genesis-builder): address_swap_map become field of Migration struct, minor renaming refactoring

* fix(iota-genesis-builder): add additional hashmap that track if address has been swapped at least once
Create dedicated function to convert and swap addresses

* fix(iota-genesis-builder): review's cmments fix

* fix(iota-genesis-builder): anyhow error has been added instead of panic

* cargo clippy

* typo spell fix

* fix(iota-genesis-builder): add comments to the public functions

* fix(iota-genesis-builder): remove mut modificator, remove extra hashmap

* fix(iota-types): parse first address from bech32 format, csv headers support

* fix(iota-types): example of CSV file in comments above initialization function

* Minor fix

* refactor(iota-genesis-builder): move output processing out of main

* feat(iota-genesis-builder): create a dedicated run migration function for iota snapshot

* feat(iota-e2e-tests): include snapshot generation and address swap in migration test

* Make some improvements to the address swap map usage (#4376)

* Make some improvements to the address swap map usage

* fix(iota-e2e-tests): update import

* fix typo

---------

Co-authored-by: Dkwcs <pavlo.botnar@gmail.com>
Co-authored-by: DaughterOfMars <chloedaughterofmars@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants