Skip to content

test: simplify process_blocks test-loop setup + enable spice #15630

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
darioush merged 1 commit into from
Apr 27, 2026
+23 −69
Merged

test: simplify process_blocks test-loop setup + enable spice #15630

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 8 additions & 69 deletions test-loop-tests/src/tests/process_blocks.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,13 @@
use crate::setup::builder::TestLoopBuilder;
use crate::setup::peer_manager_actor::HandlerResult;
use itertools::Itertools as _;
use near_async::messaging::CanSend as _;
use near_async::time::Duration;
use near_chain_configs::test_genesis::{TestEpochConfigBuilder, ValidatorsSpec};
use near_client::BlockResponse;
use near_crypto::{KeyType, PublicKey};
use near_network::types::NetworkResponses;
use near_network::types::{NetworkRequests, ReasonForBan};
use near_o11y::span_wrapped_msg::{SpanWrapped, SpanWrappedMessageExt};
use near_o11y::testonly::init_test_logger;
use near_primitives::hash::hash;
use near_primitives::shard_layout::ShardLayout;
use near_primitives::test_utils::create_test_signer;
use near_primitives::types::{Balance, validator_stake::ValidatorStake};
use parking_lot::RwLock;
Expand All @@ -31,28 +27,8 @@ enum InvalidBlockMode {
fn ban_peer_for_invalid_block_common(mode: InvalidBlockMode) {
init_test_logger();

let block_producers = ["test1", "test2", "test3", "test4"];
let validators_spec = ValidatorsSpec::desired_roles(&block_producers, &[]);
let shard_layout = ShardLayout::single_shard();
let epoch_length = 100;

let genesis = TestLoopBuilder::new_genesis_builder()
.epoch_length(epoch_length)
.shard_layout(shard_layout)
.validators_spec(validators_spec)
.build();

let epoch_config_store = TestEpochConfigBuilder::build_store_from_genesis(&genesis);
let clients = block_producers.into_iter().map(|a| a.parse().unwrap()).collect_vec();
let mut env = TestLoopBuilder::new()
.genesis(genesis)
.epoch_config_store(epoch_config_store)
.clients(clients)
.build();

let client_actor_handle = &env.node_datas[0].client_sender.actor_handle();
let client = &env.test_loop.data.get(&client_actor_handle).client;
let epoch_manager = client.epoch_manager.clone();
let mut env = TestLoopBuilder::new().validators(4, 0).epoch_length(100).build();
let epoch_manager = env.node(0).client().epoch_manager.clone();

let ban_counter: Arc<RwLock<usize>> = Arc::new(RwLock::new(0));
let bad_block_send_height = 8;
Expand Down Expand Up @@ -121,14 +97,7 @@ fn ban_peer_for_invalid_block_common(mode: InvalidBlockMode) {
}));
}

env.test_loop.run_until(
|test_loop_data| {
let client = &test_loop_data.get(client_actor_handle).client;
let head = client.chain.head().unwrap();
head.height >= 25
},
Duration::seconds(60),
);
env.node_runner(0).run_until_head_height(25);

