near-api-js icon indicating copy to clipboard operation
near-api-js copied to clipboard

Contract methods with deposits fail if user doesn't have full access key for account

Open mattlockyer opened this issue 5 years ago • 2 comments

Describe the bug With the addition of 2FA we have accounts that ONLY have limited access keys. In an app this makes signing of contract changeMethods with deposits impossible (requires full access key). Scenario: You approve an access key to call methods of a contract. All good. Now the app wants to call a method with a deposit. Requires a full access key on the account. near-api-js currently throws an error in the app that it doesn't have any access keys that can sign this type of transaction. However, we could prepare a multisig request in the wallet that would allow the user to call the method with the deposit attached.

To Reproduce Steps to reproduce the behavior: 1. create an account and enable 2FA using this render instance: https://near-wallet-pr-700.onrender.com/ 2. create-near-app some sample app 3. change the config.js to point to this wallet instance: https://near-wallet-pr-700.onrender.com/ 4. sign in, complete 2FA flow 5. try to change the greeting with the deposit amount "1" (the default) 6. see exception in app, "Cannot find matching key for transaction sent to <span class="error">[contractName]</span>"

This is because your account only has limited access keys to your multisig contract AND the current app. Meaning that no method calls with attached deposits are possible. However, creating a multisig request for a function call with an attached deposit and approving that IS POSSIBLE.

Expected behavior We should not throw an exception and instead continue to redirect the user to /sign?tx=... for their wallet.

Additional context https://github.com/near/near-api-js/blob/3993ebe9142e8d15e55eb54f94d8c852df669722/src/wallet-account.ts#L185

Should actually still redirect to wallet.

This is what @kcole16 and myself came up with as lowest hanging fruit for now given wallet with 2FA is launching soon and these transactions could be important for staking, transferring Near in apps.

@vgrichina had some ideas on how to override Account / signAndSendTransaction, curious if there's a better solution?

mattlockyer avatar Jul 24 '20 21:07 mattlockyer

WIP branch here

Basically if: https://github.com/near/near-api-js/blob/43022551758e9a4b934b0156f5058dd36644bebe/src/wallet-account.ts#L205-L209

Then do this: https://github.com/near/near-api-js/blob/43022551758e9a4b934b0156f5058dd36644bebe/src/wallet-account.ts#L115-L128

tests passed, still need to conduct e2e testing, ETA EOD 07/25 or will have to be 07/28

mattlockyer avatar Jul 25 '20 17:07 mattlockyer

@vgrichina still haven't had time to e2e test this, but what do you think of the approach so far? This should redirect to the wallet and we can include all 2FA logic for checking there.

mattlockyer avatar Jul 29 '20 18:07 mattlockyer