Skip to content

fix(pulse-scheduler): disallow all updates to permanent subs, remove GasConfig #2602

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.

Sign up for GitHub

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
merged 3 commits into from
Apr 23, 2025
Merged

fix(pulse-scheduler): disallow all updates to permanent subs, remove GasConfig #2602

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9ae2857
feat: disallow any config change to permanent feeds, remove gasconfig
tejasbadadare Apr 22, 2025
db103ef
doc: remove unnecessary comments
tejasbadadare Apr 22, 2025
4c94307
fix: consistency
tejasbadadare Apr 22, 2025
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
130 changes: 11 additions & 119 deletions target_chains/ethereum/contracts/contracts/pulse/scheduler/Scheduler.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,19 +64,21 @@ abstract contract Scheduler is IScheduler, SchedulerState {
uint256 subscriptionId,
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;
SubscriptionStatus storage currentStatus = _state.subscriptionStatuses[
subscriptionId
];
SubscriptionParams storage currentParams = _state.subscriptionParams[
subscriptionId
];

// Check for permanent subscription restrictions
// Updates to permanent subscriptions are not allowed
if (currentParams.isPermanent) {
_validatePermanentSubscriptionUpdate(currentParams, newParams);
revert CannotUpdatePermanentSubscription();
}

// If subscription is inactive and will remain inactive, no need to validate parameters
bool wasActive = currentParams.isActive;
bool willBeActive = newParams.isActive;
if (!wasActive && !willBeActive) {
// Update subscription parameters
_state.subscriptionParams[subscriptionId] = newParams;
Expand Down Expand Up @@ -182,19 +184,6 @@ abstract contract Scheduler is IScheduler, SchedulerState {
) {
revert InvalidUpdateCriteria();
}

// If gas config is unset, set it to the default (100x multipliers)
if (
params.gasConfig.maxBaseFeeMultiplierCapPct == 0 ||
params.gasConfig.maxPriorityFeeMultiplierCapPct == 0
) {
params
.gasConfig
.maxPriorityFeeMultiplierCapPct = DEFAULT_MAX_PRIORITY_FEE_MULTIPLIER_CAP_PCT;
params
.gasConfig
.maxBaseFeeMultiplierCapPct = DEFAULT_MAX_BASE_FEE_MULTIPLIER_CAP_PCT;
}
}

/**
Expand Down Expand Up @@ -415,103 +404,6 @@ abstract contract Scheduler is IScheduler, SchedulerState {
revert UpdateConditionsNotMet();
}

/**
* @notice Internal helper to validate modifications to a permanent subscription.
* @param currentParams The current subscription parameters (storage).
* @param newParams The proposed new subscription parameters (memory).
*/
function _validatePermanentSubscriptionUpdate(
SubscriptionParams storage currentParams,
SubscriptionParams memory newParams
) internal view {
// Cannot disable isPermanent flag once set
if (!newParams.isPermanent) {
revert IllegalPermanentSubscriptionModification();
}

// Cannot deactivate a permanent subscription
if (!newParams.isActive) {
revert IllegalPermanentSubscriptionModification();
}

// Cannot remove price feeds from a permanent subscription
if (newParams.priceIds.length < currentParams.priceIds.length) {
revert IllegalPermanentSubscriptionModification();
}

// Check that all existing price IDs are preserved (adding is allowed, not removing)
for (uint i = 0; i < currentParams.priceIds.length; i++) {
bool found = false;
for (uint j = 0; j < newParams.priceIds.length; j++) {
if (currentParams.priceIds[i] == newParams.priceIds[j]) {
found = true;
break;
}
}
if (!found) {
revert IllegalPermanentSubscriptionModification();
}
}

// Cannot change reader whitelist settings for permanent subscriptions
if (newParams.whitelistEnabled != currentParams.whitelistEnabled) {
revert IllegalPermanentSubscriptionModification();
}

// Check if the set of addresses in the whitelist is the same
if (
newParams.readerWhitelist.length !=
currentParams.readerWhitelist.length
) {
revert IllegalPermanentSubscriptionModification();
}
uint256 n = newParams.readerWhitelist.length;
bool[] memory currentVisited = new bool[](n);
uint256 matchesFound = 0;
for (uint256 i = 0; i < n; i++) {
bool foundInCurrent = false;
for (uint256 j = 0; j < n; j++) {
if (
!currentVisited[j] &&
newParams.readerWhitelist[i] ==
currentParams.readerWhitelist[j]
) {
currentVisited[j] = true;
foundInCurrent = true;
matchesFound++;
break;
}
}
if (!foundInCurrent) {
revert IllegalPermanentSubscriptionModification();
}
}

// Cannot change update criteria for permanent subscriptions
if (
newParams.updateCriteria.updateOnHeartbeat !=
currentParams.updateCriteria.updateOnHeartbeat ||
newParams.updateCriteria.heartbeatSeconds !=
currentParams.updateCriteria.heartbeatSeconds ||
newParams.updateCriteria.updateOnDeviation !=
currentParams.updateCriteria.updateOnDeviation ||
newParams.updateCriteria.deviationThresholdBps !=
currentParams.updateCriteria.deviationThresholdBps
) {
revert IllegalPermanentSubscriptionModification();
}

// Cannot change gas config for permanent subscriptions
if (
newParams.gasConfig.maxBaseFeeMultiplierCapPct !=
currentParams.gasConfig.maxBaseFeeMultiplierCapPct ||
newParams.gasConfig.maxPriorityFeeMultiplierCapPct !=
currentParams.gasConfig.maxPriorityFeeMultiplierCapPct
) {
revert IllegalPermanentSubscriptionModification();
}
}

/// FETCH PRICES

/**
Expand Down Expand Up @@ -635,7 +527,7 @@ abstract contract Scheduler is IScheduler, SchedulerState {

// Prevent withdrawals from permanent subscriptions
if (params.isPermanent) {
revert IllegalPermanentSubscriptionModification();
revert CannotUpdatePermanentSubscription();
}

if (status.balanceInWei < amount) {
Expand Down
3 changes: 1 addition & 2 deletions target_chains/ethereum/contracts/contracts/pulse/scheduler/SchedulerErrors.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ error Unauthorized();
// Subscription state errors
error InactiveSubscription();
error InsufficientBalance();
error IllegalPermanentSubscriptionModification();
error CannotUpdatePermanentSubscription();

// Price feed errors
error InvalidPriceId(bytes32 providedPriceId, bytes32 expectedPriceId);
Expand All @@ -20,7 +20,6 @@ error PriceSlotMismatch();

// Update criteria errors
error InvalidUpdateCriteria();
error InvalidGasConfig();
error UpdateConditionsNotMet();
error TimestampOlderThanLastUpdate(
uint256 providedUpdateTimestamp,
Expand Down
18 changes: 0 additions & 18 deletions target_chains/ethereum/contracts/contracts/pulse/scheduler/SchedulerState.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,6 @@ 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;
/// Default max gas multiplier
uint32 public constant DEFAULT_MAX_BASE_FEE_MULTIPLIER_CAP_PCT = 10_000;
/// Default max fee multiplier
uint32 public constant DEFAULT_MAX_PRIORITY_FEE_MULTIPLIER_CAP_PCT = 10_000;

// TODO: make these updateable via governance
/// Maximum time in the past (relative to current block timestamp)
Expand Down Expand Up @@ -45,7 +41,6 @@ contract SchedulerState {
bool isActive;
bool isPermanent;
UpdateCriteria updateCriteria;
GasConfig gasConfig;
}

struct SubscriptionStatus {
Expand All @@ -55,19 +50,6 @@ contract SchedulerState {
uint256 totalSpent;
}

/// @dev When pushing prices, providers will use a "fast gas" estimation as default.
/// If the gas is insufficient to land the transaction, the provider will linearly scale
/// base fee and priority fee multipliers until the transaction lands.
/// These parameters allow the subscriber to impose limits on these multipliers.
/// For example, with maxBaseFeeMultiplierCapPct = 10_000 (default), the provider can
/// use a max of 100x (10000%) of the estimated gas as reported by the RPC.
struct GasConfig {
/// Base gas fee price multiplier limit percent for update operations
uint32 maxBaseFeeMultiplierCapPct;
/// Priority fee multiplier limit for update operations
uint32 maxPriorityFeeMultiplierCapPct;
}

struct UpdateCriteria {
bool updateOnHeartbeat;
uint32 heartbeatSeconds;
Expand Down
Loading
Loading