8000 feat(wallet): add experimental ICRC subaccount recovery tools by yhabib · Pull Request #6827 · dfinity/nns-dapp · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

feat(wallet): add experimental ICRC subaccount recovery tools #6827

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 8 commits into from
May 15, 2025
Merged

feat(wallet): add experimental ICRC subaccount recovery tools #6827

merged 8 commits into from
May 15, 2025
+511 −1

Conversation

yhabib
Copy link
Contributor
@yhabib yhabib commented May 12, 2025 •
edited
Loading

Motivation

The nns-dapp currently does not support subaccounts for ICRC canisters. However, a user utilizing the CLI or other tools may accidentally send tokens to these subaccounts and then be unable to interact with them through the nns-dapp. This PR introduces a set of experimental functions to the window object, enabling users to easily recover these tokens by emptying the subaccounts and transferring the tokens to their main account, which they manage through the nns-dapp.

Screen.Recording.2025-05-13.at.18.55.28.mov

Changes

  • The SnsWallet page loads a set of functions to the window object for managing ICRC subaccounts.
    • __experimental_icrc.help(): Displays help information for available commands.
    • __experimental_icrc.listSubaccounts(): Lists subaccounts for the current project.
    • __experimental_icrc.getBalance(): Retrieves the balance for a specified subaccount.
    • __experimental_icrc.recover(): Transfers all tokens from a specified subaccount to the main account.

Tests

  • Manually tested by sending tokens from the main account to a subaccount using the commented method __testSendTokensToSubaccount, and then recovering them back to my main account.

Todos

  • Add entry to changelog (if necessary).
    Not necessary.

@yhabib yhabib changed the title DRAFT: recover subaccount tokens feat(debug): add experimental ICRC subaccount recovery tools May 13, 2025
@yhabib yhabib requested a review from Copilot May 13, 2025 21:01
Copilot
Copilot AI reviewed May 13, 2025
View reviewed changes
Copy link
Contributor
@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds experimental ICRC subaccount recovery tools to help users recover tokens sent to unsupported subaccounts through the NNS dapp.

  • Conditionally loads a new experimental component (SnsWalletDevTools) in the SnsWallet page.
  • Implements experimental window functions in SnsWalletDevTools for listing subaccounts, checking balances, and recovering tokens.
  • Includes a noted TODO in the icrc-ledger API regarding an outdated or misleading comment.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
frontend/src/lib/pages/SnsWallet.svelte Imports and conditionally renders the new SnsWalletDevTools component.
frontend/src/lib/components/experimental/SnsWalletDevTools.svelte Implements experimental helper functions on the window object.
frontend/src/lib/api/icrc-ledger.api.ts Contains a TODO comment indicating a potential documentation issue.

@yhabib yhabib requested a review from Copilot May 13, 2025 21:23
@yhabib yhabib marked this pull request as ready for review May 13, 2025 21:23
@yhabib yhabib requested a review from a team as a code owner May 13, 2025 21:23
Copilot
Copilot AI reviewed May 13, 2025
View reviewed changes
Copy link
Contributor
@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds an experimental toolkit for recovering ICRC token subaccounts by exposing helper functions on the window object.

  • Loads a new SnsWalletDevTools component when project context is available
  • Injects __experimental_icrc namespace with help, listSubaccounts, getBalance, and recover functions
  • Includes a minor comment cleanup in the ICRC ledger API

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
frontend/src/lib/pages/SnsWallet.svelte Import and conditionally render the SnsWalletDevTools panel
frontend/src/lib/components/experimental/SnsWalletDevTools.svelte Add module and runtime scripts for ICRC subaccount recovery tools
frontend/src/lib/api/icrc-ledger.api.ts Remove or clarify a stale TODO comment
Comments suppressed due to low confidence (1)

frontend/src/lib/components/experimental/SnsWalletDevTools.svelte:47

  • [nitpick] Update the placeholder <accountId> to <subaccount> (or <subaccountHex>) for consistency with the function parameter name.
__experimental_icrc.getBalance("<accountId>")

@yhabib yhabib changed the title feat(debug): add experimental ICRC subaccount recovery tools feat(wallet): add experimental ICRC subaccount recovery tools May 14, 2025
mstrasinskis
mstrasinskis reviewed May 14, 2025
View reviewed changes
Copy link
Contributor
@mstrasinskis mstrasinskis 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, just a couple of questions/suggestions.

