From de9e7cd752b2a71de857bec1151663d98a5cbade Mon Sep 17 00:00:00 2001 From: Kate Sills Date: Tue, 1 Sep 2020 11:08:19 -0700 Subject: [PATCH 1/5] fix: kickOut does not throw itself --- packages/zoe/src/contractFacet/seat.js | 2 +- packages/zoe/src/contractFacet/types.js | 2 +- packages/zoe/src/contractSupport/zoeHelpers.js | 2 +- packages/zoe/src/contracts/auction/secondPriceLogic.js | 4 +++- 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/zoe/src/contractFacet/seat.js b/packages/zoe/src/contractFacet/seat.js index 3d7ed3c582d..3e7b5373a26 100644 --- a/packages/zoe/src/contractFacet/seat.js +++ b/packages/zoe/src/contractFacet/seat.js @@ -58,7 +58,7 @@ export const makeZcfSeatAdminKit = ( assertExitedFalse(); zcfSeatAdmin.updateHasExited(); E(zoeSeatAdmin).kickOut(harden(reason)); - throw reason; + return reason; }, getNotifier: () => { return notifier; diff --git a/packages/zoe/src/contractFacet/types.js b/packages/zoe/src/contractFacet/types.js index 5767c14ff31..ec5cf16e134 100644 --- a/packages/zoe/src/contractFacet/types.js +++ b/packages/zoe/src/contractFacet/types.js @@ -131,7 +131,7 @@ /** * @typedef {Object} ZCFSeat * @property {() => void} exit - * @property {(reason?: any) => never} kickOut called with the reason this + * @property {(reason?: Error) => Error} kickOut called with the reason this * seat is being kicked out, where reason is normally an instanceof Error. * @property {() => Notifier} getNotifier * @property {() => boolean} hasExited diff --git a/packages/zoe/src/contractSupport/zoeHelpers.js b/packages/zoe/src/contractSupport/zoeHelpers.js index 0ea60a38895..d314c27989d 100644 --- a/packages/zoe/src/contractSupport/zoeHelpers.js +++ b/packages/zoe/src/contractSupport/zoeHelpers.js @@ -165,7 +165,7 @@ export const trade = (zcf, keepLeft, tryRight) => { if (!offerSafeForRight) { console.log(`offer not safe for right`); } - return tryRight.seat.kickOut( + throw tryRight.seat.kickOut( new Error( `The trade between left ${keepLeft} and right ${tryRight} failed offer safety. Please check the log for more information`, ), diff --git a/packages/zoe/src/contracts/auction/secondPriceLogic.js b/packages/zoe/src/contracts/auction/secondPriceLogic.js index 1d84a62d183..33df5321fd7 100644 --- a/packages/zoe/src/contracts/auction/secondPriceLogic.js +++ b/packages/zoe/src/contracts/auction/secondPriceLogic.js @@ -29,7 +29,9 @@ export const calcWinnerAndClose = (zcf, sellSeat, bidSeats) => { }); if (activeBidsCount === 0) { - sellSeat.kickOut(new Error(`Could not close auction. No bids were active`)); + throw sellSeat.kickOut( + new Error(`Could not close auction. No bids were active`), + ); } if (activeBidsCount === 1) { From 25820386edab82dc60356d68203e08e48532ab6a Mon Sep 17 00:00:00 2001 From: Kate Sills Date: Tue, 1 Sep 2020 14:05:53 -0700 Subject: [PATCH 2/5] refactor: don't throw if seat that was kickedOut already exited. --- packages/zoe/src/contractFacet/contractFacet.js | 6 +----- packages/zoe/src/contractFacet/seat.js | 7 ++++--- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/packages/zoe/src/contractFacet/contractFacet.js b/packages/zoe/src/contractFacet/contractFacet.js index b8c45b129e8..9eb90c24b68 100644 --- a/packages/zoe/src/contractFacet/contractFacet.js +++ b/packages/zoe/src/contractFacet/contractFacet.js @@ -368,11 +368,7 @@ export function buildRootObject() { const offerHandler = invitationHandleToHandler.get(invitationHandle); // @ts-ignore const offerResultP = E(offerHandler)(zcfSeat).catch(reason => { - if (!zcfSeat.hasExited()) { - throw zcfSeat.kickOut(reason); - } else { - throw reason; - } + throw zcfSeat.kickOut(reason); }); const exitObj = makeExitObj( seatData.proposal, diff --git a/packages/zoe/src/contractFacet/seat.js b/packages/zoe/src/contractFacet/seat.js index 3e7b5373a26..21ca13991a6 100644 --- a/packages/zoe/src/contractFacet/seat.js +++ b/packages/zoe/src/contractFacet/seat.js @@ -55,9 +55,10 @@ export const makeZcfSeatAdminKit = ( 'Kicked out of seat. Please check the log for more information.', ), ) => { - assertExitedFalse(); - zcfSeatAdmin.updateHasExited(); - E(zoeSeatAdmin).kickOut(harden(reason)); + if (!exited) { + zcfSeatAdmin.updateHasExited(); + E(zoeSeatAdmin).kickOut(harden(reason)); + } return reason; }, getNotifier: () => { From 2a8983f67ef480174d1bbf35812b491632fbeb0a Mon Sep 17 00:00:00 2001 From: Kate Sills Date: Tue, 1 Sep 2020 15:53:14 -0700 Subject: [PATCH 3/5] chore: add tests for kicking out multiple seats in one go --- .../zoe/test/unitTests/contracts/test-zcf.js | 43 +++++++++++++++++-- .../unitTests/contracts/zcfTesterContract.js | 9 ++-- 2 files changed, 44 insertions(+), 8 deletions(-) diff --git a/packages/zoe/test/unitTests/contracts/test-zcf.js b/packages/zoe/test/unitTests/contracts/test-zcf.js index 9d4b4cebfec..a0eace9d218 100644 --- a/packages/zoe/test/unitTests/contracts/test-zcf.js +++ b/packages/zoe/test/unitTests/contracts/test-zcf.js @@ -3,6 +3,7 @@ import '@agoric/install-ses'; // eslint-disable-next-line import/no-extraneous-dependencies import test from 'ava'; +import { E } from '@agoric/eventual-send'; import bundleSource from '@agoric/bundle-source'; // noinspection ES6PreferShortImport @@ -12,7 +13,7 @@ import fakeVatAdmin from './fakeVatAdmin'; const contractRoot = `${__dirname}/zcfTesterContract`; -test('zoe - test zcf', async t => { +test(`zoe - zcfSeat.kickOut() doesn't throw`, async t => { t.plan(1); const { moolaIssuer, simoleanIssuer } = setup(); const zoe = makeZoe(fakeVatAdmin); @@ -27,7 +28,43 @@ test('zoe - test zcf', async t => { Pixels: moolaIssuer, Money: simoleanIssuer, }); - await t.notThrowsAsync(() => - zoe.startInstance(installation, issuerKeywordRecord), + + const { creatorFacet } = await E(zoe).startInstance( + installation, + issuerKeywordRecord, ); + + // This contract gives ZCF as the contractFacet for testing purposes + /** @type ContractFacet */ + const zcf = creatorFacet; + + let firstSeat; + + const grabSeat = seat => { + firstSeat = seat; + return 'ok'; + }; + + const kickOutSeat = secondSeat => { + firstSeat.kickOut(new Error('kicked out first')); + throw secondSeat.kickOut(new Error('kicked out second')); + }; + + const invitation1 = zcf.makeInvitation(grabSeat, 'seat1'); + const invitation2 = zcf.makeInvitation(kickOutSeat, 'seat2'); + + const userSeat1 = await E(zoe).offer(invitation1); + const userSeat2 = await E(zoe).offer(invitation2); + const userSeat1Result = await E(userSeat1).getOfferResult(); + + t.is(userSeat1Result, 'ok'); + + await E(userSeat2).getPayouts(); + // Results in "Unhandled rejection" + // E(userSeat2).getOfferResult(); + + await t.throwsAsync(() => E(userSeat2).getOfferResult()); + await t.throwsAsync(() => E(userSeat1).tryExit(), { + message: 'seat has been exited', + }); }); diff --git a/packages/zoe/test/unitTests/contracts/zcfTesterContract.js b/packages/zoe/test/unitTests/contracts/zcfTesterContract.js index e6d18334064..d9c7501a953 100644 --- a/packages/zoe/test/unitTests/contracts/zcfTesterContract.js +++ b/packages/zoe/test/unitTests/contracts/zcfTesterContract.js @@ -1,14 +1,13 @@ // @ts-check +import '../../../exported'; + /** * Tests ZCF * @type {ContractStartFn} */ -const start = _zcf => { - // TODO: import tap/tape and do t.is - // TODO: Test ZCF here - - return harden({}); +const start = zcf => { + return { creatorFacet: zcf }; }; harden(start); From 2cca892773d1c813d37dbe9288da71bceda78da2 Mon Sep 17 00:00:00 2001 From: Kate Sills Date: Tue, 1 Sep 2020 17:10:19 -0700 Subject: [PATCH 4/5] chore: change the tests slightly, remove a console.log that shouldn't be there --- packages/zoe/src/zoeService/zoe.js | 1 - packages/zoe/test/unitTests/contracts/test-zcf.js | 13 +++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/zoe/src/zoeService/zoe.js b/packages/zoe/src/zoeService/zoe.js index 4f772d3737d..754cdf905c9 100644 --- a/packages/zoe/src/zoeService/zoe.js +++ b/packages/zoe/src/zoeService/zoe.js @@ -430,7 +430,6 @@ function makeZoe(vatAdminSvc, zcfBundleName = undefined) { }) .catch(err => { console.log(err); - console.log('right here'); }); return userSeat; diff --git a/packages/zoe/test/unitTests/contracts/test-zcf.js b/packages/zoe/test/unitTests/contracts/test-zcf.js index a0eace9d218..55620f63bb8 100644 --- a/packages/zoe/test/unitTests/contracts/test-zcf.js +++ b/packages/zoe/test/unitTests/contracts/test-zcf.js @@ -55,16 +55,17 @@ test(`zoe - zcfSeat.kickOut() doesn't throw`, async t => { const userSeat1 = await E(zoe).offer(invitation1); const userSeat2 = await E(zoe).offer(invitation2); - const userSeat1Result = await E(userSeat1).getOfferResult(); - t.is(userSeat1Result, 'ok'); + t.is(await E(userSeat1).getOfferResult(), 'ok', `userSeat1 offer result`); - await E(userSeat2).getPayouts(); + // await E(userSeat2).getPayouts(); // Results in "Unhandled rejection" // E(userSeat2).getOfferResult(); + t.deepEqual(await E(userSeat1).getPayouts(), {}); + await t.throwsAsync(() => E(userSeat2).getOfferResult()); - await t.throwsAsync(() => E(userSeat1).tryExit(), { - message: 'seat has been exited', - }); + // await t.throwsAsync(() => E(userSeat1).tryExit(), { + // message: 'seat has been exited', + // }); }); From a4b2dc1168640f9e24c7ab5768313b05a97995a1 Mon Sep 17 00:00:00 2001 From: Kate Sills Date: Tue, 1 Sep 2020 17:57:02 -0700 Subject: [PATCH 5/5] chore: remove t.plan that was there unintentionally from prior tests --- .../zoe/test/unitTests/contracts/test-zcf.js | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/packages/zoe/test/unitTests/contracts/test-zcf.js b/packages/zoe/test/unitTests/contracts/test-zcf.js index 55620f63bb8..c0989993a05 100644 --- a/packages/zoe/test/unitTests/contracts/test-zcf.js +++ b/packages/zoe/test/unitTests/contracts/test-zcf.js @@ -14,7 +14,6 @@ import fakeVatAdmin from './fakeVatAdmin'; const contractRoot = `${__dirname}/zcfTesterContract`; test(`zoe - zcfSeat.kickOut() doesn't throw`, async t => { - t.plan(1); const { moolaIssuer, simoleanIssuer } = setup(); const zoe = makeZoe(fakeVatAdmin); @@ -50,22 +49,18 @@ test(`zoe - zcfSeat.kickOut() doesn't throw`, async t => { throw secondSeat.kickOut(new Error('kicked out second')); }; - const invitation1 = zcf.makeInvitation(grabSeat, 'seat1'); - const invitation2 = zcf.makeInvitation(kickOutSeat, 'seat2'); + const invitation1 = await zcf.makeInvitation(grabSeat, 'seat1'); + const invitation2 = await zcf.makeInvitation(kickOutSeat, 'seat2'); const userSeat1 = await E(zoe).offer(invitation1); const userSeat2 = await E(zoe).offer(invitation2); t.is(await E(userSeat1).getOfferResult(), 'ok', `userSeat1 offer result`); - // await E(userSeat2).getPayouts(); - // Results in "Unhandled rejection" - // E(userSeat2).getOfferResult(); + t.deepEqual(await E(userSeat2).getPayouts(), {}); - t.deepEqual(await E(userSeat1).getPayouts(), {}); - - await t.throwsAsync(() => E(userSeat2).getOfferResult()); - // await t.throwsAsync(() => E(userSeat1).tryExit(), { - // message: 'seat has been exited', - // }); + await t.throwsAsync(E(userSeat2).getOfferResult()); + await t.throwsAsync(() => E(userSeat1).tryExit(), { + message: 'seat has been exited', + }); });