Skip to content

Commit 6df3d64

Browse files
zajkozajko
committed
Fixing era utilization score calculation. Fixed some lingering "deploy" naming and comments in places where it should be "transaction". Changed Block#has_hit_slot_capacity and Block#block_utilization so it will intake both a value and reference to TransactionConfig to minimize the need of cloning.
1 parent 0abd7c2 commit 6df3d64

28 files changed

Lines changed: 1147 additions & 361 deletions

‎Cargo.lock‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎node/src/components/block_synchronizer/block_acquisition.rs‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -383,7 +383,9 @@ impl BlockAcquisitionState {
383383
) => {
384384
if false == is_historical {
385385
Err(BlockAcquisitionError::InvalidStateTransition)
386-
} else if transaction_state.needs_transaction() {
386+
} else if block.transaction_count() == 0 || transaction_state.needs_transaction() {
387+
// There is an execution result checksum and there is a derived utilization
388+
// score that is meaningfull even when there are no transactions.
387389
BlockAcquisitionAction::maybe_execution_results(
388390
block,
389391
peer_list,
@@ -506,7 +508,8 @@ impl BlockAcquisitionState {
506508
}
507509
BlockAcquisitionState::HaveStrictFinalitySignatures(block, ..) => {
508510
if is_historical {
509-
// we have enough signatures; need to make sure we've stored the necessary bits
511+
// we have enough signatures; need to make sure we've
512+
// stored the necessary bits
510513
Ok(BlockAcquisitionAction::block_marked_complete(
511514
*block.hash(),
512515
block.height(),
@@ -1199,7 +1202,7 @@ impl BlockAcquisitionState {
11991202
Ok(())
12001203
}
12011204

1202-
/// Register a transactions for this block.
1205+
/// Register a transaction for this block.
12031206
pub(super) fn register_transaction(
12041207
&mut self,
12051208
txn_id: TransactionId,

‎node/src/components/block_synchronizer/tests.rs‎

Lines changed: 121 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2360,13 +2360,15 @@ fn historical_state(block_synchronizer: &BlockSynchronizer) -> &BlockAcquisition
23602360
.block_acquisition_state()
23612361
}
23622362

2363-
/// When there is no deploy, the state goes from `HaveGlobalState` to `HaveStrictFinalitySignature`
2364-
/// directly, skipping `HaveAllExecutionResults`, `HaveApprovalsHashes` and `HaveAllTransactions`.
2363+
/// Even if a block has no transaction it needs to go through "the regular" states becasue in those
2364+
/// states we calculate utilization tracking for the block (even if it's empty we still need to
2365+
/// calculate it's utilization).
23652366
#[tokio::test]
2366-
async fn historical_sync_skips_exec_results_and_deploys_if_block_empty() {
2367+
async fn historical_sync_does_not_skip_exec_results_if_block_empty() {
23672368
let rng = &mut TestRng::new();
23682369
let mock_reactor = MockReactor::new();
2369-
let test_env = TestEnv::random(rng);
2370+
let test_env =
2371+
TestEnv::random(rng).with_block(TestBlockBuilder::new().era(1).build(rng).into());
23702372
let peers = test_env.peers();
23712373
let block = test_env.block();
23722374
let validator_matrix = test_env.gen_validator_matrix();
@@ -2381,8 +2383,6 @@ async fn historical_sync_skips_exec_results_and_deploys_if_block_empty() {
23812383
assert!(block_synchronizer.forward.is_none());
23822384
block_synchronizer.register_peers(*block.hash(), peers.clone());
23832385

2384-
// Skip steps HaveBlockHeader, HaveWeakFinalitySignature, HaveBlock
2385-
23862386
let historical_builder = block_synchronizer
23872387
.historical
23882388
.as_mut()
@@ -2422,24 +2422,24 @@ async fn historical_sync_skips_exec_results_and_deploys_if_block_empty() {
24222422
Event::GlobalStateSynchronizer(global_state_synchronizer::Event::Request(request)),
24232423
);
24242424

2425-
// ----- HaveBlock -----
2426-
assert_matches!(
2427-
historical_state(&block_synchronizer),
2428-
BlockAcquisitionState::HaveBlock { .. }
2429-
);
2430-
24312425
// Those effects are handled directly and not through the reactor:
2432-
let events = effects
2433-
.try_one()
2434-
.expect("there should be only one effect")
2435-
.await;
2426+
let events = effects.one().await;
24362427
assert_matches!(
24372428
events.try_one(),
24382429
Some(Event::GlobalStateSynchronizer(
24392430
GlobalStateSynchronizerEvent::GetPeers(_)
24402431
))
24412432
);
24422433

2434+
// ----- HaveBlock -----
2435+
assert_matches!(
2436+
historical_state(&block_synchronizer),
2437+
BlockAcquisitionState::HaveBlock { .. }
2438+
);
2439+
2440+
// Let's not test the detail of the global synchronization event,
2441+
// since it is already tested in its unit tests.
2442+
24432443
let effects = block_synchronizer.handle_event(
24442444
mock_reactor.effect_builder(),
24452445
rng,
@@ -2457,11 +2457,115 @@ async fn historical_sync_skips_exec_results_and_deploys_if_block_empty() {
24572457
historical_state(&block_synchronizer),
24582458
BlockAcquisitionState::HaveGlobalState { .. }
24592459
);
2460+
2461+
let events = mock_reactor.process_effects(effects).await;
2462+
2463+
match events.try_one() {
2464+
Some(MockReactorEvent::ContractRuntimeRequest(
2465+
ContractRuntimeRequest::GetExecutionResultsChecksum {
2466+
state_root_hash,
2467+
responder,
2468+
},
2469+
)) => responder.respond(ExecutionResultsChecksumResult::Success { checksum: state_root_hash }).await,
2470+
other => panic!("Event should be of type `ContractRuntimeRequest(ContractRuntimeRequest::GetExecutionResultsChecksum) but it is {:?}", other),
2471+
}
2472+
2473+
let effects = block_synchronizer.handle_event(
2474+
mock_reactor.effect_builder(),
2475+
rng,
2476+
Event::GotExecutionResultsChecksum {
2477+
block_hash: *block.hash(),
2478+
result: ExecutionResultsChecksumResult::Success {
2479+
checksum: Digest::SENTINEL_NONE,
2480+
},
2481+
},
2482+
);
2483+
let events = mock_reactor.process_effects(effects).await;
2484+
2485+
for event in events {
2486+
assert_matches!(
2487+
event,
2488+
MockReactorEvent::BlockExecutionResultsOrChunkFetcherRequest(FetcherRequest { .. })
2489+
);
2490+
}
2491+
2492+
let execution_results = BlockExecutionResultsOrChunk::new_empty_value(*block.hash());
2493+
let effects = block_synchronizer.handle_event(
2494+
mock_reactor.effect_builder(),
2495+
rng,
2496+
Event::ExecutionResultsFetched {
2497+
block_hash: *block.hash(),
2498+
result: Ok(FetchedData::from_storage(Box::new(execution_results))),
2499+
},
2500+
);
2501+
2502+
let mut events = mock_reactor.process_effects(effects).await;
2503+
2504+
assert_matches!(
2505+
historical_state(&block_synchronizer),
2506+
BlockAcquisitionState::HaveGlobalState { .. }
2507+
);
2508+
2509+
assert_matches!(
2510+
events.remove(0),
2511+
MockReactorEvent::StorageRequest(StorageRequest::PutExecutionResults { .. })
2512+
);
2513+
for event in events {
2514+
assert_matches!(
2515+
event,
2516+
MockReactorEvent::ApprovalsHashesFetcherRequest(FetcherRequest { .. })
2517+
);
2518+
}
2519+
2520+
let effects = block_synchronizer.handle_event(
2521+
mock_reactor.effect_builder(),
2522+
rng,
2523+
Event::ExecutionResultsStored(*block.hash()),
2524+
);
2525+
// ----- HaveAllExecutionResults -----
2526+
assert_matches!(
2527+
historical_state(&block_synchronizer),
2528+
BlockAcquisitionState::HaveAllExecutionResults(_, _, _, checksum) if checksum.is_checkable()
2529+
);
2530+
24602531
let events = mock_reactor.process_effects(effects).await;
24612532

24622533
for event in events {
2463-
assert_matches!(event, MockReactorEvent::FinalitySignatureFetcherRequest(..));
2534+
assert_matches!(
2535+
event,
2536+
MockReactorEvent::ApprovalsHashesFetcherRequest(FetcherRequest { .. })
2537+
);
24642538
}
2539+
2540+
let effects = block_synchronizer.handle_event(
2541+
mock_reactor.effect_builder(),
2542+
rng,
2543+
Event::ApprovalsHashesFetched(Ok(FetchedData::from_storage(Box::new(
2544+
ApprovalsHashes::new(*block.hash(), vec![], dummy_merkle_proof()),
2545+
)))),
2546+
);
2547+
// ----- HaveApprovalsHashes -----
2548+
assert_matches!(
2549+
historical_state(&block_synchronizer),
2550+
BlockAcquisitionState::HaveApprovalsHashes(_, _, _)
2551+
);
2552+
2553+
let events = mock_reactor.process_effects(effects).await;
2554+
assert!(!events.is_empty());
2555+
// Since the block doesn't have any transactions,
2556+
// the next step should be to fetch the finality signatures for strict finality.
2557+
for event in events {
2558+
assert_matches!(
2559+
event,
2560+
MockReactorEvent::FinalitySignatureFetcherRequest(FetcherRequest {
2561+
id,
2562+
peer,
2563+
..
2564+
}) if peers.contains(&peer) && id.block_hash() == block.hash() && id.era_id() == block.era_id()
2565+
);
2566+
}
2567+
2568+
// The rest would be fetching finality signatures which is covered by other tests
24652569
}
24662570

24672571
#[tokio::test]

‎node/src/components/consensus/era_supervisor.rs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1331,6 +1331,11 @@ impl EraSupervisor {
13311331
let initial_era_height = self.era(era_id).start_height;
13321332
initial_era_height.saturating_add(block_context.ancestor_values().len() as u64)
13331333
}
1334+
1335+
// What is the block height of the next block we expect to execute?
1336+
pub(crate) fn next_executed_height(&self) -> u64 {
1337+
self.next_executed_height
1338+
}
13341339
}
13351340

13361341
/// A serialized consensus network message.

‎node/src/components/contract_runtime.rs‎

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ use crate::{
5454
effect::{
5555
announcements::{
5656
ContractRuntimeAnnouncement, FatalAnnouncement, MetaBlockAnnouncement,
57-
UnexecutedBlockAnnouncement,
57+
NonExecutableBlockAnnouncement, UnexecutedBlockAnnouncement,
5858
},
5959
incoming::{TrieDemand, TrieRequest as TrieRequestMessage, TrieRequestIncoming},
6060
requests::{ContractRuntimeRequest, NetworkRequest, StorageRequest},
@@ -81,7 +81,7 @@ pub(crate) use types::{
8181
BlockAndExecutionArtifacts, ExecutionArtifact, ExecutionPreState, SpeculativeExecutionResult,
8282
StepOutcome,
8383
};
84-
use utils::{exec_or_requeue, run_intensive_task};
84+
use utils::{exec_and_check_next, run_intensive_task};
8585

8686
const COMPONENT_NAME: &str = "contract_runtime";
8787

@@ -196,7 +196,7 @@ impl ContractRuntime {
196196
})
197197
}
198198

199-
pub(crate) fn set_initial_state(&mut self, sequential_block_state: ExecutionPreState) {
199+
pub(crate) fn set_execution_pre_state(&mut self, sequential_block_state: ExecutionPreState) {
200200
let next_block_height = sequential_block_state.next_block_height();
201201
let mut execution_pre_state = self.execution_pre_state.lock().unwrap();
202202
*execution_pre_state = sequential_block_state;
@@ -208,6 +208,12 @@ impl ContractRuntime {
208208
debug!(next_block_height, "ContractRuntime: set initial state");
209209
}
210210

211+
/// Returns the current execution prestate.
212+
pub(crate) fn execution_pre_state(&self) -> ExecutionPreState {
213+
let execution_pre_state = self.execution_pre_state.lock().unwrap();
214+
execution_pre_state.clone()
215+
}
216+
211217
fn new_data_access_layer(
212218
storage_dir: &Path,
213219
contract_runtime_config: &Config,
@@ -314,6 +320,7 @@ impl ContractRuntime {
314320
+ From<MetaBlockAnnouncement>
315321
+ From<UnexecutedBlockAnnouncement>
316322
+ From<FatalAnnouncement>
323+
+ From<NonExecutableBlockAnnouncement>
317324
+ Send,
318325
{
319326
match request {
@@ -540,7 +547,7 @@ impl ContractRuntime {
540547
}
541548
ContractRuntimeRequest::UpdatePreState { new_pre_state } => {
542549
let next_block_height = new_pre_state.next_block_height();
543-
self.set_initial_state(new_pre_state);
550+
self.set_execution_pre_state(new_pre_state);
544551
let current_price = self.current_gas_price.gas_price();
545552
async move {
546553
let block_header = match effect_builder
@@ -635,7 +642,7 @@ impl ContractRuntime {
635642
let current_pre_state = self.execution_pre_state.lock().unwrap();
636643
let next_block_height = current_pre_state.next_block_height();
637644
match finalized_block_height.cmp(&next_block_height) {
638-
// An old block: it won't be executed:
645+
// An old block: it won't be enqueued:
639646
Ordering::Less => {
640647
debug!(
641648
%era_id,
@@ -645,7 +652,7 @@ impl ContractRuntime {
645652
);
646653
effects.extend(
647654
effect_builder
648-
.announce_unexecuted_block(finalized_block_height)
655+
.announce_not_enqueuing_old_executable_block(finalized_block_height)
649656
.ignore(),
650657
);
651658
}
@@ -683,8 +690,13 @@ impl ContractRuntime {
683690
let chainspec = Arc::clone(&self.chainspec);
684691
let metrics = Arc::clone(&self.metrics);
685692
let shared_pre_state = Arc::clone(&self.execution_pre_state);
693+
// the way this works is inobvious. if the current executable block
694+
// executes and its child is enqueued the underlying logic will
695+
// update the pre-state to refer to the child, pop the child from the queue,
696+
// and send a new event of this kind with the child. it will then get into
697+
// this match arm and get executed without being re-enqueued.
686698
effects.extend(
687-
exec_or_requeue(
699+
exec_and_check_next(
688700
data_access_layer,
689701
execution_engine_v1,
690702
execution_engine_v2,
@@ -843,6 +855,7 @@ where
843855
+ From<MetaBlockAnnouncement>
844856
+ From<UnexecutedBlockAnnouncement>
845857
+ From<FatalAnnouncement>
858+
+ From<NonExecutableBlockAnnouncement>
846859
+ Send,
847860
{
848861
type Event = Event;

‎node/src/components/contract_runtime/tests.rs‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,8 @@ impl Unhandled for NetworkRequest<Message> {}
8989

9090
impl Unhandled for UnexecutedBlockAnnouncement {}
9191

92+
impl Unhandled for NonExecutableBlockAnnouncement {}
93+
9294
struct TestConfig {
9395
config: Config,
9496
fixture_name: Option<String>,
@@ -261,7 +263,7 @@ async fn should_not_set_shared_pre_state_to_lower_block_height() {
261263
.reactor_mut()
262264
.inner_mut()
263265
.contract_runtime
264-
.set_initial_state(initial_pre_state);
266+
.set_execution_pre_state(initial_pre_state);
265267

266268
// Create the genesis immediate switch block.
267269
let block_0 = ExecutableBlock::from_finalized_block_and_transactions(
@@ -398,7 +400,7 @@ async fn should_not_set_shared_pre_state_to_lower_block_height() {
398400
.reactor_mut()
399401
.inner_mut()
400402
.contract_runtime
401-
.set_initial_state(ExecutionPreState::new(
403+
.set_execution_pre_state(ExecutionPreState::new(
402404
next_block_height,
403405
Digest::hash(rng.next_u64().to_le_bytes()),
404406
BlockHash::random(rng),
@@ -535,7 +537,7 @@ async fn should_correctly_manage_entity_version_calls() {
535537
.reactor_mut()
536538
.inner_mut()
537539
.contract_runtime
538-
.set_initial_state(initial_pre_state);
540+
.set_execution_pre_state(initial_pre_state);
539541

540542
// Create the genesis immediate switch block.
541543
let block_0 = ExecutableBlock::from_finalized_block_and_transactions(

0 commit comments

Comments
 (0)