foundry icon indicating copy to clipboard operation
foundry copied to clipboard

review and fix string vs. bytes handling in `cast`

Open mds1 opened this issue 4 years ago • 5 comments

Component

Cast

Have you ensured that all of these are up to date?

  • [X] Foundry
  • [X] Foundryup

What version of Foundry are you on?

cast 0.1.0 (fe2dbfe 2022-02-04T00:24:58.188894+00:00)

What command(s) is the bug in?

cast keccak and possibly others

Operating System

macOS (amd)

Describe the bug

Right now, cast keccak does not handle strings vs. bytes correctly, which @SusheendharVijay first noticed when implementing cast index as part of https://github.com/gakonst/foundry/issues/29.

The correct way to handle input to cast keccak <data> is:

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

Instead, we currently treat all input as text and convert it to bytes, which is incorrect. seth and ethers.js use the behavior above

This makes me concerned that there may be other commands where we don't distinguish between text vs. bytes inputs correctly, so the scope of this issue is:

  1. Review how that's handled for the relevant cast commands, using seth as the source of truth
  2. For ones that are incorrect, fix the input handing
  3. Whether correct or incorrect, ensure we have doctests for both text (no 0x prefix) and bytes (0x prefix) on all relevant cast commands

For example, if we had a keccak doctest of assert_eq!(Cast::keccak("0x1234")?, "0x56570de287d73cd1cb6092bb8fdee6173974955fdef345ae579ee9f475ea7432");, we would have noticed this error sooner

mds1 avatar Feb 04 '22 15:02 mds1

@gakonst mind re-opening this one? we fixed cast keccak but IMO should also review other methods that should accept both text or bytes

mds1 avatar Feb 04 '22 19:02 mds1

Close-able? @mds1

lucas-manuel avatar May 18 '23 14:05 lucas-manuel

@tynes do you know offhand if this can be closed? I haven't been using cast much recently for strings/bytes so am not certain, but feel like you may be using it a lot 😅

The issue is kinda vague in that it says to review all relevant commands since some might be incorrect, so hard to know if this should be closed unless someone checks each

mds1 avatar May 18 '23 16:05 mds1

@mds1 bumping this.

Also had issues with generating init code hash via chisel and cast from uniswap pair contract creation code. It always returned an incorrect value against https://emn178.github.io/online-tools/keccak_256.html.

What I was trying in chisel: bytes32 initCode = keccak256(".....") or cast keccak .... but it reads bytecode as string and returns incorrect hash.

I think we need to be able to specify input format somehow

0xSyntellect avatar Apr 22 '24 11:04 0xSyntellect

@0xSyntellect Can you provide concrete steps to reproduce?

mds1 avatar May 02 '24 22:05 mds1