Test of experimental Wallets 2024-05-15

PhilippeR26 commented 6 months ago

Experimental Braavos wallet v0.0.1-3.49.0 of 2024-05-15. (ID:nliolbfbelmkiidjenchbhkpdpdanimc) Experimental ArgentX wallet v5.15.3 of 2024-05-15. (ID:clahjokgpdceomgegfbgoojjkebfagme)

@vladutjs , @avimak , @ivpavici , @amanusk

Test DAPP : https://starknet-wallet-account.vercel.app/ My documentation for this new API : https://github.com/PhilippeR26/Starknet-WalletAccount/blob/main/doc/walletAPIspec.md

Config : Linux mint 21.3 with Chrome 124.0.6367.207. Result of the tests of the experimental wallets, with Starknet.js v6.8.0 & get-starknet v4.0.0-next.5 :

Id	Subject	ArgentX	Braavos	Comment
1	event accountsChanged	- A change of account is also triggering a network events. - After connection, the first opening of the wallet UI is triggering unexpected events for both account and network. - After some minutes of usage, there are no more events triggered	Some unexpected triggering of both events (spaced by a random multiple of minutes: 3'00", 12'00", ...).	To avoid unnecessary refreshes in a DAPP UI, events have to trigger only in case of real change
2	event networkChanged	After some minutes of usage, there are no more events triggered	Some unexpected triggering of both events.
3	wallet_getPermissions	OK	OK
4	wallet_requestAccounts	OK	OK
5	wallet_watchAsset	- Rejected by user is throwing an error, but not an Error 113. - if not an ERC20 -> is throwing an error, but not an Error 111.	- Rejected by user is not throwing an Error 113, but provide a response `false`. - if not an ERC20 -> is not throwing an Error 111, but provide a response `false`.
6	wallet_addStarknetChain	- Rejected by user is throwing an error, but not an Error 113.	Not implemented.
7	wallet_switchStarknetChain	- Rejected by user is throwing an error, but not an Error 113. - request to switch to the same network should not ask the approval of the user, but just return `true`	Not implemented.
8	wallet_requestChainId	OK	OK
9	wallet_deploymentData	OK	Response is `null`	Consistency of the values has not been checked
10	wallet_addInvokeTransaction	Rejected by user is throwing an error, but not an Error 113.	Tx declined by the user throws a `Execute failed` Error, not an Error 113
11	wallet_addDeclareTransaction	- The request is accepted by the wallet, but the transaction fails. Seems to be a problem of stringified abi. Response is an Error `User abort` ; should be probably Error 163 - No response if reject by user.	Do not answer to the command
12	starknet_signTypedData	- Rejected by user is throwing an error, but not an Error 113.	Rejected by user is throwing an error `Signature failed`, not an Error 113.
13	starknet_supportedSpecs	OK	OK

avimak commented 6 months ago

Braavos will return null for wallet_deploymentData in case that the current active account is already deployed

PhilippeR26 commented 6 months ago

Update including new version of Braavos wallet Experimental Braavos wallet v0.0.2-3.49.0 of 2024-05-16. (ID:nliolbfbelmkiidjenchbhkpdpdanimc) Experimental ArgentX wallet v5.15.3 of 2024-05-15. (ID:okiflmmbmjkhhoeahdoamhmodlfmmlnn)

@vladutjs , @avimak , @ivpavici , @amanusk

Test DAPP : https://starknet-wallet-account.vercel.app/ My documentation for this new API : https://github.com/PhilippeR26/Starknet-WalletAccount/blob/main/doc/walletAPIspec.md

Config : Linux mint 21.3 with Chrome 124.0.6367.207. Result of the tests of the experimental wallets, with Starknet.js v6.8.0 & get-starknet v4.0.0-next.5 :

Id	Subject	ArgentX	Braavos	Comment
1	event accountsChanged	- A change of account is also triggering a network events. - After connection, the first opening of the wallet UI is triggering unexpected events for both account and network.	OK	To avoid unnecessary refreshes in a DAPP UI, events have to trigger only in case of real change
2	event networkChanged	After some minutes of usage, there are no more events triggered>	OK
3	wallet_getPermissions	OK	OK
4	wallet_requestAccounts	OK	OK
5	wallet_watchAsset	- Rejected by user is throwing an error, but not an Error 113. - if not an ERC20 -> is throwing an error, but not an Error 111.	- Rejected by user is not throwing an Error 113, but provide a response `false`. - if not an ERC20 -> is not throwing an Error 111, but provide a response `false`.
6	wallet_addStarknetChain	- Rejected by user is throwing an error, but not an Error 113.	Not implemented.
7	wallet_switchStarknetChain	- Rejected by user is throwing an error, but not an Error 113. - request to switch to the same network should not ask the approval of the user, but just return `true`	Not implemented.
8	wallet_requestChainId	OK	OK
9	wallet_deploymentData	OK	Response is `null`	Consistency of the values has not been checked
10	wallet_addInvokeTransaction	Rejected by user is throwing an error, but not an Error 113.	Tx declined by the user throws a `Execute failed` Error, not an Error 113
11	wallet_addDeclareTransaction	- The request is accepted by the wallet, but the transaction fails. Seems to be a problem of stringified abi. Response is an Error `User abort` ; should be probably Error 163 - No response if reject by user.	- Declare declined by the user throws a `Declared failed` Error, not an Error 113. - One time encountered error `-32602: Invalid Params: "Entry points not sorted by selectors`.	In this API, the Abi has to be stringified, not as in Starknet.js where it's a any[]
12	starknet_signTypedData	- Rejected by user is throwing an error, but not an Error 113.	- Rejected by user is throwing an error `Signature failed`, not an Error 113.
13	starknet_supportedSpecs	OK	OK

avimak commented 6 months ago

Braavos will return null for wallet_deploymentData in case that the current active account is already deployed

@PhilippeR26 pls see here 👆 if you want to get a valid response, make sure you are calling on a non-deployed account; returning null for deployed accounts is expected

avimak commented 6 months ago

wrt starknet_signTypedData returning different signature formats between Braavos and argent, this is expected. It's up to the wallet to return a signature that will pass an is_valid_signature check for the signed hash, and it's up to the dapp to assume nothing about the returned signature and use it as-is

PhilippeR26 commented 6 months ago

Braavos will return null for wallet_deploymentData in case that the current active account is already deployed

@PhilippeR26 pls see here 👆 if you want to get a valid response, make sure you are calling on a non-deployed account; returning null for deployed accounts is expected

The @amanusk spec is saying :

"name": "wallet_deploymentData",
      "summary": "Request from the current wallet the data required to deploy the account at the current address",
      "params": [],
      "result": {
        "name": "result",
        "schema": {
          "$ref": "#/components/schemas/ACCOUNT_DEPLOYMENT_DATA"
        }
      },
      "errors": [
        {
          "$ref": "#/components/errors/USER_REFUSED_OP"
        },
        {
          "$ref": "#/components/errors/UNKNOWN_ERROR"
        }
      ]

The error USER_REFUSED_OP is useless as the wallets are responding to this method without opening the UI and of course without asking any approval to the user. @amanusk I propose to remove it.
If the current account is already deployed:
- Argent X is throwing an error Deployment data not found
- Braavos is answering "null". @amanusk , to clarify the situation, I propose to add an error in the spec for this case, for example something as ACCOUNT_ALREADY_DEPLOYED

PhilippeR26 commented 6 months ago

wrt starknet_signTypedData returning different signature formats between Braavos and argent, this is expected. It's up to the wallet to return a signature that will pass an is_valid_signature check for the signed hash, and it's up to the dapp to assume nothing about the returned signature and use it as-is

OK. I will change the table.

starknet-io / get-starknet

Test of experimental Wallets 2024-05-15 #242