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

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

Open
dcposch opened this issue Jul 31, 2024 · 3 comments
Open

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

dcposch opened this issue Jul 31, 2024 · 3 comments

Comments

@dcposch
Copy link

dcposch commented Jul 31, 2024

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
Copy link
Collaborator

mds1 commented Jul 31, 2024

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 dcposch changed the title bug: stdJson readBytes cannot handle 40-byte values bug: stdJson readBytes cannot handle 20-byte values Jul 31, 2024
@dcposch
Copy link
Author

dcposch commented Jul 31, 2024

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: foundry-rs/foundry#5808

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

@klkvr
Copy link
Member

klkvr commented Aug 2, 2024

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?

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

No branches or pull requests

3 participants