feat(documents): contract parties and fields — schema only - #385
Closed
QSchlegel wants to merge 1 commit into
Closed
feat(documents): contract parties and fields — schema only#385QSchlegel wants to merge 1 commit into
QSchlegel wants to merge 1 commit into
Conversation
The data model for N-of-N contracts over named parties, on top of the existing M-of-N wallet sign-off. Schema and migration only: signingMode defaults to `threshold`, so this is a no-op for every document that exists. Shipped alone because the migration is the expensive half — this repo ships migrations through an action that does not self-retry, so the shape is the part that must be right first. The enforcement it needs is listed below and is several PRs. WHAT AN ADVERSARIAL REVIEW CHANGED The first draft of this schema was wrong in ways that would each have cost a second migration: - DROPPED `ContractParty.required`. Neither encoding works. Exclude optional parties from the frozen snapshot and they are denied outright; include them and evaluateThreshold's anonymous count lets a contract be Approved OVER a required party's explicit rejection. - DROPPED the status enum and viewedAt/decidedAt. Approvals reset per version while parties hang off the document, so per-round state here survives a reset it has no business surviving: an ordering gate reading a stale "signed" would wave through a countersignature on a version the first party has never seen. Whether a party signed is not an opinion this table stores — it is the existence of a DocumentReview. - ADDED @@unique([documentId, address]). One human in two roles is not a collision, it is an unrecoverable round: approvals cap at N-1 so Approved is unreachable, and rejections never make the threshold unreachable either. - KEPT @@unique([versionId, signerAddress]) and ADDED @@unique([versionId, partyId]) rather than replacing it — the proof verifier dedupes on signerAddress, so replacing would make a finished contract export as invalid. - ADDED onDelete: NoAction on DocumentReview.partyId. Prisma's default for an optional relation is SetNull, which would silently strip the capacity off an already-exported signature on an append-only table. - ADDED documentId + @@unique([documentId, anchor]) to ContractField. Anchors must be unique across a body that spans parties. - RESCOPED ContractField.value to pre-publish input. As first drafted it was an unsigned, mutable contract term outside contentHash: change 50000 to 5000 after signing and every signature still verifies. - ADDED invite expiry and consumption, mirroring EmailVerificationToken and BotClaimToken. "Single-use" was a comment with no column behind it. VERIFIED AGAINST A THROWAWAY POSTGRES, not assumed: - two threshold reviews with partyId NULL coexist on one version — the new unique does not break a single existing document, which was the biggest risk in the change; - two un-redeemed parties with address NULL coexist; - one address twice on a document is refused; - a party who has signed cannot be deleted alone (NoAction), while deleting the whole document still cascades cleanly to zero rows. RLS is in this migration rather than a follow-up, since ContractParty holds the only identifiable third-party data in the document stack. STILL REQUIRED BEFORE PARTIES MODE WORKS — none of it is in this PR: - Access. A named party is in neither signersAddresses nor ownerAddress, so assertWalletAccess rejects them before any party logic runs. This is the crux; it needs an invite-redemption procedure and a party-scoped access path. - startReview needs a parties branch building the snapshot from parties. - submitSignerAction needs the ordering gate, a row lock (two concurrent final approvals currently leave an N-of-N contract permanently InReview), and a signedAt replay guard. - publishDraft must render ContractField values into the body before hashing. - `method` belongs in the signed bytes; the column alone is an index, not evidence. 1161 tests pass; tsc clean; next build exit 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Collaborator
Author
|
Superseded by the answers to the two open questions in this PR. Both came back maximal: one human can hold two roles, and optional / non-signing parties are a launch requirement. That changes the schema materially rather than incrementally:
Reopening as a fresh branch off preprod rather than appending here, so this does not merge in its current shape. |
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.
The data model for N-of-N contracts over named parties, on top of the existing M-of-N wallet sign-off. Schema and migration only —
signingModedefaults tothreshold, so this is a no-op for every document that exists.Shipped alone because the migration is the expensive half: this repo ships migrations through an action that does not self-retry, so the shape is what has to be right first. The enforcement it needs is listed at the bottom and is several PRs.
What an adversarial review changed
I had a draft schema. Four lenses attacked it against the real code, and it was wrong in ways that would each have cost a second migration:
ContractParty.required. Neither encoding works. Exclude optional parties from the frozen snapshot and they're denied outright; include them andevaluateThreshold's anonymous count lets a contract be Approved over a required party's explicit rejection.viewedAt/decidedAt. Approvals reset per version while parties hang off the document, so per-round state here survives a reset it has no business surviving — an ordering gate reading a stalesignedwould wave through a countersignature on a version the first party has never seen. Whether a party signed isn't an opinion this table stores; it's the existence of aDocumentReview.@@unique([documentId, address]). One human in two roles isn't a collision, it's an unrecoverable round: approvals cap at N−1 so Approved is unreachable, and rejections never make the threshold unreachable either.@@unique([versionId, signerAddress])and added@@unique([versionId, partyId])rather than replacing it — the proof verifier dedupes onsignerAddress, so replacing would make a finished contract export as invalid.onDelete: NoActiononDocumentReview.partyId. Prisma's default for an optional relation isSetNull, which would silently strip the capacity off an already-exported signature on an append-only table.documentId+@@unique([documentId, anchor])toContractField— anchors must be unique across a body that spans parties.ContractField.valueto pre-publish input. As first drafted it was an unsigned, mutable contract term outsidecontentHash: change50000to5000after signing and every signature still verifies.EmailVerificationTokenandBotClaimToken. "Single-use" was a comment with no column behind it.Verified against a throwaway Postgres, not assumed
partyIdNULL on one versionaddressNULLNoActionThe first row was the biggest risk in the change and the reason I tested rather than reasoned: if NULLs weren't distinct, this migration would have broken every document with two or more approvals.
RLS ships in this migration rather than a follow-up, since
ContractPartyholds the only identifiable third-party data in the document stack.Still required before parties mode works — none of it is here
signersAddressesnorownerAddress, soassertWalletAccessrejects them before any party logic runs. This is the crux; it needs an invite-redemption procedure and a party-scoped access path.startReviewneeds a parties branch building the snapshot from parties.submitSignerActionneeds the ordering gate, a row lock (two concurrent final approvals currently leave an N-of-N contract permanentlyInReview), and asignedAtreplay guard.publishDraftmust renderContractFieldvalues into the body before hashing.methodbelongs in the signed bytes; the column alone is an index, not evidence.tsc --noEmitclean; 1161 tests pass;next buildexit 0.🤖 Generated with Claude Code