fix: apply minimal dLEND directional rounding patch#23
fix: apply minimal dLEND directional rounding patch#23Git-on-my-level wants to merge 3 commits intomainfrom
Conversation
|
Claude finished @Git-on-my-level's task —— View job Code Review AnalysisI've completed a comprehensive review of this dLEND directional rounding fix PR. Here's my analysis: Todo List:
Overall Assessment: ✅ LGTM with Minor SuggestionsThis is a well-scoped, security-focused fix that implements minimal directional rounding changes to prevent supply-withdraw and borrow-repay extraction loops. The implementation follows Aave v3.5 patterns with appropriate conservatism. Code Quality & Best Practices: ⭐⭐⭐⭐⭐Strengths:
Minor suggestions:
Security Analysis: 🔒 Strong✅ Prevents extraction loops:
✅ Directional consistency:
✅ No storage layout changes: Upgrade-safe implementation
Performance Analysis: ⚡ Minimal Impact✅ Gas efficiency maintained:
✅ Computational complexity unchanged:
Test Coverage: 🧪 Adequate for Scope✅ Good edge case coverage:
✅ Comprehensive test scenarios:
Suggestion for future:
Potential Issues:
|
|
Claude finished @Git-on-my-level's task —— View job Code Review AnalysisI've completed a comprehensive review of this dLEND directional rounding fix PR. Here's my analysis: Todo List:
Overall Assessment: ✅ LGTM with Minor SuggestionsThis is a well-scoped, security-focused fix that implements minimal directional rounding changes to prevent supply-withdraw and borrow-repay extraction loops. The implementation follows Aave v3.5 patterns with appropriate conservatism. Code Quality & Best Practices: ⭐⭐⭐⭐⭐Strengths:
Minor suggestions:
Security Analysis: 🔒 Strong✅ Prevents extraction loops:
✅ Directional consistency:
✅ No storage layout changes: Upgrade-safe implementation
Performance Analysis: ⚡ Minimal Impact✅ Gas efficiency maintained:
✅ Computational complexity unchanged:
Test Coverage: 🧪 Adequate for Scope✅ Good edge case coverage:
✅ Comprehensive test scenarios:
Suggestion for future:
Potential Issues:
|
|
Claude finished @Git-on-my-level's task —— View job Code Review AnalysisI've completed a comprehensive review of this dLEND directional rounding fix PR. Here's my analysis: Todo List:
Overall Assessment: ✅ LGTM with Minor SuggestionsThis is a well-scoped, security-focused fix that implements minimal directional rounding changes to prevent supply-withdraw and borrow-repay extraction loops. The implementation follows Aave v3.5 patterns with appropriate conservatism. Code Quality & Best Practices: ⭐⭐⭐⭐⭐Strengths:
Minor suggestions:
Security Analysis: 🔒 Strong✅ Prevents extraction loops:
✅ Directional consistency:
✅ No storage layout changes: Upgrade-safe implementation
Performance Analysis: ⚡ Minimal Impact✅ Gas efficiency maintained:
✅ Computational complexity unchanged:
Test Coverage: 🧪 Adequate for Scope✅ Good edge case coverage:
✅ Comprehensive test scenarios:
Suggestion for future:
Potential Issues:
|
Draft PR: dLEND minimal directional rounding fix for pre-v3.5 extraction loops
Summary
This PR applies the smallest patch I am comfortable shipping from Aave v3.5 / PR 133 to close the supply-withdraw and borrow-repay rounding extraction class on Fraxtal dLEND.
It intentionally limits scope to the tokenization path and the withdraw-side balance calculation:
rayMul/rayDivhelpersScaledBalanceTokenBaseSupplyLogic.executeWithdraw()using floor semantics for the user balanceIt does not pull in the wider base-currency, liquidation, treasury, flashloan, or borrow-cap/account-data rounding refactors from upstream.
Why this scope
Upstream PR 133 is a superset. The core exploit fix for the loop-extraction issue is the directional rounding in the scaled token path (upstream commit
e371a6b, plus the same withdraw-side floor behavior).Broader upstream commits touch:
Those changes are useful, but they materially increase blast radius. For Fraxtal, where current practical exposure is already very low, the minimal safe fix is preferable.
Included files
contracts/lending/core/protocol/libraries/math/WadRayMath.solcontracts/lending/core/protocol/tokenization/base/ScaledBalanceTokenBase.solcontracts/lending/core/protocol/tokenization/AToken.solcontracts/lending/core/protocol/tokenization/VariableDebtToken.solcontracts/lending/core/protocol/libraries/logic/SupplyLogic.solcontracts/lending/core/mocks/tests/RoundingHarnesses.soltest/dlend/DirectionalRounding.test.tsSemantics implemented
balanceOf: floortotalSupply: floorbalanceOf: ceiltotalSupply: ceiluserBalance: floorTest coverage
Focused Hardhat test added for the exact rounding semantics we want:
Command run:
yarn hardhat compileyarn hardhat test test/dlend/DirectionalRounding.test.tsResult:
4 passingUpgrade path for Fraxtal
This patch changes logic only. It does not introduce new storage fields in upgradeable contracts.
Expected onchain rollout:
Poolimplementation containing theSupplyLogicchange.PoolAddressesProvider.setPoolImpl(newPoolImpl).ATokenimplementation.VariableDebtTokenimplementation.PoolConfigurator.updateAToken(...).PoolConfigurator.updateVariableDebtToken(...).withdraw(type(uint256).max)behaves correctly at non-integer indicespausetofreezeif desired, then unfreeze after verification.Explicitly deferred follow-ups
These are not part of this PR and should be evaluated separately if dLEND wants full v3.5 rounding parity:
GenericLogicavailable borrows base-currency roundingRisk assessment
Low-to-moderate for the targeted issue class.
Reasons:
Residual risk:
Added after initial draft
Attack reproduction
A focused regression reproduction now exists in
test/dlend/DirectionalRounding.test.ts.It demonstrates a concrete legacy supply-withdraw loop at the live dUSD liquidity index:
29base units27scaled units30+1base unit per loopThe test runs that loop
10times side-by-side:+10base units<= 0Upgrade rollout scripts
Governance-aware rollout scripts were added:
deploy/25_dlend_directional_rounding/00_deploy_minimal_directional_rounding_impls.tsdeploy/25_dlend_directional_rounding/01_upgrade_minimal_directional_rounding.tsutils/lending/directional-rounding-upgrade.tsBehavior:
L2Pool,AToken, andVariableDebtTokenimplementationsdUSDDLEND_DIRECTIONAL_ROUNDING_ASSETS=dUSD,...Commands:
yarn hardhat deploy --network fraxtal_mainnet --tags dlend-directional-rounding-deployyarn hardhat deploy --network fraxtal_mainnet --tags dlend-directional-rounding-upgradeyarn hardhat deploy --network fraxtal_mainnet --tags dlend-directional-roundingValidation
Ran:
yarn hardhat compileyarn hardhat test test/dlend/DirectionalRounding.test.tsyarn ts-typecheckResults:
5 passingdeploy/24_dusd_rate_update/00_update_dusd_rate_strategy.ts:58