andrew-lee-work
andrew-lee-work approved these changes May 14, 2025
View reviewed changes
Copy link
@andrew-lee-work andrew-lee-work left a comment

Choose a reason for hiding this comment

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

LGTM.

Perhaps the testing could be improved, but it looks simple enough.

@yhabib yhabib requested a review from mstrasinskis May 14, 2025 18:43
mstrasinskis
mstrasinskis approved these changes May 14, 2025
View reviewed changes
Copy link
Contributor
@mstrasinskis mstrasinskis left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@yhabib yhabib added this pull request to the merge queue May 15, 2025
@yhabib yhabib mentioned this pull request May 15, 2025
Merged
1 task
Merged via the queue into main with commit 7f92e73 May 15, 2025
36 checks passed
@yhabib yhabib deleted the nns1-3793/dev-tools-component branch May 15, 2025 10:35
github-merge-queue bot pushed a commit that referenced this pull request May 15, 2025
@yhabib
feat(analytics): track ICRC subaccount recovery usage (#6835)
8f512da 
# Motivation

#6827 introduces a method for recovering tokens from an ICRC subaccount.
This feature is experimental, as the nns-dapp does not support ICRC
subaccounts. We want to gauge how many people are using this to learn
more about ICRC subaccounts.

# Changes

- Expose a method for tracking events in the analytics service.  
- Emit an event when `recover` is called with the correct setup.

# Tests

- Added unit test to the new events logic.

# Todos

- [ ] Add entry to changelog (if necessary).
Not necessary.
mstrasinskis added a commit that referenced this pull request May 15, 2025
@mstrasinskis
Squashed commit of the following:
2e6083c 
commit 8f512da
Author: Yusef Habib <yusef.fernandez@dfinity.org>
Date:   Thu May 15 13:30:12 2025 +0200

    feat(analytics): track ICRC subaccount recovery usage (#6835)

    # Motivation

    #6827 introduces a method for recovering tokens from an ICRC subaccount.
    This feature is experimental, as the nns-dapp does not support ICRC
    subaccounts. We want to gauge how many people are using this to learn
    more about ICRC subaccounts.

    # Changes

    - Expose a method for tracking events in the analytics service.
    - Emit an event when `recover` is called with the correct setup.

    # Tests

    - Added unit test to the new events logic.

    # Todos

    - [ ] Add entry to changelog (if necessary).
    Not necessary.

commit 7f92e73
Author: Yusef Habib <yusef.fernandez@dfinity.org>
Date:   Thu May 15 12:20:50 2025 +0200

    feat(wallet): add experimental ICRC subaccount recovery tools (#6827)

    # Motivation

    The nns-dapp currently does not support subaccounts for ICRC canisters.
    However, a user utilizing the CLI or other tools may accidentally send
    tokens to these subaccounts and then be unable to interact with them
    through the nns-dapp. This PR introduces a set of `experimental`
    functions to the window object, enabling users to easily recover these
    tokens by emptying the subaccounts and transferring the tokens to their
    main account, which they manage through the nns-dapp.

    https://github.com/user-attachments/assets/6b2d5838-8a76-4f19-9969-a88d66b45f4e

    # Changes

    - The `SnsWallet` page loads a set of functions to the window object for
    managing ICRC subaccounts.
    - `__experimental_icrc.help()`: Displays help information for available
    commands.
    - `__experimental_icrc.listSubaccounts()`: Lists subaccounts for the
    current project.
    - `__experimental_icrc.getBalance()`: Retrieves the balance for a
    specified subaccount.
    - `__experimental_icrc.recover()`: Transfers all tokens from a specified
    subaccount to the main account.

    # Tests

    - Manually tested by sending tokens from the main account to a
    subaccount using the commented method `__testSendTokensToSubaccount`,
    and then recovering them back to my main account.

    # Todos

    - [ ] Add entry to changelog (if necessary).
    Not necessary.

commit 2582362
Author: Max Strasinsky <98811342+mstrasinskis@users.noreply.github.com>
Date:   Thu May 15 11:27:12 2025 +0200

    Display no delegation state (#6833)

    # Motivation

    When no voting delegation is set for a neuron, the corresponding cell in
    the table appears empty — which can look like a broken or missing UI
    element. This is actually the default state for users who haven’t
    followed any topics.

    To improve clarity, this PR displays a "-" symbol in such cases, along
    with a descriptive tooltip.

    Additionally, to ensure consistency across all states, the tooltip
    labels for the other delegation states have been updated as well.

    **Before:**
    <img width="66%" alt="image"
    src="https://github.com/user-attachments/assets/def33a01-2bf7-4e60-a8cf-5686904f0168"
    />

    **After:**
    <img width="66%" alt="image"
    src="https://github.com/user-attachments/assets/e45bedfc-e46c-4767-a298-34254f9fb135"
    />

    # Changes

    - Display a dash (`-`) in the cell when no voting delegation is set.
    - Updated tooltip labels for all delegation states to ensure consistent
    wording.

    # Tests

    - Updated.

    # Todos

    - [ ] Add entry to changelog (if necessary).
    Not necessary, since the new column is not yet published.

commit c502ed8
Author: Max Strasinsky <98811342+mstrasinskis@users.noreply.github.com>
Date:   Thu May 15 09:52:31 2025 +0200

    Display vote delegation state in NNS neurons table (#6826)

    # Motivation

    To improve the visibility of a user’s vote delegation state, it should
    be displayed directly in the neuron table by adding a new column: “Vote
    Delegation”. This PR introduces a new function to calculate NNS neuron
    vote delegation state.

    # Changes

    - New `getNnsNeuronVoteDelegationState` util.
    - Add `voteDelegationState` field for NNS version of TableNeuron which
    automatically makes the column to appear in the NNS neurons table.
    - Make `voteDelegationState` not optional, since both SNS and NNS tables
    now supports it.
    - Update screenshots because of new field.

    # Tests

    - Tested `getNnsNeuronVoteDelegationState` util.
    - Tested that the NNS version of `TableNeuron` contains
    `voteDelegationState`.

    # Todos

    - [x] Add entry to changelog (if necessary).

commit 7ef47be
Author: Max Strasinsky <98811342+mstrasinskis@users.noreply.github.com>
Date:   Tue May 13 17:39:37 2025 +0200

    Display vote delegation state for SNS neurons (#6824)

    # Motivation

    To improve the visibility of a user’s vote delegation state, it should
    be displayed directly in the neuron table by adding a new column:
    **“Vote Delegation”** (see screenshot). This PR introduces that column.

    [Draft MVP PR](https://github.com/dfinity/nns-dapp/pull/6802/files)

    [Demo](https://qsgjb-riaaa-aaaaa-aaaga-cai.mstr-ingress.devenv.dfinity.network/)

    ![Screenshot 2025-05-08 at 13 38
    40](https://github.com/user-attachments/assets/700ce2e2-2581-4d59-90a8-97dcdce72357)

    # Changes

    - Added a calculated value to the optional `voteDelegationState` field
    in `TableNeuron`.
    - Rendered `NeuronVoteDelegationCell` in the neuron table when the data
    is available.

    # Tests

    - Updated unit tests.
    - Manually verified that users can view and sort neurons by vote
    delegation state.

    # Todos

    - [x] Add entry to changelog (if necessary).

commit aa4e6d9
Author: Max Strasinsky <98811342+mstrasinskis@users.noreply.github.com>
Date:   Tue May 13 17:03:03 2025 +0200

    refactor(Sns topics): Extract logic from createSnsTopicsProjectStore (#6829)

    # Motivation

    Sometimes it's redundant to create a new store just to read a project's
    SNS topics. With this PR, we extract the store-reading and sorting logic
    from the derived store into a separate utility function.

    # Changes

    - Introduced new utility function: `getSnsTopicsByProject`.
    - Applied it in `createSnsTopicsProjectStore`.
    - Drive-by: renamed `SnsParametersStore` to `SnsTopicsStore` in the
    topic store.

    # Tests

    - Added.

    # Todos

    - [ ] Add entry to changelog (if necessary).
    Not necessary.

commit 786f586
Author: Yusef Habib <yusef.fernandez@dfinity.org>
Date:   Tue May 13 12:21:01 2025 +0200

    feat(portfolio): load adopted SNS proposals for display (#6825)

    # Motivation

    We want to display proposals for new SNS projects on the Portfolio page
    that were approved before the swap begins. This PR follows #6822 and
    loads adopted proposals, passing them to the Portfolio page for display
    in the stacked card set.

    https://github.com/user-attachments/assets/0969d4ab-764f-401e-8a74-c121fa35b9af

    [NNS1-3639](https://dfinity.atlassian.net/browse/NNS1-3639)

    # Changes

    - Retrieve the list of adopted proposals and provide them to the
    Portfolio page.

    # Tests

    - Add a test to verify that the expected number of cards is displayed
    when passing one ongoing swap, one adopted SNS project proposal, and one
    open SNS project proposal.

    # Todos

    - [x] Add entry to changelog (if necessary).

    [NNS1-3639]:
    https://dfinity.atlassian.net/browse/NNS1-3639?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

commit b4deb7a
Author: pr-automation-bot-public[bot] <189003650+pr-automation-bot-public[bot]@users.noreply.github.com>
Date:   Tue May 13 07:01:42 2025 +0200

    bot: Update sns_aggregator candid bindings (#6828)

    # Motivation
    A newer release of the internet computer is available.
    We would like to parse all the latest SNS data. To do so, we update the
    candid interfaces for the SNS aggregator.
    Even with no changes, just updating the reference is good practice.

    # Changes
    * Update the version of `IC_COMMIT_FOR_SNS_AGGREGATOR` specified in
    `config.json`.
    * Updated the `sns_aggregator` candid files to the versions in that
    commit.
    * Updated the Rust code derived from `.did` files in the aggregator.

    # Tests
    - [ ] Please check the API updates for any breaking changes that affect
    our code.
      Breaking changes are:
        * New mandatory fields
        * Removing mandatory fields
        * Renaming fields
        * Changing the type of a field
        * Adding new variants

    Co-authored-by: gix-bot <gix-bot@users.noreply.github.com>

commit 303c93a
Author: pr-automation-bot-public[bot] <189003650+pr-automation-bot-public[bot]@users.noreply.github.com>
Date:   Mon May 12 16:48:46 2025 +0000

    bot: Update sns_aggregator candid bindings (#6790)

    # Motivation
    A newer release of the internet computer is available.
    We would like to parse all the latest SNS data. To do so, we update the
    candid interfaces for the SNS aggregator.
    Even with no changes, just updating the reference is good practice.

    # Changes
    * Update the version of `IC_COMMIT_FOR_SNS_AGGREGATOR` specified in
    `config.json`.
    * Updated the `sns_aggregator` candid files to the versions in that
    commit.
    * Updated the Rust code derived from `.did` files in the aggregator.

    # Tests
    - [ ] Please check the API updates for any breaking changes that affect
    our code.
      Breaking changes are:
        * New mandatory fields
        * Removing mandatory fields
        * Renaming fields
        * Changing the type of a field
        * Adding new variants

    Co-authored-by: gix-bot <gix-bot@users.noreply.github.com>

commit 8e085b0
Author: Yusef Habib <yusef.fernandez@dfinity.org>
Date:   Mon May 12 16:57:28 2025 +0200

    feat(portfolio): integrate adopted SNS proposal cards (#6822)

    # Motivation

    We want to display proposals for new SNS projects on the Portfolio page
    that were approved before the swap begins. This PR utilizes the new card
    introduced in #6811 to showcase adopted proposals for new projects.

    The order for displaying cards within the stacked card component is as
    follows:
    1. Ongoing swaps
    2. Proposals for new projects
    3. Adopted proposals for new projects.

    [NNS1-3639](https://dfinity.atlassian.net/browse/NNS1-3639)

    # Changes

    - Update Portfolio page to expected a new prop for adopted proposals.
    - Sort adopted proposals based on the swap start.

    # Tests

    - Unit tests for the new prop in the page.
    - Unit tests for checking the sorting is applied to each group of cards.

    # Todos

    - [ ] Add entry to changelog (if necessary).
    Not yet.

    [NNS1-3639]:
    https://dfinity.atlassian.net/browse/NNS1-3639?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

Copilot code review Copilot Copilot left review comments

@andrew-lee-work andrew-lee-work andrew-lee-work approved these changes

@mstrasinskis mstrasinskis mstrasinskis approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@yhabib @andrew-lee-work @mstrasinskis
0