Split the inclusion proof response from the proof itself

[MastaP]

InclusionProof carried both answers the aggregator can give — a certified leaf, and the absence of one — so every field was nullable and every consumer had to re-establish which case it held.

That cost, before this change:

• four absence branches at the top of InclusionProofVerificationRule, before any verifying began
• a status taxonomy describing states a proof should never have been able to be in: MISSING_CERTIFICATION_DATA, MISSING_REFERENCE_TIME, INCOMPLETE_INCLUSION_PROOF, INCLUSION_CERTIFICATE_MISSING
• a guard in each certified-transaction decoder rejecting a proof with no leaf
• as bigint casts on referenceTime in both certified transactions
• ?. on certificationData in the mint verification rule

The split

The absence belongs to the response, not to the proof.

class InclusionProof {                    // a certified leaf; there is nothing else it can be
  certificationData: CertificationData    // was | null
  referenceTime: bigint                   // was | null
  inclusionCertificate: InclusionCertificate  // was | null
  unicityCertificate: UnicityCertificate
}

class InclusionProofResponse {            // what the aggregator answers
  blockNumber: bigint
  inclusionProof: InclusionProof | null   // null = not certified yet
  unicityCertificate: UnicityCertificate
}

The wire form expresses both shapes, so decoding it stays where the layout lives: decodeInclusionProofOrAbsence returns the proof or the absence and rejects any partial combination; InclusionProof.fromCBOR refuses anything but a leaf.

What falls out

All four statuses are gone — none of them can occur. waitInclusionProof polls on the response having no proof rather than on a status that meant the same thing. Both certified-transaction decoders drop their guards, and their reference-time getters drop the casts.

Net 126 lines removed (−325/+199). The only nullability left in the whole area is inside the wire decoder, which is where the wire's two shapes actually are.

Two tests changed shape rather than being deleted, because what they asserted became unrepresentable: constructing a partial proof no longer compiles, so the partial-combination case is now exercised on encoded bytes at the decode boundary, which is the only place it can arise.

Also

The verification rule's comments are cut back to what the code doesn't already say — the back-dating argument lives in the README rather than being restated at the call site.

Verification

build, build:check and lint clean. 185 tests pass.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 44a699fdff

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Keep the response certificate consistent when serializing

When a caller constructs a response with a non-null proof and a different unicityCertificate, the public property reports the supplied response certificate, but toCBOR() silently serializes the certificate embedded in inclusionProof. Consequently, InclusionProofResponse.fromCBOR(response.toCBOR()).unicityCertificate changes value, potentially associating the response's block with the wrong round. Derive the certificate from the proof for certified responses or reject inconsistent constructor arguments.

Useful? React with 👍 / 👎.

[MastaP]

Fixed in cde4e97 — the local review flagged the same thing, so it's addressed on the rebased branch.

Rather than validate the two certificates against each other, the second one is gone. The constructor is private and there are two named factories:

InclusionProofResponse.certified(blockNumber, proof)     // reads the certificate off the proof
InclusionProofResponse.notCertified(blockNumber, cert)   // no proof to read it from

Neither can express a disagreement, so the round trip is exact for both shapes. Call sites also now say which of the two answers they are building, instead of leaving it to whether an argument was null.

[martti007]

Move into response and make inclusion proof take response as input for creation.

[MastaP]

Agreed the two exported free functions are the wrong shape — they live in InclusionProof.ts but exist only for the response, which is the leak you're pointing at.

I want to check which of two readings you mean before I move it, because they end up in different places.

(a) The response owns the whole tagged codec. decodeInclusionProofOrAbsence and encodeNoCertifiedLeaf move into InclusionProofResponse, and InclusionProof.fromCBOR goes with them — so InclusionProof has no knowledge of the wire's two shapes at all, only its own fields. The catch: CertifiedMintTransaction/CertifiedTransferTransaction decode an embedded proof, so they would import that decode from the response module.

(b) InclusionProof is constructed from the decoded response. The response does the tag/version/field decoding itself and hands the parts to InclusionProof; InclusionProof keeps toCBOR for embedding but has no decoder. Costs ~8 lines of tag-and-version handling duplicated between the two, and buys InclusionProof.ts knowing nothing about absence.

I read your comment as closer to (b) — "take response as input for creation" — but (a) is the one that removes more. Which did you have in mind? Happy to do either; I just don't want to guess at a large move and have you re-review the wrong one.

[MastaP]

Done, B as you called it — 65f1e1f.

InclusionProofResponse.fromCBOR now decodes the tagged structure itself, decides certified from not, and constructs the InclusionProof from the parts. It encodes the absent form itself too. InclusionProof.ts exports the class and nothing else, and mentions absence only to point at where it lives.

The tag and version check ends up in both decoders. I left it that way on purpose rather than sharing a helper: the only way to share it without the certified transactions importing their decoder from the response module — the dependency the wrong way round — would be another free function, which is the thing you were pointing at in the first place. Eight lines seemed the better trade.

185 tests pass; build, typecheck and lint clean.