From fe7a98efb4e22dc5b11cb904fbc4d62c5d7f0365 Mon Sep 17 00:00:00 2001 From: James Date: Wed, 11 Feb 2026 15:07:54 -0500 Subject: [PATCH] feat: add system tx hash indexing tests and clean up debugging scaffolding Add tests verifying system transaction hashes are properly indexed and lookable via eth_getTransactionByHash. Extend existing RPC tests to cover blocks containing both alloy and host Transact system transactions. Remove leftover debugging helpers (dbg!, dump_index_state, verbose debug panics) from active investigation. Co-Authored-By: Claude Opus 4.6 --- crates/db/src/provider.rs | 2 +- crates/db/tests/db.rs | 90 ++++++++++++++++- crates/node-tests/src/context.rs | 14 ++- crates/node-tests/tests/rpc.rs | 168 ++++++++++++++++++++++++++++--- 4 files changed, 257 insertions(+), 17 deletions(-) diff --git a/crates/db/src/provider.rs b/crates/db/src/provider.rs index 8e11930..f2cd7da 100644 --- a/crates/db/src/provider.rs +++ b/crates/db/src/provider.rs @@ -211,7 +211,7 @@ where // duration metrics have been removed, and the implementation has been // modified to work with a single signet block. // - // last reviewed at tag v1.9.0 + // last reviewed at tag v1.10.1 let from_block = body.0; let sf = self.static_file_provider(); diff --git a/crates/db/tests/db.rs b/crates/db/tests/db.rs index 3a08323..6d4f769 100644 --- a/crates/db/tests/db.rs +++ b/crates/db/tests/db.rs @@ -9,7 +9,7 @@ use alloy::{ use reth::providers::{BlockNumReader, BlockReader}; use signet_constants::test_utils::{DEPLOY_HEIGHT, RU_CHAIN_ID}; use signet_db::RuWriter; -use signet_types::primitives::{RecoveredBlock, SealedBlock, SealedHeader}; +use signet_types::primitives::{RecoveredBlock, SealedBlock, SealedHeader, TransactionSigned}; use signet_zenith::Zenith; #[test] @@ -81,3 +81,91 @@ fn test_insert_signet_block() { let loaded_header = reader.get_signet_header(block.number()).unwrap(); assert_eq!(loaded_header, header); } + +#[test] +fn test_transaction_hash_indexing() { + use reth::providers::TransactionsProvider; + use reth_db::{cursor::DbCursorRO, tables, transaction::DbTx}; + + let factory = test_common::create_test_provider_factory(); + let writer = factory.provider_rw().unwrap(); + + let journal_hash = B256::repeat_byte(0x55); + let header = Some(Zenith::BlockHeader { + rollupChainId: U256::from(RU_CHAIN_ID), + hostBlockNumber: U256::from(DEPLOY_HEIGHT), + gasLimit: U256::from(30_000_000), + rewardAddress: Address::repeat_byte(0x11), + blockDataHash: B256::repeat_byte(0x22), + }); + + // Create transactions with distinct content so they have different hashes + let transactions: Vec = (0..5u64) + .map(|i| { + let tx = TxEip1559 { nonce: i, ..Default::default() }; + TxEnvelope::Eip1559(Signed::new_unhashed(tx, Signature::test_signature())).into() + }) + .collect(); + + // Collect the expected hashes BEFORE inserting + let expected_hashes: Vec = + transactions.iter().map(|tx: &TransactionSigned| *tx.hash()).collect(); + + let block = RecoveredBlock { + block: SealedBlock { + header: SealedHeader::new(alloy::consensus::Header::default()), + body: BlockBody { transactions, ommers: vec![], withdrawals: None }, + }, + senders: std::iter::repeat_n(Address::repeat_byte(0x33), 5).collect(), + }; + + writer.insert_signet_block(header, &block, journal_hash).unwrap(); + writer.commit().unwrap(); + + let reader = factory.provider_rw().unwrap(); + + // Verify each transaction hash is in the index + for (idx, expected_hash) in expected_hashes.iter().enumerate() { + // Method 1: Use provider's transaction_by_hash + let tx_result = reader.transaction_by_hash(*expected_hash).unwrap(); + assert!( + tx_result.is_some(), + "transaction_by_hash failed for tx {} with hash {}", + idx, + expected_hash + ); + + // Method 2: Query TransactionHashNumbers directly + let mut cursor = reader.tx_ref().cursor_read::().unwrap(); + let index_result = cursor.seek_exact(*expected_hash).unwrap(); + assert!( + index_result.is_some(), + "TransactionHashNumbers entry missing for tx {} with hash {}", + idx, + expected_hash + ); + + let (hash, tx_num) = index_result.unwrap(); + assert_eq!(hash, *expected_hash, "Hash mismatch in index for tx {}", idx); + assert_eq!(tx_num, idx as u64, "Unexpected tx_num for tx {}", idx); + } + + // Verify hashes match when loading block back from storage + let loaded_block = reader + .recovered_block_range(block.number()..=block.number()) + .unwrap() + .first() + .cloned() + .unwrap(); + + for (idx, (original_hash, loaded_tx)) in + expected_hashes.iter().zip(loaded_block.body().transactions.iter()).enumerate() + { + let loaded_hash = *loaded_tx.hash(); + assert_eq!( + *original_hash, loaded_hash, + "Hash mismatch after load for tx {}: original={}, loaded={}", + idx, original_hash, loaded_hash + ); + } +} diff --git a/crates/node-tests/src/context.rs b/crates/node-tests/src/context.rs index 42f0a00..cded0bd 100644 --- a/crates/node-tests/src/context.rs +++ b/crates/node-tests/src/context.rs @@ -167,6 +167,16 @@ impl SignetTestContext { (this, node) } + /// Start a new rollup block spec + pub fn start_ru_block(&self) -> RuBlockSpec { + RuBlockSpec::new(self.constants.clone()) + } + + /// Start a new host block spec + pub fn start_host_block(&self) -> HostBlockSpec { + HostBlockSpec::new(self.constants.clone()) + } + /// Set whether an address should be aliased. This will be propagated to /// the running node. pub fn set_should_alias(&self, address: Address, should_alias: bool) { @@ -345,8 +355,8 @@ impl SignetTestContext { ) -> eyre::Result<(TxEnvelope, TransactionReceipt)> { let tx = self.fill_alloy_tx(tx).await?; - let ru_block = RuBlockSpec::new(self.constants.clone()).alloy_tx(&tx); - let host_block = HostBlockSpec::new(self.constants.clone()).submit_block(ru_block); + let ru_block = self.start_ru_block().alloy_tx(&tx); + let host_block = self.start_host_block().submit_block(ru_block); self.process_block(host_block).await?; diff --git a/crates/node-tests/tests/rpc.rs b/crates/node-tests/tests/rpc.rs index 89f8862..9c183e1 100644 --- a/crates/node-tests/tests/rpc.rs +++ b/crates/node-tests/tests/rpc.rs @@ -11,12 +11,12 @@ use alloy::{ Header, eth::{Filter, FilterBlockOption, Log}, }, - sol_types::SolEvent, + sol_types::{SolCall, SolEvent}, }; use reth::providers::{BlockNumReader, BlockReader, TransactionsProvider}; use serial_test::serial; use signet_node_tests::{ - SignetTestContext, + HostBlockSpec, SignetTestContext, constants::TEST_CONSTANTS, rpc_test, types::{Counter, TestCounterInstance}, @@ -161,9 +161,11 @@ async fn test_eth_getTransactionReceipt(ctx: &SignetTestContext, contract: &Test // - eth_getFilterChanges // - eth_newFilter // - eth_getLogs +// - eth_getTransactionByHash (for host Transact system transactions) async fn test_stateful_rpc_calls() { rpc_test(|ctx, contract| async move { let deployer = ctx.addresses[0]; + let transact_sender = ctx.addresses[1]; let (_, _, nonce, block_filter, event_filter) = tokio::join!( withBlock_pre(&ctx, &contract), @@ -176,8 +178,21 @@ async fn test_stateful_rpc_calls() { let latest_block = ctx.alloy_provider.get_block_number().await.unwrap(); tracing::info!(latest_block, "latest block"); + // Build a block that contains both an alloy tx AND a host Transact event let tx = contract.increment().from(deployer).into_transaction_request(); - let _ = ctx.process_alloy_tx(&tx).await.unwrap(); + let filled_tx = ctx.fill_alloy_tx(&tx).await.unwrap(); + + let ru_block = ctx.start_ru_block().alloy_tx(&filled_tx); + let host_block = HostBlockSpec::new(ctx.constants()) + .simple_transact( + transact_sender, + *contract.address(), + Counter::incrementCall::SELECTOR, + 0, + ) + .submit_block(ru_block); + + ctx.process_block(host_block).await.unwrap(); tokio::join!( withBlock_post(&ctx, &contract), @@ -186,6 +201,7 @@ async fn test_stateful_rpc_calls() { newBlockFilter_post(&ctx, block_filter), newFilter_post(&ctx, &contract, event_filter), getLogs_post(&ctx, &contract), + getTransactionByHash_systemTx_post(&ctx, &contract, transact_sender), ); ctx @@ -207,10 +223,53 @@ async fn getLogs_post(ctx: &SignetTestContext, contract: &TestCounterInstance) { .await .unwrap(); - assert_eq!(logs.len(), 1); + // Two logs: one from the host transact, one from the alloy tx + assert_eq!(logs.len(), 2); let log_inner = &logs[0].inner; assert_eq!(log_inner.address, *contract.address()); + // First increment is from the host transact (system tx runs first) assert_eq!(log_inner.topics(), &[Counter::Count::SIGNATURE_HASH, B256::with_last_byte(1)]); + // Second increment is from the alloy tx + let log_inner = &logs[1].inner; + assert_eq!(log_inner.address, *contract.address()); + assert_eq!(log_inner.topics(), &[Counter::Count::SIGNATURE_HASH, B256::with_last_byte(2)]); +} + +async fn getTransactionByHash_systemTx_post( + ctx: &SignetTestContext, + contract: &TestCounterInstance, + transact_sender: Address, +) { + // Retrieve the block with full transactions + let block = ctx + .alloy_provider + .get_block_by_number(BlockNumberOrTag::Latest) + .full() + .await + .unwrap() + .unwrap(); + + let txns = block.transactions.as_transactions().unwrap(); + // Block should have 2 transactions: system tx (from host transact) and alloy tx + assert_eq!(txns.len(), 2); + + // The system transaction should be last + let system_tx = &txns[1]; + assert_eq!(system_tx.from(), transact_sender); + assert_eq!(system_tx.to(), Some(*contract.address())); + assert_eq!(system_tx.value(), U256::ZERO); + assert_eq!(system_tx.input(), &Bytes::from(Counter::incrementCall::SELECTOR)); + + // Verify getTransactionByHash returns the same transaction + let tx_hash = system_tx.tx_hash(); + let rpc_tx = ctx.alloy_provider.get_transaction_by_hash(tx_hash).await.unwrap().unwrap(); + assert_eq!(rpc_tx.tx_hash(), tx_hash); + assert_eq!(rpc_tx.from(), transact_sender); + assert_eq!(rpc_tx.to(), Some(*contract.address())); + assert_eq!(rpc_tx.value(), U256::ZERO); + assert_eq!(rpc_tx.input(), &Bytes::from(Counter::incrementCall::SELECTOR)); + assert_eq!(rpc_tx.block_number, Some(block.header.number)); + assert_eq!(rpc_tx.transaction_index, Some(1)); } async fn newFilter_pre(ctx: &SignetTestContext, contract: &TestCounterInstance) -> U256 { @@ -226,11 +285,16 @@ async fn newFilter_pre(ctx: &SignetTestContext, contract: &TestCounterInstance) async fn newFilter_post(ctx: &SignetTestContext, contract: &TestCounterInstance, filter_id: U256) { let logs: Vec> = ctx.alloy_provider.get_filter_changes(filter_id).await.unwrap(); - assert_eq!(logs.len(), 1); + assert_eq!(logs.len(), 2); let log_inner = &logs[0].inner; assert_eq!(log_inner.address, *contract.address()); assert_eq!(log_inner.topics(), &[Counter::Count::SIGNATURE_HASH, B256::with_last_byte(1)]); assert_eq!(log_inner.data.data, Bytes::new()); + + let log_inner = &logs[1].inner; + assert_eq!(log_inner.address, *contract.address()); + assert_eq!(log_inner.topics(), &[Counter::Count::SIGNATURE_HASH, B256::with_last_byte(2)]); + assert_eq!(log_inner.data.data, Bytes::new()); } async fn newBlockFilter_pre(ctx: &SignetTestContext) -> U256 { @@ -242,8 +306,6 @@ async fn newBlockFilter_post(ctx: &SignetTestContext, filter_id: U256) { let latest_block = ctx.factory.last_block_number().unwrap(); let latest_hash = ctx.factory.block(latest_block.into()).unwrap().unwrap().hash_slow(); - tracing::info!(latest_block, "huh"); - assert_eq!(blocks.len(), 1); assert_eq!(blocks[0], latest_hash); } @@ -269,10 +331,10 @@ async fn getStorageAt_pre(ctx: &SignetTestContext, contract: &TestCounterInstanc } async fn getStorageAt_post(ctx: &SignetTestContext, contract: &TestCounterInstance) { - // storage updated + // storage updated 2x assert_eq!( ctx.alloy_provider.get_storage_at(*contract.address(), U256::ZERO).await.unwrap(), - U256::from(1) + U256::from(2) ); } @@ -397,10 +459,10 @@ async fn withBlock_post(ctx: &SignetTestContext, contract: &TestCounterInstance) assert_eq!(contract.count().call().block(1.into()).await.unwrap(), U256::ZERO); assert_eq!(contract.count().call().block(bh_1.into()).await.unwrap(), U256::ZERO); - // The call at block 2 should return 1 - assert_eq!(contract.count().call().await.unwrap(), U256::from(1)); - assert_eq!(contract.count().call().block(2.into()).await.unwrap(), U256::from(1)); - assert_eq!(contract.count().call().block(bh_2.into()).await.unwrap(), U256::from(1)); + // The call at block 2 should return 2 + assert_eq!(contract.count().call().await.unwrap(), U256::from(2)); + assert_eq!(contract.count().call().block(2.into()).await.unwrap(), U256::from(2)); + assert_eq!(contract.count().call().block(bh_2.into()).await.unwrap(), U256::from(2)); } #[ignore = "This test is slow and should not run by default"] @@ -557,3 +619,83 @@ async fn test_rpc_filter_edge_cases() { }) .await; } + +// -- SYSTEM TRANSACTION HASH CONSISTENCY TESTS -- +// These tests verify that system transaction hashes are properly indexed +// and can be looked up via eth_getTransactionByHash + +#[serial] +#[tokio::test] +async fn test_system_tx_hash_consistency() { + rpc_test(|ctx, contract| async move { + let transact_sender = ctx.addresses[1]; + + // Test: Two consecutive blocks with system transactions + + // Block 2: Single system tx + let host_block = HostBlockSpec::new(ctx.constants()).simple_transact( + transact_sender, + *contract.address(), + Counter::incrementCall::SELECTOR, + 0, + ); + ctx.process_block(host_block).await.unwrap(); + verify_all_txs_in_block(&ctx, 2).await; + + // Block 3: Another single system tx (consecutive system tx blocks) + let host_block = HostBlockSpec::new(ctx.constants()).simple_transact( + transact_sender, + *contract.address(), + Counter::incrementCall::SELECTOR, + 0, + ); + ctx.process_block(host_block).await.unwrap(); + verify_all_txs_in_block(&ctx, 3).await; + + ctx + }) + .await; +} + +/// Verify all transactions in a block can be looked up by hash +async fn verify_all_txs_in_block(ctx: &SignetTestContext, block_number: u64) { + // Get block with full transactions via RPC + let block = + ctx.alloy_provider.get_block_by_number(block_number.into()).full().await.unwrap().unwrap(); + + let txs = block.transactions.as_transactions().unwrap(); + + // Also get transactions directly from DB + let db_txs = ctx.factory.transactions_by_block(block_number.into()).unwrap().unwrap(); + + assert_eq!(txs.len(), db_txs.len(), "Transaction count mismatch in block {}", block_number); + + for (idx, (rpc_tx, db_tx)) in txs.iter().zip(db_txs.iter()).enumerate() { + let rpc_hash = rpc_tx.tx_hash(); + let db_hash = *db_tx.hash(); + + // Verify RPC and DB hashes match + assert_eq!( + rpc_hash, db_hash, + "Hash mismatch between RPC and DB for block {} tx {}", + block_number, idx + ); + + // Verify hash lookup works via RPC + let looked_up = ctx.alloy_provider.get_transaction_by_hash(rpc_hash).await.unwrap(); + assert!( + looked_up.is_some(), + "RPC hash lookup failed: block={block_number}, idx={idx}, hash={rpc_hash}", + ); + + // Verify hash lookup works via DB provider + let provider_lookup = ctx.factory.provider().unwrap().transaction_by_hash(db_hash).unwrap(); + assert!( + provider_lookup.is_some(), + "DB provider hash lookup failed: block={}, idx={}, hash={}", + block_number, + idx, + db_hash + ); + } +}