fix(abi): ship the Ricardian clauses, and write the eight that were missing - #32
Open
robrigo wants to merge 2 commits into
Open
fix(abi): ship the Ricardian clauses, and write the eight that were missing#32robrigo wants to merge 2 commits into
robrigo wants to merge 2 commits into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Not for v2.0.0. This changes the ABI, so it changes
abiSha256, and the v2.0.0 release plus the four live msig proposals are pinned by that hash. It targets a later release.The contract ships no Ricardian text at all
resource/atomicassets.contracts.mdis 27 KB of Ricardian clauses, correctly named and correctly placed, that CDT never embeds. The publishedatomicassets.abihasricardian_clauses: []and every one of its 47 actions carriesricardian_contract: "".The cause is a missing flag, and the sibling repository proves it:
The consequence is user-facing: a wallet asking someone to sign an AtomicAssets
transferorburnassethas no human-readable description to show them. AtomicMarket shows one. This is the document that tells a signer what they are agreeing to.Eight clauses were also missing
Turning the flag on alone would have shipped a partial Ricardian, which is worse than an obvious absence: some actions would carry a description and others silently would not.
Of the 47 actions, 9 are
log*actions guarded byrequire_auth(get_self()). They are dispatched inline by the contract, never signed by a user, so they need no clause and no wallet ever renders one. That leaves 38 user-signed actions, of which 8 had none, all introduced by v2:createtempl2,settempldata,deltemplate,redtemplmax,setschematyp,createauswap,acceptauswap,rejectauswapThey are written here in the file's existing house style, using the same
spec_version, icon, description and clauses structure, and the same handlebars conventions as neighbouring clauses ({{#if}}blocks for the transferable, burnable and max_supply variants increatetempl2, mirroringcreatetempl).Coverage after this change: 38 of 38 user-signed actions have a clause, 0 missing.
Wording that deserves review
Each clause states what a signer is agreeing to, and several carry real consequence that I have tried to make explicit rather than neutral:
acceptauswapsays plainly that accepting transfers control of the collection, including the right to authorize accounts and set the market fee.settempldatanotes that already-minted assets are affected, because mutable data is read from the template rather than copied onto each asset.setschematypnotes that descriptors are replaced in full, so an omitted descriptor is removed.deltemplateandredtemplmaxstate that the effect is permanent.These are legally-shaped text. I would want whoever owns product or legal wording to read them before they ship, rather than treating my drafting as final.
Verification
ricardian_clausesstays empty after this, which is expected: that array is populated from a<contract>.clauses.mdfile, which this repository does not have and which atomicmarket does not have either. The per-actionricardian_contractfield is what wallets render, and that is what the-Rflag populates.Worth adding a CI assertion that the built ABI carries non-empty
ricardian_contractfor every non-log action, so this cannot silently regress again. Not included here to keep the change reviewable.