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

refactor: abi err msg parse #388

Closed
wants to merge 1 commit into from
Closed

Conversation

wenyuanhust
Copy link
Contributor

@wenyuanhust wenyuanhust commented Nov 30, 2023

Closes: #274

Description

At present, if the call of solidity contract failed, things like this printed

Contract call reverted with data: 0x08c379a00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000001c74657374206661696c656420746f2063726561746520636c69656e7400000000

After this pr, following will be printed

test failed to create client

which makes error message more human friendly.
I haven't found a rust crate to do it, so I parse 2 kinds of errors mannually.


PR author checklist:

  • Added changelog entry, using unclog.
  • Added tests: integration (for Forcerelay) or unit/mock tests (for modules).
  • Linked to GitHub issue.
  • Updated code comments and documentation (e.g., docs/).
  • Tagged one reviewer who will be the one responsible for shepherding this PR.

Reviewer checklist:

  • Reviewed Files changed in the GitHub PR explorer.
  • Manually tested (in case integration/unit/mock tests are absent).

@wenyuanhust wenyuanhust changed the title tmp: abi err msg parse refactor: abi err msg parse Dec 4, 2023
@wenyuanhust wenyuanhust marked this pull request as ready for review December 4, 2023 12:58
@@ -29,6 +29,10 @@ pub fn convert_err<T: ToString>(err: T) -> Error {
Error::other_error(err.to_string())
}

pub fn convert_abi_err<T: ToString>(err: T) -> Error {
Error::other_error(parse_abi_err_data(&err.to_string()))
Copy link
Contributor

Choose a reason for hiding this comment

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

The error may not be a call revert error

let err = parse_abi_err_data(err_string);
assert_eq!(
err,
"parse_abi_err_data: unknown exception method 18c379a0!"
Copy link
Contributor

Choose a reason for hiding this comment

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

Data should be retained when we cannot decode it.

fn test_sol_invalid_data() {
let err_string = "Contract call reverted with data: 0x08c379a00000000000000000000000000000000000000000000000000000000000000050000000000000000000000000000000000000000000000000000000000000001c74657374206661696c656420746f2063726561746520636c69656e7400000000";
let err = parse_abi_err_data(err_string);
assert_eq!(err, "parse_abi_err_data decode: InvalidData, data: [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 50, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1c, 74, 65, 73, 74, 20, 66, 61, 69, 6c, 65, 64, 20, 74, 6f, 20, 63, 72, 65, 61, 74, 65, 20, 63, 6c, 69, 65, 6e, 74, 0, 0, 0, 0]!");
Copy link
Contributor

Choose a reason for hiding this comment

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

Data should be retained (in hex) when we cannot decode it.

let result = strings.join(";");
if error_type == AbiErrorType::Panic {
let panic_error = parse_panic_error(&result);
handle_panic_error(panic_error)
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't be necessary to convert the token to hex again.

@blckngm
Copy link
Contributor

blckngm commented Dec 5, 2023

See #390 where I tried to address the aforementioned problems.

let start_index = "Contract call reverted with data: 0x".len();
let method_signature_len = "08c379a0".len();

let method_signature = &err[start_index..(start_index + method_signature_len)];
Copy link
Contributor

Choose a reason for hiding this comment

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

May panic if err is not long enough.

@wenyuanhust wenyuanhust closed this Dec 5, 2023
@Flouse
Copy link
Collaborator

Flouse commented Dec 5, 2023

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The error from solidity contract is hex encoded, we should parse it to human-readable string format
3 participants