Repository navigation
Signed-context row operand reverts past 255 instead of wrapping - #2930
Conversation
A row past 255 now reverts ContextGridOverflow like a signer index past 255, instead of wrapping onto another row. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 46 minutes. View limit details
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Parser behavior and validation src/lib/LibRaindexSubParser.sol, crates/test_fixtures/abis/RaindexV6SubParser.json, test/concrete/parser/RaindexV6SubParser.routingTokenDecimalsAndMasks.t.sol, test/concrete/parser/RaindexV6SubParser.signers.t.sol |
The signed-context parser uses the full operand-derived row instead of masking it to one byte. Tests expect ContextGridOverflow for row 256 in signed-context and signer parsing. The ABI fixture bytecode object changed; ABI entries remain unchanged. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix · Severity of issue fixed: Low
Possibly related PRs
- rainlanguage/raindex.deploy#2921: Removes the same signed-context row mask and adds overflow checks for oversized rows.
- rainlanguage/raindex.deploy#2926: Changes signed-context column layout and updates its overflow test.
- rainlanguage/raindex.deploy#2917: Changes post-task context data used by
signerandsigned-contextwords.
Merge Risk: 
c3791
-
No actionable issue remains from this review; the change is mergeable after normal checks.
Pre-merge checks: 
-
Check name Status Explanation Resolution Linked Issues check Issue #2653is supported:subParserSignedContextnow computesrowwithoperand >> 8, keeps the column-byte mask, and adds a wide-rowContextGridOverflow(8, 256)test. The signer overflow test …Provide reviewable code and test evidence at the reviewed head for #2619and#2647. The evidence must show that post-action context words are gated, withheld, or populated for each affected entrypoint, including the silent-alias and out-of-…-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: signed-context row operands above 255 now revert instead of wrapping. Out of Scope Changes check The source change removes one redundant signed-context row mask. The added tests cover wide signed-context rows and wide signer indices. The regenerated bytecode fixture supports the source change. No… Docstring Coverage No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (4 skipped: 4 … Full details: Out of Scope Changes check
-
Explanation
The source change removes one redundant signed-context row mask. The added tests cover wide signed-context rows and wide signer indices. The regenerated bytecode fixture supports the source change. No unrelated change is shown.
Full details: Docstring Coverage
-
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (4 skipped: 4 unsupported.)
-
Full details: Linked Issues check
-
Explanation
Issue
#2653is supported:subParserSignedContextnow computesrowwithoperand >> 8, keeps the column-byte mask, and adds a wide-rowContextGridOverflow(8, 256)test. The signer overflow test also checksContextGridOverflow(7, 256). The available summary does not establish the implementation or tests for the#2619and#2647post-action context requirements. It only states that PRs 1–3 addressed those issues. A related pull request reference is not code evidence.Resolution
Provide reviewable code and test evidence at the reviewed head for
#2619and#2647. The evidence must show that post-action context words are gated, withheld, or populated for each affected entrypoint, including the silent-alias and out-of-bounds cases.
-
Finishing Touches: 💡 2
-
⚔️ Resolve merge conflicts: 💡
-
- Resolve merge conflict in branch
context-signed-row-mask
- Resolve merge conflict in branch
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
Comment @coderabbitai help to get the list of available commands.
Signed-context overflow tests derive their expected column from CONTEXT_SIGNED_CONTEXT_START_COLUMN / CONTEXT_SIGNED_CONTEXT_SIGNERS_COLUMN. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@coderabbitai assess this PR size classification for the totality of the PR with the following criterias and report it in your comment: S/M/L PR Classification Guidelines:This guide helps classify merged pull requests by effort and complexity rather than just line count. The goal is to assess the difficulty and scope of changes after they have been completed. Small (S)Characteristics:
Review Effort: Would have taken 5-10 minutes Examples:
Medium (M)Characteristics:
Review Effort: Would have taken 15-30 minutes Examples:
Large (L)Characteristics:
Review Effort: Would have taken 45+ minutes Examples:
Additional Factors to ConsiderWhen deciding between sizes, also consider:
Notes:
|
PR 4 of 4, based on
context-column-order(#2928). Split from #2921 (one context column per PR). Last PR of the stack: closes the three issues the stack advances.Stack: PR1 deposit column (#2926) -> PR2 withdraw column (#2927) -> PR3 order column (#2928) -> PR4 signed-context row mask (this).
Problem
subParserSignedContextcomputed the row as(operand >> 8) & 0xFF(#2653). The operand handler (handleOperandDoublePerByteNoDefault) already rejects values above 255 withOperandOverflow, so the mask is redundant; and a wide row reaching the parser would wrap onto another row instead of reverting like a wide signer index (signer<256>, unmasked).Fix
subParserSignedContext: row =operand >> 8(drop the& 0xFF); the column-byte mask is kept; signers stay unmasked.testSignedContextWideRowReverts(ContextGridOverflow(8, 256));testSubParserContextSignerUnhappyIndexOverflowpinssigner<256>->ContextGridOverflow(7, 256).Why
A wide row now reverts in
LibSubParse.subParserContextexactly like a wide signer index, instead of wrapping onto a different row: a loud failure, never a silent wrong read.Evidence
git diff e464be1d2e9815fd3ce814374c9459b39c87b165 HEAD -- src/lib src/concrete test(Context words read their true value per entrypoint or revert (M02) #2921 head) is exactly: main's takeOrders4 skips an order whose calculate fails #2923 changes (RaindexV6.soltake/calculate try-catch skip, its new tests and harness, identical togit diff f971796f..9571c502for those paths), plus the semantic-conflict fix inRaindexV6.contextWordEntrypoints.t.sol: since takeOrders4 skips an order whose calculate fails #2923takeOrders4skips a reverting calculate, so calculate-io negatives go throughquote2(which bubbles the revert) and take positives assert the order was taken (otherwise Context words read their true value per entrypoint or revert (M02) #2921's calculateensurechecks would pass vacuously).forge test: on c379154: 752 passed / 3 failed (755); the 3 are env/network (testIsStartBlockFlare,testSuitesLiveOnEverySupportedNetwork,testSupportedNetworkChainIdsAreBound, noFLARE_RPC_URL).QA
testSignedContextWideRowReverts,testSubParserContextSignerUnhappyIndexOverflow. The first fails on base (PR3): the masked row wraps 256 -> 0 and the parser succeeds.& 0xFF->testSignedContextWideRowReverts; row>> 7/>> 16->testRouteSignedContextColumnMaskAndRowShift,testSignedContextWideRowReverts,testSubParserContextSignedContextHappy1/3; drop column& 0xFF-> same four; mask signer row& 0xFF->testSubParserContextSignerUnhappyIndexOverflow. Harness and per-mutant logs: /home/gildlab/artifacts/raindex-2921-split-2026-10-26-mutation/pr4/v2/.ContextGridOverflow(column, row)with column = signed-context start (8) / signers (7) and row 256 computed from the operand by hand, independent of the parser.ContextGridOverflow. Audit (Protofire M02): order-counterparty() exposed in add/remove post-actions but its context row is never populated (reverts) #2619 and Post-action context: subparser words silently alias other fields or revert OOB (generalizes Protofire M02 #2619) #2647 are completed by PR1-PR3 (every word x all 7 entrypoints).Migration
signed-context<c r>with r > 255 now fails parse instead of reading row r & 0xFF.context<5 i>/<6+i j>-><7 i>/<8+i j>; depositcontext<1 n>-><5 n>; withdrawcontext<1 n>-><6 n>; clearercontext<0 0>-><1 3>;ContextV2order column gains a 4th row).Closes #2619
Closes #2647
Closes #2653
🤖 Generated with Claude Code
Summary by CodeRabbit