fix: count initial collateral in CoW supply cap checks - #865
Conversation
Check the combined deposit and quoted swap output before CoW preparation, using the collateral asset decimals.
|
🚅 Deployed to the euler-lite-pr-865 environment in euler-lite(dev,PR previews)
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: euler-xyz/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Essentials Run ID: 📒 Files selected for processing (2)
Included review availability: 0 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour. 📝 WalkthroughWalkthrough
ChangesCoW Swap multiply submission and cap validation
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to No current merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Use the captured deposit amount for approval and Supply rows, with regressions for edits during sub-account and allowance reads.
LeonardEulerXYZ
left a comment
There was a problem hiding this comment.
Review — cap arithmetic and preparation consistency
Reviewed exact head 7aa1826bda1dccfd89925942e48db76df8081c91 against development.
Approve the cap/deposit fix. No PR-introduced blocker found. This is not confirmation that every displayed amount is frozen.
Confirmed
- The finite-cap preflight now compares
totalAssets + initialDeposit + quote.amountOutwithsupplyCap, using bigint underlying-asset units. The form rejects CoW when margin and long vault addresses differ and excludes savings collateral, so adding both deposits against the same vault is appropriate. - With 900 supplied and a cap of 1,000, a 60 deposit plus 60 output is rejected before sub-account preparation. Exact-cap and below-cap cases pass; 6/18-decimal rejection and existing 0/maxUint256 unlimited handling are covered.
- The deposit string and raw amount are captured before the first await and reused for execution collateralAmount, approval amount/display, and Supply review row. Both pending sub-account and allowance cases pass.
- Inspected the installed Euler SDK 3.1.0 open-position planner: execution consumes captured collateralAmount and swapQuote. No dependency/config/API change in this PR.
Existing follow-up: Swap output still reads live state
At composables/borrow/useMultiplyCowSwap.ts:257, toAssetInfo.amount still reads options.multiplyLongAmount.value after the lookups. Supply/multiplier controls remain editable during preparation, and requesting a replacement quote clears/replaces this value. The original quote remains in executeParams, but the Swap row can display blank or the replacement quote's output.
I reproduced this with local throwaway tests at both deferred lookup stages: captured output 20; live output reset to blank; executeParams still contains the original quote, while the Swap row displays blank. The same tests fail against the base implementation, so this is a pre-existing follow-up rather than a regression blocking this narrow fix. To satisfy an all-amounts-frozen guarantee, derive the row from the captured quote (or capture its display amount before awaiting). The committed tests keep multiplyLongAmount constant, so they do not discriminate this case.
Validation
Node 24.21.0 / npm 11.10.0, isolated managed worktree; anchor and PR tracked source left unchanged.
vitest run tests/composables/useMultiplyCowSwap.test.ts: 8 passed.npm run test:run: 2,168 passed, 1 skipped (221 test files passed, 1 skipped).npm run typecheck: passed.- Targeted ESLint on both changed files: passed.
npm run build: passed.git diff origin/development...HEAD --check: passed.- Additional Swap-row probes: 2 expected failures on head and 2 on base; scratch probes removed from the worktree.
Browser/wallet/fork settlement smoke: not run. Fixtures are synthetic composable inputs, not a funded live position. Cap validation is snapshot-based and cannot reserve settlement-time capacity.
Keep the Swap row aligned with the execution quote when live form output clears during asynchronous preparation.
LeonardEulerXYZ
left a comment
There was a problem hiding this comment.
Re-reviewed a2a5c7d0ecfff455b701c0fcd593dac762e71ad9: approved. The previous Swap-row follow-up is resolved.
swapOutputAmount is now formatted from the captured quote using long-asset decimals before the first await. Clearing/replacing live form output during sub-account or allowance preparation cannot replace the displayed Swap output. Deposit execution, approval display and Supply row remain captured, and both deposits remain included in finite-cap arithmetic.
Reviewed the incremental fix and full two-file base-to-head diff. The committed tests now exercise live output clearing at both await stages. Independently reran the original throwaway probes that failed on the previous head: all 10 tests pass, including both former failures. Scratch tests removed; worktree clean.
Node 24.21.0 / npm 11.10.0 validation:
- Focused committed suite: 8 passed.
- Original independent regression fixture: 10 passed.
- Full repository suite: 2,168 passed, 1 skipped.
- Typecheck, targeted ESLint, production build, diff whitespace check: passed.
No new blockers found. Browser/wallet/fork smoke not repeated; this is code and regression-test validation. Snapshot cap checking does not reserve settlement-time capacity.
Issue
CoW multiply uses the same vault for the initial collateral deposit and the long side of the swap. Checking only
totalAssets + quote.amountOutagainst its supply cap omits the initial deposit.With 100 units of remaining capacity, an initial deposit of 60 and quoted swap output of 60 pass the output-only check despite requiring 120 units. This flow skips transaction simulation, so the missing check allows an order to proceed toward approvals and signing even though the combined deposits exceed the checked capacity.
Capturing the deposit before asynchronous preparation also requires using that captured value in review. If the user changes 60 to 1 during sub-account or allowance lookup, showing the live input would describe a different deposit from the execution parameters.
Resolution
Preflight compares
totalAssets + initialDeposit + quotedSwapOutputwith the supply cap. The initial deposit uses the collateral asset's actual decimals and bigint arithmetic. Exact-cap amounts are accepted, and existing unlimited-cap handling is preserved.One captured deposit value supplies the cap check, execution parameters, approval amount/display, and Supply review row. The Swap output row is formatted directly from the captured quote using the long asset decimals. Edits during preparation therefore cannot replace the displayed deposit or swap output with live form values.
Validation
Summary by CodeRabbit
Bug Fixes
Tests