a3p proposal to create new priceFeeds #9044

Chris-Hibbert · 2024-03-07T00:11:21Z

Description

Add an a3p proposal to upgrade the price feed vats to lay out an approach to replace the existing price feed vats with new ones that won't accumulate uncollectible garbage.

The next steps are feeding prices to these oracles (to demonstrate that we can name them and retrieve them from agoricNames) and updating vaults and auctions to rely on the new feeds.

Security Considerations

Not a security issue.

Scaling Considerations

This is in service of reducing an existing scaling issue.

Documentation Considerations

None

Testing/Upgrade Considerations

This is an a3p test to validate plans to upgrade on-chain contracts to address sustainability concerns.

dckc · 2024-03-12T01:13:16Z

packages/builders/scripts/inter-protocol/price-feed-core.js


-  const inLookup = JSON.parse(IN_BRAND_LOOKUP);


I think this is a regression w.r.t. CLI usage.

@michaelfig I wonder if we have any tests for that.

Reverting....

dckc · 2024-03-12T01:16:51Z

packages/builders/scripts/inter-protocol/price-feed-core.js

-  if (!brandIn) {
+  if (brandIn) {


changing this file means putting another version of it (in a bundle) on the chain. Is that essential to do?

There are other clients... they probably won't be confused, because they refer to the code by bundleID. But I think we already have 2 versions of this on chain, and keeping them straight is hard enough.

I've been exploring a new contract to do this sort of thing.

WIP: interAsset contract: addVaultCollateral etc. agoric-labs/ag-power-tools#8

I reverted these changes.

dckc · 2024-03-12T01:20:58Z

packages/builders/scripts/vats/updatePriceFeeds.js

+  // 'agoric1dxe9dqu4ejpctnp2un7s4l7l8hny6dzr8juur3',
+  // 'agoric1y750klm8sh8yh4h4mcssx5m7rmvuu8mhk89t0c',
+
+  // XXX These are the GOV1ADDR, etc addresses. What should this file specify?


scripts/inter-protocol/price-feed-core.js gets them from the environment and arranges for them to be passed to defaultProposalBuilder. or something.

@turadg pointed out that the proposals get built before the environment that would generate addresses exists, so they have to be hard-coded or substituted or something. This relevant issue (Agoric/agoric-3-proposals#5) is apparently coming soon.

dckc · 2024-03-12T01:22:21Z

packages/inter-protocol/src/proposals/price-feed-proposal.js

+  if (priceAggregatorRef) {
+    const bundleID = await E.get(priceAggregatorRef).bundleID;
+    if (bundleID) {
+      installationP = E(zoe).installBundleID(bundleID);


do we want to update agoricNames.installation.priceAggregator in this case?

I think so, but i'm going to hold off until I see how we entrain the upgrades to auctions and vaults. If those are planned to follow shortly, then yes, but if not, it depends on the reasoning.

I mis-read your question. The answer to your actual question is yes, and we do so on line 141 now.

The answer to the question I thought you asked is "yes, we want to update the instances, and we were already doing so on line 222.

dckc · 2024-03-12T01:24:19Z

a3p-integration/proposals/c:price-feeds/priceFeed.test.js

+
+const ORACLE_ADDRESSES = [GOV1ADDR, GOV2ADDR, GOV3ADDR];
+
+test('update all priceFeed vats', async t => {


I don't see where this test updates anything.

Is it testing that something was updated?

line 22 evals priceFeed-submission

@Chris-Hibbert is updatePriceFeeds.js meant to be its own proposal or part of an upgrade? If it's a CORE_EVAL proposal then make the proposal package type be like,

"agoricProposal": { "type": "/agoric.swingset.CoreEvalProposal", "source": "subdir",

b:localchain is a good example.

If it will be part of an Software Upgrade, then put it in a:upgrade-next and its Go app handler like the other proposals there. 'upgrade-next' has a mix of u14 and u15 so we'll already be removing the u14 proposals once that hits Mainnet.

Moving to a:upgrade-next

dckc · 2024-03-12T01:27:09Z

a3p-integration/proposals/c:price-feeds/package.json

+    "sdkImageTag": "unreleased",
+    "planName": "UNRELEASED_UPGRADE",
+    "upgradeInfo": {
+      "coreProposals": []


if coreProposals is empty, what exactly is happening in this Software Upgrade?

It runs the upgrade handlers in the sdkImageTag. But that will have run by a:upgrade-next, so its's not doing anything.

It looks like this proposal package is just a holder for priceFeed.test.js (and its submission).

moved to a:upgrade-next

turadg · 2024-03-12T14:11:55Z

a3p-integration/proposals/c:price-feeds/package.json

+    "sdkImageTag": "unreleased",
+    "planName": "UNRELEASED_UPGRADE",
+    "upgradeInfo": {
+      "coreProposals": []


It runs the upgrade handlers in the sdkImageTag. But that will have run by a:upgrade-next, so its's not doing anything.

It looks like this proposal package is just a holder for priceFeed.test.js (and its submission).

turadg · 2024-03-12T14:19:41Z

a3p-integration/proposals/c:price-feeds/priceFeed.test.js

+
+const ORACLE_ADDRESSES = [GOV1ADDR, GOV2ADDR, GOV3ADDR];
+
+test('update all priceFeed vats', async t => {


line 22 evals priceFeed-submission

@Chris-Hibbert is updatePriceFeeds.js meant to be its own proposal or part of an upgrade? If it's a CORE_EVAL proposal then make the proposal package type be like,

"agoricProposal": { "type": "/agoric.swingset.CoreEvalProposal", "source": "subdir",

b:localchain is a good example.

If it will be part of an Software Upgrade, then put it in a:upgrade-next and its Go app handler like the other proposals there. 'upgrade-next' has a mix of u14 and u15 so we'll already be removing the u14 proposals once that hits Mainnet.

scripts/generate-a3p-submission-dirs.sh

Chris-Hibbert · 2024-03-13T20:54:58Z

If it will be part of an Software Upgrade, then put it in a:upgrade-next and its Go app handler like the other proposals there. 'upgrade-next' has a mix of u14 and u15 so we'll already be removing the u14 proposals once that hits Mainnet.

I moved it to a:upgrade-next, but was unable to add it to app.go. Maybe the four generated scripts should be added near there rather than update-price-feeds.js (the file containing defaultProposalBuilder)? When I added the file, it was invoked without options defined, which resulted in sadness.

Chris-Hibbert · 2024-03-14T23:47:06Z

If it will be part of an Software Upgrade, then put it in a:upgrade-next and its Go app handler like the other proposals there. 'upgrade-next' has a mix of u14 and u15 so we'll already be removing the u14 proposals once that hits Mainnet.

I moved it to a:upgrade-next, but was unable to add it to app.go. Maybe the four generated scripts should be added near there rather than update-price-feeds.js (the file containing defaultProposalBuilder)? When I added the file, it was invoked without options defined, which resulted in sadness.

I split the generation of the priceFeed proposals into 4 separate invocations that work both with app.go and a3p's submission generation.

turadg · 2024-03-19T22:12:44Z

a3p-integration/proposals/a:upgrade-next/package.json

@@ -6,6 +6,7 @@
    "upgradeInfo": {
      "coreProposals": []
    },
+    "comment": "all the priceFeed submissions go in the same directory",


we've been using $comment to make clear it's not part of the data

agoric-sdk/packages/vm-config/decentral-core-config.json

Line 2 in b20a8b6

"$comment": "This SwingSet config file (see loadSwingsetConfigFile) is minimal; it has no proposals. It is used by ../cosmic-swingset/Makefile and `agoric start`, which add proposals to it.",

however I'm not sure what this comment serves. What's the misunderstanding it's meant to remedy?

I dropped this line because the stuff it explained went away. Sorry I didn't get it before you noticed it.

turadg · 2024-03-19T22:16:51Z

packages/builders/scripts/vats/updatePriceFeeds.js

this file name looks like it's going to "update price feeds" but it just exports some values and a function. please rename to disambiguate. e.g. priceFeedSupport.js.

Though for the code, why not keep it in inter-protocol/price-feed-core.js? It's a variation but belongs there just as much as the original yeah?

I think @dckc said that we already had two versions of that file on chain. My interpretation was that he preferred starting afresh. I'll rename the file. It has gone through some evolutions, and is now just a support for the actual proposals.

turadg · 2024-03-19T22:20:02Z

a3p-integration/proposals/a:upgrade-next/priceFeed.test.js

+  // XXX I don't know why value.values[-1] doesn't work.
+  const body = JSON.parse(value.values[value.values.length - 1]);


because JS arrays are also dicts and -1 is a valid key

Suggested change

// XXX I don't know why value.values[-1] doesn't work.

const body = JSON.parse(value.values[value.values.length - 1]);

const body = JSON.parse(value.values.at(-1));

turadg · 2024-03-19T22:30:22Z

a3p-integration/proposals/a:upgrade-next/priceFeed.test.js

+  const bodyTruncated = JSON.parse(body.body.substring(1));
+  const slots = body.slots;
+
+  for (const [k, v] of bodyTruncated.entries()) {
+    if (v[0] === `${price}-USD price feed`) {
+      // console.log(`PFt `, price, slots[k]);
+      return slots[k];
+    }
+  }


this is a little opaque. consider,

Suggested change

const bodyTruncated = JSON.parse(body.body.substring(1));

const slots = body.slots;

for (const [k, v] of bodyTruncated.entries()) {

if (v[0] === `${price}-USD price feed`) {

// console.log(`PFt `, price, slots[k]);

return slots[k];

}

}

const feeds = JSON.parse(body.body.substring(1));

const feedName = `${price}-USD price feed`;

const key = Object.keys(feeds).find(k => feeds[k][0] === feedName);

if (key) {

return feeds.slots[key];

}

Thanks, that's much clearer.

turadg · 2024-03-19T22:32:34Z

a3p-integration/proposals/a:upgrade-next/priceFeed.test.js

+
+test.serial('check all priceFeed vats updated', async t => {
+  await null;
+  process.env.ORACLE_ADDRESSES = JSON.stringify(ORACLE_ADDRESSES);


usually we treat env has readonly. would that be feasible?

I don't think I'm using this anymore. I'll drop it.

I'm more worried about the addresses at the top of priceFeedSupport.js, and how we're going to get usable values for upgrading on-chain.

Chris-Hibbert added performance Performance related issues test contract-upgrade oracle Related to on-chain oracles. labels Mar 7, 2024

Chris-Hibbert self-assigned this Mar 7, 2024

Chris-Hibbert force-pushed the 8740-replacePriceFeeds branch from 770a721 to 91f3a7a Compare March 7, 2024 00:37

Chris-Hibbert mentioned this pull request Mar 7, 2024

prepare updates for Upgrade 15 #8868

Closed

14 tasks

Chris-Hibbert force-pushed the 8740-replacePriceFeeds branch 7 times, most recently from 310d6b2 to 3da4b8c Compare March 11, 2024 23:19

Chris-Hibbert requested review from turadg and dckc March 11, 2024 23:49

dckc reviewed Mar 12, 2024

View reviewed changes

turadg requested changes Mar 12, 2024

View reviewed changes

Chris-Hibbert mentioned this pull request Mar 13, 2024

feat: support multiple submissions per proposal #9085

Merged

Chris-Hibbert force-pushed the 8740-replacePriceFeeds branch 2 times, most recently from 6245165 to f57210e Compare March 13, 2024 23:10

Chris-Hibbert requested a review from turadg March 13, 2024 23:52

Chris-Hibbert force-pushed the 8740-replacePriceFeeds branch from f57210e to 841c9c9 Compare March 14, 2024 23:44

Chris-Hibbert force-pushed the 8740-replacePriceFeeds branch from 841c9c9 to c3c8034 Compare March 19, 2024 22:08

feat: a3p proposal to create new priceFeeds

7d6c412

turadg mentioned this pull request Mar 19, 2024

package up utilities for dapps #9109

Closed

Chris-Hibbert added 2 commits March 19, 2024 15:35

chore: move priceFeed.test to a: & make updatePriceFeeds stand-alone

f04bf65

test: repair test; hardcode oracle addresses.

8c9e0b5

turadg approved these changes Mar 19, 2024

View reviewed changes

feat: split up priceFeed proposals and add to app.go

8296522

Chris-Hibbert force-pushed the 8740-replacePriceFeeds branch from c3c8034 to 8296522 Compare March 19, 2024 22:43

chore: cleanups from review

db5862c

Chris-Hibbert mentioned this pull request Mar 20, 2024

Document process for creating proposals and submissions in a3p-integration. #9093

Merged

Chris-Hibbert added the automerge:squash Automatically squash merge label Mar 20, 2024

mergify bot and others added 2 commits March 20, 2024 00:28

Merge branch 'master' into 8740-replacePriceFeeds

72a7ce6

chore: get GOVXADDR from environment rather than ORACLE_ADDRESSES

f18292a

mergify bot merged commit b047af2 into master Mar 20, 2024
68 checks passed

mergify bot deleted the 8740-replacePriceFeeds branch March 20, 2024 16:58

Chris-Hibbert mentioned this pull request May 21, 2024

add priceFeed/scaledPriceAuthority proposal for stkAtom #9393

Closed

dckc mentioned this pull request Jun 24, 2024

feat(vats): support ORACLE_ADDRESSES from environment #9570

Closed

1 task

This was referenced Jun 24, 2024

MainNet proposals should dynamically provide oracle addresses #9571

Closed

Proposal to reset fluxAggregator roundIDs for stTIA/stkATOM #9594

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

a3p proposal to create new priceFeeds #9044

a3p proposal to create new priceFeeds #9044

Chris-Hibbert commented Mar 7, 2024 •

edited

Loading

dckc Mar 12, 2024

Chris-Hibbert Mar 12, 2024

dckc Mar 12, 2024

Chris-Hibbert Mar 13, 2024

dckc Mar 12, 2024

Chris-Hibbert Mar 13, 2024

dckc Mar 12, 2024

Chris-Hibbert Mar 13, 2024

Chris-Hibbert Mar 19, 2024

dckc Mar 12, 2024

turadg Mar 12, 2024

Chris-Hibbert Mar 12, 2024

dckc Mar 12, 2024

turadg Mar 12, 2024

Chris-Hibbert Mar 12, 2024

turadg Mar 12, 2024

turadg Mar 12, 2024

Chris-Hibbert commented Mar 13, 2024

Chris-Hibbert commented Mar 14, 2024

turadg Mar 19, 2024

Chris-Hibbert Mar 19, 2024

turadg Mar 19, 2024

Chris-Hibbert Mar 19, 2024

turadg Mar 19, 2024

Chris-Hibbert Mar 19, 2024

turadg Mar 19, 2024

Chris-Hibbert Mar 19, 2024

turadg Mar 19, 2024

Chris-Hibbert Mar 19, 2024


		const ORACLE_ADDRESSES = [GOV1ADDR, GOV2ADDR, GOV3ADDR];

		test('update all priceFeed vats', async t => {

		// XXX I don't know why value.values[-1] doesn't work.
		const body = JSON.parse(value.values[value.values.length - 1]);

	// XXX I don't know why value.values[-1] doesn't work.
	const body = JSON.parse(value.values[value.values.length - 1]);
	const body = JSON.parse(value.values.at(-1));

a3p proposal to create new priceFeeds #9044

a3p proposal to create new priceFeeds #9044

Conversation

Chris-Hibbert commented Mar 7, 2024 • edited Loading

Description

Security Considerations

Scaling Considerations

Documentation Considerations

Testing/Upgrade Considerations

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Chris-Hibbert commented Mar 13, 2024

Chris-Hibbert commented Mar 14, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Chris-Hibbert commented Mar 7, 2024 •

edited

Loading