From 56c594330f9d6f72930a18478a476c213c2bbbda Mon Sep 17 00:00:00 2001 From: Kate Sills Date: Wed, 5 May 2021 14:44:48 -0700 Subject: [PATCH] fix!: rights conservation should allow new brands or dropped brands if sum is empty (#3036) * fix!: rights conservation should allow for the introduction or drop of brands as long as the sum is empty * chore: use a set of allBrands --- .../src/contractFacet/rightsConservation.js | 63 ++++++++++----- .../unitTests/zcf/test-reallocate-empty.js | 80 +++++++++++++++++++ 2 files changed, 125 insertions(+), 18 deletions(-) create mode 100644 packages/zoe/test/unitTests/zcf/test-reallocate-empty.js diff --git a/packages/zoe/src/contractFacet/rightsConservation.js b/packages/zoe/src/contractFacet/rightsConservation.js index e91b4f3029f..906666e2f8c 100644 --- a/packages/zoe/src/contractFacet/rightsConservation.js +++ b/packages/zoe/src/contractFacet/rightsConservation.js @@ -34,27 +34,54 @@ const sumByBrand = amounts => { * * @param {Store} leftSumsByBrand - a map of brands to sums * @param {Store} rightSumsByBrand - a map of brands to sums - * indexed by issuer */ const assertEqualPerBrand = (leftSumsByBrand, rightSumsByBrand) => { - const leftKeys = leftSumsByBrand.keys(); - const rightKeys = rightSumsByBrand.keys(); - assert.equal( - leftKeys.length, - rightKeys.length, - X`${leftKeys.length} should be equal to ${rightKeys.length}`, - ); - leftSumsByBrand - .keys() - .forEach(brand => - assert( - amountMath.isEqual( - leftSumsByBrand.get(brand), - rightSumsByBrand.get(brand), - ), - X`rights were not conserved for brand ${brand}`, - ), + // We cannot assume that all of the brand keys present in + // leftSumsByBrand are also present in rightSumsByBrand. A empty + // amount could be introduced or dropped, and this should still be + // deemed "equal" from the perspective of rights conservation. + + // Thus, we should allow for a brand to be missing from a map, but + // only if the sum for the brand in the other map is empty. + + /** + * A helper that either gets the sums for the specified brand, or if + * the brand is absent in the map, returns an empty amount. + * + * @param {Brand} brand + * @returns {{ leftSum: Amount, rightSum: Amount }} + */ + const getSums = brand => { + let leftSum; + let rightSum; + if (leftSumsByBrand.has(brand)) { + leftSum = leftSumsByBrand.get(brand); + } + if (rightSumsByBrand.has(brand)) { + rightSum = rightSumsByBrand.get(brand); + } + if (leftSum === undefined) { + assert(rightSum); + leftSum = amountMath.makeEmptyFromAmount(rightSum); + } + if (rightSum === undefined) { + rightSum = amountMath.makeEmptyFromAmount(leftSum); + } + return { leftSum, rightSum }; + }; + + const allBrands = new Set([ + ...leftSumsByBrand.keys(), + ...rightSumsByBrand.keys(), + ]); + + allBrands.forEach(brand => { + const { leftSum, rightSum } = getSums(brand); + assert( + amountMath.isEqual(leftSum, rightSum), + X`rights were not conserved for brand ${brand}`, ); + }); }; /** diff --git a/packages/zoe/test/unitTests/zcf/test-reallocate-empty.js b/packages/zoe/test/unitTests/zcf/test-reallocate-empty.js new file mode 100644 index 00000000000..6bce71a347f --- /dev/null +++ b/packages/zoe/test/unitTests/zcf/test-reallocate-empty.js @@ -0,0 +1,80 @@ +// @ts-check +// eslint-disable-next-line import/no-extraneous-dependencies +import { test } from '@agoric/zoe/tools/prepare-test-env-ava'; + +import { amountMath } from '@agoric/ertp'; + +import { setupZCFTest } from './setupZcfTest'; + +test(`zcfSeat.stage, zcf.reallocate introducing new empty amount`, async t => { + const { zcf } = await setupZCFTest(); + const { zcfSeat: zcfSeat1 } = zcf.makeEmptySeatKit(); + const { zcfSeat: zcfSeat2 } = zcf.makeEmptySeatKit(); + const zcfMint = await zcf.makeZCFMint('RUN'); + const { brand } = zcfMint.getIssuerRecord(); + + // Get the amount allocated on zcfSeat1. It is empty for the RUN brand. + const allocation = zcfSeat1.getAmountAllocated('RUN', brand); + t.true(amountMath.isEmpty(allocation)); + + // Stage zcfSeat2 with the allocation from zcfSeat1 + const zcfSeat2Staging = zcfSeat2.stage({ RUN: allocation }); + + // Stage zcfSeat1 with empty + const zcfSeat1Staging = zcfSeat1.stage({ RUN: amountMath.makeEmpty(brand) }); + + zcf.reallocate(zcfSeat1Staging, zcfSeat2Staging); + + t.deepEqual(zcfSeat1.getCurrentAllocation(), { + RUN: amountMath.make(0n, brand), + }); + t.deepEqual(zcfSeat2.getCurrentAllocation(), { + RUN: amountMath.make(0n, brand), + }); +}); + +test(`zcfSeat.stage, zcf.reallocate "dropping" empty amount`, async t => { + const { zcf } = await setupZCFTest(); + const { zcfSeat: zcfSeat1 } = zcf.makeEmptySeatKit(); + const { zcfSeat: zcfSeat2 } = zcf.makeEmptySeatKit(); + const zcfMint = await zcf.makeZCFMint('RUN'); + const { brand } = zcfMint.getIssuerRecord(); + + zcfMint.mintGains({ RUN: amountMath.make(brand, 0n) }, zcfSeat1); + zcfMint.mintGains({ RUN: amountMath.make(brand, 0n) }, zcfSeat2); + + // Now zcfSeat1 and zcfSeat2 both have an empty allocation for RUN. + t.deepEqual(zcfSeat1.getCurrentAllocation(), { + RUN: amountMath.make(0n, brand), + }); + t.deepEqual(zcfSeat2.getCurrentAllocation(), { + RUN: amountMath.make(0n, brand), + }); + + // Stage zcfSeat1 with an entirely empty allocation + const zcfSeat1Staging = zcfSeat2.stage({}); + + // Stage zcfSeat2 with an entirely empty allocation + const zcfSeat2Staging = zcfSeat1.stage({}); + + // Because of how we merge staged allocations with the current + // allocation (we don't delete keys), the RUN keyword still remains: + t.deepEqual(zcfSeat1Staging.getStagedAllocation(), { + RUN: amountMath.make(0n, brand), + }); + t.deepEqual(zcfSeat2Staging.getStagedAllocation(), { + RUN: amountMath.make(0n, brand), + }); + + zcf.reallocate(zcfSeat1Staging, zcfSeat2Staging); + + // The reallocation succeeds without error, but because of how we + // merge new allocations with old allocations (we don't delete + // keys), the RUN keyword still remains as is. + t.deepEqual(zcfSeat1.getCurrentAllocation(), { + RUN: amountMath.make(0n, brand), + }); + t.deepEqual(zcfSeat2.getCurrentAllocation(), { + RUN: amountMath.make(0n, brand), + }); +});