Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix fullOrder leak. Semi-fungibles via CopyBags #4305

Merged
merged 1 commit into from
Jan 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion packages/ERTP/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@
"devDependencies": {
"@agoric/bundle-source": "^2.0.1",
"@agoric/swingset-vat": "^0.24.1",
"ava": "^3.12.1"
"ava": "^3.12.1",
"fast-check": "^2.21.0"
},
"files": [
"src",
Expand Down
23 changes: 16 additions & 7 deletions packages/ERTP/src/amountMath.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,20 @@ import { M, matches } from '@agoric/store';
import { natMathHelpers } from './mathHelpers/natMathHelpers.js';
import { setMathHelpers } from './mathHelpers/setMathHelpers.js';
import { copySetMathHelpers } from './mathHelpers/copySetMathHelpers.js';
import { copyBagMathHelpers } from './mathHelpers/copyBagMathHelpers.js';

const { details: X, quote: q } = assert;

/**
* Constants for the kinds of assets we support.
*
* @type {{ NAT: 'nat', SET: 'set', COPY_SET: 'copySet' }}
* @type {{ NAT: 'nat', SET: 'set', COPY_SET: 'copySet', COPY_BAG: 'copyBag' }}
*/
const AssetKind = harden({
NAT: 'nat',
SET: 'set',
COPY_SET: 'copySet',
COPY_BAG: 'copyBag',
});
const assetKindNames = harden(Object.values(AssetKind).sort());

Expand Down Expand Up @@ -72,12 +74,14 @@ harden(assertAssetKind);
/** @type {{
* nat: NatMathHelpers,
* set: SetMathHelpers,
* copySet: CopySetMathHelpers }
* } */
* copySet: CopySetMathHelpers,
* copyBag: CopyBagMathHelpers
* }} */
const helpers = {
nat: natMathHelpers,
set: setMathHelpers,
copySet: copySetMathHelpers,
copyBag: copyBagMathHelpers,
};

/** @type {(value: AmountValue) => AssetKind} */
Expand All @@ -92,25 +96,30 @@ const assertValueGetAssetKind = value => {
if (matches(value, M.set())) {
return 'copySet';
}
if (matches(value, M.bag())) {
return 'copyBag';
}
assert.fail(
// TODO This isn't quite the right error message, in case valuePassStyle
// is 'tagged'. We would need to distinguish what kind of tagged
// object it is.
// Also, this kind of manual listing is a maintenance hazard we
// (TODO) will encounter when we extend the math helpers to
// include CopyMaps.
X`value ${value} must be a bigint, copySet, or an array, not ${passStyle}`,
// (TODO) will encounter when we extend the math helpers further.
X`value ${value} must be a bigint, copySet, copyBag, or an array, not ${passStyle}`,
);
};

/**
*
* Asserts that value is a valid AmountMath and returns the appropriate helpers.
*
* Made available only for testing, but it is harmless for other uses.
*
* @param {AmountValue} value
* @returns {MathHelpers<*>}
*/
const assertValueGetHelpers = value => helpers[assertValueGetAssetKind(value)];
export const assertValueGetHelpers = value =>
helpers[assertValueGetAssetKind(value)];

