Fix transaction_index issue with HOP promote

.github/workflows/zombienet-tests.yml

@@ -180,3 +180,57 @@ jobs:
           name: zombienet-sync-test-logs-bulletin-${{ matrix.runtime.name }}
           path: /tmp/zombie-*/**/*.log
           retention-days: 14
+
+  zombienet-hop-tests:
+    needs: [set-image, prepare-binaries]
+    name: hop / ${{ matrix.runtime.name }}
+    runs-on: parity-large
+    container:
+      image: ${{ needs.set-image.outputs.CI_IMAGE }}
+    timeout-minutes: 60
+    strategy:
+      fail-fast: false
+      matrix:
+        runtime:
+          - name: westend
+            para_id: 1010
+          - name: paseo
+            para_id: 1501
+    steps:
+      - name: Checkout sources
+        uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6
+
+      - name: Set up Polkadot binaries
+        uses: ./.github/actions/use-polkadot-binaries
+        with:
+          groups: polkadot-node chain-spec-builder
+          mode: consume
+
+      - name: Rust cache
+        uses: Swatinem/rust-cache@c19371144df3bb44fab255c43d04cbc2ab54d1c4 # v2.9.1
+        with:
+          workspaces: .
+          shared-key: "bulletin-cache-zombienet-hop-tests-${{ matrix.runtime.name }}"
+          save-if: ${{ github.ref == 'refs/heads/main' }}
+
+      - name: Restore bulletin chain specs cache
+        uses: actions/cache/restore@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5
+        with:
+          path: |
+            zombienet/bulletin-westend-spec.json
+            zombienet/bulletin-paseo-spec.json
+          key: bulletin-specs-${{ runner.os }}-${{ github.sha }}
+          fail-on-cache-miss: true
+
+      - name: Run zombienet HOP tests
+        env:
+          PARACHAIN_ID: ${{ matrix.runtime.para_id }}
+        run: just test-zombienet-hop ${{ matrix.runtime.name }}
+
+      - name: Upload logs on failure
+        if: failure()
+        uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
+        with:
+          name: zombienet-hop-test-logs-bulletin-${{ matrix.runtime.name }}
+          path: /tmp/zombie-*/**/*.log
+          retention-days: 14

examples/hop_round_trip.js

@@ -20,14 +20,27 @@
  *   4. Post-auth: can_account_promote → true (cross-checked papi vs SDK).
  *   5. Round-trip submit → claim → ack → assert pool empty + not promoted.
  *   6. Promotion submit → wait for on-chain promotion → assert pool empty.
+ *   7. Retrieve the promoted blob over Bitswap (via the IPFS gateway) and assert
+ *      the returned bytes match the original blob.
+ *
+ * Step 7 is the canary for the transaction-index ordering bug: the node indexes
+ * the trailing `data.len()` bytes of the promote extrinsic under the blob's
+ * content hash (see the FOOTGUN note on
+ * `pallet_bulletin_transaction_storage::Pallet::do_store`). Only when `data` is
+ * the last call argument do those bytes hash to the CID, so the served Bitswap
+ * block is accepted. With the bug the indexed bytes are the wrong extrinsic
+ * tail, the gateway rejects every block on hash mismatch, and step 7 times out.
+ *
+ * Requires an IPFS gateway peered with the bulletin node over Bitswap (see
+ * examples/README.md for the Kubo setup); defaults to the local gateway.
  *
  * Promotion timing (zombienet config):
  *   --hop-retention-secs  60   → HOP pool entry expires after 60s ~ 10 blocks
  *   --hop-check-interval  10   → maintenance task fires every 10 seconds
  *
  * Usage:
- *   node examples/hop_round_trip.js [ws_url] [sudo_derivation_path]
- *   node examples/hop_round_trip.js ws://localhost:10000 //Eve
+ *   node examples/hop_round_trip.js [ws_url] [sudo_derivation_path] [ipfs_gateway_url]
+ *   node examples/hop_round_trip.js ws://localhost:10000 //Eve http://127.0.0.1:8283
  */
 
 import { createClient } from 'polkadot-api';
