diff --git a/Cargo.lock b/Cargo.lock index 4ab17dbdbd8d6..c7d752ed1fca8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -897,6 +897,7 @@ dependencies = [ "aptos-safety-rules", "aptos-schemadb", "aptos-secure-storage", + "aptos-short-hex-str", "aptos-storage-interface", "aptos-temppath", "aptos-time-service", @@ -1347,6 +1348,7 @@ version = "0.1.0" dependencies = [ "aptos-infallible", "aptos-metrics-core", + "derive_more", "once_cell", "threadpool", ] @@ -1546,12 +1548,14 @@ version = "0.1.0" dependencies = [ "anyhow", "aptos-crypto", + "aptos-drop-helper", "aptos-scratchpad", "aptos-secure-net", "aptos-storage-interface", "aptos-types", "bcs 0.1.4", "criterion", + "derive_more", "itertools 0.13.0", "once_cell", "serde", @@ -2095,6 +2099,25 @@ dependencies = [ "uuid", ] +[[package]] +name = "aptos-indexer-grpc-file-checker" +version = "1.0.0" +dependencies = [ + "anyhow", + "aptos-indexer-grpc-server-framework", + "aptos-indexer-grpc-utils", + "aptos-metrics-core", + "async-trait", + "clap 4.4.14", + "cloud-storage", + "jemallocator", + "once_cell", + "serde", + "serde_json", + "tokio", + "tracing", +] + [[package]] name = "aptos-indexer-grpc-file-store" version = "1.0.0" diff --git a/Cargo.toml b/Cargo.toml index d1f271a95416b..60121f8b12684 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -116,6 +116,7 @@ members = [ "dkg", "ecosystem/indexer-grpc/indexer-grpc-cache-worker", "ecosystem/indexer-grpc/indexer-grpc-data-service", + "ecosystem/indexer-grpc/indexer-grpc-file-checker", "ecosystem/indexer-grpc/indexer-grpc-file-store", "ecosystem/indexer-grpc/indexer-grpc-file-store-backfiller", "ecosystem/indexer-grpc/indexer-grpc-fullnode", @@ -360,6 +361,7 @@ aptos-indexer = { path = "crates/indexer" } aptos-indexer-grpc-cache-worker = { path = "ecosystem/indexer-grpc/indexer-grpc-cache-worker" } aptos-indexer-grpc-data-service = { path = "ecosystem/indexer-grpc/indexer-grpc-data-service" } aptos-indexer-grpc-file-store = { path = "ecosystem/indexer-grpc/indexer-grpc-file-store" } +aptos-indexer-grpc-file-checker = { path = "ecosystem/indexer-grpc/indexer-grpc-file-checker" } aptos-indexer-grpc-file-store-backfiller = { path = "ecosystem/indexer-grpc/indexer-grpc-file-store-backfiller" } aptos-indexer-grpc-fullnode = { path = "ecosystem/indexer-grpc/indexer-grpc-fullnode" } aptos-indexer-grpc-in-memory-cache-benchmark = { path = "ecosystem/indexer-grpc/indexer-grpc-in-memory-cache-benchmark" } diff --git a/RUST_SECURE_CODING.md b/RUST_SECURE_CODING.md index 698114c9e0ebd..99c7768ee6fdc 100644 --- a/RUST_SECURE_CODING.md +++ b/RUST_SECURE_CODING.md @@ -6,7 +6,7 @@ These Rust Secure Coding Guidelines are essential for anyone contributing to Apt ### Rustup -Utilize Rustup for managing Rust toolchains. However, keep in mind that, from a security perspective, Rustup performs all downloads over HTTPS, but it does not yet validate signatures of downloads. Security is shifted to [create.io](http://create.io) and GitHub repository hosting the code [[rustup]](https://www.rust-lang.org/tools/install). +Utilize Rustup for managing Rust toolchains. However, keep in mind that, from a security perspective, Rustup performs all downloads over HTTPS, but it does not yet validate signatures of downloads. Security is shifted to [crates.io](http://crates.io) and GitHub repository hosting the code [[rustup]](https://www.rust-lang.org/tools/install). ### Stable Toolchain diff --git a/aptos-move/aptos-gas-schedule/src/ver.rs b/aptos-move/aptos-gas-schedule/src/ver.rs index f798c42b40145..2c5ec6d5049c0 100644 --- a/aptos-move/aptos-gas-schedule/src/ver.rs +++ b/aptos-move/aptos-gas-schedule/src/ver.rs @@ -69,7 +69,7 @@ /// global operations. /// - V1 /// - TBA -pub const LATEST_GAS_FEATURE_VERSION: u64 = gas_feature_versions::RELEASE_V1_21; +pub const LATEST_GAS_FEATURE_VERSION: u64 = gas_feature_versions::RELEASE_V1_22; pub mod gas_feature_versions { pub const RELEASE_V1_8: u64 = 11; @@ -86,4 +86,7 @@ pub mod gas_feature_versions { pub const RELEASE_V1_19: u64 = 23; pub const RELEASE_V1_20: u64 = 24; pub const RELEASE_V1_21: u64 = 25; + pub const RELEASE_V1_22: u64 = 26; + pub const RELEASE_V1_23: u64 = 27; + pub const RELEASE_V1_24: u64 = 28; } diff --git a/config/src/config/consensus_config.rs b/config/src/config/consensus_config.rs index f907072edfb67..93a3fc15c59cd 100644 --- a/config/src/config/consensus_config.rs +++ b/config/src/config/consensus_config.rs @@ -322,7 +322,7 @@ impl Default for ConsensusConfig { enable_pre_commit: true, max_pending_rounds_in_commit_vote_cache: 100, optimistic_sig_verification: false, - enable_round_timeout_msg: false, + enable_round_timeout_msg: true, } } } diff --git a/config/src/config/consensus_observer_config.rs b/config/src/config/consensus_observer_config.rs index 0ca55c31d50e9..e55f82bf052d5 100644 --- a/config/src/config/consensus_observer_config.rs +++ b/config/src/config/consensus_observer_config.rs @@ -9,8 +9,8 @@ use serde::{Deserialize, Serialize}; use serde_yaml::Value; // Useful constants for enabling consensus observer on different node types -const ENABLE_ON_VALIDATORS: bool = false; -const ENABLE_ON_VALIDATOR_FULLNODES: bool = false; +const ENABLE_ON_VALIDATORS: bool = true; +const ENABLE_ON_VALIDATOR_FULLNODES: bool = true; const ENABLE_ON_PUBLIC_FULLNODES: bool = false; #[derive(Clone, Copy, Debug, Deserialize, PartialEq, Serialize)] @@ -18,7 +18,7 @@ const ENABLE_ON_PUBLIC_FULLNODES: bool = false; pub struct ConsensusObserverConfig { /// Whether the consensus observer is enabled pub observer_enabled: bool, - /// Whether the consensus observer publisher is enabled + /// Whether the consensus publisher is enabled pub publisher_enabled: bool, /// Maximum number of pending network messages @@ -30,21 +30,27 @@ pub struct ConsensusObserverConfig { /// Interval (in milliseconds) to garbage collect peer state pub garbage_collection_interval_ms: u64, - /// The maximum number of concurrent subscriptions - pub max_concurrent_subscriptions: u64, /// Maximum number of blocks to keep in memory (e.g., pending blocks, ordered blocks, etc.) pub max_num_pending_blocks: u64, - /// Maximum timeout (in milliseconds) for active subscriptions - pub max_subscription_timeout_ms: u64, - /// Maximum timeout (in milliseconds) we'll wait for the synced version to - /// increase before terminating the active subscription. - pub max_synced_version_timeout_ms: u64, /// Interval (in milliseconds) to check progress of the consensus observer pub progress_check_interval_ms: u64, + + /// The maximum number of concurrent subscriptions + pub max_concurrent_subscriptions: u64, + /// Maximum timeout (in milliseconds) we'll wait for the synced version to + /// increase before terminating the active subscription. + pub max_subscription_sync_timeout_ms: u64, + /// Maximum message timeout (in milliseconds) for active subscriptions + pub max_subscription_timeout_ms: u64, /// Interval (in milliseconds) to check for subscription related peer changes pub subscription_peer_change_interval_ms: u64, /// Interval (in milliseconds) to refresh the subscription pub subscription_refresh_interval_ms: u64, + + /// Duration (in milliseconds) to require state sync to synchronize when in fallback mode + pub observer_fallback_duration_ms: u64, + /// Duration (in milliseconds) we'll wait for syncing progress before entering fallback mode + pub observer_fallback_sync_threshold_ms: u64, } impl Default for ConsensusObserverConfig { @@ -56,13 +62,15 @@ impl Default for ConsensusObserverConfig { max_parallel_serialization_tasks: num_cpus::get(), // Default to the number of CPUs network_request_timeout_ms: 5_000, // 5 seconds garbage_collection_interval_ms: 60_000, // 60 seconds - max_concurrent_subscriptions: 2, // 2 streams should be sufficient max_num_pending_blocks: 100, // 100 blocks - max_subscription_timeout_ms: 30_000, // 30 seconds - max_synced_version_timeout_ms: 60_000, // 60 seconds progress_check_interval_ms: 5_000, // 5 seconds + max_concurrent_subscriptions: 2, // 2 streams should be sufficient + max_subscription_sync_timeout_ms: 15_000, // 15 seconds + max_subscription_timeout_ms: 15_000, // 15 seconds subscription_peer_change_interval_ms: 60_000, // 1 minute subscription_refresh_interval_ms: 300_000, // 5 minutes + observer_fallback_duration_ms: 600_000, // 10 minutes + observer_fallback_sync_threshold_ms: 30_000, // 30 seconds } } } diff --git a/consensus/Cargo.toml b/consensus/Cargo.toml index 2aa1ee436769c..667b1afb13188 100644 --- a/consensus/Cargo.toml +++ b/consensus/Cargo.toml @@ -42,6 +42,7 @@ aptos-runtimes = { workspace = true } aptos-safety-rules = { workspace = true } aptos-schemadb = { workspace = true } aptos-secure-storage = { workspace = true } +aptos-short-hex-str = { workspace = true } aptos-storage-interface = { workspace = true } aptos-temppath = { workspace = true } aptos-time-service = { workspace = true } diff --git a/consensus/consensus-types/src/pipelined_block.rs b/consensus/consensus-types/src/pipelined_block.rs index 375dd93a0a1cf..72713f530d778 100644 --- a/consensus/consensus-types/src/pipelined_block.rs +++ b/consensus/consensus-types/src/pipelined_block.rs @@ -44,6 +44,7 @@ pub struct PipelinedBlock { /// The state_compute_result is calculated for all the pending blocks prior to insertion to /// the tree. The execution results are not persisted: they're recalculated again for the /// pending blocks upon restart. + #[derivative(PartialEq = "ignore")] state_compute_result: StateComputeResult, randomness: OnceCell, pipeline_insertion_time: OnceCell, @@ -62,14 +63,12 @@ impl Serialize for PipelinedBlock { struct SerializedBlock<'a> { block: &'a Block, input_transactions: &'a Vec, - state_compute_result: &'a StateComputeResult, randomness: Option<&'a Randomness>, } let serialized = SerializedBlock { block: &self.block, input_transactions: &self.input_transactions, - state_compute_result: &self.state_compute_result, randomness: self.randomness.get(), }; serialized.serialize(serializer) @@ -86,21 +85,19 @@ impl<'de> Deserialize<'de> for PipelinedBlock { struct SerializedBlock { block: Block, input_transactions: Vec, - state_compute_result: StateComputeResult, randomness: Option, } let SerializedBlock { block, input_transactions, - state_compute_result, randomness, } = SerializedBlock::deserialize(deserializer)?; let block = PipelinedBlock { block, input_transactions, - state_compute_result, + state_compute_result: StateComputeResult::new_dummy(), randomness: OnceCell::new(), pipeline_insertion_time: OnceCell::new(), execution_summary: Arc::new(OnceCell::new()), diff --git a/consensus/src/block_storage/block_store.rs b/consensus/src/block_storage/block_store.rs index f56dfa80c23eb..cbc4c85a139b5 100644 --- a/consensus/src/block_storage/block_store.rs +++ b/consensus/src/block_storage/block_store.rs @@ -32,7 +32,9 @@ use aptos_crypto::{hash::ACCUMULATOR_PLACEHOLDER_HASH, HashValue}; use aptos_executor_types::StateComputeResult; use aptos_infallible::{Mutex, RwLock}; use aptos_logger::prelude::*; -use aptos_types::ledger_info::LedgerInfoWithSignatures; +use aptos_types::{ + ledger_info::LedgerInfoWithSignatures, proof::accumulator::InMemoryTransactionAccumulator, +}; use futures::executor::block_on; #[cfg(test)] use std::collections::VecDeque; @@ -175,18 +177,14 @@ impl BlockStore { root_metadata.accu_hash, ); - let result = StateComputeResult::new( - root_metadata.accu_hash, - root_metadata.frozen_root_hashes, - root_metadata.num_leaves, /* num_leaves */ - vec![], /* parent_root_hashes */ - 0, /* parent_num_leaves */ - None, /* epoch_state */ - vec![], /* compute_status */ - vec![], /* txn_infos */ - vec![], /* reconfig_events */ - None, // block end info - ); + let result = StateComputeResult::new_empty(Arc::new( + InMemoryTransactionAccumulator::new( + root_metadata.frozen_root_hashes, + root_metadata.num_leaves, + ) + .expect("Failed to recover accumulator."), + )); + assert_eq!(result.root_hash(), root_metadata.accu_hash); let pipelined_root_block = PipelinedBlock::new( *root_block, diff --git a/consensus/src/block_storage/block_tree.rs b/consensus/src/block_storage/block_tree.rs index 5d1df54149cbf..4d13c742747d4 100644 --- a/consensus/src/block_storage/block_tree.rs +++ b/consensus/src/block_storage/block_tree.rs @@ -19,7 +19,7 @@ use aptos_types::{ block_info::{BlockInfo, Round}, ledger_info::LedgerInfoWithSignatures, }; -use mirai_annotations::{checked_verify_eq, precondition}; +use mirai_annotations::precondition; use std::{ collections::{vec_deque::VecDeque, BTreeMap, HashMap, HashSet}, sync::Arc, @@ -249,7 +249,6 @@ impl BlockTree { existing_block, block_id, block); - checked_verify_eq!(existing_block.compute_result(), block.compute_result()); Ok(existing_block) } else { match self.get_linkable_block_mut(&block.parent_id()) { diff --git a/consensus/src/block_storage/sync_manager.rs b/consensus/src/block_storage/sync_manager.rs index 7fad1188d4b05..07b20edf8a922 100644 --- a/consensus/src/block_storage/sync_manager.rs +++ b/consensus/src/block_storage/sync_manager.rs @@ -432,7 +432,7 @@ impl BlockStore { storage.save_tree(blocks.clone(), quorum_certs.clone())?; execution_client - .sync_to(highest_commit_cert.ledger_info().clone()) + .sync_to_target(highest_commit_cert.ledger_info().clone()) .await?; // we do not need to update block_tree.highest_commit_decision_ledger_info here diff --git a/consensus/src/consensus_observer/common/error.rs b/consensus/src/consensus_observer/common/error.rs index 7fc6a78785a96..6201e66fc4313 100644 --- a/consensus/src/consensus_observer/common/error.rs +++ b/consensus/src/consensus_observer/common/error.rs @@ -12,6 +12,9 @@ pub enum Error { #[error("Network error: {0}")] NetworkError(String), + #[error("Consensus observer progress stopped: {0}")] + ObserverProgressStopped(String), + #[error("Aptos network rpc error: {0}")] RpcError(#[from] RpcError), @@ -40,6 +43,7 @@ impl Error { match self { Self::InvalidMessageError(_) => "invalid_message_error", Self::NetworkError(_) => "network_error", + Self::ObserverProgressStopped(_) => "observer_progress_stopped", Self::RpcError(_) => "rpc_error", Self::SubscriptionDisconnected(_) => "subscription_disconnected", Self::SubscriptionProgressStopped(_) => "subscription_progress_stopped", diff --git a/consensus/src/consensus_observer/common/metrics.rs b/consensus/src/consensus_observer/common/metrics.rs index 5888bbfcaca26..6606ecab0d4da 100644 --- a/consensus/src/consensus_observer/common/metrics.rs +++ b/consensus/src/consensus_observer/common/metrics.rs @@ -10,7 +10,7 @@ use aptos_metrics_core::{ }; use once_cell::sync::Lazy; -// Useful metric labels +// Useful observer metric labels pub const BLOCK_PAYLOAD_LABEL: &str = "block_payload"; pub const COMMIT_DECISION_LABEL: &str = "commit_decision"; pub const COMMITTED_BLOCKS_LABEL: &str = "committed_blocks"; @@ -21,6 +21,10 @@ pub const PENDING_BLOCK_ENTRIES_LABEL: &str = "pending_block_entries"; pub const PENDING_BLOCKS_LABEL: &str = "pending_blocks"; pub const STORED_PAYLOADS_LABEL: &str = "stored_payloads"; +// Useful state sync metric labels +pub const STATE_SYNCING_FOR_FALLBACK: &str = "sync_for_fallback"; +pub const STATE_SYNCING_TO_COMMIT: &str = "sync_to_commit"; + /// Counter for tracking created subscriptions for the consensus observer pub static OBSERVER_CREATED_SUBSCRIPTIONS: Lazy = Lazy::new(|| { register_int_counter_vec!( @@ -149,6 +153,16 @@ pub static OBSERVER_SENT_REQUESTS: Lazy = Lazy::new(|| { .unwrap() }); +/// Gauge for tracking when consensus observer has invoked state sync +pub static OBSERVER_STATE_SYNC_EXECUTING: Lazy = Lazy::new(|| { + register_int_gauge_vec!( + "consensus_observer_state_sync_executing", + "Gauge for tracking when consensus observer has invoked state sync", + &["syncing_mode"] + ) + .unwrap() +}); + /// Counter for tracking terminated subscriptions for the consensus observer pub static OBSERVER_TERMINATED_SUBSCRIPTIONS: Lazy = Lazy::new(|| { register_int_counter_vec!( diff --git a/consensus/src/consensus_observer/observer/consensus_observer.rs b/consensus/src/consensus_observer/observer/consensus_observer.rs index 032a3fa38f8bc..0384abbe3dcde 100644 --- a/consensus/src/consensus_observer/observer/consensus_observer.rs +++ b/consensus/src/consensus_observer/observer/consensus_observer.rs @@ -16,8 +16,12 @@ use crate::{ }, }, observer::{ - active_state::ActiveObserverState, ordered_blocks::OrderedBlockStore, - payload_store::BlockPayloadStore, pending_blocks::PendingBlockStore, + active_state::ActiveObserverState, + fallback_manager::ObserverFallbackManager, + ordered_blocks::OrderedBlockStore, + payload_store::BlockPayloadStore, + pending_blocks::PendingBlockStore, + state_sync_manager::{StateSyncManager, StateSyncNotification}, subscription_manager::SubscriptionManager, }, publisher::consensus_publisher::ConsensusPublisher, @@ -40,18 +44,15 @@ use aptos_logger::{debug, error, info, warn}; use aptos_network::{ application::interface::NetworkClient, protocols::wire::handshake::v1::ProtocolId, }; -use aptos_reliable_broadcast::DropGuard; use aptos_storage_interface::DbReader; use aptos_time_service::TimeService; use aptos_types::{ block_info::{BlockInfo, Round}, epoch_state::EpochState, + ledger_info::LedgerInfoWithSignatures, validator_signer::ValidatorSigner, }; -use futures::{ - future::{AbortHandle, Abortable}, - StreamExt, -}; +use futures::StreamExt; use futures_channel::oneshot; use move_core_types::account_address::AccountAddress; use std::{sync::Arc, time::Duration}; @@ -78,12 +79,11 @@ pub struct ConsensusObserver { // The execution client to the buffer manager execution_client: Arc, - // The sender to notify the observer that state syncing to the (epoch, round) has completed - sync_notification_sender: UnboundedSender<(u64, Round)>, + // The observer fallback manager + observer_fallback_manager: ObserverFallbackManager, - // If the sync handle is set it indicates that we're in state sync mode. - // The flag indicates if we're waiting to transition to a new epoch. - sync_handle: Option<(DropGuard, bool)>, + // The state sync manager + state_sync_manager: StateSyncManager, // The consensus observer subscription manager subscription_manager: SubscriptionManager, @@ -97,7 +97,7 @@ impl ConsensusObserver { >, db_reader: Arc, execution_client: Arc, - sync_notification_sender: UnboundedSender<(u64, Round)>, + state_sync_notification_sender: UnboundedSender, reconfig_events: Option>, consensus_publisher: Option>, time_service: TimeService, @@ -105,6 +105,20 @@ impl ConsensusObserver { // Get the consensus observer config let consensus_observer_config = node_config.consensus_observer; + // Create the observer fallback manager + let observer_fallback_manager = ObserverFallbackManager::new( + consensus_observer_config, + db_reader.clone(), + time_service.clone(), + ); + + // Create the state sync manager + let state_sync_manager = StateSyncManager::new( + consensus_observer_config, + execution_client.clone(), + state_sync_notification_sender, + ); + // Create the subscription manager let subscription_manager = SubscriptionManager::new( consensus_observer_client, @@ -132,8 +146,8 @@ impl ConsensusObserver { block_payload_store: Arc::new(Mutex::new(block_payload_store)), pending_block_store: Arc::new(Mutex::new(pending_block_store)), execution_client, - sync_notification_sender, - sync_handle: None, + observer_fallback_manager, + state_sync_manager, subscription_manager, } } @@ -154,17 +168,37 @@ impl ConsensusObserver { debug!(LogSchema::new(LogEntry::ConsensusObserver) .message("Checking consensus observer progress!")); - // If we're in state sync mode, we should wait for state sync to complete - if self.in_state_sync_mode() { + // If we've fallen back to state sync, we should wait for it to complete + if self.state_sync_manager.in_fallback_mode() { + info!(LogSchema::new(LogEntry::ConsensusObserver) + .message("Waiting for state sync to complete fallback syncing!",)); + return; + } + + // If state sync is syncing to a commit decision, we should wait for it to complete + if self.state_sync_manager.is_syncing_to_commit() { info!( LogSchema::new(LogEntry::ConsensusObserver).message(&format!( - "Waiting for state sync to reach target: {:?}!", + "Waiting for state sync to reach commit decision: {:?}!", self.active_observer_state.root().commit_info() )) ); return; } + // Check if we need to fallback to state sync + if let Err(error) = self.observer_fallback_manager.check_syncing_progress() { + // Log the error and enter fallback mode + warn!( + LogSchema::new(LogEntry::ConsensusObserver).message(&format!( + "Failed to make syncing progress! Entering fallback mode! Error: {:?}", + error + )) + ); + self.enter_fallback_mode().await; + return; + } + // Otherwise, check the health of the active subscriptions if let Err(error) = self .subscription_manager @@ -205,6 +239,18 @@ impl ConsensusObserver { metrics::increment_counter_without_labels(&metrics::OBSERVER_CLEARED_BLOCK_STATE); } + /// Enters fallback mode for consensus observer by invoking state sync + async fn enter_fallback_mode(&mut self) { + // Terminate all active subscriptions (to ensure we don't process any more messages) + self.subscription_manager.terminate_all_subscriptions(); + + // Clear all the pending block state + self.clear_pending_block_state().await; + + // Start syncing for the fallback + self.state_sync_manager.sync_for_fallback(); + } + /// Finalizes the ordered block by sending it to the execution pipeline async fn finalize_ordered_block(&mut self, ordered_block: OrderedBlock) { info!( @@ -295,16 +341,6 @@ impl ConsensusObserver { } } - /// Returns true iff we are waiting for state sync to complete an epoch change - fn in_state_sync_epoch_change(&self) -> bool { - matches!(self.sync_handle, Some((_, true))) - } - - /// Returns true iff we are waiting for state sync to complete - fn in_state_sync_mode(&self) -> bool { - self.sync_handle.is_some() - } - /// Orders any ready pending blocks for the given epoch and round async fn order_ready_pending_block(&mut self, block_epoch: u64, block_round: Round) { // Get any ready ordered block @@ -445,7 +481,7 @@ impl ConsensusObserver { if epoch_changed || commit_round > last_block.round() { // If we're waiting for state sync to transition into a new epoch, // we should just wait and not issue a new state sync request. - if self.in_state_sync_epoch_change() { + if self.state_sync_manager.is_syncing_through_epoch() { info!( LogSchema::new(LogEntry::ConsensusObserver).message(&format!( "Already waiting for state sync to reach new epoch: {:?}. Dropping commit decision: {:?}!", @@ -456,15 +492,8 @@ impl ConsensusObserver { return; } - // Otherwise, we should start the state sync process - info!( - LogSchema::new(LogEntry::ConsensusObserver).message(&format!( - "Started syncing to {}!", - commit_decision.proof_block_info() - )) - ); - - // Update the root and clear the pending blocks (up to the commit) + // Otherwise, we should start the state sync process for the commit. + // Update the root and clear the pending blocks (up to the commit). self.active_observer_state .update_root(commit_decision.commit_proof().clone()); self.block_payload_store @@ -474,15 +503,9 @@ impl ConsensusObserver { .lock() .remove_blocks_for_commit(commit_decision.commit_proof()); - // Start the state sync process - let abort_handle = sync_to_commit_decision( - commit_decision, - commit_epoch, - commit_round, - self.execution_client.clone(), - self.sync_notification_sender.clone(), - ); - self.sync_handle = Some((DropGuard::new(abort_handle), epoch_changed)); + // Start state syncing to the commit decision + self.state_sync_manager + .sync_to_commit(commit_decision, epoch_changed); } } @@ -510,8 +533,8 @@ impl ConsensusObserver { .lock() .update_commit_decision(commit_decision); - // If we are not in sync mode, forward the commit decision to the execution pipeline - if !self.in_state_sync_mode() { + // If state sync is not syncing to a commit, forward the commit decision to the execution pipeline + if !self.state_sync_manager.is_syncing_to_commit() { info!( LogSchema::new(LogEntry::ConsensusObserver).message(&format!( "Forwarding commit decision to the execution pipeline: {}", @@ -684,8 +707,8 @@ impl ConsensusObserver { .lock() .insert_ordered_block(ordered_block.clone()); - // If we're not in sync mode, finalize the ordered blocks - if !self.in_state_sync_mode() { + // If state sync is not syncing to a commit, finalize the ordered blocks + if !self.state_sync_manager.is_syncing_to_commit() { self.finalize_ordered_block(ordered_block).await; } } else { @@ -698,34 +721,121 @@ impl ConsensusObserver { } } - /// Processes the sync complete notification for the given epoch and round - async fn process_sync_notification(&mut self, epoch: u64, round: Round) { - // Log the sync notification + /// Processes the given state sync notification + async fn process_state_sync_notification( + &mut self, + state_sync_notification: StateSyncNotification, + ) { + // Process the state sync notification based on the type + match state_sync_notification { + StateSyncNotification::FallbackSyncCompleted(latest_synced_ledger_info) => { + self.process_fallback_sync_notification(latest_synced_ledger_info) + .await + }, + StateSyncNotification::CommitSyncCompleted(latest_synced_ledger_info) => { + self.process_commit_sync_notification(latest_synced_ledger_info) + .await + }, + } + } + + /// Processes the state sync notification for the fallback sync + async fn process_fallback_sync_notification( + &mut self, + latest_synced_ledger_info: LedgerInfoWithSignatures, + ) { + // Get the epoch and round for the latest synced ledger info + let ledger_info = latest_synced_ledger_info.ledger_info(); + let epoch = ledger_info.epoch(); + let round = ledger_info.round(); + + // Log the state sync notification + info!( + LogSchema::new(LogEntry::ConsensusObserver).message(&format!( + "Received state sync notification for fallback completion! Epoch {}, round: {}!", + epoch, round + )) + ); + + // Verify that there is an active fallback sync + if !self.state_sync_manager.in_fallback_mode() { + // Log the error and return early + error!(LogSchema::new(LogEntry::ConsensusObserver).message( + "Failed to process fallback sync notification! No active fallback sync found!" + )); + return; + } + + // Reset the fallback manager state + self.observer_fallback_manager + .reset_syncing_progress(&latest_synced_ledger_info); + + // Update the root with the latest synced ledger info + self.active_observer_state + .update_root(latest_synced_ledger_info); + + // If the epoch has changed, end the current epoch and start the latest one + let current_epoch_state = self.get_epoch_state(); + if epoch > current_epoch_state.epoch { + // Wait for the latest epoch to start + self.execution_client.end_epoch().await; + self.wait_for_epoch_start().await; + }; + + // Reset the pending block state + self.clear_pending_block_state().await; + + // Reset the state sync manager for the synced fallback + self.state_sync_manager.clear_active_fallback_sync(); + } + + /// Processes the state sync notification for the commit decision + async fn process_commit_sync_notification( + &mut self, + latest_synced_ledger_info: LedgerInfoWithSignatures, + ) { + // Get the epoch and round for the synced commit decision + let ledger_info = latest_synced_ledger_info.ledger_info(); + let epoch = ledger_info.epoch(); + let round = ledger_info.round(); + + // Log the state sync notification info!( LogSchema::new(LogEntry::ConsensusObserver).message(&format!( - "Received sync complete notification for epoch {}, round: {}", + "Received state sync notification for commit completion! Epoch {}, round: {}!", epoch, round )) ); - // Verify that the sync notification is for the current epoch and round + // Verify that there is an active commit sync + if !self.state_sync_manager.is_syncing_to_commit() { + // Log the error and return early + error!(LogSchema::new(LogEntry::ConsensusObserver).message( + "Failed to process commit sync notification! No active commit sync found!" + )); + return; + } + + // Verify that the state sync notification is for the current epoch and round if !self .active_observer_state .check_root_epoch_and_round(epoch, round) { - info!( + // Log the error, reset the state sync manager and return early + error!( LogSchema::new(LogEntry::ConsensusObserver).message(&format!( - "Received invalid sync notification for epoch: {}, round: {}! Current root: {:?}", + "Received invalid commit sync notification for epoch: {}, round: {}! Current root: {:?}", epoch, round, self.active_observer_state.root() )) ); + self.state_sync_manager.clear_active_commit_sync(); return; } - // If the epoch has changed, end the current epoch and start the new one + // If the epoch has changed, end the current epoch and start the latest one let current_epoch_state = self.get_epoch_state(); if epoch > current_epoch_state.epoch { - // Wait for the next epoch to start + // Wait for the latest epoch to start self.execution_client.end_epoch().await; self.wait_for_epoch_start().await; @@ -743,8 +853,8 @@ impl ConsensusObserver { } }; - // Reset and drop the sync handle - self.sync_handle = None; + // Reset the state sync manager for the synced commit decision + self.state_sync_manager.clear_active_commit_sync(); // Process all the newly ordered blocks let all_ordered_blocks = self.ordered_block_store.lock().get_all_ordered_blocks(); @@ -818,7 +928,9 @@ impl ConsensusObserver { mut self, consensus_observer_config: ConsensusObserverConfig, mut consensus_observer_message_receiver: Receiver<(), ConsensusObserverNetworkMessage>, - mut sync_notification_listener: tokio::sync::mpsc::UnboundedReceiver<(u64, Round)>, + mut state_sync_notification_listener: tokio::sync::mpsc::UnboundedReceiver< + StateSyncNotification, + >, ) { // Create a progress check ticker let mut progress_check_interval = IntervalStream::new(interval(Duration::from_millis( @@ -826,7 +938,7 @@ impl ConsensusObserver { ))) .fuse(); - // Wait for the epoch to start + // Wait for the latest epoch to start self.wait_for_epoch_start().await; // Start the consensus observer loop @@ -837,8 +949,8 @@ impl ConsensusObserver { Some(network_message) = consensus_observer_message_receiver.next() => { self.process_network_message(network_message).await; } - Some((epoch, round)) = sync_notification_listener.recv() => { - self.process_sync_notification(epoch, round).await; + Some(state_sync_notification) = state_sync_notification_listener.recv() => { + self.process_state_sync_notification(state_sync_notification).await; }, _ = progress_check_interval.select_next_some() => { self.check_progress().await; @@ -866,47 +978,6 @@ fn log_received_message(message: String) { } } -/// Spawns a task to sync to the given commit decision and notifies -/// the consensus observer. Also, returns an abort handle to cancel the task. -fn sync_to_commit_decision( - commit_decision: CommitDecision, - decision_epoch: u64, - decision_round: Round, - execution_client: Arc, - sync_notification_sender: UnboundedSender<(u64, Round)>, -) -> AbortHandle { - let (abort_handle, abort_registration) = AbortHandle::new_pair(); - tokio::spawn(Abortable::new( - async move { - // Sync to the commit decision - if let Err(error) = execution_client - .clone() - .sync_to(commit_decision.commit_proof().clone()) - .await - { - warn!( - LogSchema::new(LogEntry::ConsensusObserver).message(&format!( - "Failed to sync to commit decision: {:?}! Error: {:?}", - commit_decision, error - )) - ); - } - - // Notify the consensus observer that the sync is complete - if let Err(error) = sync_notification_sender.send((decision_epoch, decision_round)) { - error!( - LogSchema::new(LogEntry::ConsensusObserver).message(&format!( - "Failed to send sync notification for decision epoch: {:?}, round: {:?}! Error: {:?}", - decision_epoch, decision_round, error - )) - ); - } - }, - abort_registration, - )); - abort_handle -} - /// Updates the metrics for the received block payload message fn update_metrics_for_block_payload_message( peer_network_id: PeerNetworkId, diff --git a/consensus/src/consensus_observer/observer/fallback_manager.rs b/consensus/src/consensus_observer/observer/fallback_manager.rs new file mode 100644 index 0000000000000..2b7bee64b424e --- /dev/null +++ b/consensus/src/consensus_observer/observer/fallback_manager.rs @@ -0,0 +1,241 @@ +// Copyright © Aptos Foundation +// SPDX-License-Identifier: Apache-2.0 + +use crate::consensus_observer::common::error::Error; +use aptos_config::config::ConsensusObserverConfig; +use aptos_storage_interface::DbReader; +use aptos_time_service::{TimeService, TimeServiceTrait}; +use aptos_types::ledger_info::LedgerInfoWithSignatures; +use std::{ + sync::Arc, + time::{Duration, Instant}, +}; + +/// The manager for fallback mode in consensus observer +pub struct ObserverFallbackManager { + // The configuration of the consensus observer + consensus_observer_config: ConsensusObserverConfig, + + // A handle to storage (used to read the latest state and check progress) + db_reader: Arc, + + // The highest synced version we've seen from storage, along with the time at which it was seen + highest_synced_version_and_time: (u64, Instant), + + // The time service (used to check the storage update time) + time_service: TimeService, +} + +impl ObserverFallbackManager { + pub fn new( + consensus_observer_config: ConsensusObserverConfig, + db_reader: Arc, + time_service: TimeService, + ) -> Self { + // Get the current time + let time_now = time_service.now(); + + // Create a new fallback manager + Self { + consensus_observer_config, + db_reader, + highest_synced_version_and_time: (0, time_now), + time_service, + } + } + + /// Verifies that the DB is continuing to sync and commit new data. + /// If not, an error is returned, indicating that we should enter fallback mode. + pub fn check_syncing_progress(&mut self) -> Result<(), Error> { + // Get the current time and synced version from storage + let time_now = self.time_service.now(); + let current_synced_version = + self.db_reader + .get_latest_ledger_info_version() + .map_err(|error| { + Error::UnexpectedError(format!( + "Failed to read highest synced version: {:?}", + error + )) + })?; + + // Verify that the synced version is increasing appropriately + let (highest_synced_version, highest_version_timestamp) = + self.highest_synced_version_and_time; + if current_synced_version <= highest_synced_version { + // The synced version hasn't increased. Check if we should enter fallback mode. + let duration_since_highest_seen = time_now.duration_since(highest_version_timestamp); + let fallback_threshold = Duration::from_millis( + self.consensus_observer_config + .observer_fallback_sync_threshold_ms, + ); + if duration_since_highest_seen > fallback_threshold { + return Err(Error::ObserverProgressStopped(format!( + "Consensus observer is not making progress! Highest synced version: {}, elapsed: {:?}", + highest_synced_version, duration_since_highest_seen + ))); + } + return Ok(()); // We haven't passed the fallback threshold yet + } + + // Update the highest synced version and time + self.highest_synced_version_and_time = (current_synced_version, time_now); + + Ok(()) + } + + /// Resets the syncing progress to the latest synced ledger info and current time + pub fn reset_syncing_progress(&mut self, latest_synced_ledger_info: &LedgerInfoWithSignatures) { + // Get the current time and highest synced version + let time_now = self.time_service.now(); + let highest_synced_version = latest_synced_ledger_info.ledger_info().version(); + + // Update the highest synced version and time + self.highest_synced_version_and_time = (highest_synced_version, time_now); + } +} + +#[cfg(test)] +mod test { + use super::*; + use aptos_crypto::HashValue; + use aptos_storage_interface::Result; + use aptos_types::{ + aggregate_signature::AggregateSignature, block_info::BlockInfo, ledger_info::LedgerInfo, + transaction::Version, + }; + use claims::assert_matches; + use mockall::mock; + + // This is a simple mock of the DbReader (it generates a MockDatabaseReader) + mock! { + pub DatabaseReader {} + impl DbReader for DatabaseReader { + fn get_latest_ledger_info_version(&self) -> Result; + } + } + + #[test] + fn test_check_syncing_progress() { + // Create a consensus observer config + let observer_fallback_sync_threshold_ms = 10_000; + let consensus_observer_config = ConsensusObserverConfig { + observer_fallback_sync_threshold_ms, + ..ConsensusObserverConfig::default() + }; + + // Create a mock DB reader with expectations + let first_synced_version = 1; + let second_synced_version = 2; + let mut mock_db_reader = MockDatabaseReader::new(); + mock_db_reader + .expect_get_latest_ledger_info_version() + .returning(move || Ok(first_synced_version)) + .times(1); // Only allow one call for the first version + mock_db_reader + .expect_get_latest_ledger_info_version() + .returning(move || Ok(second_synced_version)); // Allow multiple calls for the second version + + // Create a new fallback manager + let time_service = TimeService::mock(); + let mut fallback_manager = ObserverFallbackManager::new( + consensus_observer_config, + Arc::new(mock_db_reader), + time_service.clone(), + ); + + // Verify that the DB is making sync progress and that the highest synced version is updated + let mock_time_service = time_service.into_mock(); + assert!(fallback_manager.check_syncing_progress().is_ok()); + assert_eq!( + fallback_manager.highest_synced_version_and_time, + (first_synced_version, mock_time_service.now()) + ); + + // Elapse enough time to bypass the fallback threshold + mock_time_service.advance(Duration::from_millis( + observer_fallback_sync_threshold_ms + 1, + )); + + // Verify that the DB is still making sync progress (the next DB version is higher) + let time_now = mock_time_service.now(); + assert!(fallback_manager.check_syncing_progress().is_ok()); + assert_eq!( + fallback_manager.highest_synced_version_and_time, + (second_synced_version, time_now) + ); + + // Elapse some amount of time (but not enough to bypass the fallback threshold) + mock_time_service.advance(Duration::from_millis( + observer_fallback_sync_threshold_ms - 1, + )); + + // Verify that the DB is still making sync progress (the threshold hasn't been reached) + assert!(fallback_manager.check_syncing_progress().is_ok()); + assert_eq!( + fallback_manager.highest_synced_version_and_time, + (second_synced_version, time_now) + ); + + // Elapse enough time to bypass the fallback threshold + mock_time_service.advance(Duration::from_millis( + observer_fallback_sync_threshold_ms + 1, + )); + + // Verify that the DB is not making sync progress and that fallback mode should be entered + assert_matches!( + fallback_manager.check_syncing_progress(), + Err(Error::ObserverProgressStopped(_)) + ); + assert_eq!( + fallback_manager.highest_synced_version_and_time, + (second_synced_version, time_now) + ); + } + + #[test] + fn test_reset_syncing_progress() { + // Create a new fallback manager + let consensus_observer_config = ConsensusObserverConfig::default(); + let mock_db_reader = MockDatabaseReader::new(); + let time_service = TimeService::mock(); + let mut fallback_manager = ObserverFallbackManager::new( + consensus_observer_config, + Arc::new(mock_db_reader), + time_service.clone(), + ); + + // Verify the initial state of the highest synced version and time + let mock_time_service = time_service.into_mock(); + assert_eq!( + fallback_manager.highest_synced_version_and_time, + (0, mock_time_service.now()) + ); + + // Elapse some amount of time + mock_time_service.advance(Duration::from_secs(10)); + + // Reset the syncing progress to a new synced ledger info + let block_version = 100; + let block_info = BlockInfo::new( + 0, + 0, + HashValue::zero(), + HashValue::zero(), + block_version, + 0, + None, + ); + let latest_synced_ledger_info = LedgerInfoWithSignatures::new( + LedgerInfo::new(block_info, HashValue::zero()), + AggregateSignature::empty(), + ); + fallback_manager.reset_syncing_progress(&latest_synced_ledger_info); + + // Verify that the highest synced version and time have been updated + assert_eq!( + fallback_manager.highest_synced_version_and_time, + (block_version, mock_time_service.now()) + ); + } +} diff --git a/consensus/src/consensus_observer/observer/mod.rs b/consensus/src/consensus_observer/observer/mod.rs index 4a4e5d42881a3..757b9dc4b1e18 100644 --- a/consensus/src/consensus_observer/observer/mod.rs +++ b/consensus/src/consensus_observer/observer/mod.rs @@ -3,9 +3,11 @@ pub mod active_state; pub mod consensus_observer; +pub mod fallback_manager; pub mod ordered_blocks; pub mod payload_store; pub mod pending_blocks; +pub mod state_sync_manager; pub mod subscription; pub mod subscription_manager; pub mod subscription_utils; diff --git a/consensus/src/consensus_observer/observer/ordered_blocks.rs b/consensus/src/consensus_observer/observer/ordered_blocks.rs index a2408b3a4b20d..c0bbd01f9ca7a 100644 --- a/consensus/src/consensus_observer/observer/ordered_blocks.rs +++ b/consensus/src/consensus_observer/observer/ordered_blocks.rs @@ -339,8 +339,15 @@ mod test { #[test] fn test_get_last_ordered_block() { + // Create a new consensus observer config + let max_num_pending_blocks = 100; + let consensus_observer_config = ConsensusObserverConfig { + max_num_pending_blocks, + ..ConsensusObserverConfig::default() + }; + // Create a new ordered block store - let mut ordered_block_store = OrderedBlockStore::new(ConsensusObserverConfig::default()); + let mut ordered_block_store = OrderedBlockStore::new(consensus_observer_config); // Verify that we have no last ordered block assert!(ordered_block_store.get_last_ordered_block().is_none()); @@ -379,8 +386,15 @@ mod test { #[test] fn test_get_ordered_block() { + // Create a new consensus observer config + let max_num_pending_blocks = 100; + let consensus_observer_config = ConsensusObserverConfig { + max_num_pending_blocks, + ..ConsensusObserverConfig::default() + }; + // Create a new ordered block store - let mut ordered_block_store = OrderedBlockStore::new(ConsensusObserverConfig::default()); + let mut ordered_block_store = OrderedBlockStore::new(consensus_observer_config); // Insert several ordered blocks for the current epoch let current_epoch = 0; @@ -456,8 +470,15 @@ mod test { #[test] fn test_remove_blocks_for_commit() { + // Create a new consensus observer config + let max_num_pending_blocks = 100; + let consensus_observer_config = ConsensusObserverConfig { + max_num_pending_blocks, + ..ConsensusObserverConfig::default() + }; + // Create a new ordered block store - let mut ordered_block_store = OrderedBlockStore::new(ConsensusObserverConfig::default()); + let mut ordered_block_store = OrderedBlockStore::new(consensus_observer_config); // Insert several ordered blocks for the current epoch let current_epoch = 10; @@ -553,8 +574,15 @@ mod test { #[test] fn test_update_commit_decision() { + // Create a new consensus observer config + let max_num_pending_blocks = 100; + let consensus_observer_config = ConsensusObserverConfig { + max_num_pending_blocks, + ..ConsensusObserverConfig::default() + }; + // Create a new ordered block store - let mut ordered_block_store = OrderedBlockStore::new(ConsensusObserverConfig::default()); + let mut ordered_block_store = OrderedBlockStore::new(consensus_observer_config); // Insert several ordered blocks for the current epoch let current_epoch = 0; diff --git a/consensus/src/consensus_observer/observer/payload_store.rs b/consensus/src/consensus_observer/observer/payload_store.rs index c75ea87fa4322..fc93dd0d7cdae 100644 --- a/consensus/src/consensus_observer/observer/payload_store.rs +++ b/consensus/src/consensus_observer/observer/payload_store.rs @@ -379,8 +379,14 @@ mod test { #[test] fn test_clear_all_payloads() { + // Create a new consensus observer config + let max_num_pending_blocks = 100; + let consensus_observer_config = ConsensusObserverConfig { + max_num_pending_blocks, + ..ConsensusObserverConfig::default() + }; + // Create a new block payload store - let consensus_observer_config = ConsensusObserverConfig::default(); let mut block_payload_store = BlockPayloadStore::new(consensus_observer_config); // Add some unverified blocks to the payload store @@ -446,8 +452,14 @@ mod test { #[test] fn test_insert_block_payload() { + // Create a new consensus observer config + let max_num_pending_blocks = 100; + let consensus_observer_config = ConsensusObserverConfig { + max_num_pending_blocks, + ..ConsensusObserverConfig::default() + }; + // Create a new block payload store - let consensus_observer_config = ConsensusObserverConfig::default(); let mut block_payload_store = BlockPayloadStore::new(consensus_observer_config); // Add some verified blocks to the payload store @@ -556,8 +568,14 @@ mod test { #[test] fn test_remove_blocks_for_epoch_round_verified() { + // Create a new consensus observer config + let max_num_pending_blocks = 100; + let consensus_observer_config = ConsensusObserverConfig { + max_num_pending_blocks, + ..ConsensusObserverConfig::default() + }; + // Create a new block payload store - let consensus_observer_config = ConsensusObserverConfig::default(); let mut block_payload_store = BlockPayloadStore::new(consensus_observer_config); // Add some verified blocks to the payload store for the current epoch @@ -614,8 +632,14 @@ mod test { #[test] fn test_remove_blocks_for_epoch_round_unverified() { + // Create a new consensus observer config + let max_num_pending_blocks = 100; + let consensus_observer_config = ConsensusObserverConfig { + max_num_pending_blocks, + ..ConsensusObserverConfig::default() + }; + // Create a new block payload store - let consensus_observer_config = ConsensusObserverConfig::default(); let mut block_payload_store = BlockPayloadStore::new(consensus_observer_config); // Add some unverified blocks to the payload store for the current epoch @@ -671,8 +695,14 @@ mod test { #[test] fn test_remove_committed_blocks_verified() { + // Create a new consensus observer config + let max_num_pending_blocks = 100; + let consensus_observer_config = ConsensusObserverConfig { + max_num_pending_blocks, + ..ConsensusObserverConfig::default() + }; + // Create a new block payload store - let consensus_observer_config = ConsensusObserverConfig::default(); let mut block_payload_store = BlockPayloadStore::new(consensus_observer_config); // Add some blocks to the payload store for the current epoch @@ -740,8 +770,14 @@ mod test { #[test] fn test_remove_committed_blocks_unverified() { + // Create a new consensus observer config + let max_num_pending_blocks = 100; + let consensus_observer_config = ConsensusObserverConfig { + max_num_pending_blocks, + ..ConsensusObserverConfig::default() + }; + // Create a new block payload store - let consensus_observer_config = ConsensusObserverConfig::default(); let mut block_payload_store = BlockPayloadStore::new(consensus_observer_config); // Add some blocks to the payload store for the current epoch @@ -808,8 +844,14 @@ mod test { #[test] fn test_verify_payload_signatures() { + // Create a new consensus observer config + let max_num_pending_blocks = 100; + let consensus_observer_config = ConsensusObserverConfig { + max_num_pending_blocks, + ..ConsensusObserverConfig::default() + }; + // Create a new block payload store - let consensus_observer_config = ConsensusObserverConfig::default(); let mut block_payload_store = BlockPayloadStore::new(consensus_observer_config); // Add some verified blocks for the current epoch @@ -940,8 +982,14 @@ mod test { #[test] fn test_verify_payload_signatures_failure() { + // Create a new consensus observer config + let max_num_pending_blocks = 100; + let consensus_observer_config = ConsensusObserverConfig { + max_num_pending_blocks, + ..ConsensusObserverConfig::default() + }; + // Create a new block payload store - let consensus_observer_config = ConsensusObserverConfig::default(); let mut block_payload_store = BlockPayloadStore::new(consensus_observer_config); // Add some verified blocks for the current epoch diff --git a/consensus/src/consensus_observer/observer/state_sync_manager.rs b/consensus/src/consensus_observer/observer/state_sync_manager.rs new file mode 100644 index 0000000000000..45760cddee788 --- /dev/null +++ b/consensus/src/consensus_observer/observer/state_sync_manager.rs @@ -0,0 +1,346 @@ +// Copyright © Aptos Foundation +// SPDX-License-Identifier: Apache-2.0 + +use crate::{ + consensus_observer::{ + common::{ + logging::{LogEntry, LogSchema}, + metrics, + }, + network::observer_message::CommitDecision, + }, + pipeline::execution_client::TExecutionClient, +}; +use aptos_config::config::ConsensusObserverConfig; +use aptos_logger::{error, info}; +use aptos_reliable_broadcast::DropGuard; +use aptos_types::ledger_info::LedgerInfoWithSignatures; +use futures::future::{AbortHandle, Abortable}; +use std::{sync::Arc, time::Duration}; +use tokio::sync::mpsc::UnboundedSender; + +/// A simple state sync notification to notify consensus +/// observer that state syncing has completed. +pub enum StateSyncNotification { + FallbackSyncCompleted(LedgerInfoWithSignatures), + CommitSyncCompleted(LedgerInfoWithSignatures), +} + +impl StateSyncNotification { + /// Returns a new state sync notification that fallback syncing has completed + pub fn fallback_sync_completed(ledger_info: LedgerInfoWithSignatures) -> Self { + Self::FallbackSyncCompleted(ledger_info) + } + + /// Returns a new state sync notification that syncing to a commit has completed + pub fn commit_sync_completed(ledger_info: LedgerInfoWithSignatures) -> Self { + Self::CommitSyncCompleted(ledger_info) + } +} + +/// The manager for interacting with state sync +pub struct StateSyncManager { + // The configuration of the consensus observer + consensus_observer_config: ConsensusObserverConfig, + + // The execution client to the buffer manager + execution_client: Arc, + + // The sender to notify consensus observer that state syncing to + // the specified ledger info has completed. + state_sync_notification_sender: UnboundedSender, + + // The active fallback sync handle. If this is set, it means that + // we've fallen back to state sync, and we should wait for it to complete. + fallback_sync_handle: Option, + + // The active sync to commit handle. If this is set, it means that + // we're waiting for state sync to synchronize to a known commit decision. + // The flag indicates if the commit will transition us to a new epoch. + sync_to_commit_handle: Option<(DropGuard, bool)>, +} + +impl StateSyncManager { + pub fn new( + consensus_observer_config: ConsensusObserverConfig, + execution_client: Arc, + state_sync_notification_sender: UnboundedSender, + ) -> Self { + Self { + consensus_observer_config, + execution_client, + state_sync_notification_sender, + fallback_sync_handle: None, + sync_to_commit_handle: None, + } + } + + /// Resets the handle for any active sync to a commit decision + pub fn clear_active_commit_sync(&mut self) { + // If we're not actively syncing to a commit, log an error + if !self.is_syncing_to_commit() { + error!(LogSchema::new(LogEntry::ConsensusObserver) + .message("Failed to clear sync to commit decision! No active sync handle found!")); + } + + self.sync_to_commit_handle = None; + } + + /// Resets the handle for any active fallback sync + pub fn clear_active_fallback_sync(&mut self) { + // If we're not actively syncing in fallback mode, log an error + if !self.in_fallback_mode() { + error!(LogSchema::new(LogEntry::ConsensusObserver) + .message("Failed to clear fallback sync! No active sync handle found!")); + } + + self.fallback_sync_handle = None; + } + + /// Returns true iff state sync is currently executing in fallback mode + pub fn in_fallback_mode(&self) -> bool { + self.fallback_sync_handle.is_some() + } + + /// Returns true iff we are waiting for state sync to synchronize + /// to a commit decision that will transition us to a new epoch + pub fn is_syncing_through_epoch(&self) -> bool { + matches!(self.sync_to_commit_handle, Some((_, true))) + } + + /// Returns true iff state sync is currently syncing to a commit decision + pub fn is_syncing_to_commit(&self) -> bool { + self.sync_to_commit_handle.is_some() + } + + /// Invokes state sync to synchronize in fallback mode + pub fn sync_for_fallback(&mut self) { + // Log that we're starting to sync in fallback mode + info!( + LogSchema::new(LogEntry::ConsensusObserver).message(&format!( + "Started syncing in fallback mode! Syncing duration: {:?} ms!", + self.consensus_observer_config.observer_fallback_duration_ms + )) + ); + + // Clone the required components for the state sync task + let consensus_observer_config = self.consensus_observer_config; + let execution_client = self.execution_client.clone(); + let sync_notification_sender = self.state_sync_notification_sender.clone(); + + // Spawn a task to sync for the fallback + let (abort_handle, abort_registration) = AbortHandle::new_pair(); + tokio::spawn(Abortable::new( + async move { + // Update the state sync metrics now that we're syncing for the fallback + metrics::set_gauge_with_label( + &metrics::OBSERVER_STATE_SYNC_EXECUTING, + metrics::STATE_SYNCING_FOR_FALLBACK, + 1, // We're syncing for the fallback + ); + + // Get the fallback duration + let fallback_duration = + Duration::from_millis(consensus_observer_config.observer_fallback_duration_ms); + + // Sync for the fallback duration + let latest_synced_ledger_info = match execution_client + .clone() + .sync_for_duration(fallback_duration) + .await + { + Ok(latest_synced_ledger_info) => latest_synced_ledger_info, + Err(error) => { + error!(LogSchema::new(LogEntry::ConsensusObserver) + .message(&format!("Failed to sync for fallback! Error: {:?}", error))); + return; + }, + }; + + // Notify consensus observer that we've synced for the fallback + let state_sync_notification = + StateSyncNotification::fallback_sync_completed(latest_synced_ledger_info); + if let Err(error) = sync_notification_sender.send(state_sync_notification) { + error!( + LogSchema::new(LogEntry::ConsensusObserver).message(&format!( + "Failed to send state sync notification for fallback! Error: {:?}", + error + )) + ); + } + + // Clear the state sync metrics now that we're done syncing + metrics::set_gauge_with_label( + &metrics::OBSERVER_STATE_SYNC_EXECUTING, + metrics::STATE_SYNCING_FOR_FALLBACK, + 0, // We're no longer syncing for the fallback + ); + }, + abort_registration, + )); + + // Save the sync task handle + self.fallback_sync_handle = Some(DropGuard::new(abort_handle)); + } + + /// Invokes state sync to synchronize to a new commit decision + pub fn sync_to_commit(&mut self, commit_decision: CommitDecision, epoch_changed: bool) { + // Log that we're starting to sync to the commit decision + info!( + LogSchema::new(LogEntry::ConsensusObserver).message(&format!( + "Started syncing to commit: {}!", + commit_decision.proof_block_info() + )) + ); + + // Get the commit decision epoch and round + let commit_epoch = commit_decision.epoch(); + let commit_round = commit_decision.round(); + + // Clone the required components for the state sync task + let execution_client = self.execution_client.clone(); + let sync_notification_sender = self.state_sync_notification_sender.clone(); + + // Spawn a task to sync to the commit decision + let (abort_handle, abort_registration) = AbortHandle::new_pair(); + tokio::spawn(Abortable::new( + async move { + // Update the state sync metrics now that we're syncing to a commit + metrics::set_gauge_with_label( + &metrics::OBSERVER_STATE_SYNC_EXECUTING, + metrics::STATE_SYNCING_TO_COMMIT, + 1, // We're syncing to a commit decision + ); + + // Sync to the commit decision + if let Err(error) = execution_client + .clone() + .sync_to_target(commit_decision.commit_proof().clone()) + .await + { + error!( + LogSchema::new(LogEntry::ConsensusObserver).message(&format!( + "Failed to sync to commit decision: {:?}! Error: {:?}", + commit_decision, error + )) + ); + return; + } + + // Notify consensus observer that we've synced to the commit decision + let state_sync_notification = StateSyncNotification::commit_sync_completed( + commit_decision.commit_proof().clone(), + ); + if let Err(error) = sync_notification_sender.send(state_sync_notification) { + error!( + LogSchema::new(LogEntry::ConsensusObserver).message(&format!( + "Failed to send state sync notification for commit decision epoch: {:?}, round: {:?}! Error: {:?}", + commit_epoch, commit_round, error + )) + ); + } + + // Clear the state sync metrics now that we're done syncing + metrics::set_gauge_with_label( + &metrics::OBSERVER_STATE_SYNC_EXECUTING, + metrics::STATE_SYNCING_TO_COMMIT, + 0, // We're no longer syncing to a commit decision + ); + }, + abort_registration, + )); + + // Save the sync task handle + self.sync_to_commit_handle = Some((DropGuard::new(abort_handle), epoch_changed)); + } +} + +#[cfg(test)] +mod test { + use super::*; + use crate::pipeline::execution_client::DummyExecutionClient; + use aptos_types::{aggregate_signature::AggregateSignature, ledger_info::LedgerInfo}; + + #[tokio::test] + async fn test_clear_active_sync() { + // Create a new state sync manager + let consensus_observer_config = ConsensusObserverConfig::default(); + let (state_sync_notification_sender, _) = tokio::sync::mpsc::unbounded_channel(); + let mut state_sync_manager = StateSyncManager::new( + consensus_observer_config, + Arc::new(DummyExecutionClient), + state_sync_notification_sender, + ); + + // Verify that there are no active sync handles + assert!(!state_sync_manager.in_fallback_mode()); + assert!(!state_sync_manager.is_syncing_to_commit()); + + // Sync to a commit and verify that the active sync handle is set + let commit_decision = CommitDecision::new(LedgerInfoWithSignatures::new( + LedgerInfo::dummy(), + AggregateSignature::empty(), + )); + state_sync_manager.sync_to_commit(commit_decision, false); + assert!(state_sync_manager.is_syncing_to_commit()); + + // Clear the active sync handle and verify that it's reset + state_sync_manager.clear_active_commit_sync(); + assert!(!state_sync_manager.is_syncing_to_commit()); + + // Sync for the fallback and verify that the active sync handle is set + state_sync_manager.sync_for_fallback(); + assert!(state_sync_manager.in_fallback_mode()); + + // Clear the active sync handle and verify that it's reset + state_sync_manager.clear_active_fallback_sync(); + assert!(!state_sync_manager.in_fallback_mode()); + } + + #[tokio::test] + async fn test_is_syncing_through_epoch() { + // Create a new state sync manager + let consensus_observer_config = ConsensusObserverConfig::default(); + let (state_sync_notification_sender, _) = tokio::sync::mpsc::unbounded_channel(); + let mut state_sync_manager = StateSyncManager::new( + consensus_observer_config, + Arc::new(DummyExecutionClient), + state_sync_notification_sender, + ); + + // Verify that we're not syncing through an epoch + assert!(!state_sync_manager.is_syncing_through_epoch()); + + // Sync to a commit that doesn't transition us to a new epoch + let commit_decision = CommitDecision::new(LedgerInfoWithSignatures::new( + LedgerInfo::dummy(), + AggregateSignature::empty(), + )); + state_sync_manager.sync_to_commit(commit_decision, false); + + // Verify that we're not syncing through an epoch + assert!(!state_sync_manager.is_syncing_through_epoch()); + + // Clear the active sync handle and verify that it's reset + state_sync_manager.clear_active_commit_sync(); + assert!(!state_sync_manager.is_syncing_through_epoch()); + + // Sync to a commit that transitions us to a new epoch + let commit_decision = CommitDecision::new(LedgerInfoWithSignatures::new( + LedgerInfo::dummy(), + AggregateSignature::empty(), + )); + state_sync_manager.sync_to_commit(commit_decision, true); + + // Verify that we're syncing through an epoch + assert!(state_sync_manager.is_syncing_through_epoch()); + + // Clear the active sync handle and verify that it's reset + state_sync_manager.clear_active_commit_sync(); + assert!(!state_sync_manager.is_syncing_through_epoch()); + + // Sync for the fallback and verify that we're not syncing through an epoch + state_sync_manager.sync_for_fallback(); + assert!(!state_sync_manager.is_syncing_through_epoch()); + } +} diff --git a/consensus/src/consensus_observer/observer/subscription.rs b/consensus/src/consensus_observer/observer/subscription.rs index 5d9ae4d43def1..2ff909c4eaf2d 100644 --- a/consensus/src/consensus_observer/observer/subscription.rs +++ b/consensus/src/consensus_observer/observer/subscription.rs @@ -168,7 +168,8 @@ impl ConsensusObserverSubscription { /// Verifies that the DB is continuing to sync and commit new data fn check_syncing_progress(&mut self) -> Result<(), Error> { - // Get the current synced version from storage + // Get the current time and synced version from storage + let time_now = self.time_service.now(); let current_synced_version = self.db_reader .get_latest_ledger_info_version() @@ -185,22 +186,22 @@ impl ConsensusObserverSubscription { if current_synced_version <= highest_synced_version { // The synced version hasn't increased. Check if we should terminate // the subscription based on the last time the highest synced version was seen. - let time_now = self.time_service.now(); let duration_since_highest_seen = time_now.duration_since(highest_version_timestamp); - if duration_since_highest_seen - > Duration::from_millis( - self.consensus_observer_config.max_synced_version_timeout_ms, - ) - { + let timeout_duration = Duration::from_millis( + self.consensus_observer_config + .max_subscription_sync_timeout_ms, + ); + if duration_since_highest_seen > timeout_duration { return Err(Error::SubscriptionProgressStopped(format!( "The DB is not making sync progress! Highest synced version: {}, elapsed: {:?}", highest_synced_version, duration_since_highest_seen ))); } + return Ok(()); // We haven't timed out yet } // Update the highest synced version and time - self.highest_synced_version_and_time = (current_synced_version, self.time_service.now()); + self.highest_synced_version_and_time = (current_synced_version, time_now); Ok(()) } @@ -244,7 +245,7 @@ mod test { fn test_check_subscription_health_connected_and_timeout() { // Create a consensus observer config let consensus_observer_config = ConsensusObserverConfig { - max_synced_version_timeout_ms: 100_000_000, // Use a large value so that we don't get DB progress errors + max_subscription_sync_timeout_ms: 100_000_000, // Use a large value so that we don't get DB progress errors ..ConsensusObserverConfig::default() }; @@ -321,7 +322,7 @@ mod test { // Elapse enough time to timeout the subscription mock_time_service.advance(Duration::from_millis( - consensus_observer_config.max_synced_version_timeout_ms + 1, + consensus_observer_config.max_subscription_sync_timeout_ms + 1, )); // Verify that the DB is still making sync progress (the next version is higher) @@ -333,7 +334,7 @@ mod test { // Elapse enough time to timeout the subscription mock_time_service.advance(Duration::from_millis( - consensus_observer_config.max_synced_version_timeout_ms + 1, + consensus_observer_config.max_subscription_sync_timeout_ms + 1, )); // Verify that the DB is not making sync progress and that the subscription has timed out @@ -349,7 +350,7 @@ mod test { let consensus_observer_config = ConsensusObserverConfig { max_concurrent_subscriptions: 1, max_subscription_timeout_ms: 100_000_000, // Use a large value so that we don't time out - max_synced_version_timeout_ms: 100_000_000, // Use a large value so that we don't get DB progress errors + max_subscription_sync_timeout_ms: 100_000_000, // Use a large value so that we don't get DB progress errors ..ConsensusObserverConfig::default() }; @@ -715,39 +716,37 @@ mod test { // Verify that the DB is making sync progress and that the highest synced version is updated let mock_time_service = time_service.into_mock(); - verify_subscription_syncing_progress( - &mut subscription, - first_synced_version, - mock_time_service.now(), - ); + let time_now = mock_time_service.now(); + verify_subscription_syncing_progress(&mut subscription, first_synced_version, time_now); // Elapse some amount of time (not enough to timeout) mock_time_service.advance(Duration::from_millis( - consensus_observer_config.max_synced_version_timeout_ms / 2, + consensus_observer_config.max_subscription_sync_timeout_ms / 2, )); - // Verify that the DB is still making sync progress - verify_subscription_syncing_progress( - &mut subscription, - first_synced_version, - mock_time_service.now(), - ); + // Verify that the DB is still making sync progress (we haven't timed out yet) + verify_subscription_syncing_progress(&mut subscription, first_synced_version, time_now); // Elapse enough time to timeout the subscription mock_time_service.advance(Duration::from_millis( - consensus_observer_config.max_synced_version_timeout_ms + 1, + consensus_observer_config.max_subscription_sync_timeout_ms + 1, )); // Verify that the DB is still making sync progress (the next version is higher) - verify_subscription_syncing_progress( - &mut subscription, - second_synced_version, - mock_time_service.now(), - ); + let time_now = mock_time_service.now(); + verify_subscription_syncing_progress(&mut subscription, second_synced_version, time_now); + + // Elapse some amount of time (not enough to timeout) + mock_time_service.advance(Duration::from_millis( + consensus_observer_config.max_subscription_sync_timeout_ms / 2, + )); + + // Verify that the DB is still making sync progress (we haven't timed out yet) + verify_subscription_syncing_progress(&mut subscription, second_synced_version, time_now); // Elapse enough time to timeout the subscription mock_time_service.advance(Duration::from_millis( - consensus_observer_config.max_synced_version_timeout_ms + 1, + consensus_observer_config.max_subscription_sync_timeout_ms + 1, )); // Verify that the DB is not making sync progress and that the subscription has timed out @@ -755,6 +754,10 @@ mod test { subscription.check_syncing_progress(), Err(Error::SubscriptionProgressStopped(_)) ); + assert_eq!( + subscription.highest_synced_version_and_time, + (second_synced_version, time_now) + ); } #[test] diff --git a/consensus/src/consensus_observer/observer/subscription_manager.rs b/consensus/src/consensus_observer/observer/subscription_manager.rs index 24ae1f7d321b4..2941c3231279b 100644 --- a/consensus/src/consensus_observer/observer/subscription_manager.rs +++ b/consensus/src/consensus_observer/observer/subscription_manager.rs @@ -254,6 +254,13 @@ impl SubscriptionManager { *self.active_subscription_creation_task.lock() = Some(subscription_creation_task); } + /// Terminates all active subscriptions + pub fn terminate_all_subscriptions(&mut self) { + for peer_network_id in self.get_active_subscription_peers() { + self.unsubscribe_from_peer(peer_network_id); + } + } + /// Terminates any unhealthy subscriptions and returns the list of terminated subscriptions fn terminate_unhealthy_subscriptions( &mut self, @@ -622,7 +629,7 @@ mod test { // Elapse time to simulate a DB progress error let mock_time_service = time_service.clone().into_mock(); mock_time_service.advance(Duration::from_millis( - consensus_observer_config.max_synced_version_timeout_ms + 1, + consensus_observer_config.max_subscription_sync_timeout_ms + 1, )); // Check the active subscription and verify that it is unhealthy (the DB is not syncing) @@ -695,7 +702,7 @@ mod test { let consensus_observer_config = ConsensusObserverConfig { max_subscription_timeout_ms: 100_000_000, // Use a large value so that we don't time out max_concurrent_subscriptions: 1, // Only allow one subscription - max_synced_version_timeout_ms: 100_000_000, // Use a large value so that we don't get DB progress errors + max_subscription_sync_timeout_ms: 100_000_000, // Use a large value so that we don't get DB progress errors ..ConsensusObserverConfig::default() }; @@ -833,6 +840,57 @@ mod test { }; } + #[tokio::test] + async fn test_terminate_all_subscriptions() { + // Create a consensus observer client + let network_id = NetworkId::Public; + let (peers_and_metadata, consensus_observer_client) = + create_consensus_observer_client(&[network_id]); + + // Create a new subscription manager + let consensus_observer_config = ConsensusObserverConfig::default(); + let db_reader = create_mock_db_reader(); + let time_service = TimeService::mock(); + let mut subscription_manager = SubscriptionManager::new( + consensus_observer_client, + consensus_observer_config, + None, + db_reader.clone(), + time_service.clone(), + ); + + // Verify that no subscriptions are active + verify_active_subscription_peers(&subscription_manager, vec![]); + + // Create two new subscriptions + let subscription_peer_1 = + create_peer_and_connection(network_id, peers_and_metadata.clone(), 1, None, true); + let subscription_peer_2 = + create_peer_and_connection(network_id, peers_and_metadata.clone(), 1, None, true); + for peer in &[subscription_peer_1, subscription_peer_2] { + // Create the subscription + create_observer_subscription( + &mut subscription_manager, + consensus_observer_config, + db_reader.clone(), + *peer, + time_service.clone(), + ); + } + + // Verify the subscriptions are active + verify_active_subscription_peers(&subscription_manager, vec![ + subscription_peer_1, + subscription_peer_2, + ]); + + // Terminate all subscriptions + subscription_manager.terminate_all_subscriptions(); + + // Verify that no subscriptions are active + verify_active_subscription_peers(&subscription_manager, vec![]); + } + #[tokio::test] async fn test_terminate_unhealthy_subscriptions_multiple() { // Create a consensus observer client diff --git a/consensus/src/consensus_provider.rs b/consensus/src/consensus_provider.rs index 37789380d543a..8667c9dec1226 100644 --- a/consensus/src/consensus_provider.rs +++ b/consensus/src/consensus_provider.rs @@ -187,14 +187,14 @@ pub fn start_consensus_observer( }; // Create the consensus observer - let (sync_notification_sender, sync_notification_listener) = + let (state_sync_notification_sender, state_sync_notification_listener) = tokio::sync::mpsc::unbounded_channel(); let consensus_observer = ConsensusObserver::new( node_config.clone(), consensus_observer_client, aptos_db.reader.clone(), execution_client, - sync_notification_sender, + state_sync_notification_sender, reconfig_events, consensus_publisher, TimeService::real(), @@ -204,6 +204,6 @@ pub fn start_consensus_observer( consensus_observer_runtime.spawn(consensus_observer.start( node_config.consensus_observer, consensus_observer_message_receiver, - sync_notification_listener, + state_sync_notification_listener, )); } diff --git a/consensus/src/counters.rs b/consensus/src/counters.rs index 5f655d5a69909..277fe38ed2866 100644 --- a/consensus/src/counters.rs +++ b/consensus/src/counters.rs @@ -592,6 +592,26 @@ pub static TIMEOUT_ROUNDS_COUNT: Lazy = Lazy::new(|| { .unwrap() }); +/// Count of the round timeout by reason and by whether the aggregator is the next proposer. +pub static AGGREGATED_ROUND_TIMEOUT_REASON: Lazy = Lazy::new(|| { + register_int_counter_vec!( + "aptos_consensus_agg_round_timeout_reason", + "Count of round timeouts by reason", + &["reason", "is_next_proposer"], + ) + .unwrap() +}); + +/// Count of the missing authors if any reported in the round timeout reason +pub static AGGREGATED_ROUND_TIMEOUT_REASON_MISSING_AUTHORS: Lazy = Lazy::new(|| { + register_int_counter_vec!( + "aptos_consensus_agg_round_timeout_reason_missing_authors", + "Count of missing authors in round timeout reason", + &["author"], + ) + .unwrap() +}); + /// Count the number of timeouts a node experienced since last restart (close to 0 in happy path). /// This count is different from `TIMEOUT_ROUNDS_COUNT`, because not every time a node has /// a timeout there is an ultimate decision to move to the next round (it might take multiple diff --git a/consensus/src/dag/dag_state_sync.rs b/consensus/src/dag/dag_state_sync.rs index 6cf55c4c5dd50..bc458c4e840bb 100644 --- a/consensus/src/dag/dag_state_sync.rs +++ b/consensus/src/dag/dag_state_sync.rs @@ -254,7 +254,7 @@ impl DagStateSynchronizer { }, } - self.execution_client.sync_to(commit_li).await?; + self.execution_client.sync_to_target(commit_li).await?; let inner = Arc::into_inner(sync_dag_store).expect("Only one strong reference should exists"); diff --git a/consensus/src/epoch_manager.rs b/consensus/src/epoch_manager.rs index 9dea39ef66045..54ff9d5384eb3 100644 --- a/consensus/src/epoch_manager.rs +++ b/consensus/src/epoch_manager.rs @@ -541,7 +541,7 @@ impl EpochManager

{ // make sure storage is on this ledger_info too, it should be no-op if it's already committed // panic if this doesn't succeed since the current processors are already shutdown. self.execution_client - .sync_to(ledger_info.clone()) + .sync_to_target(ledger_info.clone()) .await .context(format!( "[EpochManager] State sync to new epoch {}", diff --git a/consensus/src/execution_pipeline.rs b/consensus/src/execution_pipeline.rs index 8743ae9fbd06f..c95d6c8e1f0f3 100644 --- a/consensus/src/execution_pipeline.rs +++ b/consensus/src/execution_pipeline.rs @@ -35,11 +35,8 @@ use std::{ }; use tokio::sync::{mpsc, oneshot}; -pub type PreCommitHook = Box< - dyn 'static - + FnOnce(&[SignedTransaction], &StateComputeResult) -> BoxFuture<'static, ()> - + Send, ->; +pub type PreCommitHook = + Box BoxFuture<'static, ()> + Send>; #[allow(clippy::unwrap_used)] pub static SIG_VERIFY_POOL: Lazy> = Lazy::new(|| { @@ -287,7 +284,7 @@ impl ExecutionPipeline { } .await; let pipeline_res = res.map(|(output, execution_duration)| { - let pre_commit_hook_fut = pre_commit_hook(&input_txns, &output); + let pre_commit_hook_fut = pre_commit_hook(&output); let pre_commit_fut: BoxFuture<'static, ExecutorResult<()>> = if output.epoch_state().is_some() || !enable_pre_commit { // hack: it causes issue if pre-commit is finished at an epoch ending, and diff --git a/consensus/src/pipeline/buffer_manager.rs b/consensus/src/pipeline/buffer_manager.rs index 4b7d2e9712ddf..1b9136efb971a 100644 --- a/consensus/src/pipeline/buffer_manager.rs +++ b/consensus/src/pipeline/buffer_manager.rs @@ -31,7 +31,7 @@ use aptos_consensus_types::{ pipeline::commit_vote::CommitVote, pipelined_block::PipelinedBlock, }; -use aptos_crypto::{bls12381, HashValue}; +use aptos_crypto::HashValue; use aptos_executor_types::ExecutorResult; use aptos_logger::prelude::*; use aptos_network::protocols::{rpc::error::RpcError, wire::handshake::v1::ProtocolId}; @@ -703,7 +703,7 @@ impl BufferManager { CommitMessage::Vote(CommitVote::new_with_signature( commit_vote.author(), commit_vote.ledger_info().clone(), - bls12381::Signature::dummy_signature(), + aptos_crypto::bls12381::Signature::dummy_signature(), )) }); CommitMessage::Vote(commit_vote) diff --git a/consensus/src/pipeline/execution_client.rs b/consensus/src/pipeline/execution_client.rs index 9d50fe08e4a3f..e9ccfaffe8fb3 100644 --- a/consensus/src/pipeline/execution_client.rs +++ b/consensus/src/pipeline/execution_client.rs @@ -25,7 +25,7 @@ use crate::{ transaction_deduper::create_transaction_deduper, transaction_shuffler::create_transaction_shuffler, }; -use anyhow::Result; +use anyhow::{anyhow, Result}; use aptos_bounded_executor::BoundedExecutor; use aptos_channels::{aptos_channel, message_queues::QueueStyle}; use aptos_config::config::{ConsensusConfig, ConsensusObserverConfig}; @@ -51,7 +51,7 @@ use futures::{ }; use futures_channel::mpsc::unbounded; use move_core_types::account_address::AccountAddress; -use std::sync::Arc; +use std::{sync::Arc, time::Duration}; #[async_trait::async_trait] pub trait TExecutionClient: Send + Sync { @@ -88,8 +88,16 @@ pub trait TExecutionClient: Send + Sync { commit_msg: IncomingCommitRequest, ) -> Result<()>; - /// Synchronize to a commit that not present locally. - async fn sync_to(&self, target: LedgerInfoWithSignatures) -> Result<(), StateSyncError>; + /// Synchronizes for the specified duration and returns the latest synced + /// ledger info. Note: it is possible that state sync may run longer than + /// the specified duration (e.g., if the node is very far behind). + async fn sync_for_duration( + &self, + duration: Duration, + ) -> Result; + + /// Synchronize to a commit that is not present locally. + async fn sync_to_target(&self, target: LedgerInfoWithSignatures) -> Result<(), StateSyncError>; /// Resets the internal state of the rand and buffer managers. async fn reset(&self, target: &LedgerInfoWithSignatures) -> Result<()>; @@ -400,18 +408,36 @@ impl TExecutionClient for ExecutionProxyClient { } } - async fn sync_to(&self, target: LedgerInfoWithSignatures) -> Result<(), StateSyncError> { - fail_point!("consensus::sync_to", |_| { - Err(anyhow::anyhow!("Injected error in sync_to").into()) + async fn sync_for_duration( + &self, + duration: Duration, + ) -> Result { + fail_point!("consensus::sync_for_duration", |_| { + Err(anyhow::anyhow!("Injected error in sync_for_duration").into()) + }); + + // Sync for the specified duration + let result = self.execution_proxy.sync_for_duration(duration).await; + + // Reset the rand and buffer managers to the new synced round + if let Ok(latest_synced_ledger_info) = &result { + self.reset(latest_synced_ledger_info).await?; + } + + result + } + + async fn sync_to_target(&self, target: LedgerInfoWithSignatures) -> Result<(), StateSyncError> { + fail_point!("consensus::sync_to_target", |_| { + Err(anyhow::anyhow!("Injected error in sync_to_target").into()) }); // Reset the rand and buffer managers to the target round self.reset(&target).await?; - // TODO: handle the sync error, should re-push the ordered blocks to buffer manager - // when it's reset but sync fails. - self.execution_proxy.sync_to(target).await?; - Ok(()) + // TODO: handle the state sync error (e.g., re-push the ordered + // blocks to the buffer manager when it's reset but sync fails). + self.execution_proxy.sync_to_target(target).await } async fn reset(&self, target: &LedgerInfoWithSignatures) -> Result<()> { @@ -523,7 +549,16 @@ impl TExecutionClient for DummyExecutionClient { Ok(()) } - async fn sync_to(&self, _: LedgerInfoWithSignatures) -> Result<(), StateSyncError> { + async fn sync_for_duration( + &self, + _: Duration, + ) -> Result { + Err(StateSyncError::from(anyhow!( + "sync_for_duration() is not supported by the DummyExecutionClient!" + ))) + } + + async fn sync_to_target(&self, _: LedgerInfoWithSignatures) -> Result<(), StateSyncError> { Ok(()) } diff --git a/consensus/src/pipeline/tests/test_utils.rs b/consensus/src/pipeline/tests/test_utils.rs index 95a6cfd85f0aa..e099abaceb183 100644 --- a/consensus/src/pipeline/tests/test_utils.rs +++ b/consensus/src/pipeline/tests/test_utils.rs @@ -96,18 +96,7 @@ pub fn prepare_executed_blocks_with_ledger_info( proposals.push(proposal); } - let compute_result = StateComputeResult::new( - executed_hash, - vec![], // dummy subtree - 0, - vec![], - 0, - None, - vec![], - vec![], - vec![], - None, // block end info - ); + let compute_result = StateComputeResult::new_dummy_with_root_hash(executed_hash); let li = LedgerInfo::new( proposals.last().unwrap().block().gen_block_info( diff --git a/consensus/src/round_manager.rs b/consensus/src/round_manager.rs index 3c8cdb6993159..24d6b9901d3f2 100644 --- a/consensus/src/round_manager.rs +++ b/consensus/src/round_manager.rs @@ -60,6 +60,7 @@ use aptos_logger::prelude::*; #[cfg(test)] use aptos_safety_rules::ConsensusState; use aptos_safety_rules::TSafetyRules; +use aptos_short_hex_str::AsShortHexStr; use aptos_types::{ block_info::BlockInfo, epoch_state::EpochState, @@ -354,14 +355,34 @@ impl RoundManager { &mut self, new_round_event: NewRoundEvent, ) -> anyhow::Result<()> { + let is_current_proposer = self + .proposer_election + .is_valid_proposer(self.proposal_generator.author(), new_round_event.round); + counters::CURRENT_ROUND.set(new_round_event.round as i64); counters::ROUND_TIMEOUT_MS.set(new_round_event.timeout.as_millis() as i64); match new_round_event.reason { NewRoundReason::QCReady => { counters::QC_ROUNDS_COUNT.inc(); }, - NewRoundReason::Timeout(_) => { + NewRoundReason::Timeout(ref reason) => { counters::TIMEOUT_ROUNDS_COUNT.inc(); + counters::AGGREGATED_ROUND_TIMEOUT_REASON + .with_label_values(&[&reason.to_string(), &is_current_proposer.to_string()]) + .inc(); + if is_current_proposer { + if let RoundTimeoutReason::PayloadUnavailable { missing_authors } = reason { + let ordered_peers = + self.epoch_state.verifier.get_ordered_account_addresses(); + for idx in missing_authors.iter_ones() { + if let Some(author) = ordered_peers.get(idx) { + counters::AGGREGATED_ROUND_TIMEOUT_REASON_MISSING_AUTHORS + .with_label_values(&[author.short_str().as_str()]) + .inc(); + } + } + } + } }, }; info!( @@ -374,10 +395,7 @@ impl RoundManager { self.proposal_status_tracker .push(new_round_event.reason.clone()); - if self - .proposer_election - .is_valid_proposer(self.proposal_generator.author(), new_round_event.round) - { + if is_current_proposer { let epoch_state = self.epoch_state.clone(); let network = self.network.clone(); let sync_info = self.block_store.sync_info(); diff --git a/consensus/src/round_manager_test.rs b/consensus/src/round_manager_test.rs index 29716947991bd..7bccb2747163e 100644 --- a/consensus/src/round_manager_test.rs +++ b/consensus/src/round_manager_test.rs @@ -524,6 +524,14 @@ impl NodeSetup { } } +fn config_with_round_timeout_msg_disabled() -> ConsensusConfig { + // Disable RoundTimeoutMsg to unless expliclity enabled. + ConsensusConfig { + enable_round_timeout_msg: false, + ..Default::default() + } +} + fn start_replying_to_block_retreival(nodes: Vec) -> ReplyingRPCHandle { let done = Arc::new(AtomicBool::new(false)); let mut handles = Vec::new(); @@ -966,7 +974,7 @@ fn sync_info_carried_on_timeout_vote() { 1, None, None, - None, + Some(config_with_round_timeout_msg_disabled()), None, None, ); @@ -1470,7 +1478,7 @@ fn nil_vote_on_timeout() { 1, None, None, - None, + Some(config_with_round_timeout_msg_disabled()), None, None, ); @@ -1553,7 +1561,7 @@ fn vote_resent_on_timeout() { 1, None, None, - None, + Some(config_with_round_timeout_msg_disabled()), None, None, ); @@ -1706,7 +1714,7 @@ fn safety_rules_crash() { 1, None, None, - None, + Some(config_with_round_timeout_msg_disabled()), None, None, ); @@ -1772,7 +1780,7 @@ fn echo_timeout() { 4, None, None, - None, + Some(config_with_round_timeout_msg_disabled()), None, None, ); @@ -1825,6 +1833,64 @@ fn echo_timeout() { }); } +#[test] +fn echo_round_timeout_msg() { + let runtime = consensus_runtime(); + let mut playground = NetworkPlayground::new(runtime.handle().clone()); + let mut nodes = NodeSetup::create_nodes( + &mut playground, + runtime.handle().clone(), + 4, + None, + None, + None, + None, + None, + ); + runtime.spawn(playground.start()); + timed_block_on(&runtime, async { + // clear the message queue + for node in &mut nodes { + node.next_proposal().await; + } + // timeout 3 nodes + for node in &mut nodes[1..] { + node.round_manager + .process_local_timeout(1) + .await + .unwrap_err(); + } + let node_0 = &mut nodes[0]; + // node 0 doesn't timeout and should echo the timeout after 2 timeout message + for i in 0..3 { + let timeout_vote = node_0.next_timeout().await; + let result = node_0 + .round_manager + .process_round_timeout_msg(timeout_vote) + .await; + // first and third message should not timeout + if i == 0 || i == 2 { + assert!(result.is_ok()); + } + if i == 1 { + // timeout is an Error + assert!(result.is_err()); + } + } + + let node_1 = &mut nodes[1]; + // it receives 4 timeout messages (1 from each) and doesn't echo since it already timeout + for _ in 0..4 { + let timeout_vote = node_1.next_timeout().await; + node_1 + .round_manager + .process_round_timeout_msg(timeout_vote) + .await + .unwrap(); + } + }); +} + #[test] fn no_next_test() { let runtime = consensus_runtime(); diff --git a/consensus/src/state_computer.rs b/consensus/src/state_computer.rs index 28e29379846d6..1fe6a69d92fbd 100644 --- a/consensus/src/state_computer.rs +++ b/consensus/src/state_computer.rs @@ -30,12 +30,15 @@ use aptos_logger::prelude::*; use aptos_metrics_core::IntGauge; use aptos_types::{ account_address::AccountAddress, block_executor::config::BlockExecutorConfigFromOnchain, - block_metadata_ext::BlockMetadataExt, epoch_state::EpochState, - ledger_info::LedgerInfoWithSignatures, randomness::Randomness, transaction::SignedTransaction, + epoch_state::EpochState, ledger_info::LedgerInfoWithSignatures, randomness::Randomness, }; use fail::fail_point; use futures::{future::BoxFuture, SinkExt, StreamExt}; -use std::{boxed::Box, sync::Arc, time::Instant}; +use std::{ + boxed::Box, + sync::Arc, + time::{Duration, Instant}, +}; use tokio::sync::Mutex as AsyncMutex; pub type StateComputeResultFut = BoxFuture<'static, ExecutorResult>; @@ -128,44 +131,36 @@ impl ExecutionProxy { fn pre_commit_hook( &self, block: &Block, - metadata: BlockMetadataExt, payload_manager: Arc, ) -> PreCommitHook { let mut pre_commit_notifier = self.pre_commit_notifier.clone(); let state_sync_notifier = self.state_sync_notifier.clone(); let payload = block.payload().cloned(); let timestamp = block.timestamp_usecs(); - let validator_txns = block.validator_txns().cloned().unwrap_or_default(); - let block_id = block.id(); - Box::new( - move |user_txns: &[SignedTransaction], state_compute_result: &StateComputeResult| { - let input_txns = Block::combine_to_input_transactions( - validator_txns, - user_txns.to_vec(), - metadata, - ); - let txns = state_compute_result.transactions_to_commit(input_txns, block_id); - let subscribable_events = state_compute_result.subscribable_events().to_vec(); - Box::pin(async move { - pre_commit_notifier - .send(Box::pin(async move { - if let Err(e) = monitor!( - "notify_state_sync", - state_sync_notifier - .notify_new_commit(txns, subscribable_events) - .await - ) { - error!(error = ?e, "Failed to notify state synchronizer"); - } - - let payload_vec = payload.into_iter().collect(); - payload_manager.notify_commit(timestamp, payload_vec); - })) - .await - .expect("Failed to send pre-commit notification"); - }) - }, - ) + Box::new(move |state_compute_result: &StateComputeResult| { + let state_compute_result = state_compute_result.clone(); + Box::pin(async move { + pre_commit_notifier + .send(Box::pin(async move { + let txns = state_compute_result.transactions_to_commit(); + let subscribable_events = + state_compute_result.subscribable_events().to_vec(); + if let Err(e) = monitor!( + "notify_state_sync", + state_sync_notifier + .notify_new_commit(txns, subscribable_events) + .await + ) { + error!(error = ?e, "Failed to notify state synchronizer"); + } + + let payload_vec = payload.into_iter().collect(); + payload_manager.notify_commit(timestamp, payload_vec); + })) + .await + .expect("Failed to send pre-commit notification"); + }) + }) } } @@ -226,7 +221,7 @@ impl StateComputer for ExecutionProxy { parent_block_id, transaction_generator, block_executor_onchain_config, - self.pre_commit_hook(block, metadata, payload_manager), + self.pre_commit_hook(block, payload_manager), lifetime_guard, ) .await; @@ -324,22 +319,67 @@ impl StateComputer for ExecutionProxy { Ok(()) } - /// Synchronize to a commit that not present locally. - async fn sync_to(&self, target: LedgerInfoWithSignatures) -> Result<(), StateSyncError> { + /// Best effort state synchronization for the specified duration + async fn sync_for_duration( + &self, + duration: Duration, + ) -> Result { + // Grab the logical time lock + let mut latest_logical_time = self.write_mutex.lock().await; + + // Before state synchronization, we have to call finish() to free the + // in-memory SMT held by the BlockExecutor to prevent a memory leak. + self.executor.finish(); + + // Inject an error for fail point testing + fail_point!("consensus::sync_for_duration", |_| { + Err(anyhow::anyhow!("Injected error in sync_for_duration").into()) + }); + + // Invoke state sync to synchronize for the specified duration. Here, the + // ChunkExecutor will process chunks and commit to storage. However, after + // block execution and commits, the internal state of the ChunkExecutor may + // not be up to date. So, it is required to reset the cache of the + // ChunkExecutor in state sync when requested to sync. + let result = monitor!( + "sync_for_duration", + self.state_sync_notifier.sync_for_duration(duration).await + ); + + // Update the latest logical time + if let Ok(latest_synced_ledger_info) = &result { + let ledger_info = latest_synced_ledger_info.ledger_info(); + let synced_logical_time = LogicalTime::new(ledger_info.epoch(), ledger_info.round()); + *latest_logical_time = synced_logical_time; + } + + // Similarly, after state synchronization, we have to reset the cache of + // the BlockExecutor to guarantee the latest committed state is up to date. + self.executor.reset()?; + + // Return the result + result.map_err(|error| { + let anyhow_error: anyhow::Error = error.into(); + anyhow_error.into() + }) + } + + /// Synchronize to a commit that is not present locally. + async fn sync_to_target(&self, target: LedgerInfoWithSignatures) -> Result<(), StateSyncError> { + // Grab the logical time lock and calculate the target logical time let mut latest_logical_time = self.write_mutex.lock().await; - let logical_time = + let target_logical_time = LogicalTime::new(target.ledger_info().epoch(), target.ledger_info().round()); - let block_timestamp = target.commit_info().timestamp_usecs(); - // Before the state synchronization, we have to call finish() to free the in-memory SMT - // held by BlockExecutor to prevent memory leak. + // Before state synchronization, we have to call finish() to free the + // in-memory SMT held by BlockExecutor to prevent a memory leak. self.executor.finish(); // The pipeline phase already committed beyond the target block timestamp, just return. - if *latest_logical_time >= logical_time { + if *latest_logical_time >= target_logical_time { warn!( "State sync target {:?} is lower than already committed logical time {:?}", - logical_time, *latest_logical_time + target_logical_time, *latest_logical_time ); return Ok(()); } @@ -348,30 +388,36 @@ impl StateComputer for ExecutionProxy { // so it can set batches expiration accordingly. // Might be none if called in the recovery path, or between epoch stop and start. if let Some(inner) = self.state.read().as_ref() { + let block_timestamp = target.commit_info().timestamp_usecs(); inner .payload_manager .notify_commit(block_timestamp, Vec::new()); } - fail_point!("consensus::sync_to", |_| { - Err(anyhow::anyhow!("Injected error in sync_to").into()) + // Inject an error for fail point testing + fail_point!("consensus::sync_to_target", |_| { + Err(anyhow::anyhow!("Injected error in sync_to_target").into()) }); - // Here to start to do state synchronization where ChunkExecutor inside will - // process chunks and commit to Storage. However, after block execution and - // commitments, the sync state of ChunkExecutor may be not up to date so - // it is required to reset the cache of ChunkExecutor in State Sync - // when requested to sync. - let res = monitor!( - "sync_to", + + // Invoke state sync to synchronize to the specified target. Here, the + // ChunkExecutor will process chunks and commit to storage. However, after + // block execution and commits, the internal state of the ChunkExecutor may + // not be up to date. So, it is required to reset the cache of the + // ChunkExecutor in state sync when requested to sync. + let result = monitor!( + "sync_to_target", self.state_sync_notifier.sync_to_target(target).await ); - *latest_logical_time = logical_time; - // Similarly, after the state synchronization, we have to reset the cache - // of BlockExecutor to guarantee the latest committed state is up to date. + // Update the latest logical time + *latest_logical_time = target_logical_time; + + // Similarly, after state synchronization, we have to reset the cache of + // the BlockExecutor to guarantee the latest committed state is up to date. self.executor.reset()?; - res.map_err(|error| { + // Return the result + result.map_err(|error| { let anyhow_error: anyhow::Error = error.into(); anyhow_error.into() }) @@ -513,6 +559,15 @@ async fn test_commit_sync_race() { Ok(()) } + async fn sync_for_duration( + &self, + _duration: std::time::Duration, + ) -> std::result::Result { + Err(Error::UnexpectedErrorEncountered( + "sync_for_duration() is not supported by the RecordedCommit!".into(), + )) + } + async fn sync_to_target( &self, target: LedgerInfoWithSignatures, @@ -567,8 +622,8 @@ async fn test_commit_sync_race() { .commit(&[], generate_li(1, 10), callback) .await .unwrap(); - assert!(executor.sync_to(generate_li(1, 8)).await.is_ok()); + assert!(executor.sync_to_target(generate_li(1, 8)).await.is_ok()); assert_eq!(*recorded_commit.time.lock(), LogicalTime::new(1, 10)); - assert!(executor.sync_to(generate_li(2, 8)).await.is_ok()); + assert!(executor.sync_to_target(generate_li(2, 8)).await.is_ok()); assert_eq!(*recorded_commit.time.lock(), LogicalTime::new(2, 8)); } diff --git a/consensus/src/state_computer_tests.rs b/consensus/src/state_computer_tests.rs index 6c79e8d3ee710..bbcb5167f1b27 100644 --- a/consensus/src/state_computer_tests.rs +++ b/consensus/src/state_computer_tests.rs @@ -21,10 +21,13 @@ use aptos_types::{ contract_event::ContractEvent, epoch_state::EpochState, ledger_info::LedgerInfoWithSignatures, - transaction::{ExecutionStatus, SignedTransaction, Transaction, TransactionStatus}, + transaction::{SignedTransaction, Transaction, TransactionStatus}, validator_txn::ValidatorTransaction, }; -use std::sync::{atomic::AtomicU64, Arc}; +use std::{ + sync::{atomic::AtomicU64, Arc}, + time::Duration, +}; use tokio::{runtime::Handle, sync::Mutex as AsyncMutex}; struct DummyStateSyncNotifier { @@ -62,6 +65,15 @@ impl ConsensusNotificationSender for DummyStateSyncNotifier { Ok(()) } + async fn sync_for_duration( + &self, + _duration: Duration, + ) -> Result { + Err(Error::UnexpectedErrorEncountered( + "sync_for_duration() is not supported by the DummyStateSyncNotifier!".into(), + )) + } + async fn sync_to_target(&self, _target: LedgerInfoWithSignatures) -> Result<(), Error> { unreachable!() } @@ -117,18 +129,19 @@ impl BlockExecutorTrait for DummyBlockExecutor { _parent_block_id: HashValue, _state_checkpoint_output: StateCheckpointOutput, ) -> ExecutorResult { - let num_txns = self + let txns = self .blocks_received .lock() .last() .unwrap() .transactions - .num_transactions(); + .clone() + .into_txns() + .into_iter() + .map(|t| t.into_inner()) + .collect(); - Ok(StateComputeResult::new_dummy_with_compute_status(vec![ - TransactionStatus::Keep(ExecutionStatus::Success); - num_txns - ])) + Ok(StateComputeResult::new_dummy_with_input_txns(txns)) } fn pre_commit_block( diff --git a/consensus/src/state_replication.rs b/consensus/src/state_replication.rs index df02ca23194c3..bbe171fc4e3e7 100644 --- a/consensus/src/state_replication.rs +++ b/consensus/src/state_replication.rs @@ -15,7 +15,7 @@ use aptos_types::{ block_executor::config::BlockExecutorConfigFromOnchain, epoch_state::EpochState, ledger_info::LedgerInfoWithSignatures, randomness::Randomness, }; -use std::sync::Arc; +use std::{sync::Arc, time::Duration}; pub type StateComputerCommitCallBackType = Box], LedgerInfoWithSignatures) + Send + Sync>; @@ -45,11 +45,20 @@ pub trait StateComputer: Send + Sync { callback: StateComputerCommitCallBackType, ) -> ExecutorResult<()>; + /// Best effort state synchronization for the specified duration. + /// This function returns the latest synced ledger info after state syncing. + /// Note: it is possible that state sync may run longer than the specified + /// duration (e.g., if the node is very far behind). + async fn sync_for_duration( + &self, + duration: Duration, + ) -> Result; + /// Best effort state synchronization to the given target LedgerInfo. /// In case of success (`Result::Ok`) the LI of storage is at the given target. /// In case of failure (`Result::Error`) the LI of storage remains unchanged, and the validator /// can assume there were no modifications to the storage made. - async fn sync_to(&self, target: LedgerInfoWithSignatures) -> Result<(), StateSyncError>; + async fn sync_to_target(&self, target: LedgerInfoWithSignatures) -> Result<(), StateSyncError>; // Reconfigure to execute transactions for a new epoch. fn new_epoch( diff --git a/consensus/src/test_utils/mock_execution_client.rs b/consensus/src/test_utils/mock_execution_client.rs index 2649af9fd3b31..cfc5f3441f773 100644 --- a/consensus/src/test_utils/mock_execution_client.rs +++ b/consensus/src/test_utils/mock_execution_client.rs @@ -14,7 +14,7 @@ use crate::{ state_replication::StateComputerCommitCallBackType, test_utils::mock_storage::MockStorage, }; -use anyhow::{format_err, Result}; +use anyhow::{anyhow, format_err, Result}; use aptos_channels::aptos_channel; use aptos_consensus_types::{ common::{Payload, Round}, @@ -33,7 +33,7 @@ use aptos_types::{ use futures::{channel::mpsc, SinkExt}; use futures_channel::mpsc::UnboundedSender; use move_core_types::account_address::AccountAddress; -use std::{collections::HashMap, sync::Arc}; +use std::{collections::HashMap, sync::Arc, time::Duration}; pub struct MockExecutionClient { state_sync_client: mpsc::UnboundedSender>, @@ -162,7 +162,16 @@ impl TExecutionClient for MockExecutionClient { Ok(()) } - async fn sync_to(&self, commit: LedgerInfoWithSignatures) -> Result<(), StateSyncError> { + async fn sync_for_duration( + &self, + _duration: Duration, + ) -> Result { + Err(StateSyncError::from(anyhow!( + "sync_for_duration() is not supported by the MockExecutionClient!" + ))) + } + + async fn sync_to_target(&self, commit: LedgerInfoWithSignatures) -> Result<(), StateSyncError> { debug!( "Fake sync to block id {}", commit.ledger_info().consensus_block_id() diff --git a/consensus/src/test_utils/mock_state_computer.rs b/consensus/src/test_utils/mock_state_computer.rs index ad602f93f4f75..58cba2eeace05 100644 --- a/consensus/src/test_utils/mock_state_computer.rs +++ b/consensus/src/test_utils/mock_state_computer.rs @@ -11,7 +11,7 @@ use crate::{ transaction_deduper::TransactionDeduper, transaction_shuffler::TransactionShuffler, }; -use anyhow::Result; +use anyhow::{anyhow, Result}; use aptos_consensus_types::{ block::Block, pipeline_execution_result::PipelineExecutionResult, pipelined_block::PipelinedBlock, @@ -67,7 +67,19 @@ impl StateComputer for EmptyStateComputer { Ok(()) } - async fn sync_to(&self, _commit: LedgerInfoWithSignatures) -> Result<(), StateSyncError> { + async fn sync_for_duration( + &self, + _duration: Duration, + ) -> Result { + Err(StateSyncError::from(anyhow!( + "sync_for_duration() is not supported by the EmptyStateComputer!" + ))) + } + + async fn sync_to_target( + &self, + _target: LedgerInfoWithSignatures, + ) -> Result<(), StateSyncError> { Ok(()) } @@ -141,7 +153,19 @@ impl StateComputer for RandomComputeResultStateComputer { Ok(()) } - async fn sync_to(&self, _commit: LedgerInfoWithSignatures) -> Result<(), StateSyncError> { + async fn sync_for_duration( + &self, + _duration: Duration, + ) -> Result { + Err(StateSyncError::from(anyhow!( + "sync_for_duration() is not supported by the RandomComputeResultStateComputer!" + ))) + } + + async fn sync_to_target( + &self, + _target: LedgerInfoWithSignatures, + ) -> Result<(), StateSyncError> { Ok(()) } diff --git a/crates/aptos-drop-helper/Cargo.toml b/crates/aptos-drop-helper/Cargo.toml index 54dc6fc482c16..936ef297eeebc 100644 --- a/crates/aptos-drop-helper/Cargo.toml +++ b/crates/aptos-drop-helper/Cargo.toml @@ -15,5 +15,6 @@ rust-version = { workspace = true } [dependencies] aptos-infallible = { workspace = true } aptos-metrics-core = { workspace = true } +derive_more = { workspace = true } once_cell = { workspace = true } threadpool = { workspace = true } diff --git a/crates/aptos-drop-helper/src/lib.rs b/crates/aptos-drop-helper/src/lib.rs index ce91987bf6525..169aae9c41fe3 100644 --- a/crates/aptos-drop-helper/src/lib.rs +++ b/crates/aptos-drop-helper/src/lib.rs @@ -2,7 +2,9 @@ // SPDX-License-Identifier: Apache-2.0 use crate::async_concurrent_dropper::AsyncConcurrentDropper; +use derive_more::{Deref, DerefMut}; use once_cell::sync::Lazy; +use std::mem::ManuallyDrop; pub mod async_concurrent_dropper; pub mod async_drop_queue; @@ -11,8 +13,31 @@ mod metrics; pub static DEFAULT_DROPPER: Lazy = Lazy::new(|| AsyncConcurrentDropper::new("default", 32, 8)); -/// Arc will be `Send + 'static`, which is requried to be able to drop Arc +/// Arc will be `Send + 'static`, which is required to be able to drop Arc /// in another thread pub trait ArcAsyncDrop: Send + Sync + 'static {} impl ArcAsyncDrop for T {} + +#[derive(Clone, Debug, Default, PartialEq, Eq, PartialOrd, Ord, Hash, Deref, DerefMut)] +#[repr(transparent)] +pub struct DropHelper { + #[deref] + #[deref_mut] + inner: ManuallyDrop, +} + +impl DropHelper { + pub fn new(inner: T) -> Self { + Self { + inner: ManuallyDrop::new(inner), + } + } +} + +impl Drop for DropHelper { + fn drop(&mut self) { + let inner = unsafe { ManuallyDrop::take(&mut self.inner) }; + DEFAULT_DROPPER.schedule_drop(inner); + } +} diff --git a/crates/aptos-rosetta/src/main.rs b/crates/aptos-rosetta/src/main.rs index 11574dd66ca18..d652ef09254c1 100644 --- a/crates/aptos-rosetta/src/main.rs +++ b/crates/aptos-rosetta/src/main.rs @@ -9,6 +9,7 @@ use aptos_config::config::{ApiConfig, DEFAULT_MAX_PAGE_SIZE}; use aptos_logger::prelude::*; use aptos_node::AptosNodeArgs; use aptos_rosetta::{bootstrap, common::native_coin, types::Currency}; +use aptos_sdk::move_types::language_storage::StructTag; use aptos_types::chain_id::ChainId; use clap::Parser; use std::{ @@ -16,6 +17,7 @@ use std::{ fs::File, net::SocketAddr, path::PathBuf, + str::FromStr, sync::{ atomic::{AtomicBool, Ordering}, Arc, @@ -243,9 +245,28 @@ impl ServerArgs for OfflineArgs { if let Some(ref filepath) = self.currency_config_file { let file = File::open(filepath).unwrap(); let currencies: Vec = serde_json::from_reader(file).unwrap(); - currencies.into_iter().for_each(|item| { - supported_currencies.insert(item); - }); + for item in currencies.into_iter() { + // Do a safety check on possible currencies on startup + if item.symbol.as_str() == "" { + warn!( + "Currency {:?} has an empty symbol, and is being skipped", + item + ); + } else if let Some(metadata) = item.metadata.as_ref() { + if let Some(move_type) = metadata.move_type.as_ref() { + if StructTag::from_str(move_type).is_ok() { + supported_currencies.insert(item); + continue; + } + } + warn!( + "Currency {:?} has an invalid metadata coin type, and is being skipped", + item + ); + } else { + supported_currencies.insert(item); + } + } } supported_currencies diff --git a/docker/builder/build-indexer.sh b/docker/builder/build-indexer.sh index 95bebee0a5de7..aa73cf3a8eb3f 100755 --- a/docker/builder/build-indexer.sh +++ b/docker/builder/build-indexer.sh @@ -16,7 +16,7 @@ cargo build --locked --profile=$PROFILE \ -p aptos-indexer-grpc-file-store \ -p aptos-indexer-grpc-data-service \ -p aptos-nft-metadata-crawler \ - -p aptos-indexer-grpc-file-store-backfiller \ + -p aptos-indexer-grpc-file-checker \ "$@" # After building, copy the binaries we need to `dist` since the `target` directory is used as docker cache mount and only available during the RUN step @@ -25,7 +25,7 @@ BINS=( aptos-indexer-grpc-file-store aptos-indexer-grpc-data-service aptos-nft-metadata-crawler - aptos-indexer-grpc-file-store-backfiller + aptos-indexer-grpc-file-checker ) mkdir dist diff --git a/docker/builder/indexer-grpc.Dockerfile b/docker/builder/indexer-grpc.Dockerfile index 255293fd3e27c..2f79181084662 100644 --- a/docker/builder/indexer-grpc.Dockerfile +++ b/docker/builder/indexer-grpc.Dockerfile @@ -17,7 +17,7 @@ RUN --mount=type=cache,target=/var/cache/apt,sharing=locked \ COPY --link --from=indexer-builder /aptos/dist/aptos-indexer-grpc-cache-worker /usr/local/bin/aptos-indexer-grpc-cache-worker COPY --link --from=indexer-builder /aptos/dist/aptos-indexer-grpc-file-store /usr/local/bin/aptos-indexer-grpc-file-store COPY --link --from=indexer-builder /aptos/dist/aptos-indexer-grpc-data-service /usr/local/bin/aptos-indexer-grpc-data-service -COPY --link --from=indexer-builder /aptos/dist/aptos-indexer-grpc-file-store-backfiller /usr/local/bin/aptos-indexer-grpc-file-store-backfiller +COPY --link --from=indexer-builder /aptos/dist/aptos-indexer-grpc-file-checker /usr/local/bin/aptos-indexer-grpc-file-checker # The health check port EXPOSE 8080 diff --git a/docker/compose/aptos-node/fullnode.yaml b/docker/compose/aptos-node/fullnode.yaml index f777f32c20f50..3721e5836d71b 100644 --- a/docker/compose/aptos-node/fullnode.yaml +++ b/docker/compose/aptos-node/fullnode.yaml @@ -7,6 +7,10 @@ base: execution: genesis_file_location: "/opt/aptos/genesis/genesis.blob" +storage: + rocksdb_configs: + enable_storage_sharding: true + full_node_networks: - network_id: private: "vfn" diff --git a/docker/compose/aptos-node/validator.yaml b/docker/compose/aptos-node/validator.yaml index 7df4ba8deaacb..33a87b2ccfcd2 100644 --- a/docker/compose/aptos-node/validator.yaml +++ b/docker/compose/aptos-node/validator.yaml @@ -21,6 +21,10 @@ consensus: execution: genesis_file_location: "/opt/aptos/genesis/genesis.blob" +storage: + rocksdb_configs: + enable_storage_sharding: true + validator_network: discovery_method: "onchain" mutual_authentication: true diff --git a/ecosystem/indexer-grpc/indexer-grpc-file-checker/Cargo.toml b/ecosystem/indexer-grpc/indexer-grpc-file-checker/Cargo.toml new file mode 100644 index 0000000000000..078984e2739ff --- /dev/null +++ b/ecosystem/indexer-grpc/indexer-grpc-file-checker/Cargo.toml @@ -0,0 +1,30 @@ +[package] +name = "aptos-indexer-grpc-file-checker" +description = "Indexer gRPC file checker." +version = "1.0.0" + +# Workspace inherited keys +authors = { workspace = true } +edition = { workspace = true } +homepage = { workspace = true } +license = { workspace = true } +publish = { workspace = true } +repository = { workspace = true } +rust-version = { workspace = true } + +[dependencies] +anyhow = { workspace = true } +aptos-indexer-grpc-server-framework = { workspace = true } +aptos-indexer-grpc-utils = { workspace = true } +aptos-metrics-core = { workspace = true } +async-trait = { workspace = true } +clap = { workspace = true } +cloud-storage = { workspace = true } +once_cell = { workspace = true } +serde = { workspace = true } +serde_json = { workspace = true } +tokio = { workspace = true } +tracing = { workspace = true } + +[target.'cfg(unix)'.dependencies] +jemallocator = { workspace = true } diff --git a/ecosystem/indexer-grpc/indexer-grpc-file-checker/README.md b/ecosystem/indexer-grpc/indexer-grpc-file-checker/README.md new file mode 100644 index 0000000000000..7edda1fc7a8d5 --- /dev/null +++ b/ecosystem/indexer-grpc/indexer-grpc-file-checker/README.md @@ -0,0 +1,14 @@ +# Indexer GRPC file checker +A program that compares files in two buckets and to make sure the content are the same. + +## How to run it. + +Example of config: + +``` +health_check_port: 8081 + server_config: + existing_bucket_name: bucket_being_used + new_bucket_name: bucket_with_new_sharding + starting_version: 123123 +``` diff --git a/ecosystem/indexer-grpc/indexer-grpc-file-checker/src/lib.rs b/ecosystem/indexer-grpc/indexer-grpc-file-checker/src/lib.rs new file mode 100644 index 0000000000000..d39ee422d748b --- /dev/null +++ b/ecosystem/indexer-grpc/indexer-grpc-file-checker/src/lib.rs @@ -0,0 +1,44 @@ +// Copyright © Aptos Foundation +// SPDX-License-Identifier: Apache-2.0 + +pub mod processor; + +use anyhow::Result; +use aptos_indexer_grpc_server_framework::RunnableConfig; +use processor::Processor; +use serde::{Deserialize, Serialize}; + +#[derive(Clone, Debug, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] +pub struct IndexerGrpcFileCheckerConfig { + pub existing_bucket_name: String, + pub new_bucket_name: String, + pub starting_version: u64, +} + +impl From for Processor { + fn from(val: IndexerGrpcFileCheckerConfig) -> Self { + Processor { + existing_bucket_name: val.existing_bucket_name, + new_bucket_name: val.new_bucket_name, + starting_version: val.starting_version, + } + } +} + +#[async_trait::async_trait] +impl RunnableConfig for IndexerGrpcFileCheckerConfig { + async fn run(&self) -> Result<()> { + let processor: Processor = self.clone().into(); + + processor + .run() + .await + .expect("File checker exited unexpectedly"); + Ok(()) + } + + fn get_server_name(&self) -> String { + "idxfilechk".to_string() + } +} diff --git a/ecosystem/indexer-grpc/indexer-grpc-file-checker/src/main.rs b/ecosystem/indexer-grpc/indexer-grpc-file-checker/src/main.rs new file mode 100644 index 0000000000000..24507092357ef --- /dev/null +++ b/ecosystem/indexer-grpc/indexer-grpc-file-checker/src/main.rs @@ -0,0 +1,20 @@ +// Copyright © Aptos Foundation +// SPDX-License-Identifier: Apache-2.0 + +use anyhow::Result; +use aptos_indexer_grpc_file_checker::IndexerGrpcFileCheckerConfig; +use aptos_indexer_grpc_server_framework::ServerArgs; +use clap::Parser; + +#[cfg(unix)] +#[global_allocator] +static ALLOC: jemallocator::Jemalloc = jemallocator::Jemalloc; + +#[tokio::main] +async fn main() -> Result<()> { + let args = ServerArgs::parse(); + args.run::() + .await + .expect("Failed to run server"); + Ok(()) +} diff --git a/ecosystem/indexer-grpc/indexer-grpc-file-checker/src/processor.rs b/ecosystem/indexer-grpc/indexer-grpc-file-checker/src/processor.rs new file mode 100644 index 0000000000000..63fc0130ad865 --- /dev/null +++ b/ecosystem/indexer-grpc/indexer-grpc-file-checker/src/processor.rs @@ -0,0 +1,200 @@ +// Copyright © Aptos Foundation +// SPDX-License-Identifier: Apache-2.0 + +use anyhow::{ensure, Context, Result}; +use aptos_indexer_grpc_utils::compression_util::{FileEntry, StorageFormat}; +use aptos_metrics_core::{register_int_counter, IntCounter}; +use cloud_storage::Client; +use once_cell::sync::Lazy; +use serde::{de::DeserializeOwned, Deserialize, Serialize}; + +pub static FILE_DIFF_COUNTER: Lazy = Lazy::new(|| { + register_int_counter!( + "indexer_grpc_file_checker_file_diff", + "Count of the files that are different.", + ) + .unwrap() +}); + +const PROGRESS_FILE_NAME: &str = "file_checker_progress.json"; +const METADATA_FILE_NAME: &str = "metadata.json"; + +// Update the progress file every 3 minutes. +const PROGRESS_FILE_UPDATE_INTERVAL_IN_SECS: u64 = 180; + +/// Checker compares the data in the existing bucket with the data in the new bucket. +/// The progress is saved in a file under the new bucket. +pub struct Processor { + /// Existing bucket name. + pub existing_bucket_name: String, + /// New bucket name; this job is to make sure the data in the new bucket is correct. + pub new_bucket_name: String, + /// The version to start from. This is for **bootstrapping** the file checker only. + pub starting_version: u64, +} + +#[derive(Debug, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct ProgressFile { + file_checker_version: u64, + file_checker_chain_id: u64, +} + +#[derive(Debug, Serialize, Deserialize)] +pub struct MetadataFile { + chain_id: u64, +} + +impl Processor { + pub async fn run(&self) -> Result<()> { + let (client, mut progress_file) = self.init().await?; + let mut last_update_time = std::time::Instant::now(); + + loop { + let current_version = progress_file.file_checker_version; + + let file_name = + FileEntry::build_key(current_version, StorageFormat::Lz4CompressedProto); + let existing_file = + download_raw_file(&client, &self.existing_bucket_name, &file_name).await?; + let new_file = download_raw_file(&client, &self.new_bucket_name, &file_name).await?; + if existing_file.is_none() || new_file.is_none() { + let bucket_name = if existing_file.is_none() { + &self.existing_bucket_name + } else { + &self.new_bucket_name + }; + tracing::info!( + bucket_name = bucket_name, + file_name = file_name.as_str(), + "Transaction file is not found in one of the buckets." + ); + // Wait for the next file to be uploaded. + tokio::time::sleep(tokio::time::Duration::from_secs(30)).await; + continue; + } + // Compare the files. + let existing_file = existing_file.unwrap(); + let new_file = new_file.unwrap(); + if existing_file != new_file { + // Files are different. + tracing::error!("Files are different: {}", file_name); + FILE_DIFF_COUNTER.inc(); + + // Sleep for a while to allow metrics to be updated. + tokio::time::sleep(tokio::time::Duration::from_secs(120)).await; + panic!("Files are different: {}", file_name); + } + tracing::info!( + file_name = file_name.as_str(), + transaction_version = progress_file.file_checker_version, + "File is verified." + ); + + progress_file.file_checker_version += 1000; + + // If the progress file is updated recently, skip the update. + if last_update_time.elapsed().as_secs() < PROGRESS_FILE_UPDATE_INTERVAL_IN_SECS { + continue; + } + // Upload the progress file. + let progress_file_bytes = + serde_json::to_vec(&progress_file).context("Failed to serialize progress file.")?; + client + .object() + .create( + &self.new_bucket_name, + progress_file_bytes, + PROGRESS_FILE_NAME, + "application/json", + ) + .await + .context("Update progress file failure")?; + tracing::info!("Progress file is updated."); + last_update_time = std::time::Instant::now(); + } + } + + /// Initialize the processor. + pub async fn init(&self) -> Result<(Client, ProgressFile)> { + let client = Client::new(); + + // All errors are considered fatal: files must exist for the processor to work. + let existing_metadata = + download_file::(&client, &self.existing_bucket_name, METADATA_FILE_NAME) + .await + .context("Failed to get metadata.")? + .expect("Failed to download metadata file"); + let new_metadata = + download_file::(&client, &self.new_bucket_name, METADATA_FILE_NAME) + .await + .context("Failed to get metadata.")? + .expect("Failed to download metadata file"); + + // Ensure the chain IDs match. + ensure!( + existing_metadata.chain_id == new_metadata.chain_id, + "Chain IDs do not match: {} != {}", + existing_metadata.chain_id, + new_metadata.chain_id + ); + + let progress_file = + download_file::(&client, &self.new_bucket_name, PROGRESS_FILE_NAME) + .await + .context("Failed to get progress file.")? + .unwrap_or(ProgressFile { + file_checker_version: self.starting_version, + file_checker_chain_id: existing_metadata.chain_id, + }); + // Ensure the chain IDs match. + ensure!( + existing_metadata.chain_id == progress_file.file_checker_chain_id, + "Chain IDs do not match: {} != {}", + existing_metadata.chain_id, + progress_file.file_checker_chain_id + ); + tracing::info!( + starting_version = self.starting_version, + "Processor initialized.", + ); + + Ok((client, progress_file)) + } +} + +async fn download_raw_file( + client: &Client, + bucket_name: &str, + file_name: &str, +) -> Result>> { + let file = client.object().download(bucket_name, file_name).await; + match file { + Ok(file) => Ok(Some(file)), + Err(cloud_storage::Error::Other(err)) => { + if err.contains("No such object: ") { + Ok(None) + } else { + anyhow::bail!( + "[Indexer File] Error happens when downloading transaction file. {}", + err + ); + } + }, + Err(e) => Err(e.into()), + } +} + +async fn download_file(client: &Client, bucket_name: &str, file_name: &str) -> Result> +where + T: DeserializeOwned, +{ + let file = download_raw_file(client, bucket_name, file_name).await?; + match file { + Some(file) => { + let file = serde_json::from_slice(&file).context("Failed to parse file.")?; + Ok(Some(file)) + }, + None => Ok(None), + } +} diff --git a/execution/executor-types/Cargo.toml b/execution/executor-types/Cargo.toml index a3df75cd01ccf..b27f6f09efbdc 100644 --- a/execution/executor-types/Cargo.toml +++ b/execution/executor-types/Cargo.toml @@ -15,12 +15,14 @@ rust-version = { workspace = true } [dependencies] anyhow = { workspace = true } aptos-crypto = { workspace = true } +aptos-drop-helper = { workspace = true } aptos-scratchpad = { workspace = true } aptos-secure-net = { workspace = true } aptos-storage-interface = { workspace = true } aptos-types = { workspace = true } bcs = { workspace = true } criterion = { workspace = true } +derive_more = { workspace = true } itertools = { workspace = true } once_cell = { workspace = true } serde = { workspace = true } diff --git a/execution/executor-types/src/ledger_update_output.rs b/execution/executor-types/src/ledger_update_output.rs index beb86a5dd2959..f0928c8f4bf8f 100644 --- a/execution/executor-types/src/ledger_update_output.rs +++ b/execution/executor-types/src/ledger_update_output.rs @@ -6,6 +6,7 @@ use crate::StateComputeResult; use anyhow::{ensure, Result}; use aptos_crypto::HashValue; +use aptos_drop_helper::DropHelper; use aptos_storage_interface::cached_state_view::ShardedStateCache; use aptos_types::{ contract_event::ContractEvent, @@ -17,11 +18,79 @@ use aptos_types::{ Version, }, }; +use derive_more::Deref; use itertools::zip_eq; use std::sync::Arc; -#[derive(Default, Debug)] +#[derive(Clone, Debug, Default, Deref)] pub struct LedgerUpdateOutput { + #[deref] + inner: Arc>, +} + +impl LedgerUpdateOutput { + pub fn new_empty(transaction_accumulator: Arc) -> Self { + Self::new_impl(Inner::new_empty(transaction_accumulator)) + } + + #[cfg(any(test, feature = "fuzzing"))] + pub fn new_dummy_with_input_txns(txns: Vec) -> Self { + Self::new_impl(Inner::new_dummy_with_input_txns(txns)) + } + + #[cfg(any(test, feature = "fuzzing"))] + pub fn new_dummy_with_txns_to_commit(txns: Vec) -> Self { + Self::new_impl(Inner::new_dummy_with_txns_to_commit(txns)) + } + + pub fn new_dummy_with_root_hash(root_hash: HashValue) -> Self { + Self::new_impl(Inner::new_dummy_with_root_hash(root_hash)) + } + + pub fn reconfig_suffix(&self) -> Self { + Self::new_impl(Inner::new_empty(self.transaction_accumulator.clone())) + } + + pub fn new( + statuses_for_input_txns: Vec, + to_commit: Vec, + subscribable_events: Vec, + transaction_info_hashes: Vec, + state_updates_until_last_checkpoint: Option, + sharded_state_cache: ShardedStateCache, + transaction_accumulator: Arc, + parent_accumulator: Arc, + block_end_info: Option, + ) -> Self { + Self::new_impl(Inner { + statuses_for_input_txns, + to_commit, + subscribable_events, + transaction_info_hashes, + state_updates_until_last_checkpoint, + sharded_state_cache, + transaction_accumulator, + parent_accumulator, + block_end_info, + }) + } + + fn new_impl(inner: Inner) -> Self { + Self { + inner: Arc::new(DropHelper::new(inner)), + } + } + + pub fn as_state_compute_result( + &self, + next_epoch_state: Option, + ) -> StateComputeResult { + StateComputeResult::new(self.clone(), next_epoch_state) + } +} + +#[derive(Default, Debug)] +pub struct Inner { pub statuses_for_input_txns: Vec, pub to_commit: Vec, pub subscribable_events: Vec, @@ -31,20 +100,56 @@ pub struct LedgerUpdateOutput { /// The in-memory Merkle Accumulator representing a blockchain state consistent with the /// `state_tree`. pub transaction_accumulator: Arc, + pub parent_accumulator: Arc, pub block_end_info: Option, } -impl LedgerUpdateOutput { +impl Inner { pub fn new_empty(transaction_accumulator: Arc) -> Self { Self { + parent_accumulator: transaction_accumulator.clone(), transaction_accumulator, ..Default::default() } } - pub fn reconfig_suffix(&self) -> Self { + #[cfg(any(test, feature = "fuzzing"))] + pub fn new_dummy_with_input_txns(txns: Vec) -> Self { + let num_txns = txns.len(); + let to_commit = txns + .into_iter() + .chain(std::iter::once( + aptos_types::transaction::Transaction::StateCheckpoint(HashValue::zero()), + )) + .map(TransactionToCommit::dummy_with_transaction) + .collect(); Self { - transaction_accumulator: Arc::clone(&self.transaction_accumulator), + to_commit, + statuses_for_input_txns: vec![ + TransactionStatus::Keep( + aptos_types::transaction::ExecutionStatus::Success + ); + num_txns + ], + ..Default::default() + } + } + + #[cfg(any(test, feature = "fuzzing"))] + pub fn new_dummy_with_txns_to_commit(txns: Vec) -> Self { + Self { + to_commit: txns, + ..Default::default() + } + } + + pub fn new_dummy_with_root_hash(root_hash: HashValue) -> Self { + let transaction_accumulator = Arc::new( + InMemoryTransactionAccumulator::new_empty_with_root_hash(root_hash), + ); + Self { + parent_accumulator: transaction_accumulator.clone(), + transaction_accumulator, ..Default::default() } } @@ -98,27 +203,6 @@ impl LedgerUpdateOutput { Ok(()) } - pub fn as_state_compute_result( - &self, - parent_accumulator: &Arc, - next_epoch_state: Option, - ) -> StateComputeResult { - let txn_accu = self.txn_accumulator(); - - StateComputeResult::new( - txn_accu.root_hash(), - txn_accu.frozen_subtree_roots().clone(), - txn_accu.num_leaves(), - parent_accumulator.frozen_subtree_roots().clone(), - parent_accumulator.num_leaves(), - next_epoch_state, - self.statuses_for_input_txns.clone(), - self.transaction_info_hashes.clone(), - self.subscribable_events.clone(), - self.block_end_info.clone(), - ) - } - pub fn next_version(&self) -> Version { self.transaction_accumulator.num_leaves() as Version } diff --git a/execution/executor-types/src/lib.rs b/execution/executor-types/src/lib.rs index 8ae1701243ca2..3af3e304f6a90 100644 --- a/execution/executor-types/src/lib.rs +++ b/execution/executor-types/src/lib.rs @@ -18,19 +18,20 @@ use aptos_types::{ epoch_state::EpochState, jwks::OBSERVED_JWK_UPDATED_MOVE_TYPE_TAG, ledger_info::LedgerInfoWithSignatures, - proof::{AccumulatorExtensionProof, SparseMerkleProofExt}, + proof::{ + accumulator::InMemoryTransactionAccumulator, AccumulatorExtensionProof, + SparseMerkleProofExt, + }, state_store::{state_key::StateKey, state_value::StateValue}, transaction::{ - block_epilogue::{BlockEndInfo, BlockEpiloguePayload}, - ExecutionStatus, Transaction, TransactionInfo, TransactionListWithProof, - TransactionOutputListWithProof, TransactionStatus, Version, + Transaction, TransactionInfo, TransactionListWithProof, TransactionOutputListWithProof, + TransactionStatus, Version, }, write_set::WriteSet, }; pub use error::{ExecutorError, ExecutorResult}; pub use ledger_update_output::LedgerUpdateOutput; pub use parsed_transaction_output::ParsedTransactionOutput; -use serde::{Deserialize, Serialize}; use std::{ cmp::max, collections::{BTreeSet, HashMap}, @@ -287,102 +288,38 @@ pub struct ChunkCommitNotification { /// of success / failure of the transactions. /// Note that the specific details of compute_status are opaque to StateMachineReplication, /// which is going to simply pass the results between StateComputer and PayloadClient. -#[derive(Debug, Default, PartialEq, Eq, Clone, Serialize, Deserialize)] +#[derive(Debug, Default, Clone)] pub struct StateComputeResult { - /// transaction accumulator root hash is identified as `state_id` in Consensus. - root_hash: HashValue, - /// Represents the roots of all the full subtrees from left to right in this accumulator - /// after the execution. For details, please see [`InMemoryAccumulator`](aptos_types::proof::accumulator::InMemoryAccumulator). - frozen_subtree_roots: Vec, - - /// The frozen subtrees roots of the parent block, - parent_frozen_subtree_roots: Vec, - - /// The number of leaves of the transaction accumulator after executing a proposed block. - /// This state must be persisted to ensure that on restart that the version is calculated correctly. - num_leaves: u64, - - /// The number of leaves after executing the parent block, - parent_num_leaves: u64, - - /// If set, this is the new epoch info that should be changed to if this block is committed. - epoch_state: Option, - /// The compute status (success/failure) of the given payload. The specific details are opaque - /// for StateMachineReplication, which is merely passing it between StateComputer and - /// PayloadClient. - /// - /// Here, only input transactions statuses are kept, and in their order. - /// Input includes BlockMetadata, but doesn't include StateCheckpoint/BlockEpilogue - compute_status_for_input_txns: Vec, - - /// The transaction info hashes of all success txns. - transaction_info_hashes: Vec, - - subscribable_events: Vec, - - block_end_info: Option, + ledger_update_output: LedgerUpdateOutput, + /// If set, this is the new epoch info that should be changed to if this is committed. + next_epoch_state: Option, } impl StateComputeResult { pub fn new( - root_hash: HashValue, - frozen_subtree_roots: Vec, - num_leaves: u64, - parent_frozen_subtree_roots: Vec, - parent_num_leaves: u64, - epoch_state: Option, - compute_status_for_input_txns: Vec, - transaction_info_hashes: Vec, - subscribable_events: Vec, - block_end_info: Option, + ledger_update_output: LedgerUpdateOutput, + next_epoch_state: Option, ) -> Self { Self { - root_hash, - frozen_subtree_roots, - num_leaves, - parent_frozen_subtree_roots, - parent_num_leaves, - epoch_state, - compute_status_for_input_txns, - transaction_info_hashes, - subscribable_events, - block_end_info, + ledger_update_output, + next_epoch_state, } } - /// generate a new dummy state compute result with a given root hash. - /// this function is used in RandomComputeResultStateComputer to assert that the compute - /// function is really called. - pub fn new_dummy_with_root_hash(root_hash: HashValue) -> Self { + pub fn new_empty(transaction_accumulator: Arc) -> Self { Self { - root_hash, - frozen_subtree_roots: vec![], - num_leaves: 0, - parent_frozen_subtree_roots: vec![], - parent_num_leaves: 0, - epoch_state: None, - compute_status_for_input_txns: vec![], - transaction_info_hashes: vec![], - subscribable_events: vec![], - block_end_info: None, + ledger_update_output: LedgerUpdateOutput::new_empty(transaction_accumulator), + next_epoch_state: None, } } - pub fn new_dummy_with_num_txns(num_txns: usize) -> Self { + /// generate a new dummy state compute result with a given root hash. + /// this function is used in RandomComputeResultStateComputer to assert that the compute + /// function is really called. + pub fn new_dummy_with_root_hash(root_hash: HashValue) -> Self { Self { - root_hash: HashValue::zero(), - frozen_subtree_roots: vec![], - num_leaves: 0, - parent_frozen_subtree_roots: vec![], - parent_num_leaves: 0, - epoch_state: None, - compute_status_for_input_txns: vec![ - TransactionStatus::Keep(ExecutionStatus::Success); - num_txns - ], - transaction_info_hashes: vec![], - subscribable_events: vec![], - block_end_info: None, + ledger_update_output: LedgerUpdateOutput::new_dummy_with_root_hash(root_hash), + next_epoch_state: None, } } @@ -395,129 +332,70 @@ impl StateComputeResult { } #[cfg(any(test, feature = "fuzzing"))] - pub fn new_dummy_with_compute_status(compute_status: Vec) -> Self { - let mut ret = Self::new_dummy(); - ret.compute_status_for_input_txns = compute_status; - ret + pub fn new_dummy_with_input_txns(txns: Vec) -> Self { + Self { + ledger_update_output: LedgerUpdateOutput::new_dummy_with_input_txns(txns), + next_epoch_state: None, + } } pub fn version(&self) -> Version { - max(self.num_leaves, 1) + max(self.ledger_update_output.next_version(), 1) .checked_sub(1) .expect("Integer overflow occurred") } pub fn root_hash(&self) -> HashValue { - self.root_hash + self.ledger_update_output.transaction_accumulator.root_hash } pub fn compute_status_for_input_txns(&self) -> &Vec { - &self.compute_status_for_input_txns + &self.ledger_update_output.statuses_for_input_txns } pub fn transactions_to_commit_len(&self) -> usize { - self.compute_status_for_input_txns() - .iter() - .filter(|status| matches!(status, TransactionStatus::Keep(_))) - .count() - // StateCheckpoint/BlockEpilogue is added if there is no reconfiguration - + (if self.has_reconfiguration() { 0 } else { 1 }) + self.ledger_update_output.to_commit.len() } /// On top of input transactions (which contain BlockMetadata and Validator txns), /// filter out those that should be committed, and add StateCheckpoint/BlockEpilogue if needed. - pub fn transactions_to_commit( - &self, - input_txns: Vec, - block_id: HashValue, - ) -> Vec { - if self.is_reconfiguration_suffix() { - return vec![]; - } - - assert_eq!( - input_txns.len(), - self.compute_status_for_input_txns().len(), - "{:?} != {:?}", - input_txns.iter().map(|t| t.type_name()).collect::>(), - self.compute_status_for_input_txns() - ); - let output = itertools::zip_eq(input_txns, self.compute_status_for_input_txns()) - .filter_map(|(txn, status)| { - assert!( - !txn.is_non_reconfig_block_ending(), - "{:?}: {:?}", - txn, - status - ); - match status { - TransactionStatus::Keep(_) => Some(txn), - _ => None, - } - }) - .chain( - (!self.has_reconfiguration()).then_some(self.block_end_info.clone().map_or( - Transaction::StateCheckpoint(block_id), - |block_end_info| { - Transaction::BlockEpilogue(BlockEpiloguePayload::V0 { - block_id, - block_end_info, - }) - }, - )), - ) - .collect::>(); - - assert!( - self.has_reconfiguration() - || output - .last() - .map_or(false, Transaction::is_non_reconfig_block_ending), - "{:?}", - output.last() - ); - - output + pub fn transactions_to_commit(&self) -> Vec { + self.ledger_update_output + .to_commit + .iter() + .map(|t| t.transaction.clone()) + .collect() } pub fn epoch_state(&self) -> &Option { - &self.epoch_state + &self.next_epoch_state } pub fn extension_proof(&self) -> AccumulatorExtensionProof { - AccumulatorExtensionProof::::new( - self.parent_frozen_subtree_roots.clone(), - self.parent_num_leaves(), - self.transaction_info_hashes().clone(), + AccumulatorExtensionProof::new( + self.ledger_update_output + .transaction_accumulator + .frozen_subtree_roots + .clone(), + self.ledger_update_output.transaction_accumulator.num_leaves, + self.transaction_info_hashes().to_vec(), ) } pub fn transaction_info_hashes(&self) -> &Vec { - &self.transaction_info_hashes + &self.ledger_update_output.transaction_info_hashes } pub fn num_leaves(&self) -> u64 { - self.num_leaves - } - - pub fn frozen_subtree_roots(&self) -> &Vec { - &self.frozen_subtree_roots - } - - pub fn parent_num_leaves(&self) -> u64 { - self.parent_num_leaves - } - - pub fn parent_frozen_subtree_roots(&self) -> &Vec { - &self.parent_frozen_subtree_roots + self.ledger_update_output.next_version() } pub fn has_reconfiguration(&self) -> bool { - self.epoch_state.is_some() + self.next_epoch_state.is_some() } pub fn subscribable_events(&self) -> &[ContractEvent] { - &self.subscribable_events + &self.ledger_update_output.subscribable_events } pub fn is_reconfiguration_suffix(&self) -> bool { diff --git a/execution/executor/src/block_executor.rs b/execution/executor/src/block_executor.rs index ae8ff5ee0035b..7cbc3811f718f 100644 --- a/execution/executor/src/block_executor.rs +++ b/execution/executor/src/block_executor.rs @@ -314,10 +314,7 @@ where return Ok(current_output .output .get_ledger_update() - .as_state_compute_result( - parent_accumulator, - current_output.output.epoch_state().clone(), - )); + .as_state_compute_result(current_output.output.epoch_state().clone())); } let output = @@ -341,10 +338,8 @@ where output.ensure_ends_with_state_checkpoint()?; } - let state_compute_result = output.as_state_compute_result( - parent_accumulator, - current_output.output.epoch_state().clone(), - ); + let state_compute_result = + output.as_state_compute_result(current_output.output.epoch_state().clone()); current_output.output.set_ledger_update(output); Ok(state_compute_result) } @@ -379,8 +374,7 @@ where parent_block.output.state().base_version, false, result_in_memory_state, - // TODO(grao): Avoid this clone. - ledger_update.state_updates_until_last_checkpoint.clone(), + ledger_update.state_updates_until_last_checkpoint.as_ref(), Some(&ledger_update.sharded_state_cache), )?; TRANSACTIONS_SAVED.observe(ledger_update.num_txns() as f64); diff --git a/execution/executor/src/chunk_executor.rs b/execution/executor/src/chunk_executor.rs index d54497c000ca9..c826ceb7fcc56 100644 --- a/execution/executor/src/chunk_executor.rs +++ b/execution/executor/src/chunk_executor.rs @@ -264,11 +264,10 @@ impl ChunkExecutorInner { chunk.ledger_info.as_ref(), false, // sync_commit chunk.result_state.clone(), - // TODO(aldenhu): avoid cloning chunk .ledger_update_output .state_updates_until_last_checkpoint - .clone(), + .as_ref(), Some(&chunk.ledger_update_output.sharded_state_cache), )?; } diff --git a/execution/executor/src/components/apply_chunk_output.rs b/execution/executor/src/components/apply_chunk_output.rs index cf018e8ea9044..ba358ec2b9caa 100644 --- a/execution/executor/src/components/apply_chunk_output.rs +++ b/execution/executor/src/components/apply_chunk_output.rs @@ -149,16 +149,17 @@ impl ApplyChunkOutput { let transaction_accumulator = Arc::new(base_txn_accumulator.append(&transaction_info_hashes)); Ok(( - LedgerUpdateOutput { + LedgerUpdateOutput::new( statuses_for_input_txns, - to_commit: txns_to_commit, + txns_to_commit, subscribable_events, transaction_info_hashes, - state_updates_until_last_checkpoint: state_updates_before_last_checkpoint, + state_updates_before_last_checkpoint, sharded_state_cache, transaction_accumulator, + base_txn_accumulator, block_end_info, - }, + ), to_discard.into_txns(), to_retry.into_txns(), )) diff --git a/execution/executor/src/components/executed_chunk.rs b/execution/executor/src/components/executed_chunk.rs index 28e8e68fcf87e..57009d3ec33c9 100644 --- a/execution/executor/src/components/executed_chunk.rs +++ b/execution/executor/src/components/executed_chunk.rs @@ -4,15 +4,10 @@ #![forbid(unsafe_code)] -use aptos_drop_helper::DEFAULT_DROPPER; use aptos_executor_types::{ should_forward_to_subscription_service, ChunkCommitNotification, LedgerUpdateOutput, }; use aptos_storage_interface::{state_delta::StateDelta, ExecutedTrees}; -#[cfg(test)] -use aptos_types::account_config::NewEpochEvent; -#[cfg(test)] -use aptos_types::contract_event::ContractEvent; use aptos_types::{ epoch_state::EpochState, ledger_info::LedgerInfoWithSignatures, transaction::TransactionToCommit, @@ -50,24 +45,20 @@ impl ExecutedChunk { Vec::with_capacity(self.ledger_update_output.to_commit.len()); let mut subscribable_events = Vec::with_capacity(self.ledger_update_output.to_commit.len() * 2); - let mut to_drop = Vec::with_capacity(self.ledger_update_output.to_commit.len()); - for txn_to_commit in self.ledger_update_output.to_commit { + for txn_to_commit in &self.ledger_update_output.to_commit { let TransactionToCommit { transaction, events, - state_updates, - write_set, .. } = txn_to_commit; - committed_transactions.push(transaction); + committed_transactions.push(transaction.clone()); subscribable_events.extend( events - .into_iter() - .filter(should_forward_to_subscription_service), + .iter() + .filter(|evt| should_forward_to_subscription_service(evt)) + .cloned(), ); - to_drop.push((state_updates, write_set)); } - DEFAULT_DROPPER.schedule_drop(to_drop); ChunkCommitNotification { committed_transactions, @@ -89,6 +80,7 @@ impl ExecutedChunk { #[test] fn into_chunk_commit_notification_should_apply_event_filters() { + use aptos_types::{account_config::NewEpochEvent, contract_event::ContractEvent}; let event_1 = ContractEvent::new_v2_with_type_tag_str( "0x2345::random_module::RandomEvent", b"random_data_x".to_vec(), @@ -101,14 +93,11 @@ fn into_chunk_commit_notification_should_apply_event_filters() { ); let event_4 = ContractEvent::from((1, NewEpochEvent::dummy())); - let ledger_update_output = LedgerUpdateOutput { - to_commit: vec![ - TransactionToCommit::dummy_with_events(vec![event_1.clone()]), - TransactionToCommit::dummy_with_events(vec![event_2.clone(), event_3.clone()]), - TransactionToCommit::dummy_with_events(vec![event_4.clone()]), - ], - ..Default::default() - }; + let ledger_update_output = LedgerUpdateOutput::new_dummy_with_txns_to_commit(vec![ + TransactionToCommit::dummy_with_events(vec![event_1.clone()]), + TransactionToCommit::dummy_with_events(vec![event_2.clone(), event_3.clone()]), + TransactionToCommit::dummy_with_events(vec![event_4.clone()]), + ]); let chunk = ExecutedChunk { ledger_update_output, diff --git a/execution/executor/src/db_bootstrapper.rs b/execution/executor/src/db_bootstrapper.rs index 5015a48e84b9c..c27d91bef7375 100644 --- a/execution/executor/src/db_bootstrapper.rs +++ b/execution/executor/src/db_bootstrapper.rs @@ -112,7 +112,7 @@ impl GenesisCommitter { self.output .ledger_update_output .state_updates_until_last_checkpoint - .clone(), + .as_ref(), Some(&self.output.ledger_update_output.sharded_state_cache), )?; info!("Genesis commited."); diff --git a/execution/executor/src/fuzzing.rs b/execution/executor/src/fuzzing.rs index 58ae0bfac412a..d801df7234792 100644 --- a/execution/executor/src/fuzzing.rs +++ b/execution/executor/src/fuzzing.rs @@ -120,7 +120,7 @@ impl DbWriter for FakeDb { _base_state_version: Option, _sync_commit: bool, _latest_in_memory_state: StateDelta, - _state_updates_until_last_checkpoint: Option, + _state_updates_until_last_checkpoint: Option<&ShardedStateUpdates>, _sharded_state_cache: Option<&ShardedStateCache>, ) -> aptos_storage_interface::Result<()> { Ok(()) diff --git a/execution/executor/src/tests/mod.rs b/execution/executor/src/tests/mod.rs index c50d2480a1b84..89b1e68dae89e 100644 --- a/execution/executor/src/tests/mod.rs +++ b/execution/executor/src/tests/mod.rs @@ -14,8 +14,7 @@ use crate::{ use aptos_crypto::{ed25519::Ed25519PrivateKey, HashValue, PrivateKey, SigningKey, Uniform}; use aptos_db::AptosDB; use aptos_executor_types::{ - BlockExecutorTrait, ChunkExecutorTrait, LedgerUpdateOutput, TransactionReplayer, - VerifyExecutionMode, + BlockExecutorTrait, ChunkExecutorTrait, TransactionReplayer, VerifyExecutionMode, }; use aptos_storage_interface::{ async_proof_fetcher::AsyncProofFetcher, DbReaderWriter, ExecutedTrees, Result, @@ -311,7 +310,7 @@ fn test_executor_execute_same_block_multiple_times() { .collect(); let mut responses = vec![]; - for _i in 0..100 { + for _i in 0..10 { let output = executor .execute_block( (block_id, block(txns.clone())).into(), @@ -321,8 +320,14 @@ fn test_executor_execute_same_block_multiple_times() { .unwrap(); responses.push(output); } - responses.dedup(); - assert_eq!(responses.len(), 1); + assert_eq!( + responses + .iter() + .map(|output| output.root_hash()) + .dedup() + .count(), + 1, + ); } fn create_blocks_and_chunks( @@ -494,27 +499,19 @@ fn apply_transaction_by_writeset( next_epoch_state: _, ledger_update_output, } = executed; - let LedgerUpdateOutput { - statuses_for_input_txns: _, - to_commit, - subscribable_events: _, - transaction_info_hashes: _, - state_updates_until_last_checkpoint: state_updates_before_last_checkpoint, - sharded_state_cache, - transaction_accumulator: _, - block_end_info: _, - } = ledger_update_output; db.writer .save_transactions( - &to_commit, + &ledger_update_output.to_commit, ledger_view.txn_accumulator().num_leaves(), ledger_view.state().base_version, ledger_info.as_ref(), true, /* sync_commit */ result_state, - state_updates_before_last_checkpoint, - Some(&sharded_state_cache), + ledger_update_output + .state_updates_until_last_checkpoint + .as_ref(), + Some(&ledger_update_output.sharded_state_cache), ) .unwrap(); } @@ -714,26 +711,18 @@ fn run_transactions_naive( next_epoch_state: _, ledger_update_output, } = executed; - let LedgerUpdateOutput { - statuses_for_input_txns: _, - to_commit, - subscribable_events: _, - transaction_info_hashes: _, - state_updates_until_last_checkpoint: state_updates_before_last_checkpoint, - sharded_state_cache, - transaction_accumulator: _, - block_end_info: _, - } = ledger_update_output; db.writer .save_transactions( - &to_commit, + &ledger_update_output.to_commit, ledger_view.txn_accumulator().num_leaves(), ledger_view.state().base_version, ledger_info.as_ref(), true, /* sync_commit */ result_state, - state_updates_before_last_checkpoint, - Some(&sharded_state_cache), + ledger_update_output + .state_updates_until_last_checkpoint + .as_ref(), + Some(&ledger_update_output.sharded_state_cache), ) .unwrap(); ledger_view = next_ledger_view; diff --git a/state-sync/inter-component/consensus-notifications/src/lib.rs b/state-sync/inter-component/consensus-notifications/src/lib.rs index 3d596f7b542a6..5a6682057a1fa 100644 --- a/state-sync/inter-component/consensus-notifications/src/lib.rs +++ b/state-sync/inter-component/consensus-notifications/src/lib.rs @@ -43,6 +43,14 @@ pub trait ConsensusNotificationSender: Send + Sync { subscribable_events: Vec, ) -> Result<(), Error>; + /// Notifies state sync to synchronize storage for at least the specified duration, + /// and returns the latest synced ledger info. Note that state sync may synchronize + /// for much longer than the specified duration, e.g., if the node is very far behind. + async fn sync_for_duration( + &self, + duration: Duration, + ) -> Result; + /// Notify state sync to synchronize storage to the specified target. async fn sync_to_target(&self, target: LedgerInfoWithSignatures) -> Result<(), Error>; } @@ -93,14 +101,10 @@ impl ConsensusNotificationSender for ConsensusNotifier { return Ok(()); } - // Construct a channel to receive a state sync response - let (callback, callback_receiver) = oneshot::channel(); - let commit_notification = - ConsensusNotification::NotifyCommit(ConsensusCommitNotification { - transactions, - subscribable_events, - callback, - }); + // Create a consensus commit notification + let (notification, callback_receiver) = + ConsensusCommitNotification::new(transactions, subscribable_events); + let commit_notification = ConsensusNotification::NotifyCommit(notification); // Send the notification to state sync if let Err(error) = self @@ -123,25 +127,68 @@ impl ConsensusNotificationSender for ConsensusNotifier { .await { match response { - Ok(consensus_notification_response) => consensus_notification_response.result, - Err(error) => Err(Error::UnexpectedErrorEncountered(format!("{:?}", error))), + Ok(consensus_notification_response) => consensus_notification_response.get_result(), + Err(error) => Err(Error::UnexpectedErrorEncountered(format!( + "Consensus commit notification failure: {:?}", + error + ))), } } else { Err(Error::TimeoutWaitingForStateSync) } } + async fn sync_for_duration( + &self, + duration: Duration, + ) -> Result { + // Create a consensus sync duration notification + let (notification, callback_receiver) = ConsensusSyncDurationNotification::new(duration); + let sync_duration_notification = ConsensusNotification::SyncForDuration(notification); + + // Send the notification to state sync + if let Err(error) = self + .notification_sender + .clone() + .send(sync_duration_notification) + .await + { + return Err(Error::NotificationError(format!( + "Failed to notify state sync of sync duration! Error: {:?}", + error + ))); + } + + // Process the response + match callback_receiver.await { + Ok(response) => match response.get_result() { + Ok(_) => response.get_latest_synced_ledger_info().ok_or_else(|| { + Error::UnexpectedErrorEncountered( + "Sync for duration returned an empty latest synced ledger info!".into(), + ) + }), + Err(error) => Err(Error::UnexpectedErrorEncountered(format!( + "Sync for duration returned an error: {:?}", + error + ))), + }, + Err(error) => Err(Error::UnexpectedErrorEncountered(format!( + "Sync for duration failure: {:?}", + error + ))), + } + } + async fn sync_to_target(&self, target: LedgerInfoWithSignatures) -> Result<(), Error> { - // Construct a channel to receive a state sync response - let (callback, callback_receiver) = oneshot::channel(); - let sync_notification = - ConsensusNotification::SyncToTarget(ConsensusSyncNotification { target, callback }); + // Create a consensus sync target notification + let (notification, callback_receiver) = ConsensusSyncTargetNotification::new(target); + let sync_target_notification = ConsensusNotification::SyncToTarget(notification); // Send the notification to state sync if let Err(error) = self .notification_sender .clone() - .send(sync_notification) + .send(sync_target_notification) .await { return Err(Error::NotificationError(format!( @@ -152,8 +199,11 @@ impl ConsensusNotificationSender for ConsensusNotifier { // Process the response match callback_receiver.await { - Ok(response) => response.result, - Err(error) => Err(Error::UnexpectedErrorEncountered(format!("{:?}", error))), + Ok(response) => response.get_result(), + Err(error) => Err(Error::UnexpectedErrorEncountered(format!( + "Sync to target failure: {:?}", + error + ))), } } } @@ -171,29 +221,52 @@ impl ConsensusNotificationListener { } } + /// Sends the specified result to the given callback + fn send_result_to_callback( + &self, + callback: oneshot::Sender, + result: Result<(), Error>, + ) -> Result<(), Error> { + callback + .send(ConsensusNotificationResponse::new(result)) + .map_err(|error| Error::UnexpectedErrorEncountered(format!("{:?}", error))) + } + /// Respond to the commit notification - pub async fn respond_to_commit_notification( - &mut self, + pub fn respond_to_commit_notification( + &self, consensus_commit_notification: ConsensusCommitNotification, result: Result<(), Error>, ) -> Result<(), Error> { - consensus_commit_notification - .callback - .send(ConsensusNotificationResponse { result }) - .map_err(|error| Error::UnexpectedErrorEncountered(format!("{:?}", error))) + self.send_result_to_callback(consensus_commit_notification.callback, result) } - /// Respond to the sync notification - pub async fn respond_to_sync_notification( - &mut self, - consensus_sync_notification: ConsensusSyncNotification, + /// Respond to the sync duration notification + pub fn respond_to_sync_duration_notification( + &self, + sync_duration_notification: ConsensusSyncDurationNotification, result: Result<(), Error>, + latest_synced_ledger_info: Option, ) -> Result<(), Error> { - consensus_sync_notification + // Create a new response with the result and latest synced ledger info + let response = + ConsensusNotificationResponse::new_with_ledger_info(result, latest_synced_ledger_info); + + // Send the response to the callback + sync_duration_notification .callback - .send(ConsensusNotificationResponse { result }) + .send(response) .map_err(|error| Error::UnexpectedErrorEncountered(format!("{:?}", error))) } + + /// Respond to the sync target notification + pub fn respond_to_sync_target_notification( + &self, + sync_target_notification: ConsensusSyncTargetNotification, + result: Result<(), Error>, + ) -> Result<(), Error> { + self.send_result_to_callback(sync_target_notification.callback, result) + } } impl Stream for ConsensusNotificationListener { @@ -213,15 +286,16 @@ impl FusedStream for ConsensusNotificationListener { #[derive(Debug)] pub enum ConsensusNotification { NotifyCommit(ConsensusCommitNotification), - SyncToTarget(ConsensusSyncNotification), + SyncForDuration(ConsensusSyncDurationNotification), + SyncToTarget(ConsensusSyncTargetNotification), } /// A commit notification to notify state sync of new commits #[derive(Debug)] pub struct ConsensusCommitNotification { - pub transactions: Vec, - pub subscribable_events: Vec, - pub(crate) callback: oneshot::Sender, + transactions: Vec, + subscribable_events: Vec, + callback: oneshot::Sender, } impl ConsensusCommitNotification { @@ -238,29 +312,93 @@ impl ConsensusCommitNotification { (commit_notification, callback_receiver) } + + /// Returns a reference to the transactions + pub fn get_transactions(&self) -> &Vec { + &self.transactions + } + + /// Returns a reference to the subscribable events + pub fn get_subscribable_events(&self) -> &Vec { + &self.subscribable_events + } } -/// The result returned by state sync for a consensus or consensus observer notification +/// The response returned by state sync for a consensus or consensus observer notification #[derive(Debug)] pub struct ConsensusNotificationResponse { - pub result: Result<(), Error>, + result: Result<(), Error>, + latest_synced_ledger_info: Option, +} + +impl ConsensusNotificationResponse { + pub fn new(result: Result<(), Error>) -> Self { + Self::new_with_ledger_info(result, None) + } + + /// Returns a new response with the given result and latest synced ledger info + pub fn new_with_ledger_info( + result: Result<(), Error>, + latest_synced_ledger_info: Option, + ) -> Self { + Self { + result, + latest_synced_ledger_info, + } + } + + /// Returns a copy of the result + pub fn get_result(&self) -> Result<(), Error> { + self.result.clone() + } + + /// Returns a copy of the latest synced ledger info + pub fn get_latest_synced_ledger_info(&self) -> Option { + self.latest_synced_ledger_info.clone() + } +} + +/// A notification for state sync to synchronize for the specified duration +#[derive(Debug)] +pub struct ConsensusSyncDurationNotification { + duration: Duration, + callback: oneshot::Sender, +} + +impl ConsensusSyncDurationNotification { + pub fn new(duration: Duration) -> (Self, oneshot::Receiver) { + let (callback, callback_receiver) = oneshot::channel(); + let notification = ConsensusSyncDurationNotification { duration, callback }; + + (notification, callback_receiver) + } + + /// Returns the duration of the notification + pub fn get_duration(&self) -> Duration { + self.duration + } } /// A notification for state sync to synchronize to the given target #[derive(Debug)] -pub struct ConsensusSyncNotification { - pub target: LedgerInfoWithSignatures, - pub(crate) callback: oneshot::Sender, +pub struct ConsensusSyncTargetNotification { + target: LedgerInfoWithSignatures, + callback: oneshot::Sender, } -impl ConsensusSyncNotification { +impl ConsensusSyncTargetNotification { pub fn new( target: LedgerInfoWithSignatures, ) -> (Self, oneshot::Receiver) { let (callback, callback_receiver) = oneshot::channel(); - let sync_notification = ConsensusSyncNotification { target, callback }; + let notification = ConsensusSyncTargetNotification { target, callback }; - (sync_notification, callback_receiver) + (notification, callback_receiver) + } + + /// Returns a reference to the target of the notification + pub fn get_target(&self) -> &LedgerInfoWithSignatures { + &self.target } } @@ -338,8 +476,11 @@ mod tests { match consensus_listener.select_next_some().now_or_never() { Some(consensus_notification) => match consensus_notification { ConsensusNotification::NotifyCommit(commit_notification) => { - assert_eq!(transactions, commit_notification.transactions); - assert_eq!(subscribable_events, commit_notification.subscribable_events); + assert_eq!(transactions, commit_notification.get_transactions().clone()); + assert_eq!( + subscribable_events, + commit_notification.get_subscribable_events().clone() + ); }, result => panic!( "Expected consensus commit notification but got: {:?}", @@ -349,9 +490,10 @@ mod tests { result => panic!("Expected consensus notification but got: {:?}", result), }; - // Send a sync notification + // Send a sync target notification + let notifier = consensus_notifier.clone(); let _thread = std::thread::spawn(move || { - let _result = block_on(consensus_notifier.sync_to_target(create_ledger_info())); + let _result = block_on(notifier.sync_to_target(create_ledger_info())); }); // Give the thread enough time to spawn and send the notification @@ -361,9 +503,34 @@ mod tests { match consensus_listener.select_next_some().now_or_never() { Some(consensus_notification) => match consensus_notification { ConsensusNotification::SyncToTarget(sync_notification) => { - assert_eq!(create_ledger_info(), sync_notification.target); + assert_eq!(create_ledger_info(), sync_notification.get_target().clone()); + }, + result => panic!( + "Expected consensus sync target notification but got: {:?}", + result + ), + }, + result => panic!("Expected consensus notification but got: {:?}", result), + }; + + // Send a sync duration notification + let _thread = std::thread::spawn(move || { + let _result = block_on(consensus_notifier.sync_for_duration(Duration::from_secs(10))); + }); + + // Give the thread enough time to spawn and send the notification + std::thread::sleep(Duration::from_millis(1000)); + + // Verify the notification arrives at the receiver + match consensus_listener.select_next_some().now_or_never() { + Some(consensus_notification) => match consensus_notification { + ConsensusNotification::SyncForDuration(sync_notification) => { + assert_eq!(Duration::from_secs(10), sync_notification.duration); }, - result => panic!("Expected consensus sync notification but got: {:?}", result), + result => panic!( + "Expected consensus sync duration notification but got: {:?}", + result + ), }, result => panic!("Expected consensus notification but got: {:?}", result), }; @@ -381,16 +548,25 @@ mod tests { let _handler = std::thread::spawn(move || loop { match consensus_listener.select_next_some().now_or_never() { Some(ConsensusNotification::NotifyCommit(commit_notification)) => { - let _result = block_on( - consensus_listener - .respond_to_commit_notification(commit_notification, Ok(())), - ); + let _result = consensus_listener + .respond_to_commit_notification(commit_notification, Ok(())); }, Some(ConsensusNotification::SyncToTarget(sync_notification)) => { - let _result = block_on(consensus_listener.respond_to_sync_notification( + let _result = consensus_listener.respond_to_sync_target_notification( sync_notification, - Err(Error::UnexpectedErrorEncountered("Oops?".into())), - )); + Err(Error::UnexpectedErrorEncountered( + "Oops! Sync to target failed!".into(), + )), + ); + }, + Some(ConsensusNotification::SyncForDuration(sync_notification)) => { + let _result = consensus_listener.respond_to_sync_duration_notification( + sync_notification, + Err(Error::UnexpectedErrorEncountered( + "Oops! Sync for duration failed!".into(), + )), + None, + ); }, _ => { /* Do nothing */ }, } @@ -401,9 +577,13 @@ mod tests { block_on(consensus_notifier.notify_new_commit(vec![create_user_transaction()], vec![])); assert_ok!(notify_result); - // Send a sync notification and very an error response + // Send a sync target notification and verify an error response let notify_result = block_on(consensus_notifier.sync_to_target(create_ledger_info())); assert_err!(notify_result); + + // Send a sync duration notification and verify an error response + let notify_result = block_on(consensus_notifier.sync_for_duration(Duration::from_secs(10))); + assert_err!(notify_result); } fn create_user_transaction() -> Transaction { diff --git a/state-sync/inter-component/event-notifications/src/lib.rs b/state-sync/inter-component/event-notifications/src/lib.rs index 6bf500a05afba..4ff95eaf18b8d 100644 --- a/state-sync/inter-component/event-notifications/src/lib.rs +++ b/state-sync/inter-component/event-notifications/src/lib.rs @@ -36,7 +36,7 @@ mod tests; // consumed, they will be dropped (oldest messages first). The remaining messages // will be retrieved using FIFO ordering. const EVENT_NOTIFICATION_CHANNEL_SIZE: usize = 100; -const RECONFIG_NOTIFICATION_CHANNEL_SIZE: usize = 1; +const RECONFIG_NOTIFICATION_CHANNEL_SIZE: usize = 1; // Note: this should be 1 to ensure only the latest reconfig is consumed #[derive(Clone, Debug, Deserialize, Error, PartialEq, Eq, Serialize)] pub enum Error { diff --git a/state-sync/state-sync-driver/src/continuous_syncer.rs b/state-sync/state-sync-driver/src/continuous_syncer.rs index d8da9a4496d52..84727902d2ae3 100644 --- a/state-sync/state-sync-driver/src/continuous_syncer.rs +++ b/state-sync/state-sync-driver/src/continuous_syncer.rs @@ -116,7 +116,7 @@ impl< let sync_request_target = consensus_sync_request .lock() .as_ref() - .map(|sync_request| sync_request.get_sync_target()); + .and_then(|sync_request| sync_request.get_sync_target()); // Initialize a new active data stream let active_data_stream = match self.get_continuous_syncing_mode() { @@ -432,7 +432,7 @@ impl< let sync_request_target = consensus_sync_request .lock() .as_ref() - .map(|sync_request| sync_request.get_sync_target()); + .and_then(|sync_request| sync_request.get_sync_target()); if let Some(sync_request_target) = sync_request_target { let sync_request_version = sync_request_target.ledger_info().version(); let proof_version = ledger_info_with_signatures.ledger_info().version(); diff --git a/state-sync/state-sync-driver/src/driver.rs b/state-sync/state-sync-driver/src/driver.rs index 832fe54e6b7fe..68481cdb354bf 100644 --- a/state-sync/state-sync-driver/src/driver.rs +++ b/state-sync/state-sync-driver/src/driver.rs @@ -22,7 +22,8 @@ use crate::{ }; use aptos_config::config::{ConsensusObserverConfig, RoleType, StateSyncDriverConfig}; use aptos_consensus_notifications::{ - ConsensusCommitNotification, ConsensusNotification, ConsensusSyncNotification, + ConsensusCommitNotification, ConsensusNotification, ConsensusSyncDurationNotification, + ConsensusSyncTargetNotification, }; use aptos_data_client::interface::AptosDataClientInterface; use aptos_data_streaming_service::streaming_client::{ @@ -45,6 +46,7 @@ use tokio::{ use tokio_stream::wrappers::IntervalStream; // Useful constants for the driver +const DRIVER_INFO_LOG_FREQ_SECS: u64 = 2; const DRIVER_ERROR_LOG_FREQ_SECS: u64 = 3; /// The configuration of the state sync driver @@ -264,14 +266,21 @@ impl< ConsensusNotification::NotifyCommit(commit_notification) => { let _ = self .consensus_notification_handler - .respond_to_commit_notification(commit_notification, Err(error.clone())) - .await; + .respond_to_commit_notification(commit_notification, Err(error.clone())); }, ConsensusNotification::SyncToTarget(sync_notification) => { let _ = self .consensus_notification_handler - .respond_to_sync_notification(sync_notification, Err(error.clone())) - .await; + .respond_to_sync_target_notification(sync_notification, Err(error.clone())); + }, + ConsensusNotification::SyncForDuration(sync_notification) => { + let _ = self + .consensus_notification_handler + .respond_to_sync_duration_notification( + sync_notification, + Err(error.clone()), + None, + ); }, } warn!(LogSchema::new(LogEntry::ConsensusNotification) @@ -287,7 +296,11 @@ impl< .await }, ConsensusNotification::SyncToTarget(sync_notification) => { - self.handle_consensus_sync_notification(sync_notification) + self.handle_consensus_sync_target_notification(sync_notification) + .await + }, + ConsensusNotification::SyncForDuration(sync_notification) => { + self.handle_consensus_sync_duration_notification(sync_notification) .await }, }; @@ -303,21 +316,21 @@ impl< /// Handles a commit notification sent by consensus or consensus observer async fn handle_consensus_commit_notification( &mut self, - consensus_commit_notification: ConsensusCommitNotification, + commit_notification: ConsensusCommitNotification, ) -> Result<(), Error> { info!( LogSchema::new(LogEntry::ConsensusNotification).message(&format!( "Received a consensus commit notification! Total transactions: {:?}, events: {:?}", - consensus_commit_notification.transactions.len(), - consensus_commit_notification.subscribable_events.len() + commit_notification.get_transactions().len(), + commit_notification.get_subscribable_events().len() )) ); - self.update_consensus_commit_metrics(&consensus_commit_notification); + self.update_consensus_commit_metrics(&commit_notification); // Handle the commit notification let committed_transactions = CommittedTransactions { - events: consensus_commit_notification.subscribable_events.clone(), - transactions: consensus_commit_notification.transactions.clone(), + events: commit_notification.get_subscribable_events().clone(), + transactions: commit_notification.get_transactions().clone(), }; utils::handle_committed_transactions( committed_transactions, @@ -330,8 +343,7 @@ impl< // Respond successfully self.consensus_notification_handler - .respond_to_commit_notification(consensus_commit_notification, Ok(())) - .await?; + .respond_to_commit_notification(commit_notification, Ok(()))?; // Check the progress of any sync requests. We need this here because // consensus might issue a sync request and then commit (asynchronously). @@ -359,13 +371,13 @@ impl< metrics::increment_gauge( &metrics::STORAGE_SYNCHRONIZER_OPERATIONS, operation.get_label(), - consensus_commit_notification.transactions.len() as u64, + consensus_commit_notification.get_transactions().len() as u64, ); } // Update the synced epoch if consensus_commit_notification - .subscribable_events + .get_subscribable_events() .iter() .any(ContractEvent::is_new_epoch_event) { @@ -373,28 +385,53 @@ impl< } } + /// Handles a consensus or consensus observer request to sync for a specified duration + async fn handle_consensus_sync_duration_notification( + &mut self, + sync_duration_notification: ConsensusSyncDurationNotification, + ) -> Result<(), Error> { + // Update the sync duration notification metrics + let latest_synced_version = utils::fetch_pre_committed_version(self.storage.clone())?; + info!( + LogSchema::new(LogEntry::ConsensusNotification).message(&format!( + "Received a consensus sync duration notification! Duration: {:?}. Latest synced version: {:?}", + sync_duration_notification.get_duration(), latest_synced_version, + )) + ); + metrics::increment_counter( + &metrics::DRIVER_COUNTERS, + metrics::DRIVER_CONSENSUS_SYNC_DURATION_NOTIFICATION, + ); + + // Initialize a new sync request + self.consensus_notification_handler + .initialize_sync_duration_request(sync_duration_notification) + .await + } + /// Handles a consensus or consensus observer request to sync to a specified target - async fn handle_consensus_sync_notification( + async fn handle_consensus_sync_target_notification( &mut self, - sync_notification: ConsensusSyncNotification, + sync_target_notification: ConsensusSyncTargetNotification, ) -> Result<(), Error> { + // Update the sync target notification metrics let latest_synced_version = utils::fetch_pre_committed_version(self.storage.clone())?; info!( LogSchema::new(LogEntry::ConsensusNotification).message(&format!( - "Received a consensus sync notification! Target version: {:?}. Latest synced version: {:?}", - sync_notification.target, latest_synced_version, + "Received a consensus sync target notification! Target version: {:?}. Latest synced version: {:?}", + sync_target_notification.get_target(), latest_synced_version, )) ); metrics::increment_counter( &metrics::DRIVER_COUNTERS, - metrics::DRIVER_CONSENSUS_SYNC_NOTIFICATION, + metrics::DRIVER_CONSENSUS_SYNC_TARGET_NOTIFICATION, ); // Initialize a new sync request let latest_synced_ledger_info = utils::fetch_latest_synced_ledger_info(self.storage.clone())?; self.consensus_notification_handler - .initialize_sync_request(sync_notification, latest_synced_ledger_info) + .initialize_sync_target_request(sync_target_notification, latest_synced_ledger_info) .await } @@ -489,31 +526,27 @@ impl< }; } - /// Checks if the node has successfully reached the sync target + /// Checks if the node has successfully reached the sync target or duration async fn check_sync_request_progress(&mut self) -> Result<(), Error> { - if !self.active_sync_request() { - return Ok(()); // There's no pending sync request - } - - // There's a sync request. Fetch it and check if we're still behind the target. - let sync_request = self.consensus_notification_handler.get_sync_request(); - let sync_target_version = sync_request - .lock() - .as_ref() - .ok_or_else(|| { - Error::UnexpectedError( - "We've already verified there is an active sync request!".into(), - ) - })? - .get_sync_target_version(); - let latest_synced_ledger_info = - utils::fetch_latest_synced_ledger_info(self.storage.clone())?; - if latest_synced_ledger_info.ledger_info().version() < sync_target_version { - return Ok(()); + // Check if the sync request has been satisfied + let consensus_sync_request = self.consensus_notification_handler.get_sync_request(); + match consensus_sync_request.lock().as_ref() { + Some(consensus_sync_request) => { + let latest_synced_ledger_info = + utils::fetch_latest_synced_ledger_info(self.storage.clone())?; + if !consensus_sync_request + .sync_request_satisfied(&latest_synced_ledger_info, self.time_service.clone()) + { + return Ok(()); // The sync request hasn't been satisfied yet + } + }, + None => { + return Ok(()); // There's no active sync request + }, } - // Wait for the storage synchronizer to drain (if it hasn't already). - // This prevents notifying consensus prematurely. + // The sync request has been satisfied. Wait for the storage synchronizer + // to drain. This prevents notifying consensus prematurely. while self.storage_synchronizer.pending_storage_data() { sample!( SampleRate::Duration(Duration::from_secs(PENDING_DATA_LOG_FREQ_SECS)), @@ -524,11 +557,39 @@ impl< yield_now().await; } - // Refresh the latest synced ledger info and handle the sync request + // If the request was to sync for a specified duration, we should only + // stop syncing when the synced version and synced ledger info version match. + // Otherwise, the DB will be left in an inconsistent state on handover. + if let Some(sync_request) = consensus_sync_request.lock().as_ref() { + if sync_request.is_sync_duration_request() { + // Get the latest synced version and ledger info version + let latest_synced_version = + utils::fetch_pre_committed_version(self.storage.clone())?; + let latest_synced_ledger_info = + utils::fetch_latest_synced_ledger_info(self.storage.clone())?; + let latest_ledger_info_version = latest_synced_ledger_info.ledger_info().version(); + + // Check if the latest synced version matches the latest ledger info version + if latest_synced_version != latest_ledger_info_version { + sample!( + SampleRate::Duration(Duration::from_secs(DRIVER_INFO_LOG_FREQ_SECS)), + info!( + "Waiting for state sync to sync to a ledger info! \ + Latest synced version: {:?}, latest ledger info version: {:?}", + latest_synced_version, latest_ledger_info_version + ) + ); + + return Ok(()); // State sync should continue to run + } + } + } + + // Handle the satisfied sync request let latest_synced_ledger_info = utils::fetch_latest_synced_ledger_info(self.storage.clone())?; self.consensus_notification_handler - .check_sync_request_progress(latest_synced_ledger_info) + .handle_satisfied_sync_request(latest_synced_ledger_info) .await?; // If the sync request was successfully handled, reset the continuous syncer @@ -537,6 +598,7 @@ impl< self.continuous_syncer.reset_active_stream(None).await?; self.storage_synchronizer.finish_chunk_executor(); // Consensus or consensus observer is now in control } + Ok(()) } diff --git a/state-sync/state-sync-driver/src/driver_factory.rs b/state-sync/state-sync-driver/src/driver_factory.rs index 0e904075aacde..0126ed889cacc 100644 --- a/state-sync/state-sync-driver/src/driver_factory.rs +++ b/state-sync/state-sync-driver/src/driver_factory.rs @@ -124,7 +124,8 @@ impl DriverFactory { ClientNotificationListener::new(client_notification_receiver); let (commit_notification_sender, commit_notification_listener) = CommitNotificationListener::new(); - let consensus_notification_handler = ConsensusNotificationHandler::new(consensus_listener); + let consensus_notification_handler = + ConsensusNotificationHandler::new(consensus_listener, time_service.clone()); let (error_notification_sender, error_notification_listener) = ErrorNotificationListener::new(); let mempool_notification_handler = diff --git a/state-sync/state-sync-driver/src/metrics.rs b/state-sync/state-sync-driver/src/metrics.rs index b87b4f910536b..8d27f2da10cd7 100644 --- a/state-sync/state-sync-driver/src/metrics.rs +++ b/state-sync/state-sync-driver/src/metrics.rs @@ -11,7 +11,10 @@ use std::time::Instant; /// Driver metric labels pub const DRIVER_CLIENT_NOTIFICATION: &str = "driver_client_notification"; pub const DRIVER_CONSENSUS_COMMIT_NOTIFICATION: &str = "driver_consensus_commit_notification"; -pub const DRIVER_CONSENSUS_SYNC_NOTIFICATION: &str = "driver_consensus_sync_notification"; +pub const DRIVER_CONSENSUS_SYNC_DURATION_NOTIFICATION: &str = + "driver_consensus_sync_duration_notification"; +pub const DRIVER_CONSENSUS_SYNC_TARGET_NOTIFICATION: &str = + "driver_consensus_sync_target_notification"; /// Data notification metric labels pub const NOTIFICATION_CREATE_TO_APPLY: &str = "notification_create_to_apply"; diff --git a/state-sync/state-sync-driver/src/notification_handlers.rs b/state-sync/state-sync-driver/src/notification_handlers.rs index dd6171527aa34..ab80d67369127 100644 --- a/state-sync/state-sync-driver/src/notification_handlers.rs +++ b/state-sync/state-sync-driver/src/notification_handlers.rs @@ -8,7 +8,7 @@ use crate::{ }; use aptos_consensus_notifications::{ ConsensusCommitNotification, ConsensusNotification, ConsensusNotificationListener, - ConsensusSyncNotification, + ConsensusSyncDurationNotification, ConsensusSyncTargetNotification, }; use aptos_data_streaming_service::data_notification::NotificationId; use aptos_event_notifications::{EventNotificationSender, EventSubscriptionService}; @@ -16,6 +16,7 @@ use aptos_infallible::Mutex; use aptos_logger::prelude::*; use aptos_mempool_notifications::MempoolNotificationSender; use aptos_storage_service_notifications::StorageServiceNotificationSender; +use aptos_time_service::{TimeService, TimeServiceTrait}; use aptos_types::{ contract_event::ContractEvent, ledger_info::LedgerInfoWithSignatures, @@ -27,6 +28,7 @@ use std::{ pin::Pin, sync::Arc, task::{Context, Poll}, + time::Instant, }; /// A notification for new data that has been committed to storage @@ -144,27 +146,66 @@ impl FusedStream for CommitNotificationListener { } } -/// A consensus sync request for a specified target ledger info -pub struct ConsensusSyncRequest { - consensus_sync_notification: ConsensusSyncNotification, +/// A consensus sync request for a specified target ledger info or duration +pub enum ConsensusSyncRequest { + SyncDuration(Instant, ConsensusSyncDurationNotification), // The start time and duration to sync for + SyncTarget(ConsensusSyncTargetNotification), // The target ledger info to sync to } impl ConsensusSyncRequest { - pub fn new(consensus_sync_notification: ConsensusSyncNotification) -> Self { - Self { - consensus_sync_notification, + /// Returns a new sync target request + pub fn new_with_target(sync_target_notification: ConsensusSyncTargetNotification) -> Self { + ConsensusSyncRequest::SyncTarget(sync_target_notification) + } + + /// Returns a new sync duration request + pub fn new_with_duration( + start_time: Instant, + sync_duration_notification: ConsensusSyncDurationNotification, + ) -> Self { + ConsensusSyncRequest::SyncDuration(start_time, sync_duration_notification) + } + + /// Returns the sync target (if one exists) + pub fn get_sync_target(&self) -> Option { + match self { + ConsensusSyncRequest::SyncTarget(sync_target_notification) => { + Some(sync_target_notification.get_target().clone()) + }, + _ => None, } } - pub fn get_sync_target(&self) -> LedgerInfoWithSignatures { - self.consensus_sync_notification.target.clone() + /// Returns true iff the sync request is a duration request + pub fn is_sync_duration_request(&self) -> bool { + matches!(self, ConsensusSyncRequest::SyncDuration(_, _)) } - pub fn get_sync_target_version(&self) -> Version { - self.consensus_sync_notification - .target - .ledger_info() - .version() + /// Returns true iff the sync request has been satisfied + pub fn sync_request_satisfied( + &self, + latest_synced_ledger_info: &LedgerInfoWithSignatures, + time_service: TimeService, + ) -> bool { + match self { + ConsensusSyncRequest::SyncDuration(start_time, sync_duration_notification) => { + // Get the duration and the current time + let sync_duration = sync_duration_notification.get_duration(); + let current_time = time_service.now(); + + // Check if the duration has been reached + current_time.duration_since(*start_time) >= sync_duration + }, + ConsensusSyncRequest::SyncTarget(sync_target_notification) => { + // Get the sync target version and latest synced version + let sync_target = sync_target_notification.get_target(); + let sync_target_version = sync_target.ledger_info().version(); + let latest_synced_version = latest_synced_ledger_info.ledger_info().version(); + + // Check if we've satisfied the target + latest_synced_version >= sync_target_version + }, + } } } @@ -175,13 +216,20 @@ pub struct ConsensusNotificationHandler { // The latest consensus sync request that has been received consensus_sync_request: Arc>>, + + // The time service + time_service: TimeService, } impl ConsensusNotificationHandler { - pub fn new(consensus_listener: ConsensusNotificationListener) -> Self { + pub fn new( + consensus_listener: ConsensusNotificationListener, + time_service: TimeService, + ) -> Self { Self { consensus_listener, consensus_sync_request: Arc::new(Mutex::new(None)), + time_service, } } @@ -195,14 +243,33 @@ impl ConsensusNotificationHandler { self.consensus_sync_request.clone() } - /// Initializes the sync request received from consensus - pub async fn initialize_sync_request( + /// Initializes the sync duration request received from consensus + pub async fn initialize_sync_duration_request( + &mut self, + sync_duration_notification: ConsensusSyncDurationNotification, + ) -> Result<(), Error> { + // Get the current time + let start_time = self.time_service.now(); + + // Save the request so we can notify consensus once we've hit the duration + let consensus_sync_request = + ConsensusSyncRequest::new_with_duration(start_time, sync_duration_notification); + self.consensus_sync_request = Arc::new(Mutex::new(Some(consensus_sync_request))); + + Ok(()) + } + + /// Initializes the sync target request received from consensus + pub async fn initialize_sync_target_request( &mut self, - sync_notification: ConsensusSyncNotification, + sync_target_notification: ConsensusSyncTargetNotification, latest_synced_ledger_info: LedgerInfoWithSignatures, ) -> Result<(), Error> { // Get the latest committed version and the target sync version - let sync_target_version = sync_notification.target.ledger_info().version(); + let sync_target_version = sync_target_notification + .get_target() + .ledger_info() + .version(); let latest_committed_version = latest_synced_ledger_info.ledger_info().version(); // If the target version is old, return an error to consensus (something is wrong!) @@ -211,8 +278,7 @@ impl ConsensusNotificationHandler { sync_target_version, latest_committed_version, )); - self.respond_to_sync_notification(sync_notification, error.clone()) - .await?; + self.respond_to_sync_target_notification(sync_target_notification, error.clone())?; return error; } @@ -221,114 +287,146 @@ impl ConsensusNotificationHandler { info!(LogSchema::new(LogEntry::NotificationHandler) .message("We're already at the requested sync target version! Returning early")); let result = Ok(()); - self.respond_to_sync_notification(sync_notification, result.clone()) - .await?; + self.respond_to_sync_target_notification(sync_target_notification, result.clone())?; return result; } // Save the request so we can notify consensus once we've hit the target - let consensus_sync_request = ConsensusSyncRequest::new(sync_notification); + let consensus_sync_request = + ConsensusSyncRequest::new_with_target(sync_target_notification); self.consensus_sync_request = Arc::new(Mutex::new(Some(consensus_sync_request))); Ok(()) } - /// Checks to see if the sync request has been successfully fulfilled - pub async fn check_sync_request_progress( + /// Notifies consensus of a satisfied sync request, and removes the active request. + /// Note: this assumes that the sync request has already been checked for satisfaction. + pub async fn handle_satisfied_sync_request( &mut self, latest_synced_ledger_info: LedgerInfoWithSignatures, ) -> Result<(), Error> { - // Fetch the sync target version - let consensus_sync_request = self.get_sync_request(); - let sync_target_version = consensus_sync_request.lock().as_ref().map(|sync_request| { - sync_request - .consensus_sync_notification - .target - .ledger_info() - .version() - }); - - // Compare our local state to the target version - if let Some(sync_target_version) = sync_target_version { - let latest_committed_version = latest_synced_ledger_info.ledger_info().version(); - - // Check if we've synced beyond the target - if latest_committed_version > sync_target_version { - return Err(Error::SyncedBeyondTarget( - latest_committed_version, - sync_target_version, - )); - } - - // Check if we've hit the target - if latest_committed_version == sync_target_version { - let consensus_sync_request = self.get_sync_request().lock().take(); - if let Some(consensus_sync_request) = consensus_sync_request { - self.respond_to_sync_notification( - consensus_sync_request.consensus_sync_notification, - Ok(()), - ) - .await?; + // Remove the active sync request + let mut sync_request_lock = self.consensus_sync_request.lock(); + let consensus_sync_request = sync_request_lock.take(); + + // Notify consensus of the satisfied request + match consensus_sync_request { + Some(ConsensusSyncRequest::SyncDuration(_, sync_duration_notification)) => { + self.respond_to_sync_duration_notification( + sync_duration_notification, + Ok(()), + Some(latest_synced_ledger_info), + )?; + }, + Some(ConsensusSyncRequest::SyncTarget(sync_target_notification)) => { + // Get the sync target version and latest synced version + let sync_target = sync_target_notification.get_target(); + let sync_target_version = sync_target.ledger_info().version(); + let latest_synced_version = latest_synced_ledger_info.ledger_info().version(); + + // Check if we've synced beyond the target. If so, notify consensus with an error. + if latest_synced_version > sync_target_version { + let error = Err(Error::SyncedBeyondTarget( + latest_synced_version, + sync_target_version, + )); + self.respond_to_sync_target_notification( + sync_target_notification, + error.clone(), + )?; + return error; } - return Ok(()); - } + + // Otherwise, notify consensus that the target has been reached + self.respond_to_sync_target_notification(sync_target_notification, Ok(()))?; + }, + None => { /* Nothing needs to be done */ }, } Ok(()) } - /// Responds to consensus for a sync notification using the specified result - pub async fn respond_to_sync_notification( - &mut self, - sync_notification: ConsensusSyncNotification, + /// Responds to consensus for a sync duration notification using the specified result + pub fn respond_to_sync_duration_notification( + &self, + sync_duration_notification: ConsensusSyncDurationNotification, result: Result<(), Error>, + latest_synced_ledger_info: Option, ) -> Result<(), Error> { // Wrap the result in an error that consensus can process - let message = result.map_err(|error| { + let result = result.map_err(|error| { aptos_consensus_notifications::Error::UnexpectedErrorEncountered(format!("{:?}", error)) }); + // Send the result info!( LogSchema::new(LogEntry::NotificationHandler).message(&format!( - "Responding to consensus sync notification with message: {:?}", - message + "Responding to consensus sync duration notification with message: {:?}", + result )) ); + self.consensus_listener + .respond_to_sync_duration_notification( + sync_duration_notification, + result, + latest_synced_ledger_info, + ) + .map_err(|error| { + Error::CallbackSendFailed(format!( + "Consensus sync duration response error: {:?}", + error + )) + }) + } + + /// Responds to consensus for a sync notification using the specified result + pub fn respond_to_sync_target_notification( + &self, + sync_target_notification: ConsensusSyncTargetNotification, + result: Result<(), Error>, + ) -> Result<(), Error> { + // Wrap the result in an error that consensus can process + let result = result.map_err(|error| { + aptos_consensus_notifications::Error::UnexpectedErrorEncountered(format!("{:?}", error)) + }); // Send the result + info!( + LogSchema::new(LogEntry::NotificationHandler).message(&format!( + "Responding to consensus sync target notification with message: {:?}", + result + )) + ); self.consensus_listener - .respond_to_sync_notification(sync_notification, message) - .await + .respond_to_sync_target_notification(sync_target_notification, result) .map_err(|error| { Error::CallbackSendFailed(format!( - "Consensus sync request response error: {:?}", + "Consensus sync target response error: {:?}", error )) }) } /// Responds successfully to consensus for a commit notification - pub async fn respond_to_commit_notification( - &mut self, + pub fn respond_to_commit_notification( + &self, commit_notification: ConsensusCommitNotification, result: Result<(), Error>, ) -> Result<(), Error> { // Wrap the result in an error that consensus can process - let message = result.map_err(|error| { + let result = result.map_err(|error| { aptos_consensus_notifications::Error::UnexpectedErrorEncountered(format!("{:?}", error)) }); + // Send the result debug!( LogSchema::new(LogEntry::NotificationHandler).message(&format!( "Responding to consensus commit notification with message: {:?}", - message + result )) ); - - // Send the result self.consensus_listener - .respond_to_commit_notification(commit_notification, message) - .await + .respond_to_commit_notification(commit_notification, result) .map_err(|error| { Error::CallbackSendFailed(format!("Consensus commit response error: {:?}", error)) }) diff --git a/state-sync/state-sync-driver/src/tests/continuous_syncer.rs b/state-sync/state-sync-driver/src/tests/continuous_syncer.rs index 575c9c4ef9e07..5bd5a93cbf03f 100644 --- a/state-sync/state-sync-driver/src/tests/continuous_syncer.rs +++ b/state-sync/state-sync-driver/src/tests/continuous_syncer.rs @@ -19,7 +19,9 @@ use crate::{ utils::OutputFallbackHandler, }; use aptos_config::config::ContinuousSyncingMode; -use aptos_consensus_notifications::ConsensusSyncNotification; +use aptos_consensus_notifications::{ + ConsensusSyncDurationNotification, ConsensusSyncTargetNotification, +}; use aptos_data_streaming_service::{ data_notification::{DataNotification, DataPayload, NotificationId}, streaming_client::{NotificationAndFeedback, NotificationFeedback}, @@ -31,7 +33,10 @@ use aptos_types::transaction::{TransactionOutputListWithProof, Version}; use claims::assert_matches; use futures::SinkExt; use mockall::{predicate::eq, Sequence}; -use std::{sync::Arc, time::Duration}; +use std::{ + sync::Arc, + time::{Duration, Instant}, +}; #[tokio::test] async fn test_critical_timeout() { @@ -111,7 +116,89 @@ async fn test_critical_timeout() { } #[tokio::test] -async fn test_data_stream_transactions_with_target() { +async fn test_data_stream_transactions_with_sync_duration() { + // Create test data + let current_synced_epoch = 10; + let current_synced_version = 1000; + let notification_id = 900; + + // Create a driver configuration + let mut driver_configuration = create_full_node_driver_configuration(); + driver_configuration.config.continuous_syncing_mode = + ContinuousSyncingMode::ExecuteTransactions; + + // Create the mock streaming client + let mut mock_streaming_client = create_mock_streaming_client(); + let mut expectation_sequence = Sequence::new(); + let (mut notification_sender_1, data_stream_listener_1) = create_data_stream_listener(); + let (_notification_sender_2, data_stream_listener_2) = create_data_stream_listener(); + let data_stream_id_1 = data_stream_listener_1.data_stream_id; + for data_stream_listener in [data_stream_listener_1, data_stream_listener_2] { + mock_streaming_client + .expect_continuously_stream_transactions() + .times(1) + .with( + eq(current_synced_version), + eq(current_synced_epoch), + eq(false), + eq(None), + ) + .return_once(move |_, _, _, _| Ok(data_stream_listener)) + .in_sequence(&mut expectation_sequence); + } + mock_streaming_client + .expect_terminate_stream_with_feedback() + .with( + eq(data_stream_id_1), + eq(Some(NotificationAndFeedback::new( + notification_id, + NotificationFeedback::EmptyPayloadData, + ))), + ) + .return_const(Ok(())); + + // Create the continuous syncer + let (mut continuous_syncer, _) = create_continuous_syncer( + driver_configuration, + mock_streaming_client, + None, + true, + current_synced_version, + current_synced_epoch, + ); + + // Drive progress to initialize the transaction output stream for the sync duration + let (sync_duration_notification, _) = + ConsensusSyncDurationNotification::new(Duration::from_secs(1)); + let sync_request = Arc::new(Mutex::new(Some(ConsensusSyncRequest::new_with_duration( + Instant::now(), + sync_duration_notification, + )))); + drive_progress(&mut continuous_syncer, &sync_request).await; + + // Send an invalid output along the stream + let data_notification = DataNotification::new( + notification_id, + DataPayload::ContinuousTransactionOutputsWithProof( + create_epoch_ending_ledger_info(), + TransactionOutputListWithProof::new_empty(), + ), + ); + notification_sender_1.send(data_notification).await.unwrap(); + + // Drive progress again and ensure we get a verification error + let error = continuous_syncer + .drive_progress(sync_request.clone()) + .await + .unwrap_err(); + assert_matches!(error, Error::VerificationError(_)); + + // Drive progress to initialize the transaction output stream. + drive_progress(&mut continuous_syncer, &sync_request).await; +} + +#[tokio::test] +async fn test_data_stream_transactions_with_sync_target() { // Create test data let current_synced_epoch = 5; let current_synced_version = 234; @@ -163,10 +250,10 @@ async fn test_data_stream_transactions_with_target() { current_synced_epoch, ); - // Drive progress to initialize the transaction output stream - let (consensus_sync_notification, _) = ConsensusSyncNotification::new(target_ledger_info); - let sync_request = Arc::new(Mutex::new(Some(ConsensusSyncRequest::new( - consensus_sync_notification, + // Drive progress to initialize the transaction output stream for the sync target + let (sync_target_notification, _) = ConsensusSyncTargetNotification::new(target_ledger_info); + let sync_request = Arc::new(Mutex::new(Some(ConsensusSyncRequest::new_with_target( + sync_target_notification, )))); drive_progress(&mut continuous_syncer, &sync_request).await; @@ -187,7 +274,7 @@ async fn test_data_stream_transactions_with_target() { .unwrap_err(); assert_matches!(error, Error::VerificationError(_)); - // Drive progress to initialize the transaction output stream. + // Drive progress to initialize the transaction output stream drive_progress(&mut continuous_syncer, &sync_request).await; } @@ -242,7 +329,7 @@ async fn test_data_stream_transaction_outputs() { current_synced_epoch, ); - // Drive progress to initialize the transaction output stream + // Drive progress to initialize the transaction output stream (without a sync target) let no_sync_request = Arc::new(Mutex::new(None)); drive_progress(&mut continuous_syncer, &no_sync_request).await; @@ -271,7 +358,89 @@ async fn test_data_stream_transaction_outputs() { } #[tokio::test] -async fn test_data_stream_transactions_or_outputs_with_target() { +async fn test_data_stream_transactions_or_outputs_with_sync_duration() { + // Create test data + let current_synced_epoch = 100; + let current_synced_version = 1000; + let notification_id = 100; + + // Create a driver configuration with a genesis waypoint and transactions or output syncing + let mut driver_configuration = create_full_node_driver_configuration(); + driver_configuration.config.continuous_syncing_mode = + ContinuousSyncingMode::ExecuteTransactionsOrApplyOutputs; + + // Create the mock streaming client + let mut mock_streaming_client = create_mock_streaming_client(); + let mut expectation_sequence = Sequence::new(); + let (mut notification_sender_1, data_stream_listener_1) = create_data_stream_listener(); + let (_notification_sender_2, data_stream_listener_2) = create_data_stream_listener(); + let data_stream_id_1 = data_stream_listener_1.data_stream_id; + for data_stream_listener in [data_stream_listener_1, data_stream_listener_2] { + mock_streaming_client + .expect_continuously_stream_transactions_or_outputs() + .times(1) + .with( + eq(current_synced_version), + eq(current_synced_epoch), + eq(false), + eq(None), + ) + .return_once(move |_, _, _, _| Ok(data_stream_listener)) + .in_sequence(&mut expectation_sequence); + } + mock_streaming_client + .expect_terminate_stream_with_feedback() + .with( + eq(data_stream_id_1), + eq(Some(NotificationAndFeedback::new( + notification_id, + NotificationFeedback::EmptyPayloadData, + ))), + ) + .return_const(Ok(())); + + // Create the continuous syncer + let (mut continuous_syncer, _) = create_continuous_syncer( + driver_configuration, + mock_streaming_client, + None, + true, + current_synced_version, + current_synced_epoch, + ); + + // Drive progress to initialize the transaction output stream for the sync duration + let (sync_duration_notification, _) = + ConsensusSyncDurationNotification::new(Duration::from_secs(1)); + let sync_request = Arc::new(Mutex::new(Some(ConsensusSyncRequest::new_with_duration( + Instant::now(), + sync_duration_notification, + )))); + drive_progress(&mut continuous_syncer, &sync_request).await; + + // Send an invalid output along the stream + let data_notification = DataNotification::new( + notification_id, + DataPayload::ContinuousTransactionOutputsWithProof( + create_epoch_ending_ledger_info(), + TransactionOutputListWithProof::new_empty(), + ), + ); + notification_sender_1.send(data_notification).await.unwrap(); + + // Drive progress again and ensure we get a verification error + let error = continuous_syncer + .drive_progress(sync_request.clone()) + .await + .unwrap_err(); + assert_matches!(error, Error::VerificationError(_)); + + // Drive progress to initialize the transaction output stream + drive_progress(&mut continuous_syncer, &sync_request).await; +} + +#[tokio::test] +async fn test_data_stream_transactions_or_outputs_with_sync_target() { // Create test data let current_synced_epoch = 5; let current_synced_version = 234; @@ -323,10 +492,10 @@ async fn test_data_stream_transactions_or_outputs_with_target() { current_synced_epoch, ); - // Drive progress to initialize the transaction output stream - let (consensus_sync_notification, _) = ConsensusSyncNotification::new(target_ledger_info); - let sync_request = Arc::new(Mutex::new(Some(ConsensusSyncRequest::new( - consensus_sync_notification, + // Drive progress to initialize the transaction output stream for the sync target + let (sync_target_notification, _) = ConsensusSyncTargetNotification::new(target_ledger_info); + let sync_request = Arc::new(Mutex::new(Some(ConsensusSyncRequest::new_with_target( + sync_target_notification, )))); drive_progress(&mut continuous_syncer, &sync_request).await; @@ -352,7 +521,138 @@ async fn test_data_stream_transactions_or_outputs_with_target() { } #[tokio::test] -async fn test_data_stream_transactions_or_outputs_with_target_fallback() { +async fn test_data_stream_transactions_or_outputs_with_sync_duration_fallback() { + // Create test data + let current_synced_epoch = 50; + let current_synced_version = 1234; + let notification_id = 101; + + // Create a driver configuration with a genesis waypoint and transactions or output syncing + let mut driver_configuration = create_full_node_driver_configuration(); + driver_configuration.config.continuous_syncing_mode = + ContinuousSyncingMode::ExecuteTransactionsOrApplyOutputs; + + // Create the mock streaming client + let mut mock_streaming_client = create_mock_streaming_client(); + + // Set expectations for stream creations and terminations + let mut expectation_sequence = Sequence::new(); + let (_notification_sender_1, data_stream_listener_1) = create_data_stream_listener(); + let data_stream_id_1 = data_stream_listener_1.data_stream_id; + let (_notification_sender_2, data_stream_listener_2) = create_data_stream_listener(); + let data_stream_id_2 = data_stream_listener_2.data_stream_id; + let (_notification_sender_3, data_stream_listener_3) = create_data_stream_listener(); + mock_streaming_client + .expect_continuously_stream_transactions_or_outputs() + .times(1) + .with( + eq(current_synced_version), + eq(current_synced_epoch), + eq(false), + eq(None), + ) + .return_once(move |_, _, _, _| Ok(data_stream_listener_1)) + .in_sequence(&mut expectation_sequence); + mock_streaming_client + .expect_terminate_stream_with_feedback() + .times(1) + .with( + eq(data_stream_id_1), + eq(Some(NotificationAndFeedback::new( + notification_id, + NotificationFeedback::PayloadProofFailed, + ))), + ) + .return_const(Ok(())) + .in_sequence(&mut expectation_sequence); + mock_streaming_client + .expect_continuously_stream_transaction_outputs() + .times(1) + .with( + eq(current_synced_version), + eq(current_synced_epoch), + eq(None), + ) + .return_once(move |_, _, _| Ok(data_stream_listener_2)) + .in_sequence(&mut expectation_sequence); + mock_streaming_client + .expect_terminate_stream_with_feedback() + .times(1) + .with( + eq(data_stream_id_2), + eq(Some(NotificationAndFeedback::new( + notification_id, + NotificationFeedback::InvalidPayloadData, + ))), + ) + .return_const(Ok(())) + .in_sequence(&mut expectation_sequence); + mock_streaming_client + .expect_continuously_stream_transactions_or_outputs() + .times(1) + .with( + eq(current_synced_version), + eq(current_synced_epoch), + eq(false), + eq(None), + ) + .return_once(move |_, _, _, _| Ok(data_stream_listener_3)) + .in_sequence(&mut expectation_sequence); + + // Create the continuous syncer + let time_service = TimeService::mock(); + let (mut continuous_syncer, mut output_fallback_handler) = create_continuous_syncer( + driver_configuration.clone(), + mock_streaming_client, + Some(time_service.clone()), + true, + current_synced_version, + current_synced_epoch, + ); + assert!(!output_fallback_handler.in_fallback_mode()); + + // Drive progress to initialize the transactions or output stream for the sync duration + let (sync_duration_notification, _) = + ConsensusSyncDurationNotification::new(Duration::from_secs(1)); + let sync_request = Arc::new(Mutex::new(Some(ConsensusSyncRequest::new_with_duration( + Instant::now(), + sync_duration_notification, + )))); + drive_progress(&mut continuous_syncer, &sync_request).await; + + // Send a storage synchronizer error to the continuous syncer so that it falls back + // to output syncing and drive progress for the new stream type. + handle_storage_synchronizer_error( + &mut continuous_syncer, + notification_id, + NotificationFeedback::PayloadProofFailed, + ) + .await; + drive_progress(&mut continuous_syncer, &sync_request).await; + assert!(output_fallback_handler.in_fallback_mode()); + + // Elapse enough time so that fallback mode is now disabled + time_service + .into_mock() + .advance_async(Duration::from_secs( + driver_configuration.config.fallback_to_output_syncing_secs, + )) + .await; + + // Send another storage synchronizer error to the bootstrapper and drive progress + // so that a regular stream is created. + handle_storage_synchronizer_error( + &mut continuous_syncer, + notification_id, + NotificationFeedback::InvalidPayloadData, + ) + .await; + drive_progress(&mut continuous_syncer, &sync_request).await; + assert!(!output_fallback_handler.in_fallback_mode()); +} + +#[tokio::test] +async fn test_data_stream_transactions_or_outputs_with_sync_target_fallback() { // Create test data let current_synced_epoch = 5; let current_synced_version = 234; @@ -444,9 +744,9 @@ async fn test_data_stream_transactions_or_outputs_with_target_fallback() { assert!(!output_fallback_handler.in_fallback_mode()); // Drive progress to initialize the transactions or output stream - let (consensus_sync_notification, _) = ConsensusSyncNotification::new(target_ledger_info); - let sync_request = Arc::new(Mutex::new(Some(ConsensusSyncRequest::new( - consensus_sync_notification, + let (sync_target_notification, _) = ConsensusSyncTargetNotification::new(target_ledger_info); + let sync_request = Arc::new(Mutex::new(Some(ConsensusSyncRequest::new_with_target( + sync_target_notification, )))); drive_progress(&mut continuous_syncer, &sync_request).await; diff --git a/state-sync/state-sync-driver/src/tests/mocks.rs b/state-sync/state-sync-driver/src/tests/mocks.rs index 462bd0a76f4ee..382c2d24ec579 100644 --- a/state-sync/state-sync-driver/src/tests/mocks.rs +++ b/state-sync/state-sync-driver/src/tests/mocks.rs @@ -332,7 +332,7 @@ mock! { ledger_info_with_sigs: Option<&'a LedgerInfoWithSignatures>, sync_commit: bool, in_memory_state: StateDelta, - state_updates_until_last_checkpoint: Option, + state_updates_until_last_checkpoint: Option<&'b ShardedStateUpdates>, sharded_state_cache: Option<&'b ShardedStateCache>, ) -> Result<()>; } diff --git a/storage/aptosdb/src/db/fake_aptosdb.rs b/storage/aptosdb/src/db/fake_aptosdb.rs index 325c9fe4228c9..7c576d12e039c 100644 --- a/storage/aptosdb/src/db/fake_aptosdb.rs +++ b/storage/aptosdb/src/db/fake_aptosdb.rs @@ -104,7 +104,7 @@ impl FakeBufferedState { pub fn update( &mut self, - updates_until_next_checkpoint_since_current_option: Option, + updates_until_next_checkpoint_since_current_option: Option<&ShardedStateUpdates>, new_state_after_checkpoint: StateDelta, ) -> Result<()> { ensure!( diff --git a/storage/aptosdb/src/db/include/aptosdb_testonly.rs b/storage/aptosdb/src/db/include/aptosdb_testonly.rs index 5e67968b5fa3b..bf8ca7580850a 100644 --- a/storage/aptosdb/src/db/include/aptosdb_testonly.rs +++ b/storage/aptosdb/src/db/include/aptosdb_testonly.rs @@ -150,7 +150,7 @@ impl AptosDB { ledger_info_with_sigs, sync_commit, latest_in_memory_state, - state_updates_until_last_checkpoint, + state_updates_until_last_checkpoint.as_ref(), None, ) } diff --git a/storage/aptosdb/src/db/include/aptosdb_writer.rs b/storage/aptosdb/src/db/include/aptosdb_writer.rs index 28f8008ee57f0..f38a62c65e1ab 100644 --- a/storage/aptosdb/src/db/include/aptosdb_writer.rs +++ b/storage/aptosdb/src/db/include/aptosdb_writer.rs @@ -11,7 +11,7 @@ impl DbWriter for AptosDB { base_state_version: Option, sync_commit: bool, latest_in_memory_state: StateDelta, - state_updates_until_last_checkpoint: Option, + state_updates_until_last_checkpoint: Option<&ShardedStateUpdates>, sharded_state_cache: Option<&ShardedStateCache>, ) -> Result<()> { gauged_api("pre_commit_ledger", || { diff --git a/storage/aptosdb/src/db/test_helper.rs b/storage/aptosdb/src/db/test_helper.rs index 68265f72c0e34..59b1634da3f7c 100644 --- a/storage/aptosdb/src/db/test_helper.rs +++ b/storage/aptosdb/src/db/test_helper.rs @@ -1010,7 +1010,8 @@ pub fn test_sync_transactions_impl( cur_ver, &in_memory_state, txns_to_commit_batch, - ), + ) + .as_ref(), None, ) .unwrap(); @@ -1025,7 +1026,8 @@ pub fn test_sync_transactions_impl( Some(ledger_info_with_sigs), false, /* sync_commit */ in_memory_state.clone(), - gather_state_updates_until_last_checkpoint(ver, &in_memory_state, txns_to_commit_batch), + gather_state_updates_until_last_checkpoint(ver, &in_memory_state, txns_to_commit_batch) + .as_ref(), None, ) .unwrap(); diff --git a/storage/aptosdb/src/fast_sync_storage_wrapper.rs b/storage/aptosdb/src/fast_sync_storage_wrapper.rs index 703cfe9326f37..4b1fac3e7e284 100644 --- a/storage/aptosdb/src/fast_sync_storage_wrapper.rs +++ b/storage/aptosdb/src/fast_sync_storage_wrapper.rs @@ -176,7 +176,7 @@ impl DbWriter for FastSyncStorageWrapper { base_state_version: Option, sync_commit: bool, latest_in_memory_state: StateDelta, - state_updates_until_last_checkpoint: Option, + state_updates_until_last_checkpoint: Option<&ShardedStateUpdates>, sharded_state_cache: Option<&ShardedStateCache>, ) -> Result<()> { self.get_aptos_db_write_ref().pre_commit_ledger( diff --git a/storage/aptosdb/src/state_store/buffered_state.rs b/storage/aptosdb/src/state_store/buffered_state.rs index 83f5f55609071..253343be00c6a 100644 --- a/storage/aptosdb/src/state_store/buffered_state.rs +++ b/storage/aptosdb/src/state_store/buffered_state.rs @@ -154,7 +154,7 @@ impl BufferedState { /// This method updates the buffered state with new data. pub fn update( &mut self, - updates_until_next_checkpoint_since_current_option: Option, + updates_until_next_checkpoint_since_current_option: Option<&ShardedStateUpdates>, new_state_after_checkpoint: StateDelta, sync_commit: bool, ) -> Result<()> { diff --git a/storage/aptosdb/src/state_store/mod.rs b/storage/aptosdb/src/state_store/mod.rs index 1b30c6343eb89..3902125a08ae1 100644 --- a/storage/aptosdb/src/state_store/mod.rs +++ b/storage/aptosdb/src/state_store/mod.rs @@ -541,7 +541,7 @@ impl StateStore { // synchronously commit the snapshot at the last checkpoint here if not committed to disk yet. buffered_state.update( - updates_until_last_checkpoint, + updates_until_last_checkpoint.as_ref(), state_after_last_checkpoint, true, /* sync_commit */ )?; diff --git a/storage/storage-interface/src/lib.rs b/storage/storage-interface/src/lib.rs index ff09eca5e7095..8a357eda90116 100644 --- a/storage/storage-interface/src/lib.rs +++ b/storage/storage-interface/src/lib.rs @@ -548,7 +548,7 @@ pub trait DbWriter: Send + Sync { ledger_info_with_sigs: Option<&LedgerInfoWithSignatures>, sync_commit: bool, latest_in_memory_state: StateDelta, - state_updates_until_last_checkpoint: Option, + state_updates_until_last_checkpoint: Option<&ShardedStateUpdates>, sharded_state_cache: Option<&ShardedStateCache>, ) -> Result<()> { // For reconfig suffix. @@ -596,7 +596,7 @@ pub trait DbWriter: Send + Sync { base_state_version: Option, sync_commit: bool, latest_in_memory_state: StateDelta, - state_updates_until_last_checkpoint: Option, + state_updates_until_last_checkpoint: Option<&ShardedStateUpdates>, sharded_state_cache: Option<&ShardedStateCache>, ) -> Result<()> { unimplemented!() diff --git a/storage/storage-interface/src/state_delta.rs b/storage/storage-interface/src/state_delta.rs index a4534e323a9fd..bbe4016a86b15 100644 --- a/storage/storage-interface/src/state_delta.rs +++ b/storage/storage-interface/src/state_delta.rs @@ -74,7 +74,7 @@ impl StateDelta { pub fn merge(&mut self, other: StateDelta) { assert!(other.follow(self)); - combine_sharded_state_updates(&mut self.updates_since_base, other.updates_since_base); + combine_sharded_state_updates(&mut self.updates_since_base, &other.updates_since_base); self.current = other.current; self.current_version = other.current_version; diff --git a/testsuite/forge-cli/src/main.rs b/testsuite/forge-cli/src/main.rs index a514d63454eaf..4c139f958d4b0 100644 --- a/testsuite/forge-cli/src/main.rs +++ b/testsuite/forge-cli/src/main.rs @@ -838,9 +838,12 @@ fn get_multi_region_test(test_name: &str) -> Option { Some(test) } -fn wrap_with_realistic_env(test: T) -> CompositeNetworkTest { +fn wrap_with_realistic_env( + num_validators: usize, + test: T, +) -> CompositeNetworkTest { CompositeNetworkTest::new_with_two_wrappers( - MultiRegionNetworkEmulationTest::default(), + MultiRegionNetworkEmulationTest::default_for_validator_count(num_validators), CpuChaosTest::default(), test, ) @@ -885,8 +888,9 @@ fn wrap_with_two_region_env(test: T) -> CompositeNetwo } fn run_consensus_only_realistic_env_max_tps() -> ForgeConfig { + let num_validators = 20; ForgeConfig::default() - .with_initial_validator_count(NonZeroUsize::new(20).unwrap()) + .with_initial_validator_count(NonZeroUsize::new(num_validators).unwrap()) .with_emit_job( EmitJobRequest::default() .mode(EmitJobMode::MaxLoad { @@ -895,7 +899,7 @@ fn run_consensus_only_realistic_env_max_tps() -> ForgeConfig { .txn_expiration_time_secs(5 * 60), ) .add_network_test(CompositeNetworkTest::new( - MultiRegionNetworkEmulationTest::default(), + MultiRegionNetworkEmulationTest::default_for_validator_count(num_validators), CpuChaosTest::default(), )) .with_genesis_helm_config_fn(Arc::new(|helm_values| { @@ -1146,7 +1150,7 @@ fn realistic_env_sweep_wrap( .with_validator_override_node_config_fn(Arc::new(|config, _| { config.execution.processed_transactions_detailed_counters = true; })) - .add_network_test(wrap_with_realistic_env(test)) + .add_network_test(wrap_with_realistic_env(num_validators, test)) // Test inherits the main EmitJobRequest, so update here for more precise latency measurements .with_emit_job( EmitJobRequest::default().latency_polling_interval(Duration::from_millis(100)), @@ -1415,10 +1419,11 @@ fn workload_vs_perf_benchmark() -> ForgeConfig { } fn realistic_env_graceful_overload(duration: Duration) -> ForgeConfig { + let num_validators = 20; ForgeConfig::default() - .with_initial_validator_count(NonZeroUsize::new(20).unwrap()) + .with_initial_validator_count(NonZeroUsize::new(num_validators).unwrap()) .with_initial_fullnode_count(20) - .add_network_test(wrap_with_realistic_env(TwoTrafficsTest { + .add_network_test(wrap_with_realistic_env(num_validators, TwoTrafficsTest { inner_traffic: EmitJobRequest::default() .mode(EmitJobMode::ConstTps { tps: 15000 }) .init_gas_price_multiplier(20), @@ -1979,7 +1984,7 @@ fn realistic_env_max_load_test( ForgeConfig::default() .with_initial_validator_count(NonZeroUsize::new(num_validators).unwrap()) .with_initial_fullnode_count(num_fullnodes) - .add_network_test(wrap_with_realistic_env(TwoTrafficsTest { + .add_network_test(wrap_with_realistic_env(num_validators, TwoTrafficsTest { inner_traffic: EmitJobRequest::default() .mode(EmitJobMode::MaxLoad { mempool_backlog }) .init_gas_price_multiplier(20), @@ -2040,7 +2045,7 @@ fn realistic_network_tuned_for_throughput_test() -> ForgeConfig { let mut forge_config = ForgeConfig::default() .with_initial_validator_count(NonZeroUsize::new(VALIDATOR_COUNT).unwrap()) - .add_network_test(MultiRegionNetworkEmulationTest::default()) + .add_network_test(MultiRegionNetworkEmulationTest::default_for_validator_count(VALIDATOR_COUNT)) .with_emit_job(EmitJobRequest::default().mode(EmitJobMode::MaxLoad { mempool_backlog: (TARGET_TPS as f64 * VFN_LATENCY_S) as usize, })) @@ -2353,8 +2358,9 @@ fn quorum_store_reconfig_enable_test() -> ForgeConfig { } fn mainnet_like_simulation_test() -> ForgeConfig { + let num_validators = 20; ForgeConfig::default() - .with_initial_validator_count(NonZeroUsize::new(20).unwrap()) + .with_initial_validator_count(NonZeroUsize::new(num_validators).unwrap()) .with_emit_job( EmitJobRequest::default() .mode(EmitJobMode::MaxLoad { @@ -2363,7 +2369,7 @@ fn mainnet_like_simulation_test() -> ForgeConfig { .txn_expiration_time_secs(5 * 60), ) .add_network_test(CompositeNetworkTest::new( - MultiRegionNetworkEmulationTest::default(), + MultiRegionNetworkEmulationTest::default_for_validator_count(num_validators), CpuChaosTest::default(), )) .with_genesis_helm_config_fn(Arc::new(|helm_values| { diff --git a/testsuite/forge-cli/src/suites/dag.rs b/testsuite/forge-cli/src/suites/dag.rs index f309c6403cba1..1e0848db8d307 100644 --- a/testsuite/forge-cli/src/suites/dag.rs +++ b/testsuite/forge-cli/src/suites/dag.rs @@ -60,7 +60,7 @@ fn dag_realistic_env_max_load_test( ForgeConfig::default() .with_initial_validator_count(NonZeroUsize::new(num_validators).unwrap()) .with_initial_fullnode_count(num_fullnodes) - .add_network_test(wrap_with_realistic_env(TwoTrafficsTest { + .add_network_test(wrap_with_realistic_env(num_validators, TwoTrafficsTest { inner_traffic: EmitJobRequest::default() .mode(EmitJobMode::MaxLoad { mempool_backlog: 50000, diff --git a/testsuite/forge/src/backend/k8s/swarm.rs b/testsuite/forge/src/backend/k8s/swarm.rs index 9211bed8a1381..9a14b69963ee8 100644 --- a/testsuite/forge/src/backend/k8s/swarm.rs +++ b/testsuite/forge/src/backend/k8s/swarm.rs @@ -735,8 +735,8 @@ trait ChaosExperimentOps { async fn list_stress_chaos(&self) -> Result>; async fn ensure_chaos_experiments_active(&self) -> Result<()> { - let timeout_duration = Duration::from_secs(300); // 5 minutes - let polling_interval = Duration::from_secs(5); + let timeout_duration = Duration::from_secs(600); // 10 minutes + let polling_interval = Duration::from_secs(10); tokio::time::timeout(timeout_duration, async { loop { @@ -793,6 +793,8 @@ fn check_all_injected(status: &Option) -> bool { .map_or(false, |conditions| { conditions.iter().any(|c| { c.r#type == ChaosConditionType::AllInjected && c.status == ConditionStatus::True + }) && conditions.iter().any(|c| { + c.r#type == ChaosConditionType::Selected && c.status == ConditionStatus::True }) }) } @@ -870,19 +872,31 @@ mod tests { ) -> (Vec, Vec) { let network_chaos = NetworkChaos { status: Some(ChaosStatus { - conditions: Some(vec![ChaosCondition { - r#type: ChaosConditionType::AllInjected, - status: network_status, - }]), + conditions: Some(vec![ + ChaosCondition { + r#type: ChaosConditionType::AllInjected, + status: network_status.clone(), + }, + ChaosCondition { + r#type: ChaosConditionType::Selected, + status: network_status, + }, + ]), }), ..NetworkChaos::new("test", Default::default()) }; let stress_chaos = StressChaos { status: Some(ChaosStatus { - conditions: Some(vec![ChaosCondition { - r#type: ChaosConditionType::AllInjected, - status: stress_status, - }]), + conditions: Some(vec![ + ChaosCondition { + r#type: ChaosConditionType::AllInjected, + status: stress_status.clone(), + }, + ChaosCondition { + r#type: ChaosConditionType::Selected, + status: stress_status, + }, + ]), }), ..StressChaos::new("test", Default::default()) }; diff --git a/testsuite/fuzzer/fuzz.sh b/testsuite/fuzzer/fuzz.sh index bdf98b9275816..c84b3ed9b0c1b 100755 --- a/testsuite/fuzzer/fuzz.sh +++ b/testsuite/fuzzer/fuzz.sh @@ -120,10 +120,18 @@ function build-oss-fuzz() { ld.lld --version clang --version - if ! build all ./target; then - env - error "Build failed. Exiting." - fi + # Limit the number of parallel jobs to avoid OOM + # export CARGO_BUILD_JOBS = 3 + + # Build the fuzz targets + # Doing one target at the time should prevent OOM, but use all thread while bulding dependecies + for fuzz_target in $(list); do + if ! build $fuzz_target ./target ; then + env + error "Build failed. Exiting." + fi + done + find ./target/*/release/ -maxdepth 1 -type f -perm /111 -exec cp {} $oss_fuzz_out \; # Download corpus zip diff --git a/testsuite/testcases/src/data/six_region_link_stats.csv b/testsuite/testcases/src/data/six_region_link_stats.csv new file mode 100644 index 0000000000000..58fc7e486e6be --- /dev/null +++ b/testsuite/testcases/src/data/six_region_link_stats.csv @@ -0,0 +1,31 @@ +sending_region,receiving_region,bitrate_bps,avgrtt +gcp--us-central1,aws--eu-west-1,300000000,103.435 +gcp--us-central1,aws--ap-northeast-1,300000000,133.996 +gcp--us-central1,aws--sa-east-1,300000000,145.483 +gcp--us-central1,aws--eu-central1,300000000,107.671 +gcp--us-central1,gcp--ca-central1,300000000,29.748 +aws--sa-east-1,gcp--us-central1,300000000,145.703 +aws--sa-east-1,aws--eu-west-1,300000000,176.894 +aws--sa-east-1,aws--ap-northeast-1,300000000,255.289 +aws--sa-east-1,aws--eu-central1,300000000,203.508 +aws--sa-east-1,gcp--ca-central-1,300000000,124.307 +aws--eu-west-1,gcp--us-central1,300000000,104.169 +aws--eu-west-1,aws--sa-east-1,300000000,176.813 +aws--eu-west-1,aws--ap-northeast-1,300000000,198.555 +aws--eu-west-1,aws--eu-central1,300000000,23.493 +aws--eu-west-1,gcp--ca-central-1,300000000,68.622 +aws--ap-northeast-1,gcp--us-central1,300000000,128.999 +aws--ap-northeast-1,aws--eu-west-1,300000000,198.539 +aws--ap-northeast-1,aws--sa-east-1,300000000,255.323 +aws--ap-northeast-1,aws--eu-central1,300000000,223.400 +aws--ap-northeast-1,gcp--ca-central-1,300000000,142.549 +aws--eu-central1,gcp--us-central1,300000000,107.671 +aws--eu-central1,aws--sa-east-1,300000000,203.508 +aws--eu-central1,aws--eu-west-1,300000000,23.493 +aws--eu-central1,aws--ap-northeast-1,300000000,223.400 +aws--eu-central1,gcp--ca-central-1,300000000,89.889 +gcp--ca-central-1,gcp--us-central1,300000000,29.748 +gcp--ca-central-1,aws--sa-east-1,300000000,124.307 +gcp--ca-central-1,aws--eu-west-1,300000000,68.622 +gcp--ca-central-1,aws--ap-northeast-1,300000000,142.549 +gcp--ca-central-1,aws--eu-central1,300000000,89.889 \ No newline at end of file diff --git a/testsuite/testcases/src/multi_region_network_test.rs b/testsuite/testcases/src/multi_region_network_test.rs index 840d37699152e..cbe5c10865c27 100644 --- a/testsuite/testcases/src/multi_region_network_test.rs +++ b/testsuite/testcases/src/multi_region_network_test.rs @@ -19,6 +19,7 @@ use std::{collections::BTreeMap, sync::Arc}; /// is measuring TCP bandwidth only which is primarily affected by RTT, and not the actual bandwidth /// across the regions, which would vary according to competing traffic, etc. const FOUR_REGION_LINK_STATS: &[u8] = include_bytes!("data/four_region_link_stats.csv"); +const SIX_REGION_LINK_STATS: &[u8] = include_bytes!("data/six_region_link_stats.csv"); /// The two regions were chosen as the most distant regions among the four regions set. const TWO_REGION_LINK_STATS: &[u8] = include_bytes!("data/two_region_link_stats.csv"); @@ -79,8 +80,8 @@ fn create_link_stats_table_with_peer_groups( "At least 2 regions are required for inter-region network chaos." ); assert!( - number_of_regions <= 4, - "ChaosMesh only supports simulating up to 4 regions." + number_of_regions <= 6, + "ChaosMesh only supports simulating up to 6 regions." ); // Create the link stats table with peer groups @@ -237,10 +238,23 @@ impl MultiRegionNetworkEmulationConfig { ..Default::default() } } + + pub fn four_regions() -> Self { + Self { + link_stats_table: get_link_stats_table(FOUR_REGION_LINK_STATS), + ..Default::default() + } + } + + pub fn six_regions() -> Self { + Self { + link_stats_table: get_link_stats_table(SIX_REGION_LINK_STATS), + ..Default::default() + } + } } /// A test to emulate network conditions for a multi-region setup. -#[derive(Default)] pub struct MultiRegionNetworkEmulationTest { network_emulation_config: MultiRegionNetworkEmulationConfig, } @@ -252,6 +266,18 @@ impl MultiRegionNetworkEmulationTest { } } + pub fn default_for_validator_count(num_validators: usize) -> Self { + if num_validators > 100 { + Self { + network_emulation_config: MultiRegionNetworkEmulationConfig::six_regions(), + } + } else { + Self { + network_emulation_config: MultiRegionNetworkEmulationConfig::four_regions(), + } + } + } + /// Creates a new SwarmNetEm to be injected via chaos. Note: network /// emulation is only done for the validators in the swarm (and not /// the fullnodes). diff --git a/third_party/move/move-compiler-v2/src/env_pipeline/spec_rewriter.rs b/third_party/move/move-compiler-v2/src/env_pipeline/spec_rewriter.rs index 1077b7b3af83e..dd9fd65ae1402 100644 --- a/third_party/move/move-compiler-v2/src/env_pipeline/spec_rewriter.rs +++ b/third_party/move/move-compiler-v2/src/env_pipeline/spec_rewriter.rs @@ -377,7 +377,7 @@ impl<'a> ExpRewriterFunctions for SpecConverter<'a> { .into_exp() }, // Deal with removing various occurrences of Abort and spec blocks - Call(id, Abort, _) | SpecBlock(id, ..) => { + SpecBlock(id, ..) => { // Replace direct call by unit Call(*id, Tuple, vec![]).into_exp() }, diff --git a/third_party/move/move-compiler-v2/src/pipeline/livevar_analysis_processor.rs b/third_party/move/move-compiler-v2/src/pipeline/livevar_analysis_processor.rs index 36e22429ee40a..c888d9e67bec2 100644 --- a/third_party/move/move-compiler-v2/src/pipeline/livevar_analysis_processor.rs +++ b/third_party/move/move-compiler-v2/src/pipeline/livevar_analysis_processor.rs @@ -198,9 +198,7 @@ impl FunctionTargetProcessor for LiveVarAnalysisProcessor { impl LiveVarAnalysisProcessor { /// Create a new instance of live variable analysis. /// `track_all_usages` determines whether both primary and secondary usages of a variable are - /// tracked (when true), or only the primary usages (when false). Also, if set, all usages - /// of temporaries in specifications are tracked, which are considered as secondary because - /// they are not part of the execution semantics. + /// tracked (when true), or only the primary usages (when false). /// Unless all usages are needed, it is recommended to set `track_all_usages` to false. pub fn new(track_all_usages: bool) -> Self { Self { track_all_usages } @@ -373,14 +371,22 @@ impl<'a> TransferFunctions for LiveVarAnalysis<'a> { Branch(id, _, _, src) => { state.insert_or_update(*src, self.livevar_info(id, offset), self.track_all_usages); }, - Prop(id, _, exp) if self.track_all_usages => { + Prop(id, _, exp) => { for idx in exp.used_temporaries() { - state.insert_or_update(idx, self.livevar_info(id, offset), true); + state.insert_or_update( + idx, + self.livevar_info(id, offset), + self.track_all_usages, + ); } }, - SpecBlock(id, spec) if self.track_all_usages => { + SpecBlock(id, spec) => { for idx in spec.used_temporaries() { - state.insert_or_update(idx, self.livevar_info(id, offset), true); + state.insert_or_update( + idx, + self.livevar_info(id, offset), + self.track_all_usages, + ); } }, _ => {}, diff --git a/third_party/move/move-compiler-v2/tests/checking/naming/module_struct_same_name.exp b/third_party/move/move-compiler-v2/tests/checking/naming/module_struct_same_name.exp new file mode 100644 index 0000000000000..5cc6f56d4dbef --- /dev/null +++ b/third_party/move/move-compiler-v2/tests/checking/naming/module_struct_same_name.exp @@ -0,0 +1,33 @@ + +Diagnostics: +warning: unused alias + ┌─ tests/checking/naming/module_struct_same_name.move:8:15 + │ +8 │ use 0x42::M::{Self, M}; + │ ^ Unused 'use' of alias 'M'. Consider removing it + +// -- Model dump before bytecode pipeline +module 0x42::M { + enum M { + M, + } +} // end 0x42::M +module 0x42::M1 { + use 0x42::M::{Self, M}; // resolved as: 0x42::M + private fun test(_m: M::M): u64 { + 3 + } +} // end 0x42::M1 + +// -- Sourcified model before bytecode pipeline +module 0x42::M { + enum M has drop { + M, + } +} +module 0x42::M1 { + use 0x42::M; + fun test(_m: M::M): u64 { + 3 + } +} diff --git a/third_party/move/move-compiler-v2/tests/checking/naming/module_struct_same_name.move b/third_party/move/move-compiler-v2/tests/checking/naming/module_struct_same_name.move new file mode 100644 index 0000000000000..a222ec7a2cb41 --- /dev/null +++ b/third_party/move/move-compiler-v2/tests/checking/naming/module_struct_same_name.move @@ -0,0 +1,13 @@ +module 0x42::M { + enum M has drop { + M + } +} + +module 0x42::M1 { + use 0x42::M::{Self, M}; + + fun test(_m: M::M): u64 { + 3 + } +} diff --git a/third_party/move/move-compiler-v2/tests/checking/specs/move_function_in_spec_ok.exp b/third_party/move/move-compiler-v2/tests/checking/specs/move_function_in_spec_ok.exp index 4ba45bf526215..b9a89a9fa7950 100644 --- a/third_party/move/move-compiler-v2/tests/checking/specs/move_function_in_spec_ok.exp +++ b/third_party/move/move-compiler-v2/tests/checking/specs/move_function_in_spec_ok.exp @@ -35,7 +35,7 @@ module 0x42::move_function_in_spec { } } spec fun $type_of(): TypeInfo { - Tuple() + Abort(1) } } // end 0x42::move_function_in_spec diff --git a/third_party/move/move-compiler-v2/tests/file-format-generator/bug_14762.exp b/third_party/move/move-compiler-v2/tests/file-format-generator/bug_14762.exp new file mode 100644 index 0000000000000..9d0f6f655fbc9 --- /dev/null +++ b/third_party/move/move-compiler-v2/tests/file-format-generator/bug_14762.exp @@ -0,0 +1,114 @@ + +============ disassembled file-format ================== +// Move bytecode v7 +module c0ffee.m { +use 0000000000000000000000000000000000000000000000000000000000000001::option; +use 0000000000000000000000000000000000000000000000000000000000000001::vector; + + +struct T has copy, drop, store { + issuer: vector, + version: u64 +} +struct J has copy, drop, store { + variant: u64 +} +struct S has copy, drop, store { + entries: vector +} + +test(Arg0: &mut S, Arg1: vector): Option /* def_idx: 0 */ { +L2: loc0: &vector +L3: loc1: bool +L4: loc2: u64 +L5: loc3: u64 +L6: loc4: u64 +L7: loc5: &T +L8: loc6: &T +L9: loc7: u64 +L10: loc8: bool +L11: loc9: u64 +L12: loc10: Option +L13: loc11: Option +B0: + 0: CopyLoc[0](Arg0: &mut S) + 1: ImmBorrowField[0](S.entries: vector) + 2: StLoc[2](loc0: &vector) + 3: LdFalse + 4: StLoc[3](loc1: bool) + 5: LdU64(0) + 6: StLoc[4](loc2: u64) + 7: LdU64(0) + 8: CopyLoc[2](loc0: &vector) + 9: VecLen(2) + 10: StLoc[5](loc3: u64) + 11: StLoc[6](loc4: u64) +B1: + 12: CopyLoc[6](loc4: u64) + 13: CopyLoc[5](loc3: u64) + 14: Lt + 15: BrFalse(41) +B2: + 16: CopyLoc[2](loc0: &vector) + 17: CopyLoc[6](loc4: u64) + 18: VecImmBorrow(2) + 19: StLoc[7](loc5: &T) + 20: MoveLoc[7](loc5: &T) + 21: StLoc[8](loc6: &T) + 22: MoveLoc[8](loc6: &T) + 23: ImmBorrowField[1](T.issuer: vector) + 24: ReadRef + 25: CopyLoc[1](Arg1: vector) + 26: Eq + 27: BrFalse(34) +B3: + 28: LdTrue + 29: StLoc[3](loc1: bool) + 30: MoveLoc[6](loc4: u64) + 31: StLoc[4](loc2: u64) + 32: Branch(43) +B4: + 33: Branch(34) +B5: + 34: LdU64(1) + 35: StLoc[9](loc7: u64) + 36: MoveLoc[6](loc4: u64) + 37: MoveLoc[9](loc7: u64) + 38: Add + 39: StLoc[6](loc4: u64) + 40: Branch(42) +B6: + 41: Branch(43) +B7: + 42: Branch(12) +B8: + 43: Nop + 44: MoveLoc[2](loc0: &vector) + 45: Pop + 46: MoveLoc[3](loc1: bool) + 47: StLoc[10](loc8: bool) + 48: MoveLoc[4](loc2: u64) + 49: StLoc[11](loc9: u64) + 50: MoveLoc[10](loc8: bool) + 51: BrFalse(59) +B9: + 52: MoveLoc[0](Arg0: &mut S) + 53: MutBorrowField[0](S.entries: vector) + 54: MoveLoc[11](loc9: u64) + 55: Call vector::remove(&mut vector, u64): T + 56: Call option::some(T): Option + 57: StLoc[12](loc10: Option) + 58: Branch(63) +B10: + 59: MoveLoc[0](Arg0: &mut S) + 60: Pop + 61: Call option::none(): Option + 62: StLoc[12](loc10: Option) +B11: + 63: MoveLoc[12](loc10: Option) + 64: StLoc[13](loc11: Option) + 65: MoveLoc[13](loc11: Option) + 66: Ret +} +} +============ bytecode verification succeeded ======== diff --git a/third_party/move/move-compiler-v2/tests/file-format-generator/bug_14762.move b/third_party/move/move-compiler-v2/tests/file-format-generator/bug_14762.move new file mode 100644 index 0000000000000..687c39ecd04be --- /dev/null +++ b/third_party/move/move-compiler-v2/tests/file-format-generator/bug_14762.move @@ -0,0 +1,54 @@ +module 0xc0ffee::m { + use std::vector; + use std::option; + use std::option::Option; + + struct J has copy, drop, store { + variant: u64, + } + + + struct T has copy, drop, store { + issuer: vector, + version: u64, + } + + struct S has copy, drop, store { + entries: vector, + } + + public inline fun find(v: &vector, f: |&Element|bool): (bool, u64) { + let find = false; + let found_index = 0; + let i = 0; + let len = vector::length(v); + while (i < len) { + if (f(vector::borrow(v, i))) { + find = true; + found_index = i; + break + }; + i = i + 1; + }; + spec { + assert find ==> f(v[found_index]); + }; + (find, found_index) + } + + fun test(s: &mut S, issuer: vector): Option { + let (found, index) = find(&s.entries, |obj| { + let set: &T = obj; + set.issuer == issuer + }); + + let ret = if (found) { + option::some(vector::remove(&mut s.entries, index)) + } else { + option::none() + }; + + ret + } + +} diff --git a/third_party/move/move-compiler-v2/tests/file-format-generator/bug_14762.opt.exp b/third_party/move/move-compiler-v2/tests/file-format-generator/bug_14762.opt.exp new file mode 100644 index 0000000000000..cce956293c50f --- /dev/null +++ b/third_party/move/move-compiler-v2/tests/file-format-generator/bug_14762.opt.exp @@ -0,0 +1,100 @@ + +============ disassembled file-format ================== +// Move bytecode v7 +module c0ffee.m { +use 0000000000000000000000000000000000000000000000000000000000000001::option; +use 0000000000000000000000000000000000000000000000000000000000000001::vector; + + +struct T has copy, drop, store { + issuer: vector, + version: u64 +} +struct J has copy, drop, store { + variant: u64 +} +struct S has copy, drop, store { + entries: vector +} + +test(Arg0: &mut S, Arg1: vector): Option /* def_idx: 0 */ { +L2: loc0: &vector +L3: loc1: bool +L4: loc2: u64 +L5: loc3: u64 +L6: loc4: u64 +L7: loc5: bool +L8: loc6: u64 +L9: loc7: Option +L10: loc8: Option +B0: + 0: CopyLoc[0](Arg0: &mut S) + 1: ImmBorrowField[0](S.entries: vector) + 2: StLoc[2](loc0: &vector) + 3: LdFalse + 4: StLoc[3](loc1: bool) + 5: LdU64(0) + 6: StLoc[4](loc2: u64) + 7: LdU64(0) + 8: StLoc[5](loc3: u64) + 9: CopyLoc[2](loc0: &vector) + 10: VecLen(2) + 11: StLoc[6](loc4: u64) +B1: + 12: CopyLoc[5](loc3: u64) + 13: CopyLoc[6](loc4: u64) + 14: Lt + 15: BrFalse(28) +B2: + 16: CopyLoc[2](loc0: &vector) + 17: CopyLoc[5](loc3: u64) + 18: VecImmBorrow(2) + 19: ImmBorrowField[1](T.issuer: vector) + 20: ReadRef + 21: CopyLoc[1](Arg1: vector) + 22: Eq + 23: BrFalse(52) +B3: + 24: LdTrue + 25: StLoc[3](loc1: bool) + 26: MoveLoc[5](loc3: u64) + 27: StLoc[4](loc2: u64) +B4: + 28: Nop + 29: MoveLoc[2](loc0: &vector) + 30: Pop + 31: MoveLoc[3](loc1: bool) + 32: StLoc[7](loc5: bool) + 33: MoveLoc[4](loc2: u64) + 34: StLoc[8](loc6: u64) + 35: MoveLoc[7](loc5: bool) + 36: BrFalse(47) +B5: + 37: MoveLoc[0](Arg0: &mut S) + 38: MutBorrowField[0](S.entries: vector) + 39: MoveLoc[8](loc6: u64) + 40: Call vector::remove(&mut vector, u64): T + 41: Call option::some(T): Option + 42: StLoc[9](loc7: Option) +B6: + 43: MoveLoc[9](loc7: Option) + 44: StLoc[10](loc8: Option) + 45: MoveLoc[10](loc8: Option) + 46: Ret +B7: + 47: MoveLoc[0](Arg0: &mut S) + 48: Pop + 49: Call option::none(): Option + 50: StLoc[9](loc7: Option) + 51: Branch(43) +B8: + 52: LdU64(1) + 53: StLoc[8](loc6: u64) + 54: MoveLoc[5](loc3: u64) + 55: MoveLoc[8](loc6: u64) + 56: Add + 57: StLoc[5](loc3: u64) + 58: Branch(12) +} +} +============ bytecode verification succeeded ======== diff --git a/third_party/move/move-compiler-v2/tests/more-v1/expansion/use_struct_overlap_with_module.exp b/third_party/move/move-compiler-v2/tests/more-v1/expansion/use_struct_overlap_with_module.exp index ea5bd62d87125..c656af6345167 100644 --- a/third_party/move/move-compiler-v2/tests/more-v1/expansion/use_struct_overlap_with_module.exp +++ b/third_party/move/move-compiler-v2/tests/more-v1/expansion/use_struct_overlap_with_module.exp @@ -6,8 +6,5 @@ warning: unused alias 7 │ use 0x2::X::{Self, S as X}; │ ^ Unused 'use' of alias 'X'. Consider removing it -error: variants not allowed in this context - ┌─ tests/more-v1/expansion/use_struct_overlap_with_module.move:8:27 - │ -8 │ struct A { f1: X, f2: X::S } - │ ^^^^ + +============ bytecode verification succeeded ======== diff --git a/third_party/move/move-compiler-v2/transactional-tests/tests/no-v1-comparison/bug_14762.exp b/third_party/move/move-compiler-v2/transactional-tests/tests/no-v1-comparison/bug_14762.exp new file mode 100644 index 0000000000000..5d92c423f3fd8 --- /dev/null +++ b/third_party/move/move-compiler-v2/transactional-tests/tests/no-v1-comparison/bug_14762.exp @@ -0,0 +1 @@ +processed 2 tasks diff --git a/third_party/move/move-compiler-v2/transactional-tests/tests/no-v1-comparison/bug_14762.move b/third_party/move/move-compiler-v2/transactional-tests/tests/no-v1-comparison/bug_14762.move new file mode 100644 index 0000000000000..eede6c62f227f --- /dev/null +++ b/third_party/move/move-compiler-v2/transactional-tests/tests/no-v1-comparison/bug_14762.move @@ -0,0 +1,79 @@ +//# publish +module 0x42::m { + use std::vector; + use std::option; + use std::option::Option; + + struct J has copy, drop, store { + variant: u64, + } + + struct T has copy, drop, store { + issuer: vector, + version: u64, + } + + struct S has copy, drop, store { + entries: vector, + } + + public inline fun find(v: &vector, f: |&Element|bool): (bool, u64) { + let find = false; + let found_index = 0; + let i = 0; + let len = vector::length(v); + while (i < len) { + if (f(vector::borrow(v, i))) { + find = true; + found_index = i; + break + }; + i = i + 1; + }; + spec { + assert find ==> f(v[found_index]); + }; + (find, found_index) + } + + fun test(s: &mut S, issuer: vector): Option { + let (found, index) = find(&s.entries, |obj| { + let set: &T = obj; + set.issuer == issuer + }); + + let ret = if (found) { + option::some(vector::remove(&mut s.entries, index)) + } else { + option::none() + }; + + ret + } + + fun test1() { + let t0 = T { + issuer: vector[1], + version: 1 + }; + let t1 = T { + issuer: vector[2], + version: 0 + }; + let s = S { + entries: vector[t0, t1] + }; + let opt_t = test(&mut s, vector[0]); + assert!(option::is_none(&opt_t), 0); + let opt_t = test(&mut s, vector[1]); + assert!(option::is_some(&opt_t), 0); + assert!(option::borrow(&opt_t).issuer == vector[1], 0); + let opt_t = test(&mut s, vector[2]); + assert!(option::is_some(&opt_t), 0); + assert!(option::borrow(&opt_t).issuer == vector[2], 0); + + } + +} + +//# run 0x42::m::test1 diff --git a/third_party/move/move-model/src/builder/exp_builder.rs b/third_party/move/move-model/src/builder/exp_builder.rs index 871bfcda81727..651680c20c8d1 100644 --- a/third_party/move/move-model/src/builder/exp_builder.rs +++ b/third_party/move/move-model/src/builder/exp_builder.rs @@ -3054,10 +3054,11 @@ impl<'env, 'translator, 'module_translator> ExpTranslator<'env, 'translator, 'mo } // Treat this as a call to a global function. - if !self.parent.check_no_variant(maccess) { + let (no_variant, maccess) = self.parent.check_no_variant_and_convert_maccess(maccess); + if !no_variant { return self.new_error_exp(); } - let (module_name, name, _) = self.parent.module_access_to_parts(maccess); + let (module_name, name, _) = self.parent.module_access_to_parts(&maccess); // Process `old(E)` scoping let is_old = diff --git a/third_party/move/move-model/src/builder/module_builder.rs b/third_party/move/move-model/src/builder/module_builder.rs index 5b5cdaeae904c..9f04fe84d1237 100644 --- a/third_party/move/move-model/src/builder/module_builder.rs +++ b/third_party/move/move-model/src/builder/module_builder.rs @@ -48,7 +48,10 @@ use move_compiler::{ parser::ast as PA, shared::{unique_map::UniqueMap, Identifier, Name}, }; -use move_ir_types::{ast::ConstantName, location::Spanned}; +use move_ir_types::{ + ast::ConstantName, + location::{sp, Spanned}, +}; use regex::Regex; use std::{ cell::RefCell, @@ -250,8 +253,8 @@ impl<'env, 'translator> ModuleBuilder<'env, 'translator> { /// Converts a ModuleAccess into a qualified symbol which can be used for lookup of /// types or functions. If the access has a struct variant, an error is produced. pub fn module_access_to_qualified(&self, access: &EA::ModuleAccess) -> QualifiedSymbol { - self.check_no_variant(access); - let (qsym, _) = self.module_access_to_qualified_with_variant(access); + let (_, access) = self.check_no_variant_and_convert_maccess(access); + let (qsym, _) = self.module_access_to_qualified_with_variant(&access); qsym } @@ -262,15 +265,46 @@ impl<'env, 'translator> ModuleBuilder<'env, 'translator> { ) } - pub fn check_no_variant(&self, maccess: &EA::ModuleAccess) -> bool { - if Self::is_variant(maccess) { - self.parent.env.error( - &self.parent.to_loc(&maccess.loc), - "variants not allowed in this context", - ); - false + /// If `maccess` takes the form `ModuleAccess(M, _, Some(V))`, + /// check `M::V` is a struct/enum, constant or schema, + /// if so, return the form `ModuleAccess(M, V, None)`, + /// see how `maccess` is created by + /// function `name_access_chain` in `expansion/translate.rs` + pub fn check_no_variant_and_convert_maccess( + &self, + maccess: &EA::ModuleAccess, + ) -> (bool, EA::ModuleAccess) { + if let EA::ModuleAccess_::ModuleAccess(mident, _, Some(var_name)) = &maccess.value { + let addr = self + .parent + .resolve_address(&self.parent.to_loc(&mident.loc), &mident.value.address); + let name = self + .symbol_pool() + .make(mident.value.module.0.value.as_str()); + let module_name = ModuleName::from_address_bytes_and_name(addr, name); + let var_name_sym = self.symbol_pool().make(var_name.value.as_str()); + let qualitifed_name = QualifiedSymbol { + module_name, + symbol: var_name_sym, + }; + if self.parent.struct_table.contains_key(&qualitifed_name) + || self.parent.spec_schema_table.contains_key(&qualitifed_name) + || self.parent.const_table.contains_key(&qualitifed_name) + { + let new_maccess = sp( + maccess.loc, + EA::ModuleAccess_::ModuleAccess(*mident, *var_name, None), + ); + (true, new_maccess) + } else { + self.parent.env.error( + &self.parent.to_loc(&maccess.loc), + "variants not allowed in this context", + ); + (false, maccess.clone()) + } } else { - true + (true, maccess.clone()) } } @@ -410,7 +444,7 @@ impl<'env, 'translator> ModuleBuilder<'env, 'translator> { self.symbol_pool().make(n.value.as_str()), ), EA::ModuleAccess_::ModuleAccess(mident, n, _) => { - self.check_no_variant(macc); + let (_, macc) = self.check_no_variant_and_convert_maccess(macc); let addr_bytes = self.parent.resolve_address( &self.parent.to_loc(&macc.loc), &mident.value.address, diff --git a/third_party/move/move-prover/boogie-backend/src/lib.rs b/third_party/move/move-prover/boogie-backend/src/lib.rs index 7db2a28429d37..4766ad0175860 100644 --- a/third_party/move/move-prover/boogie-backend/src/lib.rs +++ b/third_party/move/move-prover/boogie-backend/src/lib.rs @@ -184,6 +184,27 @@ pub fn add_prelude( sh_instances = vec![]; bv_instances = vec![]; } + + let mut all_types = mono_info + .all_types + .iter() + .filter(|ty| ty.can_be_type_argument()) + .map(|ty| TypeInfo::new(env, options, ty, false)) + .collect::>() + .into_iter() + .collect_vec(); + let mut bv_all_types = mono_info + .all_types + .iter() + .filter(|ty| ty.can_be_type_argument()) + .map(|ty| TypeInfo::new(env, options, ty, true)) + .filter(|ty_info| !all_types.contains(ty_info)) + .collect::>() + .into_iter() + .collect_vec(); + all_types.append(&mut bv_all_types); + context.insert("uninterpreted_instances", &all_types); + context.insert("sh_instances", &sh_instances); context.insert("bv_instances", &bv_instances); let mut vec_instances = mono_info diff --git a/third_party/move/move-prover/boogie-backend/src/prelude/prelude.bpl b/third_party/move/move-prover/boogie-backend/src/prelude/prelude.bpl index 117131d1cc827..af262f11c9fbf 100644 --- a/third_party/move/move-prover/boogie-backend/src/prelude/prelude.bpl +++ b/third_party/move/move-prover/boogie-backend/src/prelude/prelude.bpl @@ -24,6 +24,18 @@ options provided to the prover. {% include "custom-natives" %} {%- endif %} + +// Uninterpreted function for all types + +{% for instance in uninterpreted_instances %} + +{%- set S = "'" ~ instance.suffix ~ "'" -%} +{%- set T = instance.name -%} + +function $Arbitrary_value_of{{S}}(): {{T}}; + +{% endfor %} + // ============================================================================================ // Primitive Types diff --git a/third_party/move/move-prover/boogie-backend/src/spec_translator.rs b/third_party/move/move-prover/boogie-backend/src/spec_translator.rs index 53235c65f1116..43e54ef7ceff7 100644 --- a/third_party/move/move-prover/boogie-backend/src/spec_translator.rs +++ b/third_party/move/move-prover/boogie-backend/src/spec_translator.rs @@ -724,13 +724,18 @@ impl<'env> SpecTranslator<'env> { // Single-element sequence is just a wrapped value. self.translate_exp(exp_vec.first().expect("list has an element")); }, - ExpData::Return(..) - | ExpData::Sequence(..) - | ExpData::Loop(..) - | ExpData::Assign(..) - | ExpData::Mutate(..) - | ExpData::SpecBlock(..) - | ExpData::LoopCont(..) => panic!("imperative expressions not supported"), + ExpData::Return(id, ..) + | ExpData::Sequence(id, ..) + | ExpData::Loop(id, ..) + | ExpData::Assign(id, ..) + | ExpData::Mutate(id, ..) + | ExpData::SpecBlock(id, ..) + | ExpData::LoopCont(id, ..) => { + self.env.error( + &self.env.get_node_loc(*id), + "imperative expressions not supported in specs", + ); + }, } } @@ -991,15 +996,31 @@ impl<'env> SpecTranslator<'env> { self.translate_call(node_id, oper, &[args[args.len() - 1].clone()]); emit!(self.writer, &")".repeat(count)); }, + Operation::Abort => { + let exp_bv_flag = global_state.get_node_num_oper(node_id) == Bitwise; + emit!( + self.writer, + &format!( + "$Arbitrary_value_of'{}'()", + boogie_type_suffix_bv(self.env, &self.get_node_type(node_id), exp_bv_flag) + ) + ); + }, Operation::MoveFunction(_, _) | Operation::BorrowGlobal(_) | Operation::Borrow(..) | Operation::Deref | Operation::MoveTo | Operation::MoveFrom - | Operation::Abort | Operation::Old => { - panic!("operation unexpected: {}", oper.display(self.env, node_id)) + self.env.error( + &self.env.get_node_loc(node_id), + &format!( + "bug: operation {} is not supported \ + in the current context", + oper.display(self.env, node_id) + ), + ); }, } } diff --git a/third_party/move/move-prover/tests/sources/functional/ModifiesErrorTest.v2_exp b/third_party/move/move-prover/tests/sources/functional/ModifiesErrorTest.v2_exp index a4c827857f788..620e4ee6a4e64 100644 --- a/third_party/move/move-prover/tests/sources/functional/ModifiesErrorTest.v2_exp +++ b/third_party/move/move-prover/tests/sources/functional/ModifiesErrorTest.v2_exp @@ -1,64 +1,4 @@ Move prover returns: exiting with verification errors -warning: Unused assignment to `x0`. Consider removing or prefixing with an underscore: `_x0` - ┌─ tests/sources/functional/ModifiesErrorTest.move:37:18 - │ -37 │ let x0 = A::read_at(addr2); - │ ^^^^^^^^^^^^^^^^^ - -warning: Unused assignment to `x1`. Consider removing or prefixing with an underscore: `_x1` - ┌─ tests/sources/functional/ModifiesErrorTest.move:40:18 - │ -40 │ let x1 = A::read_at(addr2); - │ ^^^^^^^^^^^^^^^^^ - -warning: Unused assignment to `x0`. Consider removing or prefixing with an underscore: `_x0` - ┌─ tests/sources/functional/ModifiesErrorTest.move:51:18 - │ -51 │ let x0 = A::read_at(addr2); - │ ^^^^^^^^^^^^^^^^^ - -warning: Unused assignment to `x1`. Consider removing or prefixing with an underscore: `_x1` - ┌─ tests/sources/functional/ModifiesErrorTest.move:53:18 - │ -53 │ let x1 = A::read_at(addr2); - │ ^^^^^^^^^^^^^^^^^ - -warning: Unused assignment to `x0`. Consider removing or prefixing with an underscore: `_x0` - ┌─ tests/sources/functional/ModifiesErrorTest.move:64:18 - │ -64 │ let x0 = A::read_at(addr2); - │ ^^^^^^^^^^^^^^^^^ - -warning: Unused assignment to `x1`. Consider removing or prefixing with an underscore: `_x1` - ┌─ tests/sources/functional/ModifiesErrorTest.move:66:18 - │ -66 │ let x1 = A::read_at(addr2); - │ ^^^^^^^^^^^^^^^^^ - -warning: Unused assignment to `x0`. Consider removing or prefixing with an underscore: `_x0` - ┌─ tests/sources/functional/ModifiesErrorTest.move:78:18 - │ -78 │ let x0 = A::read_at(addr2); - │ ^^^^^^^^^^^^^^^^^ - -warning: Unused assignment to `x1`. Consider removing or prefixing with an underscore: `_x1` - ┌─ tests/sources/functional/ModifiesErrorTest.move:80:18 - │ -80 │ let x1 = A::read_at(addr2); - │ ^^^^^^^^^^^^^^^^^ - -warning: Unused assignment to `x0`. Consider removing or prefixing with an underscore: `_x0` - ┌─ tests/sources/functional/ModifiesErrorTest.move:91:18 - │ -91 │ let x0 = A::read_at(addr); - │ ^^^^^^^^^^^^^^^^ - -warning: Unused assignment to `x1`. Consider removing or prefixing with an underscore: `_x1` - ┌─ tests/sources/functional/ModifiesErrorTest.move:93:18 - │ -93 │ let x1 = A::read_at(addr); - │ ^^^^^^^^^^^^^^^^ - error: caller does not have permission to modify `B::T` at given address ┌─ tests/sources/functional/ModifiesErrorTest.move:38:17 │ diff --git a/third_party/move/move-prover/tests/sources/functional/ModifiesTest.v2_exp b/third_party/move/move-prover/tests/sources/functional/ModifiesTest.v2_exp deleted file mode 100644 index 66abe10c08bf4..0000000000000 --- a/third_party/move/move-prover/tests/sources/functional/ModifiesTest.v2_exp +++ /dev/null @@ -1,11 +0,0 @@ -warning: Unused assignment to `x0`. Consider removing or prefixing with an underscore: `_x0` - ┌─ tests/sources/functional/ModifiesTest.move:67:18 - │ -67 │ let x0 = A::read_at(addr2); - │ ^^^^^^^^^^^^^^^^^ - -warning: Unused assignment to `x1`. Consider removing or prefixing with an underscore: `_x1` - ┌─ tests/sources/functional/ModifiesTest.move:69:18 - │ -69 │ let x1 = A::read_at(addr2); - │ ^^^^^^^^^^^^^^^^^ diff --git a/third_party/move/move-prover/tests/sources/functional/abort_in_fun.move b/third_party/move/move-prover/tests/sources/functional/abort_in_fun.move new file mode 100644 index 0000000000000..bc1fbf119fd3f --- /dev/null +++ b/third_party/move/move-prover/tests/sources/functional/abort_in_fun.move @@ -0,0 +1,121 @@ +module 0x42::TestAbortInFunction { + + fun aborts_with(x: u64, y: u64): u64 { + if (x == 1) { + abort 2 + } else if (y == 2) { + abort 3 + } else { + x + } + } + spec aborts_with { + aborts_if x == 1 with 2; + aborts_if y == 2 with 3; + ensures result == x; + } + + fun call_aborts_with(): u64 { + aborts_with(2, 3) + } + + spec call_aborts_with { + ensures result == aborts_with(2, 3); + } + + fun abort_generic(x: Element, y: Element): Element { + if (x == y) { + abort 0 + } else { + x + } + } + + fun call_aborts_generic(): u64 { + abort_generic(2, 3) + } + + spec call_aborts_generic { + ensures result == abort_generic(2, 3); + } + + struct S has copy, drop { + value: Element + } + + fun abort_generic_struct(x: S, y: S): S { + if (x == y) { + abort 0 + } else { + x + } + } + + fun spec_abort_generic_struct(x: S, y: S): S { + if (x == y) { + abort 0 + } else { + x + } + } + + fun call_abort_generic_struct(x: Element, y: Element): Element { + let sx = S { + value: x + }; + let sy = S { + value: y + }; + abort_generic_struct(sx, sy).value + } + + spec call_abort_generic_struct { + aborts_if x == y; + ensures result == call_abort_generic_struct(x, y); + } + + struct T has copy, drop { + v: u64 + } + + spec T { + pragma bv=b"0"; + } + + fun call_abort_generic_struct_concrete(x: u64, y: u64, test_assert1: bool): T { + let sx = S { + value: T { + v: x + } + }; + let sy = S { + value: T { + v: y + } + }; + assert!(test_assert1, 0); + abort_generic_struct(sx, sy).value + } + + spec call_abort_generic_struct_concrete { + aborts_if x == y; + aborts_if !test_assert1; + ensures result == call_abort_generic_struct_concrete(x, y, test_assert1); + ensures result == spec_call_abort_generic_struct_concrete(x, y); + } + + spec fun spec_call_abort_generic_struct_concrete(x: u64, y: u64): T { + let sx = S { + value: T { + v: x + } + }; + let sy = S { + value: T { + v: y + } + }; + spec_abort_generic_struct(sx, sy).value + } + +} diff --git a/third_party/move/move-prover/tests/sources/functional/bitset.v2_exp b/third_party/move/move-prover/tests/sources/functional/bitset.v2_exp deleted file mode 100644 index e50ba75b20d2e..0000000000000 --- a/third_party/move/move-prover/tests/sources/functional/bitset.v2_exp +++ /dev/null @@ -1,11 +0,0 @@ -warning: Unused assignment to `s2`. Consider removing or prefixing with an underscore: `_s2` - ┌─ tests/sources/functional/bitset.move:59:18 - │ -59 │ let s2 = intersect(s, s); - │ ^^^^^^^^^^^^^^^ - -warning: Unused assignment to `s3`. Consider removing or prefixing with an underscore: `_s3` - ┌─ tests/sources/functional/bitset.move:60:18 - │ -60 │ let s3 = union(s, s); - │ ^^^^^^^^^^^ diff --git a/third_party/move/move-prover/tests/sources/functional/bitwise_features.v2_exp b/third_party/move/move-prover/tests/sources/functional/bitwise_features.v2_exp deleted file mode 100644 index 7c523ceea6767..0000000000000 --- a/third_party/move/move-prover/tests/sources/functional/bitwise_features.v2_exp +++ /dev/null @@ -1,5 +0,0 @@ -warning: Unused assignment to `old_features`. Consider removing or prefixing with an underscore: `_old_features` - ┌─ tests/sources/functional/bitwise_features.move:50:28 - │ -50 │ let old_features = *features; // ghost var - │ ^^^^^^^^^ diff --git a/third_party/move/move-prover/tests/sources/functional/bug_14762.exp b/third_party/move/move-prover/tests/sources/functional/bug_14762.exp new file mode 100644 index 0000000000000..4b2683e417420 --- /dev/null +++ b/third_party/move/move-prover/tests/sources/functional/bug_14762.exp @@ -0,0 +1,9 @@ +Move prover returns: exiting with model building errors +error: invalid transfer of references + ┌─ tests/sources/functional/bug_14762.move:45:26 + │ +39 │ let (found, index) = find(&s.entries, |obj| { + │ ---------- It is still being borrowed by this reference + · +45 │ option::some(vector::remove(&mut s.entries, index)) + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Invalid usage of reference as function argument. Cannot transfer a mutable reference that is being borrowed diff --git a/third_party/move/move-prover/tests/sources/functional/bug_14762.move b/third_party/move/move-prover/tests/sources/functional/bug_14762.move new file mode 100644 index 0000000000000..b1e1587553120 --- /dev/null +++ b/third_party/move/move-prover/tests/sources/functional/bug_14762.move @@ -0,0 +1,69 @@ +module 0x42::m { + use std::vector; + use std::option; + use std::option::Option; + + struct J has copy, drop, store { + variant: u64, + } + + struct T has copy, drop, store { + issuer: vector, + version: u64, + } + + struct S has copy, drop, store { + entries: vector, + } + + public inline fun find(v: &vector, f: |&Element|bool): (bool, u64) { + let find = false; + let found_index = 0; + let i = 0; + let len = vector::length(v); + while (i < len) { + if (f(vector::borrow(v, i))) { + find = true; + found_index = i; + break + }; + i = i + 1; + }; + spec { + assert find ==> f(v[found_index]); + }; + (find, found_index) + } + + fun test(s: &mut S, issuer: vector): Option { + let (found, index) = find(&s.entries, |obj| { + let set: &T = obj; + set.issuer == issuer + }); + + let ret = if (found) { + option::some(vector::remove(&mut s.entries, index)) + } else { + option::none() + }; + + ret + } + + fun test1() { + let t0 = T { + issuer: vector[1], + version: 1 + }; + let t1 = T { + issuer: vector[2], + version: 0 + }; + let s = S { + entries: vector[t0, t1] + }; + let _opt_t = test(&mut s, vector[0]); + + } + +} diff --git a/third_party/move/move-prover/tests/sources/functional/inline-lambda.v2_exp b/third_party/move/move-prover/tests/sources/functional/inline-lambda.v2_exp index a939ef7f0c9c6..4d220149d9a54 100644 --- a/third_party/move/move-prover/tests/sources/functional/inline-lambda.v2_exp +++ b/third_party/move/move-prover/tests/sources/functional/inline-lambda.v2_exp @@ -1,16 +1,4 @@ Move prover returns: exiting with verification errors -warning: Unused assignment to `r1`. Consider removing or prefixing with an underscore: `_r1` - ┌─ tests/sources/functional/inline-lambda.move:11:31 - │ -11 │ let r1 = apply(0, |v| v >= 0); - │ ^^^^^^ - -warning: Unused assignment to `r2`. Consider removing or prefixing with an underscore: `_r2` - ┌─ tests/sources/functional/inline-lambda.move:16:31 - │ -16 │ let r2 = apply(0, |v| v != a1 + a2); - │ ^^^^^^^^^^^^ - error: unknown assertion failed ┌─ tests/sources/functional/inline-lambda.move:5:13 │ diff --git a/third_party/move/move-prover/tests/sources/functional/inline_fun_simple.v2_exp b/third_party/move/move-prover/tests/sources/functional/inline_fun_simple.v2_exp index a71d8eee327fb..00f90006edacb 100644 --- a/third_party/move/move-prover/tests/sources/functional/inline_fun_simple.v2_exp +++ b/third_party/move/move-prover/tests/sources/functional/inline_fun_simple.v2_exp @@ -1,28 +1,4 @@ Move prover returns: exiting with verification errors -warning: Unused assignment to `r1`. Consider removing or prefixing with an underscore: `_r1` - ┌─ tests/sources/functional/inline_fun_simple.move:10:32 - │ -10 │ let r1 = apply(42, |v| v >= 1); - │ ^^^^^^ - -warning: Unused assignment to `r2`. Consider removing or prefixing with an underscore: `_r2` - ┌─ tests/sources/functional/inline_fun_simple.move:15:32 - │ -15 │ let r2 = apply(43, |v| v <= 2); - │ ^^^^^^ - -warning: Unused assignment to `r1`. Consider removing or prefixing with an underscore: `_r1` - ┌─ tests/sources/functional/inline_fun_simple.move:22:32 - │ -22 │ let r1 = apply(42, |v| v >= 1); - │ ^^^^^^ - -warning: Unused assignment to `r2`. Consider removing or prefixing with an underscore: `_r2` - ┌─ tests/sources/functional/inline_fun_simple.move:27:31 - │ -27 │ let r2 = apply(3, |v| v <= 2); - │ ^^^^^^ - error: unknown assertion failed ┌─ tests/sources/functional/inline_fun_simple.move:4:13 │ diff --git a/third_party/move/move-prover/tests/sources/functional/loops_with_memory_ops.exp b/third_party/move/move-prover/tests/sources/functional/loops_with_memory_ops.exp index 6a012dba56f8d..32b6d76bc6da8 100644 --- a/third_party/move/move-prover/tests/sources/functional/loops_with_memory_ops.exp +++ b/third_party/move/move-prover/tests/sources/functional/loops_with_memory_ops.exp @@ -45,6 +45,7 @@ error: induction case of the loop invariant does not hold = at tests/sources/functional/loops_with_memory_ops.move:80: nested_loop2 = at tests/sources/functional/loops_with_memory_ops.move:81: nested_loop2 = b = + = at tests/sources/functional/loops_with_memory_ops.move:81: nested_loop2 = a = = at tests/sources/functional/loops_with_memory_ops.move:85: nested_loop2 = i = @@ -104,8 +105,8 @@ error: unknown assertion failed = at tests/sources/functional/loops_with_memory_ops.move:80: nested_loop2 = at tests/sources/functional/loops_with_memory_ops.move:81: nested_loop2 = b = - = a = = at tests/sources/functional/loops_with_memory_ops.move:81: nested_loop2 + = a = = at tests/sources/functional/loops_with_memory_ops.move:85: nested_loop2 = i = = at tests/sources/functional/loops_with_memory_ops.move:86: nested_loop2 diff --git a/third_party/move/move-prover/tests/sources/functional/loops_with_memory_ops.v2_exp b/third_party/move/move-prover/tests/sources/functional/loops_with_memory_ops.v2_exp index 00ce62137346b..294a93f8a4f5a 100644 --- a/third_party/move/move-prover/tests/sources/functional/loops_with_memory_ops.v2_exp +++ b/third_party/move/move-prover/tests/sources/functional/loops_with_memory_ops.v2_exp @@ -43,7 +43,6 @@ error: induction case of the loop invariant does not hold = at tests/sources/functional/loops_with_memory_ops.move:81: nested_loop2 = a = = b = - = at tests/sources/functional/loops_with_memory_ops.move:81: nested_loop2 = at tests/sources/functional/loops_with_memory_ops.move:85: nested_loop2 = = = i = @@ -100,9 +99,8 @@ error: unknown assertion failed = at tests/sources/functional/loops_with_memory_ops.move:75: nested_loop2 = at tests/sources/functional/loops_with_memory_ops.move:80: nested_loop2 = at tests/sources/functional/loops_with_memory_ops.move:81: nested_loop2 - = a = - = at tests/sources/functional/loops_with_memory_ops.move:81: nested_loop2 = b = + = at tests/sources/functional/loops_with_memory_ops.move:81: nested_loop2 = at tests/sources/functional/loops_with_memory_ops.move:85: nested_loop2 = = = i = diff --git a/third_party/move/move-prover/tests/sources/functional/spec_fun_imperative_expression_err.exp b/third_party/move/move-prover/tests/sources/functional/spec_fun_imperative_expression_err.exp new file mode 100644 index 0000000000000..0fb3da8c0043d --- /dev/null +++ b/third_party/move/move-prover/tests/sources/functional/spec_fun_imperative_expression_err.exp @@ -0,0 +1,13 @@ +Move prover returns: exiting with condition generation errors +error: imperative expressions not supported in specs + ┌─ tests/sources/functional/spec_fun_imperative_expression_err.move:2:27 + │ + 2 │ fun sequential(): u64 { + │ ╭───────────────────────────^ + 3 │ │ let _x = 2; + 4 │ │ let _y = 3; + 5 │ │ while(_y > 0) { + · │ +16 │ │ _x +17 │ │ } + │ ╰─────^ diff --git a/third_party/move/move-prover/tests/sources/functional/spec_fun_imperative_expression_err.move b/third_party/move/move-prover/tests/sources/functional/spec_fun_imperative_expression_err.move new file mode 100644 index 0000000000000..2196a453b5bc3 --- /dev/null +++ b/third_party/move/move-prover/tests/sources/functional/spec_fun_imperative_expression_err.move @@ -0,0 +1,26 @@ +module 0x42::M { + fun sequential(): u64 { + let _x = 2; + let _y = 3; + while(_y > 0) { + break + }; + if (_x > 0) { + abort(0) + }; + let _z = if (_x > 5) { + _x + } else { + _y + }; + _x + } + + fun m() { + let _z = 2; + spec { + assert _z == sequential(); + }; + } + +} diff --git a/third_party/move/move-prover/tests/sources/functional/spec_fun_imperative_expression_err.v2_exp b/third_party/move/move-prover/tests/sources/functional/spec_fun_imperative_expression_err.v2_exp new file mode 100644 index 0000000000000..0fb3da8c0043d --- /dev/null +++ b/third_party/move/move-prover/tests/sources/functional/spec_fun_imperative_expression_err.v2_exp @@ -0,0 +1,13 @@ +Move prover returns: exiting with condition generation errors +error: imperative expressions not supported in specs + ┌─ tests/sources/functional/spec_fun_imperative_expression_err.move:2:27 + │ + 2 │ fun sequential(): u64 { + │ ╭───────────────────────────^ + 3 │ │ let _x = 2; + 4 │ │ let _y = 3; + 5 │ │ while(_y > 0) { + · │ +16 │ │ _x +17 │ │ } + │ ╰─────^ diff --git a/third_party/move/move-prover/tests/sources/functional/specs_in_fun.v2_exp b/third_party/move/move-prover/tests/sources/functional/specs_in_fun.v2_exp index a41ce4e659842..4dc5d49a730b3 100644 --- a/third_party/move/move-prover/tests/sources/functional/specs_in_fun.v2_exp +++ b/third_party/move/move-prover/tests/sources/functional/specs_in_fun.v2_exp @@ -1,28 +1,4 @@ Move prover returns: exiting with verification errors -warning: Unused assignment to `y`. Consider removing or prefixing with an underscore: `_y` - ┌─ tests/sources/functional/specs_in_fun.move:18:9 - │ -18 │ y = x + 1; - │ ^^^^^^^^^ - -warning: Unused assignment to `z`. Consider removing or prefixing with an underscore: `_z` - ┌─ tests/sources/functional/specs_in_fun.move:33:9 - │ -33 │ z = x + y; - │ ^^^^^^^^^ - -warning: Unused assignment to `y`. Consider removing or prefixing with an underscore: `_y` - ┌─ tests/sources/functional/specs_in_fun.move:51:9 - │ -51 │ y = x + 1; - │ ^^^^^^^^^ - -warning: Unused assignment to `z`. Consider removing or prefixing with an underscore: `_z` - ┌─ tests/sources/functional/specs_in_fun.move:66:9 - │ -66 │ z = x + y; - │ ^^^^^^^^^ - error: unknown assertion failed ┌─ tests/sources/functional/specs_in_fun.move:45:13 │ diff --git a/third_party/move/move-prover/tests/sources/functional/specs_in_fun_ref.v2_exp b/third_party/move/move-prover/tests/sources/functional/specs_in_fun_ref.v2_exp deleted file mode 100644 index a7c557355adc9..0000000000000 --- a/third_party/move/move-prover/tests/sources/functional/specs_in_fun_ref.v2_exp +++ /dev/null @@ -1,29 +0,0 @@ -warning: Unused assignment to `z`. Consider removing or prefixing with an underscore: `_z` - ┌─ tests/sources/functional/specs_in_fun_ref.move:11:9 - │ -11 │ z = x + y; - │ ^^^^^^^^^ - -warning: Unused assignment to `z`. Consider removing or prefixing with an underscore: `_z` - ┌─ tests/sources/functional/specs_in_fun_ref.move:44:9 - │ -44 │ z = *x + *y; - │ ^^^^^^^^^^^ - -warning: Unused assignment to `vx`. Consider removing or prefixing with an underscore: `_vx` - ┌─ tests/sources/functional/specs_in_fun_ref.move:45:18 - │ -45 │ let vx = *x; - │ ^^ - -warning: Unused assignment to `vy`. Consider removing or prefixing with an underscore: `_vy` - ┌─ tests/sources/functional/specs_in_fun_ref.move:46:18 - │ -46 │ let vy = *y; - │ ^^ - -warning: Unused assignment to `fx`. Consider removing or prefixing with an underscore: `_fx` - ┌─ tests/sources/functional/specs_in_fun_ref.move:47:18 - │ -47 │ let fx = freeze(x); - │ ^^^^^^^^^ diff --git a/third_party/move/move-prover/tests/sources/functional/verify_custom_table.v2_exp b/third_party/move/move-prover/tests/sources/functional/verify_custom_table.v2_exp index 53156854c8c86..a859dc99038f4 100644 --- a/third_party/move/move-prover/tests/sources/functional/verify_custom_table.v2_exp +++ b/third_party/move/move-prover/tests/sources/functional/verify_custom_table.v2_exp @@ -1,16 +1,4 @@ Move prover returns: exiting with verification errors -warning: Unused assignment to `k`. Consider removing or prefixing with an underscore: `_k` - ┌─ tests/sources/functional/verify_custom_table.move:115:22 - │ -115 │ let (k, v) = table::remove_return_key(&mut t, 2); - │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - -warning: Unused assignment to `v`. Consider removing or prefixing with an underscore: `_v` - ┌─ tests/sources/functional/verify_custom_table.move:115:22 - │ -115 │ let (k, v) = table::remove_return_key(&mut t, 2); - │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - error: post-condition does not hold ┌─ tests/sources/functional/verify_custom_table.move:76:9 │ diff --git a/third_party/move/move-prover/tests/sources/regression/bug_828.v2_exp b/third_party/move/move-prover/tests/sources/regression/bug_828.v2_exp deleted file mode 100644 index 1632a07cecacd..0000000000000 --- a/third_party/move/move-prover/tests/sources/regression/bug_828.v2_exp +++ /dev/null @@ -1,5 +0,0 @@ -warning: Unused assignment to `old_i`. Consider removing or prefixing with an underscore: `_old_i` - ┌─ tests/sources/regression/bug_828.move:4:21 - │ -4 │ let old_i = i; - │ ^ diff --git a/types/src/proof/accumulator/mod.rs b/types/src/proof/accumulator/mod.rs index 5858db4438959..89bb18ba2269e 100644 --- a/types/src/proof/accumulator/mod.rs +++ b/types/src/proof/accumulator/mod.rs @@ -85,10 +85,14 @@ where } pub fn new_empty() -> Self { + Self::new_empty_with_root_hash(*ACCUMULATOR_PLACEHOLDER_HASH) + } + + pub fn new_empty_with_root_hash(root_hash: HashValue) -> Self { Self { frozen_subtree_roots: Vec::new(), num_leaves: 0, - root_hash: *ACCUMULATOR_PLACEHOLDER_HASH, + root_hash, phantom: PhantomData, } } diff --git a/types/src/state_store/mod.rs b/types/src/state_store/mod.rs index f9632a48ff86c..3b16a4f75d6fe 100644 --- a/types/src/state_store/mod.rs +++ b/types/src/state_store/mod.rs @@ -102,25 +102,14 @@ pub fn create_empty_sharded_state_updates() -> ShardedStateUpdates { arr![HashMap::new(); 16] } -pub fn combine_or_add_sharded_state_updates( - lhs: &mut Option, - rhs: ShardedStateUpdates, -) { - if let Some(lhs) = lhs { - combine_sharded_state_updates(lhs, rhs); - } else { - *lhs = Some(rhs); - } -} - -pub fn combine_sharded_state_updates(lhs: &mut ShardedStateUpdates, rhs: ShardedStateUpdates) { +pub fn combine_sharded_state_updates(lhs: &mut ShardedStateUpdates, rhs: &ShardedStateUpdates) { use rayon::prelude::*; THREAD_MANAGER.get_exe_cpu_pool().install(|| { lhs.par_iter_mut() - .zip_eq(rhs.into_par_iter()) + .zip_eq(rhs.par_iter()) .for_each(|(l, r)| { - l.extend(r); + l.extend(r.clone()); }) }) } diff --git a/types/src/transaction/mod.rs b/types/src/transaction/mod.rs index f3e8a96dfc5c4..a76817793e90c 100644 --- a/types/src/transaction/mod.rs +++ b/types/src/transaction/mod.rs @@ -1567,6 +1567,15 @@ impl TransactionToCommit { } } + #[cfg(any(test, feature = "fuzzing"))] + pub fn dummy_with_transaction(transaction: Transaction) -> Self { + Self { + transaction, + transaction_info: TransactionInfo::dummy(), + ..Self::dummy() + } + } + #[cfg(any(test, feature = "fuzzing"))] pub fn dummy_with_transaction_auxiliary_data( transaction_auxiliary_data: TransactionAuxiliaryData,