Skip to content

Commit

Permalink
fix(fast-usdc): disallow all non-positive integer counts in Multiset (#…
Browse files Browse the repository at this point in the history
…11079)

_Incidental_
refs: #11066

## Description
A negative or zero value is not a non-positive value. Even in values that are verified `typeof number`, `NaN` doesn't satisfy the former check but should be excluded as a non-positive.

For removal, if a missing value returns `false` for any requested removal count, for consistency so should a removal request for a count larger than the current count.

After feedback, also fail if the count is not an integer.

### Security Considerations
Avoiding corrupted `currentCount` state if an unexpected value finds its way to the add / remove.

### Scaling Considerations
None

### Documentation Considerations
I don't believe this has any

### Testing Considerations
Updated unit tests

### Upgrade Considerations
This changes the behavior in 2 ways:
- Refusing currently unexpected values that would result in a corrupted state
- Allowing but returning false for removal of counts larger than current value.

For existing valid usages it should have no effective impact. This change would only be picked up by an upgrade of fast-usdc.
  • Loading branch information
mhofman authored Mar 8, 2025
1 parent 9348280 commit 152d3cc
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 13 deletions.
12 changes: 5 additions & 7 deletions packages/fast-usdc/src/utils/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,8 @@ export const asMultiset = mapStore =>
* @param {number} [count] How many to add (defaults to 1)
*/
add: (item, count = 1) => {
if (count <= 0) {
throw Fail`Cannot add a non-positive count ${count} to bag`;
}
(count > 0 && Number.isInteger(count)) ||
Fail`Cannot add a non-positive integer count ${count} to bag`;

if (mapStore.has(item)) {
const currentCount = mapStore.get(item);
Expand All @@ -42,17 +41,16 @@ export const asMultiset = mapStore =>
* @throws {Error} If trying to remove more items than exist
*/
remove: (item, count = 1) => {
if (count <= 0) {
throw Fail`Cannot remove a non-positive count ${count} from bag`;
}
(count > 0 && Number.isInteger(count)) ||
Fail`Cannot remove a non-positive integer count ${count} from bag`;

if (!mapStore.has(item)) {
return false;
}

const currentCount = mapStore.get(item);
if (currentCount < count) {
throw Fail`Cannot remove ${count} of ${item} from bag; only ${currentCount} exist`;
return false;
}

if (currentCount === count) {
Expand Down
37 changes: 31 additions & 6 deletions packages/fast-usdc/test/utils/store.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,16 @@ test('add with invalid count', t => {

// Should throw when adding with count <= 0
t.throws(() => multiset.add('apple', 0), {
message: /Cannot add a non-positive count/,
message: /Cannot add a non-positive integer count/,
});
t.throws(() => multiset.add('apple', -1), {
message: /Cannot add a non-positive count/,
message: /Cannot add a non-positive integer count/,
});
t.throws(() => multiset.add('apple', 1.1), {
message: /Cannot add a non-positive integer count/,
});
t.throws(() => multiset.add('apple', NaN), {
message: /Cannot add a non-positive integer count/,
});
});

Expand Down Expand Up @@ -101,16 +107,35 @@ test('remove', t => {
t.is(multiset.remove('apple'), false);
});

test('remove with excessive count should throw', t => {
test('remove with invalid count', t => {
const mapStore = makeScalarMapStore();
const multiset = asMultiset(mapStore);

multiset.add('apple');

// Should throw when adding with count <= 0
t.throws(() => multiset.remove('apple', 0), {
message: /Cannot remove a non-positive integer count/,
});
t.throws(() => multiset.remove('apple', -1), {
message: /Cannot remove a non-positive integer count/,
});
t.throws(() => multiset.remove('apple', 0.5), {
message: /Cannot remove a non-positive integer count/,
});
t.throws(() => multiset.remove('apple', NaN), {
message: /Cannot remove a non-positive integer count/,
});
});

test('remove with excessive count should return false', t => {
const mapStore = makeScalarMapStore();
const multiset = asMultiset(mapStore);

multiset.add('apple', 3);
t.is(multiset.count('apple'), 3);

t.throws(() => multiset.remove('apple', 5), {
message: /Cannot remove 5 of "apple" from bag; only 3 exist/,
});
t.false(multiset.remove('apple', 5));

t.is(multiset.count('apple'), 3, 'original count remains unchanged');

Expand Down

0 comments on commit 152d3cc

Please sign in to comment.