Updates for oracle bootstrap #4891

michaelfig · 2022-03-22T05:00:32Z

Description

Several changes needed for bootstrapping the oracle price feeds. Aside from the price aggregator mechanics, here are some of the additional tweaks:

more AmountKind type propagation
scale the maximum mempool transaction size based on our changes to the maximum rpc message size
move startPriceAuthority into pre-economy bootstrap
make preapproved wallet bridge more useful (allow it to specify all the offer arguments)

Security Considerations

Documentation Considerations

Testing Considerations

Being tested with third parties as part of the oracle bootstrap process.

michaelfig · 2022-03-25T23:20:30Z

@dckc, can you please review? You were closer to many of these changes than the code owners.

@turadg, please review the AmountKind change and the contract updates.

turadg

@turadg, please review the AmountKind change and the contract updates.

Done. Thanks for the instructions and clean commits.

packages/ERTP/src/types.js

turadg · 2022-03-25T23:27:42Z

packages/wallet/api/src/wallet.js

@@ -238,8 +238,8 @@ export function buildRootObject(vatPowers) {
   * @type {WalletBridge}
   */
  const preapprovedBridge = Far('preapprovedBridge', {
-    addOffer(offer) {
-      return walletAdmin.addOffer(offer);
+    addOffer(offer, meta = undefined) {


jsdoc better for optional.

Okay, I added a jsdoc to it. How do I indicate to the reader of this code (who isn't using Typescript) that the meta argument is optional without overriding the underlying default?

https://jsdoc.app/tags-param.html#optional-parameters-and-default-values

I don't know the types of those params so improvising:

/** * @param {Offer} offer * @param {Meta} [meta] */ addOffer(offer, meta) {

(default values can be specified with = but [meta=undefined] I think is redundant because undefined is what any param without an argument resolves to)

turadg · 2022-03-25T23:32:16Z

packages/zoe/src/contracts/priceAggregator.js

+          // See if we have associated parameters or just a raw value.
+          const data = value.data || value;


why would it be one or the other?

turadg · 2022-03-25T23:32:33Z

packages/zoe/src/contracts/priceAggregator.js

+        }
+
+        // Adapt the notifier to push results.
+        const recurse = ({ value, updateCount }) => {


I'm not clear on what value can be. A type annotation would help. Is value readonly?

turadg · 2022-03-25T23:40:47Z

packages/zoe/src/contracts/priceAggregator.js

+          const scaledData = Math.floor(parseInt(data, 10) * scaleValueOut);
+          const newData = BigInt(scaledData);
+
+          if (value.data) {


line 268 sort of also has this condition. consider making one conditional:

const scaleData = (data) => BigInt(Math.floor(parseInt(data, 10) * scaleValueOut)); let result; if (value.data) { // We have some associated parameters to push. result = { ...value, data: scaleData(value.data) }; } else { result = scaleData(value); } admin.pushResult(result).catch(console.error);

turadg · 2022-03-25T23:42:07Z

packages/zoe/src/contracts/priceAggregator.js

+
+      return zcf.makeInvitation(offerHandler, 'oracle invitation');
+    },
+    async initOracle(oracleInstance = undefined, query = undefined) {


ditto on jsdoc as way to expression optional param

turadg · 2022-03-25T23:46:23Z

packages/zoe/src/contracts/priceAggregator.js

      assert(quoteKit, X`Must initializeQuoteMint before adding an oracle`);

+      const oracleKey = oracleInstance || Far('fresh key');


I don't understand this "fresh key". Will be be fresh each time it's constructed?

Yes. Every call to Far creates a new object identity. I was leaning on default arguments to the Far function, but have made it more explicit now.

turadg · 2022-03-25T23:49:58Z

packages/zoe/src/contracts/priceAggregatorChainlink.js

-  /** @type {LegacyMap<Instance, Set<OracleRecord>>} */
-  const instanceToRecords = makeLegacyMap('oracleInstance');
+  /**
+   * @typedef {{}} OracleKey


because TS is structurally typed, this make the type just documentation. Reading above it looks like

Suggested change

* @typedef {{}} OracleKey

* @typedef {Instance | Remotable<'fresh key'> } OracleKey

It is the type of an object without properties, which is what I want to indicate. Maybe this is where JSDoc types diverge from TS interfaces?

It is the type of an object without properties, which is what I want to indicate.

In that case, 👍 . I assumed an OracleKey was something more specific than an object without properties.

Maybe this is where JSDoc types diverge from TS interfaces?

I'm not aware of any divergences.

dckc

I looked at each commit. I'm not quite confident enough to approve. I'd like to see responses to some of my questions or maybe talk about it with you.

packages/zoe/src/contracts/priceAggregatorChainlink.js

packages/zoe/test/unitTests/contracts/test-priceAggregator.js

packages/zoe/src/contracts/priceAggregator.js

packages/zoe/test/unitTests/contracts/test-priceAggregator.js

packages/zoe/src/contracts/priceAggregator.js

packages/zoe/src/contracts/priceAggregatorChainlink.js

dckc

This looks like forward progress, to me.

I trust you'll consider @turadg 's comments as well.

michaelfig self-assigned this Mar 22, 2022

michaelfig added the oracle Related to on-chain oracles. label Mar 22, 2022

michaelfig changed the base branch from master to mfig-cl-aggregator March 22, 2022 05:01

michaelfig force-pushed the mfig-cl-aggregator branch from f686c2a to aa965c0 Compare March 25, 2022 19:02

michaelfig force-pushed the mfig-oracle-bootstrap branch 2 times, most recently from 3d15399 to 55eeddc Compare March 25, 2022 23:11

michaelfig changed the base branch from mfig-cl-aggregator to master March 25, 2022 23:12

michaelfig marked this pull request as ready for review March 25, 2022 23:18

michaelfig requested review from erights, Chris-Hibbert, samsiegart, turadg and kriskowal as code owners March 25, 2022 23:18

michaelfig requested a review from dckc March 25, 2022 23:18

turadg reviewed Mar 25, 2022

View reviewed changes

dckc reviewed Mar 25, 2022

View reviewed changes

dckc approved these changes Mar 28, 2022

View reviewed changes

michaelfig added 12 commits March 28, 2022 12:27

fix(oracle): wake up to update priceAggregator push-only oracles

c9a59fe

feat(oracle): add getRoundStartNotifier to aggregators

e97a2d9

refactor(ertp): propagate AmountKinds to getAmountOf

b9168f8

feat(vats): introduce 'oracleBrand'

4f3f5e0

fix(agoric-cli): increase max mempool transaction size

2859856

fix(zoe): pass brands (not issuers) to priceAggregator

5800711

feat(wallet-api): allow preapproved bridge to set offer meta

adbbaaf

feat(priceAggregator): implement makeOracleInvitation

35354bb

fix(chainlinkAggregator): implement oracleKeys for mapping

76605ce

fix(vats): move startPriceAuthority earlier in the boot sequence

bf93171

test(vaultFactory): reject instead of using an invalid PriceQuote

7cfb9c7

refactor: use JSDoc to indicate optional params

d1c10bc

michaelfig force-pushed the mfig-oracle-bootstrap branch from 3824d92 to d1c10bc Compare March 28, 2022 18:28

michaelfig added the automerge:no-update (expert!) Automatically merge without updates label Mar 28, 2022

mergify bot merged commit 1ee1739 into master Mar 28, 2022

mergify bot deleted the mfig-oracle-bootstrap branch March 28, 2022 18:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updates for oracle bootstrap #4891

Updates for oracle bootstrap #4891

michaelfig commented Mar 22, 2022 •

edited

Loading

michaelfig commented Mar 25, 2022

turadg left a comment

turadg Mar 25, 2022

michaelfig Mar 26, 2022

turadg Mar 28, 2022

turadg Mar 25, 2022

turadg Mar 25, 2022

turadg Mar 25, 2022

turadg Mar 25, 2022

turadg Mar 25, 2022

michaelfig Mar 26, 2022

turadg Mar 25, 2022

michaelfig Mar 26, 2022

turadg Mar 28, 2022

dckc left a comment

dckc left a comment

		// See if we have associated parameters or just a raw value.
		const data = value.data \|\| value;

		assert(quoteKit, X`Must initializeQuoteMint before adding an oracle`);

		const oracleKey = oracleInstance \|\| Far('fresh key');

	* @typedef {{}} OracleKey
	* @typedef {Instance \| Remotable<'fresh key'> } OracleKey

Updates for oracle bootstrap #4891

Updates for oracle bootstrap #4891

Conversation

michaelfig commented Mar 22, 2022 • edited Loading

Description

Security Considerations

Documentation Considerations

Testing Considerations

michaelfig commented Mar 25, 2022

turadg left a 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

dckc left a comment

Choose a reason for hiding this comment

dckc left a comment

Choose a reason for hiding this comment

michaelfig commented Mar 22, 2022 •

edited

Loading