Tejas Badadare 7 maanden geleden
bovenliggende
commit
4e4126324f

+ 2 - 2
target_chains/ethereum/contracts/contracts/pulse/scheduler/IScheduler.sol

@@ -44,7 +44,7 @@ interface IScheduler is SchedulerEvents {
     function updateSubscription(
         uint256 subscriptionId,
         SchedulerState.SubscriptionParams calldata newSubscriptionParams
-    ) external payable;
+    ) external;
 
     // Deactivation is now handled through updateSubscription by setting isActive to false
 
@@ -113,7 +113,7 @@ interface IScheduler is SchedulerEvents {
      */
     function getMinimumBalance(
         uint8 numPriceFeeds
-    ) external view returns (uint256 minimumBalance);
+    ) external view returns (uint256 minimumBalanceInWei);
 
     /**
      * @notice Gets all active subscriptions with their parameters

+ 48 - 54
target_chains/ethereum/contracts/contracts/pulse/scheduler/Scheduler.sol

@@ -77,7 +77,6 @@ abstract contract Scheduler is IScheduler, SchedulerState {
         status.balanceInWei = msg.value;
         status.totalUpdates = 0;
         status.totalSpent = 0;
-        status.isActive = true;
 
         // Map subscription ID to manager
         _state.subscriptionManager[subscriptionId] = msg.sender;
@@ -86,66 +85,47 @@ abstract contract Scheduler is IScheduler, SchedulerState {
         return subscriptionId;
     }
 
-    function getSubscription(
-        uint256 subscriptionId
-    )
-        external
-        view
-        override
-        returns (
-            SubscriptionParams memory params,
-            SubscriptionStatus memory status
-        )
-    {
-        return (
-            _state.subscriptionParams[subscriptionId],
-            _state.subscriptionStatuses[subscriptionId]
-        );
-    }
-
     function updateSubscription(
         uint256 subscriptionId,
-        SubscriptionParams memory newSubscriptionParams
-    ) external payable override onlyManager(subscriptionId) {
-        SubscriptionStatus storage status = _state.subscriptionStatuses[
-            subscriptionId
-        ];
-        bool wasActive = status.isActive;
-        bool willBeActive = newSubscriptionParams.isActive;
+        SubscriptionParams memory newParams
+    ) external override onlyManager(subscriptionId) {
+        SchedulerState.SubscriptionStatus storage currentStatus = _state
+            .subscriptionStatuses[subscriptionId];
+        SchedulerState.SubscriptionParams storage currentParams = _state
+            .subscriptionParams[subscriptionId];
+        bool wasActive = currentParams.isActive;
+        bool willBeActive = newParams.isActive;
 
         // If subscription is inactive and will remain inactive, no need to validate parameters
         if (!wasActive && !willBeActive) {
             // Update subscription parameters
-            _state.subscriptionParams[subscriptionId] = newSubscriptionParams;
+            _state.subscriptionParams[subscriptionId] = newParams;
             emit SubscriptionUpdated(subscriptionId);
             return;
         }
 
         // Validate parameters for active or to-be-activated subscriptions
-        if (newSubscriptionParams.priceIds.length > MAX_PRICE_IDS) {
-            revert TooManyPriceIds(
-                newSubscriptionParams.priceIds.length,
-                MAX_PRICE_IDS
-            );
+        if (newParams.priceIds.length > MAX_PRICE_IDS) {
+            revert TooManyPriceIds(newParams.priceIds.length, MAX_PRICE_IDS);
         }
 
         // Validate update criteria
         if (
-            !newSubscriptionParams.updateCriteria.updateOnHeartbeat &&
-            !newSubscriptionParams.updateCriteria.updateOnDeviation
+            !newParams.updateCriteria.updateOnHeartbeat &&
+            !newParams.updateCriteria.updateOnDeviation
         ) {
             revert InvalidUpdateCriteria();
         }
 
         // If gas config is unset, set it to the default (100x multipliers)
         if (
-            newSubscriptionParams.gasConfig.maxGasMultiplierCapPct == 0 ||
-            newSubscriptionParams.gasConfig.maxFeeMultiplierCapPct == 0
+            newParams.gasConfig.maxGasMultiplierCapPct == 0 ||
+            newParams.gasConfig.maxFeeMultiplierCapPct == 0
         ) {
-            newSubscriptionParams
+            newParams
                 .gasConfig
                 .maxFeeMultiplierCapPct = DEFAULT_MAX_FEE_MULTIPLIER_CAP_PCT;
-            newSubscriptionParams
+            newParams
                 .gasConfig
                 .maxGasMultiplierCapPct = DEFAULT_MAX_GAS_MULTIPLIER_CAP_PCT;
         }
@@ -154,33 +134,28 @@ abstract contract Scheduler is IScheduler, SchedulerState {
         if (!wasActive && willBeActive) {
             // Reactivating a subscription - ensure minimum balance
             uint256 minimumBalance = this.getMinimumBalance(
-                uint8(newSubscriptionParams.priceIds.length)
+                uint8(newParams.priceIds.length)
             );
 
-            // Add any funds sent with this transaction
-            status.balanceInWei += msg.value;
-
             // Check if balance meets minimum requirement
-            if (status.balanceInWei < minimumBalance) {
+            if (currentStatus.balanceInWei < minimumBalance) {
                 revert InsufficientBalance();
             }
 
-            status.isActive = true;
+            currentParams.isActive = true;
             emit SubscriptionActivated(subscriptionId);
         } else if (wasActive && !willBeActive) {
             // Deactivating a subscription
-            status.isActive = false;
+            currentParams.isActive = false;
             emit SubscriptionDeactivated(subscriptionId);
         }
 
         // Update subscription parameters
-        _state.subscriptionParams[subscriptionId] = newSubscriptionParams;
+        _state.subscriptionParams[subscriptionId] = newParams;
 
         emit SubscriptionUpdated(subscriptionId);
     }
 
-    // Removed standalone deactivateSubscription function as it's now handled in updateSubscription
-
     function updatePriceFeeds(
         uint256 subscriptionId,
         bytes[] calldata updateData,
@@ -193,7 +168,7 @@ abstract contract Scheduler is IScheduler, SchedulerState {
             subscriptionId
         ];
 
-        if (!status.isActive) {
+        if (!params.isActive) {
             revert InactiveSubscription();
         }
 
@@ -369,7 +344,7 @@ abstract contract Scheduler is IScheduler, SchedulerState {
         uint256 subscriptionId,
         bytes32[] calldata priceIds
     ) internal view returns (PythStructs.PriceFeed[] memory priceFeeds) {
-        if (!_state.subscriptionStatuses[subscriptionId].isActive) {
+        if (!_state.subscriptionParams[subscriptionId].isActive) {
             revert InactiveSubscription();
         }
 
@@ -462,7 +437,7 @@ abstract contract Scheduler is IScheduler, SchedulerState {
     function addFunds(
         uint256 subscriptionId
     ) external payable override onlyManager(subscriptionId) {
-        if (!_state.subscriptionStatuses[subscriptionId].isActive) {
+        if (!_state.subscriptionParams[subscriptionId].isActive) {
             revert InactiveSubscription();
         }
 
@@ -485,7 +460,7 @@ abstract contract Scheduler is IScheduler, SchedulerState {
         }
 
         // If subscription is active, ensure minimum balance is maintained
-        if (status.isActive) {
+        if (params.isActive) {
             uint256 minimumBalance = this.getMinimumBalance(
                 uint8(params.priceIds.length)
             );
@@ -500,6 +475,25 @@ abstract contract Scheduler is IScheduler, SchedulerState {
         require(sent, "Failed to send funds");
     }
 
+    // FETCH SUBSCRIPTIONS
+
+    function getSubscription(
+        uint256 subscriptionId
+    )
+        external
+        view
+        override
+        returns (
+            SubscriptionParams memory params,
+            SubscriptionStatus memory status
+        )
+    {
+        return (
+            _state.subscriptionParams[subscriptionId],
+            _state.subscriptionStatuses[subscriptionId]
+        );
+    }
+
     // This function is intentionally public with no access control to allow keepers to discover active subscriptions
     function getActiveSubscriptions(
         uint256 startIndex,
@@ -518,7 +512,7 @@ abstract contract Scheduler is IScheduler, SchedulerState {
         // TODO: Optimize this. store numActiveSubscriptions or something.
         totalCount = 0;
         for (uint256 i = 1; i < _state.subscriptionNumber; i++) {
-            if (_state.subscriptionStatuses[i].isActive) {
+            if (_state.subscriptionParams[i].isActive) {
                 totalCount++;
             }
         }
@@ -547,7 +541,7 @@ abstract contract Scheduler is IScheduler, SchedulerState {
             i < _state.subscriptionNumber && resultIndex < resultCount;
             i++
         ) {
-            if (_state.subscriptionStatuses[i].isActive) {
+            if (_state.subscriptionParams[i].isActive) {
                 if (activeIndex >= startIndex) {
                     subscriptionIds[resultIndex] = i;
                     subscriptionParams[resultIndex] = _state.subscriptionParams[
@@ -568,7 +562,7 @@ abstract contract Scheduler is IScheduler, SchedulerState {
      */
     function getMinimumBalance(
         uint8 numPriceFeeds
-    ) external pure override returns (uint256 minimumBalance) {
+    ) external pure override returns (uint256 minimumBalanceInWei) {
         // Simple implementation - minimum balance is 0.01 ETH per price feed
         return numPriceFeeds * 0.01 ether;
     }

