Use IRaindexV6.TokenSelfTrade from raindex-interface 0.1.3 - #2796
Merged
Conversation
Bump raindex-interface 0.1.1 -> 0.1.3 and drop the local file-scope TokenSelfTrade declaration; the three revert sites now bind to the error declared on IRaindexV6, and tests reference it via the interface. Creation bytecode is byte-identical to main (bytecode_hash=none), so no redeploy is implied. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
|
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 selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
Included review availability: Your plan includes up to 1 review per rolling hour; 0 remain after this review. 📝 WalkthroughWalkthroughThis PR updates ChangesRaindex interface dependency bump
Estimated code review effort: 2 (Simple) | ~15 minutes Merge Risk: ⚪ Minimal · up to This PR centralizes the existing self-trade error declaration without changing deployed creation bytecode or reported runtime behavior. No actionable merge-blocking risk remains beyond normal checks and review. Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/concrete/raindex/RaindexV6.takeOrder.freshMutation.t.sol (1)
331-332: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
IRaindexV6.TokenSelfTrade.selectorinstead of a hardcoded hash.Other self-trade tests in this PR were updated to reference
IRaindexV6.TokenSelfTrade.selectordirectly (per PR objectives), but this one still manually recomputes the selector viakeccak256("TokenSelfTrade()"). Functionally equivalent today, but it's inconsistent with the rest of the cohort and won't automatically track future signature changes to the interface error.♻️ Proposed fix
- // selector for TokenSelfTrade() as declared on IRaindexV6. - vm.expectRevert(bytes4(keccak256("TokenSelfTrade()"))); + vm.expectRevert(IRaindexV6.TokenSelfTrade.selector);🤖 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 `@test/concrete/raindex/RaindexV6.takeOrder.freshMutation.t.sol` around lines 331 - 332, In RaindexV6.takeOrder.freshMutation.t.sol, the TokenSelfTrade revert expectation is still computed manually with keccak256 instead of using the interface-defined selector. Update the self-trade assertion in the relevant test to reference IRaindexV6.TokenSelfTrade.selector directly so it stays consistent with the other tests and automatically follows any future error signature changes.
🤖 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 `@test/concrete/raindex/RaindexV6.takeOrder.freshMutation.t.sol`:
- Around line 331-332: In RaindexV6.takeOrder.freshMutation.t.sol, the
TokenSelfTrade revert expectation is still computed manually with keccak256
instead of using the interface-defined selector. Update the self-trade assertion
in the relevant test to reference IRaindexV6.TokenSelfTrade.selector directly so
it stays consistent with the other tests and automatically follows any future
error signature changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a4729e7c-45a5-4450-8066-3442429a096e
⛔ Files ignored due to path filters (1)
soldeer.lockis excluded by!**/*.lock
📒 Files selected for processing (141)
foundry.tomlremappings.txtsrc/abstract/RaindexV6ArbCommon.solsrc/abstract/RaindexV6ArbOrderTaker.solsrc/abstract/RaindexV6ArbTaskGated.solsrc/abstract/RaindexV6FlashBorrower.solsrc/abstract/RaindexV6FlashLender.solsrc/concrete/raindex/RaindexV6.solsrc/lib/LibOrder.solsrc/lib/LibRaindex.solsrc/lib/LibRaindexArb.soltest/abstract/RaindexV6ArbOrderTaker.badRaindex.t.soltest/abstract/RaindexV6ArbOrderTaker.beforeArbOrder.t.soltest/abstract/RaindexV6ArbOrderTaker.context.t.soltest/abstract/RaindexV6ArbOrderTaker.finalizeDecimals.t.soltest/abstract/RaindexV6ArbOrderTaker.finalizeTaskArg.t.soltest/abstract/RaindexV6ArbOrderTaker.ierc165.t.soltest/abstract/RaindexV6ArbOrderTaker.ioIndexSelection.t.soltest/abstract/RaindexV6ArbOrderTaker.noOrders.t.soltest/abstract/RaindexV6ArbOrderTaker.onTakeOrders2.t.soltest/abstract/RaindexV6ArbOrderTaker.onTakeOrders2Base.t.soltest/abstract/RaindexV6ArbOrderTaker.onTakeOrders2Direct.t.soltest/abstract/RaindexV6ArbOrderTaker.reentrancy.t.soltest/abstract/RaindexV6ArbTaskGated.checkTaskHashPassMatch.t.soltest/abstract/RaindexV6ArbTaskGated.checkTaskHashPassZero.t.soltest/abstract/RaindexV6ArbTaskGated.checkTaskHashWrongTask.t.soltest/abstract/RaindexV6ArbTaskGated.construct.t.soltest/abstract/RaindexV6ArbTaskGated.iTaskHashBoundary.t.soltest/abstract/RaindexV6ArbTaskGated.iTaskHashNonzero.t.soltest/abstract/RaindexV6ArbTaskGated.iTaskHashZero.t.soltest/abstract/RaindexV6FlashBorrower.badLenderApproval.t.soltest/abstract/RaindexV6FlashBorrower.beforeArbOrder.t.soltest/abstract/RaindexV6FlashBorrower.finalizeDecimals.t.soltest/abstract/RaindexV6FlashBorrower.flashLoanAmountLossless.t.soltest/abstract/RaindexV6FlashBorrower.flashLoanAmountValue.t.soltest/abstract/RaindexV6FlashBorrower.flashLoanFailed.t.soltest/abstract/RaindexV6FlashBorrower.inputIoIndexSelection.t.soltest/abstract/RaindexV6FlashBorrower.ioIsInput.t.soltest/abstract/RaindexV6FlashBorrower.mixedDecimals.t.soltest/abstract/RaindexV6FlashBorrower.noOrders.t.soltest/abstract/RaindexV6FlashBorrower.orders0Selection.t.soltest/abstract/RaindexV6FlashBorrower.realTokenTransfers.t.soltest/abstract/RaindexV6FlashBorrower.reentrancy.t.soltest/abstract/RaindexV6FlashLender.griefRecipient.t.soltest/abstract/RaindexV6FlashLender.mockSuccess.t.soltest/abstract/RaindexV6FlashLender.reentrant.t.soltest/abstract/RaindexV6FlashLender.transfers.t.soltest/concrete/arb/GenericPoolRaindexV6ArbOrderTaker.sender.t.soltest/concrete/arb/GenericPoolRaindexV6FlashBorrower.exchangeTokenSelection.t.soltest/concrete/arb/GenericPoolRaindexV6FlashBorrower.sender.t.soltest/concrete/arb/RouteProcessorRaindexV6ArbOrderTaker.lossyRounding.t.soltest/concrete/arb/RouteProcessorRaindexV6ArbOrderTaker.noEthForward.t.soltest/concrete/arb/RouteProcessorRaindexV6ArbOrderTaker.nonStandardDecimals.t.soltest/concrete/arb/RouteProcessorRaindexV6ArbOrderTaker.onTakeOrders2.t.soltest/concrete/arb/RouteProcessorRaindexV6ArbOrderTaker.onTakeOrders2Direct.t.soltest/concrete/arb/RouteProcessorRaindexV6ArbOrderTaker.onTakeOrders2Fuzz.t.soltest/concrete/arb/RouteProcessorRaindexV6ArbOrderTaker.sender.t.soltest/concrete/arb/RouteProcessorRaindexV6ArbOrderTaker.tofuDecimals.t.soltest/concrete/raindex/RaindexV6.addOrder.entask.t.soltest/concrete/raindex/RaindexV6.addOrder.mock.t.soltest/concrete/raindex/RaindexV6.addOrder.nonce.t.soltest/concrete/raindex/RaindexV6.addOrder.owner.t.soltest/concrete/raindex/RaindexV6.addOrder.t.soltest/concrete/raindex/RaindexV6.calculateOrderIOHarness.t.soltest/concrete/raindex/RaindexV6.clear.badStack.t.soltest/concrete/raindex/RaindexV6.clear.calculateStateChange.t.soltest/concrete/raindex/RaindexV6.clear.context.t.soltest/concrete/raindex/RaindexV6.clear.deadOrder.t.soltest/concrete/raindex/RaindexV6.clear.freshDriver.t.soltest/concrete/raindex/RaindexV6.clear.handleIO.revert.t.soltest/concrete/raindex/RaindexV6.clear.mock.t.soltest/concrete/raindex/RaindexV6.clear.sameOwner.t.soltest/concrete/raindex/RaindexV6.clear.sameToken.t.soltest/concrete/raindex/RaindexV6.clear.zeroAmount.t.soltest/concrete/raindex/RaindexV6.deposit.entask.t.soltest/concrete/raindex/RaindexV6.deposit.t.soltest/concrete/raindex/RaindexV6.depositWithdrawHarness.t.soltest/concrete/raindex/RaindexV6.doPost.fresh.t.soltest/concrete/raindex/RaindexV6.entask.t.soltest/concrete/raindex/RaindexV6.multicall.t.soltest/concrete/raindex/RaindexV6.orderMgmt.fresh.t.soltest/concrete/raindex/RaindexV6.quote.sameToken.t.soltest/concrete/raindex/RaindexV6.quote.t.soltest/concrete/raindex/RaindexV6.removeOrder.entask.t.soltest/concrete/raindex/RaindexV6.removeOrder.mock.t.soltest/concrete/raindex/RaindexV6.removeOrder.owner.t.soltest/concrete/raindex/RaindexV6.takeOrder.badStack.t.soltest/concrete/raindex/RaindexV6.takeOrder.batchEvalConsistency.t.soltest/concrete/raindex/RaindexV6.takeOrder.batchStateAdversarial.t.soltest/concrete/raindex/RaindexV6.takeOrder.batchStateBypass.t.soltest/concrete/raindex/RaindexV6.takeOrder.calculationsContext.t.soltest/concrete/raindex/RaindexV6.takeOrder.exceedsMaxRatio.t.soltest/concrete/raindex/RaindexV6.takeOrder.freshMutation.t.soltest/concrete/raindex/RaindexV6.takeOrder.freshVaultZero.t.soltest/concrete/raindex/RaindexV6.takeOrder.handleIO.revert.t.soltest/concrete/raindex/RaindexV6.takeOrder.maximumInput.t.soltest/concrete/raindex/RaindexV6.takeOrder.maximumOutput.t.soltest/concrete/raindex/RaindexV6.takeOrder.minimumIO.t.soltest/concrete/raindex/RaindexV6.takeOrder.minimumIOOutput.t.soltest/concrete/raindex/RaindexV6.takeOrder.noop.t.soltest/concrete/raindex/RaindexV6.takeOrder.precision.t.soltest/concrete/raindex/RaindexV6.takeOrder.sameToken.t.soltest/concrete/raindex/RaindexV6.takeOrder.takenFlag.t.soltest/concrete/raindex/RaindexV6.takeOrder.tokenMismatch.t.soltest/concrete/raindex/RaindexV6.takeOrder.vaultZeroInput.t.soltest/concrete/raindex/RaindexV6.takeOrder.zeroAmount.t.soltest/concrete/raindex/RaindexV6.takeOrder.zeroMaxOOBIndex.t.soltest/concrete/raindex/RaindexV6.vaultBalance.t.soltest/concrete/raindex/RaindexV6.vaultBalanceVault0.t.soltest/concrete/raindex/RaindexV6.withdraw.entask.t.soltest/concrete/raindex/RaindexV6.withdraw.t.soltest/lib/LibOrder.t.soltest/lib/LibRaindex.doPost.t.soltest/lib/LibRaindexArb.finalizeArbTaskContext.t.soltest/util/abstract/IRaindexV6Stub.soltest/util/abstract/MockRaindexBase.soltest/util/abstract/RaindexV6ExternalMockTest.soltest/util/abstract/RaindexV6ExternalRealTest.soltest/util/concrete/Alice.soltest/util/concrete/ChildGatedRaindexV6ArbOrderTaker.soltest/util/concrete/ChildGatedRaindexV6FlashBorrower.soltest/util/concrete/ChildRaindexV6ArbTaskGated.soltest/util/concrete/FlashLendingMockRaindex.soltest/util/concrete/LeftoverFlashLendingMockRaindex.soltest/util/concrete/LibRaindexArbFinalizeHarness.soltest/util/concrete/LibRaindexDoPostHarness.soltest/util/concrete/MaliciousRaindex.soltest/util/concrete/RaindexV6CalculateOrderIOHarness.soltest/util/concrete/RaindexV6ClearStateChangeHarness.soltest/util/concrete/RealisticFlashLendingMockRaindex.soltest/util/concrete/RealisticOrderTakerMockRaindex.soltest/util/concrete/RecordingFlashLendingMockRaindex.soltest/util/concrete/RecordingOrderTaker.soltest/util/concrete/RecordingTaker.soltest/util/concrete/Reenteroor.soltest/util/concrete/ReentrantExchange.soltest/util/concrete/ReentrantMockRaindex.soltest/util/lib/LibTestAddOrder.soltest/util/lib/LibTestArb.soltest/util/lib/LibTestFlashBorrowerArb.soltest/util/lib/LibTestTakeOrder.sol
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
thedavidmeister
added
the
ai:ready
Contributor
Author
|
🤖 ai:vetter |
Contributor
Author
|
🤖 ai:vetter |
Contributor
Author
|
🤖 ai:producer |
thedavidmeister
added
ai:blocked-on
Contributor
Author
|
🤖 ai:producer |
Contributor
|
@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:
|
Contributor
|
Tip For best results, initiate chat on the files or code changes. SIZE=M You are interacting with an AI system. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Companion to rainlanguage/raindex.interface#64 (which declared
TokenSelfTradeonIRaindexV6): an interface declaration is only meaningful if the implementation uses it, so this PR makes raindex consume the interface's declaration instead of keeping a duplicate.raindex-interface0.1.1 → 0.1.3 (foundry.toml + remappings + soldeer.lock + all import paths).error TokenSelfTrade();inRaindexV6.sol— the three revert sites (checkTokenSelfTradeused byquote2/takeOrders4, and the inlineclear3guard) now bind to the inheritedIRaindexV6.TokenSelfTrade.IRaindexV6.TokenSelfTrade.selector; one deadTokenSelfTradeimport removed fromRaindexV6.quote.t.sol.This matches the existing convention: the other 9 interface-declared errors (
NoOrders,ZeroVaultId, …) are already used from the interface without local duplicates.Verification
35d21e65…before and after;bytecode_hash = "none",cbor_metadata = false) — same error signature ⇒ same selector ⇒ no redeploy implied, prod pins unaffected.quote/clear/takeOrdersameToken, calculateOrderIOHarness, freshDriver, freshMutation, deadOrder).script/build.shregenerated; any artifact diffs are staged in the commit.Refs #2671 (item 6).
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests