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

Plan for replacing priceFeeds, redirecting oracles #8740

Closed
Chris-Hibbert opened this issue Jan 10, 2024 · 17 comments · Fixed by #9283
Closed

Plan for replacing priceFeeds, redirecting oracles #8740

Chris-Hibbert opened this issue Jan 10, 2024 · 17 comments · Fixed by #9283
Assignees
Labels
contract-upgrade enhancement New feature or request oracle Related to on-chain oracles. performance Performance related issues

Comments

@Chris-Hibbert
Copy link
Contributor

Chris-Hibbert commented Jan 10, 2024

What is the Problem Being Solved?

#8400 explains that priceFeed vats are holding onto quote payments via recoverySets, creating an uncollectible cycle. In order to stop adding to the growing amount of garbage, we need to make the Auction and VaultFactory rely on a different priceFeed. Once the clients are pointed to a different source, we can decide when to kill those vats to cause the garbage to be freed.

Description of the Design

In order to switch to new priceFeeds, we need a plan that describes what it takes to redirect the oracles so we can replace the priceFeeds in the priceAuthorityRegistry.

Replacing the priceFeeds should be simple: the priceAuthorityRegistry has registerPriceAuthority(pa, brandIn, brandOut, force). If force = true, the old one is dropped.

Security Considerations

More robustness, rather than a security issue.

Scaling Considerations

Garbage is accumulating.

Test Plan

Test in a3p to ensure the new fluxAggregator is getting price data.

Upgrade Considerations

Do we need new code anywhere in order to redirect oracles?

@Chris-Hibbert Chris-Hibbert added enhancement New feature or request performance Performance related issues contract-upgrade oracle Related to on-chain oracles. labels Jan 10, 2024
@Chris-Hibbert
Copy link
Contributor Author

I should also have linked #8498, which is the change to ERTP and priceFeeds to stop adding the priceQuotes to recoverySets.

@dckc
Copy link
Member

dckc commented Jan 10, 2024

price-feed-proposal.js seems to have the relevant pieces.

More later...

@Chris-Hibbert
Copy link
Contributor Author

To the extent I understand that script, it looks like it's doing the initial set-up of oracles and priceFeeds. I presume that the process is different if the oracles already exist. Does it make more sense to create new oracles than to redirect existing ones?

@dckc
Copy link
Member

dckc commented Jan 10, 2024

I presume that the process is different if the oracles already exist.

I'm studying the code, looking for reasons why it wouldn't also work for the case where the oracles already exist, and I haven't seen one yet.

It's possible that the plan is as simple as:

  1. replace the priceAggregator installation in agoricNames
  2. run the price feed proposal again, which sends out new invitations to oracle operators
  3. have the operators exercise their new invitations so that they're talking to the new contract instances

@dckc
Copy link
Member

dckc commented Jan 10, 2024

instance, installation not updating in agoricNames

before and after the core-eval, I see the same board id for the installation and instance:

$ endo eval "E(wk.query).lookup('agoricNames', 'instance', 'stATOM-USD price feed')" wk:alice-wk
Object [Alleged: InstanceHandle#board00694] {}

$ endo eval "E(wk.query).lookup('agoricNames', 'installation', 'priceAggregator')" wk:alice-wk
Object [Alleged: BundleIDInstallation#board02021] {}
core eval details

Using an a3p / dapp-offer-up local chain, I submitted add-stATOM-oracles.js and add-stATOM-oracles-permit.json using the
cosgov CoreEval form.

app-offer-up-agd-1  | 2024-01-10T22:18:06.660Z SwingSet: vat: v1: evaluateBundleCap { manifestBundleRef: { bundleID: 'b1-80e6fe68b299c82c2d26802c312bc37966a559f7b28f87d058887a79a9db48ad97da2240e71e3f98986071da8fc3c5d02358bec577b17a89cee2b1cb3cd23958' }, exportedGetManifest: 'getManifestForPriceFeed', vatAdminSvc: Promise [Promise] {} }
dapp-offer-up-agd-1  | 2024-01-10T22:18:07.008Z SwingSet: vat: v1: execute { exportedGetManifest: 'getManifestForPriceFeed', behaviors: [ 'createPriceFeed', 'ensureOracleBrands', 'getManifestForPriceFeed', 'instanceNameFor', 'startPriceFeeds' ] }
dapp-offer-up-agd-1  | 2024-01-10T22:18:07.008Z SwingSet: vat: v1: coreProposal: createPriceFeed
dapp-offer-up-agd-1  | 2024-01-10T22:18:07.009Z SwingSet: vat: v1: ----- RunPriceFeed.2  2 createPriceFeed
dapp-offer-up-agd-1  | 2024-01-10T22:18:07.009Z SwingSet: vat: v1: coreProposal: ensureOracleBrands
dapp-offer-up-agd-1  | 2024-01-10T22:18:07.009Z SwingSet: vat: v1: ----- RunPriceFeed.2  3 ensureOracleBrands
dapp-offer-up-agd-1  | 2024-01-10T22:18:07.071Z SwingSet: vat: v1: ----- RunPriceFeed.2  4 got terms
dapp-offer-up-agd-1  | 2024-01-10T22:18:07.080Z SwingSet: vat: v1: ----- RunPriceFeed.2  5 awaiting startInstance
dapp-offer-up-agd-1  | 2024-01-10T22:18:10.192Z SwingSet: vat: v1: ----- RunPriceFeed.2  6 registered stATOM-USD price feed Object [Alleged: InstanceHandle] {}
dapp-offer-up-agd-1  | 2024-01-10T22:18:10.192Z SwingSet: vat: v1: ----- RunPriceFeed.2  7 distributing invitations [ 'agoric1krunjcqfrf7la48zrvdfeeqtls5r00ep68mzkr', 'agoric19uscwxdac6cf6z7d5e26e0jm0lgwstc47cpll8', 'agoric144rrhh4m09mh7aaffhm6xy223ym76gve2x7y78', 'agoric19d6gnr9fyp6hev4tlrg87zjrzsd5gzr5qlfq2p', 'agoric1n4fcxsnkxe4gj6e24naec99hzmc4pjfdccy5nj' ]
dapp-offer-up-agd-1  | 2024-01-10T22:18:10.192Z SwingSet: vat: v1: ----- RunPriceFeed.2  8 createPriceFeed complete
dapp-offer-up-agd-1  | 2024-01-10T22:18:10.199Z SwingSet: vat: v15: charter: adding instance stATOM-USD price feed
dapp-offer-up-agd-1  | 2024-01-10T22:18:10.204Z SwingSet: vat: v60: ----- FlxAgg.7  2 makeOracleInvitation agoric1krunjcqfrf7la48zrvdfeeqtls5r00ep68mzkr
dapp-offer-up-agd-1  | 2024-01-10T22:18:10.207Z SwingSet: vat: v60: ----- FlxAgg.7  3 makeOracleInvitation agoric19uscwxdac6cf6z7d5e26e0jm0lgwstc47cpll8
dapp-offer-up-agd-1  | 2024-01-10T22:18:10.211Z SwingSet: vat: v60: ----- FlxAgg.7  4 makeOracleInvitation agoric144rrhh4m09mh7aaffhm6xy223ym76gve2x7y78
dapp-offer-up-agd-1  | 2024-01-10T22:18:10.214Z SwingSet: vat: v60: ----- FlxAgg.7  5 makeOracleInvitation agoric19d6gnr9fyp6hev4tlrg87zjrzsd5gzr5qlfq2p
dapp-offer-up-agd-1  | 2024-01-10T22:18:10.217Z SwingSet: vat: v60: ----- FlxAgg.7  6 makeOracleInvitation agoric1n4fcxsnkxe4gj6e24naec99hzmc4pjfdccy5nj
dapp-offer-up-agd-1  | 2024-01-10T22:18:10.349Z SwingSet: vat: v1: awaiting depositFacet for stATOM-USD price feed member agoric1krunjcqfrf7la48zrvdfeeqtls5r00ep68mzkr
dapp-offer-up-agd-1  | 2024-01-10T22:18:10.352Z SwingSet: vat: v1: awaiting depositFacet for stATOM-USD price feed member agoric19uscwxdac6cf6z7d5e26e0jm0lgwstc47cpll8
dapp-offer-up-agd-1  | 2024-01-10T22:18:10.354Z SwingSet: vat: v1: awaiting depositFacet for stATOM-USD price feed member agoric144rrhh4m09mh7aaffhm6xy223ym76gve2x7y78
dapp-offer-up-agd-1  | 2024-01-10T22:18:10.356Z SwingSet: vat: v1: awaiting depositFacet for stATOM-USD price feed member agoric19d6gnr9fyp6hev4tlrg87zjrzsd5gzr5qlfq2p
dapp-offer-up-agd-1  | 2024-01-10T22:18:10.358Z SwingSet: vat: v1: awaiting depositFacet for stATOM-USD price feed member agoric1n4fcxsnkxe4gj6e24naec99hzmc4pjfdccy5nj
dapp-offer-up-agd-1  | 2024-01-10T22:18:10.402Z block-manager: block 81186 commit
dapp-offer-up-agd-1  | 2024-01-10T22:18:10.490Z block-manager: block 81187 begin

@dckc
Copy link
Member

dckc commented Jan 11, 2024

Since the existing price-feed-proposal.js doesn't cut it, I'm tempted to build a contract to do it. That is: I started building such a contract in hackathon mode, and now I'm tempted to pursue the idea in production:

p.s. idea: merge this with the econCommitteeCharter? maybe not... this one needs a lot of power, and that one doesn't... or does it?

@Chris-Hibbert
Copy link
Contributor Author

@dckc I created #8868 to track the tasks I need to take care of in order to upgrade Zoe, Price feeds, Auction, and Vaults for upgrade-15. When I started thinking about replacing the price feeds and providing them to the Auction and Vaults as part of the upgrade, I remembered this discussion, and took another look.

Re-reading it, I see one of the steps is have the operators exercise their new invitations. Does this require manual intervention on the operators' part? If so, upgrade-15 might include creating the new price feeds, but the Auction and Vaults would have to be restarted using the existing priceAuthorities because the new ones wouldn't get any prices until some time later. I'm pretty sure that auctions and vaults assume there will be a current price when they first talk to a priceAuthority.

@dckc
Copy link
Member

dckc commented Feb 8, 2024

I see one of the steps is

have the operators exercise their new invitations
Does this require manual intervention on the operators' part?

Yes.

If so, upgrade-15 might include creating the new price feeds, but the Auction and Vaults would have to be restarted using the existing priceAuthorities because the new ones wouldn't get any prices until some time later. I'm pretty sure that auctions and vaults assume there will be a current price when they first talk to a priceAuthority.

I wonder how they (auctions and vaults) can tell what the priceAuthority does to answer their request for a quote. For all they know, doesn't it wait for operators to submit prices?

@Chris-Hibbert
Copy link
Contributor Author

I wonder how they (auctions and vaults) can tell what the priceAuthority does to answer their request for a quote. For all they know, doesn't it wait for operators to submit prices?

In the steady state, all they know is the last quote they received. They could check to see how old it is, but they don't currently.

During startup or after upgrade, they're dependent on getting an initial price. Without it, they don't know how much collateral is worth, when/whether to liquidate, or what prices to charge in the auction.

Perhaps it would be worth making the fixes that would be required to make the auction and vaultFactory be robust in the face of missing quotes, but we haven't chosen to do that yet. In the meantime, the auction and the vaultFactory expect the priceAuthority to have started before them.

@dckc
Copy link
Member

dckc commented Feb 10, 2024

... During startup or after upgrade, they're dependent on getting an initial price.

Right... but the way they get that initial price is by making an eventual send to a price authority, right? And for all they know, the price authority is a price aggregator, and the eventual send doesn't complete until enough operators submit prices that the price aggregator reports a price, right?

@warner
Copy link
Member

warner commented Feb 21, 2024

From today's kernel meeting:

8400-recovery-set-fix

There are three on-chain events that must happen:

  • first, instantiate new price-feed vats
    • this requires the new price-feed bundles to be installed on the chain, either through a $20 txn or embedded in a chain-software upgrade
    • this will automatically send new invitations to the oracles
  • second, instantiate the new auctioneer vat, configured to use the price-feed vat
  • third, upgrade the Vaults vat, with configuration to use the new auctioneer

And there are three off-chain events that must happen:

  • oracles must accept their new invitations (cannot happen until after the new price-feed vats create those invitaions)
  • oracles must start sending data to the new price feed vats
    • we want this to happen before we upgrade the vaults vat, to avoid a gap in liquidation coverage
  • oracles must eventually stop sending data to the old price feed vats
    • this should wait until the new Vaults is up and running, and receiving new price-feed data, to avoid the same gap

The dependencies mean we split the on-chain events into two phases. The first phase deploys the new price-feed vats and also starts the new auctioneer vat. The second phase upgrades the Vaults vat. The phases are separated by off-chain oracle activity, so the overall sequence will be:

  • first on-chain phase: deploy new price-feed vats, deploy new auctioneer vat
  • wait for oracles to accept invitations and start sending data to the new price feeds
    • oracles will be sending data to both new and old price feeds for a while
  • now trigger the second on-chain phase: upgrade Vaults vat
  • after that finishes, tell oracles to stop sending data to the old price feeds

The #8400 -related storage growth (Quote Payments / recovery set, roughly half of the total) will stop once the oracles stop sending data to the old price feeds. Later, after we can survive it, we can terminate the old price feed vats, to actually reclaim the DB space consumed by the earlier growth (after #8928 is implemented and deployed).

Note that the two on-chain phases could either be core-eval proposals (which only require a governance vote), or as chain-software upgrades (which require a governance vote, a chain halt, and validators to install new software). Installing the bundles is slightly cheaper ($20 each) to do during a chain-software upgrade, but not enough to drive the decision.

The first step is to land a PR that makes the recovery sets optional. #8418 or #8498 would do, but @Chris-Hibbert pointed out that it would be smaller/safer to make a new PR which omits the delete-the-old-set code entirely (since we can't take advantage of that feature, since we're replacing the price-feed vats instead of upgrading them). So the real first step is for him to make a smaller branch and get it landed. We also need to change the price-feed contracts to set the option correctly.

We believe that the existing on-chain auctioneer vat bundle is adequate (TODO @Chris-Hibbert: are there upgrade-enabling fixes we should deploy?), so we don't need any new code for it. Therefore, once the price-feed code isn't using recovery sets, we could cut bundles and install them to chain via txns. Then, we could make a core-eval proposal to deploy those bundles, and then wait for oracles to make their changes.

Interaction with Zoe-Cycle Fixes

We also examined the changes we want to address #8401. We are close to having Zoe fixes that stop leaving cycles around for old (exited) seats, which will address the majority of the problem. Once deployed, this will remove the other half of the kvStore growth rate.

8401-zoe-cycle-fix

PR #8697 breaks these cycles upon seat exit. That has been merged into a branch that is destined (via #8682) for landing in master, and is tenatively scheduled for chain-upgrade upgrade-15.

Given the timing, it might make sense to perform both the Zoe upgrade and the price-feed/auctioneer deployments together, in upgrade-15. Then we could perform the Vaults upgrade in a core-eval proposal, after oracles accept their invitations and start sending data to the new price-feed vats.

But, upgrade-15 might get commandeered for other purposes (vat-network groundwork to enable IBC orchestration features), so the Zoe upgrade might get kicked out. If that happens, we should consider doing the Zoe upgrade as part of a core-eval proposal instead of a chain-software upgrade. We're worried about validator fatigue if the chain-software upgrades happen too quickly, but we're also worried about stopping the kvStore growth as quickly as possible (especially since we just doubled the growth rate by deploying stOSMO and stTIA). Using a core-eval proposal, which requires a stakeholder governance vote but not validator involvement, might be a good compromise.

After the dust has settled, we can think about how we upgrade Zoe to avoid creating cycles in the first place (even for non-exited seats). Later still, we can figure out how to remediate the existing cycles (most of which will go away when we terminate the old price-feed vats, but some will still be left from non- price-feed contracts).

@Chris-Hibbert
Copy link
Contributor Author

Right... but the way they get that initial price is by making an eventual send to a price authority, right? And for all they know, the price authority is a price aggregator, and the eventual send doesn't complete until enough operators submit prices that the price aggregator reports a price, right?

That's all true, but the consequences are that the Vaults and Auction wouldn't get a price update until the oracles are completely ready, which means that our protection from price fluctuations would be inoperable until the priceFeeds come back. I'd be fine with this theory as a product decision, but absent that, I'd like to find a way to not switch Auctions and Vaults to new feeds until the new priceFeeds are functional.

@Chris-Hibbert
Copy link
Contributor Author

before and after the core-eval, I see the same board id for the installation and instance:

I've looked at the transcript above, and price-feed-proposal.js, and it looks to me as if it's doing the right thing, down to

  await E(priceAuthorityAdmin).registerPriceAuthority(
    E(faKit.publicFacet).getPriceAuthority(),
    brandIn,
    brandOut,
    forceReplace,
  );

I'm not convinced it won't work. I'd like to see if the resulting stATOM-USD price feed is connected to a new or old vat, and whether the oracles can feed prices to that priceAuthority.

Another thing I'm looking for is a list of all the priceFeeds and the parameters used to create them. Are those collected in one place?

@dckc
Copy link
Member

dckc commented Mar 4, 2024

... Another thing I'm looking for is a list of all the priceFeeds and the parameters used to create them. Are those collected in one place?

One list is published.priceFeed

  • ATOM-USD_price_feed
  • stATOM-USD_price_feed
  • stOSMO-USD_price_feed
  • stkATOM-USD_price_feed

The parameters used to create them.... let's see... I suppose they're all in a3p somewhere... but... ATOM-USD was part of the vaults launch, so...

{
"module": "@agoric/inter-protocol/scripts/price-feed-core.js",
"entrypoint": "defaultProposalBuilder",
"args": [
{
"contractTerms": {
"POLL_INTERVAL": 30,
"maxSubmissionCount": 1000,
"minSubmissionCount": 3,
"restartDelay": 1,
"timeout": 10,
"minSubmissionValue": 1,
"maxSubmissionValue": 9007199254740991
},
"AGORIC_INSTANCE_NAME": "ATOM-USD price feed",
"oracleAddresses": [
"agoric1krunjcqfrf7la48zrvdfeeqtls5r00ep68mzkr",
"agoric19uscwxdac6cf6z7d5e26e0jm0lgwstc47cpll8",
"agoric144rrhh4m09mh7aaffhm6xy223ym76gve2x7y78",
"agoric19d6gnr9fyp6hev4tlrg87zjrzsd5gzr5qlfq2p",
"agoric1n4fcxsnkxe4gj6e24naec99hzmc4pjfdccy5nj"
],
"IN_BRAND_LOOKUP": [
"agoricNames",
"oracleBrand",
"ATOM"
],
"IN_BRAND_DECIMALS": 6,
"OUT_BRAND_LOOKUP": [
"agoricNames",
"oracleBrand",
"USD"
],
"OUT_BRAND_DECIMALS": 4
}
]
},

The others are from core-eval proposals.

  • stATOM was #55:
  • stOSMO was #68
  • stTIA was #69

stATOM proposal details include:

  {
    AGORIC_INSTANCE_NAME: "stATOM-USD price feed",
    IN_BRAND_DECIMALS: 6,
    IN_BRAND_LOOKUP: ["agoricNames", "oracleBrand", "stATOM"],
    IN_BRAND_NAME: "stATOM",
    OUT_BRAND_DECIMALS: 4,
    OUT_BRAND_LOOKUP: ["agoricNames", "oracleBrand", "USD"],
    OUT_BRAND_NAME: "USD",
    brandInRef: undefined,
    brandOutRef: undefined,
    contractTerms: {
      POLL_INTERVAL: 30n,
      maxSubmissionCount: 1000,
      maxSubmissionValue:
        115792089237316195423570985008687907853269984665640564039457584007913129639936n,
      minSubmissionCount: 2,
      minSubmissionValue: 1n,
      restartDelay: 1,
      timeout: 10,
    },

@Chris-Hibbert
Copy link
Contributor Author

Thanks.

I found details for stATOM, ATOM, and stTIA. I'll verify with the above, and add stOSMO from the above link.

@dckc
Copy link
Member

dckc commented Mar 6, 2024

brands are added to agoricNames.brand in publishInterchainAssetFromBank; registerScaledPriceAuthority adds to agoricNames.oracleBrand.

They're in addAssetToVault.js. That link goes to master. I don't think they have changed significantly, but to find the code that's on chain, we have to trace back from

const manifestBundleRef = {
  bundleID:
    "b1-903e41a7c448a41b456298404a1c32c69302574209c6a5228723ed19e2dd99f2a693641196445bc27a90e19e1dfadfe6b3d9c9a93f080ffa33a70908e5af4fff",
};

to https://github.com/0xpatrickdev/agoric-vault-collateral-proposal/tree/main/bundles

@dckc
Copy link
Member

dckc commented Mar 7, 2024

  • oracles will be sending data to both new and old price feeds for a while

@Chris-Hibbert , I was thinking that this would require new tooling... but I was thinking that each time they pushed a price, they specified a pair (such as --pair ATOM.USD), which would get looked up in published.priceFeed.${pair[0]}-${pair[1]}_price_feed to get (board IDs for) the relevant aggregator contract instance. I thought we would need new tooling to skip this lookup for old price feeds and just use known boardIDs of old instances.

But on review of our agops oracle code, I see that's not how it works.

When they push a price, they specify the relevant contract using --oracleAdminAcceptOfferId; i.e. by reference to when they accepted an invitation to that contract.

To push to both new and old price feeds, they can just accept an invitation to the new feed and use agops oracle pushPriceRound --oracleAdminAcceptOfferId OLD and agops oracle pushPriceRound --oracleAdminAcceptOfferId NEW.

Perhaps their tooling needs to be enhanced to deal with this situation, but agops oracle already handles it.

mergify bot added a commit that referenced this issue Apr 12, 2024
…ation. (#9093)

refs: #8740

## Description

Improved description of creating proposals and submissions in
a3p-integration

### Security Considerations

None

### Scaling Considerations

None

### Documentation Considerations

That's it.

### Testing Considerations

None

### Upgrade Considerations

None

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
mergify bot added a commit that referenced this issue Apr 29, 2024
refs: #8740
closes: #8918
refs: #8400

## Description

Add a new Auction instance in A3P, so #8757 can make use of it. Also
provides upgrade proposals which can be be applied to MainNet and other
chains.

### Security Considerations

N/A

### Scaling Considerations

This is largely in service of #8400, which reports that priceFeed vats
are accumulating garbage. This PR gives a new auction which can rely on
new priceFeeds. The existing auction is not upgradeable and its
pricefeeds can't be updated.

### Documentation Considerations

No user-visible changes to behavior.

### Testing Considerations

Tested in A3P

### Upgrade Considerations

Auctions are not upgradeable, so we have to replace them and update
their clients.
@mergify mergify bot closed this as completed in #9283 May 6, 2024
@mergify mergify bot closed this as completed in 24f7f32 May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contract-upgrade enhancement New feature or request oracle Related to on-chain oracles. performance Performance related issues
Projects
None yet
3 participants