From 7c22b24a7d91b1e2f57b6c7707e10f67a20d0c1f Mon Sep 17 00:00:00 2001 From: Kate Sills Date: Tue, 4 May 2021 16:10:55 -0700 Subject: [PATCH 1/3] fix!: rights conservation should allow for the introduction or drop of brands as long as the sum is empty --- .../src/contractFacet/rightsConservation.js | 51 ++++++++---- .../unitTests/zcf/test-reallocate-empty.js | 80 +++++++++++++++++++ 2 files changed, 115 insertions(+), 16 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..5ee3db06b02 100644 --- a/packages/zoe/src/contractFacet/rightsConservation.js +++ b/packages/zoe/src/contractFacet/rightsConservation.js @@ -37,24 +37,43 @@ const sumByBrand = amounts => { * 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 => + // 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 sum for the specified brand, or if + * the brand is absent in the map, returns an empty amount. + * + * @param {Store} sumsByBrandMap + * @param {Brand} brand + * @param {Amount} amount + * @returns {Amount} + */ + const getOrEmpty = (sumsByBrandMap, brand, amount) => { + if (!sumsByBrandMap.has(brand)) { + return amountMath.makeEmptyFromAmount(amount); + } + return sumsByBrandMap.get(brand); + }; + + const assertSumsEqualInMap = (mapToIterate, mapToCheck) => { + mapToIterate.keys().forEach(brand => { + const toIterateSum = mapToIterate.get(brand); + const toCheckSumOrEmpty = getOrEmpty(mapToCheck, brand, toIterateSum); assert( - amountMath.isEqual( - leftSumsByBrand.get(brand), - rightSumsByBrand.get(brand), - ), + amountMath.isEqual(toIterateSum, toCheckSumOrEmpty), X`rights were not conserved for brand ${brand}`, - ), - ); + ); + }); + }; + + assertSumsEqualInMap(leftSumsByBrand, rightSumsByBrand); + assertSumsEqualInMap(rightSumsByBrand, leftSumsByBrand); }; /** 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), + }); +}); From 940f33db2b9e2326bbcbc74262001b44740fbc99 Mon Sep 17 00:00:00 2001 From: Kate Sills Date: Tue, 4 May 2021 19:40:32 -0700 Subject: [PATCH 2/3] chore: use a set of allBrands --- .../src/contractFacet/rightsConservation.js | 49 +++++++++++-------- 1 file changed, 29 insertions(+), 20 deletions(-) diff --git a/packages/zoe/src/contractFacet/rightsConservation.js b/packages/zoe/src/contractFacet/rightsConservation.js index 5ee3db06b02..fb6e77cda69 100644 --- a/packages/zoe/src/contractFacet/rightsConservation.js +++ b/packages/zoe/src/contractFacet/rightsConservation.js @@ -46,34 +46,43 @@ const assertEqualPerBrand = (leftSumsByBrand, rightSumsByBrand) => { // only if the sum for the brand in the other map is empty. /** - * A helper that either gets the sum for the specified brand, or if + * 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 {Store} sumsByBrandMap * @param {Brand} brand - * @param {Amount} amount - * @returns {Amount} + * @returns {{ leftSum: Amount, rightSum: Amount }} */ - const getOrEmpty = (sumsByBrandMap, brand, amount) => { - if (!sumsByBrandMap.has(brand)) { - return amountMath.makeEmptyFromAmount(amount); + const getSums = brand => { + let leftSum; + let rightSum; + if (leftSumsByBrand.has(brand)) { + leftSum = leftSumsByBrand.get(brand); } - return sumsByBrandMap.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 assertSumsEqualInMap = (mapToIterate, mapToCheck) => { - mapToIterate.keys().forEach(brand => { - const toIterateSum = mapToIterate.get(brand); - const toCheckSumOrEmpty = getOrEmpty(mapToCheck, brand, toIterateSum); - assert( - amountMath.isEqual(toIterateSum, toCheckSumOrEmpty), - X`rights were not conserved for brand ${brand}`, - ); - }); - }; + const allBrands = new Set([ + ...leftSumsByBrand.keys(), + ...rightSumsByBrand.keys(), + ]); - assertSumsEqualInMap(leftSumsByBrand, rightSumsByBrand); - assertSumsEqualInMap(rightSumsByBrand, leftSumsByBrand); + allBrands.forEach(brand => { + const { leftSum, rightSum } = getSums(brand); + assert( + amountMath.isEqual(leftSum, rightSum), + X`rights were not conserved for brand ${brand}`, + ); + }); }; /** From 2331e5ea74b0518ab62279cd5b436d58b2a4c002 Mon Sep 17 00:00:00 2001 From: Kate Sills Date: Wed, 5 May 2021 14:29:18 -0700 Subject: [PATCH 3/3] chore: delete comment on line 37 --- packages/zoe/src/contractFacet/rightsConservation.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/zoe/src/contractFacet/rightsConservation.js b/packages/zoe/src/contractFacet/rightsConservation.js index fb6e77cda69..906666e2f8c 100644 --- a/packages/zoe/src/contractFacet/rightsConservation.js +++ b/packages/zoe/src/contractFacet/rightsConservation.js @@ -34,7 +34,6 @@ 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) => { // We cannot assume that all of the brand keys present in