SOV-5206: introduce loan id guard implementation #561
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a loan ID-specific reentrancy guard mechanism to prevent flash borrow-and-close attacks. The implementation adds a LoanIdMutex contract that tracks loan operations at the block level and a LoanIdGuard modifier that prevents multiple operations on the same loan ID within a single block.
Key changes:
- New
LoanIdMutexcontract for tracking loan ID operations per block LoanIdGuardabstract contract providing theloanIdNonReentrantmodifier- Integration of the guard into
LoanOpeningsandLoanClosingsWithmodules - Comprehensive test coverage for the new mutex and attack prevention
Reviewed Changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| contracts/reentrancy/LoanIdMutex.sol | Core mutex contract mapping loan IDs to block numbers |
| contracts/reentrancy/LoanIdGuard.sol | Abstract guard providing the loanIdNonReentrant modifier |
| contracts/core/State.sol | Added LoanIdGuard to inheritance chain |
| contracts/modules/LoanOpenings.sol | Added mutex check when creating loans |
| contracts/modules/LoanClosingsWith.sol | Added loanIdNonReentrant modifier to closing functions |
| contracts/testhelpers/LoanIdMutexTester.sol | Helper contract for testing same-block protection |
| contracts/testhelpers/FlashBorrowAttack.sol | Attack contract demonstrating the vulnerability |
| tests/reentrancy/LoanIdMutex.test.js | Comprehensive unit tests for LoanIdMutex |
| tests/loan-openings/LoanIdGuardBorrow.test.js | Integration tests for flash borrow attack prevention |
| deployment/helpers/reentrancy/utils.js | Utility functions for deterministic deployment |
| deployment/deploy/6-deployLoanIdMutex.js | Deployment script for LoanIdMutex |
| scripts/generateLoanIdMutexDeployTx.js | Script to generate deterministic deployment transaction |
| hardhat/tasks/sips/args/sipArgs.js | SIP-0088 proposal arguments |
| tests-onchain/sip0088.test.js | On-chain test for SIP-0088 governance proposal |
| Multiple test files | Updated to deploy LoanIdMutex in test fixtures |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 25 out of 25 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| // Test basic functionality | ||
| const testLoanId = ethers.utils.formatBytes32String('test'); | ||
| const blockNum = await mutex.getBlockNumber(testLoanId); |
There was a problem hiding this comment.
| const blockNum = await mutex.getBlockNumber(testLoanId); | |
| const blockNum = await mutex.getLoanIdLastBlockNumber(testLoanId); |
| console.log('✓ checkAndToggle successful'); | ||
|
|
||
| // Verify block number was recorded | ||
| const blockNum = await mutex.getBlockNumber(testLoanId); |
There was a problem hiding this comment.
| const blockNum = await mutex.getBlockNumber(testLoanId); | |
| const blockNum = await mutex.getLoanIdLastBlockNumber(testLoanId); |
| * | ||
| * @param loanId The ID of the loan to check and mark. | ||
| */ | ||
| function checkAndToggle(bytes32 loanId) external { |
There was a problem hiding this comment.
anyone can call this function. it exposes a vulnerability - anyone can block closing of any loan.
it should be only available for setting from within the protocol contract.
alternatively you can implement it as a new module or a shared logic.
No description provided.