Skip to content

Commit 25a3789

Browse files
tejasbadadaretejasbadadare
authored
refactor(pulse-scheduler): improve SubscriptionParams validation (#2594)
* feat: add permanent subscriptions, clean up tests * refactor: extract out subscription parameter validation and improve error handling * test(pulse-scheduler): add gas benchmark (#2596) * test(pulse-scheduler): add gas benchmark, refactor common helpers into util * doc: test comment * fix: import * fix: overflow, DRY * doc: comment * fix: remove unused constants * fix: merge
1 parent ca1200a commit 25a3789

File tree

7 files changed

+664
-147
lines changed

7 files changed

+664
-147
lines changed

‎target_chains/ethereum/contracts/contracts/pulse/scheduler/Scheduler.sol‎

Lines changed: 87 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -23,35 +23,8 @@ abstract contract Scheduler is IScheduler, SchedulerState {
2323
function createSubscription(
2424
SubscriptionParams memory subscriptionParams
2525
) external payable override returns (uint256 subscriptionId) {
26-
if (
27-
subscriptionParams.priceIds.length > MAX_PRICE_IDS_PER_SUBSCRIPTION
28-
) {
29-
revert TooManyPriceIds(
30-
subscriptionParams.priceIds.length,
31-
MAX_PRICE_IDS_PER_SUBSCRIPTION
32-
);
33-
}
34-
35-
// Validate update criteria
36-
if (
37-
!subscriptionParams.updateCriteria.updateOnHeartbeat &&
38-
!subscriptionParams.updateCriteria.updateOnDeviation
39-
) {
40-
revert InvalidUpdateCriteria();
41-
}
42-
43-
// If gas config is unset, set it to the default (100x multipliers)
44-
if (
45-
subscriptionParams.gasConfig.maxBaseFeeMultiplierCapPct == 0 ||
46-
subscriptionParams.gasConfig.maxPriorityFeeMultiplierCapPct == 0
47-
) {
48-
subscriptionParams
49-
.gasConfig
50-
.maxPriorityFeeMultiplierCapPct = DEFAULT_MAX_PRIORITY_FEE_MULTIPLIER_CAP_PCT;
51-
subscriptionParams
52-
.gasConfig
53-
.maxBaseFeeMultiplierCapPct = DEFAULT_MAX_BASE_FEE_MULTIPLIER_CAP_PCT;
54-
}
26+
// Validate params and set default gas config
27+
_validateAndPrepareSubscriptionParams(subscriptionParams);
5528

5629
// Calculate minimum balance required for this subscription
5730
uint256 minimumBalance = this.getMinimumBalance(
@@ -111,34 +84,8 @@ abstract contract Scheduler is IScheduler, SchedulerState {
11184
return;
11285
}
11386

114-
// Validate parameters for active or to-be-activated subscriptions
115-
if (newParams.priceIds.length > MAX_PRICE_IDS_PER_SUBSCRIPTION) {
116-
revert TooManyPriceIds(
117-
newParams.priceIds.length,
118-
MAX_PRICE_IDS_PER_SUBSCRIPTION
119-
);
120-
}
121-
122-
// Validate update criteria
123-
if (
124-
!newParams.updateCriteria.updateOnHeartbeat &&
125-
!newParams.updateCriteria.updateOnDeviation
126-
) {
127-
revert InvalidUpdateCriteria();
128-
}
129-
130-
// If gas config is unset, set it to the default (100x multipliers)
131-
if (
132-
newParams.gasConfig.maxBaseFeeMultiplierCapPct == 0 ||
133-
newParams.gasConfig.maxPriorityFeeMultiplierCapPct == 0
134-
) {
135-
newParams
136-
.gasConfig
137-
.maxPriorityFeeMultiplierCapPct = DEFAULT_MAX_PRIORITY_FEE_MULTIPLIER_CAP_PCT;
138-
newParams
139-
.gasConfig
140-
.maxBaseFeeMultiplierCapPct = DEFAULT_MAX_BASE_FEE_MULTIPLIER_CAP_PCT;
141-
}
87+
// Validate the new parameters, including setting default gas config
88+
_validateAndPrepareSubscriptionParams(newParams);
14289

14390
// Handle activation/deactivation
14491
if (!wasActive && willBeActive) {
@@ -173,6 +120,83 @@ abstract contract Scheduler is IScheduler, SchedulerState {
173120
emit SubscriptionUpdated(subscriptionId);
174121
}
175122

123+
/**
124+
* @notice Validates subscription parameters and sets default gas config if needed.
125+
* @dev This function modifies the passed-in params struct in place for gas config defaults.
126+
* @param params The subscription parameters to validate and prepare.
127+
*/
128+
function _validateAndPrepareSubscriptionParams(
129+
SubscriptionParams memory params
130+
) internal pure {
131+
// No zero‐feed subscriptions
132+
if (params.priceIds.length == 0) {
133+
revert EmptyPriceIds();
134+
}
135+
136+
// Price ID limits and uniqueness
137+
if (params.priceIds.length > MAX_PRICE_IDS_PER_SUBSCRIPTION) {
138+
revert TooManyPriceIds(
139+
params.priceIds.length,
140+
MAX_PRICE_IDS_PER_SUBSCRIPTION
141+
);
142+
}
143+
for (uint i = 0; i < params.priceIds.length; i++) {
144+
for (uint j = i + 1; j < params.priceIds.length; j++) {
145+
if (params.priceIds[i] == params.priceIds[j]) {
146+
revert DuplicatePriceId(params.priceIds[i]);
147+
}
148+
}
149+
}
150+
151+
// Whitelist size limit and uniqueness
152+
if (params.readerWhitelist.length > MAX_READER_WHITELIST_SIZE) {
153+
revert TooManyWhitelistedReaders(
154+
params.readerWhitelist.length,
155+
MAX_READER_WHITELIST_SIZE
156+
);
157+
}
158+
for (uint i = 0; i < params.readerWhitelist.length; i++) {
159+
for (uint j = i + 1; j < params.readerWhitelist.length; j++) {
160+
if (params.readerWhitelist[i] == params.readerWhitelist[j]) {
161+
revert DuplicateWhitelistAddress(params.readerWhitelist[i]);
162+
}
163+
}
164+
}
165+
166+
// Validate update criteria
167+
if (
168+
!params.updateCriteria.updateOnHeartbeat &&
169+
!params.updateCriteria.updateOnDeviation
170+
) {
171+
revert InvalidUpdateCriteria();
172+
}
173+
if (
174+
params.updateCriteria.updateOnHeartbeat &&
175+
params.updateCriteria.heartbeatSeconds == 0
176+
) {
177+
revert InvalidUpdateCriteria();
178+
}
179+
if (
180+
params.updateCriteria.updateOnDeviation &&
181+
params.updateCriteria.deviationThresholdBps == 0
182+
) {
183+
revert InvalidUpdateCriteria();
184+
}
185+
186+
// If gas config is unset, set it to the default (100x multipliers)
187+
if (
188+
params.gasConfig.maxBaseFeeMultiplierCapPct == 0 ||
189+
params.gasConfig.maxPriorityFeeMultiplierCapPct == 0
190+
) {
191+
params
192+
.gasConfig
193+
.maxPriorityFeeMultiplierCapPct = DEFAULT_MAX_PRIORITY_FEE_MULTIPLIER_CAP_PCT;
194+
params
195+
.gasConfig
196+
.maxBaseFeeMultiplierCapPct = DEFAULT_MAX_BASE_FEE_MULTIPLIER_CAP_PCT;
197+
}
198+
}
199+
176200
/**
177201
* @notice Internal helper to clear stored PriceFeed data for price IDs removed from a subscription.
178202
* @param subscriptionId The ID of the subscription being updated.
@@ -243,10 +267,11 @@ abstract contract Scheduler is IScheduler, SchedulerState {
243267

244268
// Parse price feed updates with an expected timestamp range of [-10s, now]
245269
// We will validate the trigger conditions and timestamps ourselves
246-
// using the returned PriceFeeds.
247270
uint64 curTime = SafeCast.toUint64(block.timestamp);
248271
uint64 maxPublishTime = curTime + FUTURE_TIMESTAMP_MAX_VALIDITY_PERIOD;
249-
uint64 minPublishTime = curTime - PAST_TIMESTAMP_MAX_VALIDITY_PERIOD;
272+
uint64 minPublishTime = curTime > PAST_TIMESTAMP_MAX_VALIDITY_PERIOD
273+
? curTime - PAST_TIMESTAMP_MAX_VALIDITY_PERIOD
274+
: 0;
250275
PythStructs.PriceFeed[] memory priceFeeds;
251276
uint64[] memory slots;
252277
(priceFeeds, slots) = pyth.parsePriceFeedUpdatesWithSlots{
@@ -721,8 +746,9 @@ abstract contract Scheduler is IScheduler, SchedulerState {
721746
function getMinimumBalance(
722747
uint8 numPriceFeeds
723748
) external pure override returns (uint256 minimumBalanceInWei) {
724-
// Simple implementation - minimum balance is 0.01 ETH per price feed
725-
return numPriceFeeds * 0.01 ether;
749+
// Placeholder implementation
750+
// TODO: make this governable
751+
return uint256(numPriceFeeds) * 0.01 ether;
726752
}
727753

728754
// ACCESS CONTROL MODIFIERS

‎target_chains/ethereum/contracts/contracts/pulse/scheduler/SchedulerErrors.sol‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,31 @@
22

33
pragma solidity ^0.8.0;
44

5+
// Authorization errors
6+
error Unauthorized();
7+
8+
// Subscription state errors
59
error InactiveSubscription();
610
error InsufficientBalance();
7-
error Unauthorized();
11+
error IllegalPermanentSubscriptionModification();
12+
13+
// Price feed errors
814
error InvalidPriceId(bytes32 providedPriceId, bytes32 expectedPriceId);
915
error InvalidPriceIdsLength(bytes32 providedLength, bytes32 expectedLength);
16+
error EmptyPriceIds();
17+
error TooManyPriceIds(uint256 provided, uint256 maximum);
18+
error DuplicatePriceId(bytes32 priceId);
19+
error PriceSlotMismatch();
20+
21+
// Update criteria errors
1022
error InvalidUpdateCriteria();
1123
error InvalidGasConfig();
12-
error PriceSlotMismatch();
13-
error TooManyPriceIds(uint256 provided, uint256 maximum);
1424
error UpdateConditionsNotMet();
15-
error IllegalPermanentSubscriptionModification();
1625
error TimestampOlderThanLastUpdate(
1726
uint256 providedUpdateTimestamp,
1827
uint256 lastUpdatedAt
1928
);
29+
30+
// Whitelist errors
31+
error TooManyWhitelistedReaders(uint256 provided, uint256 maximum);
32+
error DuplicateWhitelistAddress(address addr);

‎target_chains/ethereum/contracts/contracts/pulse/scheduler/SchedulerState.sol‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import "@pythnetwork/pyth-sdk-solidity/PythStructs.sol";
77
contract SchedulerState {
88
/// Maximum number of price feeds per subscription
99
uint8 public constant MAX_PRICE_IDS_PER_SUBSCRIPTION = 255;
10+
/// Maximum number of addresses in the reader whitelist
11+
uint8 public constant MAX_READER_WHITELIST_SIZE = 255;
1012
/// Default max gas multiplier
1113
uint32 public constant DEFAULT_MAX_BASE_FEE_MULTIPLIER_CAP_PCT = 10_000;
1214
/// Default max fee multiplier

0 commit comments

Comments
 (0)