foundry icon indicating copy to clipboard operation
foundry copied to clipboard

feat(cast): Add revert reason decoding to `cast receipt`

Open marktoda opened this issue 2 years ago • 7 comments

Component

Cast

Describe the feature you would like

I was playing in an anvil fork environment today and some transactions were failing without outputting the revert string. It would be nice if we had cast tx <hash> output some more information about reverting transactions.

This can generally be done by making an eth_call with all of the same args as the write transaction, see a js implementation here: https://www.npmjs.com/package/eth-revert-reason

For reference this is what a cast tx for a failing tx currently looks like:

❯ cast tx --rpc-url infura 0x0e07d8b53ed3d91314c80e53cf25bcde02084939395845cbb625b029d568135c


blockHash               0x883f974b17ca7b28cb970798d1c80f4d4bb427473dc6d39b2a7fe24edc02902d
blockNumber             14839405
from                    0x3cf412d970474804623bb4e3a42de13f9bca5436
gas                     289282
gasPrice                21491736378
hash                    0x0e07d8b53ed3d91314c80e53cf25bcde02084939395845cbb625b029d568135c
input                   0x5ae401dc00000000000000000000000000000000000000000000000000000000628ced5b000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000040000000000000000000000000000000000000000000000000000000000000016000000000000000000000000000000000000000000000000000000000000000e442712a6700000000000000000000000000000000000000000000b3ff1489674e11c40000000000000000000000000000000000000000000000000000004a6ed55bbcc18000000000000000000000000000000000000000000000000000000000000000800000000000000000000000003cf412d970474804623bb4e3a42de13f9bca54360000000000000000000000000000000000000000000000000000000000000002000000000000000000000000c02aaa39b223fe8d0a0e5c4f27ead9083c756cc20000000000000000000000003a75941763f31c930b19c041b709742b0b31ebb600000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000412210e8a00000000000000000000000000000000000000000000000000000000
nonce                   365
r                       0x7f2153019a74025d83a73effdd91503ceecefac7e35dd933adc1901c875539aa
s                       0x334ab2f714796d13c825fddf12aad01438db3a8152b2fe3ef7827707c25ecab3
to                      0x68b3465833fb72a70ecdf485e0e4c7bd8665fc45
transactionIndex        173
v                       0
value                   20951010922774912

I'd suggest we add a new required field status and an optional one reason:

status: Failed
reason: Transaction too old

An alternative option is to add a separate command cast reason <hash> for this specific case - if we want cast tx to stay purely tx blob info

marktoda avatar May 25 '22 02:05 marktoda

I'm happy to add this myself, wanted to get consensus around it first

marktoda avatar May 25 '22 03:05 marktoda

This would require an archive node for older transactions. I definitely think that would be a nice feature for cast send and cast call --json. It seems like it could be a better feature for cast receipt compared to cast tx since the receipt includes info about the results of the execution?

tynes avatar May 25 '22 06:05 tynes

definitely, a useful feature that we can then incorporate into the commands that they might need it.

Agree on receipt over tx, depending on the status field we then can try to replay the tx and extract the revert reason?

mattsse avatar May 25 '22 07:05 mattsse

I also agree, should be in cast receipt

onbjerg avatar May 27 '22 16:05 onbjerg

There's an implicit performance concern here, because if you're doing cast receipt now it's ~instant, whereas if the transaction fails, you'll need to instantiate a forking provider and do a cast run, which can be slow esp if we end up re-executing all the transactions in the block.

Also, it might require a refactor of the RunArgs::run_tx logic so that it can be re-used easily.

gakonst avatar Jun 08 '22 20:06 gakonst

Do we need to do a cast run? I'd assume an eth_call is enough to get the revert reason, but maybe I'm wrong

onbjerg avatar Jun 09 '22 14:06 onbjerg

Yikes - you're right.

gakonst avatar Jun 11 '22 15:06 gakonst