Skip to content

test(StdJson): cover readBytes at parseRaw's inferred-type lengths - #900

Open
developer3516 wants to merge 1 commit into
foundry-rs:masterfrom
developer3516:test/stdjson-readbytes-regression
Open

test(StdJson): cover readBytes at parseRaw's inferred-type lengths#900
developer3516 wants to merge 1 commit into
foundry-rs:masterfrom
developer3516:test/stdjson-readbytes-regression

Conversation

@developer3516

Copy link
Copy Markdown

Motivation

Closes #592.

The reported bug — readBytes returning wrong data or reverting on 20-byte
values — does not reproduce on master. It was real on forge-std v1.2.0, where
readBytes was abi.decode(vm.parseJson(json, key), (bytes)). It now calls
vm.parseJsonBytes directly, which sidesteps the cause.

The cause itself is still there, one layer down. parseRaw infers a value's
type instead of taking a declared one:

hex string length parseRaw output inferred as
1, 19, 21, 31, 33 bytes 96 bytes dynamic bytes
exactly 20 bytes 32 bytes address
exactly 32 bytes 32 bytes bytes32

So abi.decode(json.parseRaw(key), (bytes)) still reverts at exactly 20 and 32
bytes. readBytes avoids it, but nothing in the suite says so — readBytes has
no test coverage at all — and nothing in the docs warns about it.

Solution

  • test_ReadBytesAtInferredTypeLengths asserts readBytes round-trips 19, 20,
    21, 31, 32 and 33 byte values, bracketing both inference boundaries. If
    readBytes is ever routed back through parseRaw, this fails.
  • A @dev note on parseRaw documents the inference, so the next person
    decoding its output as bytes knows why it reverts.

Testing

forge test: 207 passing, 0 failing. forge fmt --check clean. Build matrix
(stable, --via-ir, solc 0.8.13 and 0.8.35, each with and without IR) all green.

`readBytes` has no test coverage at all, and the length at which it once
broke is still a special case in the layer beneath it.

`parseRaw` infers a value's type rather than taking a declared one: a hex
string of exactly 20 bytes encodes as an `address` and one of exactly 32
bytes as a `bytes32`, while every other length encodes as dynamic `bytes`.
That inference is what broke `readBytes` in v1.2.0, where it decoded
`parseRaw` output as `bytes` instead of calling `vm.parseJsonBytes`.

Reading through the dedicated cheatcode is correct today, so pin it: assert
`readBytes` round-trips 19, 20, 21, 31, 32 and 33 byte values, bracketing
both inference boundaries. Also document the inference on `parseRaw`, since
reaching it through that function still reverts and nothing said so.

Closes foundry-rs#592
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

Successfully merging this pull request may close these issues.

bug(StdJson): readBytes cannot handle 20-byte values

1 participant