Skip to content

Commit

Permalink
fix!(smart-wallet): omit redundant chainStorage balances updates
Browse files Browse the repository at this point in the history
  • Loading branch information
@dckc
dckc committed Feb 7, 2023
1 parent c693f8d commit 33814ad
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 144 deletions.
11 changes: 8 additions & 3 deletions packages/inter-protocol/test/smartWallet/contexts.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,18 @@ export const makeDefaultTestContext = async (t, makeSpace) => {
);

/** @param {string} address */
const simpleProvideWallet = async address => {
const provideBankAndWallet = async address => {
// copied from makeClientBanks()
const bank = E(consume.bankManager).getBankForAddress(address);
const bank = await E(consume.bankManager).getBankForAddress(address);

const [wallet, _isNew] = await E(
walletFactory.creatorFacet,
).provideSmartWallet(address, bank, consume.namesByAddressAdmin);
return wallet;
return { bank, wallet };
};

const simpleProvideWallet = async address => {
return provideBankAndWallet(address).then(({ wallet }) => wallet);
};

/**
Expand Down Expand Up @@ -121,6 +125,7 @@ export const makeDefaultTestContext = async (t, makeSpace) => {
sendToBridge:
walletBridgeManager && (obj => E(walletBridgeManager).toBridge(obj)),
consume,
provideBankAndWallet,
simpleProvideWallet,
simpleCreatePriceFeed,
};
Expand Down
56 changes: 28 additions & 28 deletions packages/inter-protocol/test/smartWallet/test-psm-integration.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,18 +52,14 @@ test.before(async t => {
});

/**
* @param {Awaited<ReturnType<typeof coalesceUpdates>>} state
* @param {Brand<'nat'>} brand
* @param {ERef<import('@agoric/vats/src/vat-bank.js').Bank>} bank
* @param {Brand} brand
*/
const purseBalance = (state, brand) => {
const balances = Array.from(state.balances.values());
const match = balances.find(b => b.brand === brand);
if (!match) {
console.debug('balances', ...balances);
assert.fail(`${brand} not found in record`);
}
return match.value;
};
const bankBalance = (bank, brand) =>
E(E(bank).getPurse(brand))
.getCurrentAmount()
.then(a => a.value);

/**
* @param {import('@agoric/smart-wallet/src/smartWallet.js').CurrentWalletRecord} record
* @param {Brand<'nat'>} brand
Expand All @@ -83,8 +79,9 @@ test('null swap', async t => {
const { agoricNames } = await E.get(t.context.consume);
const mintedBrand = await E(agoricNames).lookup('brand', 'IST');

const wallet = await t.context.simpleProvideWallet('agoric1nullswap');
const computedState = coalesceUpdates(E(wallet).getUpdatesSubscriber());
const { bank, wallet } = await t.context.provideBankAndWallet(
'agoric1nullswap',
);
const offersFacet = wallet.getOffersFacet();

const psmInstance = await E(agoricNames).lookup('instance', 'psm-IST-AUSD');
Expand Down Expand Up @@ -112,8 +109,8 @@ test('null swap', async t => {
await offersFacet.executeOffer(offerSpec);
await eventLoopIteration();

t.is(purseBalance(computedState, anchor.brand), 0n);
t.is(purseBalance(computedState, mintedBrand), 0n);
t.is(await bankBalance(bank, anchor.brand), 0n);
t.is(await bankBalance(bank, mintedBrand), 0n);

// success if nothing threw
t.pass();
Expand All @@ -130,21 +127,16 @@ test('want stable', async t => {
const psmInstance = await E(agoricNames).lookup('instance', 'psm-IST-AUSD');
const stableBrand = await E(agoricNames).lookup('brand', Stable.symbol);

const wallet = await t.context.simpleProvideWallet('agoric1wantstable');
const current = await E(E(wallet).getCurrentSubscriber())
.subscribeAfter()
.then(pub => pub.head.value);
const computedState = coalesceUpdates(E(wallet).getUpdatesSubscriber());
const { bank, wallet } = await t.context.provideBankAndWallet(
'agoric1wantstable',
);

const offersFacet = wallet.getOffersFacet();
t.assert(offersFacet, 'undefined offersFacet');
// let promises settle to notify brands and create purses
await eventLoopIteration();

t.deepEqual(current.purses.find(b => b.brand === anchor.brand).balance, {
brand: anchor.brand,
value: 0n,
});
t.is(await bankBalance(bank, anchor.brand), 0n);

t.log('Fund the wallet');
assert(anchor.mint);
Expand Down Expand Up @@ -173,8 +165,8 @@ test('want stable', async t => {
t.log('Execute the swap');
await offersFacet.executeOffer(offerSpec);
await eventLoopIteration();
t.is(purseBalance(computedState, anchor.brand), 0n);
t.is(purseBalance(computedState, stableBrand), swapSize); // assume 0% fee
t.is(await bankBalance(bank, anchor.brand), 0n);
t.is(await bankBalance(bank, stableBrand), swapSize); // assume 0% fee
});

test('govern offerFilter', async t => {
Expand All @@ -184,7 +176,10 @@ test('govern offerFilter', async t => {
const { psm: psmInstance } = await E(psmKit).get(anchor.brand);

const wallet = await t.context.simpleProvideWallet(committeeAddress);
const computedState = coalesceUpdates(E(wallet).getUpdatesSubscriber());
const computedState = coalesceUpdates(
E(wallet).getUpdatesSubscriber(),
invitationBrand,
);
const currentSub = E(wallet).getCurrentSubscriber();

const offersFacet = wallet.getOffersFacet();
Expand Down Expand Up @@ -212,9 +207,14 @@ test('govern offerFilter', async t => {
E(E(zoe).getInvitationIssuer())
.getBrand()
.then(brand => {
t.is(
brand,
invitationBrand,
'invitation brand from context matches zoe',
);
/** @type {Amount<'set'>} */
const invitationsAmount = NonNullish(balances.get(brand));
t.is(invitationsAmount?.value.length, len);
t.is(invitationsAmount?.value.length, len, 'invitaiton count');
return invitationsAmount.value.filter(i => i.description === desc);
});

