Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion target_chains/ethereum/contracts/contracts/pulse/Scheduler.sol
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@ abstract contract Scheduler is IScheduler, SchedulerState {
revert InsufficientBalance();
}

// Check deposit limit for permanent subscriptions
if (subscriptionParams.isPermanent && msg.value > MAX_DEPOSIT_LIMIT) {
revert MaxDepositLimitExceeded();
}

// Set subscription to active
subscriptionParams.isActive = true;

Expand Down Expand Up @@ -523,7 +528,22 @@ abstract contract Scheduler is IScheduler, SchedulerState {
revert InactiveSubscription();
}

_state.subscriptionStatuses[subscriptionId].balanceInWei += msg.value;
SubscriptionParams storage params = _state.subscriptionParams[
subscriptionId
];
SubscriptionStatus storage status = _state.subscriptionStatuses[
subscriptionId
];

// Check deposit limit for permanent subscriptions
if (
params.isPermanent &&
status.balanceInWei + msg.value > MAX_DEPOSIT_LIMIT
) {
revert MaxDepositLimitExceeded();
}

status.balanceInWei += msg.value;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The check should be on the incoming deposit, not on the balance

}

function withdrawFunds(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,4 @@ error DuplicateWhitelistAddress(address addr);

// Payment errors
error KeeperPaymentFailed();
error MaxDepositLimitExceeded();
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ contract SchedulerState {
uint8 public constant MAX_PRICE_IDS_PER_SUBSCRIPTION = 255;
/// Maximum number of addresses in the reader whitelist
uint8 public constant MAX_READER_WHITELIST_SIZE = 255;
/// Maximum deposit limit for permanent subscriptions in wei (100 ETH)
uint256 public constant MAX_DEPOSIT_LIMIT = 100 ether;

/// Maximum time in the past (relative to current block timestamp)
/// for which a price update timestamp is considered valid
Expand Down
106 changes: 106 additions & 0 deletions target_chains/ethereum/contracts/forge-test/PulseScheduler.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -779,6 +779,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 that would exceed MAX_DEPOSIT_LIMIT should fail
uint256 additionalFunds = 1 wei;
vm.deal(address(this), additionalFunds);

vm.expectRevert(
abi.encodeWithSelector(MaxDepositLimitExceeded.selector)
);
scheduler.addFunds{value: additionalFunds}(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(
Expand Down
Loading