Skip to content

Commit

Permalink
Save the contractKit for the auctioneer before overwriting it (#10694)
Browse files Browse the repository at this point in the history
refs: #10680

## Description

We misplaced the adminFacets that would allow us to manage Auctions after they've been replaced and were about to do it again. The auctions were [last upgraded](https://github.com/Agoric/agoric-sdk/blob/43345a561fbdf7621c369abb15e6839f7c696565/packages/inter-protocol/src/proposals/add-auction.js#L157) in `agoric-upgrade-16av`. That code fails to save the instance's adminFacet, and only stores the contractKit in bootstrap promise space under the name `auctioneerKit`, where it will be overwritten on upgrade. Our other contracts now save a copy of the `contractKit` in either `contractKits` or `governedContractKits`, indexed by the instance, so the facets will hang around. This saves the old auctioneer during upgrade so we can manage it later (upgrade, terminate, change parameters).

### Security Considerations

Losing our last handle for vats is a problem.

### Scaling Considerations

We're upgrading vats to deal with scaling.

### Documentation Considerations

None.

### Testing Considerations

there was a test in #10680 which looked for this kit in `governedContractKits`, but I commented it out when it didn't succeed. It succeeds now.

### Upgrade Considerations

Yes.
  • Loading branch information
Chris-Hibbert authored Dec 14, 2024
1 parent 4823e85 commit 140c1be
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const trace = makeTracer('RecordedRetired', true);
export const testRecordedRetiredInstances = async ({
consume: {
contractKits,
// governedContractKits,
governedContractKits,
retiredContractInstances: retiredContractInstancesP,
},
}) => {
Expand All @@ -26,8 +26,7 @@ export const testRecordedRetiredInstances = async ({
assert(auctionIDs.length === 1);
const auctionInstance = retiredContractInstances.get(auctionIDs[0]);
trace({ auctionInstance });
// I don't know why it's neither in governedContractKits nor contractKits
// assert(await E(governedContractKits).get(auctionInstance));
assert(await E(governedContractKits).get(auctionInstance));

const committeeIDs = Array.from(retiredContractInstances.keys()).filter(k =>
k.startsWith('economicCommittee'),
Expand All @@ -47,7 +46,7 @@ export const getManifestForRecordedRetiredInstances = () => {
[testRecordedRetiredInstances.name]: {
consume: {
contractKits: true,
// governedContractKits: true,
governedContractKits: true,
retiredContractInstances: true,
},
},
Expand Down
15 changes: 14 additions & 1 deletion packages/inter-protocol/src/proposals/add-auction.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,20 @@ export const addAuction = async (
produceRetiredInstances,
);

const governedContractKits = await governedContractKitsP;
trace('has', governedContractKits.has(legacyKit.instance));
if (governedContractKits.has(legacyKit.instance)) {
// bootstrap tests start having already run this upgrade. Actual upgrades on
// mainNet or testnets should start with the promiseSpace post upgrade-17,
// which doesn't have this entry in the map.
trace(
'⚠️ WARNING: not expected during chain upgrade. It IS normal during bootstrap tests',
);
} else {
// @ts-expect-error The original auctioneerKit had everything it needs
governedContractKits.init(legacyKit.instance, legacyKit);
}

// save the auctioneer instance so we can manage it later
const boardID = await E(board).getId(legacyKit.instance);
const identifier = `auctioneer-${boardID}`;
Expand Down Expand Up @@ -205,7 +219,6 @@ export const addAuction = async (
governedInstance,
);

const governedContractKits = await governedContractKitsP;
governedContractKits.init(kit.instance, kit);
auctionUpgradeNewInstance.resolve(governedInstance);
auctionUpgradeNewGovCreator.resolve(kit.governorCreatorFacet);
Expand Down

0 comments on commit 140c1be

Please sign in to comment.