Repository navigation
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the
 |
`abi.calldata.toString` produced output that does not match the reference
implementation in genvm (runners/genlayer-py-std, calldata/__init__.py::to_str).
Three defects:
- Bytes and addresses were serialised with `toString(16)` and no zero padding,
so `0x0a` became `a`. This is lossy: `Uint8Array([0x01, 0x02])` and
`Uint8Array([0x12])` both rendered as `b#12`, and a 20-byte address could
render as fewer than 40 hex characters.
- Map entries had no separator at all, producing `{"a":1"b":2}`.
- Arrays emitted a trailing comma, producing `[1,2,3,]`.
The function is public API and also feeds the `readable` field of
`decodeTransaction` / `simplifyTransactionReceipt` via
`calldataToUserFriendlyJson`, so the malformed output is user visible.
Adds tests mirroring the reference test suite
(runners/genlayer-py-std/tests/test_calldata_to_str.py).
Note: `string.ts` does not satisfy prettier on `v1` either; left untouched to
keep the diff reviewable.
698f3bf to
e66d339
Compare
|
Retargeted from v1 to v2-dev, the active dev branch per support/ci/ACTIVE_DEV_BRANCH; string.ts is unchanged between the two, so the bug is still present there. Rebased onto v2-dev with no conflicts. vitest --typecheck: 212 passed, no type errors. With the fix reverted, the 6 new toString tests fail. |
Fixes #210
What
Fixes three defects in
abi.calldata.toString()(src/abi/calldata/string.ts):Adds
tests/calldata-to-string.test.ts, mirroring the reference test suite atrunners/genlayer-py-std/tests/test_calldata_to_str.pyin the genvm repo.Why
The output did not match the reference implementation (
runners/genlayer-py-std/src/genlayer/calldata/__init__.py,to_str):new Uint8Array([0x01, 0x02])b#12b#01020x01addr#11111111111111111111{x: true, y: null}{"x":true"y":null}{"x":true,"y":null}[1, 2, 3][1,2,3,][1,2,3]The missing hex padding is lossy rather than cosmetic:
Uint8Array([0x01, 0x02])andUint8Array([0x12])both rendered asb#12, so distinct values collapsed to the same string. Addresses rendered shorter than 40 characters whenever a byte was below0x10.This is public API and also feeds the
readablefield ofdecodeTransaction/simplifyTransactionReceiptviacalldataToUserFriendlyJson, so the malformed output was user visible.Testing done
v1before the fix and pass after.npx vitest run --typecheckgives 82 passed, no type errors. Baseline before the change was 76 passed, so no regressions.npx eslint src/abi/calldata/string.ts tests/calldata-to-string.test.tsis clean.Decisions made
src/abi/calldata/string.tsdoes not satisfyprettier --checkonv1today, before this change. I left the formatting untouched so the diff stays reviewable. Happy to add a separate formatting commit if you'd prefer.v1per CONTRIBUTING, since this is a bug fix. The same defects are present onv2,v2-devandmain; let me know if you want a companion PR.Checks
Reviewing tips
The diff is 15 lines in
string.ts. The comma changes follow the samefirstflag pattern the reference uses. The twopadStart(2, "0")calls are the lossy part worth checking first.User facing release notes
abi.calldata.toString()now produces output matching the GenVM reference format. Byte and address values are zero padded, map entries are comma separated, and arrays no longer include a trailing comma. This also affects thereadablefield returned bydecodeTransactionandsimplifyTransactionReceipt.