Skip to content

fix: mask signer row operand to low byte - #2778

Open
thedavidmeister wants to merge 8 commits into
mainfrom
2026-06-17-issue-2653-signer-row-mask
Open

fix: mask signer row operand to low byte#2778
thedavidmeister wants to merge 8 commits into
mainfrom
2026-06-17-issue-2653-signer-row-mask

Conversation

@thedavidmeister

@thedavidmeister thedavidmeister commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Summary

  • subParserSigners read the full 16-bit OperandV2 value as the context row index, so signer<256>() silently resolved to the same row as signer<0>() (256 mod 256 = 0 at the EVM word level) but only by accident — larger values could land on entirely wrong rows.
  • Fix: apply & 0xFF to clamp the operand to the intended 0–255 range before passing it to LibSubParse.subParserContext.
  • Regression test: testSubParserContextSignerRowMaskLowByte verifies signer<256>() resolves to row 0.
  • Pre-pins 0.1.13 deploy constants: SubParser address changes (new bytecode), all other contracts (RaindexV6, RouteProcessor, arb contracts) are unchanged from 0.1.12.

REQUIRES redeploy at land — the SubParser bytecode changes, so it must be redeployed to its new CREATE2 address before testProdDeploy* goes green.

Closes #2653

Co-Authored-By: Claude noreply@anthropic.com

Summary by CodeRabbit

  • New Features

    • Added support for Raindex ecosystem version 0.1.13 with pinned deployment addresses and code hashes for core components.
  • Bug Fixes

    • Updated operand interpretation in signer-grid row derivation to use low byte only instead of full value.
  • Tests

    • Added test verifying signer-grid row masking behavior wraps correctly.

`subParserSigners` extracted the full 16-bit operand as the context row,
so operands ≥ 256 silently wrapped at the EVM word boundary instead of
being clamped to the intended 0-255 range.  Apply `& 0xFF` to pin the
row to the low byte and add a regression test (`signer<256>()` resolves
to row 0).  Pre-pins 0.1.13 deploy constants (SubParser address changes,
all other contracts unchanged).

Co-Authored-By: Claude <noreply@anthropic.com>
@thedavidmeister thedavidmeister self-assigned this Jun 17, 2026
@coderabbitai

coderabbitai Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: bbced90e-4ac2-4c02-879f-5a228f6a8c0b

📥 Commits

Reviewing files that changed from the base of the PR and between 175e84a and 8ca5816.

