starknet-io / starknet.js

JavaScript library for StarkNet
https://www.starknetjs.com
MIT License
1.21k stars 739 forks source link

Invoke gas estimate does not work if I use AccountInterface object received from get-starknet #838

Closed lazarmitic closed 10 months ago

lazarmitic commented 10 months ago

Describe the bug If I create Account object using private key invoke gas estimate works fine without any issues, but if I try to use account object (AccountInterface) received from get-starknet, it fails.

To Reproduce Connect wallet using get-starknet. Try to estimate invoke gas estimate. It fails with error:

Jy: Execution was reverted; failure reason: [0x496e70757420746f6f2073686f727420666f7220617267756d656e7473

Expected behavior Invoke gas estimate should be returned for speicified calls array.

Screenshots N/A

Desktop (please complete the following information):

Additional context I tried to use latest version of Starknet.js as well (pre-release) and it still fails.

lazarmitic commented 10 months ago

I also tried just now to create new Account object using signer instead of private key, and this is the error I get:

LibraryError: RPC: starknet_estimateFee with params {"request":[{"type":"INVOKE","sender_address":"0x5295882956890bb9c05e54c1e7efa903a4a21083ebd6a3c175507f74e549574","calldata":["0x2","0xda114221cb83fa859dbdb4c44beeaa0bb37c7537ad5ae66fe5e0efd20e6eb3","0x219209e083275171774dab1df80982e9df2096516f06319c5c6d71ae0a8480c","0x3","0x5c4676bcb21454659479b3cd0129884d914df9c9b922c1c649696d2e058d70","0xde0b6b3a7640000","0x0","0x5c4676bcb21454659479b3cd0129884d914df9c9b922c1c649696d2e058d70","0x2f0b3c5710379609eb5495f1ecd348cb28167711b73609fe565a72734550354","0x3","0x5295882956890bb9c05e54c1e7efa903a4a21083ebd6a3c175507f74e549574","0xde0b6b3a7640000","0x0"],"version":"0x100000000000000000000000000000001","signature":["0x4a556e7b2738022a03c18d9795214adaa4ce2e81943842b2baf8133e37dc469","0x320510cb1df7dbd45708582a19b7d9bdc2a9434b6161842c29ae6798f34cc3b"],"nonce":"0x47","max_fee":"0x0"}],"block_id":"pending"}
 -32603: Internal error: {"error":"Internal error: Execution failed. Failure reason: \"argent/invalid-owner-sig\"."}
lazarmitic commented 10 months ago

More info, I just noticed that account.signer.pk is different after every page refresh when using ArgentX, and it's always 0x0 when using bravoos, I guess that's why I get argent/invalid-owner-sig error when trying to use signer.

ivpavici commented 10 months ago

hello! Without checking too much, this could be a problem with the get-starknet library itself

lazarmitic commented 10 months ago

Yeah, Starknet.js works when I use private key, but window.starknet_argentX.account does not work. So it seems it's extension problem. I'm speaking with ArgentX team, will post update here if we manage to resolve the issue.

janek26 commented 10 months ago

can you try to skip the validation with the window object? As an alternative, I think you should load your own rpc provider to do these requests, see wallet/provider split, like in viem https://viem.sh/docs/faq.html#why-doesn-t-wallet-client-support-public-actions

lazarmitic commented 10 months ago

Even if I use 'skipValidate' it still fails with error "argent/invalid-owner-sig\"

lazarmitic commented 10 months ago

I can see that event if i use account.estimateInvokeFee, under the hood, it uses RPC to estimate the fee, but it needs to pass signature to estimate, so that's why it needs account.

    const response = await super.getInvokeEstimateFee(
      { ...invocation },
      { version, nonce },
      blockIdentifier,
      skipValidate
    );
janek26 commented 10 months ago

As I said you should split between publicClient (read) and walletClient (write)

If you dont want to do that, you should be able to estimate with the walletClient too, by using skipValidate. I remember RPC may not support that flag yet, or just supports it on simulate or sth. Please check the code argent/invalid-owner-sig can only happen if the flag is not passed/ignored

lazarmitic commented 10 months ago

Yeah, you are right, it seems because starknet.js uses RPC under the hood to send estimate request, and RPC does not support skipValidate, this flag is not passed. And sequencer is getting deprecated for public reads starting december, so I can't use it.

ivpavici commented 10 months ago

I think this will be possible with RPC v0.5, when all node operators support it (I'm not sure which ones do currently tbh, have to check). Your version of sn.js (the latest official - 5.19.5) still works with rpc 0.4... The next official release will work with v0.5!

lazarmitic commented 10 months ago

It works with sequencer provider + validate flag. Hopefully RPC v0.5 gets supported before starknet deprecates sequencer.

ivpavici commented 10 months ago

yes yes, that is the point of RPC v0.5, to reach feature parity with the sequencer

lazarmitic commented 10 months ago

Thanks for the help guys, I will close the ticket now.