diff --git a/frame/ismp-messaging/src/lib.rs b/frame/ismp-messaging/src/lib.rs index 8cecd238..2a624822 100644 --- a/frame/ismp-messaging/src/lib.rs +++ b/frame/ismp-messaging/src/lib.rs @@ -77,6 +77,16 @@ pub mod pallet { #[pallet::constant] type MaxBodyLen: Get; + /// Largest number of storage keys a single GET may request. + /// + /// Bounds work we impose on someone else: every key in a GET is a separate + /// membership proof the relayer must fetch from the destination and the + /// destination must include. An unbounded GET is a cheap way to make a remote + /// chain and a relayer do expensive work, so it is capped for the same reason + /// [`Config::MaxBodyLen`] caps a body. + #[pallet::constant] + type MaxGetKeys: Get; + type WeightInfo: WeightInfo; } @@ -259,6 +269,16 @@ pub mod pallet { DestinationIsSelf, /// `pallet-ismp` refused the request. DispatchFailed, + /// A GET with no keys asks for nothing and would still cost a round trip. + NoKeysRequested, + /// Exceeded [`Config::MaxGetKeys`]. + TooManyKeys, + /// A GET must name the height to read at, and `0` is never a real one. + /// + /// The response handler requires the proof height to equal the requested height + /// exactly (`ismp-2606.1.0/src/handlers/response.rs:71`), so a height nobody can + /// prove leaves the request hanging until it expires rather than failing fast. + InvalidGetHeight, } #[pallet::call] @@ -281,6 +301,43 @@ pub mod pallet { outbound::post::(dest, to, body, timeout) } + /// Read state from `dest` over ISMP. + /// + /// A GET is answered differently from a POST, and the difference is the point: no + /// module runs on the destination. A relayer reads the requested keys, proves them + /// against a state commitment we already hold, and the answer arrives back here as + /// [`Event::GetResponseReceived`] via our own `on_response`. So a destination + /// cannot refuse us the way a receiving module can refuse a POST. + /// + /// "A relayer" is not the public one: Tesseract delivers GET responses only to EVM + /// sources (`tesseract/messaging/messaging/src/events.rs:314-336`), so on this chain + /// the answer is carried by `scripts/hyperbridge/relay-get-response.mjs`. The proof + /// is verified here regardless of who submits it. + /// + /// `height` must be one this chain can already prove — a height for which + /// `pallet-ismp` holds a state commitment of `dest`. The response handler compares + /// it for equality, not as a lower bound, so an unprovable height means the request + /// simply expires. + /// + /// `keys` are proven against what this chain holds of `dest`. For the coprocessor + /// that is its ISMP **child trie** root, not its state root + /// (`ismp-grandpa/src/consensus.rs:142-150`), so a GET to Hyperbridge can only read + /// keys inside `:child_storage:default:ISMPv2`. + /// + /// `timeout` is **relative seconds**; `0` means *never expires*. + #[pallet::call_index(3)] + #[pallet::weight(T::WeightInfo::dispatch_get(keys.len() as u32))] + pub fn dispatch_get( + origin: OriginFor, + dest: StateMachine, + keys: Vec>, + height: u64, + timeout: u64, + ) -> DispatchResult { + T::DispatchOrigin::ensure_origin(origin)?; + outbound::get::(dest, keys, height, timeout) + } + #[pallet::call_index(1)] #[pallet::weight(T::WeightInfo::accept_source())] pub fn accept_source(origin: OriginFor, source: StateMachine) -> DispatchResult { diff --git a/frame/ismp-messaging/src/mock.rs b/frame/ismp-messaging/src/mock.rs index e43b4f87..5a19ec60 100644 --- a/frame/ismp-messaging/src/mock.rs +++ b/frame/ismp-messaging/src/mock.rs @@ -115,6 +115,8 @@ impl pallet_ismp::Config for Test { impl pallet_ismp_messaging::Config for Test { type DispatchOrigin = frame_system::EnsureRoot; type MaxBodyLen = ConstU32<8192>; + // Small on purpose, so the TooManyKeys path is cheap to exercise. + type MaxGetKeys = ConstU32<4>; type WeightInfo = (); } diff --git a/frame/ismp-messaging/src/outbound.rs b/frame/ismp-messaging/src/outbound.rs index 26d93912..78fe16c5 100644 --- a/frame/ismp-messaging/src/outbound.rs +++ b/frame/ismp-messaging/src/outbound.rs @@ -9,7 +9,7 @@ use crate::{Config, Error, Event, PALLET_ID_BYTES, Pallet, RequestKind}; use alloc::vec::Vec; use frame_support::{ensure, traits::Get, traits::UnixTime}; use ismp::{ - dispatcher::{DispatchPost, DispatchRequest, FeeMetadata, IsmpDispatcher}, + dispatcher::{DispatchGet, DispatchPost, DispatchRequest, FeeMetadata, IsmpDispatcher}, host::StateMachine, }; use pallet_ismp::pallet::ModuleId; @@ -110,6 +110,100 @@ pub fn post( Ok(()) } +/// Read state from `dest` at `height`. +/// +/// The mirror of [`post`], and the asymmetry is what makes it useful: a POST is handed to a +/// module on the destination, which may refuse it — `pallet-ismp-demo` on Hyperbridge, for +/// one, rejects any `Substrate(_)` source outright. A GET is answered by a relayer reading +/// the destination's storage and proving it, so nothing on the far side can turn us away. +/// The answer lands back here in our own `on_response` — carried by our own relaying +/// script for now, since Tesseract only delivers GET responses to EVM sources. +/// +/// `timeout` is **relative seconds**; `0` means the request never expires. +pub fn get( + dest: StateMachine, + keys: Vec>, + height: u64, + timeout: u64, +) -> DispatchResult { + // Reading our own state over ISMP is never meaningful: the round trip proves something + // we can already read directly. + ensure!( + dest != ::HostStateMachine::get(), + Error::::DestinationIsSelf + ); + + // A keyless GET still costs a dispatch, a relayer round trip and a response, and + // answers nothing. + ensure!(!keys.is_empty(), Error::::NoKeysRequested); + + // Each key is a separate membership proof the destination must produce, so this bounds + // work we impose on someone else — the same reason a body is bounded. + ensure!( + keys.len() as u32 <= T::MaxGetKeys::get(), + Error::::TooManyKeys + ); + + // Rejected here rather than left to expire. `handlers/response.rs:71` requires the + // proof height to EQUAL the requested height, so a height no relayer can prove is not + // a slow request — it is one that can never be answered, and failing now says so. + ensure!(height > 0, Error::::InvalidGetHeight); + + // Same reasoning as `post`: not the destination, just proof a route exists. + ensure!( + ::Coprocessor::get().is_some(), + Error::::CoprocessorNotSet + ); + + // Captured before the dispatch consumes the nonce, and before `keys` is moved. + let nonce = pallet_ismp::Nonce::::get(); + let timeout_timestamp = if timeout == 0 { + 0 + } else { + <::TimestampProvider as UnixTime>::now() + .as_secs() + .saturating_add(timeout) + }; + + let get = DispatchGet { + dest, + from: PALLET_ID_BYTES.to_vec(), + keys, + height, + timeout, + // Application metadata travels with the request and comes back on the response. + // Nothing here needs it, and it is remote-visible, so it stays empty. + context: Default::default(), + }; + + let commitment = pallet_ismp::Pallet::::default() + .dispatch_request( + DispatchRequest::Get(get), + // Zero fee, for the same reason as `post` — see the note there. + FeeMetadata { + payer: payer::(), + fee: Default::default(), + }, + ) + .map_err(|_| Error::::DispatchFailed)?; + + Pallet::::deposit_event(Event::RequestDispatched { + dest, + // A GET addresses storage, not a module, so there is no `to`. Our own module id + // goes here because that is what the protocol records as `from` and what the + // response is routed back to. + to: PALLET_ID_BYTES.to_vec(), + commitment, + nonce, + timeout_timestamp, + // A GET has no body. Reported as 0 rather than omitted so the column means the + // same thing on every row. + body_len: 0, + kind: RequestKind::Get, + }); + Ok(()) +} + /// Account recorded as the fee payer. /// /// Derived from the pallet id because Root has no account; the fee is zero, so nothing diff --git a/frame/ismp-messaging/src/tests.rs b/frame/ismp-messaging/src/tests.rs index f8f9dafc..8057f4da 100644 --- a/frame/ismp-messaging/src/tests.rs +++ b/frame/ismp-messaging/src/tests.rs @@ -851,3 +851,350 @@ fn a_get_response_reports_dest_height_and_nonce() { assert_eq!(found.3, 1_788_970_000); }); } + +// ─── dispatch_get ───────────────────────────────────────────────────────────── +// +// A GET exists as a separate path because a POST can be refused by a module on the +// destination and a GET cannot: nobody runs code there. The relayer reads the requested +// keys, proves them, and the answer comes back to OUR `on_response`. These tests pin the +// guards that make an unanswerable GET fail immediately instead of hanging until expiry. + +/// A GET to the counterparty with `n` distinct keys. +fn get_keys(n: usize) -> alloc::vec::Vec> { + (0..n).map(|i| alloc::vec![i as u8; 32]).collect() +} + +#[test] +fn dispatch_get_reports_kind_get_and_no_body() { + new_test_ext().execute_with(|| { + pallet_timestamp::Pallet::::set_timestamp(NOW_SECS * 1_000); + let nonce_before = pallet_ismp::Nonce::::get(); + + assert_ok!(crate::Pallet::::dispatch_get( + RuntimeOrigin::root(), + COUNTERPARTY, + get_keys(2), + 10_403_542, + 3_600, + )); + + let (_, nonce, timeout_timestamp, body_len, kind) = + dispatched_event().expect("dispatch_get must emit RequestDispatched"); + + assert_eq!( + kind, + crate::RequestKind::Get, + "a GET must not report itself as a POST" + ); + // 0 rather than omitted: the field means the same thing on every row, and a GET + // genuinely has no body. + assert_eq!(body_len, 0); + assert_eq!(nonce, nonce_before); + assert_eq!(timeout_timestamp, NOW_SECS + 3_600); + }); +} + +#[test] +fn dispatch_get_with_zero_timeout_never_expires() { + new_test_ext().execute_with(|| { + pallet_timestamp::Pallet::::set_timestamp(NOW_SECS * 1_000); + + assert_ok!(crate::Pallet::::dispatch_get( + RuntimeOrigin::root(), + COUNTERPARTY, + get_keys(1), + 10_403_542, + 0, + )); + + let (_, _, timeout_timestamp, _, _) = dispatched_event().expect("must emit"); + // Same branch as a POST: 0 means never, not `now + 0`. + assert_eq!(timeout_timestamp, 0); + }); +} + +#[test] +fn dispatch_get_rejects_a_request_for_nothing() { + new_test_ext().execute_with(|| { + // A keyless GET still costs a dispatch, a relayer round trip and a response, and + // answers nothing. + assert_noop!( + crate::Pallet::::dispatch_get( + RuntimeOrigin::root(), + COUNTERPARTY, + alloc::vec![], + 10_403_542, + 0, + ), + crate::Error::::NoKeysRequested + ); + }); +} + +#[test] +fn dispatch_get_bounds_the_work_it_asks_of_the_destination() { + new_test_ext().execute_with(|| { + // Each key is a separate membership proof the remote chain must produce. The mock + // caps this at 4, so 5 is over. + assert_noop!( + crate::Pallet::::dispatch_get( + RuntimeOrigin::root(), + COUNTERPARTY, + get_keys(5), + 10_403_542, + 0, + ), + crate::Error::::TooManyKeys + ); + // The boundary itself is allowed. + assert_ok!(crate::Pallet::::dispatch_get( + RuntimeOrigin::root(), + COUNTERPARTY, + get_keys(4), + 10_403_542, + 0, + )); + }); +} + +#[test] +fn dispatch_get_rejects_an_unprovable_height() { + new_test_ext().execute_with(|| { + // `handlers/response.rs:71` compares the proof height for EQUALITY, so height 0 is + // not a slow request — it is one no relayer can ever answer. Failing now says so, + // rather than leaving it to expire silently. + assert_noop!( + crate::Pallet::::dispatch_get( + RuntimeOrigin::root(), + COUNTERPARTY, + get_keys(1), + 0, + 0, + ), + crate::Error::::InvalidGetHeight + ); + }); +} + +#[test] +fn dispatch_get_rejects_reading_our_own_state() { + new_test_ext().execute_with(|| { + // A round trip to prove something we can read directly. + assert_noop!( + crate::Pallet::::dispatch_get( + RuntimeOrigin::root(), + StateMachine::Substrate(*b"orbi"), + get_keys(1), + 10_403_542, + 0, + ), + crate::Error::::DestinationIsSelf + ); + }); +} + +#[test] +fn dispatch_get_rejects_non_root() { + new_test_ext().execute_with(|| { + assert_noop!( + crate::Pallet::::dispatch_get( + RuntimeOrigin::signed(1), + COUNTERPARTY, + get_keys(1), + 10_403_542, + 0, + ), + sp_runtime::DispatchError::BadOrigin + ); + }); +} + +#[test] +fn a_get_response_closes_out_the_get_it_answers() { + new_test_ext().execute_with(|| { + pallet_timestamp::Pallet::::set_timestamp(NOW_SECS * 1_000); + let keys = get_keys(2); + + assert_ok!(crate::Pallet::::dispatch_get( + RuntimeOrigin::root(), + COUNTERPARTY, + keys.clone(), + 10_403_542, + 0, + )); + let (dispatched, nonce, timeout_timestamp, _, _) = dispatched_event().expect("must emit"); + + // Rebuild the GET from the event's own fields and answer it. This is the GET + // counterpart of `dispatched_fields_rebuild_the_committed_request`: if the emitted + // nonce, height or timeout did not describe the request that was actually + // committed, the two commitments would differ and this fails. + let response = ismp::router::GetResponse { + get: GetRequest { + source: StateMachine::Substrate(*b"orbi"), + dest: COUNTERPARTY, + nonce, + from: PALLET_ID_BYTES.to_vec(), + keys: keys.clone(), + height: 10_403_542, + context: alloc::vec![], + timeout_timestamp, + }, + values: keys + .iter() + .map(|k| ismp::router::StorageValue { + key: k.clone(), + value: Some(alloc::vec![7u8]), + }) + .collect(), + }; + + frame_system::Pallet::::reset_events(); + assert_ok!(IsmpModuleCallback::::default().on_response(response)); + + assert_eq!( + emitted_commitment(), + Some(dispatched), + "the response must name the very GET that was dispatched" + ); + + // And the spec-13 fields describe the read that was actually performed. + let found = frame_system::Pallet::::events() + .into_iter() + .find_map(|r| match r.event { + crate::mock::RuntimeEvent::IsmpMessaging(crate::Event::GetResponseReceived { + dest, + height, + keys, + found, + .. + }) => Some((dest, height, keys, found)), + _ => None, + }) + .expect("must emit GetResponseReceived"); + assert_eq!(found.0, COUNTERPARTY); + assert_eq!( + found.1, 10_403_542, + "the remote height the read was proven against" + ); + assert_eq!(found.2, 2); + assert_eq!(found.3, 2); + }); +} + +// ── the commitment's wire encoding ─────────────────────────────────────────────── + +/// A commitment is `keccak256(abi.encode(request))` — Solidity ABI, **not SCALE**. +/// +/// `Request::encode()` is an inherent method that shadows the SCALE `Encode` trait +/// (`ismp-2606.1.0/src/router.rs:263-266`), so Rust code reads as if it were SCALE while +/// producing 32-byte-word ABI output. Nothing in this pallet chooses that; it inherits it. +/// But everything off-chain that rebuilds a commitment — the probe, an indexer, a relayer +/// — has to know, and a SCALE-based rebuild fails silently: it hashes fine and the chain +/// answers `UnknownRequest` much later. Pinning the exact bytes here gives those tools a +/// ground truth to test against, and fails loudly if an upstream bump changes the wire. +/// +/// The two vectors below are shared with `scripts/hyperbridge/lib/harness.mjs`. Change one +/// side only if you change both. +#[test] +fn commitments_hash_the_abi_encoding_not_scale() { + let post = Request::Post(PostRequest { + source: StateMachine::Substrate(*b"orbi"), + dest: COPROCESSOR, + nonce: 4, + from: PALLET_ID_BYTES.to_vec(), + to: b"demo/mod".to_vec(), + timeout_timestamp: 0, + body: alloc::vec![1, 2, 3, 4], + }); + let get = Request::Get(GetRequest { + source: StateMachine::Substrate(*b"orbi"), + dest: COPROCESSOR, + nonce: 7, + from: PALLET_ID_BYTES.to_vec(), + keys: alloc::vec![alloc::vec![0xaa; 32]], + height: 4242, + context: alloc::vec![], + timeout_timestamp: 0, + }); + + new_test_ext().execute_with(|| { + let hash = + |r: &Request| alloc::format!("{:?}", hash_request::>(r)); + assert_eq!( + hash(&post), + "0xe51e536288e74f85fb16bc89fe4d43feb49c15049776de908bb402310bd389bc" + ); + assert_eq!( + hash(&get), + "0xc7f995f640ae3d0ccab18b0ff1280f68d5c906c60e5d62862aa457150d8792ee" + ); + + // The state machines travel as their DISPLAY strings ("SUBSTRATE-orbi", + // "KUSAMA-4009"), not as SCALE variants — `abi.rs:64-76`. That is why the + // human-readable form `ismp_queryRequests` returns can be hashed as-is. + let abi = post.encode(); + assert!(abi.windows(14).any(|w| w == b"SUBSTRATE-orbi")); + assert!(abi.windows(11).any(|w| w == b"KUSAMA-4009")); + assert_eq!( + abi.len() % 32, + 0, + "abi.encode output is whole 32-byte words" + ); + + // And the SCALE encoding — reachable only through the trait, fully qualified — + // is a different byte string with a different hash. This is the trap. + let scale = ::encode(&post); + assert_ne!(scale, abi); + assert_ne!( + alloc::format!("{:?}", sp_core::H256(sp_io::hashing::keccak_256(&scale))), + hash(&post), + "a SCALE-based rebuild must not accidentally reproduce the commitment" + ); + }); +} + +/// A GET response is accepted with `AcceptedSources` EMPTY, while a POST from the very same +/// chain is refused. +/// +/// This is why the self-relayed GET works on testnet without touching `AcceptedSources`: +/// `on_response` answers a request WE dispatched — the response handler already proved it +/// against our own commitment (`handlers/response.rs:65-71`) — so there is no remote sender +/// to vet. `AcceptedSources` gates who may *initiate* a message to us, which is a POST. +#[test] +fn a_get_response_is_accepted_without_any_accepted_source() { + new_test_ext().execute_with(|| { + assert!( + AcceptedSources::::iter().next().is_none(), + "the default must be to accept no POST source" + ); + + let module = IsmpModuleCallback::::default(); + assert_ok!(module.on_response(GetResponse { + get: GetRequest { + source: StateMachine::Substrate(*b"orbi"), + dest: COPROCESSOR, + nonce: 0, + from: PALLET_ID_BYTES.to_vec(), + keys: alloc::vec![alloc::vec![1u8; 32]], + height: 10, + context: alloc::vec![], + timeout_timestamp: 0, + }, + values: alloc::vec![StorageValue { + key: alloc::vec![1u8; 32], + value: Some(alloc::vec![9]) + }], + })); + assert!( + emitted_commitment().is_some(), + "GetResponseReceived must be emitted" + ); + + // Same chain, opposite direction: refused, because nobody accepted it as a source. + assert!( + module + .on_accept(post_from(COPROCESSOR, Message::Ping { nonce: 1 }.encode())) + .is_err() + ); + }); +} diff --git a/frame/ismp-messaging/src/weights.rs b/frame/ismp-messaging/src/weights.rs index adc3d40a..835ac7e9 100644 --- a/frame/ismp-messaging/src/weights.rs +++ b/frame/ismp-messaging/src/weights.rs @@ -71,6 +71,7 @@ use core::marker::PhantomData; /// Weight functions needed for pallet_ismp_messaging. pub trait WeightInfo { fn dispatch_post(b: u32, ) -> Weight; + fn dispatch_get(k: u32, ) -> Weight; fn accept_source() -> Weight; fn remove_source() -> Weight; fn on_accept(b: u32, ) -> Weight; @@ -204,6 +205,24 @@ impl WeightInfo for SubstrateWeight { .saturating_add(T::DbWeight::get().writes(4_u64)) .saturating_add(Weight::from_parts(0, 21).saturating_mul(b.into())) } + /// NOT BENCHMARKED — derived from `dispatch_post`, deliberately. + /// + /// A GET walks the same dispatch path as a POST (one `Ismp::Nonce` read/write, the + /// commitment write, the event) and its only per-item cost is encoding the keys, the + /// same shape as encoding a body. So `dispatch_post`'s measured base is reused and the + /// per-item term scaled up: a key is a `Vec` with its own length prefix, not one + /// byte, so 256x the per-byte term covers a key of any realistic size. + /// + /// Over-estimating is the safe direction: it charges more block weight than the call + /// needs, and never lets an under-priced call through. Replace with a real benchmark + /// (linear over key count) when the suite is next run. + fn dispatch_get(k: u32, ) -> Weight { + Weight::from_parts(31_725_511, 3550) + .saturating_add(Weight::from_parts(1_638_400, 0).saturating_mul(k.into())) + .saturating_add(T::DbWeight::get().reads(6_u64)) + .saturating_add(T::DbWeight::get().writes(4_u64)) + .saturating_add(Weight::from_parts(0, 5376).saturating_mul(k.into())) + } /// Storage: `System::Number` (r:1 w:0) /// Proof: `System::Number` (`max_values`: Some(1), `max_size`: Some(4), added: 499, mode: `MaxEncodedLen`) /// Storage: `System::ExecutionPhase` (r:1 w:0) @@ -430,6 +449,24 @@ impl WeightInfo for () { .saturating_add(RocksDbWeight::get().writes(4_u64)) .saturating_add(Weight::from_parts(0, 21).saturating_mul(b.into())) } + /// NOT BENCHMARKED — derived from `dispatch_post`, deliberately. + /// + /// A GET walks the same dispatch path as a POST (one `Ismp::Nonce` read/write, the + /// commitment write, the event) and its only per-item cost is encoding the keys, the + /// same shape as encoding a body. So `dispatch_post`'s measured base is reused and the + /// per-item term scaled up: a key is a `Vec` with its own length prefix, not one + /// byte, so 256x the per-byte term covers a key of any realistic size. + /// + /// Over-estimating is the safe direction: it charges more block weight than the call + /// needs, and never lets an under-priced call through. Replace with a real benchmark + /// (linear over key count) when the suite is next run. + fn dispatch_get(k: u32, ) -> Weight { + Weight::from_parts(31_725_511, 3550) + .saturating_add(Weight::from_parts(1_638_400, 0).saturating_mul(k.into())) + .saturating_add(RocksDbWeight::get().reads(6_u64)) + .saturating_add(RocksDbWeight::get().writes(4_u64)) + .saturating_add(Weight::from_parts(0, 5376).saturating_mul(k.into())) + } /// Storage: `System::Number` (r:1 w:0) /// Proof: `System::Number` (`max_values`: Some(1), `max_size`: Some(4), added: 499, mode: `MaxEncodedLen`) /// Storage: `System::ExecutionPhase` (r:1 w:0) diff --git a/template/runtime/RUNTIME_VERSIONS.md b/template/runtime/RUNTIME_VERSIONS.md index 045df4ee..b1ee75c2 100644 --- a/template/runtime/RUNTIME_VERSIONS.md +++ b/template/runtime/RUNTIME_VERSIONS.md @@ -41,8 +41,64 @@ cross-chain observability: | `GetResponseReceived` | `dest`, `height`, `nonce`, `timeout_timestamp` | | `RequestTimedOut` | `kind`, `nonce`, `timeout_timestamp`, `body_len` | -New enum `RequestKind { Post, Get }`, one byte, so a future `dispatch_get` reuses -these events rather than reshaping them. +New enum `RequestKind { Post, Get }`, one byte, shared by `RequestDispatched` and +`RequestTimedOut`. + +**New extrinsic `dispatch_get` (`call_index(3)`).** `transaction_version` stays at +3: adding a call index leaves indices 0-2 and their argument encodings untouched, +so an offline-signed extrinsic still decodes. Per this file's own rule, `tx` moves +only when the encoding of existing extrinsics changes. + +It exists because a POST can be refused and a GET cannot. A POST is handed to a +module on the destination, which may reject it — `pallet-ismp-demo` on Hyperbridge +rejects any `Substrate(_)` source outright (`modules/pallets/demo/src/lib.rs:372-395`), +and the relayer dry-runs before submitting, so such a message is silently dropped +rather than delivered. A GET has no receiving module: whoever answers it reads the +requested keys on `dest`, and this chain verifies that read against a state commitment +of `dest` it already holds (`ismp-2606.1.0/src/handlers/response.rs`, +`SubstrateStateMachine::verify_state_proof`). Nothing on the far side can turn us away. + +**Who answers it is the honest caveat.** The public relayer does not, today: Tesseract +resolves GETs on Hyperbridge (`tesseract/messaging/messaging/src/get_requests.rs` → +`StateCoprocessor.handle_unsigned`) and delivers the response **only to EVM sources** — +`events.rs:314-336`, *"Substrate sinks can't verify the mmr proof, so they are +skipped"*. So a GET dispatched here is answered by our own relaying script +(`scripts/hyperbridge/relay-get-response.mjs`), which is a legitimate ISMP relayer: it +carries a `state_getReadProof` of Hyperbridge at `height` into `Ismp.handle_unsigned`, +and the chain verifies it against the GRANDPA-tracked commitment. The relayer adds no +trust; it adds bytes. Until upstream delivers to Substrate sinks, `dispatch_get` without +that script is a request nobody will answer. + +**What of Hyperbridge is readable: its ISMP child trie, not its global state.** For the +coprocessor, `ismp-grandpa` records `state_root = child_trie_root` and +`overlay_root = mmr_root` (`ismp-grandpa-2606.0.0/src/consensus.rs:142-150`) — verified +live: our stored commitment equals Gargantua's `ismp.childTrieRoot` at that height, not its +header's `state_root`. So a GET to Hyperbridge must name keys inside `:child_storage:default:ISMPv2` +(a `RequestReceipts`/`RequestCommitments` entry), the proof is `ismp_queryChildTrieProof` +with Hyperbridge's hasher (Keccak), and a GET for a global key such as `Ismp::Nonce` can never +verify here. This holds for any relayer, not only ours. + +Three guards make an unanswerable GET fail at dispatch instead of hanging until it +expires: no keys (asks nothing, still costs a round trip), more than +`MaxGetKeys` = 16 (each key is a membership proof a REMOTE chain must produce, so this +bounds work we impose on someone else), and `height == 0` — the response handler +compares the proof height for *equality* +(`ismp-2606.1.0/src/handlers/response.rs:71`), so a height nobody can prove can never +be answered. + +`dispatch_get`'s weight is **not benchmarked**: it reuses `dispatch_post`'s measured +base with a deliberately generous per-key term, which over-charges rather than +under-charges. Noted in `weights.rs` for whoever next runs the suite. + +**Mainnet builds now whitelist Hyperbridge with `slot_duration = 12000`** (was 6000 +for both targets). The value is the counterparty's Aura slot, and it differs per +deployment: Paseo 6000, Polkadot 12000 (`developers/polkadot/solochains`; confirmed live +against `aura.slotDuration` on Gargantua and Nexus). `ismp-grandpa` reconstructs every +Hyperbridge header timestamp as `aura_slot * slot_duration`, so the old value would have +dated every mainnet state commitment ~28 years early. **Testnet builds are unchanged** +(`--features hyperbridge-testnet` still yields 6000), so nothing on the live chain moves; +the constant now follows the build feature exactly as `coprocessor()` does. Not a storage +change — it only affects what `setup-deployed.mjs` whitelists on a fresh mainnet chain. **`timeout_timestamp` has three states downstream and conflating any two is a bug.** `0` means the message never expires — reproducing upstream's explicit diff --git a/template/runtime/src/configs/ismp/mod.rs b/template/runtime/src/configs/ismp/mod.rs index 23de8299..7a0c2b65 100644 --- a/template/runtime/src/configs/ismp/mod.rs +++ b/template/runtime/src/configs/ismp/mod.rs @@ -263,6 +263,14 @@ impl pallet_ismp_messaging::Config for Runtime { /// are measured over. Keep this and the benchmark's upper bound equal. type MaxBodyLen = ConstU32<8192>; + /// Storage keys per GET. + /// + /// Low on purpose: this bounds work we impose on a REMOTE chain and a relayer, since + /// every key is a separate membership proof they must produce and include. 16 covers + /// any realistic read — a handful of storage items, or one map's worth of entries — + /// and a caller who needs more can send a second request. + type MaxGetKeys = ConstU32<16>; + type WeightInfo = pallet_ismp_messaging::weights::SubstrateWeight; } diff --git a/template/runtime/src/configs/ismp/network.rs b/template/runtime/src/configs/ismp/network.rs index 9a924779..1453675c 100644 --- a/template/runtime/src/configs/ismp/network.rs +++ b/template/runtime/src/configs/ismp/network.rs @@ -20,9 +20,23 @@ pub const HYPERBRIDGE_TESTNET_PARA_ID: u32 = 4009; /// Hyperbridge's slot duration, in milliseconds — the value to whitelist it with. /// -/// This is the *counterparty's* block time, not Orbinum's; they coincide at 6s today. -/// It reaches the chain through `ismp_grandpa::add_state_machines`, so the setup scripts -/// read it from here rather than restating it. +/// This is the *counterparty's* Aura slot, not Orbinum's block time, and it differs per +/// deployment: **6000 on Paseo, 12000 on Polkadot** (`developers/polkadot/solochains`, +/// the `consensus.toml` sample; confirmed live against `aura.slotDuration` on both +/// chains). It is load-bearing: `ismp-grandpa` rebuilds every Hyperbridge header +/// timestamp as `aura_slot * slot_duration` +/// (`ismp-grandpa-2606.0.0/src/consensus.rs:134,182`), so whitelisting mainnet with the +/// testnet value would date every state commitment at half its real age — measured live, +/// ~10 000 days early — and every challenge-period and timeout check would run off that. +/// +/// Switches with the build feature, like [`coprocessor`], so the two cannot drift apart. +/// It reaches the chain through `ismp_grandpa::add_state_machines`; the setup scripts read +/// it from here (via `OrbinumIsmpApi::hyperbridge_slot_duration`) rather than restating it. +#[cfg(not(feature = "hyperbridge-testnet"))] +pub const HYPERBRIDGE_SLOT_DURATION_MS: u64 = 12_000; + +/// See the mainnet variant above. +#[cfg(feature = "hyperbridge-testnet")] pub const HYPERBRIDGE_SLOT_DURATION_MS: u64 = 6_000; /// Orbinum's own four-byte identifier on the ISMP network. @@ -96,6 +110,20 @@ mod tests { ); } + /// The slot duration is the docs' number for the coprocessor this build targets — + /// derived from the coprocessor, not restated, so the two constants cannot be + /// switched independently. + #[test] + fn slot_duration_follows_the_coprocessor() { + // `developers/polkadot/solochains`, `consensus.toml`: Paseo 6000, Polkadot 12000. + let documented = match coprocessor() { + Some(StateMachine::Kusama(HYPERBRIDGE_TESTNET_PARA_ID)) => 6_000, + Some(StateMachine::Polkadot(HYPERBRIDGE_MAINNET_PARA_ID)) => 12_000, + other => panic!("no documented slot duration for coprocessor {other:?}"), + }; + assert_eq!(HYPERBRIDGE_SLOT_DURATION_MS, documented); + } + #[test] fn polkadot_and_kusama_variants_are_not_interchangeable() { assert_ne!( diff --git a/template/runtime/src/configs/ismp/slot_duration.rs b/template/runtime/src/configs/ismp/slot_duration.rs index f0168998..b7a7f929 100644 --- a/template/runtime/src/configs/ismp/slot_duration.rs +++ b/template/runtime/src/configs/ismp/slot_duration.rs @@ -57,8 +57,8 @@ mod tests { #[test] fn accepts_real_chain_slot_durations() { - assert!(validate_slot_duration(6_000)); // Polkadot, Orbinum - assert!(validate_slot_duration(12_000)); // Ethereum + assert!(validate_slot_duration(6_000)); // Hyperbridge on Paseo, Orbinum + assert!(validate_slot_duration(12_000)); // Hyperbridge on Polkadot, Ethereum assert!(validate_slot_duration(MIN_SLOT_DURATION_MS)); assert!(validate_slot_duration(MAX_SLOT_DURATION_MS)); }