-
Notifications
You must be signed in to change notification settings - Fork 2
Nialexsan/position lock #113
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
2bbc457
position lock
nialexsan 4bc9345
fixes
nialexsan 68cb959
check topup source type
nialexsan c941b82
manadory health check
nialexsan 8917d16
internal rebalance no lock
nialexsan 358a8a1
fix dup code
nialexsan 7bf8601
fix missing vars
nialexsan 02d1c50
Apply suggestions from code review
nialexsan b158827
Merge branch 'main' into nialexsan/position-lock
nialexsan 37689da
add post on lock method
nialexsan 4e2d345
Merge branch 'main' into nialexsan/position-lock
nialexsan 29b504b
fix typo
nialexsan 3cd0dd2
testing contracts and helpers
nialexsan 670f154
fix internal calls
nialexsan d1c0545
add test for async update position
nialexsan 78ebc9c
address PR comments
nialexsan 666809b
Merge remote-tracking branch 'origin/main' into nialexsan/position-lock
nialexsan 4ab129f
fix tests
nialexsan 95f517c
fix tests
nialexsan c3c9ce5
post check unlock position
nialexsan 7ebf14d
Merge branch 'main' into nialexsan/position-lock
Kay-Zee eefde30
Merge branch 'main' into nialexsan/position-lock
Kay-Zee b7bf0e6
address PR comments
nialexsan File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
Large diffs are not rendered by default.
Oops, something went wrong.
133 changes: 133 additions & 0 deletions
133
cadence/tests/adversarial_recursive_withdraw_source_test.cdc
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,133 @@ | ||
| import Test | ||
| import BlockchainHelpers | ||
|
|
||
| import "MOET" | ||
| import "FlowCreditMarket" | ||
| import "DeFiActions" | ||
| import "DeFiActionsUtils" | ||
| import "FlowToken" | ||
| import "test_helpers.cdc" | ||
| import "FungibleToken" | ||
|
|
||
| access(all) let protocolAccount = Test.getAccount(0x0000000000000007) | ||
| access(all) let protocolConsumerAccount = Test.getAccount(0x0000000000000008) | ||
| access(all) let userAccount = Test.createAccount() | ||
|
|
||
| access(all) let flowTokenIdentifier = "A.0000000000000003.FlowToken.Vault" | ||
| access(all) let moetTokenIdentifier = "A.0000000000000007.MOET.Vault" | ||
| access(all) let flowVaultStoragePath = /storage/flowTokenVault | ||
|
|
||
| access(all) let flowBorrowFactor = 1.0 | ||
| access(all) let flowStartPrice = 1.0 | ||
| access(all) let positionFundingAmount = 1_000.0 | ||
|
|
||
| access(all) var snapshot: UInt64 = 0 | ||
| access(all) var positionID: UInt64 = 0 | ||
|
|
||
| access(all) | ||
| fun setup() { | ||
| deployContracts() | ||
|
|
||
| grantBetaPoolParticipantAccess(protocolAccount, protocolConsumerAccount) | ||
| grantBetaPoolParticipantAccess(protocolAccount, userAccount) | ||
|
|
||
| // Price setup | ||
| setMockOraclePrice(signer: protocolAccount, forTokenIdentifier: flowTokenIdentifier, price: flowStartPrice) | ||
| setMockOraclePrice(signer: protocolAccount, forTokenIdentifier: moetTokenIdentifier, price: 1.0) | ||
|
|
||
| // Create the Pool & add FLOW as supported token | ||
| createAndStorePool(signer: protocolAccount, defaultTokenIdentifier: moetTokenIdentifier, beFailed: false) | ||
| addSupportedTokenZeroRateCurve( | ||
| signer: protocolAccount, | ||
| tokenTypeIdentifier: flowTokenIdentifier, | ||
| collateralFactor: 0.65, | ||
| borrowFactor: 1.0, | ||
| depositRate: 1_000_000.0, | ||
| depositCapacityCap: 1_000_000.0 | ||
| ) | ||
|
|
||
| // Prep user's account | ||
| setupMoetVault(userAccount, beFailed: false) | ||
| mintFlow(to: userAccount, amount: positionFundingAmount * 2.0) | ||
|
|
||
| snapshot = getCurrentBlockHeight() | ||
| } | ||
|
|
||
| access(all) | ||
| fun testRecursiveWithdrawSource() { | ||
| // Ensure we always run from the same post-setup chain state. | ||
| // This makes the test deterministic across multiple runs. | ||
| if snapshot < getCurrentBlockHeight() { | ||
| Test.reset(to: snapshot) | ||
| } | ||
|
|
||
| // ------------------------------------------------------------------------- | ||
| // Seed pool liquidity / establish a baseline lender position | ||
| // ------------------------------------------------------------------------- | ||
| // Create a separate account (user1) that funds the pool by opening a position | ||
| // with a large initial deposit. This ensures the pool has reserves available | ||
| // for subsequent borrow/withdraw paths in this test. | ||
| let user1 = Test.createAccount() | ||
| setupMoetVault(user1, beFailed: false) | ||
| mintMoet(signer: protocolAccount, to: user1.address, amount: 10000.0, beFailed: false) | ||
| mintFlow(to: user1, amount: 10000.0) | ||
|
|
||
| let initialDeposit1 = 10000.0 | ||
| createPosition( | ||
| signer: user1, | ||
| amount: initialDeposit1, | ||
| vaultStoragePath: /storage/flowTokenVault, | ||
| pushToDrawDownSink: false | ||
| ) | ||
| log("[TEST] USER1 POSITION ID: \(positionID)") | ||
|
|
||
| // ------------------------------------------------------------------------- | ||
| // Attempt a reentrancy / recursive-withdraw scenario | ||
| // ------------------------------------------------------------------------- | ||
| // Open a new position for `userAccount` using a special transaction that wires | ||
| // a *malicious* topUpSource (or wrapper behavior) designed to attempt recursion | ||
| // during `withdrawAndPull(..., pullFromTopUpSource: true)`. | ||
| // | ||
| // The goal is to prove the pool rejects the attempt (e.g. via position lock / | ||
| // reentrancy guard), rather than allowing nested withdraw/deposit effects. | ||
| let openRes = executeTransaction( | ||
| "./transactions/position-manager/create_position_reentrancy.cdc", | ||
| [positionFundingAmount, flowVaultStoragePath, false], | ||
| userAccount | ||
| ) | ||
| Test.expect(openRes, Test.beSucceeded()) | ||
|
|
||
| // Read the newly opened position id from the latest Opened event. | ||
| var evts = Test.eventsOfType(Type<FlowCreditMarket.Opened>()) | ||
| let openedEvt = evts[evts.length - 1] as! FlowCreditMarket.Opened | ||
| positionID = openedEvt.pid | ||
| log("[TEST] Position opened with ID: \(positionID)") | ||
|
|
||
| // Log balances for debugging context only (not assertions). | ||
| let remainingFlow = getBalance(address: userAccount.address, vaultPublicPath: /public/flowTokenReceiver) ?? 0.0 | ||
| log("[TEST] User FLOW balance after open: \(remainingFlow)") | ||
| let moetBalance = getBalance(address: userAccount.address, vaultPublicPath: MOET.VaultPublicPath) ?? 0.0 | ||
| log("[TEST] User MOET balance after open: \(moetBalance)") | ||
|
|
||
| // ------------------------------------------------------------------------- | ||
| // Trigger the vulnerable path: withdraw with pullFromTopUpSource=true | ||
| // ------------------------------------------------------------------------- | ||
| // This withdrawal is intentionally oversized so it cannot be satisfied purely | ||
| // from the position’s current available balance. The pool will attempt to pull | ||
| // funds from the configured topUpSource to keep the position above minHealth. | ||
| // | ||
| // In this test, the topUpSource behavior is adversarial: it attempts to re-enter | ||
| // the pool during the pull/deposit flow. We expect the transaction to fail. | ||
| let withdrawRes = executeTransaction( | ||
| "./transactions/flow-credit-market/pool-management/withdraw_from_position.cdc", | ||
| [positionID, flowTokenIdentifier, 1500.0, true], // pullFromTopUpSource: true | ||
| userAccount | ||
| ) | ||
| Test.expect(withdrawRes, Test.beFailed()) | ||
|
|
||
| // Log post-failure balances for debugging context. | ||
| let currentFlow = getBalance(address: userAccount.address, vaultPublicPath: /public/flowTokenReceiver) ?? 0.0 | ||
| log("[TEST] User FLOW balance after failed withdraw: \(currentFlow)") | ||
| let currentMoet = getBalance(address: userAccount.address, vaultPublicPath: MOET.VaultPublicPath) ?? 0.0 | ||
| log("[TEST] User MOET balance after failed withdraw: \(currentMoet)") | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| import Test | ||
| import BlockchainHelpers | ||
|
|
||
| import "MOET" | ||
| import "FlowCreditMarket" | ||
| import "DeFiActions" | ||
| import "DeFiActionsUtils" | ||
| import "FlowToken" | ||
| import "test_helpers.cdc" | ||
| import "FungibleToken" | ||
|
|
||
| access(all) let protocolAccount = Test.getAccount(0x0000000000000007) | ||
| access(all) let liquidityAccount = Test.getAccount(0x0000000000000009) | ||
| access(all) var hackerAccount = Test.getAccount(0x0000000000000008) | ||
|
|
||
| access(all) let flowTokenIdentifier = "A.0000000000000003.FlowToken.Vault" | ||
| access(all) let moetTokenIdentifier = "A.0000000000000007.MOET.Vault" | ||
| access(all) let flowVaultStoragePath = /storage/flowTokenVault | ||
|
|
||
| access(all) | ||
| fun setup() { | ||
| deployContracts() | ||
|
|
||
| grantBetaPoolParticipantAccess(protocolAccount, liquidityAccount) | ||
| grantBetaPoolParticipantAccess(protocolAccount, hackerAccount) | ||
|
|
||
| setMockOraclePrice(signer: protocolAccount, forTokenIdentifier: flowTokenIdentifier, price: 0.0001) | ||
| setMockOraclePrice(signer: protocolAccount, forTokenIdentifier: moetTokenIdentifier, price: 1.0) | ||
|
|
||
| // Create the Pool & add FLOW as supported token | ||
| createAndStorePool(signer: protocolAccount, defaultTokenIdentifier: moetTokenIdentifier, beFailed: false) | ||
| addSupportedTokenZeroRateCurve( | ||
| signer: protocolAccount, | ||
| tokenTypeIdentifier: flowTokenIdentifier, | ||
| collateralFactor: 0.65, | ||
| borrowFactor: 1.0, | ||
| depositRate: 1_000_000.0, | ||
| depositCapacityCap: 1_000_000.0 | ||
| ) | ||
|
|
||
| mintFlow(to: liquidityAccount, amount: 10000.0) | ||
| mintFlow(to: hackerAccount, amount: 2.0) | ||
| setupMoetVault(hackerAccount, beFailed: false) | ||
|
|
||
| // provide liquidity to the pool we can extract | ||
| createPosition(signer: liquidityAccount, amount: 10000.0, vaultStoragePath: flowVaultStoragePath, pushToDrawDownSink: false) | ||
| } | ||
|
|
||
| access(all) | ||
| fun testMaliciousSource() { | ||
| let hackerBalanceBefore = getBalance(address: hackerAccount.address, vaultPublicPath: /public/flowTokenReceiver) ?? 0.0 | ||
| log("[TEST] Hacker's Flow balance before: \(hackerBalanceBefore)") | ||
|
|
||
| // deposit 1 Flow into the position | ||
| let openRes = executeTransaction( | ||
| "./transactions/position-manager/create_position_spoofing_source.cdc", | ||
| [1.0, flowVaultStoragePath, false], | ||
| hackerAccount | ||
| ) | ||
| Test.expect(openRes, Test.beSucceeded()) | ||
|
|
||
| // withdraw 1337 Flow from the position | ||
| let withdrawRes = executeTransaction( | ||
| "./transactions/flow-credit-market/pool-management/withdraw_from_position.cdc", | ||
| [1 as UInt64, flowTokenIdentifier, 1337.0, true], | ||
| hackerAccount | ||
| ) | ||
| Test.expect(withdrawRes, Test.beFailed()) | ||
|
|
||
| // check the balance of the hacker's account | ||
| let hackerBalance = getBalance(address: hackerAccount.address, vaultPublicPath: /public/flowTokenReceiver) ?? 0.0 | ||
| log("[TEST] Hacker's Flow balance: \(hackerBalance)") | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| import Test | ||
| import BlockchainHelpers | ||
| import "FlowCreditMarket" | ||
|
|
||
| import "test_helpers.cdc" | ||
|
|
||
| access(all) var snapshot: UInt64 = 0 | ||
|
|
||
| access(all) | ||
| fun setup() { | ||
| deployContracts() | ||
|
|
||
| snapshot = getCurrentBlockHeight() | ||
| } | ||
|
|
||
| access(all) | ||
| fun testUpdatePosition() { | ||
| let initialPrice = 1.0 | ||
| let priceIncreaseFactor: UFix64 = 1.2 | ||
| setMockOraclePrice(signer: PROTOCOL_ACCOUNT, forTokenIdentifier: FLOW_TOKEN_IDENTIFIER, price: initialPrice) | ||
| setMockOraclePrice(signer: PROTOCOL_ACCOUNT, forTokenIdentifier: MOET_TOKEN_IDENTIFIER, price: initialPrice) | ||
|
|
||
| createAndStorePool(signer: PROTOCOL_ACCOUNT, defaultTokenIdentifier: MOET_TOKEN_IDENTIFIER, beFailed: false) | ||
| addSupportedTokenZeroRateCurve( | ||
| signer: PROTOCOL_ACCOUNT, | ||
| tokenTypeIdentifier: FLOW_TOKEN_IDENTIFIER, | ||
| collateralFactor: 0.8, | ||
| borrowFactor: 1.0, | ||
| depositRate: 1_000.0, | ||
| depositCapacityCap: 1_000.0 | ||
| ) | ||
|
|
||
| let user = Test.createAccount() | ||
| setupMoetVault(user, beFailed: false) | ||
| mintFlow(to: user, amount: 1_000.0) | ||
|
|
||
| createPosition(signer: user, amount: 100.0, vaultStoragePath: FLOW_VAULT_STORAGE_PATH, pushToDrawDownSink: false) | ||
|
|
||
| // increase price | ||
| setMockOraclePrice(signer: PROTOCOL_ACCOUNT, forTokenIdentifier: FLOW_TOKEN_IDENTIFIER, price: initialPrice * priceIncreaseFactor) | ||
|
|
||
| depositToPosition(signer: user, positionID: 0, amount: 600.0, vaultStoragePath: FLOW_VAULT_STORAGE_PATH, pushToDrawDownSink: false) | ||
|
|
||
| let updatePositionRes = _executeTransaction( | ||
| "./transactions/flow-credit-market/pool-management/async_update_position.cdc", | ||
| [ 0 as UInt64 ], | ||
| PROTOCOL_ACCOUNT | ||
| ) | ||
| Test.expect(updatePositionRes, Test.beSucceeded()) | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there any other expected side effects of the position update we can test here besides that the transaction succeeded? I think the user's position collateral amount (denominated in Flow) should have increased, and the user's vault which backs their top-up source should have decreased.
If this is the only test case we have for the async update function, I'd also suggest adding an issue to expand test coverage (no top-up source, top-up source insufficient, position has queued updates, etc.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this test is specifically to test locking mechanism, but I can add more checks here