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

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

Merged
merged 2 commits into from
Feb 5, 2025

Conversation

khanti42
Copy link
Collaborator

@khanti42 khanti42 commented Feb 4, 2025

This PR is to remove the many use of DUMMY_ADDRESS throughout the code base by using a dummy account at the redux state level.

@khanti42 khanti42 requested a review from a team as a code owner February 4, 2025 15:41
@khanti42 khanti42 requested review from Julink-eth and wantedsystem and removed request for a team February 4, 2025 15:41
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.

LGTM, just 1 nit comment, but i give a pass first

packages/wallet-ui/src/slices/walletSlice.ts Show resolved Hide resolved
@khanti42 khanti42 merged commit 42a7144 into feat/sf-794 Feb 5, 2025
3 checks passed
@khanti42 khanti42 deleted the fix/dummy-current-account branch February 5, 2025 06:37
Copy link

sonarqubecloud bot commented Feb 5, 2025

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

Issues
17 New issues
0 Accepted issues

Measures
0 Security Hotspots
1.8% Coverage on New Code
0.6% Duplication on New Code

See analysis details on SonarQube Cloud

Copy link

sonarqubecloud bot commented Feb 5, 2025

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

Issues
2 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud

khanti42 added a commit that referenced this pull request Feb 5, 2025
* 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
github-merge-queue bot pushed a commit that referenced this pull request Feb 6, 2025
* feat: add account service (#470)

* feat: add account service

* feat: add account service factory (#472)

* feat: add account service factory

* fix: rename deployPayload

* refactor: adopt account discovery (#473)

* feat: add account serviceΩ
---------

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

* feat: add new RPC `starkNet_addAccount ` (#474)

* feat: add `AddAccount` RPC

* feat: add max account create limit

---------

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

* feat: add account UI (#475)

* feat: add account ui

---------

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

* feat: add new RPC `starkNet_getCurrentAccount` (#476)

* 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 new RPC `starkNet_listAccounts` (#477)

* 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 new RPC `starkNet_switchAccount` (#478)

* feat: add swtich account rpc
---------

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

* chore: set new account to current current in RPC `starkNet_addAccount` (#479)

* chore: set new account to current in snap

* fix: lint

* chore: update logger mocking

* feat: add UI hooks for manage multi accounts (#480)

* 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

* chore: update get current account rpc to return data from state only (#483)

* feat: [Wallet-UI] account selection dropdown (#482)

* 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]>

* fix: after a account switched, the address is not reflected on the Snap home page UI (#487)

* fix: use correct account on snap homepage

* fix: eof

* fix: account contract isRequiredDeploy Upgrade order mismatch (#491)

* fix: typo of SwitchAccount RPC api name

* feat: account event change get starknet (#484)

* 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 for get-starknet event (#490)

* chore: add unit test

* chore: update wallet test in get-starknet

* chore: block init event not send out

---------

Co-authored-by: stanleyyuen <[email protected]>

* fix: incorrect switch account rpc endpoint (#496)

* chore: unblock account create limit (#493)

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

* feat: Add RPC `starkNet_toggleAccountVisibility` to toggle the account visibility (#494)

* chore: unblock account create limit

* feat: add toggle account visibility RPC

* fix: typo

* fix: add permission

* feat: wallet-ui multiple accounts scrollbar (#499)

* feat: wallet-ui multiple accounts scrollbar

* feat: wallet-ui multiple accounts hide/unhide feature (#501)

* 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

* chore: add missing translation

* chore: lint + test

* refactor: add account discover service

* feat: support multiple account in SNAP

* feat: watch the SNAP current account

* fix: lint

* feat: support multiple account

* chore: qa fix

* chore: lint

* chore: lint

---------

Co-authored-by: Stanley Yuen <[email protected]>
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