Skip to content

feat(pulse-scheduler): add permanent subscriptions, allow permissionless funding, clean up tests #2593

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 5 commits into from
Apr 21, 2025
Merged

feat(pulse-scheduler): add permanent subscriptions, allow permissionless funding, clean up tests #2593

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3ef5e7f
feat: add permanent subscriptions, clean up tests
tejasbadadare Apr 17, 2025
6e4ccc2
Merge branch 'main' of github.com:pyth-network/pyth-crosschain into t…
tejasbadadare Apr 21, 2025
f2989ab
fix: disallow config and whitelist updates for permanent subs
tejasbadadare Apr 21, 2025
06833ca
fix: disallow updating isActive for permanent subscriptions
tejasbadadare Apr 21, 2025
a35f4a3
doc: clarify comment
tejasbadadare Apr 21, 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
36 changes: 33 additions & 3 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 @@ -98,6 +98,33 @@ abstract contract Scheduler is IScheduler, SchedulerState {
bool wasActive = currentParams.isActive;
bool willBeActive = newParams.isActive;

// Check for permanent subscription restrictions
if (currentParams.isPermanent) {
// Cannot disable isPermanent flag once set
if (!newParams.isPermanent) {
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
for (uint i = 0; i < currentParams.priceIds.length; i++) {
bool found = false;
for (uint j = 0; j < newParams.priceIds.length; j++) {
Copy link
Contributor

@merolish merolish Apr 21, 2025

Choose a reason for hiding this comment

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

Heh, we're stuck doing an m x n linear search in Solidity?

Copy link
Contributor Author

@tejasbadadare tejasbadadare Apr 21, 2025

Choose a reason for hiding this comment

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

we cant create in-memory hashmaps so yeah, back to the basics 🫠

if (currentParams.priceIds[i] == newParams.priceIds[j]) {
found = true;
break;
}
}
if (!found) {
revert IllegalPermanentSubscriptionModification();
}
}
}
Copy link
Contributor

@jayantk jayantk Apr 21, 2025

Choose a reason for hiding this comment

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

I think you can also effectively disable a subscription by (1) changing the readerWhitelist or (2) making the gas config so limiting that no tx ever lands on chain?

Copy link
Contributor

@jayantk jayantk Apr 21, 2025

Choose a reason for hiding this comment

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

hmm I think you could also do it by changing the updateCriteria

Copy link
Contributor Author

@tejasbadadare tejasbadadare Apr 21, 2025

Choose a reason for hiding this comment

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

yeah good points -- will disallow changing the whitelist or the config. truly "permanent."

Copy link
Contributor Author

@tejasbadadare tejasbadadare Apr 21, 2025

Choose a reason for hiding this comment

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

just realized they can also just set isActive to false 🤦 updated


// If subscription is inactive and will remain inactive, no need to validate parameters
if (!wasActive && !willBeActive) {
// Update subscription parameters
Expand Down Expand Up @@ -487,9 +514,7 @@ abstract contract Scheduler is IScheduler, SchedulerState {

/// BALANCE MANAGEMENT

function addFunds(
uint256 subscriptionId
) external payable override onlyManager(subscriptionId) {
function addFunds(uint256 subscriptionId) external payable override {
if (!_state.subscriptionParams[subscriptionId].isActive) {
revert InactiveSubscription();
}
Expand All @@ -508,6 +533,11 @@ abstract contract Scheduler is IScheduler, SchedulerState {
subscriptionId
];

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

if (status.balanceInWei < amount) {
revert InsufficientBalance();
}
Expand Down
1 change: 1 addition & 0 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 @@ -12,6 +12,7 @@ error InvalidGasConfig();
error PriceSlotMismatch();
error TooManyPriceIds(uint256 provided, uint256 maximum);
error UpdateConditionsNotMet();
error IllegalPermanentSubscriptionModification();
error TimestampOlderThanLastUpdate(
uint256 providedUpdateTimestamp,
uint256 lastUpdatedAt
Expand Down
1 change: 1 addition & 0 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 @@ -41,6 +41,7 @@ contract SchedulerState {
address[] readerWhitelist;
bool whitelistEnabled;
bool isActive;
bool isPermanent;
UpdateCriteria updateCriteria;
GasConfig gasConfig;
}
Expand Down
Loading
Loading