let ban_counter = *ban_counter.read();
match mode {
Expand Down Expand Up @@ -160,33 +129,11 @@ fn test_ban_peer_for_ill_formed_block() {
}

#[test]
// TODO(spice-test): Assess if this test is relevant for spice and if yes fix it.
#[cfg_attr(feature = "protocol_feature_spice", ignore)]
fn test_produce_block_with_approvals_arrived_early() {
init_test_logger();

let block_producers = ["test1", "test2", "test3", "test4"];
let validators_spec = ValidatorsSpec::desired_roles(&block_producers, &[]);
let shard_layout = ShardLayout::multi_shard(4, 3);
let epoch_length = 100;

let genesis = TestLoopBuilder::new_genesis_builder()
.epoch_length(epoch_length)
.shard_layout(shard_layout)
.validators_spec(validators_spec)
.build();

let epoch_config_store = TestEpochConfigBuilder::build_store_from_genesis(&genesis);
let clients = block_producers.into_iter().map(|a| a.parse().unwrap()).collect_vec();
let mut env = TestLoopBuilder::new()
.genesis(genesis)
.epoch_config_store(epoch_config_store)
.clients(clients)
.build();

let client_actor_handle = &env.node_datas[0].client_sender.actor_handle();
let client = &env.test_loop.data.get(&client_actor_handle).client;
let epoch_manager = client.epoch_manager.clone();
let mut env = TestLoopBuilder::new().validators(4, 0).num_shards(4).epoch_length(100).build();
let epoch_manager = env.node(0).client().epoch_manager.clone();
Comment on lines +135 to +136
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This refactor changes the shard layout version used by the test: the previous setup used ShardLayout::multi_shard(4, 3), while TestLoopBuilder::num_shards(4) currently hard-codes ShardLayout::multi_shard(.., 1). If the test is meant to exercise shard-layout version 3 behavior (or keep semantics identical while simplifying setup), consider using .shard_layout(ShardLayout::multi_shard(4, 3)) here (or extending num_shards to accept a version).

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

@darioush darioush Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think version matters for this test.


let block_holder: Arc<RwLock<Option<SpanWrapped<BlockResponse>>>> = Arc::new(RwLock::new(None));
let approval_counter: Arc<RwLock<usize>> = Arc::new(RwLock::new(0));
Expand All @@ -197,7 +144,7 @@ fn test_produce_block_with_approvals_arrived_early() {
.collect();

let block_withholding_height = 10;
let epoch_id = client.chain.head().unwrap().epoch_id;
let epoch_id = env.node(0).head().epoch_id;

let (block_producer_for_next_height, _) = epoch_manager
.get_block_producer_info(&epoch_id, block_withholding_height + 1)
Expand Down Expand Up @@ -270,17 +217,9 @@ fn test_produce_block_with_approvals_arrived_early() {
}));
}

env.test_loop.run_until(
|test_loop_data| {
let client = &test_loop_data.get(client_actor_handle).client;
let head = client.chain.final_head().unwrap();
head.height >= block_withholding_height + 1
},
Duration::seconds(60),
);
env.node_runner(0).run_until_final_head_height(block_withholding_height + 1);

// Block after delayed one should still be produced though approvals for it arrived before the
// block.
let client = &env.test_loop.data.get(client_actor_handle).client;
assert!(client.chain.get_block_by_height(block_withholding_height + 1).is_ok());
assert!(env.node(0).client().chain.get_block_by_height(block_withholding_height + 1).is_ok());
}
15 changes: 15 additions & 0 deletions test-loop-tests/src/utils/node.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,10 @@ impl<'a> TestLoopNode<'a> {
self.client().chain.head().unwrap()
}

pub fn final_head(&self) -> Arc<Tip> {
self.client().chain.final_head().unwrap()
}

pub fn last_executed(&self) -> Arc<Tip> {
if ProtocolFeature::Spice.enabled(PROTOCOL_VERSION) {
self.client().chain.chain_store().spice_execution_head().unwrap()
Expand Down Expand Up @@ -423,6 +427,13 @@ impl<'a> NodeRunner<'a> {
self.run_until(|node| node.head().height >= height, timeout);
}

pub fn run_until_final_head_height(&mut self, height: BlockHeight) {
let initial_height = self.final_head().height;
let height_diff = height.saturating_sub(initial_height) as usize;
let timeout = self.calculate_block_distance_timeout(height_diff);
self.run_until(|node| node.final_head().height >= height, timeout);
}
Comment on lines +430 to +435
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

run_until_final_head_height derives its timeout from height - initial_final_head_height, but final_head is expected to lag behind head by (at least) the Doomslug finality window (e.g. chain tests show final_head at height 4 after processing up to height 6). That means reaching a given final-head height typically requires producing additional blocks beyond height_diff, so this timeout can be too short and make callers flaky. Consider adding a small finality slack (e.g. +2/+3 blocks) or deriving the timeout from run_until on head/last_final_block semantics rather than assuming 1:1 final-head progression.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

@darioush darioush Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems fine to me, we are starting from the original final_head height. We can add slack here if needed or accept a timeout.


pub fn run_until_executed_height(&mut self, height: BlockHeight) {
let initial_height = self.last_executed().height;
let height_diff = height.saturating_sub(initial_height) as usize;
Expand Down Expand Up @@ -596,6 +607,10 @@ impl<'a> NodeRunner<'a> {
self.client().chain.head().unwrap()
}

fn final_head(&self) -> Arc<Tip> {
self.client().chain.final_head().unwrap()
}

fn last_executed(&self) -> Arc<Tip> {
if ProtocolFeature::Spice.enabled(PROTOCOL_VERSION) {
self.client().chain.chain_store().spice_execution_head().unwrap()
Expand Down
Loading