Decoupled Starknet.js - Experimental Braavos wallet 0.0.1

[PhilippeR26]

Hello, PR #194 has been created by @avimak to decouple Starknet.js from get-Starknet. One consequence is that Starknet.js will have to communicate directly with the Wallets (ArgentX, Braavos, ...). Work is in progress in the Starknet.js team (@tabaktoni).

My understanding is that the DAPP will request to get-starknet-ui a simplified StarknetWindowObject (without any .provider and .account), and will have access to some requests & events. Requests :

• wallet_requestAccounts
• wallet_watchAsset
• wallet_addStarknetChain
• wallet_switchStarknetChain
• starknet_addInvokeTransaction
• starknet_addDeclareTransaction
• starknet_addDeployAccountTransaction
• starknet_signTypedData

Events :

• accountsChanged
• networkChanged

I have some questions :

Transaction rejected

• If Starknet.js send one of these requests : starknet_addInvokeTransaction starknet_addDeclareTransaction starknet_addDeployAccountTransaction & starknet_signTypedData, and if the user reject the request of signature, how will be the DAPP informed?
• If Starknet.js send the request : starknet_signTypedData, and if the typed Message has a wrong format, how the DAPP will be informed?

Current account

• With the request wallet_requestAccounts or the event accountsChanged, the DAPP is informed of the list of accounts related to the current network. How Starknet.js will be able to switch to one of these accounts?

Event for account changed in Wallet

If the current network is switched in the Wallet, the DAPP is informed by the event networkChanged. If the current account is switched in the Wallet, there is no event for this situation. Couldn't it be beneficial to add an event addressChanged?

estimateFee

There are no requests for estimateFee. How will the DAPPs handles this subject?

simulateTransaction

Same question than the previous subject.

[avimak]

Transaction rejected

• If Starknet.js send one of these requests : starknet_addInvokeTransaction starknet_addDeclareTransaction starknet_addDeployAccountTransaction & starknet_signTypedData, and if the user reject the request of signature, how will be the DAPP informed?

the Promise returned from request will be rejected.

• If Starknet.js send the request : starknet_signTypedData, and if the typed Message has a wrong format, how the DAPP will be informed?

the Promise returned from request will be rejected.

• With the request wallet_requestAccounts or the event accountsChanged, the DAPP is informed of the list of accounts related to the current network.

requestAccounts actually return a single account (see MM specs), not all network accounts

How Starknet.js will be able to switch to one of these accounts?

1. we don't support this functionality today, e.g. dapps can't switch/request-to-switch accounts.
2. why would snjs (or even a dApp/client) need to switch to another account on behalf of the user?

If the current network is switched in the Wallet, the DAPP is informed by the event networkChanged. If the current account is switched in the Wallet, there is no event for this situation. Couldn't it be beneficial to add an event addressChanged?

accountsChanged is triggered every time the wallet's active account changes, including when changing networks.

There are no requests for estimateFee. How will the DAPPs handles this subject?

we already have these APIs today:

const provider = new RpcProvider(<rpc_url>);
provider.getInvokeEstimateFee(...)
provider.getDeployAccountEstimateFee(...)
...

simulateTransaction

Same question than the previous subject.

const provider = new RpcProvider(<rpc_url>);
provider.getSimulateTransaction(...)

[PhilippeR26]

Thanks for the answers.

• Does it means that starknet_addInvokeTransaction starknet_addDeclareTransaction starknet_addDeployAccountTransaction starknet_signTypedData & starknet_signTypedData have to be called in all cases in a try/catch to avoid to throw an Error in the DAPP?
• If the request wallet_requestAccounts and the event accountsChanged are answering with only one address, why is it a 's' at the end of the 'account' word in their names? And why the response of wallet_requestAccounts is this:

  {
  type: "wallet_requestAccounts"
  params?: RequestAccountsParameters
  result: string[]
  }

  with a response as an array of string (just for an account address)? And why the accountsChanged event message is :

  export type AccountChangeEventHandler = (accounts?: string[]) => void

  with again an array of string (with again a 's' at the end of 'account'?

• If there is not list of accounts provided by the Wallet, it's then normal to have no possibility to switch the account from the DAPP.
• For EstimateFee and simulateTransaction, we need as input a nonce and a signature. How to get them?

[PhilippeR26]

Hello, I have coded a small DAPP to test all the functionalities of communication of the wallet. It works with the experimental wallet provided by Braavos : braavos-wallet-chrome_0.0.1.zip. DAPP available here : https://cairo1-js-git-testnewwallet-philipper26.vercel.app/ @ivpavici

[PhilippeR26]

I have extensively used it, and I have some comments : 1. Event accountsChanged :
This event is triggered when needed, but also at unexpected moments : each minute (on testnet), we have an event with an empty response, followed immediately by an other event including the right data.

2. Event accountsChanged : The interface of this event is

export type WalletEvents =
  | {
      type: "accountsChanged"
      handler: AccountChangeEventHandler
    }
export type AccountChangeEventHandler = (accounts?: string[]) => void

So, we can expect an array of string, but the event message is just a string :

As it’s only one active account at a time, it’s logical to have a message with only one account address. I propose to change the interface to be in conformity with this logic :

export type WalletEvents =
  | {
      type: "accountChanged"
      handler: AccountChangeEventHandler
    }
export type AccountChangeEventHandler = (account?: string) => void

Name of the event should also be modified to be representative of the reality : “accountChanged”

[PhilippeR26]

3. Event networkChanged : When this event trigger, the event accountsChanged trigger also. Perfect. My proposal is to officialize this behavior explicitly with a new comment in the StarknetWindowObject.js file. 4. Event networkChanged : It’s not specified what has to be indicated in the message for each network. Today, the message includes mainnet-alpha or goerli-alpha. It’s more common to use in the ecosystem SN_MAIN and SN_GOERLI. I propose to specify this subject in the StarknetWindowObject.js file.

[PhilippeR26]

5. request wallet_requestAccounts : The interface of this request is :

export type RpcMessage =
  | {
      type: "wallet_requestAccounts"
      params?: RequestAccountsParameters
      result: string[]
    }

The answer of the experimental wallet for this message is an array including only one account address : This subject has been discussed during the last coordination video conference ; it has been said that the answer can’t be an array of all accounts addresses, for security reason, but interface will stay as it is. So, if it’s a security issue, I think that we have to solve it. This array has to be replaced by just one single account address. My proposal of interface :

export type RpcMessage =
  | {
      type: "wallet_requestAccount"
      params?: RequestAccountParameters
      result: string
    }

6. request wallet_requestAccounts : It’s not clear what should be the response if the wallet has a problem to answer : a string Error answer, or a Error? I consider that an Error is not an elegant way to proceed, and should be always avoided. I propose to add a comment in the StarknetWindowObject.js file, explaining that if the right answer can’t be send, a string Error has to be answered.

[PhilippeR26]

7. Request for Network : During the coding of this DAPP, it was an evidence for me that one request is missing. We needs a wallet_requestNetwork request. I propose the following interface :

export type RpcMessage =
  | {
      type: "wallet_requestNetwork"
      params?: RequestNetworkParameters
      result: string // authorized answers : SN_MAIN or SN_GOERLI or Error
    }

export interface RequestNetworkParameters {
  /**
   * If true, the wallet will not show the wallet-unlock UI in case of a locked wallet,
   * nor the dApp-approve UI in case of a non-allowed dApp.
   */
  silentMode?: boolean
}

[PhilippeR26]

8. request wallet_watchAsset : Works fine :

Are decimals, name, symbol useful? Whatever value you enter, it’s not used. Does it mean that these data are in fact collected in the ERC20 contract? If it’s the case, my proposal is to add a comment in the interface to explain this behavior. 9. request wallet_watchAsset : If this request is sent for a token already visible, it returns a boolean true. Seems to be different in ArgentX. It has to be clarified in the spec. My proposal is to add a comment explaining : In case of already visible token, the response shall be true.

[PhilippeR26]

10. requests wallet_switchStarknetChain and wallet_addStarknetChain : They are nor working, and return an Error. Tried with SN_MAIN, mainnet-alpha, 0x534e5f4d41494e, ... Are they implemented?

[PhilippeR26]

11. request starknet_addInvokeTransaction : Call type : This type name already exists in Starknet.js, but their definition are not the same, generating confusion when coding. My proposal is to rename in the interface Call to CallParameters. 12. request starknet_addInvokeTransaction : If the user reject the request, we have an Error as response (valid for all Starknet requests). Is this Error due to the user, or from an internal error of the wallet. My proposal is to have for all Starknet requests a better handling of non nominal behavior, and to specify it in the interface.

[PhilippeR26]

13. request starknet_addDeclareTransaction : In interface AddDeclareTransactionParameters, the abi type is not working. My proposal is to modify to abi?:any 14. request starknet_addDeclareTransaction : I didn’t succeeded to use this request :

Do you have an example of valid code using this request?

[PhilippeR26]

15. request starknet_addDeployAccountTransaction : response interface is :

export interface AddDeployAccountTransactionResult {
  transaction_hash: FELT
  contract_address: FELT
}

But the real response is an array for contract_address. My proposal is to modify the wallet to be in accordance with the specification.

[PhilippeR26]

16. request starknet_signTypedData : Works fine. Just the problem already related in item 12.

[PhilippeR26]

17. estimateFee and simulateTransaction : It was noticed during the last coordination video conference that estimateFee can be made with a Provider, using skipValidate ... but SkipValidate is accessible only with a Sequencer, not with a rpc node. And as Sequencers will be deprecated soon.… This is a real issue that has to be investigated seriously.

[PhilippeR26]

In summary, my feeling is that the StarknetWindowObject.js file has to be used as a specification, and that many corrections/adds/comments have to be performed.

[PhilippeR26]

I have made some extra-tests, and I just added 3 comments : 15a, 15b & 18.

[PhilippeR26]

18. Unsubscribe In the DAPP, I stopped the subscriptions to the events with such command :

wallet.off('networkChanged', () => {});

but the events continue to be received. The subscription do not stop. Has to be analyzed.

[avimak]

• Does it means that starknet_addInvokeTransaction starknet_addDeclareTransaction starknet_addDeployAccountTransaction starknet_signTypedData & starknet_signTypedData have to be called in all cases in a try/catch to avoid to throw an Error in the DAPP?

Yes, just like today when a dapp calls enable/request, it shouldn't assume anything about the wallet object's behavior and all its API requests should be covered by a catch handler.

• If the request wallet_requestAccounts and the event accountsChanged are answering with only one address, why is it a 's' at the end of the 'account' word in their names? And why the response of wallet_requestAccounts is this:

{
  type: "wallet_requestAccounts"
  params?: RequestAccountsParameters
  result: string[]
}

...

• If there is not list of accounts provided by the Wallet, it's then normal to have no possibility to switch the account from the DAPP.

1. see https://eips.ethereum.org/EIPS/eip-1102
2. even if wallets currently always return a single account, this might change in the future, so it's beneficial to maintain flexibility in the API.

• For EstimateFee and simulateTransaction, we need as input a nonce and a signature. How to get them?
  1. The nonce can be directly obtained from the contract using RpcProvider, eliminating the need for a wallet API.
  2. We have never incorporated estimate-fee in dapp-wallet APIs; you could use simulateTransaction with skip-validate instead.

[avimak]

As it’s only one active account at a time, it’s logical to have a message with only one account address. I propose to change the interface to be in conformity with this logic :

export type WalletEvents =
  | {
      type: "accountChanged"
      handler: AccountChangeEventHandler
    }
export type AccountChangeEventHandler = (account?: string) => void

Name of the event should also be modified to be representative of the reality : “accountChanged”

L1 compatibility considerations, see https://docs.metamask.io/wallet/how-to/connect/access-accounts/#handle-accounts

[avimak]

3. Event networkChanged : When this event trigger, the event accountsChanged trigger also. Perfect. My proposal is to officialize this behavior explicitly with a new comment in the StarknetWindowObject.js file.

Actually, following Francesco's suggestion, we should also include the accounts array alongside the network to prevent a situation where the dApp has accounts of network A while the wallet has already switched to network B (even if an account-changed event is about to be triggered asap), so -

export type NetworkChangeEventHandler = (
  network?: string,
  accounts?: string[],
) => void

4. Event networkChanged : It’s not specified what has to be indicated in the message for each network. Today, the message includes mainnet-alpha or goerli-alpha. It’s more common to use in the ecosystem SN_MAIN and SN_GOERLI. I propose to specify this subject in the StarknetWindowObject.js file.

~OK, will do~ perhaps ChainId, not NetworkName, to be compatible with the chainId deprecation API (it's a breaking change version anyway).

[avimak]

7. Request for Network : During the coding of this DAPP, it was an evidence for me that one request is missing. We needs a wallet_requestNetwork request. I propose the following interface :

export type RpcMessage =
  | {
      type: "wallet_requestNetwork"
      params?: RequestNetworkParameters
      result: string // authorized answers : SN_MAIN or SN_GOERLI or Error
    }

export interface RequestNetworkParameters {
  /**
   * If true, the wallet will not show the wallet-unlock UI in case of a locked wallet,
   * nor the dApp-approve UI in case of a non-allowed dApp.
   */
  silentMode?: boolean
}

Actually, as part of chainId: string deprecation, we probably need to return this data via wallet_requestChainId. Also, silentMode isn't required since you must be connected (via requestAccounts) in order to communicate with the wallet.

[avimak]

Are decimals, name, symbol useful? Whatever value you enter, it’s not used. Does it mean that these data are in fact collected in the ERC20 contract? If it’s the case, my proposal is to add a comment in the interface to explain this behavior.

That's an implementation detail of the wallet. It's up to the wallet to decide whether to accept the data as-is in certain cases or to ignore everything except the address and fetch the information from the chain. Not sure it should be specified in specs.

9. request wallet_watchAsset : If this request is sent for a token already visible, it returns a boolean true. Seems to be different in ArgentX. It has to be clarified in the spec. My proposal is to add a comment explaining : In case of already visible token, the response shall be true.

this request must return a boolean as defined in -

  | {
      type: "wallet_watchAsset"
      params: WatchAssetParameters
      result: boolean
    }

if argent isn't returning a boolean, that should be communicated directly to them and flagged as a bug (cc @dhruvkelawala )

[avimak]

10. requests wallet_switchStarknetChain and wallet_addStarknetChain : They are nor working, and return an Error. Tried with SN_MAIN, mainnet-alpha, 0x534e5f4d41494e, ... Are they implemented?

we didn't implement it (yet) on Braavos.

[avimak]

11. request starknet_addInvokeTransaction : Call type : This type name already exists in Starknet.js, but their definition are not the same, generating confusion when coding. My proposal is to rename in the interface Call to CallParameters.

Naor is working on a common types library for snjs/get-starknet, which I guess will help us align on types.

12. request starknet_addInvokeTransaction : If the user reject the request, we have an Error as response (valid for all Starknet requests). Is this Error due to the user, or from an internal error of the wallet. My proposal is to have for all Starknet requests a better handling of non nominal behavior, and to specify it in the interface.

Would you prefer receiving undefined instead? How does this compare to the behavior in other SN wallets?

[avimak]

18. Unsubscribe In the DAPP, I stopped the subscriptions to the events with such command :

wallet.off('networkChanged', () => {});

but the events continue to be received. The subscription do not stop. Has to be analyzed.

A dApp can register multiple callbacks for the same event. If you want to stop receiving events in a specific callback, you should pass the same callback registered in on to off. i.e.:

const handler = (...) => {...}
wallet.on(<event>, handler);
...
wallet.off(<event>, handler);