foundry-rs / forge-std

Forge Standard Library is a collection of helpful contracts for use with forge and foundry. It leverages forge's cheatcodes to make writing tests easier and faster, while improving the UX of cheatcodes. For more in-depth usage examples checkout the tests.
Apache License 2.0
819 stars 319 forks source link

bug: stdJson readBytes cannot handle 20-byte values #592

Open dcposch opened 1 month ago

dcposch commented 1 month ago

Summary

stdJson appears to be broken in latest forge-std. Reading 20-byte values with readBytes(json, path) returns incorrect results or reverts.

Versions

forge-std: v1.2.0

forge --version
forge 0.2.0 (26a7559 2024-07-31T00:19:23.655582000Z)

Minimal reproduction

pragma solidity ^0.8.21;

import {Test} from "forge-std/Test.sol";
import {stdJson} from "forge-std/StdJson.sol";

using stdJson for string;

contract JsonTest is Test {
    function testJson() public {
        string memory data = '{"data":"0x1234"}';
        bytes memory hello = data.readBytes(".data");
        assertEq(hello, hex"1234");

        // 19 bytes = works
        data = '{"data":"0x00000000000000000000000000000000000000"}';
        hello = data.readBytes(".data");
        assertEq(hello, hex"00000000000000000000000000000000000000");

        // 21 bytes = works
        data = '{"data":"0x000000000000000000000000000000000000000000"}';
        hello = data.readBytes(".data");
        assertEq(hello, hex"000000000000000000000000000000000000000000");

        // 20 bytes = returns without error, WRONG DATA
        data = '{"data":"0x0000000000000000000000000000000000000000"}';
        hello = data.readBytes(".data");
        assertEq(hello, hex"");

        // 20 bytes = REVERTS
        data = '{"data":"0x4bf5122f344554c53bde2ebb8cd2b7e3d1600ad6"}';
        data.readBytes(".data");
    }
}
mds1 commented 1 month ago

Do you know if this was always the case, or was there a regression in foundry or forge-std? The reverting values are actually 20 bytes long (40 hex chars), so my guess is that it's getting parsed as an address

cc @mattsse to assign someone to look into this :)

dcposch commented 1 month ago

Regression. The test suite where we ran into this issue passed on an earlier version of forge-std

20 bytes

Yeah exactly, typo; fixed.

& yes: I think something internal is automatically parsing 20-byte hex as an address. If true, it's very similar to the the last bug we found in Foundry JSON handling: https://github.com/foundry-rs/foundry/issues/5808

Maybe a root cause fix: no automatic type inference. Users of stdJson should always explicitly specify types + formats.

klkvr commented 1 month ago

Looks like this is only happening on older forge-std versions

forge-std 1.2.0 does abi.decode(vm.parseJson(json, key), (bytes)); instead of vm.parseJsonBytes(json, key);, so automatic coercion to address might cause errors during abi decoding. Not sure why it may have regressed, I think parseJson always did that coercion?