+ 0 - 1
target_chains/ethereum/contracts/contracts/pulse/scheduler/SchedulerState.sol

@@ -42,7 +42,6 @@ contract SchedulerState {
         uint256 balanceInWei;
         uint256 totalUpdates;
         uint256 totalSpent;
-        bool isActive;
     }
 
     /// @dev When pushing prices, providers will use a "fast gas" estimation as default.

+ 0 - 15
target_chains/ethereum/contracts/contracts/pulse/scheduler/SchedulerUpgradeable.sol

@@ -61,19 +61,4 @@ contract SchedulerUpgradeable is
     function version() public pure returns (string memory) {
         return "1.0.0";
     }
-
-    // Implementation of deactivateSubscription that forwards to updateSubscription
-    function deactivateSubscription(
-        uint256 subscriptionId
-    ) external onlyManager(subscriptionId) {
-        // Get current subscription parameters
-        SchedulerState.SubscriptionParams memory params = _state
-            .subscriptionParams[subscriptionId];
-
-        // Set isActive to false
-        params.isActive = false;
-
-        // Call updateSubscription to handle the deactivation
-        this.updateSubscription(subscriptionId, params);
-    }
 }

+ 13 - 27
target_chains/ethereum/contracts/forge-test/PulseScheduler.t.sol

