test: stackToFlow malformed-stack revert paths (#328)#478
test: stackToFlow malformed-stack revert paths (#328)#478thedavidmeister wants to merge 1 commit into
Conversation
Cover the two remaining uncovered stackToFlow malformed-stack sub-cases for issue #328: a non-sentinel-aligned tuple count (reverts MissingSentinel) and a sentinel planted inside a tuple field (silently shifts and corrupts the parsed sections). Closes #328 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 8 minutes and 20 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ 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: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ 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 |
What
Completes test coverage for issue #328 (
LibFlow.stackToFlowmalformed-stack reverts). Prior work already covered the empty / one-sentinel / two-sentinel paths viatestStackToFlowRevertsOnEmptyStack,testStackToFlowRevertsOnOneSentinelOnly,testStackToFlowRevertsOnTwoSentinelsOnlyintest/src/concrete/Flow.preview.t.sol. This PR adds the two remaining uncovered sub-cases.TEST-ONLY: no
src/changes.Already covered (verified, not duplicated)
testStackToFlowRevertsOn*tests.Added
testStackToFlowRevertsOnMalformedTupleCount— a stack whose top section is not a multiple of the ERC20 tuple size (4).consumeSentinelTuplesstrides4 * 0x20bytes from the top, so the misaligned section makes the ERC20 pass bind to the ERC721 sentinel, the ERC721 pass bind to the ERC1155 sentinel, and the ERC1155 pass find no sentinel left. Asserts the exactMissingSentinel(RAIN_FLOW_SENTINEL)revert.testStackToFlowSentinelInsideTupleCorrupts— aRAIN_FLOW_SENTINELplanted in a tuple field (where the ERC20amountwould be). It is matched as the ERC20 boundary, so the genuine ERC20 sentinel is consumed by the ERC721 pass and the genuine ERC20 tuple is misread as an ERC721 tuple whosetokenis the truncated sentinel. This path does NOT revert; the test pins the full corruptedFlowTransferV1(erc20.length=0, erc721.length=1 with the planted-sentinel token + shifted fields, erc1155.length=0).Mutation matrix
Each new test was mutation-validated against the tuple-count / sentinel-position handling in
src/lib/LibFlow.sol(mutations reverted after;src/is clean in this PR).LibFlow.stackToFlowRevertsOnMalformedTupleCountSentinelInsideTupleCorrupts4→34→5MissingSentinel)Each test is killed by a distinct arity mutation, confirming both are discriminating.
Build / test
forge build: clean.forge test --match-contract FlowPreviewTest: 13 passed, 0 failed (11 pre-existing + 2 new).Pre-existing reds on
main(NOT introduced here)rainix-sol-static(slither) is red onmain; PR fix: remove redundant statement flagged by slither (unblocks rainix-sol-static) #476 fixes it (unmerged).forge testonmainalso has one pre-existing fuzz red,testFlowBasicValidateMultipleSignedContextsintest/src/concrete/Flow.signedContext.t.sol("next call did not revert as expected", deterministic under the fixed0xdeadbeefseed). Confirmed to fail on a clean772e568bcheckout with this branch's change stashed, so it is unrelated to and untouched by these test additions. No issue currently tracks it.Closes #328
🤖 Generated with Claude Code