From 7d071837232b1282d34886a1e7f67040485b5c30 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Mon, 12 May 2025 18:51:12 +0000 Subject: [PATCH 1/4] feat: add tests for price updates removal and max price IDs validation Co-Authored-By: Tejas Badadare --- .../contracts/forge-test/PulseScheduler.t.sol | 115 ++++++++++++++++++ 1 file changed, 115 insertions(+) diff --git a/target_chains/ethereum/contracts/forge-test/PulseScheduler.t.sol b/target_chains/ethereum/contracts/forge-test/PulseScheduler.t.sol index ee06ff171f..2e70e43d3e 100644 --- a/target_chains/ethereum/contracts/forge-test/PulseScheduler.t.sol +++ b/target_chains/ethereum/contracts/forge-test/PulseScheduler.t.sol @@ -2007,4 +2007,119 @@ 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); + } } From 9052d1b598ff61f8f225df556d2861c4cb6bf9d6 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Mon, 12 May 2025 18:55:48 +0000 Subject: [PATCH 2/4] style: fix formatting in PulseScheduler.t.sol Co-Authored-By: Tejas Badadare --- .../contracts/forge-test/PulseScheduler.t.sol | 45 +++++++++++-------- 1 file changed, 27 insertions(+), 18 deletions(-) diff --git a/target_chains/ethereum/contracts/forge-test/PulseScheduler.t.sol b/target_chains/ethereum/contracts/forge-test/PulseScheduler.t.sol index 2e70e43d3e..00266e70b1 100644 --- a/target_chains/ethereum/contracts/forge-test/PulseScheduler.t.sol +++ b/target_chains/ethereum/contracts/forge-test/PulseScheduler.t.sol @@ -2008,7 +2008,9 @@ contract SchedulerTest is Test, SchedulerEvents, PulseSchedulerTestUtils { // Required to receive ETH when withdrawing funds receive() external payable {} - function testUpdateSubscriptionRemovesPriceUpdatesForRemovedPriceIds() public { + function testUpdateSubscriptionRemovesPriceUpdatesForRemovedPriceIds() + public + { // 1. Setup: Add subscription with 3 price feeds, update prices uint8 numInitialFeeds = 3; uint256 subscriptionId = addTestSubscriptionWithFeeds( @@ -2021,42 +2023,49 @@ contract SchedulerTest is Test, SchedulerEvents, PulseSchedulerTestUtils { // 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); + ( + PythStructs.PriceFeed[] memory priceFeeds, + uint64[] memory slots + ) = createMockPriceFeedsWithSlots(publishTime, numInitialFeeds); mockParsePriceFeedUpdatesWithSlots(pyth, priceFeeds, slots); - + vm.prank(pusher); - scheduler.updatePriceFeeds(subscriptionId, createMockUpdateData(priceFeeds)); + 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); - + (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); + (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) { @@ -2077,7 +2086,7 @@ contract SchedulerTest is Test, SchedulerEvents, PulseSchedulerTestUtils { 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 @@ -2090,8 +2099,6 @@ contract SchedulerTest is Test, SchedulerEvents, PulseSchedulerTestUtils { ); } } - - function testUpdateSubscriptionRevertsWithTooManyPriceIds() public { // 1. Setup: Create a subscription with a valid number of price IDs @@ -2105,10 +2112,12 @@ contract SchedulerTest is Test, SchedulerEvents, PulseSchedulerTestUtils { // 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; + + uint16 tooManyFeeds = uint16( + scheduler.MAX_PRICE_IDS_PER_SUBSCRIPTION() + ) + 1; bytes32[] memory tooManyPriceIds = createPriceIds(tooManyFeeds); - + SchedulerState.SubscriptionParams memory newParams = currentParams; newParams.priceIds = tooManyPriceIds; From 3720575b3009abf30592c8bce07a6f3fe5cbcb8d Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 27 May 2025 12:26:18 +0000 Subject: [PATCH 3/4] test: address PR comments for price updates removal test Co-Authored-By: Tejas Badadare --- .../contracts/forge-test/PulseScheduler.t.sol | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/target_chains/ethereum/contracts/forge-test/PulseScheduler.t.sol b/target_chains/ethereum/contracts/forge-test/PulseScheduler.t.sol index 00266e70b1..04810f3ab5 100644 --- a/target_chains/ethereum/contracts/forge-test/PulseScheduler.t.sol +++ b/target_chains/ethereum/contracts/forge-test/PulseScheduler.t.sol @@ -2087,17 +2087,17 @@ contract SchedulerTest is Test, SchedulerEvents, PulseSchedulerTestUtils { "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" - ); - } + // - Verify that trying to get the price of the removed feed directly reverts + bytes32[] memory removedIdArray = new bytes32[](1); + removedIdArray[0] = removedPriceId; + vm.expectRevert( + abi.encodeWithSelector( + InvalidPriceId.selector, + removedPriceId, + bytes32(0) + ) + ); + scheduler.getPricesUnsafe(subscriptionId, removedIdArray); } function testUpdateSubscriptionRevertsWithTooManyPriceIds() public { From 7225d59d3c4becb3fad25a106efc0c80010ce526 Mon Sep 17 00:00:00 2001 From: Tejas Badadare Date: Tue, 27 May 2025 09:38:14 -0700 Subject: [PATCH 4/4] test: fix refs to mockParsePriceFeedUpdatesWithSlots --- .../ethereum/contracts/forge-test/PulseScheduler.t.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target_chains/ethereum/contracts/forge-test/PulseScheduler.t.sol b/target_chains/ethereum/contracts/forge-test/PulseScheduler.t.sol index 781b6927f4..f894a4b7f8 100644 --- a/target_chains/ethereum/contracts/forge-test/PulseScheduler.t.sol +++ b/target_chains/ethereum/contracts/forge-test/PulseScheduler.t.sol @@ -2139,7 +2139,7 @@ contract SchedulerTest is Test, SchedulerEvents, PulseSchedulerTestUtils { PythStructs.PriceFeed[] memory priceFeeds, uint64[] memory slots ) = createMockPriceFeedsWithSlots(publishTime, numInitialFeeds); - mockParsePriceFeedUpdatesWithSlots(pyth, priceFeeds, slots); + mockParsePriceFeedUpdatesWithSlotsStrict(pyth, priceFeeds, slots); vm.prank(pusher); scheduler.updatePriceFeeds(