fix: include counterparty (zero address) as 3rd context row in addOrder/removeOrder - #2779
fix: include counterparty (zero address) as 3rd context row in addOrder/removeOrder#2779thedavidmeister wants to merge 9 commits into
Conversation
…er/removeOrder (#2619) The entask post-context for addOrder4 and removeOrder3 only had two rows (orderHash, msg.sender). The spec requires a third row for the counterparty address; because these are order-management operations (not trades) the counterparty is always the zero address. Add `bytes32(uint256(uint160(address(0))))` as that third row and test it. Pre-pins 0.1.7 deploy constants (RaindexV6 address changes, all other contracts unchanged). REQUIRES redeploy at land — testProdDeploy* and testIsStartBlock* / testNetworksJson* will be red until the new RaindexV6 is deployed and build-start-blocks.sh is run. Co-Authored-By: Claude <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (10)
📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (2)
Included review availability: Your plan includes up to 1 review per rolling hour; 0 remain after this review. 📝 WalkthroughWalkthroughRaindexV6 post-tasks now receive a zero-address order counterparty context. Add-order and remove-order tests validate this context. Release metadata, ABI bytecode, deployment pointers, and runtime-code checks are updated for version 0.1.14. ChangesRaindex V6 context and release metadata
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The change adds the required zero-address context row for order-management operations and includes passing coverage for both add and remove paths; no actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
Regenerate arb-contract pointer files and update 0.1.7 pre-pin constants. RaindexV6 bytecode change causes arb contracts to deploy at new addresses since they embed the new RaindexV6 address in their deployment parameters.
…ct-per-file static check Merge origin/main (0.1.12) into branch. Resolves conflicts in: - src/concrete/raindex/RaindexV6.sol: keep PR's 3-row context (orderHash, msg.sender, address(0)) for addOrder4/removeOrder3 post-hooks - src/lib/deploy/LibRaindexDeploy.sol: take main's published 0.1.7-0.1.12 constants, add 0.1.13 pre-pin for this PR's new RaindexV6 bytecode - src/generated/*.pointers.sol + crates/test_fixtures/abis/: keep HEAD (new bytecode from this PR) - foundry.toml: bump version to 0.1.13 The merge brings in main's one-contract-per-file fix (PR #2731/#2749) which resolves the rainix-sol/static check failures. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
test/concrete/raindex/RaindexV6.addOrder.entask.t.sol (1)
205-225: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMirror this regression on
removeOrder3.This PR fixes both order-management flows, but the new regression only exercises
addOrder4. A matching remove-order entask test would keep the second call site from drifting back to the old 2-row context.src/concrete/raindex/RaindexV6.sol (1)
379-386: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDeduplicate the 3-row post-context builder.
These two entrypoints now hardcode the same matrix while the shared helper still represents the old 2-row shape. Keeping the context contract in two places makes this easy to drift again.
♻️ Proposed refactor
- LibRaindex.doPost( - LibBytes32Matrix.matrixFrom( - LibBytes32Array.arrayFrom( - orderHash, bytes32(uint256(uint160(msg.sender))), bytes32(uint256(uint160(address(0)))) - ) - ), - post - ); + _doOrderPost(orderHash, address(0), post); ... - LibRaindex.doPost( - LibBytes32Matrix.matrixFrom( - LibBytes32Array.arrayFrom( - orderHash, bytes32(uint256(uint160(msg.sender))), bytes32(uint256(uint160(address(0)))) - ) - ), - post - ); + _doOrderPost(orderHash, address(0), post);function _doOrderPost(bytes32 orderHash, address counterparty, TaskV2[] calldata post) internal { LibRaindex.doPost( LibBytes32Matrix.matrixFrom( LibBytes32Array.arrayFrom( orderHash, bytes32(uint256(uint160(msg.sender))), bytes32(uint256(uint160(counterparty))) ) ), post ); }Also applies to: 407-414
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/concrete/raindex/RaindexV6.sol` around lines 379 - 386, The post-context matrix is duplicated in the RaindexV6 entrypoints, while the shared helper still reflects the old shape. Update RaindexV6 to centralize this 3-row context construction in a single internal helper such as _doOrderPost, and have both entrypoints call it with the appropriate counterparty so LibRaindex.doPost and the LibBytes32Matrix/LibBytes32Array assembly stay in one place.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/concrete/raindex/RaindexV6.sol`:
- Around line 379-386: The post-context matrix is duplicated in the RaindexV6
entrypoints, while the shared helper still reflects the old shape. Update
RaindexV6 to centralize this 3-row context construction in a single internal
helper such as _doOrderPost, and have both entrypoints call it with the
appropriate counterparty so LibRaindex.doPost and the
LibBytes32Matrix/LibBytes32Array assembly stay in one place.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c693ebc8-03eb-4b9a-9e16-b4039564bdc2
⛔ Files ignored due to path filters (4)
src/generated/GenericPoolRaindexV6ArbOrderTaker.pointers.solis excluded by!**/generated/**src/generated/GenericPoolRaindexV6FlashBorrower.pointers.solis excluded by!**/generated/**src/generated/RaindexV6.pointers.solis excluded by!**/generated/**src/generated/RouteProcessorRaindexV6ArbOrderTaker.pointers.solis excluded by!**/generated/**
📒 Files selected for processing (5)
crates/test_fixtures/abis/RaindexV6.jsonfoundry.tomlsrc/concrete/raindex/RaindexV6.solsrc/lib/deploy/LibRaindexDeploy.soltest/concrete/raindex/RaindexV6.addOrder.entask.t.sol
…rom merged source The merge-update (step 3d) pulled main's source back into the PR but did not regenerate src/generated/RaindexV6.pointers.sol. The generated file still carried bytecode compiled from the PR's original source (which had the OrderNoSources / OrderNoHandleIO source-count check removed to match an older rainlang version). After the merge the source has those checks again, so the etched-bytecode mock tests now expect them to fire — but the stale RUNTIME_CODE etched at RAINDEX_DEPLOYED_ADDRESS lacked them, causing testAddOrderWithoutCalculationsReverts (and friends) to fail with "next call did not revert as expected". Fix: re-run script/build-meta.sh → forge script BuildPointers.sol → forge build → script/build.sh → forge fmt to regenerate the pointers, update the 0_1_13 pinned constants in LibRaindexDeploy to the new address and codehash, and refresh crates/test_fixtures/abis/RaindexV6.json. testProdDeploy* / testIsStartBlock* / testNetworksJson* remain red (WAITING-DEPLOY): the new RaindexV6 must be deployed before they pass. Co-Authored-By: Claude <noreply@anthropic.com>
|
Producer note: HAND-OFF (three prior 3b attempts, latest 2026-07-04, checks still red). Three distinct causes: (1) copy-artifacts + 9 codehash/runtime-code test fails: the 2026-07-04 regen covered RaindexV6 pointers only — the arb-contract pointers (GenericPool*/RouteProcessor*) are still stale; fix is a FULL ./build.sh run with ALL regenerated artifacts staged. (2) 5 testProdDeploy* + 3 isStartBlock + networks.json/subgraph.yaml: WAITING-DEPLOY (bytecode-changing PR, routine pre-merge deploy). (3) rainix-sol static + rs-static: rainix's new no-ignored-tests gate bans vm.skip, and the flagged vm.skip lines exist on raindex MAIN too (test/lib/deploy/*.t.sol) — an org-level collision that will red main's own next static run; needs an org decision (allow-list or rework those skips), not a branch fix. Per one-attempt-per-check back-off I am not pushing again. |
|
🤖 ai:producer |
thedavidmeister
added
the
ai:ready
|
🤖 ai:vetter |
…-counterparty-context
thedavidmeister
added
ai:needs-work
|
🤖 ai:vetter |
…-counterparty-context # Conflicts: # src/lib/deploy/LibRaindexDeploy.sol
…rk [3b-attempt] Merge current main (frozen snapshot-dir redesign) into the branch. The one conflict, src/lib/deploy/LibRaindexDeploy.sol, resolves to main's deletion of the flat per-tag constants: the branch's added 0.1.13 flat block is the old machinery that main moved into src/generated/<tag>/ snapshots. Vetter rework: the 3-element context row (order hash, caller, zero-address counterparty) is built once in _doOrderPost; both addOrder4 and removeOrder3 call the helper again instead of inlining the construction, and the helper's doc comment describes the 3-row context. testRemoveOrderContext now adds the order then removes it so the post tasks actually run in the remove path, and pins the third row with an order-counterparty() == address(0) assertion alongside the raindex/order-hash/order-owner rows. foundry.toml [package].version bumps 0.1.13 -> 0.1.14: the bytecode change would otherwise mutate the frozen 0_1_13 snapshot (Build.sol rejects that). Build.sol freezes the new src/generated/0_1_14/ snapshot and the tagged-constants test gains the matching 0_1_14 block. Full canonical regen with all artifacts staged: script/build-meta.sh, forge script script/Build.sol run to its fixed point (the arb contracts embed RAINDEX_DEPLOYED_ADDRESS at compile time, so the first pass leaves their pointers one iteration stale; a second pass converges and a third pass is a verified no-op), forge build, script/build.sh, forge fmt. testProdDeploy*, testIsStartBlock*, testNetworksJsonAddresses and testSubgraphYamlAddress stay red until the new deterministic addresses are deployed and the start-block bookkeeping is refreshed (deploy-before-merge flow). Co-Authored-By: Claude <noreply@anthropic.com>
|
🤖 ai:vetter |
thedavidmeister
added
ai:blocked-deploy
and removed
ai:needs-work
|
🤖 ai:producer |
|
👤 human |
|
Rework note @90b1e5ecaf2a47cc41ee75d4ae7cd737382a18da: rework the PR to fit the split release lifecycle — deploys never gate merges (the deploy-before-merge choreography is superseded); remove or restructure anything in the PR that waits on a deploy; where deploy constants/pins are involved, follow the *.deploy repo convention (audited code only; version ↔ snapshot ↔ pins internally consistent; tag-release lifecycle). Whatever states follow the rework (including a typed blocked-on the repo's migration if one is genuinely needed) are the producer's ordinary transitions. Executes the 2026-08-06 ruling: rainlanguage/issue-pr-cron#221 |
thedavidmeister
added
ai:needs-work
…-counterparty-context
|
🤖 ai:producer |
1 participant
Summary
addOrder4andremoveOrder3populate a post-hook entask context with two rows:orderHashandmsg.sender. The spec requires a third row for the counterparty address; for order-management operations the counterparty is alwaysaddress(0).bytes32(uint256(uint160(address(0))))as the third row in thematrixFromcall for both functions.testAddOrderCounterpartyIsZeroAddressverifies the new third context slot is the zero address.REQUIRES redeploy at land — RaindexV6 bytecode changes, so
testProdDeploy*will be red until it is deployed.testIsStartBlock*/testNetworksJson*/testSubgraphYamlAddresswill also be red untilscript/build-start-blocks.shis run against the new deployment address.Closes #2619
Co-Authored-By: Claude noreply@anthropic.com
Summary by CodeRabbit
New Features
Bug Fixes
Chores
QA
testAddOrderCounterpartyIsZeroAddress(RaindexV6.addOrder.entask.t.sol),testRemoveOrderContext(RaindexV6.removeOrder.entask.t.sol) — each fails on base. Verified in this checkout's own CI toolchain (nix develop github:rainlanguage/rainix/53e96a7d#sol-shell): restoring the pre-change 2-rowarrayFromin_doOrderPostand re-runningforge script script/Build.solmakes add-order fail withpanic: array out-of-bounds access (0x32)— the exact Audit (Protofire M02): order-counterparty() exposed in add/remove post-actions but its context row is never populated (reverts) #2619 symptom — andtestRemoveOrderContextfail; with the third row restored both pass (19/19 across the two entask suites).src/concrete/raindex/RaindexV6.sol_doOrderPost→ dropbytes32(uint256(uint160(address(0))))from thearrayFromcall → killed by both tests above. IMPORTANT: the mutation only bites AFTER the pointer regen —RaindexV6ExternalRealTestetchesRUNTIME_CODEout ofsrc/generated/RaindexV6.pointers.solviaLibEtchRaindex, so a source-only mutation SURVIVES and is not evidence in this repo. Separately,LibRaindexDeployTaggedConstants.t.solpasses 88/88 including the new 0.1.14 pins, and a full regen (script/build-meta.sh→forge script script/Build.sol→forge build→forge fmt) leaves ZERO working-tree drift.order-counterpartyrow, and an add/remove has no counterparty, so the expected value isaddress(0). Asserted through the rainlang wordorder-counterparty()inside the task, never recomputed from_doOrderPostitself.addOrder4andremoveOrder3; covered both, via the single shared_doOrderPosthelper plus a discriminating test on each side. Closes Audit (Protofire M02): order-counterparty() exposed in add/remove post-actions but its context row is never populated (reverts) #2619.[package].version0.1.13 → 0.1.14 bump and the frozensrc/generated/0_1_14/snapshot are not a pre-deploy pin:script/Build.sol'sfreezeSnapshotREVERTS on any bytecode change made without a version bump, so they are mechanically required by main's own build, and they are internally consistent (version ↔ snapshot ↔ pins, proven by the 88 tagged-constant tests and the zero-drift regen). Residual, and NOT fixable from this branch:src/lib/deploy/LibRaindexDeploy.solstill derives the prod deploy constants from the moving flatsrc/generated/*.pointers.sol, sotestProdDeploy*/testIsStartBlock*/testNetworksJson*/testSubgraphYamlAddress(onrainix-sol / test, which is NOT a required check) stay red until 0.1.14 is deployed. Repointing them at the last released snapshot is a repo-wide migration —LibEtchRaindexand every*ExternalRealTestread the same flat pointers — and is out of scope here.