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.

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

Jump to bottom

MainNet proposals should dynamically provide oracle addresses #9571

Closed
Chris-Hibbert opened this issue Jun 24, 2024 · 1 comment · Fixed by #9575
Closed

MainNet proposals should dynamically provide oracle addresses #9571

Chris-Hibbert opened this issue Jun 24, 2024 · 1 comment · Fixed by #9575
Labels
bug Something isn't working contract-upgrade deployment Chain deployment mechanism (e.g. testnet) oracle Related to on-chain oracles.

Comments

@Chris-Hibbert
Copy link
Contributor

Describe the bug

#9044 included a proposal for upgrading price feeds, and included fallback oracle addresses.

While putting together the release @mhofman commented

I'm not sure how the proposal should figure out the current oracle addresses TBH, but I don't think we should bake any address in the image

Expected behavior

The proposal in #9044 tries to look up GOV1, GOV2, and GOV3 from process.env, before falling back to baked-in addresses. If we need to remove the hard-coded addresses, we need to ensure that test nets and a3p will always have the GOV addresses defined.
@Chris-Hibbert Chris-Hibbert added bug Something isn't working deployment Chain deployment mechanism (e.g. testnet) contract-upgrade oracle Related to on-chain oracles. labels Jun 24, 2024
@mhofman
Copy link
Member

mhofman commented Jun 24, 2024

To clarify, the 2 requirements are:

  • do not rely on environment variables. if used environment variables should only be used for some kind of override mechanism
  • have a single image be compatible with all existing networks that already have oracles set, such as mainnet, Devnet etc. these networks cannot expect validators to set environment variables for correctness.

@mhofman mhofman mentioned this issue Jun 26, 2024
Merged
@mergify mergify bot closed this as completed in #9575 Jun 26, 2024
@mergify mergify bot closed this as completed in cbe061c Jun 26, 2024
mhofman pushed a commit that referenced this issue Jun 26, 2024
@mergify @mhofman
feat: make software upgrade coreProposals conditional on upgrade pl…
079b8a6 
…an name (#9575)

closes: #9571

## Description

Parametrize the price feed core proposals by the upgrade handler name. The proposal for a3p remains with the deprecated ambient (from environment) oracle addresses.

Additionally fix and add some logging to the upgrade vault proposal

### Security Considerations
None

### Scaling Considerations
Not a typical scaling consideration, but this introduces a "basic" upgrade name that skips all vaults related proposals. The correctly parametrized info can be provided in the chain software upgrade proposal's upgrade info for any chain that has different configuration.

### Documentation Considerations
Need to communicate to validators that the pattern of upgrade names has changed.

### Testing Considerations
A3P and manually tested on mainfork

### Upgrade Considerations
See above
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working contract-upgrade deployment Chain deployment mechanism (e.g. testnet) oracle Related to on-chain oracles.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: make software upgrade coreProposals conditional on upgrade plan name Agoric/agoric-sdk
2 participants
@mhofman @Chris-Hibbert