Skip to content

feat(midnight): destination-side Mailbox via submitter subprocess (#20) - #26

Merged
ctoyan merged 7 commits into
midnight-v1from
feat/20-midnight-mailbox
Jun 11, 2026
Merged

feat(midnight): destination-side Mailbox via submitter subprocess (#20)#26
ctoyan merged 7 commits into
midnight-v1from
feat/20-midnight-mailbox

Conversation

@ctoyan

@ctoyan ctoyan commented May 19, 2026

Copy link
Copy Markdown

What

Implements Hyperlane's Mailbox trait for Midnight in the agent fork. Closes the destination-side delivery half of the inbound flow.

  • MidnightMailbox parses MessageIdMultisigIsmMetadata, spawns the handle submitter binary, returns TxOutcome.
  • delivered(id) reuses the same submitter with op: "isDelivered".
  • Monolithic-WarpRoute defaults for count / default_ism / recipient_ism (0, self, self).
  • Fixed-placeholder process_estimate_costs (matches Aleo — Midnight uses DUST-based fees the wallet computes at submission time).
  • process_calldata / delivered_calldata are Lander-only → NotImplemented / None.
  • Replaces the eyre stub at hyperlane-base/src/settings/chains.rs:448 with the real builder.

Divergence from ticket

The ticket says wrap midnight-node-toolkit directly. This PR uses a custom Node submitter in the hyperlane-midnight repo's new relayer/ workspace instead — one subprocess hop instead of two, type-safe against the generated bindings, reuses devnet TS infrastructure. Full rationale on issue #20. ConnectionConf.toolkit_path is preserved as the field name.

Companion PR in hyperlane-midnight (the submitter itself): https://github.com/equilibriumco/hyperlane-midnight/pulls?q=is%3Apr+head%3Afeat%2F20-mailbox-submitter

AC mapping

AC Where
Mailbox trait impl for Midnight chains/hyperlane-midnight/src/mailbox.rs
Successful submission delivers a message process()toolkit::submit_handle → submitter → contract
Error paths mapped to ChainCommunicationError HyperlaneMidnightError variants in error.rs; submitter error kinds → SubmitterReported
Retries and idempotency tested against local devnet Documented in relayer/src/devnet-smoke.ts + relies on the contract's Mailbox.deliveries Set for the idempotency safety net

Open follow-ups

Notes

  • midnight-v1 is ~30 commits behind upstream main — flagged only, not merged here.
  • Built clean: cargo clippy --features aleo,integration_test -- -D warnings passes; cargo test -p hyperlane-midnight passes (10 tests).

Implements Hyperlane's `Mailbox` trait for Midnight in the agent fork.
The `process` method parses MessageIdMultisigIsmMetadata, spawns the
handle submitter binary (Node script in `equilibriumco/hyperlane-midnight`
relayer workspace), and maps known failure kinds to
`HyperlaneMidnightError` variants which flow through to
`ChainCommunicationError`.

`delivered(id)` reuses the same submitter binary with the `isDelivered`
op, querying the contract state via the indexer's GraphQL
`contractAction` query and decoding via `pureCircuits.isDelivered`.

Read methods (`count`, `default_ism`, `recipient_ism`) return sensible
defaults for the monolithic WarpRoute (0, self, self).
`process_estimate_costs` returns a fixed placeholder — Midnight uses
DUST-based fees that the wallet computes at submission time, matching
the Aleo crate's pattern. `process_calldata` / `delivered_calldata` are
Lander-only and return `NotImplemented` / `None`.

Diverges from the original ticket description by using a custom Node
submitter rather than `midnight-node-toolkit` directly. Reasons + full
design notes on the GitHub issue.
@ctoyan ctoyan self-assigned this May 19, 2026
…s timeout (#20)

C2 — `MidnightProvider::is_contract` returned `Ok(false)`, which caused the
relayer's `pending_message` state machine to drop every inbound message at
the `is_recipient_contract` gate before `Mailbox::process` was ever called.
Returns `Ok(true)` now with a doc comment explaining the monolithic-WarpRoute
rationale and the swap point when non-WarpRoute recipients arrive (#14).

H1 — `submit_handle` and `query_delivered` had no timeout and no
`kill_on_drop`, so a wedged Node child could block the relayer worker
indefinitely and leaked on cancellation. Both ops now share a `run_submitter`
helper that wraps `wait_with_output` in `tokio::time::timeout` (120s for
submit — generous for ZK proof generation; 30s for delivered — indexer
fetch + ledger decode), sets `kill_on_drop(true)` on the Child, and surfaces
elapsed-timeout as a new `HyperlaneMidnightError::SubmitterTimeout` variant.

A new test exercises the timeout path with a hanging shell stub and a
100ms budget, asserting the future returns inside 2s (proving kill_on_drop
reaped the child).
ctoyan added 5 commits June 5, 2026 13:26
Strips multi-paragraph module docs, field-level explanations that re-state
what types say, section dividers, and "why this exists" preambles. Keeps
single-line doc comments on pub items (the missing_docs lint still passes),
the `is_contract` rationale (since the false-default is a known footgun),
and the fee placeholder note in `process_estimate_costs`.

Test coverage unchanged (11 pass).
…27)

* test(midnight): cross-boundary integration tests for MidnightMailbox

Adds chains/hyperlane-midnight/src/cross_boundary_tests.rs — 17 tests
exercising the Mailbox trait impl + Provider against a stubbed
submitter binary in a tempdir. Covers the three classes of bug unit
tests on either side of the Rust/TS boundary cannot catch:

1. Provider/Mailbox contract drift. c2_is_contract_returns_true is the
   regression for C2 from the PR review — verified the test fails when
   the bug is present, passes when fixed.

2. Wire-format drift. process_request_json_pins_wire_shape and
   delivered_request_json_pins_wire_shape snapshot the JSON the Rust
   side sends so a refactor that drops/renames a field fails loud
   without needing the live devnet smoke. Partial L10 coverage.

3. Error-kind plumbing. Per-kind tests confirm replay and
   contractRevert propagate as SubmitterReported { kind } errors the
   relayer can branch on. process_rejects_malformed_response_* pins
   the parser's failure mode for non-JSON stdout.

Plus coverage for the read-only Mailbox methods (default_ism,
recipient_ism, count), the Lander-only stubs (process_calldata,
delivered_calldata), and the gas-estimate placeholder.

The harness writes a tiny shell-script stub per test that captures
stdin to a log file and echoes a canned response. Each test owns its
own tempdir so parallel runs do not collide. No new production code
touched — every test exercises the public API.

tempfile added as a dev-dependency.

* docs(midnight): reword cross-boundary test comments to stand on their own

Drops references to the external review document so the file reads
cleanly without context. Renames `c2_is_contract_returns_true` to
`is_contract_returns_true_for_monolithic_warp_route` — the new name
describes what the test pins instead of what bug it catches.

* chore(midnight): trim cross_boundary_tests comments and dividers

Drops the module preamble, section headers, and inline justifications.
Test names + assert messages carry the intent; the running diff
between cluttered and signal is what survives. ~250 lines removed.
The on-chain MessageIdMultisigIsm on Midnight requires signatures in
validator-set-index order (two-pointer match) and reverts with
"ISM: signer not in validators or signatures unsorted" otherwise. The
relayer's metadata builder concatenates signatures in fetch-completion
order; only Aleo gets a special-case sort in
hyperlane-base/src/types/multisig.rs.

Add `validators: Vec<H160>` to ConnectionConf, parsed from the
`validators` array under each Midnight chain entry. In
MidnightMailbox::process, when `validators` is non-empty:

  1. compute the checkpoint inner hash matching the on-chain ISM
     (keccak(domainHash(origin, merkleTreeHook) || root || index_be
      || messageId), pre-EIP-191);
  2. for each real signature in the parsed metadata, ecrecover the
     signer and look up its index in the configured set;
  3. sort by index and re-pad to 16 signatures.

Skips entirely when `validators` is empty, preserving the existing
cross_boundary_tests::process_* behaviour (those tests stub the
submitter and pass dummy signatures that wouldn't be expected to
recover to anyone).

Adds the `ethers` workspace dep to chains/hyperlane-midnight for
`keccak256` and `Signature::recover`. ConnectionConf::new keeps its
two-arg shape via a default-empty `validators` field; callers needing
the sort use the new `with_validators` builder.
…tubs (#20)

The relayer's `OriginFactory::create` requires every chain in
`HYP_RELAYCHAINS` to satisfy origin-side traits (ValidatorAnnounce +
ApplicationOperationVerifier + three SequenceAwareIndexers) regardless
of whether messages are ever dispatched from that chain — if any one
fails to build, the chain drops from `origins` and destination
delivery also breaks. The previous version of this branch had those
slots stubbed as `Err("not yet implemented")` in `chains.rs`,
which prevented building the relayer with Midnight as a relayChain.

This fills them in:

- `stubs.rs` (new): `MidnightValidatorAnnounceStub` and a generic
  `MidnightStubIndexer<T>` with type aliases for `HyperlaneMessage`,
  `MerkleTreeInsertion`, and `InterchainGasPayment`. All return
  empty results / no-op data. Replaced by real impls under #33 / #15 /
  #16 / #19 (all depending on #14).
- `application.rs` (new): `MidnightApplicationOperationVerifier` —
  mirrors the Aleo crate's pattern; lets the relayer instantiate the
  app-level verifier without changes to its core.
- `ism.rs` (new): `MidnightInterchainSecurityModule` (returns a
  fixed `ModuleType::MessageIdMultisig`) + `MidnightMultisigIsm`
  (returns validators + threshold from agent-config until #14 ships a
  state reader). H160-to-H256 padding for validator addresses on the
  way out.
- `config.rs`: adds `threshold: u8` + `with_threshold()` builder.
- `connection_parser.rs`: parses `threshold` from agent-config with
  `u8::try_from` instead of silent truncation — out-of-range values
  raise a clear `ConfigParsingError` instead of wrapping to a wrong
  threshold.
- `chains.rs`: replaces the six `Err("not yet implemented")`
  Midnight arms with real constructors calling into the above.

TODO(#14) markers sit at every chain-state-substitution site.
`@midnight-ntwrk/midnight-js-utils` throws `TypeError` on a leading
`0x` for contract addresses — Midnight tooling renders them as raw
hex. The submitter JSON's `contractAddress` field has to match that
or every callTx fails at the SDK boundary. Message senders, recipients,
and message IDs keep the `0x` prefix because they're not Midnight
SDK contract-address inputs.

Updates `cross_boundary_tests.rs` to assert the raw-hex form so
`cargo test -p hyperlane-midnight` stays green.
@ctoyan
ctoyan merged commit 5b88e40 into midnight-v1 Jun 11, 2026
50 of 55 checks passed
@ctoyan
ctoyan deleted the feat/20-midnight-mailbox branch August 25, 2026 15:45
@ctoyan
ctoyan restored the feat/20-midnight-mailbox branch August 25, 2026 15:45
@ctoyan
ctoyan deleted the feat/20-midnight-mailbox branch August 25, 2026 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant