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

feat: support account name management from snap #508

Open
wants to merge 48 commits into
base: main
Choose a base branch
from
Open

Conversation

khanti42
Copy link
Collaborator

@khanti42 khanti42 commented Feb 12, 2025

PR Summary: Account Naming in Snap

New Features

1. Account Naming

  • Introduces an accountName field for accounts.
  • Default names follow the pattern: "Account {index}" (e.g., Account 1, Account 2).
  • Names are assigned during:
    • Account creation
    • Account recovery
  • Users can rename accounts via the new SetAccountName RPC method.

2. New RPC Methods

  • SetAccountName: Allows renaming an account.

Code Changes

Account Management Enhancements

  • Modified AccountStateManager to store and update accountName.
  • Introduced getNextIndex() in AccountService to fetch the next account index, used in the frontend to display the default Name.
  • Updated createAccount, recoverAccounts, and addAccount to handle account names.

RPC Handling

  • Added new RPC handlers:
    • SetAccountNameRpc (for renaming accounts)
  • Updated onRpcRequest to support these methods.

Testing

  • Added test cases for:

    • setAccountName
    • getNextAccountIndex
  • Updated existing tests to validate account name assignments.

  • Removed unnecessary whitespace in config.ts.

  • Updated superstruct.ts to validate accountName.

BEGIN_COMMIT_OVERRIDE

feat: Support account name management in Snap

END_COMMIT_OVERRIDE

stanleyyconsensys and others added 30 commits January 9, 2025 10:16
* feat: add account service
* feat: add account service factory

* fix: rename deployPayload
* feat: add account serviceΩ
---------

Co-authored-by: khanti42 <[email protected]>
* feat: add `AddAccount` RPC

* feat: add max account create limit

---------

Co-authored-by: khanti42 <[email protected]>
* feat: add account ui

---------

Co-authored-by: khanti42 <[email protected]>
* feat: add get current account RPC

* fix: add some detail comment on contract discovery

* fix: comments on test title

---------

Co-authored-by: khanti42 <[email protected]>
* feat: add list accounts rpc

* fix: lint

* fix: add some detail comment on contract discovery

* fix: comments on test title

---------

Co-authored-by: khanti42 <[email protected]>
* feat: add swtich account rpc
---------

Co-authored-by: khanti42 <[email protected]>
#479)

* chore: set new account to current in snap

* fix: lint

* chore: update logger mocking
* feat: add manage multi account hooks

* fix: remove non exist component

* fix: var naming in UI

* chore: update logger mocking

* chore: update setAccounts logic in UI

* chore: remove duplicate when set account from UI

* chore: remove non necessary account array in UI
* feat: add account service

* chore: fix lint

* chore: update account contract discovery logic

* fix: code comment

* chore: add discovery logic description

* fix: lint

* feat: add account service factory

* fix: rename deployPayload

* chore: adopt account discovery in RPCs

* chore: update execute txn test

* fix: execute test

* fix: account discovery bug

* fix: discovery logic

* feat: add `AddAccount` RPC

* feat: add max account create limit

* fix: add `isMaxAccountLimitExceeded` unit  test

* feat: add account ui

* fix: account deploy require result

* fix: lint

* fix: lint

* feat: add get current account RPC

* feat: add list accounts rpc

* fix: lint

* feat: add swtich account rpc

* fix: comments on add account icon

* chore: set new account to current in snap

* fix: lint

* feat: add manage multi account hooks

* fix: remove non exist component

* fix: var naming in UI

* chore: update logger mocking

* chore: update setAccounts logic in UI

* chore: remove duplicate when set account from UI

* chore: remove non necessary account array in UI

* feat: account selection dropdown

* chore: connect ui to snap for switch account

* chore: fix comments

* chore: lint + prettier

* chore: fix bugs qa

* chore: fix comments

* chore: revert snap changes

* chore: revert snap changes

---------

