Calculated-context panic, signer row mask, take/clear guard ordering (#2667, #2653, #2671)#2692
Calculated-context panic, signer row mask, take/clear guard ordering (#2667, #2653, #2671)#2692thedavidmeister wants to merge 4 commits into
Conversation
Four localized contract-correctness / error-quality fixes, each mutation-validated against the pre-fix bytecode: - #2667: seed the calculations context column before the calculate eval so calculated-max-output() / calculated-io-ratio() read 0 during calculate instead of reverting Panic(0x32) on an out-of-bounds read. - #2653: mask the signer row operand to a byte, matching signed-context. - #2671 (1): check ZeroMaximumIO before dereferencing an order IO index in takeOrders4, so a zero max with an out-of-range index reverts the typed error rather than a bounds panic. - #2671 (2): check NegativeBounty before any vault settlement in clear3, so a spread reverts the explicit error rather than an incidental ERC20 revert. Redeploy cascade: regenerated pointers + derived artifacts, pinned the _0_1_7 deploy-constant suite. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 30 minutes and 14 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR introduces the 0.1.7 release for Raindex with three core behavioral improvements to ChangesRaindex 0.1.7 Release: Behavioral Fixes and Deployment
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
this is not correct IO is valid as zero consider the case of claims, the sender of the claim needs to be able to accept 0 input for the outgoing claim, the receiver of the claim needs to be able to send 0 and receive the claim there needs to be comprehensive test coverage around this case at least |
RaindexV6 moved to 0x13b8f830...; update RAINDEX_START_BLOCK_* constants, subgraph/networks.json (address + startBlock per network), and subgraph/subgraph.yaml to the new deployment so the indexing-bookkeeping fork tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A stray clone of the interface repo was swept into the prior commit by git add -A; remove the gitlink (bookkeeping files are unaffected). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Single-line two _0_1_7 arb address constants to satisfy forge fmt --check (they fit within the line limit). Formatting only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Split into 4 independently-reviewable PRs per request — one issue + one mutation-validated fix each, all rebased on current main (post-#2701): #2735 calc-context-panic (#2667), #2734 signer-row-mask (#2653), #2736 zeromax-OOB-ordering (#2671 item 1), #2732 negbounty-ordering (#2671 item 2). Closing this bundle in favour of those so each can be reviewed individually. |
Contract-correctness + error-quality batch (#2667, #2653, #2671)
Four localized in-repo fixes to
RaindexV6/LibRaindexSubParser, eachmutation-validated against the pre-fix bytecode (every test fails with the old
opaque revert and passes with the fix). Because
RaindexV6bytecode moves, thisPR also carries the redeploy cascade: regenerated pointers/artifacts, the
pre-pinned
_0_1_7deploy-constant suite, and refreshed start-block bookkeeping.Fixes
subparser calculated-max-output()/calculated-io-ratio() revert Panic(0x32) in the calculate entrypoint instead of returning 0 per NatSpec #2667 —
calculated-max-output()/calculated-io-ratio()panic during calculate.calculateOrderIOnever seeded the calculations context column before thecalculate eval (it is populated only afterward, for handle IO), so reading
those words during calculate hit a Solidity out-of-bounds bounds-check and
reverted
Panic(0x32)instead of returning 0 per their NatSpec. The column isnow seeded with a zero-filled array of the right length before the eval; the
real values still overwrite it afterward.
Test:
RaindexV6.takeOrder.calculationsContext.t.sol.signers subparser word uses the raw operand as row without & 0xFF masking (unlike signed-context) #2653 —
signer<n>()row operand unmasked.subParserSignerspassed theraw operand as the row while the sibling
signed-contextword masks it to abyte. An operand > 255 therefore overflowed the context grid
(
ContextGridOverflow). The row is now masked with& 0xFF, matchingsigned-context. Test:signers.t.sol::testSubParserContextSignerRowLowByte.Error-quality + NatSpec-accuracy bundle (OOB->Panic 0x32, clear revert ordering, stale clear/LibOpContext/quote2 NatSpec) #2671 item 1 —
takeOrders4zero-max with an out-of-range IO index. TheZeroMaximumIOguard ran after the order'svalidInputs[inputIOIndex]dereference, so a zero max with an out-of-range index reverted
Panic(0x32)instead of the typed error. The guard now runs before the dereference.
Test:
RaindexV6.takeOrder.zeroMaxOOBIndex.t.sol.Error-quality + NatSpec-accuracy bundle (OOB->Panic 0x32, clear revert ordering, stale clear/LibOpContext/quote2 NatSpec) #2671 item 2 —
clear3negative bounty pre-empted by a vault settlementrevert. The
NegativeBountyguard ran after the vault settlement, so aspread with a vault-0 input and zero ambient balance reverted
ERC20InsufficientBalancefrom the vault-0 push before the explicit guard. Theguard now runs before any settlement. Test:
RaindexV6.takeOrder.vaultZeroInput.t.sol::testClearNegativeBountyVaultZeroRevertsNegativeBounty.Not in this PR (flagged for follow-up)
DivisionByZero(-1,0)→ a semantic error)and items 4–6 (NatSpec / interface-doc accuracy) — the error-type and
interface-doc changes are a separate decision.
OrderNoSources/OrderNoHandleIOdead selectors) — theselectors and NatSpec live in the published
raindex-interfacepackage; thein-repo alternative is a behavior change against existing passing tests.
Cascade
Regenerated pointers + derived artifacts, pre-pinned the
_0_1_7constant suite(
testAllPublishedSoldeerTagsHaveAFullConstantSuite), and refreshedRAINDEX_START_BLOCK_*/networks.json/subgraph.yamlafter the redeploy.Summary by CodeRabbit
Bug Fixes
New Features