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

Jump to bottom

fix(zcf): mint kind during reincarnation #7508

Merged
merged 1 commit into from
Apr 26, 2023
Merged

fix(zcf): mint kind during reincarnation #7508

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
42 changes: 19 additions & 23 deletions packages/zoe/src/contractFacet/zcfMint.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,14 @@ import './types.js';

const { Fail } = assert;

export const makeZCFMintFactory = async (
zcfBaggage,
recordIssuer,
getAssetKindByBrand,
makeEmptySeatKit,
reallocator,
export const makeZCFMintFactory = (
/** @type {import('@agoric/vat-data').Baggage} */ zcfBaggage,
/** @type {{ (keyword: string, issuerRecord: IssuerRecord): void }} */ recordIssuer,
/** @type {GetAssetKindByBrand} */ getAssetKindByBrand,
/** @type { (exit?: undefined) => { zcfSeat: any; userSeat: Promise<UserSeat> }} */ makeEmptySeatKit,
/** @type {ZcfMintReallocator} */ reallocator,
Comment on lines +22 to +26
Copy link
Member

Choose a reason for hiding this comment

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

This seems like a radical shift in style from @param. Why? At least at first, I do not find it more readable.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's the style that "infer params from usage" automates. I think it's worth allowing because it removes the need to repeat the parameter name.

) => {
/** @type {SetStore<ZCFMint>} The set of baggages for zcfMints */
/** @type {SetStore<import('@agoric/vat-data').Baggage>} The set of baggages for zcfMints */
Copy link
Member

Choose a reason for hiding this comment

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

Is there any way to make this type more specific, to declare that it holds a set of zcfMintBaggage (which has a particular set of keys), rather than a set of generic Baggage (which itself is just a MapStore)?

Copy link
Member Author

Choose a reason for hiding this comment

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

It would be pretty awkward since a MapStore allows any keys. We could have a subtype that maintains a list of keyname to type mappings and conditions get/init/set accordingly but I'm not sure it's a good idea and it's definitely out of scope :)

const zcfMintBaggageSet = provideDurableSetStore(zcfBaggage, 'baggageSet');

/**
Expand All @@ -34,15 +34,14 @@ export const makeZCFMintFactory = async (
*
* @param {import('@agoric/vat-data').Baggage} zcfMintBaggage
*/
const provideDurableZcfMint = async zcfMintBaggage => {
const provideDurableZcfMint = zcfMintBaggage => {
const keyword = zcfMintBaggage.get('keyword');
const zoeMint = zcfMintBaggage.get('zoeMint');
const {
brand: mintyBrand,
issuer: mintyIssuer,
displayInfo: mintyDisplayInfo,
} = await E(zoeMint).getIssuerRecord();
// AWAIT
} = zcfMintBaggage.get('issuerRecord');
const mintyIssuerRecord = makeIssuerRecord(
mintyBrand,
mintyIssuer,
Expand Down Expand Up @@ -126,12 +125,6 @@ export const makeZCFMintFactory = async (
);
};

const makeDurableZcfMint = async (keyword, zoeMint, zcfMintBaggage) => {
zcfMintBaggage.init('keyword', keyword);
zcfMintBaggage.init('zoeMint', zoeMint);
return provideDurableZcfMint(zcfMintBaggage);
};

const ZCFMintFactoryI = M.interface('ZCFMintFactory', {
makeZCFMintInternal: M.call(M.string(), M.remotable('ZoeMint')).returns(
M.promise(),
Expand All @@ -150,21 +143,24 @@ export const makeZCFMintFactory = async (
ZCFMintFactoryI,
{
async makeZCFMintInternal(keyword, zoeMint) {
const issuerRecord = await E(zoeMint).getIssuerRecord();

const zcfMintBaggage = makeScalarBigMapStore('zcfMintBaggage', {
durable: true,
});
const zcfMint = await makeDurableZcfMint(
keyword,
zoeMint,
zcfMintBaggage,
);
zcfMintBaggageSet.add(zcfMint);
return zcfMint;

zcfMintBaggage.init('issuerRecord', issuerRecord);
Copy link
Member

Choose a reason for hiding this comment

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

This works, but it now looks awfully like per-zcfMint state, and I wouldn't mind a cleanup pass that replaced the prepareSingleton (one Kind per mint) with a prepare something else that shared a single Kind among all mints, with state.keyword, state.zoeMint, and state.issuerRecord. That would make startup faster, remove the extra Kinds, reduce memory use a little bit, and be generally simpler.

But, probably out of scope for this PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea. I'll follow up with that.

zcfMintBaggage.init('keyword', keyword);
zcfMintBaggage.init('zoeMint', zoeMint);
zcfMintBaggageSet.add(zcfMintBaggage);

return provideDurableZcfMint(zcfMintBaggage);
},
},
);

for (const zcfMintBaggage of zcfMintBaggageSet.values()) {
// call for side-effect of redefining the kinds
provideDurableZcfMint(zcfMintBaggage);
}

Expand Down
7 changes: 5 additions & 2 deletions packages/zoe/src/contractFacet/zcfZygote.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,10 @@ export const makeZCFZygote = async (
// Make the instanceRecord
const makeInstanceRecord = makeInstanceRecordStorage(zcfBaggage);

const recordIssuer = (keyword, issuerRecord) => {
const recordIssuer = (
/** @type {string} */ keyword,
/** @type {IssuerRecord} */ issuerRecord,
Comment on lines +94 to +95
Copy link
Member

Choose a reason for hiding this comment

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

ditto

) => {
getInstanceRecHolder().addIssuer(keyword, issuerRecord);
storeIssuerRecord(issuerRecord);
};
Expand Down Expand Up @@ -332,7 +335,7 @@ export const makeZCFZygote = async (
zcfBaggage,
));

zcfMintFactory = await makeZCFMintFactory(
zcfMintFactory = makeZCFMintFactory(
zcfBaggage,
recordIssuer,
getAssetKindByBrand,
Expand Down