@@ -42,8 +55,9 @@ import {
 	ss58Address,
 	blake2b256,
 } from '@polkadot-labs/hdkd-helpers';
-import { waitForBlockProduction, toHex } from './common.js';
+import { waitForBlockProduction, toHex, DEFAULT_IPFS_GATEWAY_URL } from './common.js';
 import { authorizeAccount, TX_MODE_FINALIZED_BLOCK } from './api.js';
+import { cidFromBytes } from './cid_dag_metadata.js';
 import {
 	logHeader,
 	logConnection,
@@ -59,13 +73,17 @@ import { bulletin } from './.papi/descriptors/dist/index.js';
 const args = process.argv.slice(2);
 const NODE_WS = args[0] || 'ws://localhost:10000';
 const SUDO_PATH = args[1] || '//Eve';
+const IPFS_GATEWAY = args[2] || DEFAULT_IPFS_GATEWAY_URL;
 const SENDER_PATH = '//CustomSigner';
 
 // ── Timing constants ─────────────────────────────────────────────────────────
 const CLAIM_POLL_INTERVAL_MS = 2_000;
 const CLAIM_TIMEOUT_MS = 60_000;
 const PROMOTION_POLL_INTERVAL_MS = 6_000;   // ~1 block at 6 s/block
 const PROMOTION_TIMEOUT_MS = 300_000;       // ~50 blocks
+const BITSWAP_POLL_INTERVAL_MS = 3_000;     // gap between retrieval attempts
+const BITSWAP_TIMEOUT_MS = 120_000;         // overall deadline for retrieval
+const BITSWAP_ATTEMPT_TIMEOUT_MS = 10_000;  // per-attempt fetch timeout
 
 // HOP requires raw byte signing — never use getPolkadotSigner, which wraps the
 // payload in <Bytes>…</Bytes> and the node would reject the signature.
@@ -115,6 +133,35 @@ async function pollPromotion(hopClient, contentHash) {
 	throw new Error(`Timed out after ${PROMOTION_TIMEOUT_MS / 1000}s waiting for promotion`);
 }
 
+/**
+ * Retrieve a CID from the IPFS gateway over Bitswap, retrying until the block is
+ * served or the deadline elapses. The gateway only returns a block whose hash
+ * matches the requested CID, so a stored blob that was indexed from the wrong
+ * extrinsic bytes (the transaction-index ordering bug) can never satisfy this —
+ * the loop then times out rather than returning corrupt data.
+ */
+async function pollFetchCid(gatewayUrl, cid) {
+	const url = `${gatewayUrl}/ipfs/${cid.toString()}`;
+	const deadline = Date.now() + BITSWAP_TIMEOUT_MS;
+	let lastError;
+	while (Date.now() < deadline) {
+		const controller = new AbortController();
+		const timer = setTimeout(() => controller.abort(), BITSWAP_ATTEMPT_TIMEOUT_MS);
+		try {
+			const res = await fetch(url, { signal: controller.signal });
+			if (res.ok) return Buffer.from(await res.arrayBuffer());
+			lastError = new Error(`HTTP ${res.status}`);
+		} catch (err) {
+			lastError = err;
+		} finally {
+			clearTimeout(timer);
+		}
+		logInfo(`Blob not retrievable over Bitswap yet (${lastError.message}) — retrying in ${BITSWAP_POLL_INTERVAL_MS / 1000}s`);
+		await new Promise(r => setTimeout(r, BITSWAP_POLL_INTERVAL_MS));
+	}
+	throw new Error(`Timed out after ${BITSWAP_TIMEOUT_MS / 1000}s retrieving ${cid.toString()} over Bitswap`);
+}
+
 async function main() {
 	logHeader('HOP END-TO-END TEST');
 	logConnection(NODE_WS, SUDO_PATH, '');
@@ -283,6 +330,25 @@ async function main() {
 			}
 		}
 
+		// ── Step 7: retrieve promoted blob over Bitswap and verify bytes ─────
+		logStep('7️⃣', 'Retrieving promoted blob over Bitswap and verifying bytes…');
+		// RAW codec + blake2b-256 — matches `do_store(data, Blake2b256, RAW_CODEC)`.
+		const promotionCid = await cidFromBytes(promotionData);
+		logInfo(`Promotion CID  : ${promotionCid.toString()}`);
+		logInfo(`IPFS gateway   : ${IPFS_GATEWAY}`);
+		const retrieved = await pollFetchCid(IPFS_GATEWAY, promotionCid);
+		if (!retrieved.equals(Buffer.from(promotionData))) {
+			throw new Error(
+				`Bitswap blob mismatch: gateway returned ${retrieved.length} bytes that differ `
+				+ `from the ${promotionData.length}-byte promoted blob (transaction-index corruption?)`,
+			);
+		}
+		const retrievedMessage = new TextDecoder().decode(retrieved);
+		if (retrievedMessage !== promotionMessage) {
+			throw new Error(`Bitswap content mismatch: expected "${promotionMessage}", got "${retrievedMessage}"`);
+		}
+		logSuccess(`Bitswap retrieval matches the promoted blob (${retrieved.length} bytes) — transaction index is intact.`);
+
 		logTestResult(true, 'HOP End-to-End Test');
 	} catch (err) {
 		logError(err.message);

justfile

@@ -121,6 +121,24 @@ test-zombienet-auto-renew runtime="westend" group="all":
         --features bulletin-chain-zombienet-sdk-tests/zombie-auto-renew-tests \
         -- --test-threads=1 --nocapture "${filter_args[@]}"
 
+# Zombienet HOP-promotion suite. runtime ∈ westend | paseo; filter is the cargo-test substring.
+test-zombienet-hop runtime="westend" filter="parachain_hop_promotion":
+    #!/usr/bin/env bash
+    set -euo pipefail
+    POLKADOT_BIN_DIR="$(just binaries-polkadot)"
+    CSB_DIR="$(just binaries-chain-spec-builder)"
+    export PATH="$CSB_DIR:$PATH"
+    ./scripts/create_bulletin_{{runtime}}_spec.sh
+    export ZOMBIE_PROVIDER=native
+    export POLKADOT_RELAY_BINARY_PATH="$POLKADOT_BIN_DIR/polkadot"
+    export POLKADOT_PARACHAIN_BINARY_PATH="$POLKADOT_BIN_DIR/polkadot-omni-node"
+    export PARACHAIN_CHAIN_SPEC_PATH="$PWD/zombienet/bulletin-{{runtime}}-spec.json"
+    export PARACHAIN_CHAIN_ID="${PARACHAIN_CHAIN_ID:-bulletin-{{runtime}}}"
+    cargo test --release -p bulletin-chain-zombienet-sdk-tests \
+        --features bulletin-chain-zombienet-sdk-tests/zombie-hop-tests \
+        "{{filter}}" \
+        -- --test-threads=1 --nocapture
+
 # Zombienet sync suite. runtime ∈ westend | paseo; filter is the cargo-test substring.
 test-zombienet-sync runtime="westend" filter="parachain_sync_storage":
     #!/usr/bin/env bash

pallets/hop-promotion/src/lib.rs

@@ -126,10 +126,10 @@ pub mod pallet {
 		#[allow(clippy::ptr_arg)]
 		pub fn authorize_promote(
 			source: TransactionSource,
-			data: &Vec<u8>,
 			signer: &MultiSigner,
 			signature: &MultiSignature,
 			submit_timestamp: &u64,
+			data: &Vec<u8>,
 		) -> Result<(ValidTransaction, Weight), TransactionValidityError> {
 			if matches!(source, TransactionSource::External) {
 				return Err(InvalidTransaction::Call.into());
@@ -188,12 +188,17 @@ pub mod pallet {
 		#[pallet::weight_of_authorize(<T as Config>::WeightInfo::authorize_promote(data.len() as u32))]
 		// `signer`/`signature`/`submit_timestamp` are validated by `authorize_promote`
 		// above; the dispatch body trusts them and only runs after authorization.
+		//
+		// `data` MUST be the last argument — see the FOOTGUN note on
+		// `pallet_bulletin_transaction_storage::Pallet::do_store`: the trailing
+		// `data.len()` bytes of the encoded extrinsic get indexed, so any field
+		// encoded after `data` corrupts the stored blob.
 		pub fn promote(
 			origin: OriginFor<T>,
-			data: Vec<u8>,
 			_signer: MultiSigner,
 			_signature: MultiSignature,
 			_submit_timestamp: u64,
+			data: Vec<u8>,
 		) -> DispatchResult {
 			ensure_authorized(origin)?;
 			pallet_bulletin_transaction_storage::Pallet::<T>::do_store(

pallets/hop-promotion/src/tests.rs

@@ -80,7 +80,7 @@ fn promote_succeeds_with_valid_data() {
 		frame_system::Pallet::<Test>::set_extrinsic_index(0);
 		let data = vec![42u8; 100];
 		let (signer, sig) = dummy_signer_and_sig();
-		assert_ok!(HopPromotion::promote(authorized_origin(), data, signer, sig, 0));
+		assert_ok!(HopPromotion::promote(authorized_origin(), signer, sig, 0, data));
 	});
 }
 
@@ -91,7 +91,7 @@ fn promote_rejects_empty_data() {
 		frame_system::Pallet::<Test>::set_extrinsic_index(0);
 		let (signer, sig) = dummy_signer_and_sig();
 		assert_noop!(
-			HopPromotion::promote(authorized_origin(), vec![], signer, sig, 0),
+			HopPromotion::promote(authorized_origin(), signer, sig, 0, vec![]),
 			pallet_bulletin_transaction_storage::Error::<Test>::BadDataSize,
 		);
 	});
@@ -106,10 +106,10 @@ fn promote_rejects_oversized_data() {
 		assert_noop!(
 			HopPromotion::promote(
 				authorized_origin(),
-				vec![0u8; TEST_MAX_TRANSACTION_SIZE as usize + 1],
 				signer,
 				sig,
 				0,
+				vec![0u8; TEST_MAX_TRANSACTION_SIZE as usize + 1],
 			),
 			pallet_bulletin_transaction_storage::Error::<Test>::BadDataSize,
 		);
@@ -125,25 +125,25 @@ fn promote_rejects_non_authorized_origins() {
 		assert_noop!(
 			HopPromotion::promote(
 				RuntimeOrigin::none(),
-				data.clone(),
 				signer.clone(),
 				sig.clone(),
-				0
+				0,
+				data.clone(),
 			),
 			sp_runtime::traits::BadOrigin,
 		);
 		assert_noop!(
 			HopPromotion::promote(
 				RuntimeOrigin::signed(Sr25519Keyring::Alice.to_account_id()),
-				data.clone(),
 				signer.clone(),
 				sig.clone(),
 				0,
+				data.clone(),
 			),
 			sp_runtime::traits::BadOrigin,
 		);
 		assert_noop!(
-			HopPromotion::promote(RuntimeOrigin::root(), data, signer, sig, 0),
+			HopPromotion::promote(RuntimeOrigin::root(), signer, sig, 0, data),
 			sp_runtime::traits::BadOrigin,
 		);
 	});

pallets/transaction-storage/src/lib.rs

@@ -1505,6 +1505,15 @@ pub mod pallet {
 
 		/// Common implementation for [`store`](Self::store) and
 		/// [`store_with_cid_config`](Self::store_with_cid_config).
+		///
+		/// FOOTGUN: `sp_io::transaction_index::index` (called below) indexes the
+		/// *trailing* `data_len` bytes of the encoded extrinsic. Since an extrinsic
+		/// encodes as `preamble ++ call`, `data` must be the LAST field of any
+		/// dispatchable that funnels into `do_store` (e.g. [`store`](Self::store),
+		/// [`store_with_cid_config`](Self::store_with_cid_config),
+		/// `pallet-bulletin-hop-promotion::promote`). A field encoded after `data`
+		/// shifts the indexed window onto the wrong bytes and corrupts the stored
+		/// blob — without any dispatch error to flag it.
 		pub fn do_store(
 			data: Vec<u8>,
 			hashing: HashingAlgorithm,
@@ -1544,6 +1553,8 @@ pub mod pallet {
 				TransactionKind::Store,
 			)?;
 			// Index after the runtime mutation — index ops aren't rolled back on dispatch error.
+			// Indexes the trailing `data_len` bytes of the extrinsic, so `data` must be the
+			// caller's last call argument (see the FOOTGUN note on `do_store`).
 			sp_io::transaction_index::index(extrinsic_index, data_len, cid.content_hash);
 
 			Self::deposit_event(Event::Stored {

zombienet-sdk-tests/Cargo.toml

@@ -44,6 +44,7 @@ pallet-bulletin-transaction-storage = { path = "../pallets/transaction-storage",
 [features]
 zombie-sync-tests = []
 zombie-auto-renew-tests = []
+zombie-hop-tests = []
 
 [build-dependencies]
 prost-build = "0.13"

zombienet-sdk-tests/README.md

@@ -48,8 +48,8 @@ export ROCKSDB_LDB_PATH=/path/to/ldb
 
 ## Running tests
 
-Tests are gated behind feature flags (`zombie-sync-tests`, `zombie-auto-renew-tests`)
-so `cargo test --workspace` doesn't accidentally fire them.
+Tests are gated behind feature flags (`zombie-sync-tests`, `zombie-auto-renew-tests`,
+`zombie-hop-tests`) so `cargo test --workspace` doesn't accidentally fire them.
 
 Recommended path — `just` recipes from the repo root:
 
@@ -65,6 +65,9 @@ just test-zombienet-auto-renew
 
 # Single auto-renew test:
 just test-zombienet-auto-renew westend parachain_auto_renew_quota_exhaustion_test
+
+# HOP promotion suite:
+just test-zombienet-hop
 ```
 
 The recipes fetch the right binaries, generate the chain spec, export the env
@@ -92,6 +95,7 @@ are resource-intensive.
 | `parachain_full_sync_relay_warp_sync_test` | full + warp (relay) | no | Relay warp syncs, parachain full syncs, bitswap works |
 | `parachain_rpc_node_bitswap_test` | full | no | RPC node syncs and serves data via bitswap |
 | `parachain_ldb_storage_verification_test` | - | yes | Verifies col11 refcounting and data expiration |
+| `parachain_hop_promotion_bitswap_test` | full | no | HOP `hop_submit` → promote → `ProofChecked` passes; bitswap-content-match assertion is **expected to fail** (drives out the HOP↔bitswap CID/indexing gap) |
 
 ## Environment variables
 
@@ -145,6 +149,7 @@ A single workflow (`.github/workflows/zombienet-tests.yml`) hosts both suites:
 |---|---|
 | `zombienet-auto-renew-tests` | Every PR push + `workflow_dispatch` |
 | `zombienet-sync-tests` | `zombienet-sync-tests` PR label + `workflow_dispatch` |
+| `zombienet-hop-tests` | Every PR push + `workflow_dispatch` (test is expected to fail until the HOP→col11/bitswap gap is fixed) |
 
 A shared `prepare-binaries` job fetches/builds the polkadot binaries once and uploads them
 as an artifact; both suites download that artifact instead of building locally. Each suite