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

Tests of ArgentX & Braavos experimental wallets #212

Closed
PhilippeR26 opened this issue Feb 6, 2024 · 2 comments
Closed

Tests of ArgentX & Braavos experimental wallets #212

PhilippeR26 opened this issue Feb 6, 2024 · 2 comments

Comments

@PhilippeR26
Copy link

PhilippeR26 commented Feb 6, 2024

Experimental Braavos wallet v0.0.2.
Experimental ArgentX wallet v5.13.0 (06/feb/2024).

@dhruvkelawala , @avimak , @ivpavici , @amanusk

Test DAPP : https://cairo1-js-git-testbraavos002-philipper26.vercel.app/
My documentation for this new APi : https://github.com/PhilippeR26/Cairo1JS/blob/testBraavos002/doc/walletAPIspec.md

Result of the tests of the experimental wallets with PR #194 :

Id Subject ArgentX Braavos Comment
1 event accountsChanged never released. Unexpected release at each new block.
2 event networkChanged neither accountChanged nor networkChanged released OK
3 wallet_getPermissions OK OK
4 wallet_requestAccounts OK OK
5 wallet_watchAsset return false if asset already visible return true if asset already visible Braavos behavior preferred
6 wallet_addStarknetChain Works, but fail if already added. Not implemented preferable : returns true if already added
7 wallet_switchStarknetChain Works, but
if not existing : fails.
If already active : fails
Not implemented Preferable: if not existing : returns false. If active : returns true
8 wallet_requestChainId OK OK
9 starknet_addInvokeTransaction OK OK
10 starknet_addDeclareTransaction Impossible to proceed. In AddDeclareTransactionParameters type, abi key is optional, but call returns Error: Missing ABI. Abi type expected is string, but in reality is an array of object. How to process? What's the format and values of contract_class_version parameter? Decline button is not generating any response. Wallet window opened, but do not proceed. One example of a valid request seems necessary.
11 starknet_addDeployAccountTransaction FAIL, with message Error: Not implemented. OK.
But do not deploy at the pre-calculated address
Braavos uses the current account to fund automatically the account deployment.
12 starknet_signTypedData Improved display (nice!), but not processing after click on sign OK.
Raw format display
tested with legacy V5 format
13 starknet_supportedSpecs OK OK
14 GetDeploymentDataResult - - What is it? How does it work?
@dhruvkelawala
Copy link
Collaborator

  • On ArgentX, the accountsChanged event is only triggered if the account is preAuthorized. So you can test this by connecting Account 1 and Account 2, then when you switch between those two accounts, you will see accountsChanged. Same for networkChange.
  • We can fix watchAsset, switchChainId and addChainId as per expected behaviour but the expected behaviour is TBD by all wallets I think
  • For declareTransaction we will wait for WalletAccount API on starknet.js as I don't see any better option.
  • ArgentX might never support addDeployAccountTransaction. We will decide this internally.
  • We will try to fix signTypedData.

@dhruvkelawala
Copy link
Collaborator

Also we should consider error cases for watchAsset, switchChainId and addChainId. For example, if the user rejects the requests. Returning false might not exactly give the right information to the dapp

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

No branches or pull requests

2 participants