Skip to content
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.

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

chore: upgrade 18 cherrypicks round 4 #10645

Merged
merged 5 commits into from
Dec 9, 2024

Conversation

mujahidkay
Copy link
Member

@mujahidkay mujahidkay commented Dec 9, 2024

Description

Cherry-picks the following commits from master:

Since we plan to verify this rc on devnet rather than emerynet, there is no apparent need for a new upgrade name. Not aware of any deployments on devnet before so can reuse the previous upgrade name. However, skipping emerynet is dependent on comms with the validators so I have added a new upgrade name agoric-ugprade-18-emerynet-rc3 just in case. Only added to bypass the need for a new rc if for some reason emerynet validation is needed anyways - best case scenario is that it remains unused.

commits added using git cherry-pick

mujahidkay and others added 4 commits December 9, 2024 11:40
)

## Description

It was observed, after applying upgrade 18 to EmeryNet, that a whole slew of promises resolved when a price was provided. The current belief is that `observeQuoteNotifier` will be called every auction round until a price is available, and that creates a new observer with each call that wait until a price is published, and then they all continue waiting for each successive update.

This change adds an interlock, so if there's already a notifier waiting, we don't add a new one.

### Security Considerations

No security implication.

### Scaling Considerations

Processing about 19000 actions waiting on a notifier in EmeryNet took several hours. If we're correct that the notifiers will continue to cycle, we expect to see a similar wait for each price update on that currency. That's unsustainable.

The only current theory about dropping all those actions waiting for notifiers is to kill the vat. We can't kill the priceAuthority vats that hold the notifiers, but we might be able to cleanly kill the abandoned auctioneers.

### Documentation Considerations

Not needed.

### Testing Considerations

Tough to test in unit tests. It's conceivable that we could recreate the situation in `a3p-integration`, though it would be hard to observe the results.

### Upgrade Considerations

We probably shouldn't ship upgrade 18 with something to address this problem.
@mujahidkay mujahidkay requested a review from a team as a code owner December 9, 2024 06:44
Copy link

cloudflare-workers-and-pages bot commented Dec 9, 2024

Deploying agoric-sdk with  Cloudflare Pages  Cloudflare Pages

Latest commit: 4292ab5
Status: ✅  Deploy successful!
Preview URL: https://87d8dd42.agoric-sdk.pages.dev
Branch Preview URL: https://mk-u18-rc3-cherrypicks.agoric-sdk.pages.dev

View logs

@mujahidkay mujahidkay changed the base branch from master to dev-upgrade-18 December 9, 2024 06:49
@mujahidkay mujahidkay self-assigned this Dec 9, 2024
@Agoric Agoric deleted a comment from mergify bot Dec 9, 2024
@mujahidkay mujahidkay added the force:integration Force integration tests to run on PR label Dec 9, 2024
@mujahidkay mujahidkay force-pushed the mk/u18-rc3-cherrypicks branch from 2c5ab2c to 4292ab5 Compare December 9, 2024 07:10
@mujahidkay mujahidkay requested a review from mhofman December 9, 2024 11:10
@mujahidkay mujahidkay merged commit 89128d3 into dev-upgrade-18 Dec 9, 2024
78 checks passed
@mujahidkay mujahidkay deleted the mk/u18-rc3-cherrypicks branch December 9, 2024 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
force:integration Force integration tests to run on PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants