-
Notifications
You must be signed in to change notification settings - Fork 306
test(pulse): add tests for price updates removal and max price IDs validation #2676
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 5 commits into
main
from
devin/1747075585-add-pulse-scheduler-tests
May 27, 2025
Merged
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
7d07183
feat: add tests for price updates removal and max price IDs validation
devin-ai-integration[bot] 9052d1b
style: fix formatting in PulseScheduler.t.sol
devin-ai-integration[bot] 3720575
test: address PR comments for price updates removal test
devin-ai-integration[bot] 3c0e710
Merge branch 'main' of github.com:pyth-network/pyth-crosschain into d…
tejasbadadare 7225d59
test: fix refs to mockParsePriceFeedUpdatesWithSlots
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2007,4 +2007,128 @@ contract SchedulerTest is Test, SchedulerEvents, PulseSchedulerTestUtils { | |
|
|
||
| // Required to receive ETH when withdrawing funds | ||
| receive() external payable {} | ||
|
|
||
| function testUpdateSubscriptionRemovesPriceUpdatesForRemovedPriceIds() | ||
| public | ||
| { | ||
| // 1. Setup: Add subscription with 3 price feeds, update prices | ||
| uint8 numInitialFeeds = 3; | ||
| uint256 subscriptionId = addTestSubscriptionWithFeeds( | ||
| scheduler, | ||
| numInitialFeeds, | ||
| address(reader) | ||
| ); | ||
| scheduler.addFunds{value: 1 ether}(subscriptionId); | ||
|
|
||
| // Get initial price IDs and create mock price feeds | ||
| bytes32[] memory initialPriceIds = createPriceIds(numInitialFeeds); | ||
| uint64 publishTime = SafeCast.toUint64(block.timestamp); | ||
|
|
||
| // Setup and perform initial price update | ||
| ( | ||
| PythStructs.PriceFeed[] memory priceFeeds, | ||
| uint64[] memory slots | ||
| ) = createMockPriceFeedsWithSlots(publishTime, numInitialFeeds); | ||
| mockParsePriceFeedUpdatesWithSlots(pyth, priceFeeds, slots); | ||
|
|
||
| vm.prank(pusher); | ||
| scheduler.updatePriceFeeds( | ||
| subscriptionId, | ||
| createMockUpdateData(priceFeeds) | ||
| ); | ||
|
|
||
| // Store the removed price ID for later use | ||
| bytes32 removedPriceId = initialPriceIds[numInitialFeeds - 1]; | ||
|
|
||
| // 2. Action: Update subscription to remove the last price feed | ||
| (SchedulerState.SubscriptionParams memory params, ) = scheduler | ||
| .getSubscription(subscriptionId); | ||
|
|
||
| // Create new price IDs array without the last ID | ||
| bytes32[] memory newPriceIds = new bytes32[](numInitialFeeds - 1); | ||
| for (uint i = 0; i < newPriceIds.length; i++) { | ||
| newPriceIds[i] = initialPriceIds[i]; | ||
| } | ||
|
|
||
| params.priceIds = newPriceIds; | ||
|
|
||
| vm.expectEmit(); | ||
| emit SubscriptionUpdated(subscriptionId); | ||
| scheduler.updateSubscription(subscriptionId, params); | ||
|
|
||
| // 3. Verification: | ||
| // - Verify that the removed price ID is no longer part of the subscription's price IDs | ||
| (SchedulerState.SubscriptionParams memory updatedParams, ) = scheduler | ||
| .getSubscription(subscriptionId); | ||
| assertEq( | ||
| updatedParams.priceIds.length, | ||
| numInitialFeeds - 1, | ||
| "Subscription should have one less price ID" | ||
| ); | ||
|
|
||
| bool removedPriceIdFound = false; | ||
| for (uint i = 0; i < updatedParams.priceIds.length; i++) { | ||
| if (updatedParams.priceIds[i] == removedPriceId) { | ||
| removedPriceIdFound = true; | ||
| break; | ||
| } | ||
| } | ||
| assertFalse( | ||
| removedPriceIdFound, | ||
| "Removed price ID should not be in the subscription's price IDs" | ||
| ); | ||
|
|
||
| // - Querying all feeds should return only the remaining feeds | ||
| PythStructs.Price[] memory allPricesAfterUpdate = scheduler | ||
| .getPricesUnsafe(subscriptionId, new bytes32[](0)); | ||
| assertEq( | ||
| allPricesAfterUpdate.length, | ||
| newPriceIds.length, | ||
| "Querying all should only return remaining feeds" | ||
| ); | ||
|
|
||
| // - Verify that the removed price ID is not included in the returned prices | ||
| for (uint i = 0; i < allPricesAfterUpdate.length; i++) { | ||
| // We can't directly check the price ID from the Price struct | ||
| // But we can verify that the number of returned prices matches the number of price IDs | ||
| // in the updated subscription, which indirectly confirms that removed price IDs are not included | ||
| assertEq( | ||
| allPricesAfterUpdate.length, | ||
| updatedParams.priceIds.length, | ||
| "Number of returned prices should match number of price IDs in subscription" | ||
| ); | ||
|
||
| } | ||
| } | ||
|
|
||
| function testUpdateSubscriptionRevertsWithTooManyPriceIds() public { | ||
| // 1. Setup: Create a subscription with a valid number of price IDs | ||
| uint8 initialNumFeeds = 2; | ||
| uint256 subscriptionId = addTestSubscriptionWithFeeds( | ||
| scheduler, | ||
| initialNumFeeds, | ||
| address(reader) | ||
| ); | ||
|
|
||
| // 2. Prepare params with too many price IDs (MAX_PRICE_IDS_PER_SUBSCRIPTION + 1) | ||
| (SchedulerState.SubscriptionParams memory currentParams, ) = scheduler | ||
| .getSubscription(subscriptionId); | ||
|
|
||
| uint16 tooManyFeeds = uint16( | ||
| scheduler.MAX_PRICE_IDS_PER_SUBSCRIPTION() | ||
| ) + 1; | ||
| bytes32[] memory tooManyPriceIds = createPriceIds(tooManyFeeds); | ||
|
|
||
| SchedulerState.SubscriptionParams memory newParams = currentParams; | ||
| newParams.priceIds = tooManyPriceIds; | ||
|
|
||
| // 3. Expect revert when trying to update with too many price IDs | ||
| vm.expectRevert( | ||
| abi.encodeWithSelector( | ||
| TooManyPriceIds.selector, | ||
| tooManyFeeds, | ||
| scheduler.MAX_PRICE_IDS_PER_SUBSCRIPTION() | ||
| ) | ||
| ); | ||
| scheduler.updateSubscription(subscriptionId, newParams); | ||
| } | ||
| } | ||
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 for is unnecessary, but why we cannot check all the priceIds directly? Does the order change?