Skip to content

Conversation

@mbjorkqvist
Copy link
Contributor

Set the index canister principal ID in the ledgers of the SNS and chain fusion ledgers.

…ledger

# Conflicts:
#	rs/ethereum/ledger-suite-orchestrator/src/scheduler/mod.rs
#	rs/ledger_suite/icrc1/ledger/src/lib.rs
#	rs/ledger_suite/icrc1/ledger/src/tests.rs
#	rs/ledger_suite/icrc1/ledger/tests/tests.rs
#	rs/ledger_suite/tests/sm-tests/src/lib.rs
…ledger

# Conflicts:
#	rs/ledger_suite/icrc1/ledger/tests/tests.rs
…ledger

# Conflicts:
#	rs/ledger_suite/icrc1/ledger/tests/tests.rs
@aterga
Copy link
Contributor

aterga commented May 27, 2025

Approved from the perspective of the SNS Ledger migration. Thanks!

Just one more ask, which could be done in a follow-up PR if easier: Could you please add comments, e.g.,

// TODO[Jira-ABCD]: Remove migration after this Ledger is released / published to mainnet.

So that we keep track of the temporary migrations code, simplifying a future cleanup.

Copy link
Contributor

@gregorydemay gregorydemay left a comment

Choose a reason for hiding this comment

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

Thanks a lot @mbjorkqvist for this PR and sorry for the delayed review. A minor comment from my side.

Base automatically changed from mathias-FI-1592-implement-icrc106-in-icrc-ledger to master June 6, 2025 09:59
@github-actions github-actions bot requested review from a team as code owners June 6, 2025 09:59
…ledgers

# Conflicts:
#	rs/ledger_suite/icrc1/ledger/tests/tests.rs
#	rs/ledger_suite/tests/sm-tests/src/icrc_106.rs
@mbjorkqvist
Copy link
Contributor Author

mbjorkqvist commented Jun 6, 2025

Approved from the perspective of the SNS Ledger migration. Thanks!

Just one more ask, which could be done in a follow-up PR if easier: Could you please add comments, e.g.,

// TODO[Jira-ABCD]: Remove migration after this Ledger is released / published to mainnet.

So that we keep track of the temporary migrations code, simplifying a future cleanup.

Good idea! I added a TODO pointing to FI-1747 for cleaning up this migration code.

@mbjorkqvist mbjorkqvist added this pull request to the merge queue Jun 19, 2025
@mbjorkqvist mbjorkqvist removed this pull request from the merge queue due to a manual request Jun 19, 2025
@mbjorkqvist mbjorkqvist added this pull request to the merge queue Jun 19, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 19, 2025
@mbjorkqvist mbjorkqvist added this pull request to the merge queue Jun 19, 2025
Merged via the queue into master with commit 00713b9 Jun 19, 2025
25 checks passed
@mbjorkqvist mbjorkqvist deleted the mathias-FI-1604-set-index-in-existing-sns-ledgers branch June 19, 2025 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants