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

test(inter-protocol): snapshot storage/RPC API for auction, vaultFactory, reserve, psm, committees #7368

Merged
merged 10 commits into from
Apr 14, 2023
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
# Snapshot report for `test/unitTests/test-committee.js`

The actual snapshot is saved in `test-committee.js.snap`.

Generated by [AVA](https://avajs.dev).

## committee-open question:mixed, with snapshot

> Under "published", the "committees.Economic_Committee" node is delegated to a committee contract.
> The example below illustrates the schema of the data published there.
>
> See also board marshalling conventions (_to appear_).

[
[
'published.committees.Economic_Committee.latestOutcome',
{
outcome: 'fail',
question: {
iface: 'Alleged: QuestionHandle',
},
reason: 'No quorum',
},
],
[
'published.committees.Economic_Committee.latestQuestion',
{
closingRule: {
deadline: 1n,
timer: {
iface: 'Alleged: ManualTimer',
},
},
counterInstance: {
iface: 'Alleged: InstanceHandle',
},
electionType: 'survey',
issue: {
text: 'why3',
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This lacks detail on parameter change issues. I think developers of dapp-gov, i.e. us, are the most interested consumers of that API, and we already figured it out.

},
maxChoices: 1,
maxWinners: 1,
method: 'unranked',
positions: [
{
text: 'because',
},
{
text: 'why not?',
},
],
questionHandle: {
iface: 'Alleged: QuestionHandle',
},
quorumRule: 'majority',
tieOutcome: {
text: 'why not?',
},
},
],
]
Binary file not shown.
11 changes: 10 additions & 1 deletion packages/governance/test/unitTests/test-committee.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
QuorumRule,
coerceQuestionSpec,
} from '../../src/index.js';
import { documentStorageSchema } from '../../tools/storageDoc.js';

const filename = new URL(import.meta.url).pathname;
const dirname = path.dirname(filename);
Expand Down Expand Up @@ -134,7 +135,7 @@ test('committee-open question:one', async t => {
);
});

test('committee-open question:mixed', async t => {
test('committee-open question:mixed, with snapshot', async t => {
const {
electorateStartResult: { creatorFacet, publicFacet },
counterInstallation,
Expand Down Expand Up @@ -224,4 +225,12 @@ test('committee-open question:mixed', async t => {

const questions = await publicFacet.getOpenQuestions();
t.deepEqual(questions.length, 2);

const doc = {
node: 'committees.Economic_Committee',
owner: 'a committee contract',
pattern: 'mockChainStorageRoot.thisElectorate.',
replacement: 'published.committees.Economic_Committee.',
Comment on lines +232 to +233
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these would read more easily as a map, even if it has just one item.

Suggested change
pattern: 'mockChainStorageRoot.thisElectorate.',
replacement: 'published.committees.Economic_Committee.',
rekey: {'mockChainStorageRoot.thisElectorate': published.committees.Economic_Committee`},

};
await documentStorageSchema(t, mockChainStorageRoot, doc);
});
37 changes: 37 additions & 0 deletions packages/governance/tools/storageDoc.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import { eventLoopIteration } from '@agoric/zoe/tools/eventLoopIteration.js';

/**
* @param {import('ava').ExecutionContext<unknown>} t
* @param {import('@agoric/internal/src/storage-test-utils.js').MockChainStorageRoot} storage
* @param {({ note: string } | { node: string, owner: string }) &
* ({ pattern: string, replacement: string } | {})
* } opts
*/
export const documentStorageSchema = async (t, storage, opts) => {
// chainStorage publication is unsynchronized
await eventLoopIteration();

const { pattern, replacement } =
'pattern' in opts
? opts
: { pattern: 'mockChainStorageRoot.', replacement: 'published.' };
const illustration = [...storage.keys()].sort().map(
/** @type {(k: string) => [string, unknown]} */
key => [key.replace(pattern, replacement), storage.getBody(key)],
);
const pruned = illustration.filter(
'node' in opts
? ([key, _]) => key.startsWith(`published.${opts.node}`)
: _entry => true,
);

const note =
'note' in opts
? opts.note
: `Under "published", the "${opts.node}" node is delegated to ${opts.owner}.`;
const boilerplate = `
The example below illustrates the schema of the data published there.

See also board marshalling conventions (_to appear_).`;
t.snapshot(pruned, note + boilerplate);
};
23 changes: 14 additions & 9 deletions packages/inter-protocol/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ To maintain that the keys of vaults to liquidate are stable requires that its ke

VaultFactory publishes data using StoredPublishKit which tees writes to off-chain storage. These can then be followed off-chain like so,
```js
import { makeFollower } from '@agoric/casting';
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where does makeDefaultLeader come from? I don't see it in @agoric/casting.


const key = `published.vaultFactory.metrics`; // or whatever the stream of interest is
const leader = makeDefaultLeader();
const follower = makeFollower(storeKey, leader);
Expand All @@ -40,32 +42,35 @@ VaultFactory publishes data using StoredPublishKit which tees writes to off-chai

The canonical keys (under `published`) are as follows. Non-terminal nodes could have data but don't yet. A `0` indicates the index of that child in added order. To get the actual key look it up in parent. High cardinality types get a parent key for enumeration (e.g. `vaults`.)
- `published`
- `vaultFactory`
- `vaultFactory` - [snapshot of details](./test/vaultFactory/snapshots/test-vaultFactory.js.md)
- `governance`
- `metrics`
- `manager0`
- `metrics`
- `governance`
- `vaults`
- `vault0`
- `auction`
- `auction` - [snapshot of details](./test/auction/snapshots/test-auctionContract.js.md)
- `schedule`
- `governance`
- `books`
- `book0`
- `data`
- `priceFeed`
- '<inputBrand>_<outputBrand>_price_feed'
- `psm`
- `book0`
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so we want book0, book1 etc. under auction after all?

I'm standing by to see if by "correct this" @turadg meant to change the published structure to include a book or books node rather than tweaking this README.

Copy link
Member

@turadg turadg Apr 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @Chris-Hibbert my earlier comment wasn't clear on the request:. I meant to fix the code so it matches the README, and the enumeration principle sketched in #6111

If keys are an index of high cardinality, they should be under their own parent key so that they don't make siblings of other types hard to find. E.g. if manager102 sibling to metrics is bad because the latter will be easy to miss. managers sibling to metrics is good.

Managers and Books are high enough cardinality to merit an grouping key.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That doesn't seem to me like a change that should be made by adding a commit to a PR on another subject.
@dckc would you excise the commit I inserted?
@turadg, would you mind creating an issue that can be prioritized?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

each commit ends up separately in master so a bugfix commit is reasonable to me to include in any PR that has the appropriate reviewers. if you want to do the work in a different PR up to you.

as an issue to prioritize, #6111 encompasses the requirement. it's prioritized as a blocker for Vaults Release.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried integrating the change with the existing commit stack, but the changes to the README are in a different commit than where I'd put the code changes. When I tried it, it turned into a mess, because several later commits also edit the same section of the README. It'll be far simpler as a follow-on.

- `reserve` - [snapshot of details](./test/reserve/snapshots/test-reserve.js.md)
- `governance`
- `metrics`
- `priceFeed` - [snapshot of details](./test/price/snapshots/test-fluxAggregator.js.md)
- `${inputBrand}-${outputBrand}_price_feed`
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this formerly had a _ as in ATOM_USD when snapshotting shows it's ATOM-USD.

- `${inputBrand}-${outputBrand}_price_feed.latestRound`
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

latestRound was missing.

Is it worth trying to maintain this tree of all the keys? Now that I have snapshots for all but stakeFactory, maybe it's better to just list the top level?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

latestOutcome below was missing as well.

- `psm` - [snapshot of details](./test/psm/snapshots/test-psm.js.md)
- `<minted>`
- `<anchor>`
- `governance`
- `metrics`
- `stakeFactory`
- `governance`
- `committees`
- `committees` - [snapshot of details](../governance/test/unitTests/snapshots/test-committee.js.md)
- `Economic_Committee`
- `latestQuestion`
- `latestOutcome`

### Demo

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
# Snapshot report for `test/auction/test-auctionContract.js`

The actual snapshot is saved in `test-auctionContract.js.snap`.

Generated by [AVA](https://avajs.dev).

## onDeadline exit, with chainStorage RPC snapshot

> Under "published", the "auction" node is delegated to the auctioneer contract.
> The example below illustrates the schema of the data published there.
>
> See also board marshalling conventions (_to appear_).

[
[
'published.auction.book0',
{
collateralAvailable: {
brand: {
iface: 'Alleged: Collateral brand',
},
value: 100n,
},
currentPriceLevel: {
denominator: {
brand: {
iface: 'Alleged: Collateral brand',
},
value: 10000000000000n,
},
numerator: {
brand: {
iface: 'Alleged: Currency brand',
},
value: 9350000000000n,
},
},
proceedsRaised: undefined,
remainingProceedsGoal: null,
startCollateral: {
brand: {
iface: 'Alleged: Collateral brand',
},
value: 100n,
},
startPrice: {
denominator: {
brand: {
iface: 'Alleged: Collateral brand',
},
value: 1000000000n,
},
numerator: {
brand: {
iface: 'Alleged: Currency brand',
},
value: 1100000000n,
},
},
startProceedsGoal: null,
},
],
[
'published.auction.governance',
{
current: {
AuctionStartDelay: {
type: 'relativeTime',
value: {
relValue: 10n,
timerBrand: {
iface: 'Alleged: timerBrand',
},
},
},
ClockStep: {
type: 'relativeTime',
value: {
relValue: 5n,
timerBrand: {
iface: 'Alleged: timerBrand',
},
},
},
DiscountStep: {
type: 'nat',
value: 2000n,
},
Electorate: {
type: 'invitation',
value: {
brand: {
iface: 'Alleged: Zoe Invitation brand',
},
value: [
{
description: 'getRefund',
handle: {
iface: 'Alleged: InvitationHandle',
},
installation: {
iface: 'Alleged: BundleInstallation',
},
instance: {
iface: 'Alleged: InstanceHandle',
},
},
],
},
},
LowestRate: {
type: 'nat',
value: 4500n,
},
PriceLockPeriod: {
type: 'relativeTime',
value: {
relValue: 3n,
timerBrand: {
iface: 'Alleged: timerBrand',
},
},
},
StartFrequency: {
type: 'relativeTime',
value: {
relValue: 40n,
timerBrand: {
iface: 'Alleged: timerBrand',
},
},
},
StartingRate: {
type: 'nat',
value: 10500n,
},
},
},
],
[
'published.auction.schedule',
{
activeStartTime: {
absValue: 170n,
timerBrand: {
iface: 'Alleged: timerBrand',
},
},
nextDescendingStepTime: {
absValue: 180n,
timerBrand: {
iface: 'Alleged: timerBrand',
},
},
nextStartTime: {
absValue: 210n,
timerBrand: {
iface: 'Alleged: timerBrand',
},
},
},
],
]
Binary file not shown.
11 changes: 10 additions & 1 deletion packages/inter-protocol/test/auction/test-auctionContract.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { test as anyTest } from '@agoric/zoe/tools/prepare-test-env-ava.js';
import '@agoric/zoe/exported.js';

import { makeIssuerKit, AmountMath } from '@agoric/ertp';
import { documentStorageSchema } from '@agoric/governance/tools/storageDoc.js';
import { deeplyFulfilledObject, makeTracer } from '@agoric/internal';
import { eventLoopIteration } from '@agoric/notifier/tools/testSupports.js';
import { buildManualTimer } from '@agoric/swingset-vat/tools/manual-timer.js';
Expand Down Expand Up @@ -869,7 +870,7 @@ test.serial('onDemand exit', async t => {
});

// serial because dynamicConfig is shared across tests
test.serial('onDeadline exit', async t => {
test.serial('onDeadline exit, with chainStorage RPC snapshot', async t => {
const { collateral, currency } = t.context;
const driver = await makeAuctionDriver(t);
const timerBrand = await E(driver.getTimerService()).getTimerBrand();
Expand Down Expand Up @@ -946,6 +947,14 @@ test.serial('onDeadline exit', async t => {
currentPriceLevel: { numerator: { value: 9_350_000_000_000n } },
});

const doc = {
node: 'auction',
owner: 'the auctioneer contract',
pattern: 'mockChainStorageRoot.thisAuction',
replacement: 'published.auction',
};
await documentStorageSchema(t, driver.mockChainStorage, doc);

await driver.advanceTo(180n, 'wait');
await bookTracker.assertChange({
currentPriceLevel: { numerator: { value: 7_150_000_000_000n } },
Expand Down
Loading