starknet.js icon indicating copy to clipboard operation
starknet.js copied to clipboard

chainId of RpcProvider class not initalized

Open ivpavici opened this issue 2 years ago • 1 comments

From Consensys: We're recently exploring the use of starknet.js v4.1.0 for calling node API via Infura node, but I found a potential bug that the "chainId" of the "RpcProvider" class is not initalized (remain as undefined) in the class constructor, and the reason is the use of async call inside the constructor:

Line 48 from rpc.ts:

this.getChainId().then((chainId) => {
      this.chainId = chainId;
 });

The undefined value of chainId would lead to invalid signature when send a transaction (e.g. via account.execute()) given that the txn hash calculation would include the chainId...

Can you please help take a look with your team to check if this could be a bug or not, thanks a lot!

ivpavici avatar Aug 09 '22 14:08 ivpavici

I tested this and I can confirm it's undefined

https://github.com/0xs34n/starknet.js/commit/575f4a737c2782413ec9912ed1281be86c302c9e

0xs34n avatar Aug 09 '22 14:08 0xs34n

@ivpavici I just try to reproduced it in starknet.js v4.3.0 but failed to do so for many attempts, but I think the problem is still there but it's a bit elusive and not very easy to reproduce: e.g. if I have the following codes:

const provider = new Provider(rpcParam); console.log(chainId: ${provider.chainId});

I would always see the print log showing chainId as undefined, because the constructor would immediately return (constructor cannot be async and being await on in Javascript) and then after a fraction of second (depends on how long it takes for receiving the starknet_chainId endpoint response) the chainId would be filled with a correct value.

So for our use case with the following call sequence: const provider = getProvider(network); const account = new Account(provider, senderAddress, senderKeyPair); return account.execute(txnInvocation, undefined, { maxFee, }); Most of the time, there would be no issue, since chainId is not immediately used (there's an Account contructor call sitting between) and by the time it's being used, it was already initialized.

So I think the issue is still there but only affects use cases where chainId is immediately used after the RpcProvider constructor call, I would recommend to fix it since it's elusive and you never know how the developers gonna use the provider's chainId, but its entirely up to your team to decide.

jonesho avatar Aug 25 '22 14:08 jonesho