7495 price vat upgradable #7555

turadg · 2023-04-29T18:02:12Z

closes: #7495

Description

Makes the priceAuthority vat upgradable.

First clarifies the responsibility of the vat: to hold the registry of price authorities. Moreover it's the singular and canonical registry for the vat set. (If there needs to be another registry, there will have to be another vat made with its own baggage.)

This simplification allowed the durability to be implemented as singleton of two Exo facets instead of an Exo class.

Security Considerations

--

Scaling Considerations

--

Documentation Considerations

--

Testing Considerations

Has a test of restarting the vat. We'll need more test coverage but the requirements are being sorted out. I expect it will fall under #7562

dckc

getSingleton API change seems fine

dckc · 2023-04-29T20:56:05Z

packages/vats/src/vat-priceAuthority.js

-export function buildRootObject() {
-  return Far('root', {
-    makePriceAuthorityRegistry,
-    makeFakePriceAuthority: async options => makeFakePriceAuthority(options),


makeFakePriceAuthority isn't used?

nope, not from the vat. whenever a fake is necessary it's because the vat is skipped.

dckc · 2023-04-29T21:00:24Z

packages/zoe/tools/priceAuthorityRegistry.js

+        if (assetToPriceStore.has(brandIn)) {
+          priceStore = assetToPriceStore.get(brandIn);
+        } else {
+          priceStore = provideDurableMapStore(baggage, 'brandOut');
+          assetToPriceStore.init(brandIn, priceStore);
+        }


this looks like the provide pattern. use provide()?

indeed! done

dckc · 2023-04-29T21:16:25Z

packages/zoe/tools/priceAuthorityRegistry.js

+    M.interface('compositePriceAuthority', {
+      getQuoteIssuer: M.call(BrandShape, BrandShape).returns(M.promise()),
+      getTimerService: M.call(BrandShape, BrandShape).returns(M.promise()),
+      quoteGiven: M.call(AmountShape, BrandShape).returns(M.promise()),
+      quoteWanted: M.call(BrandShape, AmountShape).returns(M.promise()),
+      makeQuoteNotifier: M.call(AmountShape, BrandShape).returns(M.promise()),
+      quoteAtTime: M.call(TimestampShape, AmountShape, BrandShape).returns(
+        M.promise(),
+      ),
+      quoteWhenLT: GuardCallAmountTuple,
+      quoteWhenLTE: GuardCallAmountTuple,
+      quoteWhenGTE: GuardCallAmountTuple,
+      quoteWhenGT: GuardCallAmountTuple,
+      mutableQuoteWhenLT: GuardCallAmountTuple,
+      mutableQuoteWhenLTE: GuardCallAmountTuple,
+      mutableQuoteWhenGTE: GuardCallAmountTuple,
+      mutableQuoteWhenGT: GuardCallAmountTuple,
+    }),


I'm surprised we haven't written this interface guard before. The fluxAggregator is an exo price authority, isn't it?

Hm... there's one price authority held in the prepareFluxAggregatorKit closure...

agoric-sdk/packages/inter-protocol/src/price/fluxAggregatorKit.js

Lines 178 to 179 in 6e658d9

// not durable, held in closure and remade in every call of enclosing

const { priceAuthority } = makeOnewayPriceAuthorityKit({

It seems to be referenced by all instances of the fluxAggregator exo class kit.

agoric-sdk/packages/inter-protocol/src/price/fluxAggregatorKit.js

Lines 204 to 206 in 6e658d9

const makeFluxAggregatorKit = prepareExoClassKit(

baggage,

'fluxAggregator',

Maybe that's OK, since there's only one instance of that exo class... but then shouldn't it be an exo singleton?

It seems like scaledPriceAuthority.js should also use this interface guard. Is making that durable in our plans anywhere?

for scaledPriceAuthority I added some docs: 34d2d7d

for the Exo singleton idea let's chat

dckc · 2023-04-30T18:48:27Z

packages/zoe/src/contracts/scaledPriceAuthority.js

@@ -22,6 +22,10 @@ import { makeInitialTransform } from '../contractSupport/priceAuthorityInitial.j
 * A contract that scales a source price authority to a target price authority
 * via ratios.
 *
+ * Not durable. Because it only transforms there's nothing important to save.


that much makes sense

dckc · 2023-04-30T18:55:39Z

packages/zoe/src/contracts/scaledPriceAuthority.js

+ * The facets may sever upon vat restart, but the priceAuthorityRegistry that
+ * holds them can be given a fresh scaling contract to replace them.


Hm. I wonder how that works. Replacing a price authority in the registry is one thing, but what about the reference from the vaultFactory terms?

agoric-sdk/packages/inter-protocol/src/vaultFactory/vaultManager.js

Line 171 in 1265a79

const { priceAuthority, timerService, reservePublicFacet } = zcf.getTerms();

It makes more sense to me that we could upgrade or restart this contract than wire up a fresh one.

dckc

good stuff

dckc · 2023-04-30T21:09:03Z

packages/vats/test/upgrading/test-upgrade-vats.js

+  const reincarnatedRegistry = await EV.vat('priceAuthority').getRegistry();
+  // facet holder object changes but the members have the same identity
+  t.not(reincarnatedRegistry, registry);
+  t.is(reincarnatedRegistry.priceAuthority, registry.priceAuthority);


dckc · 2023-05-01T19:19:57Z

packages/SwingSet/tools/bundleTool.js

@@ -6,6 +6,9 @@ import styles from 'ansi-styles'; // less authority than 'chalk'

 const { quote: q, Fail } = assert;

+/** @type {Map<string, Promise<BundleMeta>>} output filename to its meta info */
+const fileMetas = new Map();


does this state need to be shared across bundle caches?

yes because each multiple bundle caches are trying to write to the same file paths. I suppose it would be better for "makeBundleCache" to be idempotent. I'll work on that. At least now for this PR I know that the failure is in the bundleTool

@michaelfig you recently did some work to address concurrent writes; take a look at this, please?

see also #3609

yes because each multiple bundle caches are trying to write to the same file paths.

I wonder if they're in the same process / heap, though. What @michaelfig did was to add .lock files, IIRC.

@turadg, can you document the issue you ran into with the bundle cache implementation that existed before this PR? I'm most curious how the problem manifested, such as unexpected output or exceptions.

The .lock file handshake should have been idempotent and robust against multiple uses of the same cache, whether or not they are inside the same process or multiple processes (including bundleTool.js).

It's visible in the previous build: 7675e71

It failed but only in CI,

# create initial version # now perform the upgrade [bundleTool] bundles add: vat-board from /home/runner/work/agoric-sdk/agoric-sdk/packages/vats/src/vat-board.js [bundleTool] bundles add: vat-bridge from /home/runner/work/agoric-sdk/agoric-sdk/packages/vats/src/vat-bridge.js [bundleTool] bundles add: vat-priceAuthority from /home/runner/work/agoric-sdk/agoric-sdk/packages/vats/src/vat-priceAuthority.js [bundleTool] bundles add: bootstrap-relay from /home/runner/work/agoric-sdk/agoric-sdk/packages/SwingSet/test/bootstrap-relay.js [bundleTool] bundles add: bootstrap-relay from /home/runner/work/agoric-sdk/agoric-sdk/packages/SwingSet/test/bootstrap-relay.js [bundleTool] bundles add: bootstrap-relay from /home/runner/work/agoric-sdk/agoric-sdk/packages/SwingSet/test/bootstrap-relay.js REJECTED from ava test("upgrade vat-board"): (Error#1) Error#1: config.vats.bootstrap: Invalid or unexpected token at packages/SwingSet/src/controller/initializeSwingset.js:538:15 at async Promise.all (index 0) at async processGroup (packages/SwingSet/src/controller/initializeSwingset.js:541:27) at async initializeSwingset (packages/SwingSet/src/controller/initializeSwingset.js:571:31) at async buildVatController (packages/SwingSet/src/controller/controller.js:479:23) at async makeScenario (packages/vats/test/upgrading/test-upgrade-vats.js:53:13) at async packages/vats/test/upgrading/test-upgrade-vats.js:74:18

idempotent and robust against multiple uses of the same cache

Note that the problem here was three tests in the same file each having a cacher configured on the same path. Now they share one provided cache and bootstrap-relay gets bundled once.

It's visible in the previous build: ...

Thanks for sharing that.

Oops. Turns out in the bustle of trying to sync Endo, I forgot to create an Agoric SDK PR to have bundleTool.js use Endo's idempotent-between-processes bundle cache.

Now I did that in #7596.

turadg requested review from dckc and michaelfig April 29, 2023 18:02

dckc reviewed Apr 29, 2023

View reviewed changes

dckc reviewed Apr 30, 2023

View reviewed changes

turadg force-pushed the 7495-price-vat-upgradable branch 2 times, most recently from f23d939 to 6e12a60 Compare April 30, 2023 21:02

turadg marked this pull request as ready for review April 30, 2023 21:05

dckc approved these changes Apr 30, 2023

View reviewed changes

turadg added the automerge:no-update (expert!) Automatically merge without updates label Apr 30, 2023

turadg force-pushed the 7495-price-vat-upgradable branch from 6e12a60 to 7675e71 Compare April 30, 2023 21:59

dckc reviewed May 1, 2023

View reviewed changes

turadg added 9 commits May 1, 2023 12:28

chore(types): simplify ERTP math generic

1914f63

chore(types): work around type resolution debt

a597915

chore(types): misc fixes

4abcecc

docs(price): justify scaled authority not durable

8100f19

feat(priceAuthority)!: canonicalize registry

9a6466d

feat(priceAuthority): registry singleton durable

3d20838

test(bootstrapTests): vat-priceAuthority restart

0692410

feat(prices): scaledPriceAuthority upgradable

91c6181

feat(bundleTool): idempotent provideBundleCache

026bcd4

turadg force-pushed the 7495-price-vat-upgradable branch from 7134270 to 026bcd4 Compare May 1, 2023 19:50

mergify bot merged commit 130d73f into master May 1, 2023

mergify bot deleted the 7495-price-vat-upgradable branch May 1, 2023 20:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

7495 price vat upgradable #7555

7495 price vat upgradable #7555

turadg commented Apr 29, 2023 •

edited

Loading

dckc left a comment

dckc Apr 29, 2023

turadg Apr 29, 2023

dckc Apr 29, 2023

turadg Apr 29, 2023

dckc Apr 29, 2023

dckc Apr 29, 2023

turadg Apr 29, 2023

dckc Apr 30, 2023

dckc Apr 30, 2023

dckc left a comment

dckc Apr 30, 2023

dckc May 1, 2023

turadg May 1, 2023

dckc May 1, 2023

dckc May 1, 2023

michaelfig May 2, 2023

turadg May 2, 2023 •

edited

Loading

michaelfig May 2, 2023

michaelfig May 3, 2023

	// not durable, held in closure and remade in every call of enclosing
	const { priceAuthority } = makeOnewayPriceAuthorityKit({

	const makeFluxAggregatorKit = prepareExoClassKit(
	baggage,
	'fluxAggregator',

		* The facets may sever upon vat restart, but the priceAuthorityRegistry that
		* holds them can be given a fresh scaling contract to replace them.

7495 price vat upgradable #7555

7495 price vat upgradable #7555

Conversation

turadg commented Apr 29, 2023 • edited Loading

Description

Security Considerations

Scaling Considerations

Documentation Considerations

Testing Considerations

dckc 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

dckc 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

turadg May 2, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

turadg commented Apr 29, 2023 •

edited

Loading

turadg May 2, 2023 •

edited

Loading