feat(cheatcodes): Add `vm.ffiUint`, `vm.ffiString` and `vm.ffiBytes` cheatcodes to address ambiguity in `vm.ffi`

[Tudmotu]

Component

Forge

Describe the feature you would like

Today vm.ffi automatically tries to decode output as hex.

This can cause some very sneaky bugs, such as with commands that output decimal numbers:

• If the number is even-length, it will be interpreted as hex bytes
• If it's odd-length, it will be converted into ASCII

Personally I've encountered this issue many times and while sometimes you can work around this, you have to be aware it's happening which is very difficult to figure out.

The change I propose is to add an overload of vm.ffi that accepts a bool argument indicating whether forge should try to auto-decode into hex bytes.

I've made the change on my local fork and made sure it works for my use case.

If this is a desired feature, I will happily open a PR implementing it.

The gist of the change:

-fn ffi(state: &Cheatcodes, args: &[String]) -> Result {
+fn ffi(state: &Cheatcodes, args: &[String], autohex: bool) -> Result {
     if args.is_empty() || args[0].is_empty() {
         bail!("Can't execute empty command");
     }
@@ -91,13 +91,22 @@ fn ffi(state: &Cheatcodes, args: &[String]) -> Result {

     let output = String::from_utf8(output.stdout)?;
     let trimmed = output.trim();
-    if let Ok(hex) = hex::decode(trimmed) {
-        Ok(abi::encode(&[Token::Bytes(hex)]).into())
-    } else {
+    if autohex {
+        if let Ok(hex) = hex::decode(trimmed) {
+            Ok(abi::encode(&[Token::Bytes(hex)]).into())
+        } else {
+            Ok(trimmed.encode().into())
+        }
+    }
+    else {
         Ok(trimmed.encode().into())
     }
 }

+fn ffi_autohex(state: &Cheatcodes, args: &[String]) -> Result {
+    ffi(state, args, true)
+}
+

[mattsse]

wdyt @mds1 ?

[mds1]

This can cause some very sneaky bugs, such as with commands that output decimal numbers:

• If the number is even-length, it will be interpreted as hex bytes
• If it's odd-length, it will be converted into ASCII

Can you clarify what you mean here? Would appreciate a code snippet I can use to reproduce. It's been a while since I've used ffi so I'm a bit rusty on its behavior.

I'm also unclear on what is returned back to solidity when you'd have autohex=false. All FFI cheats currently return bytes, with autohex=false what is the bytes representation of an ffi command that prints 1.23?

[Tudmotu]

@mds1 consider the following command:

$ python -c 'import random; print(random.randomint(0, 200))'

This will print a random number between 0 and 200. It can be 1 character (0-9), 2 characters (10-99) or 3 characters (100-199).

If it returns a 2-character number, e.g. 42, ffi would decode it as a hex value, returning hex"42" as a result. If it returns a 1/3-character number, e.g. 123, ffi would fail to decode it as a hex value, and will return hex"313233", which is ASCII for "123". The decoding fails because a hex number has to have an even number of nibbles.

Now if you try to string(...) it, you will get two very different results:

1. string(hex"42") == "B"
2. string(hex"313233") == "123"

Here is a simple example test:

pragma solidity ^0.8.13;

import { Test, console2 } from 'forge-std/Test.sol';

contract FfiTest is Test {
    function test_ffi () public {
        string[] memory args = new string[](2);
        args[0] = 'echo';
        args[1] = '42';
        bytes memory res1 = vm.ffi(args);
        args[1] = '123';
        bytes memory res2 = vm.ffi(args);

        console2.log(string(res1));
        console2.log(string(res2));
    }
}

The results:

The autohex flag I added simply skips the decoding attempt altogether and returns an ASCII string (encoded as bytes).

Edit: to make it clear why this is problematic, consider that if you want to use the return value from the python -c command as uint, you'd try to call vm.parseUint(string(vm.ffi(...))). But this would fail randomly (e.g. "B" will cause a revert).

[mds1]

This makes sense and is a great example, thanks.

A brief counterargument here is that, given the documented behavior of vm.ffi, your original command should actually have been wrapped in cast --to-hex. Such as:

cast --to-hex $(python -c 'import random; print(random.randint(0, 200))')

So your proposal here is:

• When autohex is true, which is the default, maintain existing behavior of "By default the ffi cheatcode assumes the output of the command is a hex encoded value (e.g. a hex string of an ABI encoded value). If hex decoding fails, it will return the output as UTF8 bytes that you can cast to a string."
• When autohex is false, return the output as UTF8 bytes that you can cast to a string

Is that correct?

My hesitation here is that autohex is too specific to this use case. I think if we are going to try improving the ffi UX, a better way might be to tell forge the expected output type to remove ambiguity altogther

• vm.ffiUint is what you'd use because you're expecting a single uint to be returned
• vm.ffiString would be used if you're expecting a string
• vm.ffiBytes would match the current behavior, but would NOT return output as UTF8 bytes if hex decoding fails
• etc. for all solidity types

This would be analogous to how JSON parsing works: vm.parseJson tries to infer the type where as vm.parseJsonUint, vm.parseJsonString, etc. all try decoding as the specified type and are stricter and safer

[Tudmotu]

I think your suggestion makes sense and would indeed be better :slightly_smiling_face: