Agoric / agoric-sdk

monorepo for the Agoric Javascript smart contract platform
Apache License 2.0
327 stars 206 forks source link

Invitation amounts should include terms #5903

Open erights opened 2 years ago

erights commented 2 years ago

Background on invitation amount namespace collision:

Background on terms and amount patterns:

To solve the second problem, we should add a terms property to the invitation amount properties that are according to zoe/zcf, and therefore reliable. These could be the terms snapshot as of the time the contract was created, or more plausibly snapshot at the time this invitation was created. (Frankly it is a bit weird that an instance's terms can change over time. This makes sense because the term namespace is similarly split, which a similar silent override on collision. We should revisit this too.)

But adding a term properties faces us with the first problem. It is early enough that we could add it anyway. But it would be better to take the opportunity to address the first problem as well, so that we don't face it again the next time we want to add a property to the terms. (Though any change to amount representations will present hard upgrade problems that might deter us from doing it post-deploy anyway.)

I propose we terms and customProperties as top level properties, where all the custom properties are properties of the record value of the customProperties field, without worry about collision. We cannot simply remove the flattening of the non-colliding customProperties via ...customProperties. Nor can we @deprecate it because there's no specific flattened property names to deprecate. But we should explain and document it as deprecated, encouraging old code to switch to using the customProperties property explicitly.

Note that anything included in amounts must be a valid Key, which excludes promises, errors, and patterns. Also that they must pass canBeDurable, since ERTP stores amounts in durable storage. These two changes are synergistic, in that durability also requires us to omit promises, which is where we expect to pay all the transition costs. As part of making Zoe durable, we expect to require terms to be durable anyway, so this is an opportune time to do both.

dckc commented 2 years ago

This would be terms as of when a contract starts, I suppose? Terms are, in general, mutable; for example, by way of zcf.saveIssuer().

Chris-Hibbert commented 2 years ago

This would be terms as of when a contract starts, I suppose?

Yes. The contract's terms are mutable (I think additive only), but the terms in an invitation are not.

It's probably the case that when someone has cause to rely on the terms, they're most concerned with things that won't be impacted going forward, even if the terms evolve.

erights commented 1 year ago

7080 was a step towards fixing this, but by itself does not. Reopening.

erights commented 1 year ago

7067 was a step towards fixing this, but by itself does not. Reopening.