Conversation
|
Warning Rate limit exceeded@jpthor has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 17 minutes and 47 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (29)
WalkthroughAdds comprehensive DeFi support: planning docs for a canonical bridge router, new EVM ABIs and Solana IDLs, chain registries, metarule DeFi types/constraint parsing and routing with tests, large RESOURCES expansions, and minor formatting tweaks across several chains and engines. Changes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
RESOURCES.md (2)
1-7196: Pipeline failure: RESOURCES.md requires regeneration.The CI reports this file is out of date. Run the generator command to update:
go run cmd/generator/main.go --output RESOURCES.md
27-43: Invalid JSON in example policy rules (trailing commas).The example JSON blocks contain trailing commas before closing braces, which is invalid JSON syntax:
{ "constraints": { "amount": { "type": "fixed", "value": "example_value" }, } // <- trailing comma before this brace }This pattern appears throughout the file (e.g., lines 40-42, 77-79, etc.). If this file is auto-generated, the generator template needs fixing.
🤖 Fix all issues with AI Agents
In @chain/evm/defi_registry.go:
- Around line 12-61: The GMX V2 ExchangeRouter addresses in the
GMXV2ExchangeRouter map are outdated; update the Arbitrum entry in
GMXV2ExchangeRouter to "0x602b805EedddBbD9ddff44A7dcBD46cb07849685" and the
Avalanche entry to "0xFa843af557824Be5127eaCB3c4B5D86EADEB73A1". While editing,
double-check the GMXV2OrderVault map and the CompoundV3CometUSDC and AaveV3Pool
maps against official sources and correct any mismatches you find, making
changes directly to those map entries (GMXV2OrderVault, CompoundV3CometUSDC,
AaveV3Pool) to reflect current deployments.
In @chain/solana/defi_registry.go:
- Around line 8-38: The Kamino lending program ID constant
KaminoLendingProgramID is using the wrong Base58 string; update the value passed
to solana.MustPublicKeyFromBase58 in the KaminoLendingProgramID declaration to
the official mainnet program ID "GzFgdRJXmawPhGeBsyRCDLx4jAKPsvbUqoqitzppkzkW"
so KaminoLendingProgramID is constructed from the correct public key.
In @idl/kamino_lending.json:
- Around line 1-76: The IDL metadata contains the wrong program address; update
metadata.address to the official Kamino Lending program ID. Edit the JSON's
metadata.address field (symbol: metadata.address in idl/kamino_lending.json) and
replace "KLend2g3cP87ber41GYr72yfE9j6eBJYwRqVNMi6mHL" with
"GzFgdRJXmawPhGeBsyRCDLx4jAKPsvbUqoqitzppkzkW"; leave the existing instruction
definitions (deposit, withdraw, borrow, repay) and their account/arg structures
unchanged. Ensure the file remains valid JSON after the replacement.
In @metarule/internal/defi/constraints.go:
- Around line 1-238: The helper constructors Fixed, Any, Min, and Max are
unused—either remove them or mark them as exported/documented for external use
(e.g., add a comment above Fixed/Any/Min/Max explaining intended public API
usage) and update tests to use them if you keep them; also add a brief comment
in ParseLendConstraints or above LendConstraints noting that asset and amount
are required intentionally (stricter validation), and add short field comments
to LPConstraints and PerpsConstraints to match the inline comments style used in
BetConstraints so struct documentation is consistent.
🧹 Nitpick comments (12)
chain/utxo/dash/protocol.go (1)
67-156: Clarify the purpose of creating two parameter maps with identical values.Lines 79-86 create both
params(used for constraint checking) anddisplayParams(returned) with nearly identical content—only the amount differs (as*big.Intvs. string). If both maps are necessary, document the distinction; if not, avoid the duplication. IfdisplayParamsis intended for user-facing output, it should handle unit conversion for duffs explicitly.For example:
- Are both needed, or should only the return value be used?
- Should
displayParams["amount"]apply a duffs-to-DASH conversion (if applicable)?- Why not return a single, well-typed structure or comment the trade-off?
.cursor/plans/defi_recipes_implementation_8fba4444.plan.md (2)
121-121: Markdown table formatting is broken.The table content appears on a single line without proper row separators. This will render incorrectly in most Markdown viewers.
🔎 Proposed fix for table formatting
-**Supported Protocols:**| Protocol | Chain | Type ||----------|-------|------|| Uniswap V3 | EVM (Ethereum, Arbitrum, Base, etc.) | Concentrated liquidity || Raydium CLMM | Solana | Concentrated liquidity || Orca Whirlpools | Solana | Concentrated liquidity || Meteora | Solana | Dynamic pools |**Action mappings (EVM - UniswapV3):** +**Supported Protocols:** + +| Protocol | Chain | Type | +|----------|-------|------| +| Uniswap V3 | EVM (Ethereum, Arbitrum, Base, etc.) | Concentrated liquidity | +| Raydium CLMM | Solana | Concentrated liquidity | +| Orca Whirlpools | Solana | Concentrated liquidity | +| Meteora | Solana | Dynamic pools | + +**Action mappings (EVM - UniswapV3):**
117-118: Minor: Inconsistent indentation in parameter list.
min_amount1has extra leading indentation compared tomin_amount0.🔎 Proposed fix
- min_amount0: uint256 (slippage protection for add/remove) - - min_amount1: uint256 + - min_amount1: uint256metarule/metarule.go (3)
355-515: DeFi EVM handlers: LP/lend/perps/bet wiring looks correct; consider tightening a few constraint defaultsThe new EVM DeFi handlers are wired coherently (chain resolution, registry lookups, address helpers, approve‑then‑action sequencing). A few places are more permissive than necessary for high‑risk operations:
- LP add (
handleEVMLP, Uniswap V3):amount0/amount1are optional ingetLPConstraintsand default toanyConstraint(). If the caller omits them, both theerc20.approveandmintrules allow unbounded amounts. Given the intent to “prepend exact‑amount approve rules”, it would be safer to treatamount0/amount1as required foraction == "add"(and possiblymin_amount*), or at least error when they are left asANYfor this action.- Lend / perps:
getLendConstraintsandgetPerpsConstraintscorrectly enforce the minimum required fields and default the rest toANY. That matches existing meta‑rule style, but for flows that mint approvals (Aave/Compound supply, GMX collateral) you may want to ensureamount/collateral_deltaare not left as unconstrainedANY.- Polymarket bet: the single‑order
fillOrderandcancelOrderrules correctly bindorder.tokenIdandorder.maker(andorder.sidefor buy/sell). The batch variants (fillOrders,cancelOrders) currently expose theordersarrays as fullyANY, which makes the policy strictly more permissive than the single‑order case. If you wantbetrules to stay tightly scoped per market/maker, consider either dropping the batch methods from this meta‑protocol or documenting thatbet.*intentionally allows arbitrary batch fills/cancels.Functionally this is all sound and matches the tests; the above are about tightening policy defaults on sensitive DeFi actions rather than fixing bugs.
Also applies to: 2035-3332
247-365: Solana DeFi handlers: perps side/direction not bound toopen_long/open_shortactionsThe new Solana LP/lend handlers are straightforward and consistent with the added IDLs. For perps:
- EVM perps (
handleEVMPerps) deriveisLong/isBiddirectly fromactionand fix those fields in the GMX/Hyperliquid rules.- Solana perps (
handleSolanaPerps) forjupiter_perpsanddriftleavearg_side,arg_direction, andarg_reduceOnlyasANYfor all actions, soopen_long,open_short, andcloseall share identical rule shapes.That means the meta‑rule cannot enforce “long vs short vs close” semantics on Solana the way it does on EVM; it relies entirely on downstream code to set those fields correctly at execution time.
If you want parity and stronger safety, consider:
- For
jupiter_perps: fixarg_sidebased onaction(e.g.,"long"vs"short"or the numeric convention the program expects).- For
drift: mapactioninto appropriatearg_directionandarg_reduceOnlyconstraints for opens vs closes.This keeps the meta‑protocol’s semantics aligned across chains and reduces the chance of a policy that intends “long‑only” silently allowing shorts or non‑reduce‑only closes.
Also applies to: 2520-2840
3005-3243: Constraint parsers: required vs optional fields are sensible; might strengthen LP amount requirementsThe new constraint structs (
lpConstraints,lendConstraints,perpsConstraints,betConstraints) and their parsers correctly:
- Enforce presence of critical fields (
action,protocol, plusasset/amountwhere needed).- Default all other missing fields to
anyConstraint()to avoid nil dereferences downstream.Only minor tightening worth considering:
- For LP
addactions, treatamount0/amount1as required instead of optional, since they drive both approvals and mint bounds.- Optionally enforce that
assetmust beFIXED(not just non‑nil) in the contexts where you immediately callGetFixedValue()and expect an address (you already do this for several paths, but centralizing the validation in the parser could simplify handlers).Otherwise the parsing layer looks robust and matches how the handlers consume these constraints.
.cursor/plans/canonical_bridge_router_c7ea46e1.plan.md (2)
160-173: Add language to fenced code block to satisfy markdownlintThe file‑structure code block currently uses bare triple backticks, which triggers MD040 (“fenced code blocks should have a language specified”).
Recommend adding an explicit language, e.g.:
Proposed markdown tweak
-``` +```text recipes/sdk/bridge/ ├── types.go # BridgeProvider interface, request/response types ├── router.go # BridgeRouter with priority selection ├── bridge.go # Package docs & convenience functions ├── lifi.go # LiFi bridge provider ├── native.go # Native L2 bridge provider ├── native_arbitrum.go # Arbitrum Gateway implementation ├── native_base.go # Base Standard Bridge implementation ├── native_optimism.go # Optimism Standard Bridge implementation └── router_test.go # Tests -``` +``` </details> --- `4-40`: **Plan status vs code: double‑check `metarule-bridge` “completed” flag** The plan lists `metarule-bridge` (“Add bridge metaProtocol to metarule/metarule.go”) as `status: completed`. In the metarule constants shown in this PR, only `send`, `swap`, `lp`, `lend`, `perps`, and `bet` are present. If `bridge` was added in a different change or later removed, it might be worth adjusting this TODO’s status/comment so the plan reflects the current state and avoids confusion for future work. </blockquote></details> <details> <summary>metarule/defi_test.go (1)</summary><blockquote> `11-1086`: **DeFi meta‑protocol tests provide good coverage of new flows** The new `defi_test.go` suite exercises: - Happy paths for EVM LP (Uniswap V3), lend (Aave, Aave on Arbitrum), perps (GMX, Hyperliquid), Solana LP/lend/perps, and Polymarket bet buy/sell/cancel. - Key safety properties like “approve before action” ordering and correct target contract addresses. - Error cases for missing `action`/`protocol`, unsupported protocols/actions, and Polymarket on the wrong chain. This is a solid baseline. If you want to harden things further later, you could add focused tests for: - Aave/Compound `repay`/`withdraw` branches. - GMX `close` and Hyperliquid `adjust_margin`. - Solana perps edge cases (e.g., verifying side/direction mapping once constrained). But as written, the tests align well with the handlers. </blockquote></details> <details> <summary>chain/solana/defi_registry.go (2)</summary><blockquote> `59-97`: **Silent failure on unknown protocol may mask bugs.** The getter functions return an empty `PublicKey{}` with `nil` error for unknown protocols (lines 69, 83, 95). This makes it difficult for callers to distinguish between "protocol not found" and a successful lookup. Consider returning an error for unknown protocols to enable proper error handling: <details> <summary>🔎 Proposed fix</summary> ```diff func GetLPProgramID(protocol DeFiProtocol) (solana.PublicKey, error) { switch protocol { case ProtocolRaydium: return RaydiumCLMMProgramID, nil case ProtocolOrca: return OrcaWhirlpoolProgramID, nil case ProtocolMeteora: return MeteoraProgramID, nil default: - return solana.PublicKey{}, nil + return solana.PublicKey{}, fmt.Errorf("unknown LP protocol: %s", protocol) } }Apply the same pattern to
GetLendingProgramIDandGetPerpsProgramID.
40-57: Type duplication withmetarule/internal/defi/types.go.
DeFiProtocol(line 41) and its constants duplicateProtocolfrommetarule/internal/defi/types.go(lines 8-24 in that file). This creates maintenance burden and potential for drift.Consider either:
- Importing and using
defi.Protocolhere, or- Creating a shared package for protocol definitions
chain/evm/defi_registry.go (1)
63-183: Inconsistent error message capitalization.Error messages have inconsistent capitalization patterns:
- Line 67:
"UniswapV3 Position Manager"(title case)- Line 76:
"AAVE V3 Pool"(all caps for AAVE)- Line 95:
"compound V3"(lowercase)- Line 122:
"hyperliquid bridge"(lowercase)- Line 153:
"polymarket CTF Exchange"(mixed)Consider standardizing to match protocol branding (e.g., "Compound V3", "Hyperliquid", "Polymarket").
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
.cursor/plans/canonical_bridge_router_c7ea46e1.plan.md.cursor/plans/defi_recipes_implementation_8fba4444.plan.mdREADME.mdRESOURCES.mdchain/evm/abi/aaveV3_pool.jsonchain/evm/abi/compoundV3_comet.jsonchain/evm/abi/gmxV2_exchange_router.jsonchain/evm/abi/hyperliquid_bridge.jsonchain/evm/abi/polymarket_ctf.jsonchain/evm/abi/polymarket_ctf_exchange.jsonchain/evm/abi/uniswapV3_nonfungible_position_manager.jsonchain/evm/defi_registry.gochain/solana/defi_registry.gochain/utxo/dash/chain.gochain/utxo/dash/decode.gochain/utxo/dash/protocol.gochain/xrpl/chain.gochain/xrpl/protocol.goengine/utxo/dogecoin/dogecoin.goengine/utxo/litecoin/litecoin.goidl/drift_protocol.jsonidl/jupiter_perpetuals.jsonidl/kamino_lending.jsonidl/orca_whirlpool.jsonidl/raydium_clmm.jsonmetarule/defi_test.gometarule/internal/defi/constraints.gometarule/internal/defi/types.gometarule/metarule.gometarule/metarule_test.gotypes/transaction.go
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-16T15:18:29.097Z
Learnt from: neavra
Repo: vultisig/recipes PR: 123
File: engine/xrpl/xrpl.go:41-51
Timestamp: 2025-09-16T15:18:29.097Z
Learning: XRPL engine should support multiple functions including "send" and "thorchain_swap" rather than being restricted to just "send".
Applied to files:
README.md
📚 Learning: 2025-07-28T17:33:10.162Z
Learnt from: webpiratt
Repo: vultisig/recipes PR: 50
File: ethereum/ethereum.go:165-166
Timestamp: 2025-07-28T17:33:10.162Z
Learning: The Protocol interface in types/protocol.go includes a ChainID() string method, so calling ChainID() on any Protocol implementation (like NewETH()) is valid and will not cause compilation errors.
Applied to files:
chain/utxo/dash/chain.gometarule/metarule.gochain/solana/defi_registry.go
🧬 Code graph analysis (5)
metarule/defi_test.go (3)
metarule/metarule.go (1)
NewMetaRule(21-23)types/constraint.pb.go (2)
ConstraintType_CONSTRAINT_TYPE_FIXED(29-29)ConstraintType_CONSTRAINT_TYPE_MAX(30-30)sdk/tron/tron.go (1)
Value(67-71)
metarule/metarule_test.go (2)
types/rule.pb.go (1)
TargetType_TARGET_TYPE_ADDRESS(28-28)types/constraint.pb.go (1)
ConstraintType_CONSTRAINT_TYPE_FIXED(29-29)
metarule/internal/defi/types.go (1)
chain/solana/defi_registry.go (8)
ProtocolRaydium(45-45)ProtocolOrca(46-46)ProtocolMeteora(47-47)ProtocolKamino(50-50)ProtocolMarginfi(51-51)ProtocolSolend(52-52)ProtocolJupiterPerps(55-55)ProtocolDrift(56-56)
metarule/metarule.go (3)
types/constraint.pb.go (3)
Constraint(145-165)Constraint(178-178)Constraint(193-195)types/rule.pb.go (2)
Effect_EFFECT_ALLOW(78-78)TargetType_TARGET_TYPE_ADDRESS(28-28)chain/evm/defi_registry.go (3)
GetPolymarketCTFExchange(150-156)GetPolymarketUSDC(177-183)GetPolymarketCTF(168-174)
chain/solana/defi_registry.go (1)
metarule/internal/defi/types.go (8)
ProtocolRaydium(9-9)ProtocolOrca(10-10)ProtocolMeteora(11-11)ProtocolKamino(16-16)ProtocolMarginfi(17-17)ProtocolSolend(18-18)ProtocolJupiterPerps(24-24)ProtocolDrift(25-25)
🪛 GitHub Actions: Check Resources Documentation
RESOURCES.md
[error] 1-1: RESOURCES.md is out of date. Please run 'go run cmd/generator/main.go --output RESOURCES.md' and commit the changes.
🪛 LanguageTool
.cursor/plans/defi_recipes_implementation_8fba4444.plan.md
[grammar] ~242-~242: Ensure spelling is correct
Context: ...V3_nonfungible_position_manager.json- UniswapV3 LP positions -aaveV3_pool.json` - AAV...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.18.1)
.cursor/plans/canonical_bridge_router_c7ea46e1.plan.md
162-162: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (24)
engine/utxo/dogecoin/dogecoin.go (1)
39-43: Formatting improvement approved.Adding a trailing newline at the end of the file aligns with Go conventions and linter standards.
chain/utxo/dash/decode.go (1)
120-124: Formatting improvement approved.Adding a trailing newline at the end of the file aligns with Go conventions.
chain/xrpl/chain.go (1)
189-196: Formatting improvement approved.Adding a trailing newline at the end of the file aligns with Go conventions and linter standards.
chain/utxo/dash/chain.go (1)
96-101: Proper file formatting.The addition of a trailing newline follows Go conventions. No logic changes.
chain/evm/abi/polymarket_ctf.json (1)
1-13: Polymarket CTF ABI integration is correctly implemented.Both ABI files exist and are properly integrated:
polymarket_ctf.jsoncontainssetApprovalForAllpolymarket_ctf_exchange.jsoncontainsfillOrderandfillOrders- DeFi registry defines all required contracts with getter functions
- Metarule handler correctly chains the three-step flow: CTF approval → ERC20 approval → order filling
README.md (1)
117-143: Remove unsupported Lighter protocol and correct perps action names in DeFi meta-protocols table.The documentation lists protocols and actions that don't match the actual implementation:
Lighter (EVM) is not implemented — Although
ProtocolLighteris defined inmetarule/internal/defi/types.go, thehandleEVMPerpsfunction only supports"gmx"and"hyperliquid"protocols. Remove Lighter from the table.Perps actions are incorrect — The README lists
increaseanddecreaseactions, but the implementation only supportsopen_long,open_short,close, andadjust_margin. Update the table to reflect the actual supported actions.Corrected table snippet
| **perps** | `open_long`, `open_short`, `close`, `adjust_margin` | GMX V2, Hyperliquid (EVM), Jupiter Perps, Drift (Solana) |Likely an incorrect or invalid review comment.
idl/raydium_clmm.json (1)
1-96: LGTM!The Raydium CLMM IDL correctly defines the concentrated liquidity operations needed for the LP meta-protocol. The slippage protection parameters are appropriately named (
amount*Maxfor deposits,amount*Minfor withdrawals), and the account structures are comprehensive..cursor/plans/defi_recipes_implementation_8fba4444.plan.md (1)
165-165: Action inconsistency between plan and PR description.The plan shows
adjust_marginas a perps action, but the PR description listsincreaseanddecreaseas separate actions. Ensure alignment between the plan and implementation.chain/evm/abi/aaveV3_pool.json (1)
1-106: LGTM!The AAVE V3 Pool ABI correctly defines the core lending operations (supply, withdraw, borrow, repay) and includes useful additions like permit-based variants and
getUserAccountDatafor health factor checks. This covers all actions needed for thelendmeta-protocol.chain/evm/abi/gmxV2_exchange_router.json (1)
1-83: LGTM!The GMX V2 Exchange Router ABI is well-structured with comprehensive order parameters. The
payablestate mutability correctly reflects GMX's execution fee requirements. Themulticallfunction will be useful for batching approval and order creation operations.chain/evm/abi/hyperliquid_bridge.json (2)
32-57: Verify iforderandcancelfunctions belong in the bridge ABI.The file is named
hyperliquid_bridge.json, but includes trading functions (order,cancel) alongside bridge operations (batchedDepositWithPermit,initiateWithdrawal,finalizeWithdrawal). The PR objectives note that Hyperliquid uses EIP-712 signing on its custom L1 and initial support is via the EVM bridge for deposits/withdrawals only.Consider whether these trading functions are accessible via the EVM bridge or if they should be in a separate ABI for the native Hyperliquid interface.
3-5: Note: uint64 type for USD amounts.The ABI uses
uint64for USD amounts instead of the typicaluint256. This aligns with Hyperliquid's 6-decimal precision model but differs from most EVM DeFi conventions. Ensure downstream handlers account for this precision difference.idl/drift_protocol.json (1)
1-62: Drift Protocol IDL is well-structured and follows Anchor conventions.The program address (dRiftyHA39MWEi3m9aunc5MzRF1JYuBsbn6VPcn33UH) is correct for Drift Protocol v2 on Solana mainnet. The subset of instructions (placePerpOrder, deposit, withdraw) is appropriate for the perps meta-protocol use cases. Account mutability flags are consistent with expected behavior—notably, the
stateaccount is mutable inwithdrawbut immutable indeposit, which is intentional.metarule/metarule.go (2)
441-507: EVM swap: approve‑then‑swap sequencing and router constraints look solidThe updated EVM
swappath now prepends a boundederc20.approverule (spender = 1inch router or THORCHAIN router magic constant, amount =from_amount) before the actual swap rule, and the test adjustment forTestTryFormat_EvmSwapmatches this behavior. This sequencing should work well for clients that need two sequential signatures (approve, then swap), and the router constraint reuses existing magic constants correctly for the ThorChain path.No functional issues spotted here.
3249-3332: Helper mappings and ERC20 approve helper are consistent with usageThe new helpers (
getUniswapV3PositionManager,getAaveV3Pool,getCompoundV3Comet,getGMXV2ExchangeRouter,getHyperliquidBridge,createERC20ApproveRule) are internally consistent:
- Chain → address maps align with the chains actually used in tests.
- Error messages are explicit for unsupported chains, which will surface cleanly through
TryFormat.createERC20ApproveRulecentralizes the approve pattern and is used consistently by LP, lend, perps, and bet flows.No issues here.
idl/orca_whirlpool.json (1)
1-66: Orca Whirlpool IDL matches Solana LP handler usageThe IDL for
orca_whirlpoolcleanly exposesincreaseLiquidity,decreaseLiquidity, andcollectFeeswith account and argument names that align withhandleSolanaLP(arg_liquidityAmount,arg_tokenMaxA/B,arg_tokenMinA/B). Program address also matches the handler’s hardcoded ID.Looks good.
metarule/metarule_test.go (1)
753-879: EVM swap test correctly updated for approve‑then‑swap flowThe
TestTryFormat_EvmSwapchanges accurately reflect the new behavior:
- Asserting the first rule is
ethereum.erc20.approvetargeting USDC with fixedamountandspendermatching the 1inch router.- Asserting the second rule is the 1inch
swapwith the expected router address and parameter set.This gives good coverage of the new two‑tx sequence.
chain/evm/abi/polymarket_ctf_exchange.json (1)
1-116: Polymarket CTF Exchange ABI aligns with bet meta‑protocolThe ABI for
polymarket_ctf_exchangematches howhandleEVMBetconstructs rules:
- Functions and names (
fillOrder,fillOrders,cancelOrder,cancelOrders) are present.- The
Ordertuple exposestokenId,maker,side, andsignaturefields with the names referenced by the metarule.- Batch variants use
ordersandfillAmountsas expected.No issues from the ABI side.
chain/evm/abi/compoundV3_comet.json (1)
1-106: Compound V3 Comet ABI matches lend handler expectationsThe
compoundV3_cometABI exposes:
supply(address asset, uint256 amount)andwithdraw(address asset, uint256 amount)exactly as used inhandleEVMLend.- Additional helpers (balanceOf, borrowBalanceOf, collateralBalanceOf, etc.) that can be constrained later if needed.
Interface and naming are consistent with the new lend rules.
chain/evm/abi/uniswapV3_nonfungible_position_manager.json (1)
1-137: LGTM — Standard Uniswap V3 Position Manager ABI.The ABI correctly defines the core functions for managing liquidity positions:
mint,increaseLiquidity,decreaseLiquidity,collect,burn,positions, andmulticall. The struct types and state mutability annotations align with the official Uniswap V3 interface.metarule/internal/defi/types.go (2)
53-58: Perps action naming may differ from PR objectives.The PR summary mentions perps actions as "open_long, open_short, close, increase, decrease" but the code defines
PerpsActionAdjustMargin(line 57) instead of separateincreaseanddecreaseactions. Verify this consolidated action is intentional.
1-87: LGTM — Clean protocol taxonomy with ecosystem helpers.The type definitions provide a clear classification of DeFi protocols and actions. The
IsEVMProtocolandIsSolanaProtocolhelper functions (lines 70-86) enable clean routing logic.chain/evm/defi_registry.go (1)
81-98: Good market selector pattern for Compound V3.The
GetCompoundV3Cometfunction (lines 81-98) handles case-insensitive market selection cleanly. This is a good pattern for user-facing inputs.idl/jupiter_perpetuals.json (1)
1-130: LGTM — Well-structured Jupiter Perpetuals IDL.The IDL correctly defines the perpetuals instructions with appropriate account metadata and argument types. The program address (line 5) matches the
JupiterPerpsProgramIDin the Solana registry, ensuring consistency.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
metarule/internal/defi/constraints.go (3)
9-60: Consider adding field comments for consistency.The
LPConstraintsstruct andParseLPConstraintsfunction are well-structured. However,BetConstraints(lines 160-167) includes inline field comments while this struct does not. Consider adding brief comments to each field for consistency and better documentation.
62-107: Document stricter validation requirements.
ParseLendConstraintsvalidates four required fields (Action,Protocol,Asset,Amount) compared to only two for LP and Perps parsers. While this stricter validation is appropriate for lending operations, consider adding a brief comment above the struct or in the parser explaining whyAssetandAmountare mandatory for lending. Also, add inline field comments to match the style inBetConstraints(lines 160-167).
109-157: Add field comments for consistency.The
PerpsConstraintsstruct and parser are well-implemented. Consider adding inline field comments similar toBetConstraints(lines 160-167) to maintain consistent documentation across all constraint types.chain/solana/defi_registry.go (1)
59-97: Return explicit errors for unsupported protocols.The getter functions (
GetLPProgramID,GetLendingProgramID,GetPerpsProgramID) currently return an emptyPublicKeywithnilerror for unsupported protocols (lines 69, 83, 95). This could lead to silent failures if callers don't explicitly check whether the returned key is empty. Since the function signatures include an error return, consider returning an explicit error instead.🔎 Proposed fix for explicit error handling
func GetLPProgramID(protocol DeFiProtocol) (solana.PublicKey, error) { switch protocol { case ProtocolRaydium: return RaydiumCLMMProgramID, nil case ProtocolOrca: return OrcaWhirlpoolProgramID, nil case ProtocolMeteora: return MeteoraProgramID, nil default: - return solana.PublicKey{}, nil + return solana.PublicKey{}, fmt.Errorf("unsupported LP protocol: %s", protocol) } }Apply similar changes to
GetLendingProgramIDandGetPerpsProgramID. You'll also need to import"fmt".
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
chain/evm/defi_registry.gochain/solana/defi_registry.goidl/kamino_lending.jsonmetarule/internal/defi/constraints.go
🚧 Files skipped from review as they are similar to previous changes (2)
- idl/kamino_lending.json
- chain/evm/defi_registry.go
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-28T17:33:10.162Z
Learnt from: webpiratt
Repo: vultisig/recipes PR: 50
File: ethereum/ethereum.go:165-166
Timestamp: 2025-07-28T17:33:10.162Z
Learning: The Protocol interface in types/protocol.go includes a ChainID() string method, so calling ChainID() on any Protocol implementation (like NewETH()) is valid and will not cause compilation errors.
Applied to files:
chain/solana/defi_registry.go
🧬 Code graph analysis (2)
chain/solana/defi_registry.go (1)
metarule/internal/defi/types.go (8)
ProtocolRaydium(9-9)ProtocolOrca(10-10)ProtocolMeteora(11-11)ProtocolKamino(16-16)ProtocolMarginfi(17-17)ProtocolSolend(18-18)ProtocolJupiterPerps(24-24)ProtocolDrift(25-25)
metarule/internal/defi/constraints.go (1)
metarule/internal/defi/types.go (1)
Protocol(4-4)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: check-resources
🔇 Additional comments (4)
metarule/internal/defi/constraints.go (1)
159-198: LGTM! Good documentation model.The
BetConstraintsstruct and parser are well-implemented with clear inline field comments. This serves as a good documentation model for the other constraint types.chain/solana/defi_registry.go (3)
7-17: LGTM! LP program IDs verified.The LP protocol program IDs for Raydium CLMM, Orca Whirlpools, and Meteora have been verified as correct per previous review. Using
MustPublicKeyFromBase58is appropriate for these compile-time constants.
19-29: ✅ Kamino program ID corrected!The Kamino Lending program ID has been updated to the correct official mainnet address (
GzFgdRJXmawPhGeBsyRCDLx4jAKPsvbUqoqitzppkzkW), addressing the issue raised in the previous review. Marginfi and Solend program IDs are also verified as correct.
31-38: LGTM! Perps program IDs verified.The perpetuals protocol program IDs for Jupiter Perps and Drift have been verified as correct per previous review.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Add high-level DeFi recipe meta-protocols that transform into low-level ABI/IDL calls: Meta-protocols: - lp: Liquidity pool management (add/remove/collect_fees) - lend: Lending protocol operations (supply/withdraw/borrow/repay) - perps: Perpetual DEX trading (open_long/open_short/close/adjust_margin) - bet: Prediction markets (buy/sell/cancel) - Polymarket on Polygon EVM Protocols: - UniswapV3 (LP) - Ethereum, Arbitrum, Optimism, Base, Polygon, BSC, Avalanche - AAVE V3 (Lending) - Ethereum, Arbitrum, Optimism, Base, Polygon, Avalanche - Compound V3 (Lending) - Ethereum, Arbitrum, Base, Polygon - GMX V2 (Perps) - Arbitrum, Avalanche - Hyperliquid (Perps) - Arbitrum bridge - Polymarket (Betting) - Polygon Solana Protocols (IDLs added): - Raydium CLMM (LP) - Orca Whirlpools (LP) - Kamino (Lending) - Jupiter Perps (Perps) - Drift Protocol (Perps) New files: - chain/evm/defi_registry.go - EVM protocol addresses per chain - chain/solana/defi_registry.go - Solana program IDs - chain/evm/abi/*.json - Protocol ABIs - idl/*.json - Solana IDLs - metarule/internal/defi/ - DeFi constraint types and helpers - metarule/defi_test.go - Meta-protocol tests The bet meta-protocol for Polymarket correctly handles: - CTF.setApprovalForAll for conditional token transfers - ERC20.approve for USDC collateral (buy only) - CTFExchange.fillOrder with proper order.side mapping (0=buy, 1=sell)
- Update GMX V2 ExchangeRouter addresses to latest deployed contracts - Fix Kamino lending program ID to official mainnet address - Remove unused helper functions (Fixed, Any, Min, Max) from constraints.go - Regenerate RESOURCES.md
1 participant
Summary
Adds comprehensive DeFi meta-protocol support to the recipes engine, enabling high-level abstractions for common DeFi operations.
New Meta-Protocols
LP (Liquidity Provisioning)
add,remove,collect_feesLend (Lending/Borrowing)
supply,withdraw,borrow,repayPerps (Perpetuals Trading)
open_long,open_short,close,increase,decreaseBet (Prediction Markets)
buy,sell,cancelKey Features
erc20.approverule with the exact amount constraint, enabling TSS to sign both approval and action in sequencechain/evm/defi_registry.go,chain/solana/defi_registry.go)Files Changed
New Files
chain/evm/defi_registry.go- EVM DeFi protocol addresseschain/solana/defi_registry.go- Solana program IDschain/evm/abi/*.json- Uniswap V3, AAVE, Compound, GMX, Hyperliquid, Polymarket ABIsidl/*.json- Raydium, Orca, Meteora, Kamino, Marginfi, Solend, Jupiter Perps, Drift IDLsmetarule/internal/defi/types.go- DeFi constants and helpersmetarule/internal/defi/constraints.go- Constraint parsing for DeFi rulesmetarule/defi_test.go- Comprehensive test suiteModified Files
metarule/metarule.go- Added lp, lend, perps, bet handlersREADME.md- Documentation for DeFi meta-protocolsTesting
go build ./... go test ./... golangci-lint run ./...All tests pass (24 new DeFi tests + existing tests).
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.