Repository navigation
feat(ismp): model DeliveryConfirmed, the spec 14 delivery proof - #4
Merged
Merged
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.
What
Adds
DeliveryConfirmedEventand a matching arm onIsmpMessagingEvent, modelling runtime spec 14'sismpMessaging.DeliveryConfirmed { commitment, relayer, height }. Version0.4.0→0.5.0.Why it exists at all
Until spec 14 the chain had no witness to its own outbound message landing.
PostRequestHandledfires on the destination, so from Orbinum's own state a delivered POST and one still in flight were indistinguishable — permanently, not just until some later block.The protocol offers no way to fix that by asking: upstream #840 removed
PostResponseentirely, so a receiving module cannot reply. What the destination does leave behind is a receipt, written before the receiving module runs and deleted again if that module errs — so its presence proves delivered and executed, and being ordinary storage, a GET can prove it. That is what the runtime now does, and this event is the result.What the event does and does not claim
The receipt read is the coprocessor's, not the final chain's. Hyperbridge's proxy re-dispatches through the same
on_acceptthat writes receipts, soDeliveryConfirmedproves Hyperbridge accepted and forwarded the message. Execution on the far side is one hop further and is not witnessed here — that remains a destination-chain log, which consumers read separately. The doc comment on the type says this explicitly rather than leaving a reader to infer a stronger guarantee than the event carries.Two field notes worth carrying into any consumer:
commitmentnames the POST being confirmed, not the GET that proved it. Those are different messages with different commitments. Reporting the GET's would produce a value nothing else has seen and join to nothing.relayeris opaque bytes, not necessarily an account — the same caveatRequestHandledEvent.relayeralready documents. 32 bytes for a Substrate account, 20 for an EVM relayer, and a self-relaying script may put an ASCII tag there. Rendering arbitrary bytes as an address is a lie in monospace.heightis au64and therefore abigint, consistent with every other height in this file.Breaking-ness
Not breaking for data shapes — nothing existing changed. It is breaking for exhaustive matching:
IsmpMessagingEventnow has eight arms instead of seven, so a consumer with an exhaustiveswitchover it will fail to compile until it handles the new case. That is the intended loud failure, and it is the reason this is a minor bump rather than a patch.The union's doc comment claimed "All seven variants the pallet emits are here". Corrected to eight.
Deliberately not included
confirm_delivery(call index 4) is not modelled.IsmpModuleis read-only by an explicit, documented decision — "Dispatching a message is an extrinsic … so it is deliberately not exposed here" — and the repo has no call-index model for ISMP at all. Adding one to accommodate a single new extrinsic would open a surface nobody asked for, and the extrinsic is root-gated today anyway.Verification
pnpm checkclean: 866 tests across 36 files, plus lint, format and typecheck.The new test lives in
tests/types/ismp-types.test.ts, which already pins event shapes by runtime spec. It builds aDeliveryConfirmedEvent, places it in the union, and assertsheightis abigint— the field most likely to be silently typed asnumberby a future edit.Notes for the reviewer
release.ymlwatchespackage.jsononmain, so landing this publishes0.5.0to npm.commitment,relayerandget_heightalready exist), and the explorer renders it as aconfirmedstatus kept distinct fromexecuted, since the two carry different evidence and onlyexecutednames a transaction.