Skip to content

Commit

Permalink
fix: move the assertion that allStagedSeatsUsed into reallocate r…
Browse files Browse the repository at this point in the history
…ather than `reallocateInternal` (#3615)
  • Loading branch information
katelynsills authored Aug 6, 2021
1 parent 4400288 commit f8d62cc
Show file tree
Hide file tree
Showing 2 changed files with 102 additions and 10 deletions.
22 changes: 12 additions & 10 deletions packages/zoe/src/contractFacet/zcfSeat.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ export const createSeatManager = (
seats.forEach(assertStagedAllocation);

// Keep track of seats used so far in this call, to prevent aliasing.
const zcfSeatsSoFar = new WeakSet();
const zcfSeatsSoFar = new Set();

seats.forEach(seat => {
assert(
Expand All @@ -138,15 +138,6 @@ export const createSeatManager = (
zcfSeatsSoFar.add(seat);
});

// Ensure that all stagings are present in this reallocate call.
const allStagedSeatsUsed = zcfSeatToStagedAllocations
.keys()
.every(stagedSeat => zcfSeatsSoFar.has(stagedSeat));
assert(
allStagedSeatsUsed,
X`At least one seat has a staged allocation but was not included in the call to reallocate`,
);

try {
// No side effects above. All conditions checked which could have
// caused us to reject this reallocation.
Expand Down Expand Up @@ -206,6 +197,17 @@ export const createSeatManager = (
);
});

const zcfSeatsReallocatedOver = new Set(seats);

// Ensure that all stagings are present in this reallocate call.
const allStagedSeatsUsed = zcfSeatToStagedAllocations
.keys()
.every(stagedSeat => zcfSeatsReallocatedOver.has(stagedSeat));
assert(
allStagedSeatsUsed,
X`At least one seat has a staged allocation but was not included in the call to reallocate`,
);

// Note COMMIT POINT within reallocateInternal
reallocateInternal(seats);
};
Expand Down
90 changes: 90 additions & 0 deletions packages/zoe/test/unitTests/zcf/test-allStagedSeatsUsed.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
// @ts-check
// eslint-disable-next-line import/no-extraneous-dependencies
import { test } from '@agoric/zoe/tools/prepare-test-env-ava.js';

import { AssetKind, AmountMath } from '@agoric/ertp';
import { makeOffer } from '../makeOffer.js';

import { setup } from '../setupBasicMints.js';

import { setupZCFTest } from './setupZcfTest.js';

test(`allStagedSeatsUsed should not be asserted`, async t => {
const { moolaKit, moola } = setup();
const { zoe, zcf } = await setupZCFTest({
Moola: moolaKit.issuer,
});

const { zcfSeat: zcfSeat1 } = await makeOffer(
zoe,
zcf,
harden({ give: { B: moola(3) } }),
harden({ B: moolaKit.mint.mintPayment(moola(3)) }),
);

const { zcfSeat: zcfSeat2 } = await makeOffer(
zoe,
zcf,
harden({ give: { B: moola(3) } }),
harden({ B: moolaKit.mint.mintPayment(moola(3)) }),
);

zcfSeat1.incrementBy(zcfSeat2.decrementBy({ B: moola(2) }));
// Seats have staged allocations
t.true(zcfSeat1.hasStagedAllocation());

const zcfMint = await zcf.makeZCFMint('Token', AssetKind.NAT, {
decimalPlaces: 6,
});

const { brand: tokenBrand } = zcfMint.getIssuerRecord();

const zcfSeat3 = zcfMint.mintGains({
MyToken: AmountMath.make(tokenBrand, 100n),
});

// This test was failing due to this bug: https://github.com/Agoric/agoric-sdk/issues/3613
t.deepEqual(zcfSeat3.getCurrentAllocation(), {
MyToken: AmountMath.make(tokenBrand, 100n),
});
});

test(`allStagedSeatsUsed should be asserted`, async t => {
const { moolaKit, moola } = setup();
const { zoe, zcf } = await setupZCFTest({
Moola: moolaKit.issuer,
});

const { zcfSeat: zcfSeat1 } = await makeOffer(
zoe,
zcf,
harden({ give: { B: moola(3) } }),
harden({ B: moolaKit.mint.mintPayment(moola(3)) }),
);

const { zcfSeat: zcfSeat2 } = await makeOffer(
zoe,
zcf,
harden({ give: { B: moola(3) } }),
harden({ B: moolaKit.mint.mintPayment(moola(3)) }),
);

const { zcfSeat: zcfSeat3 } = await makeOffer(
zoe,
zcf,
harden({ give: { B: moola(3) } }),
harden({ B: moolaKit.mint.mintPayment(moola(3)) }),
);

zcfSeat1.incrementBy(zcfSeat2.decrementBy({ B: moola(2) }));

zcfSeat3.incrementBy({ B: moola(3) });

t.true(zcfSeat3.hasStagedAllocation());

// zcfSeat3 has a staged allocation but was not included in reallocate
t.throws(() => zcf.reallocate(zcfSeat1, zcfSeat2), {
message:
'At least one seat has a staged allocation but was not included in the call to reallocate',
});
});

0 comments on commit f8d62cc

Please sign in to comment.