-
Notifications
You must be signed in to change notification settings - Fork 306
fix(pulse): ensure subscription balance is greater than minimum balance after adding funds #2680
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
tejasbadadare
merged 13 commits into
main
from
devin/1747076033-ensure-minimum-balance-after-adding-funds
May 27, 2025
Merged
Changes from 6 commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
de61d29
feat(pulse): add maximum deposit limit for permanent subscriptions
devin-ai-integration[bot] c629578
fix: remove unused variable in PulseScheduler.t.sol
devin-ai-integration[bot] 0551da5
fix: update deposit limit check to only check incoming deposit amount
devin-ai-integration[bot] 422dca8
fix: remove redundant access
tejasbadadare 2ee0169
fix: ensure subscription balance is greater than minimum balance afte…
devin-ai-integration[bot] f94c03f
fix: format code to pass CI
devin-ai-integration[bot] ea438a1
test: improve addFunds minimum balance test
devin-ai-integration[bot] f7d8b61
test: fix unused variable warnings in PulseScheduler tests
devin-ai-integration[bot] 899027e
test: fix unused variable in PulseScheduler test
devin-ai-integration[bot] caa5953
test: remove duplicate test functions
devin-ai-integration[bot] 89198f4
Merge branch 'main' of github.com:pyth-network/pyth-crosschain into d…
tejasbadadare 929bef1
ci: fmt
tejasbadadare 96f1bca
test: fix testAddFundsEnforcesMinimumBalance
tejasbadadare 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
There are no files selected for viewing
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
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
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
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 |
|---|---|---|
|
|
@@ -542,6 +542,145 @@ contract SchedulerTest is Test, SchedulerEvents, PulseSchedulerTestUtils { | |
| ); | ||
| } | ||
|
|
||
| function testAddFundsEnforcesMinimumBalance() public { | ||
| // First add a subscription with minimum balance | ||
| uint256 subscriptionId = addTestSubscription( | ||
| scheduler, | ||
| address(reader) | ||
| ); | ||
|
|
||
| // Get subscription parameters and initial balance | ||
| ( | ||
| SchedulerState.SubscriptionParams memory params, | ||
| SchedulerState.SubscriptionStatus memory status | ||
| ) = scheduler.getSubscription(subscriptionId); | ||
|
|
||
| // Calculate minimum balance | ||
| uint256 minimumBalance = scheduler.getMinimumBalance( | ||
| uint8(params.priceIds.length) | ||
| ); | ||
|
|
||
| // Verify initial balance is at minimum | ||
| assertEq( | ||
| status.balanceInWei, | ||
| minimumBalance, | ||
| "Initial balance should equal minimum balance" | ||
| ); | ||
|
|
||
| // Add some funds to the subscription | ||
| uint256 additionalFunds = 0.1 ether; | ||
| scheduler.addFunds{value: additionalFunds}(subscriptionId); | ||
|
|
||
| // Verify funds were added | ||
| (, SchedulerState.SubscriptionStatus memory statusAfterAdd) = scheduler | ||
| .getSubscription(subscriptionId); | ||
| assertEq( | ||
| statusAfterAdd.balanceInWei, | ||
| minimumBalance + additionalFunds, | ||
| "Balance should be increased by the added funds" | ||
| ); | ||
|
|
||
| // Now create a new subscription but don't fund it fully | ||
| SchedulerState.SubscriptionParams | ||
| memory newParams = createDefaultSubscriptionParams( | ||
| 2, | ||
| address(reader) | ||
| ); | ||
|
|
||
| // Calculate minimum balance for this new subscription | ||
| uint256 newMinimumBalance = scheduler.getMinimumBalance( | ||
| uint8(newParams.priceIds.length) | ||
| ); | ||
|
|
||
| // Try to create with insufficient funds | ||
| uint256 insufficientFunds = newMinimumBalance - 1 wei; | ||
| vm.expectRevert(abi.encodeWithSelector(InsufficientBalance.selector)); | ||
| scheduler.createSubscription{value: insufficientFunds}(newParams); | ||
|
|
||
| // Create with sufficient funds | ||
| uint256 newSubscriptionId = scheduler.createSubscription{ | ||
| value: newMinimumBalance | ||
| }(newParams); | ||
|
|
||
| // Verify subscription was created with minimum balance | ||
| (, SchedulerState.SubscriptionStatus memory newStatus) = scheduler | ||
| .getSubscription(newSubscriptionId); | ||
| assertEq( | ||
| newStatus.balanceInWei, | ||
| newMinimumBalance, | ||
| "New subscription balance should equal minimum balance" | ||
| ); | ||
| } | ||
|
|
||
| function testAddFundsEnforcesMinimumBalanceForPermanentSubscription() | ||
|
||
| public | ||
| { | ||
| // Create a non-permanent subscription first | ||
| uint256 subscriptionId = addTestSubscription( | ||
| scheduler, | ||
| address(reader) | ||
| ); | ||
|
|
||
| // Get subscription parameters and initial balance | ||
| ( | ||
| SchedulerState.SubscriptionParams memory params, | ||
| SchedulerState.SubscriptionStatus memory status | ||
| ) = scheduler.getSubscription(subscriptionId); | ||
|
|
||
| // Calculate minimum balance | ||
| uint256 minimumBalance = scheduler.getMinimumBalance( | ||
| uint8(params.priceIds.length) | ||
| ); | ||
|
|
||
| // Verify initial balance is at minimum | ||
| assertEq( | ||
| status.balanceInWei, | ||
| minimumBalance, | ||
| "Initial balance should equal minimum balance" | ||
| ); | ||
|
|
||
| // Make it permanent | ||
| params.isPermanent = true; | ||
| scheduler.updateSubscription(subscriptionId, params); | ||
|
|
||
| // Try to add funds - this should succeed since we're adding to a permanent subscription | ||
| uint256 additionalFunds = 0.1 ether; | ||
| scheduler.addFunds{value: additionalFunds}(subscriptionId); | ||
|
|
||
| // Verify funds were added | ||
| (, SchedulerState.SubscriptionStatus memory statusAfter) = scheduler | ||
| .getSubscription(subscriptionId); | ||
| assertEq( | ||
| statusAfter.balanceInWei, | ||
| minimumBalance + additionalFunds, | ||
| "Balance should be increased by the added funds" | ||
| ); | ||
|
|
||
| // Now test the deposit limit for permanent subscriptions | ||
| uint256 maxDepositLimit = 100 ether; // MAX_DEPOSIT_LIMIT from SchedulerState | ||
|
|
||
| // Try to add funds exceeding the deposit limit | ||
| vm.expectRevert( | ||
| abi.encodeWithSelector(MaxDepositLimitExceeded.selector) | ||
| ); | ||
| scheduler.addFunds{value: maxDepositLimit + 1}(subscriptionId); | ||
|
|
||
| // Add funds within the deposit limit | ||
| uint256 validDeposit = 1 ether; | ||
| scheduler.addFunds{value: validDeposit}(subscriptionId); | ||
|
|
||
| // Verify funds were added | ||
| ( | ||
| , | ||
| SchedulerState.SubscriptionStatus memory statusAfterValidDeposit | ||
| ) = scheduler.getSubscription(subscriptionId); | ||
| assertEq( | ||
| statusAfterValidDeposit.balanceInWei, | ||
| minimumBalance + additionalFunds + validDeposit, | ||
| "Balance should be increased by the valid deposit" | ||
| ); | ||
| } | ||
|
|
||
| function testWithdrawFunds() public { | ||
| // Add a subscription and get the parameters | ||
| uint256 subscriptionId = addTestSubscription( | ||
|
|
@@ -779,6 +918,112 @@ contract SchedulerTest is Test, SchedulerEvents, PulseSchedulerTestUtils { | |
| scheduler.updateSubscription(subscriptionId, params); | ||
| } | ||
|
|
||
| function testPermanentSubscriptionDepositLimit() public { | ||
| // Test 1: Creating a permanent subscription with deposit exceeding MAX_DEPOSIT_LIMIT should fail | ||
| SchedulerState.SubscriptionParams | ||
| memory params = createDefaultSubscriptionParams(2, address(reader)); | ||
| params.isPermanent = true; | ||
|
|
||
| uint256 maxDepositLimit = 100 ether; // Same as MAX_DEPOSIT_LIMIT in SchedulerState | ||
| uint256 excessiveDeposit = maxDepositLimit + 1 ether; | ||
| vm.deal(address(this), excessiveDeposit); | ||
|
|
||
| vm.expectRevert( | ||
| abi.encodeWithSelector(MaxDepositLimitExceeded.selector) | ||
| ); | ||
| scheduler.createSubscription{value: excessiveDeposit}(params); | ||
|
|
||
| // Test 2: Creating a permanent subscription with deposit within MAX_DEPOSIT_LIMIT should succeed | ||
| uint256 validDeposit = maxDepositLimit; | ||
| vm.deal(address(this), validDeposit); | ||
|
|
||
| uint256 subscriptionId = scheduler.createSubscription{ | ||
| value: validDeposit | ||
| }(params); | ||
|
|
||
| // Verify subscription was created correctly | ||
| ( | ||
| SchedulerState.SubscriptionParams memory storedParams, | ||
| SchedulerState.SubscriptionStatus memory status | ||
| ) = scheduler.getSubscription(subscriptionId); | ||
|
|
||
| assertTrue( | ||
| storedParams.isPermanent, | ||
| "Subscription should be permanent" | ||
| ); | ||
| assertEq( | ||
| status.balanceInWei, | ||
| validDeposit, | ||
| "Balance should match deposit amount" | ||
| ); | ||
|
|
||
| // Test 3: Adding funds to a permanent subscription with deposit exceeding MAX_DEPOSIT_LIMIT should fail | ||
| uint256 largeAdditionalFunds = maxDepositLimit + 1; | ||
| vm.deal(address(this), largeAdditionalFunds); | ||
|
|
||
| vm.expectRevert( | ||
| abi.encodeWithSelector(MaxDepositLimitExceeded.selector) | ||
| ); | ||
| scheduler.addFunds{value: largeAdditionalFunds}(subscriptionId); | ||
|
|
||
| // Test 4: Adding funds to a permanent subscription within MAX_DEPOSIT_LIMIT should succeed | ||
| // Create a non-permanent subscription to test partial funding | ||
| SchedulerState.SubscriptionParams | ||
| memory nonPermanentParams = createDefaultSubscriptionParams( | ||
| 2, | ||
| address(reader) | ||
| ); | ||
| uint256 minimumBalance = scheduler.getMinimumBalance( | ||
| uint8(nonPermanentParams.priceIds.length) | ||
| ); | ||
| vm.deal(address(this), minimumBalance); | ||
|
|
||
| uint256 nonPermanentSubId = scheduler.createSubscription{ | ||
| value: minimumBalance | ||
| }(nonPermanentParams); | ||
|
|
||
| // Add funds to the non-permanent subscription (should be within limit) | ||
| uint256 validAdditionalFunds = 5 ether; | ||
| vm.deal(address(this), validAdditionalFunds); | ||
|
|
||
| scheduler.addFunds{value: validAdditionalFunds}(nonPermanentSubId); | ||
|
|
||
| // Verify funds were added correctly | ||
| ( | ||
| , | ||
| SchedulerState.SubscriptionStatus memory nonPermanentStatus | ||
| ) = scheduler.getSubscription(nonPermanentSubId); | ||
|
|
||
| assertEq( | ||
| nonPermanentStatus.balanceInWei, | ||
| minimumBalance + validAdditionalFunds, | ||
| "Balance should be increased by the funded amount" | ||
| ); | ||
|
|
||
| // Test 5: Non-permanent subscriptions should not be subject to the deposit limit | ||
| uint256 largeDeposit = maxDepositLimit * 2; | ||
| vm.deal(address(this), largeDeposit); | ||
|
|
||
| SchedulerState.SubscriptionParams | ||
| memory unlimitedParams = createDefaultSubscriptionParams( | ||
| 2, | ||
| address(reader) | ||
| ); | ||
| uint256 unlimitedSubId = scheduler.createSubscription{ | ||
| value: largeDeposit | ||
| }(unlimitedParams); | ||
|
|
||
| // Verify subscription was created with the large deposit | ||
| (, SchedulerState.SubscriptionStatus memory unlimitedStatus) = scheduler | ||
| .getSubscription(unlimitedSubId); | ||
|
|
||
| assertEq( | ||
| unlimitedStatus.balanceInWei, | ||
| largeDeposit, | ||
| "Non-permanent subscription should accept large deposits" | ||
| ); | ||
| } | ||
|
|
||
| function testAnyoneCanAddFunds() public { | ||
| // Create a subscription | ||
| uint256 subscriptionId = addTestSubscription( | ||
|
|
||
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.
this is not testing balance check in
addFunds. here you need to send some updates to drain the balance and then try to add funds