search

trufflesuite / truffle

:warning: The Truffle Suite is being sunset. For information on ongoing support, migration options and FAQs, visit the Consensys blog. Thank you for all the support over the years.
https://consensys.io/blog/consensys-announces-the-sunset-of-truffle-and-ganache-and-new-hardhat?utm_source=github&utm_medium=referral&utm_campaign=2023_Sep_truffle-sunset-2023_announcement_
MIT License
14.02k stars 2.31k forks source link

"Custom error (could not decode)" for tx rejection #5881

Open wbt opened 1 year ago

wbt commented 1 year ago

Issue

When a user uses a Truffle contract object to trigger a transaction in a browser (tested using MetaMask), and the user rejects the transaction, the error coming back has members

message: "MetaMask Tx Signature: User denied transaction signature. -- Reason given: Custom error (could not decode)."
reason: "Custom error (could not decode)"

likely due to this line.

It also has code: 4001 which could be used to figure out that this is a user rejecting the transaction.

That seems like a common enough issue that it should probably be handled well. It might also lead to different decisions about what to do in the UI.

Steps to Reproduce

Use a Truffle contract method to trigger a transaction in a browser with MetaMask, and reject the transaction. Have code that listens for and displays errors, in the console and/or on-screen.

Expected Behavior

A notice that the user rejected the transaction.

Actual Results

A notice that the issue was a custom error Truffle couldn't decode. The numeric code is present and can be caught/decoded in the application code, so this is not a major blocker, but it is a common enough use case that "could not decode" suggests Truffle immaturity as a tool for smart contract interactions.

Environment

haltman-at commented 1 year ago

Hm, I'm confused why there was data attached if the transaction was rejected by the user. I would expect that to hit either line 58 or line 61, so you wouldn't get this message. In order to hit line 86 (which is obviously what it's hitting), there has to be data attached. What is this data and why...?

wbt commented 1 year ago

In a recent repro, after a tx was rejected the following was shown as originating from Metamask's inpage.js:1 (i.e. before Truffle tries to decode it): 

{
code: 4001,
message: "MetaMask Tx Signature: User denied transaction signature."
}

Approximately 0.2 seconds later, the error was output to console by application code (i.e. just after Truffle's decoding attempt), with a few more keys (modifications made to stack value only):

{
code: 4001,
hijackedStack: "{\n  \"code\": 4001,\n  \"message\": \"MetaMask Tx Signature: User denied transaction signature.\",\n  \"stack\": \"Error: MetaMask Tx Signature: User denied transaction signature.\\n    at new n (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-1.js:12:23059)\\n    at new t.EthereumProviderError (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-1.js:12:23574)\\n    at o (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-1.js:12:25909)\\n    at Object.userRejectedRequest (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-1.js:12:27140)\\n    at g.<anonymous> (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/background-3.js:1:6733)\\n    at Object.l (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-3.js:10:210791)\\n    at u (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-6.js:3:1144)\\n    at a.emit (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-6.js:3:1680)\\n    at g._setTransactionStatus (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/background-3.js:1:53365)\\n    at g.setTxStatusRejected (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/background-3.js:1:51552)\\n    at B.cancelTransaction (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/background-3.js:1:23301)\\n    at a.<anonymous> (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/background-4.js:10:104664)\\n    at b (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-1.js:1:96305)\\n    at a.emit (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-1.js:1:99300)\\n    at A (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-6.js:1:111283)\\n    at E (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-6.js:1:111098)\\n    at _.push (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-6.js:1:111912)\\n    at e.exports._write (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-6.js:1:45435)\\n    at _ (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-6.js:1:124328)\\n    at chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-6.js:1:127512\\n    at y.write (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-6.js:1:127539)\\n    at e.exports.m (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-6.js:1:116483)\\n    at b (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-1.js:1:96305)\\n    at a.emit (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-1.js:1:99300)\\n    at A (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-6.js:1:111283)\\n    at E (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-6.js:1:111098)\\n    at _.push (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-6.js:1:111912)\\n    at e.exports._onMessage (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-3.js:10:216138)\\n    at chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-3.js:10:215985\"\n}\n  at new n (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-1.js:12:23059)\n  at new t.EthereumProviderError (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-1.js:12:23574)\n  at o (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-1.js:12:25909)\n  at Object.userRejectedRequest (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-1.js:12:27140)\n  at g.<anonymous> (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/background-3.js:1:6733)\n  at Object.l (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-3.js:10:210791)\n  at u (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-6.js:3:1144)\n  at a.emit (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-6.js:3:1680)\n  at g._setTransactionStatus (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/background-3.js:1:53365)\n  at g.setTxStatusRejected (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/background-3.js:1:51552)\n  at B.cancelTransaction (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/background-3.js:1:23301)\n  at a.<anonymous> (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/background-4.js:10:104664)\n  at b (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-1.js:1:96305)\n  at a.emit (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-1.js:1:99300)\n  at A (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-6.js:1:111283)\n  at E (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-6.js:1:111098)\n  at _.push (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-6.js:1:111912)\n  at e.exports._write (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-6.js:1:45435)\n  at _ (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-6.js:1:124328)\n  at chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-6.js:1:127512\n  at y.write (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-6.js:1:127539)\n  at e.exports.m (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-6.js:1:116483)\n  at b (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-1.js:1:96305)\n  at a.emit (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-1.js:1:99300)\n  at A (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-6.js:1:111283)\n  at E (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-6.js:1:111098)\n  at _.push (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-6.js:1:111912)\n  at e.exports._onMessage (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-3.js:10:216138)\n  at chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-3.js:10:215985",
message: MetaMask Tx Signature: User denied transaction signature. -- Reason given: Custom error (could not decode).",
reason: "Custom error (could not decode)",
stack: "{\n    at https://server/js/bundles/myBundle.js:2:1234567\n    at new Promise (<anonymous>)\n    at Object.contractCallWrapper (https://server/js/bundles/myBundle.js:2:1237654)\n    at Object.callingContext (https://server/js/bundles/myBundle.js:2:1234675)\n    at https://server/js/bundles/myBundle.js:2:1234576"
}

The hijackedStack references look to all be in MetaMask (v10.24.2), and the stack references are in application code.

My first guess is that the bug is somewhere in the _extract function, probably one of the first three lines, but maybe in its calling context.

I'm not seeing result or error keys in either of these error objects.

haltman-at commented 1 year ago

OK, I think I know what's going on. The thing we weren't considering here is that Truffle Contract, in order to get the revert message, replays the transaction as a call; that's what's hitting _extract, etc. I forgot about that step.

But, in this case, replaying the transaction as a call will succeed! And if that successful call has a return value... you'll hit "could not decode". Oops.

Indeed, I'm able to reproduce this without using MetaMask by just sending a transaction that has a return value and giving it too little gas. Notably, when I do this for a transaction with no return value, the error doesn't occur.

Hm, that's troublesome. Will have to think about what to do here...

haltman-at commented 1 year ago

I guess a quick fix might be, check the length of the response and see if it's 4 mod 32...? I guess I'll go do that, that seems like probably the best solution to me.

haltman-at commented 1 year ago

Well, OK, that might not quite be enough -- that'll fix the immediate problem, but if the replay as a call itself fails, and has a revert message, then you could still get a revert message inappropriately attached. So checking for code 4001 might also be a good idea? Possibly certain other codes? That seems to me like it's probably better handled separately, though.

haltman-at commented 1 year ago

OK, I've put up #5903, which should fix the bulk of the problem. We can look into a more comprehensive solution later, I figure.

wbt commented 1 year ago

Thanks! I'd recommend at least checking for code 4001 as that's a pretty common issue.