⛔ Files ignored due to path filters (1)
  • src/generated/RaindexV6SubParser.pointers.sol is excluded by !**/generated/**
📒 Files selected for processing (4)
  • crates/test_fixtures/abis/RaindexV6SubParser.json
  • src/lib/LibRaindexSubParser.sol
  • src/lib/deploy/LibRaindexDeploy.sol
  • test/concrete/parser/RaindexV6SubParser.signers.t.sol

📝 Walkthrough

Walkthrough

subParserSigners is updated to mask the OperandV2 operand to its low 8 bits (& 0xFF) before using it as the signer-grid row index, matching the behavior of the sibling subParserSignedContext word. A test verifying that signer<256>() maps to row 0 is added, the compiled SubParser bytecode fixture is refreshed, and twelve new pinned deployment address/codehash constants for the 0.1.13 release are appended to LibRaindexDeploy.

Changes

Signer operand masking fix

Layer / File(s) Summary
Low-byte operand masking in subParserSigners and test
src/lib/LibRaindexSubParser.sol, test/concrete/parser/RaindexV6SubParser.signers.t.sol, crates/test_fixtures/abis/RaindexV6SubParser.json
subParserSigners masks the unwrapped OperandV2 with 0xFF before computing the row index; a new test asserts signer<256>() resolves to the same row as signer<0>(); the compiled bytecode fixture is updated to match the recompiled contract.

0.1.13 deployment constants

Layer / File(s) Summary
Pinned 0.1.13 address and codehash constants
src/lib/deploy/LibRaindexDeploy.sol
Twelve new address and bytes32 constants covering all six RaindexV6 ecosystem contracts at tag 0.1.13 are appended after the existing 0.1.12 entries.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • rainlanguage/raindex#2635: Adds routing tests for subParserSigners row-from-operand behavior that directly anticipates the & 0xFF masking fix introduced in this PR.
  • rainlanguage/raindex#2683: Follows the same pattern of appending pinned address/bytes32 codehash constants to LibRaindexDeploy, adding the 0.1.2 tag entries.

Poem

🐇 A byte too wide, the signer would roam,
Past row 255, far away from home.
One mask of 0xFF, and the number wraps clean,
signer<256>() now rows to row 0, serene.
New constants pinned at 0.1.13 with care —
The rabbit deployed them with flair! 🎉

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title "fix: mask signer row operand to low byte" directly describes the main change: applying & 0xFF masking to the operand in the subParserSigners function, which is the core fix across all modifications.
Linked Issues check ✅ Passed The PR fully addresses #2653 by masking the signer row operand to & 0xFF in LibRaindexSubParser.sol, aligning signers with signed-context behavior, adding a regression test, and pinning deploy constants for the fixed bytecode.
Out of Scope Changes check ✅ Passed All changes are directly related to issue #2653: the operand masking fix, its test, updated ABI fixture, and pre-pinned deploy constants for the new bytecode. No unrelated modifications detected.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 2026-06-17-issue-2653-signer-row-mask

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@thedavidmeister thedavidmeister added the ai:ready AI vetter: passes review, ready for human decision label Jul 6, 2026
@thedavidmeister

thedavidmeister commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

🤖 ai:vetter
Reviewed 0b65ace: ready — adds uint256(OperandV2.unwrap(operand)) & 0xFF mask matching sibling subParserSignedContext; test ve
cost 150 — one-line operand mask plus regression test

@thedavidmeister thedavidmeister added human:design and removed ai:ready AI vetter: passes review, ready for human decision labels Jul 9, 2026
@thedavidmeister

Copy link
Copy Markdown
Contributor Author

Design hold (human). The PR premise is incorrect: LibSubParse.subParserContext bounds-checks row > type(uint8).max and reverts ContextGridOverflow before any truncation, so on current main signer<256>() does not "silently resolve to row 0" — it reverts at parse time. Applying & 0xFF therefore does not make explicit what already happens; it converts a fail-safe revert into a silent mod-256 wrap (signer<256>() → row 0, signer<257>() → row 1, …), letting an out-of-range signers index silently read the wrong signer. The sibling signed-context masks to extract two byte-fields from a 16-bit operand (never overflows by construction), which does not justify range-clamping signers single-field operand from revert to wrap. Parked for a design ruling: keep the fail-safe revert (in which case #2653 is arguably invalid), or accept the silent wrap. Not merged pending that decision.

@thedavidmeister

Copy link
Copy Markdown
Contributor Author

🤖 ai:producer
Producer note: both red checks (rainix-sol/static + rs-static) are MAIN-STATE, not PR-caused — the no-vm.skip gate flags 5 pre-existing test/lib/deploy files on main (e.g. LibRaindexDeployTaggedConstants.t.sol:24); identical red on main HEAD. rainix-sol/test PASSES here. Greens when main resolves the vm.skip gate.

@thedavidmeister

Copy link
Copy Markdown
Contributor Author

🤖 ai:vetter
vet-protocol 4
lens no source read — the ai:design label's provenance decides this routing, not the diff
Reviewed 37988de: needs-work — ROUTING FIX, not a code defect (design doctor, rainlanguage/issue-pr-cron#241): this PR carried ai:design with NO trusted comment raising a design question at its head, which is a state no actor consumes — the human's queue withholds it (there is no claim to present) and every AI actor skips it. It is routed here so it has an owner again. Two moves, both yours: if the PR still needs work, push it — the question was superseded or never recorded, and the push IS the transition. If a genuine design question remains, re-raise it with pr-review-report flag-design <owner/repo> <pr> "<the question>", which puts the row back in the human's queue with a claim behind it. Do not push a no-op commit to clear this.

@thedavidmeister thedavidmeister added ai:needs-work Needs rework — the producer's inbox (vetter verdict or human ruling) and removed ai:design AI vetter: raises a design question labels Aug 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai:needs-work Needs rework — the producer's inbox (vetter verdict or human ruling)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

signers subparser word uses the raw operand as row without & 0xFF masking (unlike signed-context)

1 participant