Co-authored-by: stanleyyuen <[email protected]>
…ap home page UI (#487)

* fix: use correct account on snap homepage

* fix: eof
* feat: get-starknet support for account change event

* chore: lint + prettier

* chore: fix comments

* chore: lint + prettier

* fix: use fromState=true

* chore: fix comments

* chore: fix tests

* chore: fix comments

* chore: fix comments

* chore: fix comments

* fix: rollback change

---------

Co-authored-by: stanleyyuen <[email protected]>
* chore: add unit test

* chore: update wallet test in get-starknet

* chore: block init event not send out

---------

Co-authored-by: stanleyyuen <[email protected]>
…t visibility (#494)

* chore: unblock account create limit

* feat: add toggle account visibility RPC

* fix: typo

* fix: add permission
* feat: wallet-ui multiple accounts scrollbar
* feat: wallet-ui multiple accounts scrollbar

* feat: hide account feature

* feat: show account feature

* chore: fix sonar qube

* chore: fix comments

* chore: lint

* chore: fix comments

* chore: lint

* chore: fix comments

* chore: fix comments

* chore: fix comments

* chore: fix comments

* chore: fix comments

* chore: fix comments

* chore: fix comments

* chore: fix comments

* chore: fix comments

* chore: currentAccount is always defined in walletSlice redux state (#502)

* chore: fix comments

* chore: fix comments
@khanti42 khanti42 requested a review from a team as a code owner February 12, 2025 21:32
@khanti42 khanti42 requested review from Julink-eth and jonesho and removed request for a team February 12, 2025 21:32
@khanti42 khanti42 changed the title feat: account Naming in Starknet Snap feat: account Naming in Snap Feb 12, 2025
@stanleyyconsensys stanleyyconsensys changed the title feat: account Naming in Snap feat: support account name management from snap Feb 13, 2025
Copy link
Collaborator

@stanleyyconsensys stanleyyconsensys left a comment

Choose a reason for hiding this comment

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

Thanks for the work
some comments and some of them are coming from copy and paste
Lets be more careful next time

packages/starknet-snap/src/rpcs/add-account.ts Outdated Show resolved Hide resolved
packages/starknet-snap/src/config.ts Outdated Show resolved Hide resolved
packages/starknet-snap/src/types/snapState.ts Outdated Show resolved Hide resolved
packages/starknet-snap/src/state/account-state-manager.ts Outdated Show resolved Hide resolved
packages/starknet-snap/src/wallet/account/account.ts Outdated Show resolved Hide resolved
packages/starknet-snap/src/wallet/account/account.ts Outdated Show resolved Hide resolved
packages/starknet-snap/src/wallet/account/service.ts Outdated Show resolved Hide resolved
packages/starknet-snap/test/constants.test.ts Outdated Show resolved Hide resolved
@khanti42 khanti42 changed the base branch from main to chore/refine-account-service February 13, 2025 11:37
Copy link
Collaborator

@stanleyyconsensys stanleyyconsensys left a comment

Choose a reason for hiding this comment

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

Almost prefect, just 3 comments for enhancement

packages/starknet-snap/src/utils/account.test.ts Outdated Show resolved Hide resolved
packages/starknet-snap/src/utils/superstruct.ts Outdated Show resolved Hide resolved
packages/starknet-snap/src/rpcs/set-account-name.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

@stanleyyconsensys stanleyyconsensys left a comment

Choose a reason for hiding this comment

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

few more comments

packages/starknet-snap/src/rpcs/set-account-name.ts Outdated Show resolved Hide resolved
packages/starknet-snap/src/utils/superstruct.ts Outdated Show resolved Hide resolved
packages/starknet-snap/src/rpcs/set-account-name.test.ts Outdated Show resolved Hide resolved
Base automatically changed from chore/refine-account-service to main February 13, 2025 12:35
Copy link

Quality Gate Passed Quality Gate passed for 'consensys_starknet-snap-wallet-ui'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Copy link

Quality Gate Passed Quality Gate passed for 'consensys_starknet-snap-starknet-snap'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
1.6% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

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.

2 participants