feat(cct-sdk): Add lock-release liquidity + rebalancer evm ops - #413
feat(cct-sdk): Add lock-release liquidity + rebalancer evm ops#413apedrob wants to merge 8 commits into
Conversation
|
You must have Developer access to commit code to Chainlink Labs on Vercel. If you contact an administrator and receive Developer access, commit again to see your changes. Learn more: https://vercel.com/docs/accounts/team-members-and-roles/access-roles#team-level-roles |
CI Test Report❌ 1 of 2714 tests did not pass (pass 2707, fail 1, cancelled 0, skipped 6) in 6m 37s Failed suites: Failed testsSummaryCoverage report |
Overview✅ APPROVE — 0 blockers, 0 correctness/architecture majors. Exceptionally careful: floor-match version dispatch with explicit What it adds: Complete EVM LockRelease liquidity surface:
Architecture:
Live Sepolia Verification:
[COMMENT 1/6] 🔴 Medium (DX) —
|
[COMMENT 2/6] 🔴 Medium (Doc, Borderline Major) — Stale correctness rationale on
|
[COMMENT 3/6] 🟡 Low (DX) — Approve→provide cross-link is one-directionalProblem: Impact: A developer reading Fix: Add a backward In facade method /**
* @remarks
* ...
* The deposit is a `transferFrom` on the rebalancer, so the tokens must be approved to the pool first.
* Use {@link generateUnsignedApproveToken} / {@link approveToken} to set the allowance.
*
* @see {@link setRebalancer}
* @see {@link generateUnsignedApproveToken}
*/In op file /**
* @remarks The deposit is a `transferFrom` on the rebalancer, so the tokens must be **approved to
* the pool** first with {@link ApproveToken} / its facade method before calling this op.
*/Location: |
[COMMENT 4/6] 🟡 Low (Scope/Doc) — "v1.5.0–v1.6.1" prose overstates coverage for one Siloed edgeProblem: A real Result: Context: This is a pre-existing table gap that this PR makes newly reachable (by adding liquidity ops that would target it). Not a regression — an intentional design decision being made explicit. Fix — either option: Option A: Add a one-line comment in export enum TokenPoolVersion {
V1_5_0,
V1_5_1,
// Note: v1.6.0 omitted — supported only for Siloed pools, not in this generic ops scope
V1_6_1,
V2_0_0,
}Option B: Narrow the ops' version prose: /**
* @remarks Targets LockRelease v1.5.0–v1.6.1 (generic pools only; Siloed 1.6.0 is out of scope).
*/Recommended: Option A (document the table design decision). Location: |
[COMMENT 5/6] 🟡 Low (Cross-family Consistency) — Error-class divergence with SolanaProblem: EVM throws different error classes than Solana for the same conditions:
Impact: A caller writing one error-handler across families can't catch "sender is not rebalancer" or "pool won't accept it" with a single try/catch. EVM's choice (blame the bad Recommendation: Converge Solana onto EVM's choice (treat bad sender/pool-config as For this PR: No change needed. Raise with the Location (reference):
Flag to: |
[COMMENT 6/6] 🟡 Nits — @throws, idiom, example imports, error messaging(a) Missing
|
692e751 to
56d6fee
Compare
|
What
EVMTokenManager:provideLiquidity,withdrawLiquidity,transferLiquiditysetRebalancer,getRebalancerapproveToken: ERC-20approve, without it the liquidity flow cannot be completed through the SDK alone; also it matches the solana op of the same name1.5.0to1.6.1Why
Testing
LockReleaseTokenPool 1.6.1+Token:approveTokenprovideLiquiditywithdrawLiquiditysetRebalancerfor new pooltransferLiquiditybetween poolsNotes
I suggest reviewing commit by commit to make things easier. I kept everything in the same PR since these ops are tightly coupled and needed for the full flow to work