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.

Sign up for GitHub

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

Jump to bottom

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

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

wants to merge 3 commits into from

Conversation

rplusq
Copy link
Contributor

@rplusq rplusq commented Sep 12, 2024 •
edited
Loading

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
View reviewed changes
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 •
edited
Loading

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"

grandizzy
grandizzy reviewed Sep 25, 2024
View reviewed changes
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

foundry/crates/cast/tests/cli/main.rs

Lines 1290 to 1294 in ccabf8b

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

foundry/crates/cast/tests/cli/main.rs

Lines 1308 to 1312 in ccabf8b

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 •
edited
Loading

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
Copy link
Contributor Author

rplusq commented Sep 26, 2024

Fixed, thanks @grandizzy

@jenpaff jenpaff mentioned this pull request Sep 26, 2024
Open
@rplusq
Copy link
Contributor Author

rplusq commented Sep 26, 2024 •
edited
Loading

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

@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
@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 3 times, most recently from 7a0ce4d to 9e05f8a Compare October 3, 2024 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@grandizzy grandizzy grandizzy left review comments

@DaniPopes DaniPopes Awaiting requested review from DaniPopes DaniPopes is a code owner

@klkvr klkvr Awaiting requested review from klkvr klkvr is a code owner

@mattsse mattsse Awaiting requested review from mattsse mattsse is a code owner

@yash-atreya yash-atreya Awaiting requested review from yash-atreya yash-atreya is a code owner

@zerosnacks zerosnacks Awaiting requested review from zerosnacks zerosnacks is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@rplusq @grandizzy @jenpaff @zerosnacks