/** @type {(allegedBrand: Brand, brand?: Brand) => void} */
const optionalBrandCheck = (allegedBrand, brand) => {
Expand Down
32 changes: 32 additions & 0 deletions packages/ERTP/src/mathHelpers/copyBagMathHelpers.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// @ts-check

import {
keyEQ,
makeCopyBag,
fit,
M,
getCopyBagEntries,
bagIsSuperbag,
bagUnion,
bagDisjointSubtract,
} from '@agoric/store';
import '../types.js';

/** @type {CopyBagValue} */
const empty = makeCopyBag([]);

/**
* @type {CopyBagMathHelpers}
*/
export const copyBagMathHelpers = harden({
doCoerce: bag => {
fit(bag, M.bag());
return bag;
},
doMakeEmpty: () => empty,
doIsEmpty: bag => getCopyBagEntries(bag).length === 0,
doIsGTE: bagIsSuperbag,
doIsEqual: keyEQ,
doAdd: bagUnion,
doSubtract: bagDisjointSubtract,
});
18 changes: 4 additions & 14 deletions packages/ERTP/src/mathHelpers/copySetMathHelpers.js
Original file line number Diff line number Diff line change
@@ -1,30 +1,20 @@
// @ts-check

import {
makeSetOps,
keyEQ,
makeCopySet,
fit,
M,
getCopySetKeys,
makeFullOrderComparatorKit,
setIsSuperset,
setDisjointUnion,
setDisjointSubtract,
} from '@agoric/store';
import '../types.js';

/** @type {CopySetValue} */
const empty = makeCopySet([]);

/**
* TODO SECURITY BUG: https://github.com/Agoric/agoric-sdk/issues/4261
* This creates observable mutable static state, in the
* history-based ordering of remotables.
*/
const fullCompare = makeFullOrderComparatorKit(true).antiComparator;

const { isSetSuperset, setDisjointUnion, setDisjointSubtract } = makeSetOps(
fullCompare,
);

/**
* @type {CopySetMathHelpers}
*/
Expand All @@ -35,7 +25,7 @@ export const copySetMathHelpers = harden({
},
doMakeEmpty: () => empty,
doIsEmpty: set => getCopySetKeys(set).length === 0,
doIsGTE: isSetSuperset,
doIsGTE: setIsSuperset,
doIsEqual: keyEQ,
doAdd: setDisjointUnion,
doSubtract: setDisjointSubtract,
Expand Down
44 changes: 11 additions & 33 deletions packages/ERTP/src/mathHelpers/setMathHelpers.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
// @ts-check

import { assertChecker, passStyleOf } from '@agoric/marshal';
import { passStyleOf } from '@agoric/marshal';
import {
assertKey,
makeSetOps,
sortByRank,
keyEQ,
makeFullOrderComparatorKit,
elementsIsSuperset,
elementsDisjointUnion,
elementsDisjointSubtract,
coerceToElements,
elementsCompare,
} from '@agoric/store';
import '../types.js';

Expand All @@ -15,47 +16,24 @@ import '../types.js';
/** @type {SetValue} */
const empty = harden([]);

/**
* TODO SECURITY BUG: https://github.com/Agoric/agoric-sdk/issues/4261
* This creates observable mutable static state, in the
* history-based ordering of remotables.
*/
const fullCompare = makeFullOrderComparatorKit(true).antiComparator;

const {
checkNoDuplicates,
isListSuperset,
listDisjointUnion,
listDisjointSubtract,
} = makeSetOps(fullCompare);

const assertNoDuplicates = list => checkNoDuplicates(list, assertChecker);

/**
* @deprecated Replace array-based SetMath with CopySet-based CopySetMath
* @type {SetMathHelpers}
*/
export const setMathHelpers = harden({
doCoerce: list => {
assert(
passStyleOf(list) === 'copyArray',
`The value of a non-fungible token must be an array but was ${list}`,
);
list = coerceToElements(list);
// Assert that list contains only
// * pass-by-copy primitives,
// * pass-by-copy containers containing keys,
// * remotables.
assertKey(list);
list = sortByRank(list, fullCompare);
// As a coercion, should we also deduplicate here? Might mask bugs
// elsewhere though, so probably not.
assertNoDuplicates(list);
return list;
},
doMakeEmpty: () => empty,
doIsEmpty: list => passStyleOf(list) === 'copyArray' && list.length === 0,
doIsGTE: isListSuperset,
doIsEqual: keyEQ,
doAdd: listDisjointUnion,
doSubtract: listDisjointSubtract,
doIsGTE: elementsIsSuperset,
doIsEqual: (x, y) => elementsCompare(x, y) === 0,
doAdd: elementsDisjointUnion,
doSubtract: elementsDisjointSubtract,
});
40 changes: 38 additions & 2 deletions packages/ERTP/src/typeGuards.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const NatValueShape = M.nat();
/**
* When the AmountValue of an Amount fits the CopySetValueShape, i.e., when it
* is a CopySet, then it represents the set of those
* keys, where each key represents some individual non fungible
* keys, where each key represents some individual non-fungible
* item, like a concert ticket, from the non-fungible asset class
* represented by that amount's brand. The amount itself represents
* the set of these items, as opposed to any of the other items
Expand All @@ -38,7 +38,34 @@ const CopySetValueShape = M.set();
*/
const SetValueShape = M.arrayOf(M.key());

const AmountValueShape = M.or(NatValueShape, CopySetValueShape, SetValueShape);
/**
* When the AmountValue of an Amount fits the CopyBagValueShape, i.e., when it
* is a CopyBag, then it represents the bag (multiset) of those
* keys, where each key represents some individual semi-fungible
* item, like a concert ticket, from the semi-fungible asset class
* represented by that amount's brand. The number of times that key
* appears in the bag is the number of fungible units of that key.
* The amount itself represents
* the bag of these items, as opposed to any of the other items
* from the same asset class.
*
* If a given value class represents concert tickets, it seems bizarre
* that we can form amounts of any key. The hard constraint is that
* the code that holds the mint for that asset class---the one associated
* with that brand, only mints the items representing the real units
* of that asset class as defined by it. Anyone else can put together
* an amount expressing, for example, that they "want" some items that
* will never be minted. That want will never be satisfied.
* "You can't always get..."
*/
const CopyBagValueShape = M.bagOf();

const AmountValueShape = M.or(
NatValueShape,
CopySetValueShape,
SetValueShape,
CopyBagValueShape,
);

export const AmountShape = harden({
brand: M.remotable(),
Expand Down Expand Up @@ -74,3 +101,12 @@ harden(isCopySetValue);
*/
export const isSetValue = value => matches(value, SetValueShape);
harden(isSetValue);

/**
* Returns true if value is a CopyBag
*
* @param {AmountValue} value
* @returns {value is CopyBagValue}
*/
export const isCopyBagValue = value => matches(value, CopyBagValueShape);
harden(isCopyBagValue);
14 changes: 11 additions & 3 deletions packages/ERTP/src/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
*/

/**
* @typedef {NatValue | SetValue | CopySetValue} AmountValue
* @typedef {NatValue | SetValue | CopySetValue | CopyBagValue} AmountValue
* An `AmountValue` describes a set or quantity of assets that can be owned or
* shared.
*
Expand All @@ -37,7 +37,7 @@
* to represent the array of its elements. `CopySetValue` is the proper
* representation using a CopySet.
*
* TODO Eventually add `CopyBagValue` for semi-fungible rights represented as a
* A semi-fungible `CopyBagValue` is represented as a
* `CopyBag` of `Key` objects. "Bag" is synonymous with MultiSet, where an
* element of a bag can be present once or more times, i.e., some positive
* bigint number of times, representing that quantity of the asset represented
Expand All @@ -51,7 +51,7 @@
*/

/**
* @typedef {'nat' | 'set' | 'copySet' } AssetKind
* @typedef {'nat' | 'set' | 'copySet' | 'copyBag' } AssetKind
*
* See doc-comment for `AmountValue`.
*/
Expand Down Expand Up @@ -556,6 +556,14 @@
* @typedef {MathHelpers<CopySetValue>} CopySetMathHelpers
*/

/**
* @typedef {CopyBag<Key>} CopyBagValue
*/

/**
* @typedef {MathHelpers<CopyBagValue>} CopyBagMathHelpers
*/

/**
* @callback AssertAssetKind
* @param {AssetKind} allegedAK
Expand Down
Loading