npm run test throws error about non-frozen objects

[tgrecojs]

Attempting to run the test code associated with this contract results in failing tests stemming from non-frozen objects. I've attempted to work through the error using harden but similar errors continue to appear, leading me to think that there might've been a change in the platform causing these tests to no longer run.

Quick screengrab below.

dapp-nft-drop.test.err.mp4

Any chance there's a quick fix for this? Migrating the code to use multiple duplicate harden functions seems like a lot of duplication so wondering if there's an easier fix for this 🤔

[dckc]

I think we did recently have a breaking API change that requires harden() in more places.
I thought perhaps it was Agoric/agoric-sdk#3892 but harden isn't mentioned there. @erights maybe you can help me find it?

meanwhile, @tgrecojs note this repo is a quick demo; don't expect too much by way of support.
(I wonder if it belongs more in agoric-labs or some such.)

Also... any particular reason you're running npm run test rather than yarn test?

[dckc]

Agoric/agoric-sdk#3892 is indeed the breaking change; we can tell from https://github.com/Agoric/agoric-sdk/blame/e0f7ee8884648973c63bbcae587ea36e51f58a64/packages/ERTP/src/amountMath.js#L74

I'm not sure why the changelog doesn't mention harden().

[erights]

Yes, that is the PR. It fixes ERTP to do proper input validation of its inputs. A consequence is that improper inputs which had previously been mistakenly accepted are now properly rejected. That PR does add the file https://github.com/Agoric/agoric-sdk/blob/master/packages/ERTP/docs/INPUT_VALIDATION.md#who-hardens to document the consequences of the stricter input validation, including the section "Who hardens?" that explains this issue in particular. I'm sorry we didn't draw more attention to this breaking change.

At the line in question:

dapp-nft-drop/contract/test/test-NFTDropContractBasic.js

Line 61 in a865ed7

NFTs: AmountMath.make(nftBrand, [1n]),

the [1n] is invalid input that the previous code mistakenly allowed. Unfrozen, it is not a CopyArray, and so not a proper value for an Amount. Changing it to harden([1n]) should fix the problem. In general, hardening values before releasing them avoids communicating unintentional mutability, making code safer.

We recognize that the current obligation to use harden a lot is notationally unpleasant. On a speculative branch I'm experimenting with an approach that may let us safely omit harden much more often. But I don't know if this will work out. Until then, we should all continue to use harden to make our code more obviously safe against the consequences of unintended mutability.

[tgrecojs]

meanwhile, @tgrecojs note this repo is a quick demo; don't expect too much by way of support. (I wonder if it belongs more in agoric-labs or some such.)

The purpose is to provide an API documentation writing sample for the assessment of Dean + others on the team.

Also... any particular reason you're running npm run test rather than yarn test?

No particular reason - I guess it's a little bit of muscle-memory with using npm run. They both accomplish the same thing though. It's when you mix npm install and yarn install that you get into a sticky situation 🙂