@@ -91,6 +91,9 @@ contract SchedulerTest is Test, SchedulerEvents, PulseTestUtils {
         // Start tests at timestamp 100 to avoid underflow when we set
         // `minPublishTime = timestamp - 10 seconds` in updatePriceFeeds
         vm.warp(100);
+
+        // Give pusher 100 ETH for testing
+        vm.deal(pusher, 100 ether);
     }
 
     function testAddSubscription() public {
@@ -157,6 +160,7 @@ contract SchedulerTest is Test, SchedulerEvents, PulseTestUtils {
             true,
             "whitelistEnabled should be true"
         );
+        assertTrue(storedParams.isActive, "Subscription should be active");
         assertEq(
             storedParams.updateCriteria.heartbeatSeconds,
             60,
@@ -173,7 +177,6 @@ contract SchedulerTest is Test, SchedulerEvents, PulseTestUtils {
             "Max gas multiplier mismatch"
         );
 
-        assertTrue(status.isActive, "Subscription should be active");
         assertEq(
             status.balanceInWei,
             minimumBalance,
@@ -257,7 +260,7 @@ contract SchedulerTest is Test, SchedulerEvents, PulseTestUtils {
         );
     }
 
-    function testAddSubscriptionInsufficientFunds() public {
+    function testAddSubscriptionWithInsufficientFundsReverts() public {
         // Create subscription parameters
         bytes32[] memory priceIds = createPriceIds();
         address[] memory readerWhitelist = new address[](1);
@@ -348,11 +351,7 @@ contract SchedulerTest is Test, SchedulerEvents, PulseTestUtils {
             SchedulerState.SubscriptionStatus memory status
         ) = scheduler.getSubscription(subscriptionId);
 
-        assertFalse(status.isActive, "Subscription should be inactive");
-        assertFalse(
-            storedParams.isActive,
-            "Subscription params should show inactive"
-        );
+        assertFalse(storedParams.isActive, "Subscription should be inactive");
 
         // Reactivate subscription using updateSubscription
         params.isActive = true;
@@ -367,7 +366,7 @@ contract SchedulerTest is Test, SchedulerEvents, PulseTestUtils {
         // Verify subscription was reactivated
         (storedParams, status) = scheduler.getSubscription(subscriptionId);
 
-        assertTrue(status.isActive, "Subscription should be active");
+        assertTrue(storedParams.isActive, "Subscription should be active");
         assertTrue(
             storedParams.isActive,
             "Subscription params should show active"
@@ -969,13 +968,9 @@ contract SchedulerTest is Test, SchedulerEvents, PulseTestUtils {
     }
 
     function testGetActiveSubscriptions() public {
-        console.log("Starting testGetActiveSubscriptions");
-
         // Add two subscriptions with the test contract as manager
-        uint256 sub1 = addTestSubscription();
-        uint256 sub2 = addTestSubscription();
-
-        console.log("Added 2 test subscriptions with IDs:", sub1, sub2);
+        addTestSubscription();
+        addTestSubscription();
 
         // Create a subscription with pusher as manager
         vm.startPrank(pusher);
@@ -1008,10 +1003,8 @@ contract SchedulerTest is Test, SchedulerEvents, PulseTestUtils {
         uint256 minimumBalance = scheduler.getMinimumBalance(
             uint8(priceIds.length)
         );
-        uint256 pusherSub = scheduler.addSubscription{value: minimumBalance}(
-            pusherParams
-        );
-        console.log("Added pusher subscription with ID:", pusherSub);
+        vm.deal(pusher, minimumBalance);
+        scheduler.addSubscription{value: minimumBalance}(pusherParams);
         vm.stopPrank();
 
         // Get active subscriptions directly - should work without any special permissions
@@ -1021,10 +1014,6 @@ contract SchedulerTest is Test, SchedulerEvents, PulseTestUtils {
 
         (activeIds, activeParams, totalCount) = scheduler
             .getActiveSubscriptions(0, 10);
-        console.log(
-            "getActiveSubscriptions succeeded, total count:",
-            totalCount
-        );
 
         // We added 3 subscriptions and all should be active
         assertEq(activeIds.length, 3, "Should have 3 active subscriptions");
@@ -1081,11 +1070,8 @@ contract SchedulerTest is Test, SchedulerEvents, PulseTestUtils {
 
         // Test pagination - start index beyond total count
         vm.prank(owner);
-        (
-            uint256[] memory emptyPageIds,
-            SchedulerState.SubscriptionParams[] memory emptyPageParams,
-            uint256 emptyPageTotal
-        ) = scheduler.getActiveSubscriptions(10, 10);
+        (uint256[] memory emptyPageIds, , uint256 emptyPageTotal) = scheduler
+            .getActiveSubscriptions(10, 10);
 
         assertEq(
             emptyPageIds.length,