Expand Down
116 changes: 27 additions & 89 deletions packages/smart-wallet/src/smartWallet.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ const mapToRecord = map => Object.fromEntries(map.entries());
* Purses is an array to support a future requirement of multiple purses per brand.
*
* @typedef {{
* brands: BrandDescriptor[],
* purses: Array<{brand: Brand, balance: Amount}>,
* offerToUsedInvitation: { [offerId: string]: Amount },
* offerToPublicSubscriberPaths: { [offerId: string]: { [subscriberName: string]: string } },
Expand All @@ -64,19 +63,13 @@ const mapToRecord = map => Object.fromEntries(map.entries());
*/

/**
* @typedef {{ updated: 'offerStatus', status: import('./offers.js').OfferStatus } |
* { updated: 'balance'; currentAmount: Amount } |
* { updated: 'brand', descriptor: BrandDescriptor }
* @typedef {{ updated: 'offerStatus', status: import('./offers.js').OfferStatus }
* } UpdateRecord Record of an update to the state of this wallet.
*
* Client is responsible for coalescing updates into a current state. See `coalesceUpdates` utility.
*
* The reason for this burden on the client is that transferring the full state is untenable
* (because it would grow monotonically).
*
* `balance` update supports forward-compatibility for more than one purse per
* brand. An additional key will be needed to disambiguate. For now the brand in
* the amount suffices.
* The reason for this burden on the client is that publishing
* the full history of offers with each change is untenable.
*/

/**
Expand Down Expand Up @@ -104,8 +97,9 @@ const mapToRecord = map => Object.fromEntries(map.entries());
* @typedef {{
* agoricNames: ERef<import('@agoric/vats').NameHub>,
* registry: {
* getRegisteredAsset: (b: Brand) => BrandDescriptor,
* getRegisteredBrands: () => BrandDescriptor[],
* has: (b: Brand) => boolean,
* get: (b: Brand) => BrandDescriptor,
* values: () => BrandDescriptor[],
* },
* invitationIssuer: Issuer<'set'>,
* invitationBrand: Brand<'set'>,
Expand All @@ -125,7 +119,6 @@ const mapToRecord = map => Object.fromEntries(map.entries());
* offerToInvitationMakers: MapStore<string, import('./types').RemoteInvitationMakers>,
* offerToPublicSubscriberPaths: MapStore<string, Record<string, string>>,
* offerToUsedInvitation: MapStore<string, Amount>,
* brandPurses: MapStore<Brand, RemotePurse>,
* purseBalances: MapStore<RemotePurse, Amount>,
* updatePublishKit: PublishKit<UpdateRecord>,
* currentPublishKit: PublishKit<CurrentWalletRecord>,
Expand Down Expand Up @@ -180,8 +173,6 @@ export const prepareSmartWallet = (baggage, shared) => {
);

const preciousState = {
// Private purses. This assumes one purse per brand, which will be valid in MN-1 but not always.
brandPurses: makeScalarBigMapStore('brand purses', { durable: true }),
// Payments that couldn't be deposited when received.
// NB: vulnerable to uncapped growth by unpermissioned deposits.
paymentQueues: makeScalarBigMapStore('payments queues', {
Expand Down Expand Up @@ -253,15 +244,7 @@ export const prepareSmartWallet = (baggage, shared) => {
helper: M.interface('helperFacetI', {
updateBalance: M.call(PurseShape, AmountShape).optional('init').returns(),
publishCurrentState: M.call().returns(),
addBrand: M.call(
{
brand: BrandShape,
issuer: M.eref(IssuerShape),
petname: M.string(),
displayInfo: DisplayInfoShape,
},
M.eref(PurseShape),
).returns(M.promise()),
watchPurse: M.call(M.eref(PurseShape)).returns(M.promise()),
}),
deposit: M.interface('depositFacetI', {
receive: M.callWhen(M.await(M.eref(PaymentShape))).returns(AmountShape),
Expand Down Expand Up @@ -321,10 +304,7 @@ export const prepareSmartWallet = (baggage, shared) => {
offerToPublicSubscriberPaths,
purseBalances,
} = this.state;
const { registry } = shared;

currentPublishKit.publisher.publish({
brands: registry.getRegisteredBrands(),
purses: [...purseBalances.values()].map(a => ({
brand: a.brand,
balance: a,
Expand All @@ -338,17 +318,12 @@ export const prepareSmartWallet = (baggage, shared) => {
});
},

/** @type {(desc: BrandDescriptor, purse: ERef<RemotePurse>) => Promise<void>} */
async addBrand(desc, purseRef) {
const { address, brandPurses, paymentQueues, updatePublishKit } =
this.state;
const pursesHas = brandPurses.has(desc.brand);
assert(!pursesHas, 'repeated brand from bank asset subscription');
/** @type {(purse: ERef<RemotePurse>) => Promise<void>} */
async watchPurse(purseRef) {
const { address } = this.state;

const purse = await purseRef; // promises don't fit in durable storage

brandPurses.init(desc.brand, purse);

const { helper } = this.facets;
// publish purse's balance and changes
void E.when(
Expand All @@ -369,21 +344,6 @@ export const prepareSmartWallet = (baggage, shared) => {
console.error(address, `failed updateState observer`, reason);
},
});

updatePublishKit.publisher.publish({
updated: 'brand',
descriptor: desc,
});

// deposit queued payments
const payments = paymentQueues.has(desc.brand)
? paymentQueues.get(desc.brand)
: [];
// @ts-expect-error xxx with DataOnly / FarRef types
const deposits = payments.map(p => E(purse).deposit(p));
Promise.all(deposits).catch(err =>
console.error('ERROR depositing queued payments', err),
);
},
},
/**
Expand All @@ -399,21 +359,25 @@ export const prepareSmartWallet = (baggage, shared) => {
* @returns {Promise<Amount>} amounts for deferred deposits will be empty
*/
async receive(payment) {
const { brandPurses, paymentQueues: queues } = this.state;
const { paymentQueues: queues, bank, invitationPurse } = this.state;
const { registry, invitationBrand } = shared;
const brand = await E(payment).getAllegedBrand();

// When there is a purse deposit into it
if (brandPurses.has(brand)) {
const purse = brandPurses.get(brand);
if (registry.has(brand)) {
const purse = E(bank).getPurse(brand);
// @ts-expect-error deposit does take a FarRef<Payment>
return E(purse).deposit(payment);
} else if (invitationBrand === brand) {
// @ts-expect-error deposit does take a FarRef<Payment>
return E(invitationPurse).deposit(payment);
}

// When there is no purse, queue the payment
if (queues.has(brand)) {
queues.get(brand).push(payment);
} else {
queues.init(brand, harden([payment]));
queues.init(brand, harden([payment])); // @@ this can't be right
}
return AmountMath.makeEmpty(brand);
},
Expand All @@ -437,7 +401,7 @@ export const prepareSmartWallet = (baggage, shared) => {
const { facets } = this;
const {
address,
brandPurses,
bank,
invitationPurse,
offerToInvitationMakers,
offerToUsedInvitation,
Expand Down Expand Up @@ -467,16 +431,13 @@ export const prepareSmartWallet = (baggage, shared) => {
* @returns {Promise<RemotePurse>}
*/
purseForBrand: async brand => {
if (brandPurses.has(brand)) {
return brandPurses.get(brand);
if (registry.has(brand)) {
return E(bank).getPurse(brand);
} else if (invitationBrand === brand) {
// @ts-expect-error RemotePurse cast
return invitationPurse;
}
const desc = registry.getRegisteredAsset(brand);
const { bank } = this.state;
/** @type {RemotePurse} */
// @ts-expect-error cast to RemotePurse
const purse = E(bank).getPurse(desc.brand);
await facets.helper.addBrand(desc, purse);
return purse;
throw assert.error(`cannot find/make purse for ${brand}`);
},
logger,
},
Expand Down Expand Up @@ -573,33 +534,10 @@ export const prepareSmartWallet = (baggage, shared) => {
},
{
finish: async ({ state, facets }) => {
const { invitationBrand, invitationDisplayInfo, invitationIssuer } =
shared;

const { invitationPurse } = state;
const { helper } = facets;

// Ensure a purse for each issuer
void helper.addBrand(
{
brand: invitationBrand,
issuer: invitationIssuer,
petname: 'invitations',
displayInfo: invitationDisplayInfo,
},
// @ts-expect-error cast to RemotePurse
/** @type {RemotePurse} */ (state.invitationPurse),
);

// Schedule creation of a purse for each registered brand.
shared.registry.getRegisteredBrands().forEach(desc => {
// In this sync method, we can't await the outcome.
void E(state.bank)
.getPurse(desc.brand)
// @ts-expect-error cast
.then((/** @type {RemotePurse} */ purse) =>
helper.addBrand(desc, purse),
);
});
helper.watchPurse(invitationPurse);
},
},
);
Expand Down
Loading

0 comments on commit 33814ad

Please sign in to comment.