Skip to content

fix cast keccak bug #671

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

Merged
merged 2 commits into from
Feb 4, 2022
Merged

fix cast keccak bug #671

merged 2 commits into from
Feb 4, 2022

Conversation

calldata
Copy link
Contributor

@calldata calldata commented Feb 4, 2022

fixes #669

Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

lgtm,
additional doc would be great

cast/src/lib.rs Outdated
Comment on lines 1043 to 1047
let hash: String = match data.as_bytes() {
[b'0', b'x', rest @ ..] => keccak256(hex::decode(rest)?).to_hex(),
_ => keccak256(data).to_hex(),
};

Copy link
Member

Choose a reason for hiding this comment

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

this is really neat,

can we document this accordingly

If has a 0x prefix, read it as hexdata. Multiple hexstrings can be concatenated with :
If has no 0x prefix, read it as text

Copy link
Collaborator

Choose a reason for hiding this comment

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

Multiple hexstrings can be concatenated with :

I don't think this functionality is supported on this branch. Do we want to add it on this branch or open an issue and handle in a separate PR?

$ seth keccak 0x1234:0x5678
0x30ca65d5da355227c97ff836c9c6719af9d3835fc6bc72bddc50eeecc1bb2b25

$ seth keccak 0x12345678
0x30ca65d5da355227c97ff836c9c6719af9d3835fc6bc72bddc50eeecc1bb2b25

$ cast keccak 0x1234:0x5678
# errors with "Odd number of digits" which I believe means decoding the hex string failed

$ cast keccak 0x12345678
0x30ca65d5da355227c97ff836c9c6719af9d3835fc6bc72bddc50eeecc1bb2b25

Copy link
Member

Choose a reason for hiding this comment

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

Let's do this in a separate PR

@onbjerg onbjerg added the T-bug Type: bug label Feb 4, 2022
cast/src/lib.rs Outdated
Comment on lines 1043 to 1047
let hash: String = match data.as_bytes() {
[b'0', b'x', rest @ ..] => keccak256(hex::decode(rest)?).to_hex(),
_ => keccak256(data).to_hex(),
};

Copy link
Member

Choose a reason for hiding this comment

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

Let's do this in a separate PR

@gakonst gakonst merged commit bea3ddc into foundry-rs:master Feb 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-bug Type: bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

review and fix string vs. bytes handling in cast
5 participants