From 54562a49df3f7c4fb24667cc36681cda4d9ea62e Mon Sep 17 00:00:00 2001 From: Marcelo Diop-Gonzalez Date: Mon, 4 Nov 2024 22:34:27 -0500 Subject: [PATCH 1/2] state-sync: rename current epoch state sync protocol feature The old name `StateSyncHashUpdate` just indicates that something is changing, but doesn't say what. CurrentEpochStateSync is more descriptive. --- chain/chain/src/chain.rs | 4 ++-- core/primitives-core/src/version.rs | 4 ++-- core/primitives/src/sharding.rs | 2 +- integration-tests/src/tests/client/process_blocks.rs | 8 ++++---- integration-tests/src/tests/client/sync_state_nodes.rs | 6 +++--- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/chain/chain/src/chain.rs b/chain/chain/src/chain.rs index 5a92862c908..dd33521ca05 100644 --- a/chain/chain/src/chain.rs +++ b/chain/chain/src/chain.rs @@ -2534,7 +2534,7 @@ impl Chain { } let header = self.get_block_header(block_hash)?; let protocol_version = self.epoch_manager.get_epoch_protocol_version(header.epoch_id())?; - if ProtocolFeature::StateSyncHashUpdate.enabled(protocol_version) { + if ProtocolFeature::CurrentEpochStateSync.enabled(protocol_version) { self.chain_store.get_current_epoch_sync_hash(header.epoch_id()) } else { // In the first epoch, it doesn't make sense to sync state to the previous epoch. @@ -3954,7 +3954,7 @@ impl Chain { match snapshot_config.state_snapshot_type { // For every epoch, we snapshot if the next block is the state sync "sync_hash" block StateSnapshotType::EveryEpoch => { - if !ProtocolFeature::StateSyncHashUpdate.enabled(protocol_version) { + if !ProtocolFeature::CurrentEpochStateSync.enabled(protocol_version) { if is_epoch_boundary { // Here we return head.last_block_hash as the prev_hash of the first block of the next epoch Ok(SnapshotAction::MakeSnapshot(head.last_block_hash)) diff --git a/core/primitives-core/src/version.rs b/core/primitives-core/src/version.rs index 3a3cc57897d..28fbd5243a7 100644 --- a/core/primitives-core/src/version.rs +++ b/core/primitives-core/src/version.rs @@ -182,7 +182,7 @@ pub enum ProtocolFeature { /// should no longer be the first block of the epoch, but a couple blocks after that in order /// to sync the current epoch's state. This is not strictly a protocol feature, but is included /// here to coordinate among nodes - StateSyncHashUpdate, + CurrentEpochStateSync, } impl ProtocolFeature { @@ -259,7 +259,7 @@ impl ProtocolFeature { // TODO(#11201): When stabilizing this feature in mainnet, also remove the temporary code // that always enables this for mocknet (see config_mocknet function). ProtocolFeature::ShuffleShardAssignments => 143, - ProtocolFeature::StateSyncHashUpdate => 144, + ProtocolFeature::CurrentEpochStateSync => 144, ProtocolFeature::SimpleNightshadeV4 => 145, ProtocolFeature::ExcludeContractCodeFromStateWitness => 146, ProtocolFeature::BandwidthScheduler => 147, diff --git a/core/primitives/src/sharding.rs b/core/primitives/src/sharding.rs index 1a3471f533a..da70eea68e8 100644 --- a/core/primitives/src/sharding.rs +++ b/core/primitives/src/sharding.rs @@ -153,7 +153,7 @@ impl StateSyncInfo { shard_layout: &ShardLayout, shards: &[ShardId], ) -> Result { - if ProtocolFeature::StateSyncHashUpdate.enabled(protocol_version) { + if ProtocolFeature::CurrentEpochStateSync.enabled(protocol_version) { Self::new_current_epoch(epoch_first_block, prev_block, shard_layout, shards) } else { Self::new_previous_epoch(epoch_first_block, prev_block, shard_layout, shards) diff --git a/integration-tests/src/tests/client/process_blocks.rs b/integration-tests/src/tests/client/process_blocks.rs index 04f4e826a21..253ad205bd5 100644 --- a/integration-tests/src/tests/client/process_blocks.rs +++ b/integration-tests/src/tests/client/process_blocks.rs @@ -2190,7 +2190,7 @@ fn test_sync_hash_validity() { let block_hash = *header.hash(); let valid = env.clients[0].chain.check_sync_hash_validity(&block_hash).unwrap(); println!("height {} -> {}", i, valid); - if ProtocolFeature::StateSyncHashUpdate.enabled(PROTOCOL_VERSION) { + if ProtocolFeature::CurrentEpochStateSync.enabled(PROTOCOL_VERSION) { // This assumes that all shards have new chunks in every block, which should be true // with TestEnv::produce_block() assert_eq!(valid, (i % epoch_length) == 3); @@ -3732,7 +3732,7 @@ mod contract_precompilation_tests { start_height, ); - let sync_height = if ProtocolFeature::StateSyncHashUpdate.enabled(PROTOCOL_VERSION) { + let sync_height = if ProtocolFeature::CurrentEpochStateSync.enabled(PROTOCOL_VERSION) { // `height` is one more than the start of the epoch. Produce two more blocks with chunks, // and then one more than that so the node will generate the neede snapshot. produce_blocks_from_height(&mut env, 3, height) - 2 @@ -3846,7 +3846,7 @@ mod contract_precompilation_tests { height, ); - let sync_height = if ProtocolFeature::StateSyncHashUpdate.enabled(PROTOCOL_VERSION) { + let sync_height = if ProtocolFeature::CurrentEpochStateSync.enabled(PROTOCOL_VERSION) { // `height` is one more than the start of the epoch. Produce two more blocks with chunks, // and then one more than that so the node will generate the neede snapshot. produce_blocks_from_height(&mut env, 3, height) - 2 @@ -3929,7 +3929,7 @@ mod contract_precompilation_tests { // so if we want to state sync the old way, we produce `EPOCH_LENGTH` + 1 new blocks // to get to produce the first block of the next epoch. If we want to state sync the new // way, we produce two more than that, plus one more so that the node will generate the needed snapshot. - let sync_height = if ProtocolFeature::StateSyncHashUpdate.enabled(PROTOCOL_VERSION) { + let sync_height = if ProtocolFeature::CurrentEpochStateSync.enabled(PROTOCOL_VERSION) { produce_blocks_from_height(&mut env, EPOCH_LENGTH + 4, height) - 2 } else { produce_blocks_from_height(&mut env, EPOCH_LENGTH + 1, height) - 1 diff --git a/integration-tests/src/tests/client/sync_state_nodes.rs b/integration-tests/src/tests/client/sync_state_nodes.rs index e94fcf01053..edf6655d5f2 100644 --- a/integration-tests/src/tests/client/sync_state_nodes.rs +++ b/integration-tests/src/tests/client/sync_state_nodes.rs @@ -612,7 +612,7 @@ fn test_dump_epoch_missing_chunk_in_last_block() { // Note that the height to skip here refers to the height at which not to produce chunks for the next block, so really // one before the block height that will have no chunks. The sync_height is the height of the sync_hash block. let (start_skipping_chunks, sync_height) = - if ProtocolFeature::StateSyncHashUpdate.enabled(protocol_version) { + if ProtocolFeature::CurrentEpochStateSync.enabled(protocol_version) { // At the beginning of the epoch, produce one block with chunks and then start skipping chunks. let start_skipping_chunks = next_epoch_start + 1; // Then we will skip `num_chunks_missing` chunks, and produce one more with chunks, which will be the sync height. @@ -862,7 +862,7 @@ fn test_state_sync_headers() { }; tracing::info!(epoch_start_height, "got epoch_start_height"); - let sync_height = if ProtocolFeature::StateSyncHashUpdate.enabled(PROTOCOL_VERSION) + let sync_height = if ProtocolFeature::CurrentEpochStateSync.enabled(PROTOCOL_VERSION) { // here since there's only one block/chunk producer, we assume that no blocks will be missing chunks. epoch_start_height + 2 @@ -1047,7 +1047,7 @@ fn test_state_sync_headers_no_tracked_shards() { return ControlFlow::Continue(()); } - let sync_height = if ProtocolFeature::StateSyncHashUpdate.enabled(PROTOCOL_VERSION) + let sync_height = if ProtocolFeature::CurrentEpochStateSync.enabled(PROTOCOL_VERSION) { // here since there's only one block/chunk producer, we assume that no blocks will be missing chunks. epoch_start_height + 2 From f944dba87942aad07e51c7d4a45804f0f2e8c002 Mon Sep 17 00:00:00 2001 From: Marcelo Diop-Gonzalez Date: Tue, 5 Nov 2024 13:41:58 -0500 Subject: [PATCH 2/2] cargo fmt --- .../src/tests/client/sync_state_nodes.rs | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/integration-tests/src/tests/client/sync_state_nodes.rs b/integration-tests/src/tests/client/sync_state_nodes.rs index edf6655d5f2..2772665fe76 100644 --- a/integration-tests/src/tests/client/sync_state_nodes.rs +++ b/integration-tests/src/tests/client/sync_state_nodes.rs @@ -862,13 +862,13 @@ fn test_state_sync_headers() { }; tracing::info!(epoch_start_height, "got epoch_start_height"); - let sync_height = if ProtocolFeature::CurrentEpochStateSync.enabled(PROTOCOL_VERSION) - { - // here since there's only one block/chunk producer, we assume that no blocks will be missing chunks. - epoch_start_height + 2 - } else { - epoch_start_height - }; + let sync_height = + if ProtocolFeature::CurrentEpochStateSync.enabled(PROTOCOL_VERSION) { + // here since there's only one block/chunk producer, we assume that no blocks will be missing chunks. + epoch_start_height + 2 + } else { + epoch_start_height + }; let block_id = BlockReference::BlockId(BlockId::Height(sync_height)); let block_view = view_client1.send(GetBlock(block_id).with_span_context()).await; let Ok(Ok(block_view)) = block_view else { @@ -1047,13 +1047,13 @@ fn test_state_sync_headers_no_tracked_shards() { return ControlFlow::Continue(()); } - let sync_height = if ProtocolFeature::CurrentEpochStateSync.enabled(PROTOCOL_VERSION) - { - // here since there's only one block/chunk producer, we assume that no blocks will be missing chunks. - epoch_start_height + 2 - } else { - epoch_start_height - }; + let sync_height = + if ProtocolFeature::CurrentEpochStateSync.enabled(PROTOCOL_VERSION) { + // here since there's only one block/chunk producer, we assume that no blocks will be missing chunks. + epoch_start_height + 2 + } else { + epoch_start_height + }; let block_id = BlockReference::BlockId(BlockId::Height(sync_height)); let block_view = view_client2.send(GetBlock(block_id).with_span_context()).await; let Ok(Ok(block_view)) = block_view else {