Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(cast): cast send tries to decode custom errors #8860

Closed
wants to merge 7 commits into from

Conversation

rplusq
Copy link
Contributor

@rplusq rplusq commented Sep 12, 2024

Co-authored-by: Howard [email protected]

Motivation

Even though issue #8606 is partially solved, this would be an improvement over the UX. Only applies this for custom errors

Nightly cast send:

cast send 0x5FbDB2315678afecb367f032d93F642f64180aa3 "setValue(uint256)" 101 --private-key=0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80
Error:
server returned an error response: error code 3: execution reverted: custom error 7a0e1985:ed, data: "0x7a0e198500000000000000000000000000000000000000000000000000000000000000650000000000000000000000000000000000000000000000000000000000000064"

Our PR:

cast send 0x5FbDB2315678afecb367f032d93F642f64180aa3 "setValue(uint256)" 101 --private-key=0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80

Error: 
Reverted with custom error: 
 Possible methods:
 - ValueTooHigh(uint256,uint256)
 ------------
 [000]: 0000000000000000000000000000000000000000000000000000000000000065
 [020]: 0000000000000000000000000000000000000000000000000000000000000064


Context:
- server returned an error response: error code 3: execution reverted: custom error 7a0e1985:ed, data: "0x7a0e198500000000000000000000000000000000000000000000000000000000000000650000000000000000000000000000000000000000000000000000000000000064"

Solution

Using pretty_calldata to decode the custom error.

@rplusq
Copy link
Contributor Author

rplusq commented Sep 17, 2024

Thinking it solves #8603 as well

zerosnacks
zerosnacks previously approved these changes Sep 25, 2024
Copy link
Member

@zerosnacks zerosnacks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your PR! LGTM

Confirming previously:

cast send 0x5FbDB2315678afecb367f032d93F642f64180aa3 "setValue(uint256)" 101 --private-key=0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80
Error: 
server returned an error response: error code 3: execution reverted: custom error 0x7a0e1985: ed, data: "0x7a0e198500000000000000000000000000000000000000000000000000000000000000650000000000000000000000000000000000000000000000000000000000000064"
cast send 0x5FbDB2315678afecb367f032d93F642f64180aa3 "setValue(uint256)" 101 --private-key=0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80
Error: 
Reverted with custom error: 
 Possible methods:
 - ValueTooHigh(uint256,uint256)
 ------------
 [000]: 0000000000000000000000000000000000000000000000000000000000000065
 [020]: 0000000000000000000000000000000000000000000000000000000000000064


Context:
- server returned an error response: error code 3: execution reverted: custom error 0x7a0e1985: ed, data: "0x7a0e198500000000000000000000000000000000000000000000000000000000000000650000000000000000000000000000000000000000000000000000000000000064"

@rplusq
Copy link
Contributor Author

rplusq commented Sep 25, 2024

You can try in Sepolia at https://sepolia.etherscan.io/address/0xbabec3df164f14672c08aa277af9936532c283ba (verified for debugging)

UnknownError (not uploaded in OpenChain Signatures)

cast send 0xbabeC3dF164f14672c08AA277Af9936532c283Ba 'setValueUnknownError(uint256)' 101 --account rplusq.eth`
Enter keystore password:
Error: 
Reverted with custom error: 
 Method: a0370657
 ------------
 [000]: 0000000000000000000000000000000000000000000000000000000000000065
 [020]: 0000000000000000000000000000000000000000000000000000000000000064


Context:
- server returned an error response: error code 3: execution reverted: custom error a0370657:ed, data: "0xa037065700000000000000000000000000000000000000000000000000000000000000650000000000000000000000000000000000000000000000000000000000000064"

KnownError (uploaded in OpenChain Signatures)

cast send 0xbabeC3dF164f14672c08AA277Af9936532c283Ba 'setValueKnownError(uint256)' 101 --account rplusq.eth`
Enter keystore password:
Error: 
Reverted with custom error: 
 Possible methods:
 - ValueTooHigh(uint256,uint256)
 ------------
 [000]: 0000000000000000000000000000000000000000000000000000000000000065
 [020]: 0000000000000000000000000000000000000000000000000000000000000064


Context:
- server returned an error response: error code 3: execution reverted: custom error 7a0e1985:ed, data: "0x7a0e198500000000000000000000000000000000000000000000000000000000000000650000000000000000000000000000000000000000000000000000000000000064"

Copy link
Collaborator

@grandizzy grandizzy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think would worth adding a test similar with this one which starts anvil

casttest!(send_eip7702, async |_prj, cmd| {
let (_api, handle) =
anvil::spawn(NodeConfig::test().with_hardfork(Some(EthereumHardfork::PragueEOF.into())))
.await;
let endpoint = handle.http_endpoint();

, deploys contract, calls a fn that always revert with a custom error and make sure output is expected

cmd.cast_fuse()
.args(["code", "0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266", "--rpc-url", &endpoint])
.assert_success()
.stdout_eq(str![[r#"
0xef010070997970c51812dc3a010c7d01b50e0d17dc79c8

Since #8603 is mentioned, I see in issue description that reporter says custom error used to be displayed in old version of cast, so wonder if it's not only a regression and could avoid fetching signatures from openchain...

@rplusq
Copy link
Contributor Author

rplusq commented Sep 25, 2024

I think would worth adding a test similar with this one which starts anvil

Sounds good, will add

Since #8603 is mentioned, I see in issue description that reporter says custom error used to be displayed in old version of cast, so wonder if it's not only a regression and could avoid fetching signatures from openchain...

Yep, that was eventually fixed. The addition to fetch signatures from openchain I added to make those more readable when possible. It's opinionated of course, but I think it improves the ux.

@rplusq rplusq force-pushed the feat(cast)/send-decodes branch 3 times, most recently from 88b9ddc to 62e596f Compare September 25, 2024 21:18
@rplusq
Copy link
Contributor Author

rplusq commented Sep 25, 2024

@grandizzy any idea why my test keeps failing? Seems like something is missing and I cannot figure it out

CleanShot 2024-09-25 at 22 38 21@2x

@grandizzy
Copy link
Collaborator

@grandizzy any idea why my test keeps failing? Seems like something is missing and I cannot figure it out

you can run test with SNAPSHOTS=override but since that's not relevant for test you could just redact as

        .stderr_eq(str![[r#"
Error: 
Reverted with custom error: 
 Possible methods:
 - ValueTooHigh(uint256,uint256)
 ------------
 [000]: 0000000000000000000000000000000000000000000000000000000000000065
 [020]: 0000000000000000000000000000000000000000000000000000000000000064
...
"#]]);

@rplusq rplusq force-pushed the feat(cast)/send-decodes branch from 7d04af4 to 8bbfbb6 Compare September 26, 2024 10:38
@rplusq
Copy link
Contributor Author

rplusq commented Sep 26, 2024

Fixed, thanks @grandizzy

@rplusq
Copy link
Contributor Author

rplusq commented Sep 26, 2024

Not sure if this PR will solve #8348. My PR is currently limited to cast send, though would be interested in making it more generic for the rest of the stack.

cc: @jenpaff

@rplusq rplusq force-pushed the feat(cast)/send-decodes branch from 8bbfbb6 to 7845009 Compare September 26, 2024 14:29
@jenpaff
Copy link
Collaborator

jenpaff commented Sep 26, 2024

Not sure if this PR will solve #8348. My PR is currently limited to cast send, though would be interested in making it more generic for the rest of the stack.

cc: @jenpaff

Thanks for flagging, let me reopen then & re-evaluate

@zerosnacks zerosnacks self-requested a review September 26, 2024 14:49
@zerosnacks zerosnacks dismissed their stale review September 26, 2024 14:50

to re-review

@rplusq rplusq force-pushed the feat(cast)/send-decodes branch 6 times, most recently from 798c8f9 to 7cde6d1 Compare October 2, 2024 18:34
@rplusq rplusq force-pushed the feat(cast)/send-decodes branch 4 times, most recently from 9e05f8a to c6acc83 Compare October 9, 2024 11:49
@rplusq rplusq requested a review from grandizzy November 29, 2024 13:49
@rplusq
Copy link
Contributor Author

rplusq commented Nov 29, 2024

@zerosnacks @grandizzy trying to get this merged, would've been useful for me a couple of times already and it's been pending for almost 3 months

@rplusq rplusq force-pushed the feat(cast)/send-decodes branch from e180207 to 526ced0 Compare November 29, 2024 17:35
@rplusq rplusq force-pushed the feat(cast)/send-decodes branch from 89efafb to fe50a7e Compare November 29, 2024 17:53
@rplusq rplusq requested a review from zerosnacks November 29, 2024 18:22
@zerosnacks
Copy link
Member

Hi @rplusq,

My apologies for the delay. The reason why the PR has been open for some time is because there has been an ongoing discussion around what the PR is attempting to achieve and whether there are better ways to address the core issue.

We've decided not to move ahead with the approach of this PR. Instead we've added cast decode-error in #9428 which handles the resolving of custom errors by either --sig, local cache and the openchain API

The proposed way of enabling what this PR is attempting to achieve is by piping the output of cast send through cast decode-error as follows (or manually):

$ forge selectors cache

$ cast send 0x5FbDB2315678afecb367f032d93F642f64180aa3 "setValue(uint256)" 101 --private-key=0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80 2>&1 | grep -oP '(?<=data: ")0x[0-9a-fA-F]+' | xargs -I{} cast decode-error {}'

It was not an easy decision considering the time and effort you've spend on this PR and for that we apologize. For the future we aim for quicker feedback loops.

@zerosnacks zerosnacks closed this Dec 3, 2024
@rplusq rplusq deleted the feat(cast)/send-decodes branch December 3, 2024 21:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants