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.

Sign up for GitHub

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

feat: update auction in agoricNames, test that the boardId changed #9970

Merged
merged 2 commits into from
Aug 27, 2024
Merged

feat: update auction in agoricNames, test that the boardId changed #9970

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
abf2168
feat: update auction in agoricNames, test that the boardId changed
Chris-Hibbert Aug 27, 2024
87b945d
chore: introduce economic committee charter to new auctioneer
dckc Aug 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions a3p-integration/proposals/a:vaults-auctions/agd-tools.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,3 +201,18 @@ export const getProvisionPoolMetrics = async () => {
const path = `published.provisionPool.metrics`;
return getQuoteBody(path);
};

export const getAuctionInstance = async price => {
Copy link
Member

Choose a reason for hiding this comment

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

not really an "agd tool". Also would be more general with a little tweak:

Suggested change
export const getAuctionInstance = async price => {
export const getInstanceBoardId = async instanceName => {

(price isn't used)

Not a blocker for this repo but in synthetic-chain package it should be designed for re-use.

const instanceRec = await queryVstorage(`published.agoricNames.instance`);

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

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

const key = Object.keys(feeds).find(k => feeds[k][0] === 'auctioneer');
if (key) {
return body.slots[key];
}
return null;
};
9 changes: 9 additions & 0 deletions a3p-integration/proposals/a:vaults-auctions/eval.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# we have an eval.sh so we can run prepare.sh before the rest

echo "[$PROPOSAL] Running prepare.sh"
./prepare.sh
Comment on lines +1 to +4
Copy link
Member

Choose a reason for hiding this comment

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

prepare is for chain-halting upgrade proposals. There should be no prepare.sh in a CoreEvalProposal.

Please remove the indirection:

Suggested change
# we have an eval.sh so we can run prepare.sh before the rest
echo "[$PROPOSAL] Running prepare.sh"
./prepare.sh
./saveAuctionInstance.js


echo "[$PROPOSAL] Running proposal declared in package.json"
# copy to run in the proposal package so the dependencies can be resolved
cp /usr/src/upgrade-test-scripts/eval_submission.js .
./eval_submission.js
2 changes: 2 additions & 0 deletions ...ration/proposals/a:vaults-auctions/use.sh → ...on/proposals/a:vaults-auctions/prepare.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,5 @@
# actions are executed in the upgraded chain software and the effects are
# persisted in the generated image for the upgrade, so they can be used in
# later steps, such as the "test" step, or further proposal layers.

Copy link
Member

Choose a reason for hiding this comment

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

another reason to remove this file is it says it's for after the proposal is executed but you're running it as part of the eval

./saveAuctionInstance.js
14 changes: 14 additions & 0 deletions a3p-integration/proposals/a:vaults-auctions/saveAuctionInstance.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#!/usr/bin/env node

import { writeFile } from 'fs/promises';
import { getAuctionInstance } from './agd-tools.js';

const { env } = process;

const oldAuctionInstance = await getAuctionInstance();
console.log('old auction instance ', oldAuctionInstance, env.HOME);

await writeFile(
`${env.HOME}/.agoric/previousInstance.json`,
Copy link
Member

Choose a reason for hiding this comment

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

this file is part of the permanent history a3p. please give it a more informative name for someone who comes across it without this context. or delete it after it's read.

also in general I think f a3p is writing to this directory it should be under a sub-path

oldAuctionInstance,
);
23 changes: 23 additions & 0 deletions a3p-integration/proposals/a:vaults-auctions/upgradeVaults.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
openVault,
USER1ADDR,
} from '@agoric/synthetic-chain';
import { readFile } from 'fs/promises';

import {
bankSend,
Expand All @@ -16,9 +17,12 @@ import {
getVaultPrices,
pushPrices,
addPreexistingOracles,
getAuctionInstance,
} from './agd-tools.js';
import { getDetailsMatchingVats } from './vatDetails.js';

const { env } = process;

const oraclesByBrand = new Map();

let roundId = 2;
Expand Down Expand Up @@ -103,6 +107,22 @@ const verifyVaultPriceUpdate = async t => {
t.is(quote.value[0].amountOut.value, '+5200000');
};

const verifyAuctionInstance = async t => {
const newAuctionInstance = await getAuctionInstance();
const oldInstance = await readFile(
`${env.HOME}/.agoric/previousInstance.json`,
'utf-8',
);

console.log(
`new: ${newAuctionInstance} should be different from ${oldInstance}`,
);
t.true(
newAuctionInstance !== oldInstance,
Comment on lines +120 to +121
Copy link
Member

Choose a reason for hiding this comment

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

Probably not worth a re-spin for this alone, but for future ref:

If you use t.not, ava will show the 2 values when the assertion fails.

Suggested change
t.true(
newAuctionInstance !== oldInstance,
t.not(newAuctionInstance,
oldInstance,

`new: ${newAuctionInstance} should be different from ${oldInstance}`,
);
};

// test.serial() isn't guaranteed to run tests in order, so we run the intended tests here
test('liquidation post upgrade', async t => {
t.log('setup Oracles');
Expand All @@ -125,4 +145,7 @@ test('liquidation post upgrade', async t => {

t.log('vault price updated');
await verifyVaultPriceUpdate(t);

t.log('auction instance changed in agoricNames');
await verifyAuctionInstance(t);
});
45 changes: 33 additions & 12 deletions packages/inter-protocol/src/proposals/add-auction.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,20 @@ const trace = makeTracer('NewAuction', true);
export const addAuction = async (
{
consume: {
agoricNamesAdmin,
auctioneerKit: legacyKitP,
board,
chainStorage,
chainTimerService,
economicCommitteeCreatorFacet: electorateCreatorFacet,
econCharterKit,
priceAuthority,
zoe,
},
produce: { auctioneerKit: produceAuctioneerKit, auctionUpgradeNewInstance },
instance: {
consume: { reserve: reserveInstance },
produce: { auctioneer: auctionInstance },
},
installation: {
consume: { contractGovernor: contractGovernorInstallation },
Expand Down Expand Up @@ -151,19 +154,34 @@ export const addAuction = async (
),
);

const kit = harden({
label: 'auctioneer',
creatorFacet: governedCreatorFacet,
adminFacet: governorStartResult.adminFacet,
publicFacet: governedPublicFacet,
instance: governedInstance,

governor: governorStartResult.instance,
governorCreatorFacet: governorStartResult.creatorFacet,
governorAdminFacet: governorStartResult.adminFacet,
});
produceAuctioneerKit.reset();
produceAuctioneerKit.resolve(
harden({
label: 'auctioneer',
creatorFacet: governedCreatorFacet,
adminFacet: governorStartResult.adminFacet,
publicFacet: governedPublicFacet,
instance: governedInstance,

governor: governorStartResult.instance,
governorCreatorFacet: governorStartResult.creatorFacet,
governorAdminFacet: governorStartResult.adminFacet,
}),
produceAuctioneerKit.resolve(kit);

// introduce economic committee charter to new auctioneer
// cf addGovernorsToEconCharter() in committee-proposal.js
await E(E.get(econCharterKit).creatorFacet).addInstance(
kit.instance,
kit.governorCreatorFacet,
kit.label,
);

auctionInstance.reset();
await auctionInstance.resolve(governedInstance);
// belt and suspenders; the above is supposed to also do this
await E(E(agoricNamesAdmin).lookupAdmin('instance')).update(
Copy link
Member

Choose a reason for hiding this comment

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

agoricNamesAdmin is a big hammer; it's unfortunate that resolve() alone didn't do the trick.

here's hoping for some later convenient time to figure out why not

Copy link
Member

Choose a reason for hiding this comment

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

the comment reads "belt-and-suspenders" as if the belt sometimes works. Does it not?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I haven't found the code that "is supposed to", so I don't know what it's trying to save, or what the corner cases might be. What code in promiseSpace is supposed to treat instances specially? Or what else updates agoricNames for instances?

'auctioneer',
governedInstance,
);

auctionUpgradeNewInstance.resolve(governedInstance);
Expand All @@ -172,10 +190,12 @@ export const addAuction = async (
export const ADD_AUCTION_MANIFEST = harden({
[addAuction.name]: {
consume: {
agoricNamesAdmin: true,
auctioneerKit: true,
board: true,
chainStorage: true,
chainTimerService: true,
econCharterKit: true,
economicCommitteeCreatorFacet: true,
priceAuthority: true,
zoe: true,
Expand All @@ -186,6 +206,7 @@ export const ADD_AUCTION_MANIFEST = harden({
},
instance: {
consume: { reserve: true },
produce: { auctioneer: true },
},
installation: {
consume: {
Expand Down
Loading