diff --git a/Cargo.lock b/Cargo.lock index 78f0c82bf..0593c2899 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1410,7 +1410,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0c2a198fb6b0eada2a8df47933734e6d35d350665a33a3593d7164fa52c75c19" dependencies = [ "cfg-if", - "windows-targets 0.48.5", + "windows-targets 0.52.5", ] [[package]] @@ -2911,6 +2911,17 @@ dependencies = [ "tokio", ] +[[package]] +name = "tokio-stream" +version = "0.1.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "267ac89e0bec6e691e5813911606935d77c476ff49024f98abcea3e7b15e37af" +dependencies = [ + "futures-core", + "pin-project-lite", + "tokio", +] + [[package]] name = "tokio-util" version = "0.7.11" @@ -3307,6 +3318,7 @@ dependencies = [ "everscale-crypto", "futures-util", "hex", + "humantime", "itertools", "metrics", "parking_lot", @@ -3317,6 +3329,8 @@ dependencies = [ "sha2", "tikv-jemallocator", "tokio", + "tokio-stream", + "tokio-util", "tracing", "tracing-appender", "tracing-flame", diff --git a/Cargo.toml b/Cargo.toml index 16fd2e322..61fe6156c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -86,6 +86,7 @@ tikv-jemallocator = { version = "0.5", features = [ ] } tl-proto = "0.4" tokio = { version = "1", default-features = false } +tokio-stream = "0.1.15" tokio-util = { version = "0.7.10", features = ["codec"] } tower = "0.4" tower-http = "0.5" diff --git a/collator/src/mempool/mempool_adapter.rs b/collator/src/mempool/mempool_adapter.rs index cfced4a0c..46236a5a9 100644 --- a/collator/src/mempool/mempool_adapter.rs +++ b/collator/src/mempool/mempool_adapter.rs @@ -80,9 +80,9 @@ pub trait MempoolAdapter: Send + Sync + 'static { pub struct MempoolAdapterStdImpl { // TODO: replace with rocksdb anchors: Arc>>>, - externals_tx: tokio::sync::mpsc::UnboundedSender, + externals_tx: mpsc::UnboundedSender, - anchor_added: Arc, + anchor_added: Arc, } impl MempoolAdapterStdImpl { @@ -95,8 +95,7 @@ impl MempoolAdapterStdImpl { tracing::info!(target: tracing_targets::MEMPOOL_ADAPTER, "Creating mempool adapter..."); let anchors = Arc::new(RwLock::new(IndexMap::new())); - let (sender, receiver) = - tokio::sync::mpsc::unbounded_channel::<(Arc, Vec>)>(); + let (sender, receiver) = mpsc::unbounded_channel(); // TODO receive from outside let (externals_tx, externals_rx) = mpsc::unbounded_channel(); @@ -150,18 +149,19 @@ impl MempoolAdapterFactory for Arc { pub async fn handle_anchors( adapter: Arc, - mut rx: UnboundedReceiver<(Arc, Vec>)>, + mut rx: UnboundedReceiver<(Point, Vec)>, ) { let mut cache = ExternalMessageCache::new(1000); while let Some((anchor, points)) = rx.recv().await { + let anchor_id: MempoolAnchorId = anchor.body().location.round.0; let mut messages = Vec::new(); let mut total_messages = 0; let mut total_bytes = 0; let mut messages_bytes = 0; for point in points.iter() { - total_messages += point.body.payload.len(); - 'message: for message in &point.body.payload { + total_messages += point.body().payload.len(); + 'message: for message in &point.body().payload { total_bytes += message.len(); let cell = match Boc::decode(message) { Ok(cell) => cell, @@ -191,15 +191,14 @@ pub async fn handle_anchors( } }; - if cache.check_unique(anchor.body.location.round.0, cell.repr_hash()) { + if cache.check_unique(anchor_id, cell.repr_hash()) { messages.push(Arc::new(ExternalMessage::new(cell.clone(), ext_in_message))); messages_bytes += message.len(); } } } - cache.clean(anchor.body.location.round.0); - metrics::gauge!("tycho_mempool_last_anchor_round").set(anchor.body.location.round.0); + metrics::gauge!("tycho_mempool_last_anchor_round").set(anchor.body().location.round.0); metrics::counter!("tycho_mempool_externals_count_total").increment(messages.len() as _); metrics::counter!("tycho_mempool_externals_bytes_total").increment(messages_bytes as _); metrics::counter!("tycho_mempool_duplicates_count_total") @@ -209,20 +208,22 @@ pub async fn handle_anchors( tracing::info!( target: tracing_targets::MEMPOOL_ADAPTER, - round = anchor.body.location.round.0, - time = anchor.body.time.as_u64(), + round = anchor_id, + time = anchor.body().time.as_u64(), externals_unique = messages.len(), externals_skipped = total_messages - messages.len(), "new anchor" ); let anchor = Arc::new(MempoolAnchor::new( - anchor.body.location.round.0, - anchor.body.time.as_u64(), + anchor_id, + anchor.body().time.as_u64(), messages, )); adapter.add_anchor(anchor); + + cache.clean(anchor_id); } } diff --git a/consensus/Cargo.toml b/consensus/Cargo.toml index 11b509032..b881c2f75 100644 --- a/consensus/Cargo.toml +++ b/consensus/Cargo.toml @@ -11,10 +11,12 @@ license.workspace = true [[example]] name = "consensus-node" path = "examples/consensus_node.rs" +required-features = ["test"] [[example]] name = "engine" path = "examples/engine/main.rs" +required-features = ["test"] [dependencies] ahash = { workspace = true } @@ -29,11 +31,13 @@ futures-util = { workspace = true } itertools = { workspace = true } metrics = { workspace = true } parking_lot = { workspace = true } -rand = { workspace = true, features = ["small_rng"] } +rand = { workspace = true } rand_pcg = { workspace = true } serde = { workspace = true, features = ["derive"] } sha2 = { workspace = true } tokio = { workspace = true, default-features = false } +tokio-stream = { workspace = true, optional = true } +tokio-util = { workspace = true } tracing = { workspace = true } weedb = { workspace = true } @@ -43,6 +47,7 @@ tycho-storage = { workspace = true } tycho-util = { workspace = true } [dev-dependencies] +humantime = { workspace = true } parking_lot = { workspace = true, features = ["deadlock_detection"] } tikv-jemallocator = { workspace = true, features = [ "unprefixed_malloc_on_supported_platforms", @@ -62,3 +67,6 @@ tycho-util = { workspace = true, features = ["test"] } [lints] workspace = true + +[features] +test = ["dep:tokio-stream"] \ No newline at end of file diff --git a/consensus/examples/consensus_node.rs b/consensus/examples/consensus_node.rs index f8a4ea2c9..807b27d8a 100644 --- a/consensus/examples/consensus_node.rs +++ b/consensus/examples/consensus_node.rs @@ -15,7 +15,7 @@ use serde::{Deserialize, Serialize}; use tokio::sync::mpsc; use tracing_subscriber::layer::SubscriberExt; use tracing_subscriber::{fmt, EnvFilter, Layer}; -use tycho_consensus::test_utils::drain_anchors; +use tycho_consensus::test_utils::AnchorConsumer; use tycho_consensus::{Engine, InputBufferStub}; use tycho_network::{Address, DhtConfig, NetworkConfig, PeerId, PeerInfo}; use tycho_util::time::now_sec; @@ -140,7 +140,9 @@ impl CmdRun { InputBufferStub::new(100, 5), ); engine.init_with_genesis(all_peers.as_slice()).await; - tokio::spawn(drain_anchors(committed_rx)); + let mut anchor_consumer = AnchorConsumer::default(); + anchor_consumer.add(local_id, committed_rx); + tokio::spawn(anchor_consumer.drain()); tracing::info!( local_id = %dht_client.network().peer_id(), diff --git a/consensus/examples/engine/main.rs b/consensus/examples/engine/main.rs index 0fc345e69..892ee7eee 100644 --- a/consensus/examples/engine/main.rs +++ b/consensus/examples/engine/main.rs @@ -6,20 +6,18 @@ use std::time::Duration; use clap::Parser; use everscale_crypto::ed25519::{KeyPair, SecretKey}; use parking_lot::deadlock; -use tokio::sync::{mpsc, Mutex}; +use tokio::sync::mpsc; use tycho_consensus::test_utils::*; use tycho_consensus::{Engine, InputBufferStub}; use tycho_network::{Address, DhtConfig, NetworkConfig, PeerId}; -use tycho_util::FastHashMap; mod logger; #[global_allocator] static ALLOC: tikv_jemallocator::Jemalloc = tikv_jemallocator::Jemalloc; -#[tokio::main] -async fn main() -> anyhow::Result<()> { - Cli::parse().run().await +fn main() -> anyhow::Result<()> { + Cli::parse().run() } /// Tycho network node. @@ -47,7 +45,7 @@ struct Cli { } impl Cli { - async fn run(self) -> anyhow::Result<()> { + fn run(self) -> anyhow::Result<()> { let fun = if self.flame { logger::flame } else { @@ -95,9 +93,7 @@ fn make_network(cli: Cli) -> Vec> { .map(|((_, key_pair), addr)| Arc::new(make_peer_info(key_pair, vec![addr.clone()], None))) .collect::>(); - let anchors_map = Arc::new(Mutex::new(FastHashMap::default())); - let refs_map = Arc::new(Mutex::new(FastHashMap::default())); - + let mut anchor_consumer = AnchorConsumer::default(); let mut handles = vec![]; for (((secret_key, key_pair), bind_address), peer_id) in keys .into_iter() @@ -106,8 +102,8 @@ fn make_network(cli: Cli) -> Vec> { { let all_peers = all_peers.clone(); let peer_info = peer_info.clone(); - let anchors_map = anchors_map.clone(); - let refs_map = refs_map.clone(); + let (committed_tx, committed_rx) = mpsc::unbounded_channel(); + anchor_consumer.add(peer_id, committed_rx); let handle = std::thread::Builder::new() .name(format!("engine-{peer_id:.4}")) @@ -144,16 +140,6 @@ fn make_network(cli: Cli) -> Vec> { .expect("add peer to dht client"); } } - - let (committed_tx, committed_rx) = mpsc::unbounded_channel(); - tokio::spawn(check_anchors( - committed_rx, - peer_id, - anchors_map, - refs_map, - cli.nodes, - )); - // tokio::spawn(drain_anchors(committed_rx)); let mut engine = Engine::new( key_pair, &dht_client, @@ -169,6 +155,18 @@ fn make_network(cli: Cli) -> Vec> { .unwrap(); handles.push(handle); } + handles.push( + std::thread::Builder::new() + .name("anchor-consumer".to_string()) + .spawn(move || { + tokio::runtime::Builder::new_current_thread() + .worker_threads(1) + .build() + .expect("new tokio runtime") + .block_on(anchor_consumer.check()) + }) + .unwrap(), + ); handles } diff --git a/consensus/src/dag/anchor_stage.rs b/consensus/src/dag/anchor_stage.rs index 776922c12..fab201d72 100644 --- a/consensus/src/dag/anchor_stage.rs +++ b/consensus/src/dag/anchor_stage.rs @@ -24,13 +24,11 @@ impl AnchorStage { // reproducible global coin let leader_index = rand_pcg::Pcg32::seed_from_u64(anchor_candidate_round as u64) .gen_range(0..leader_peers.len()); - let Some(leader) = leader_peers + let leader = leader_peers .iter() .nth(leader_index) .map(|(peer_id, _)| peer_id) - else { - panic!("selecting a leader from an empty validator set") - }; + .expect("selecting a leader from an empty validator set"); // the leader cannot produce three points in a row, so we have an undefined leader, // rather than an intentional leaderless support round - all represented by `None` if !current_peers.contains_key(leader) { diff --git a/consensus/src/dag/dag.rs b/consensus/src/dag/dag.rs index 1fe4dafc9..6efa165f8 100644 --- a/consensus/src/dag/dag.rs +++ b/consensus/src/dag/dag.rs @@ -1,17 +1,19 @@ use std::collections::BTreeMap; +use std::convert::identity; use std::sync::atomic::Ordering; use std::sync::Arc; use futures_util::FutureExt; use parking_lot::Mutex; use tokio::sync::mpsc::UnboundedSender; +use tycho_network::PeerId; use crate::dag::anchor_stage::AnchorStage; use crate::dag::DagRound; -use crate::effects::{CurrentRoundContext, Effects}; +use crate::effects::{AltFormat, CurrentRoundContext, Effects}; use crate::engine::MempoolConfig; use crate::intercom::PeerSchedule; -use crate::models::{LinkField, Point, Round, ValidPoint}; +use crate::models::{Digest, LinkField, Location, Point, PointId, Round, ValidPoint}; #[derive(Clone)] pub struct Dag { @@ -56,22 +58,29 @@ impl Dag { pub fn commit( self, next_dag_round: DagRound, - committed: UnboundedSender<(Arc, Vec>)>, + committed: UnboundedSender<(Point, Vec)>, effects: Effects, ) { // finding the latest trigger must not take long, better try later // than wait long for some DagPoint::NotFound, slowing down whole Engine - let _guard = effects.span().enter(); + let _parent_guard = effects.span().enter(); let Some(latest_trigger) = Self::latest_trigger(&next_dag_round) else { return; }; + let _span = tracing::error_span!( + "commit trigger", + author = display(&latest_trigger.point.body().location.author.alt()), + round = latest_trigger.point.body().location.round.0, + digest = display(&latest_trigger.point.digest().alt()), + ) + .entered(); // when we have a valid trigger, its every point of it's subdag is validated successfully let mut anchor_stack = Self::anchor_stack(&latest_trigger, next_dag_round.clone()); let mut ordered = Vec::new(); while let Some((anchor, anchor_round)) = anchor_stack.pop() { // Note every next "little anchor candidate that could" must have at least full dag depth // Note if sync is implemented as a second sub-graph - drop up to the last linked in chain - self.drop_tail(anchor.point.body.location.round); + self.drop_tail(anchor.point.body().location.round); let committed = Self::gather_uncommitted(&anchor.point, &anchor_round); ordered.push((anchor.point, committed)); } @@ -88,7 +97,7 @@ impl Dag { fn latest_trigger(next_dag_round: &DagRound) -> Option { let mut next_dag_round = next_dag_round.clone(); let mut latest_trigger = None; - while let Some(current_dag_round) = next_dag_round.prev().get() { + while let Some(current_dag_round) = next_dag_round.prev().upgrade() { if let Some(AnchorStage::Trigger { ref is_used, ref leader, @@ -104,7 +113,7 @@ impl Dag { // better try later than wait now if some point is still downloading .filter_map(|version| version.clone().now_or_never()) // take any suitable - .find_map(move |(dag_point, _)| dag_point.into_valid()) + .find_map(move |dag_point| dag_point.into_valid()) }) .flatten() { @@ -129,14 +138,20 @@ impl Dag { "invalid anchor proof link, trigger point must have been invalidated" ); let mut anchor_stack = Vec::new(); - let mut proof = future_round - .vertex_by_proof(last_trigger) - .now_or_never() - .expect("validation must be completed") - .expect("latest anchor proof must be in DAG"); let mut proof_round = future_round - .scan(proof.point.body.location.round) + .scan(last_trigger.point.body().location.round.prev()) .expect("anchor proof round not in DAG while a point from it was received"); + let mut proof = Self::ready_valid_point( + &proof_round, + &last_trigger.point.body().location.author, + &last_trigger + .point + .body() + .proof + .as_ref() + .expect("validation broken: anchor trigger with empty proof field") + .digest, + ); loop { if proof_round.round() == MempoolConfig::GENESIS_ROUND { break; @@ -149,27 +164,28 @@ impl Dag { panic!("anchor proof round is not expected, validation is broken") }; assert_eq!( - proof.point.body.location.round, + proof.point.body().location.round, proof_round.round(), "anchor proof round does not match" ); assert_eq!( - proof.point.body.location.author, leader, + proof.point.body().location.author, + leader, "anchor proof author does not match prescribed by round" ); - let Some(anchor_round) = proof_round.prev().get() else { + let Some(anchor_round) = proof_round.prev().upgrade() else { break; }; if is_used.load(Ordering::Relaxed) { break; }; - let anchor_digest = match &proof.point.body.proof { + let anchor_digest = match &proof.point.body().proof { Some(prev) => &prev.digest, None => panic!("anchor proof must prove to anchor point, validation is broken"), }; let anchor = anchor_round .view(leader, |loc| { - let (dag_point, _) = loc + let dag_point = loc .versions() .get(anchor_digest) .expect("anchor proof is not linked to anchor, validation broken") @@ -188,11 +204,11 @@ impl Dag { Some(next_proof_round) => proof_round = next_proof_round, None => break, }; - proof = proof_round - .valid_point_exact(&next_proof_id.location.author, &next_proof_id.digest) - .now_or_never() - .expect("validation must be completed") - .expect("next proof point in chain must exist in its dag round"); + proof = Self::ready_valid_point( + &proof_round, + &next_proof_id.location.author, + &next_proof_id.digest, + ); } anchor_stack } @@ -200,7 +216,7 @@ impl Dag { fn drop_tail(&self, anchor_at: Round) { if let Some(tail) = anchor_at.0.checked_sub(MempoolConfig::COMMIT_DEPTH as u32) { let mut rounds = self.rounds.lock(); - *rounds = rounds.split_off(&Round(tail)); + rounds.retain(|k, _| k.0 >= tail); }; } @@ -210,21 +226,21 @@ impl Dag { fn gather_uncommitted( anchor: &Point, // @ r+1 anchor_round: &DagRound, // r+1 - ) -> Vec> { + ) -> Vec { assert_eq!( anchor_round.round(), - anchor.body.location.round, + anchor.body().location.round, "passed anchor round does not match anchor point's round" ); let mut proof_round /* r+0 */ = anchor_round .prev() - .get() + .upgrade() .expect("previous round for anchor point round must stay in DAG"); let mut r = [ - anchor.body.includes.clone(), // points @ r+0 - anchor.body.witness.clone(), // points @ r-1 - BTreeMap::new(), // points @ r-2 - BTreeMap::new(), // points @ r-3 + anchor.body().includes.clone(), // points @ r+0 + anchor.body().witness.clone(), // points @ r-1 + BTreeMap::new(), // points @ r-2 + BTreeMap::new(), // points @ r-3 ]; _ = anchor; // anchor payload will be committed the next time @@ -232,7 +248,7 @@ impl Dag { while let Some(vertex_round /* r-1 */) = proof_round .prev() - .get() + .upgrade() .filter(|_| !r.iter().all(BTreeMap::is_empty)) { // take points @ r+0, and select their vertices @ r-1 for commit @@ -243,21 +259,15 @@ impl Dag { // but some points don't have previous one to proof as vertex. // Any valid point among equivocated will do, as they include the same vertex. let proof = // point @ r+0 - proof_round.valid_point_exact(node, digest) - .now_or_never() - .expect("validation must be completed") - .expect("point to commit not found in DAG"); - let author = &proof.point.body.location.author; - r[1].extend(proof.point.body.includes.clone()); // points @ r-1 - r[2].extend(proof.point.body.witness.clone()); // points @ r-2 - let Some(digest) = proof.point.body.proof.as_ref().map(|a| &a.digest) else { + Self::ready_valid_point(&proof_round, node, digest); + let author = &proof.point.body().location.author; + r[1].extend(proof.point.body().includes.clone()); // points @ r-1 + r[2].extend(proof.point.body().witness.clone()); // points @ r-2 + let Some(digest) = proof.point.body().proof.as_ref().map(|a| &a.digest) else { continue; }; let vertex = // point @ r-1 - vertex_round.valid_point_exact(author, digest) - .now_or_never() - .expect("validation must be completed") - .expect("point to commit not found in DAG or wrong round"); + Self::ready_valid_point(&vertex_round, author, digest); // select uncommitted ones, marking them as committed // to exclude from the next commit if vertex @@ -266,8 +276,8 @@ impl Dag { .is_ok() { // vertex will be skipped in r_1 as committed - r[2].extend(vertex.point.body.includes.clone()); // points @ r-2 - r[3].extend(vertex.point.body.witness.clone()); // points @ r-3 + r[2].extend(vertex.point.body().includes.clone()); // points @ r-2 + r[3].extend(vertex.point.body().witness.clone()); // points @ r-3 uncommitted.push(vertex.point); } } @@ -277,4 +287,29 @@ impl Dag { uncommitted.reverse(); uncommitted } + + // needed only in commit where all points are validated and stored in DAG + fn ready_valid_point(dag_round: &DagRound, author: &PeerId, digest: &Digest) -> ValidPoint { + dag_round + .view(author, |loc| { + loc.versions() + .get(digest) + .cloned() + .ok_or("point digest not found in location's versions") + }) + .ok_or("point author not found among dag round's locations") + .and_then(identity) // flatten result + .and_then(|fut| fut.now_or_never().ok_or("validation must be completed")) + .and_then(|dag_point| dag_point.into_valid().ok_or("point is not valid")) + .unwrap_or_else(|msg| { + let point_id = PointId { + location: Location { + round: dag_round.round(), + author: *author, + }, + digest: digest.clone(), + }; + panic!("{msg}: {:?}", point_id.alt()) + }) + } } diff --git a/consensus/src/dag/dag_location.rs b/consensus/src/dag/dag_location.rs index d86d18c99..f3bfece05 100644 --- a/consensus/src/dag/dag_location.rs +++ b/consensus/src/dag/dag_location.rs @@ -1,10 +1,14 @@ use std::collections::{btree_map, BTreeMap}; use std::future::Future; use std::ops::RangeInclusive; +use std::pin::Pin; use std::sync::{Arc, OnceLock}; +use std::task::{Context, Poll}; use everscale_crypto::ed25519::KeyPair; use futures_util::FutureExt; +use tokio::sync::mpsc; +use tycho_network::PeerId; use tycho_util::futures::{JoinTask, Shared}; use crate::models::{DagPoint, Digest, Round, Signature, UnixTime, ValidPoint}; @@ -30,54 +34,66 @@ pub struct DagLocation { /// only one of the point versions at current location /// may become proven by the next round point(s) of a node; /// even if we marked a proven point as invalid, consensus may ignore our decision - versions: BTreeMap>>, + versions: BTreeMap, +} + +#[derive(Clone)] +pub enum DagPointFuture { + Broadcast(Shared>), + Download { + task: Shared>, + dependents: mpsc::UnboundedSender, + }, + Local(futures_util::future::Ready), +} + +impl DagPointFuture { + pub fn add_depender(&self, dependent: &PeerId) { + if let Self::Download { dependents, .. } = self { + // receiver is dropped upon completion + _ = dependents.send(*dependent); + } + } +} + +impl Future for DagPointFuture { + type Output = DagPoint; + + #[inline] + fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { + match &mut *self { + Self::Broadcast(task) | Self::Download { task, .. } => match task.poll_unpin(cx) { + Poll::Ready((dag_point, _)) => Poll::Ready(dag_point), + Poll::Pending => Poll::Pending, + }, + Self::Local(ready) => ready.poll_unpin(cx), + } + } } impl DagLocation { - pub fn get_or_init(&mut self, digest: &Digest, init: I) -> Shared> + // point that is validated depends on other equivocated points futures (if any) + // in the same location, so need to keep order of futures' completion; + // to make signature we are interested in the single validated point only + // (others are at least suspicious and cannot be signed) + pub fn get_or_init(&mut self, digest: &Digest, init: F) -> &DagPointFuture where - I: FnOnce() -> F, - F: Future + Send + 'static, + F: FnOnce(&InclusionState) -> DagPointFuture, { - match self.versions.entry(digest.clone()) { - btree_map::Entry::Occupied(entry) => entry.get().clone(), - btree_map::Entry::Vacant(entry) => { - let state = self.state.clone(); - entry - .insert(Shared::new(JoinTask::new( - init().inspect(move |dag_point| state.init(dag_point)), - ))) - .clone() - } - } + self.versions + .entry(digest.clone()) + .or_insert_with(|| init(&self.state)) } - pub fn init(&mut self, digest: &Digest, init: I) -> Option<&'_ Shared>> + pub fn init(&mut self, digest: &Digest, init: F) -> Option<&DagPointFuture> where - I: FnOnce() -> F, - F: Future + Send + 'static, + F: FnOnce(&InclusionState) -> DagPointFuture, { - // point that is validated depends on other equivocated points futures (if any) - // in the same location, so order of insertion matches order of futures' completion; - // to make signature we are interested in the first validated point only - // (others are at least suspicious and cannot be signed) match self.versions.entry(digest.clone()) { btree_map::Entry::Occupied(_) => None, - btree_map::Entry::Vacant(entry) => { - let state = self.state.clone(); - let shared = entry.insert(Shared::new(JoinTask::new({ - // Note: cannot sign during validation, - // because current DAG round may advance concurrently - // TODO either leave output as is and reduce locking in 'inclusion state' - // (as single thread consumes them and makes signature), - // or better add global Watch CurrentDagRound (unify with broadcast filter!) - // and sign inside this future (remove futures unordered in collector) - init().inspect(move |dag_point| state.init(dag_point)) - }))); - Some(shared) - } + btree_map::Entry::Vacant(entry) => Some(entry.insert(init(&self.state))), } } - pub fn versions(&self) -> &'_ BTreeMap>> { + pub fn versions(&self) -> &'_ BTreeMap { &self.versions } pub fn state(&self) -> &'_ InclusionState { @@ -109,8 +125,8 @@ impl InclusionState { None => panic!("Coding error: own point is not trusted"), Some(valid) => { _ = signed.set(Ok(Signed { - at: valid.point.body.location.round, - with: valid.point.signature.clone(), + at: valid.point.body().location.round, + with: valid.point.signature().clone(), })); } }; @@ -174,15 +190,15 @@ impl Signable { ) -> bool { let mut this_call_signed = false; if let Some((valid, key_pair)) = self.first_completed.trusted().zip(key_pair) { - if time_range.contains(&valid.point.body.time) { + if time_range.contains(&valid.point.body().time) { _ = self.signed.get_or_init(|| { this_call_signed = true; Ok(Signed { at, - with: Signature::new(key_pair, &valid.point.digest), + with: Signature::new(key_pair, valid.point.digest()), }) }); - } else if &valid.point.body.time < time_range.start() { + } else if &valid.point.body().time < time_range.start() { self.reject(); } // else decide later } else { diff --git a/consensus/src/dag/dag_round.rs b/consensus/src/dag/dag_round.rs index a80cbbb34..896f1071e 100644 --- a/consensus/src/dag/dag_round.rs +++ b/consensus/src/dag/dag_round.rs @@ -3,15 +3,18 @@ use std::sync::{Arc, Weak}; use everscale_crypto::ed25519::KeyPair; use futures_util::future::BoxFuture; use futures_util::FutureExt; +use tokio::sync::mpsc; use tracing::Span; use tycho_network::PeerId; +use tycho_util::futures::{JoinTask, Shared}; use tycho_util::FastDashMap; use crate::dag::anchor_stage::AnchorStage; -use crate::dag::{DagLocation, InclusionState, Verifier}; +use crate::dag::{DagLocation, DagPointFuture, InclusionState, Verifier}; +use crate::effects::{CurrentRoundContext, Effects, ValidateContext}; use crate::engine::MempoolConfig; use crate::intercom::{Downloader, PeerSchedule}; -use crate::models::{DagPoint, Digest, NodeCount, Point, PointId, Round, ValidPoint}; +use crate::models::{DagPoint, Digest, Location, NodeCount, Point, PointId, Round, ValidPoint}; #[derive(Clone)] /// Allows memory allocated by DAG to be freed @@ -37,7 +40,7 @@ struct DagRoundInner { impl WeakDagRound { const BOTTOM: Self = WeakDagRound(Weak::new()); - pub fn get(&self) -> Option { + pub fn upgrade(&self) -> Option { self.0.upgrade().map(DagRound) } } @@ -61,7 +64,7 @@ impl DagRound { Self(Arc::new(DagRoundInner { round, node_count: NodeCount::try_from(peers.len()) - .unwrap_or_else(|_| panic!("no peers scheduled for {round:?}")), + .unwrap_or_else(|e| panic!("{e} for {round:?}")), key_pair: peer_schedule.local_keys(round), anchor_stage: AnchorStage::of(round, peer_schedule), locations, @@ -76,17 +79,17 @@ impl DagRound { Self(Arc::new(DagRoundInner { round: next_round, node_count: NodeCount::try_from(peers.len()) - .unwrap_or_else(|_| panic!("no peers scheduled for {next_round:?}")), + .unwrap_or_else(|e| panic!("{e} for {next_round:?}")), key_pair: peer_schedule.local_keys(next_round), anchor_stage: AnchorStage::of(next_round, peer_schedule), locations, - prev: self.to_weak(), + prev: self.downgrade(), })) } - pub fn genesis(genesis: &Arc, peer_schedule: &PeerSchedule) -> Self { + pub fn genesis(genesis: &Point, peer_schedule: &PeerSchedule) -> Self { let locations = FastDashMap::with_capacity_and_hasher(1, Default::default()); - let round = genesis.body.location.round; + let round = genesis.body().location.round; Self(Arc::new(DagRoundInner { round, node_count: NodeCount::GENESIS, @@ -113,7 +116,7 @@ impl DagRound { self.0.anchor_stage.as_ref() } - pub fn edit(&self, author: &PeerId, edit: F) -> R + fn edit(&self, author: &PeerId, edit: F) -> R where F: FnOnce(&mut DagLocation) -> R, { @@ -142,82 +145,94 @@ impl DagRound { &self.0.prev } - pub fn to_weak(&self) -> WeakDagRound { + pub fn downgrade(&self) -> WeakDagRound { WeakDagRound(Arc::downgrade(&self.0)) } - pub async fn vertex_by_proof(&self, proof: &ValidPoint) -> Option { - match proof.point.body.proof { - Some(ref proven) => { - let dag_round = self.scan(proof.point.body.location.round.prev())?; - dag_round - .valid_point_exact(&proof.point.body.location.author, &proven.digest) - .await - } - None => None, - } - } - - pub async fn valid_point(&self, point_id: &PointId) -> Option { - match self.scan(point_id.location.round) { - Some(linked) => { - linked - .valid_point_exact(&point_id.location.author, &point_id.digest) - .await - } - None => None, - } - } - - pub async fn valid_point_exact(&self, node: &PeerId, digest: &Digest) -> Option { - let point_fut = self.view(node, |loc| loc.versions().get(digest).cloned())??; - point_fut.await.0.into_valid() - } - - pub fn add( + pub fn add_broadcast_exact( &self, - point: &Arc, + point: &Point, downloader: &Downloader, - span: &Span, + effects: &Effects, ) -> Option> { - let dag_round = span.in_scope(|| self.scan(point.body.location.round))?; - dag_round.add_exact(point, downloader, span) - } - - fn add_exact( - &self, - point: &Arc, - downloader: &Downloader, - span: &Span, - ) -> Option> { - if point.body.location.round != self.round() { - let _span = span.enter(); - panic!( - "Coding error: dag round mismatches point round on add {:?}", - point.id() - ) - } - let dag_round = self.to_weak(); - let digest = &point.digest; - self.edit(&point.body.location.author, |loc| { + let _guard = effects.span().enter(); + assert_eq!( + point.body().location.round, + self.round(), + "Coding error: point round does not match dag round" + ); + let digest = point.digest(); + self.edit(&point.body().location.author, |loc| { + let downloader = downloader.clone(); + let span = effects.span().clone(); let state = loc.state().clone(); + let point_dag_round = self.downgrade(); let point = point.clone(); - let downloader = downloader.clone(); - loc.init(digest, || { - Verifier::validate(point, dag_round, downloader, span.clone()) + loc.init(digest, |state| { + // FIXME: prior Responder refactor: could not sign during validation, + // because current DAG round could advance concurrently; + // now current dag round changes consistently, + // maybe its possible to reduce locking in 'inclusion state' + let state = state.clone(); + DagPointFuture::Broadcast(Shared::new(JoinTask::new( + Verifier::validate(point, point_dag_round, downloader, span) + .inspect(move |dag_point| state.init(dag_point)), + ))) }) .map(|first| first.clone().map(|_| state).boxed()) }) } + /// notice: `round` must exactly match point's round, + /// otherwise dependency will resolve to [`DagPoint::NotExists`] + pub fn add_dependency_exact( + &self, + author: &PeerId, + digest: &Digest, + depender: &PeerId, + downloader: &Downloader, + effects: &Effects, + ) -> DagPointFuture { + let future = self.edit(author, |loc| { + loc.get_or_init(digest, |state| { + let downloader = downloader.clone(); + let effects = effects.clone(); + let state = state.clone(); + let point_dag_round = self.clone(); + let (tx, rx) = mpsc::unbounded_channel(); + let point_id = PointId { + location: Location { + author: *author, + round: self.round(), + }, + digest: digest.clone(), + }; + DagPointFuture::Download { + task: Shared::new(JoinTask::new( + downloader + .run(point_id, point_dag_round, rx, effects) + .inspect(move |dag_point| state.init(dag_point)), + )), + dependents: tx, + } + }) + .clone() + }); + future.add_depender(depender); + future + } + /// for genesis and own points pub fn insert_exact_sign( &self, - point: &Arc, + point: &Point, peer_schedule: &PeerSchedule, span: &Span, ) -> InclusionState { - let state = self.insert_exact(&DagPoint::Trusted(ValidPoint::new(point.clone()))); + let state = self.insert_exact( + &point.body().location.author, + &DagPoint::Trusted(ValidPoint::new(point.clone())), + ); if let Some(signable) = state.signable() { signable.sign( self.round(), @@ -225,29 +240,32 @@ impl DagRound { MempoolConfig::sign_time_range(), ); } - if state.signed_point(self.round()).is_none() { - let _guard = span.enter(); - panic!("Coding or configuration error: valid point cannot be signed; time issue?") - } + let _guard = span.enter(); + assert!( + state.signed_point(self.round()).is_some(), + "Coding or configuration error: valid point cannot be signed; time issue?" + ); state } - pub fn insert_invalid(&self, dag_point: &DagPoint) -> Option { - if dag_point.valid().is_some() { - panic!("Coding error: failed to insert valid point as invalid") - } - let round = self.scan(dag_point.location().round)?; - Some(round.insert_exact(dag_point)) + pub fn insert_invalid_exact(&self, sender: &PeerId, dag_point: &DagPoint) { + assert!( + dag_point.valid().is_none(), + "Coding error: failed to insert valid point as invalid" + ); + self.insert_exact(sender, dag_point); } - fn insert_exact(&self, dag_point: &DagPoint) -> InclusionState { - if dag_point.location().round != self.round() { - panic!("Coding error: dag round mismatches point round on insert") - } - self.edit(&dag_point.location().author, |loc| { - loc.state().init(dag_point); - let _existing = loc.init(dag_point.digest(), || { - futures_util::future::ready(dag_point.clone()) + fn insert_exact(&self, sender: &PeerId, dag_point: &DagPoint) -> InclusionState { + assert_eq!( + dag_point.location().round, + self.round(), + "Coding error: dag round mismatches point round on insert" + ); + self.edit(sender, |loc| { + let _existing = loc.init(dag_point.digest(), |state| { + state.init(dag_point); + DagPointFuture::Local(futures_util::future::ready(dag_point.clone())) }); loc.state().clone() }) @@ -259,12 +277,19 @@ impl DagRound { "Coding error: cannot scan DAG rounds chain for a future round" ); let mut visited = self.clone(); - if round == self.round() { + if visited.round() == round { return Some(visited); } - while let Some(dag_round) = visited.prev().get() { + while let Some(dag_round) = visited.prev().upgrade() { match dag_round.round().cmp(&round) { - core::cmp::Ordering::Less => return None, + core::cmp::Ordering::Less => panic!( + "Coding error: linked list of dag rounds cannot contain gaps, \ + found {} to be prev for {}, scanned for {} from {}", + dag_round.round().0, + visited.round().0, + round.0, + self.round().0 + ), core::cmp::Ordering::Equal => return Some(dag_round), core::cmp::Ordering::Greater => visited = dag_round, } diff --git a/consensus/src/dag/producer.rs b/consensus/src/dag/producer.rs index 9e7182b67..c9ea9a524 100644 --- a/consensus/src/dag/producer.rs +++ b/consensus/src/dag/producer.rs @@ -1,5 +1,4 @@ use std::collections::BTreeMap; -use std::sync::Arc; use tycho_network::PeerId; @@ -17,8 +16,8 @@ impl Producer { current_round: &DagRound, prev_point: Option<&PrevPoint>, input_buffer: &InputBuffer, - ) -> Option> { - let finished_round = current_round.prev().get()?; + ) -> Option { + let finished_round = current_round.prev().upgrade()?; let key_pair = current_round.key_pair()?; let payload = input_buffer.fetch(prev_point.is_some()); let local_id = PeerId::from(key_pair.public_key); @@ -54,7 +53,7 @@ impl Producer { let includes = includes .into_iter() - .map(|point| (point.body.location.author, point.digest.clone())) + .map(|point| (point.body().location.author, point.digest().clone())) .collect::>(); assert_eq!( @@ -70,7 +69,7 @@ impl Producer { let witness = witness .into_iter() - .map(|point| (point.body.location.author, point.digest.clone())) + .map(|point| (point.body().location.author, point.digest().clone())) .collect::>(); Some(Point::new(key_pair, PointBody { @@ -89,7 +88,7 @@ impl Producer { })) } - fn includes(finished_round: &DagRound) -> Vec> { + fn includes(finished_round: &DagRound) -> Vec { let includes = finished_round .select(|(_, loc)| { loc.state() @@ -109,8 +108,8 @@ impl Producer { includes } - fn witness(finished_round: &DagRound) -> Vec> { - match finished_round.prev().get() { + fn witness(finished_round: &DagRound) -> Vec { + match finished_round.prev().upgrade() { Some(witness_round) => witness_round .select(|(_, loc)| { loc.state() @@ -125,7 +124,7 @@ impl Producer { fn link_from_includes( local_id: &PeerId, current_round: &DagRound, - includes: &[Arc], + includes: &[Point], link_field: LinkField, ) -> Link { use AnchorStage::{Proof, Trigger}; @@ -145,14 +144,14 @@ impl Producer { .max_by_key(|point| point.anchor_round(link_field)) .expect("non-empty list of includes for own point"); - if point.body.location.round == current_round.round().prev() + if point.body().location.round == current_round.round().prev() && point.anchor_link(link_field) == &Link::ToSelf { - Link::Direct(Through::Includes(point.body.location.author)) + Link::Direct(Through::Includes(point.body().location.author)) } else { Link::Indirect { to: point.anchor_id(link_field), - path: Through::Includes(point.body.location.author), + path: Through::Includes(point.body().location.author), } } } @@ -160,7 +159,7 @@ impl Producer { fn update_link_from_witness( link: &mut Link, current_round: Round, - witness: &[Arc], + witness: &[Point], link_field: LinkField, ) { let link_round = match link { @@ -176,14 +175,14 @@ impl Producer { return; }; - if point.body.location.round == current_round.prev().prev() + if point.body().location.round == current_round.prev().prev() && point.anchor_link(link_field) == &Link::ToSelf { - *link = Link::Direct(Through::Witness(point.body.location.author)); + *link = Link::Direct(Through::Witness(point.body().location.author)); } else { *link = Link::Indirect { to: point.anchor_id(link_field), - path: Through::Witness(point.body.location.author), + path: Through::Witness(point.body().location.author), }; } } @@ -191,17 +190,17 @@ impl Producer { fn get_time( anchor_proof: &Link, prev_point: Option<&PrevPoint>, - includes: &[Arc], - witness: &[Arc], + includes: &[Point], + witness: &[Point], ) -> (UnixTime, UnixTime) { let mut time = UnixTime::now(); let prev_point = prev_point.and_then(|prev| { includes .iter() - .find(|point| point.digest == prev.digest) + .find(|point| point.digest() == &prev.digest) .map(|point| { - time = point.body.time.max(time); + time = point.body().time.max(time); point }) }); @@ -210,7 +209,7 @@ impl Producer { Link::ToSelf => { let point = prev_point.expect("anchor candidate should exist"); - let anchor_time = point.body.time; + let anchor_time = point.body().time; (time.max(anchor_time), anchor_time) } @@ -222,10 +221,10 @@ impl Producer { let point = through .iter() - .find(|point| point.body.location.author == peer_id) + .find(|point| point.body().location.author == peer_id) .expect("path to anchor proof should exist in new point dependencies"); - let anchor_time = point.body.anchor_time; + let anchor_time = point.body().anchor_time; (time.max(anchor_time), anchor_time) } diff --git a/consensus/src/dag/verifier.rs b/consensus/src/dag/verifier.rs index 04b50d212..856ab8744 100644 --- a/consensus/src/dag/verifier.rs +++ b/consensus/src/dag/verifier.rs @@ -1,48 +1,41 @@ use std::sync::Arc; use futures_util::stream::FuturesUnordered; -use futures_util::{Future, StreamExt}; +use futures_util::StreamExt; use tracing::{Instrument, Span}; -use tycho_network::PeerId; -use tycho_util::futures::{JoinTask, Shared}; use tycho_util::sync::rayon_run; use crate::dag::anchor_stage::AnchorStage; -use crate::dag::{DagRound, WeakDagRound}; +use crate::dag::{DagPointFuture, DagRound, WeakDagRound}; use crate::effects::{AltFormat, Effects, EffectsContext, ValidateContext}; use crate::engine::MempoolConfig; use crate::intercom::{Downloader, PeerSchedule}; -use crate::models::{ - DagPoint, Digest, Link, LinkField, Location, NodeCount, Point, PointId, ValidPoint, -}; +use crate::models::{DagPoint, Link, LinkField, Location, NodeCount, Point, ValidPoint}; // Note on equivocation. // Detected point equivocation does not invalidate the point, it just -// prevents us (as a fair actor) from returning our signature to the author. +// prevents us (as a reliable peer) from returning our signature to the author. // Such a point may be included in our next "includes" or "witnesses", // but neither its inclusion nor omitting is required: as we don't // return our signature, our dependencies cannot be validated against it. // Equally, we immediately stop communicating with the equivocating node, // without invalidating any of its points (no matter historical or future). -// We will not sign the proof for equivocated point -// as we've banned the author on network layer. +// We will not sign the proof for equivocated point as we ban the author on network layer. // Anyway, no more than one of equivocated points may become a vertex. pub struct Verifier; -impl Verifier { - // FIXME outside, for points to sign only: check time bounds before validation, sign only Trusted - // FIXME shallow verification during sync to close a gap, trusting first vertex contents: - // take any vertex and its proof point, check signatures for the vertex, - // and use all vertex dependencies recursively as Trusted without any checks - // with 3-rounds-wide sliding window that moves backwards +// If any round exceeds dag depth, the arg point @ r+0 is considered valid by itself. +// Any point @ r+0 will be committed, only if it has valid proof @ r+1 +// included into valid anchor chain, i.e. validated by consensus. +impl Verifier { /// the first and mandatory check of any Point received no matter where from - pub fn verify(point: &Arc, peer_schedule: &PeerSchedule) -> Result<(), DagPoint> { + pub fn verify(point: &Point, peer_schedule: &PeerSchedule) -> Result<(), DagPoint> { if !point.is_integrity_ok() { Err(DagPoint::NotExists(Arc::new(point.id()))) // cannot use point body } else if !(point.is_well_formed() && Self::is_list_of_signers_ok(point, peer_schedule)) { - // the last task spawns if ok - in order not to walk through every dag round twice + // point links, etc. will not be used Err(DagPoint::Invalid(point.clone())) } else { Ok(()) @@ -51,38 +44,30 @@ impl Verifier { /// must be called iff [`Self::verify`] succeeded pub async fn validate( - point: Arc, // @ r+0 + point: Point, // @ r+0 r_0: WeakDagRound, // r+0 downloader: Downloader, parent_span: Span, ) -> DagPoint { let effects = Effects::::new(&parent_span, &point); let span_guard = effects.span().enter(); - let Some(r_0) = r_0.get() else { - tracing::warn!( - author = display(point.body.location.author.alt()), - round = point.body.location.round.0, - digest = display(point.digest.alt()), - "cannot validate point, local DAG moved far forward", - ); - return DagPoint::NotExists(Arc::new(point.id())); - }; - // TODO upgrade Weak whenever used to let Dag Round drop if some future hangs up for long - if point.body.location.round != r_0.round() { - panic!("Coding error: dag round mismatches point round") - } - let signatures_fut = rayon_run({ - let proof = point.body.proof.clone(); - let span = effects.span().clone(); - move || match proof { - None => true, - Some(proof) => { - let _guard = span.enter(); - proof.signatures_match() - } - } - }); + // for genesis point it's sufficient to be well-formed and pass integrity check, + // it cannot be validated against AnchorStage (as it knows nothing about genesis) + // and cannot contain dependencies + assert!( + point.body().location.round > MempoolConfig::GENESIS_ROUND, + "Coding error: can only validate points older than genesis" + ); + let Some(r_0) = r_0.upgrade() else { + tracing::info!("cannot (in)validate point, no round in local DAG"); + return DagPoint::Suspicious(ValidPoint::new(point.clone())); + }; + assert_eq!( + point.body().location.round, + r_0.round(), + "Coding error: dag round mismatches point round" + ); let dependencies = FuturesUnordered::new(); @@ -107,11 +92,9 @@ impl Verifier { return DagPoint::Invalid(point.clone()); } - let Some(r_1) = r_0.prev().get() else { - // If r-1 exceeds dag depth, the arg point @ r+0 is considered valid by itself. - // Any point @ r+0 will be committed, only if it has valid proof @ r+1 - // included into valid anchor chain, i.e. validated by consensus. - return DagPoint::Trusted(ValidPoint::new(point.clone())); + let Some(r_1) = r_0.prev().upgrade() else { + tracing::info!("cannot (in)validate point's 'includes', no round in local DAG"); + return DagPoint::Suspicious(ValidPoint::new(point.clone())); }; Self::gather_deps(&point, &r_1, &downloader, &effects, &dependencies); @@ -119,9 +102,44 @@ impl Verifier { drop(r_0); drop(r_1); drop(span_guard); - Self::check_deps(&point, dependencies, signatures_fut) - .instrument(effects.span().clone()) - .await + + let mut signatures_fut = std::pin::pin!(match point.body().proof.as_ref() { + None => futures_util::future::Either::Left(futures_util::future::ready(true)), + Some(proof) => futures_util::future::Either::Right( + rayon_run({ + let proof = proof.clone(); + move || proof.signatures_match() + }) + .instrument(effects.span().clone()), + ), + }); + let mut check_deps_fut = std::pin::pin!( + Self::check_deps(&point, dependencies).instrument(effects.span().clone()) + ); + + let mut sig_checked = false; + let mut deps_checked = None; + loop { + tokio::select! { + is_sig_ok = &mut signatures_fut, if !sig_checked => if is_sig_ok { + match deps_checked { + None => sig_checked = true, + Some(result) => break result + } + } else { + break DagPoint::Invalid(point.clone()) + }, + dag_point = &mut check_deps_fut, if deps_checked.is_none() => { + if sig_checked || dag_point.valid().is_none() { + // either invalid or signature check passed + // this cancels `rayon_run` task as receiver is dropped + break dag_point; + } else { + deps_checked = Some(dag_point); + } + } + } + } } fn is_self_links_ok( @@ -132,49 +150,48 @@ impl Verifier { (match &dag_round.anchor_stage() { // no one may link to self None => { - point.body.anchor_proof != Link::ToSelf && point.body.anchor_trigger != Link::ToSelf + point.body().anchor_proof != Link::ToSelf + && point.body().anchor_trigger != Link::ToSelf } // leader must link to own point while others must not Some(AnchorStage::Proof { leader, .. }) => { - (leader == point.body.location.author) == (point.body.anchor_proof == Link::ToSelf) + (leader == point.body().location.author) + == (point.body().anchor_proof == Link::ToSelf) } Some(AnchorStage::Trigger { leader, .. }) => { - (leader == point.body.location.author) - == (point.body.anchor_trigger == Link::ToSelf) + (leader == point.body().location.author) + == (point.body().anchor_trigger == Link::ToSelf) } - }) || point.body.location.round == MempoolConfig::GENESIS_ROUND + }) || point.body().location.round == MempoolConfig::GENESIS_ROUND } /// the only method that scans the DAG deeper than 2 rounds fn add_anchor_link_if_ok( - linked_field: LinkField, + link_field: LinkField, point: &Point, // @ r+0 dag_round: &DagRound, // start with r+0 downloader: &Downloader, effects: &Effects, - dependencies: &FuturesUnordered>>, + dependencies: &FuturesUnordered, ) -> bool { - let point_id = point.anchor_id(linked_field); + let linked_id = point.anchor_id(link_field); - let Some(round) = dag_round.scan(point_id.location.round) else { + let Some(round) = dag_round.scan(linked_id.location.round) else { // too old indirect reference does not invalidate the point, // because its direct dependencies ('link through') will be validated anyway return true; }; if round.round() == MempoolConfig::GENESIS_ROUND { - // for genesis point it's sufficient to be well-formed and pass integrity check, - // it cannot be validated against AnchorStage (as it knows nothing about genesis); - // notice that point is required to link to the freshest leader point // among all its (in)direct dependencies, which is checked later - return true; + return linked_id == crate::test_utils::genesis_point_id(); } - match (round.anchor_stage(), linked_field) { + match (round.anchor_stage(), link_field) { (Some(AnchorStage::Proof { leader, .. }), LinkField::Proof) | (Some(AnchorStage::Trigger { leader, .. }), LinkField::Trigger) - if leader == point_id.location.author => {} + if leader == linked_id.location.author => {} _ => { // link does not match round's leader, prescribed by AnchorStage return false; @@ -182,7 +199,7 @@ impl Verifier { }; #[allow(clippy::match_same_arms)] - match point.anchor_link(linked_field) { + match point.anchor_link(link_field) { Link::ToSelf => { // do not search in DAG the point that is currently under validation; // link's destination is already checked by AnchorStage above, cannot be reordered @@ -192,11 +209,10 @@ impl Verifier { } Link::Indirect { .. } => { // actually no need to check indirect dependencies, but let's reinsure while we can - dependencies.push(Self::dependency( - &point_id.location.author, - &point_id.digest, - &round, - &point.body.location.author, + dependencies.push(round.add_dependency_exact( + &linked_id.location.author, + &linked_id.digest, + &point.body().location.author, downloader, effects, )); @@ -206,71 +222,57 @@ impl Verifier { true } - // notice: `round` exactly matches point's round, - // otherwise dependency will resolve to NotFound and invalidate the point - fn dependency( - author: &PeerId, - digest: &Digest, - dag_round: &DagRound, - dependant: &PeerId, - downloader: &Downloader, - effects: &Effects, - ) -> Shared> { - dag_round.edit(author, |loc| { - loc.get_or_init(digest, || { - let downloader = downloader.clone(); - let effects = effects.clone(); - let point_id = PointId { - location: Location { - author: *author, - round: dag_round.round(), - }, - digest: digest.clone(), - }; - downloader.run(point_id, dag_round.to_weak(), *dependant, effects) - }) - }) - } - fn gather_deps( point: &Point, // @ r+0 r_1: &DagRound, // r-1 downloader: &Downloader, effects: &Effects, - dependencies: &FuturesUnordered>>, + dependencies: &FuturesUnordered, ) { - let author = &point.body.location.author; - r_1.view(author, |loc| { - for shared in loc.versions().values() { - dependencies.push(shared.clone()); + r_1.view(&point.body().location.author, |loc| { + // to check for equivocation + let prev_digest = point.body().proof.as_ref().map(|p| &p.digest); + for (digest, shared) in loc.versions() { + if prev_digest.as_ref().map_or(true, |prev| *prev != digest) { + dependencies.push(shared.clone()); + } } }); - for (node, digest) in &point.body.includes { + + for (author, digest) in &point.body().includes { // integrity check passed, so includes contain author's prev point proof - dependencies.push(Self::dependency( - node, digest, r_1, author, downloader, effects, + dependencies.push(r_1.add_dependency_exact( + author, + digest, + &point.body().location.author, + downloader, + effects, )); } - let Some(r_2) = r_1.prev().get() else { + let Some(r_2) = r_1.prev().upgrade() else { + tracing::info!("cannot (in)validate point's 'witness', no round in local DAG"); return; }; - for (node, digest) in &point.body.witness { - dependencies.push(Self::dependency( - node, digest, &r_2, author, downloader, effects, + for (author, digest) in &point.body().witness { + dependencies.push(r_2.add_dependency_exact( + author, + digest, + &point.body().location.author, + downloader, + effects, )); } } async fn check_deps( - point: &Arc, - mut dependencies: FuturesUnordered>>, - is_sig_ok: impl Future + Send, + point: &Point, + mut dependencies: FuturesUnordered, ) -> DagPoint { // point is well-formed if we got here, so point.proof matches point.includes - let proven_vertex = point.body.proof.as_ref().map(|p| &p.digest); + let proven_vertex = point.body().proof.as_ref().map(|p| &p.digest); let prev_loc = Location { - round: point.body.location.round.prev(), - author: point.body.location.author, + round: point.body().location.round.prev(), + author: point.body().location.author, }; // The node must have no points in previous location @@ -289,12 +291,12 @@ impl Verifier { let anchor_trigger_link_id = point.anchor_link_id(LinkField::Trigger); let anchor_proof_link_id = point.anchor_link_id(LinkField::Proof); - while let Some((dag_point, _)) = dependencies.next().await { + while let Some(dag_point) = dependencies.next().await { match dag_point { DagPoint::Trusted(valid) | DagPoint::Suspicious(valid) => { - if prev_loc == valid.point.body.location { + if prev_loc == valid.point.body().location { match proven_vertex { - Some(vertex_digest) if &valid.point.digest == vertex_digest => { + Some(vertex_digest) if valid.point.digest() == vertex_digest => { if !Self::is_proof_ok(point, &valid.point) { return DagPoint::Invalid(point.clone()); } // else: ok proof @@ -324,16 +326,16 @@ impl Verifier { return DagPoint::Invalid(point.clone()); } if valid_point_id == anchor_proof_link_id - && valid.point.body.anchor_time != point.body.anchor_time + && valid.point.body().anchor_time != point.body().anchor_time { // anchor candidate's time is not inherited from its proof return DagPoint::Invalid(point.clone()); } } DagPoint::Invalid(invalid) => { - if prev_loc == invalid.body.location { + if prev_loc == invalid.body().location { match proven_vertex { - Some(vertex_digest) if &invalid.digest == vertex_digest => { + Some(vertex_digest) if invalid.digest() == vertex_digest => { return DagPoint::Invalid(point.clone()) } Some(_) => is_suspicious = true, // equivocation @@ -359,9 +361,7 @@ impl Verifier { } } - if !is_sig_ok.await { - DagPoint::Invalid(point.clone()) - } else if is_suspicious { + if is_suspicious { DagPoint::Suspicious(ValidPoint::new(point.clone())) } else { DagPoint::Trusted(ValidPoint::new(point.clone())) @@ -373,33 +373,30 @@ impl Verifier { point: &Point, // @ r+0 peer_schedule: &PeerSchedule, ) -> bool { - if point.body.location.round == MempoolConfig::GENESIS_ROUND { - return true; // all maps are empty for a well-formed genesis - } let [ witness_peers/* @ r-2 */ , includes_peers /* @ r-1 */ , proof_peers /* @ r+0 */ ] = peer_schedule.peers_for_array([ - point.body.location.round.prev().prev(), - point.body.location.round.prev(), - point.body.location.round, + point.body().location.round.prev().prev(), + point.body().location.round.prev(), + point.body().location.round, ]); - for (peer_id, _) in point.body.witness.iter() { + for (peer_id, _) in point.body().witness.iter() { if !witness_peers.contains_key(peer_id) { return false; } } let node_count = NodeCount::new(includes_peers.len()); - if point.body.includes.len() < node_count.majority() { + if point.body().includes.len() < node_count.majority() { return false; }; - for (peer_id, _) in point.body.includes.iter() { + for (peer_id, _) in point.body().includes.iter() { if !includes_peers.contains_key(peer_id) { return false; } } - let Some(proven /* @ r-1 */) = &point.body.proof else { + let Some(proven /* @ r-1 */) = &point.body().proof else { return true; }; // Every point producer @ r-1 must prove its delivery to 2/3+1 producers @ r+0 @@ -429,23 +426,32 @@ impl Verifier { point: &Point, // @ r+0 proven: &Point, // @ r-1 ) -> bool { - if point.body.location.author != proven.body.location.author { - panic!("Coding error: mismatched authors of proof and its vertex") - } - if point.body.location.round.prev() != proven.body.location.round { - panic!("Coding error: mismatched rounds of proof and its vertex") - } - let Some(proof) = &point.body.proof else { - panic!("Coding error: passed point doesn't contain proof for a given vertex") - }; - if proof.digest != proven.digest { - panic!("Coding error: mismatched previous point of the same author") - } - if point.body.time < proven.body.time { + assert_eq!( + point.body().location.author, + proven.body().location.author, + "Coding error: mismatched authors of proof and its vertex" + ); + assert_eq!( + point.body().location.round.prev(), + proven.body().location.round, + "Coding error: mismatched rounds of proof and its vertex" + ); + let proof = point + .body() + .proof + .as_ref() + .expect("Coding error: passed point doesn't contain proof for a given vertex"); + assert_eq!( + &proof.digest, proven.digest(), + "Coding error: mismatched previous point of the same author, must have been checked before" + ); + if point.body().time < proven.body().time { // time must be non-decreasing by the same author return false; } - if point.body.anchor_proof == Link::ToSelf && point.body.anchor_time != proven.body.time { + if point.body().anchor_proof == Link::ToSelf + && point.body().anchor_time != proven.body().time + { // anchor proof must inherit its candidate's time return false; } @@ -459,9 +465,9 @@ impl Effects { Self::new_child(parent_span, || { tracing::error_span!( "validate", - author = display(point.body.location.author.alt()), - round = point.body.location.round.0, - digest = display(point.digest.alt()), + author = display(point.body().location.author.alt()), + round = point.body().location.round.0, + digest = display(point.digest().alt()), ) }) } diff --git a/consensus/src/effects/alt_format.rs b/consensus/src/effects/alt_format.rs index 038a0d81f..392ef1095 100644 --- a/consensus/src/effects/alt_format.rs +++ b/consensus/src/effects/alt_format.rs @@ -65,7 +65,7 @@ impl Debug for AltFmt<'_, PointId> { impl AltFormat for DagPoint {} impl Display for AltFmt<'_, DagPoint> { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + fn fmt(&self, f: &mut Formatter<'_>) -> Result { f.write_str(match AltFormat::unpack(self) { DagPoint::Trusted(_) => "Trusted", DagPoint::Suspicious(_) => "Suspicious", diff --git a/consensus/src/effects/mod.rs b/consensus/src/effects/mod.rs index fd721147d..91fe90eec 100644 --- a/consensus/src/effects/mod.rs +++ b/consensus/src/effects/mod.rs @@ -1,8 +1,8 @@ -#[macro_use] -mod macros; - pub use alt_format::*; pub use context::*; +#[macro_use] +mod macros; + mod alt_format; mod context; diff --git a/consensus/src/engine/engine.rs b/consensus/src/engine/engine.rs index 5612abd13..2e8dfad84 100644 --- a/consensus/src/engine/engine.rs +++ b/consensus/src/engine/engine.rs @@ -16,7 +16,7 @@ use crate::intercom::{ BroadcastFilter, Broadcaster, BroadcasterSignal, Collector, Dispatcher, Downloader, PeerSchedule, PeerScheduleUpdater, Responder, }; -use crate::models::{Point, PrevPoint, Round}; +use crate::models::{Link, Point, PrevPoint, Round}; use crate::LogFlavor; pub struct Engine { @@ -30,7 +30,7 @@ pub struct Engine { broadcast_filter: BroadcastFilter, collector: Collector, tasks: JoinSet<()>, // should be JoinSet - committed: mpsc::UnboundedSender<(Arc, Vec>)>, + committed: mpsc::UnboundedSender<(Point, Vec)>, input_buffer: InputBuffer, } @@ -39,7 +39,7 @@ impl Engine { key_pair: Arc, dht_client: &DhtClient, overlay_service: &OverlayService, - committed: mpsc::UnboundedSender<(Arc, Vec>)>, + committed: mpsc::UnboundedSender<(Point, Vec)>, input_buffer: InputBuffer, ) -> Self { let peer_schedule = Arc::new(PeerSchedule::new(key_pair)); @@ -92,23 +92,30 @@ impl Engine { pub async fn init_with_genesis(&mut self, next_peers: &[PeerId]) { let genesis = crate::test_utils::genesis(); let span = tracing::error_span!("init engine with genesis"); - let span_guard = span.enter(); + let _guard = span.enter(); // check only genesis round as it is widely used in point validation. // if some nodes use distinct genesis data, their first points will be rejected assert_eq!( - genesis.body.location.round, - MempoolConfig::GENESIS_ROUND, - "genesis point round must match genesis round from config" + genesis.id(), + crate::test_utils::genesis_point_id(), + "genesis point id does not match one from config" ); + assert!( + genesis.is_integrity_ok(), + "genesis point does not pass integrity check" + ); + assert!(genesis.is_well_formed(), "genesis point is not well formed"); // finished epoch self.peer_schedule - .set_next_start(genesis.body.location.round); - self.peer_schedule_updater - .set_next_peers(&[genesis.body.location.author], false); + .set_next_start(MempoolConfig::GENESIS_ROUND); + self.peer_schedule_updater.set_next_peers( + &[crate::test_utils::genesis_point_id().location.author], + false, + ); self.peer_schedule.rotate(); // current epoch self.peer_schedule - .set_next_start(genesis.body.location.round.next()); + .set_next_start(genesis.body().location.round.next()); // start updater only after peers are populated into schedule self.peer_schedule_updater.set_next_peers(next_peers, true); self.peer_schedule.rotate(); @@ -120,16 +127,6 @@ impl Engine { current_dag_round.insert_exact_sign(&genesis, &self.peer_schedule, &span); self.collector .init(current_dag_round.round().next(), iter::once(genesis_state)); - drop(span_guard); - Self::expect_trusted_point( - current_dag_round.to_weak(), - genesis.clone(), - self.peer_schedule.clone(), - self.downloader.clone(), - span, - ) - .await - .expect("genesis must be valid"); } pub async fn run(mut self) -> ! { @@ -150,7 +147,7 @@ impl Engine { .dag .top(current_dag_round.round().next(), &self.peer_schedule); - let (bcaster_ready_tx, bcaster_ready_rx) = mpsc::channel(1); + let (bcaster_ready_tx, bcaster_ready_rx) = oneshot::channel(); // let this channel unbounded - there won't be many items, but every of them is essential let (collector_signal_tx, mut collector_signal_rx) = mpsc::unbounded_channel(); let (own_point_state_tx, own_point_state_rx) = oneshot::channel(); @@ -172,21 +169,19 @@ impl Engine { })) } else { drop(own_point_state_tx); - futures_util::future::Either::Left(futures_util::future::ready(Ok(None::< - Arc, - >))) + futures_util::future::Either::Left(futures_util::future::ready(Ok(None::))) }; let bcaster_run = tokio::spawn({ - let own_ppint_round = current_dag_round.to_weak(); + let own_point_round = current_dag_round.downgrade(); let round_effects = round_effects.clone(); let peer_schedule = self.peer_schedule.clone(); let mut broadcaster = self.broadcaster; let downloader = self.downloader.clone(); async move { if let Some(own_point) = own_point_fut.await.expect("new point producer") { - let paranoid = Self::expect_trusted_point( - own_ppint_round, + let paranoid = Self::expect_own_trusted_point( + own_point_round, own_point.clone(), peer_schedule.clone(), downloader, @@ -204,13 +199,13 @@ impl Engine { // join the check, just not to miss it; it must have completed already paranoid.await.expect("verify own produced point"); let prev_point = PrevPoint { - digest: own_point.digest.clone(), + digest: own_point.digest().clone(), evidence: evidence.into_iter().collect(), }; (broadcaster, Some(Arc::new(prev_point))) } else { collector_signal_rx.close(); - bcaster_ready_tx.send(BroadcasterSignal::Ok).await.ok(); + bcaster_ready_tx.send(BroadcasterSignal::Ok).ok(); (broadcaster, None) } } @@ -232,10 +227,12 @@ impl Engine { bcaster_ready_rx, )); - self.responder - .update(&self.broadcast_filter, &next_dag_round, &round_effects); - self.broadcast_filter - .advance_round(next_dag_round.round(), &round_effects); + self.responder.update( + &self.broadcast_filter, + &next_dag_round, + &self.downloader, + &round_effects, + ); match tokio::join!(collector_run, bcaster_run, commit_run) { (Ok(collector_upd), Ok((bcaster, new_prev_point)), Ok(())) => { @@ -264,16 +261,18 @@ impl Engine { peer_schedule: Arc, own_point_state: oneshot::Sender, input_buffer: InputBuffer, - ) -> Option> { + ) -> Option { if let Some(own_point) = Producer::new_point(¤t_dag_round, prev_point.as_deref(), &input_buffer) { tracing::info!( parent: round_effects.span(), - digest = display(own_point.digest.alt()), + digest = display(own_point.digest().alt()), payload_bytes = own_point - .body.payload.iter().map(|bytes| bytes.len()).sum::(), - externals = own_point.body.payload.len(), + .body().payload.iter().map(|bytes| bytes.len()).sum::(), + externals = own_point.body().payload.len(), + is_proof = Some(own_point.body().anchor_proof == Link::ToSelf).filter(|x| *x), + is_trigger = Some(own_point.body().anchor_trigger == Link::ToSelf).filter(|x| *x), "produced point" ); let state = current_dag_round.insert_exact_sign( @@ -300,24 +299,29 @@ impl Engine { .join("; \n") } - fn expect_trusted_point( + fn expect_own_trusted_point( point_round: WeakDagRound, - point: Arc, + point: Point, peer_schedule: Arc, downloader: Downloader, span: Span, ) -> JoinHandle<()> { tokio::spawn(async move { - if Verifier::verify(&point, &peer_schedule).is_err() { + if let Err(dag_point) = Verifier::verify(&point, &peer_schedule) { let _guard = span.enter(); - panic!("Failed to verify, expected Trusted point: {:?}", point.id()) + panic!( + "Failed to verify own point: {} {:?}", + dag_point.alt(), + point.id() + ) } let dag_point = Verifier::validate(point.clone(), point_round, downloader, span.clone()).await; if dag_point.trusted().is_none() { let _guard = span.enter(); panic!( - "Failed to validate, expected Trusted point: {:?}", + "Failed to validate own point: {} {:?}", + dag_point.alt(), point.id() ) }; @@ -341,7 +345,7 @@ impl Effects { }) } - pub(crate) fn log_committed(&self, committed: &[(Arc, Vec>)]) { + pub(crate) fn log_committed(&self, committed: &[(Point, Vec)]) { if !committed.is_empty() && MempoolConfig::LOG_FLAVOR == LogFlavor::Truncated && tracing::enabled!(tracing::Level::DEBUG) @@ -356,7 +360,7 @@ impl Effects { format!( "anchor {:?} time {} : [ {history} ]", anchor.id().alt(), - anchor.body.time + anchor.body().time ) }) .join(" ; "); diff --git a/consensus/src/engine/input_buffer.rs b/consensus/src/engine/input_buffer.rs index 9c82ec749..7751e43d6 100644 --- a/consensus/src/engine/input_buffer.rs +++ b/consensus/src/engine/input_buffer.rs @@ -89,6 +89,7 @@ impl InputBufferData { ); let max_data_bytes = MempoolConfig::PAYLOAD_BUFFER_BYTES - payload_bytes; + let data_bytes_pre = self.data_bytes; if self.data_bytes > max_data_bytes { let to_drop = self .data @@ -104,6 +105,14 @@ impl InputBufferData { self.offset_elements = self.offset_elements.saturating_sub(to_drop); _ = self.data.drain(..to_drop); + + metrics::counter!("tycho_mempool_evicted_externals_count").increment(to_drop as _); + + tracing::warn!( + count = to_drop, + size = data_bytes_pre - self.data_bytes, + "evicted externals", + ); } self.data_bytes += payload_bytes; diff --git a/consensus/src/engine/mempool_config.rs b/consensus/src/engine/mempool_config.rs index 6939bb05d..0e743a7c8 100644 --- a/consensus/src/engine/mempool_config.rs +++ b/consensus/src/engine/mempool_config.rs @@ -48,16 +48,22 @@ impl MempoolConfig { /// (dependencies and/or signatures), and waiting for slow nodes pub const RETRY_INTERVAL: Duration = Duration::from_millis(150); - /// should not be less than 3 (as in average 1 of 3 is unreliable and another one did not sign); - /// includes at least the author of dependant point and the author of dependency point; - /// increases exponentially on every attempt, until every node out of 2F+1 is queried once - /// or a verifiable point is found (ill-formed or incorrectly signed points are not eligible) - pub const DOWNLOAD_PEERS: u8 = 3; - /// hard limit on cached external messages ring buffer, see [`Self::PAYLOAD_BATCH_BYTES`] pub const PAYLOAD_BUFFER_BYTES: usize = 50 * 1024 * 1024; - /// every failed response is accounted as point is not found; - /// 1/3+1 failed responses leads to invalidation of the point and all its dependants - pub const DOWNLOAD_SPAWN_INTERVAL: Duration = Duration::from_millis(50); + /// should not be less than 3 (as in average 1 of 3 is unreliable and another one did not sign); + /// value is multiplied by the current attempt number, until 2F+1 successfully responded + /// or a verifiable point is found (ill-formed or incorrectly signed points are not eligible) + pub const DOWNLOAD_PEERS: u8 = 5; + + /// [`Downloader`](crate::intercom::Downloader) makes responses in groups after previous + /// group completed or this interval elapsed (in order to not wait for some slow responding peer) + /// + /// 2F+1 "point not found" responses lead to invalidation of all referencing points; + /// failed network queries are retried after all peers were queried the same amount of times, + /// and only successful responses that point is not found are taken into account. + /// + /// Notice that reliable peers respond immediately with points they already have + /// validated successfully, or return `None`. + pub const DOWNLOAD_INTERVAL: Duration = Duration::from_millis(30); } diff --git a/consensus/src/intercom/broadcast/broadcast_filter.rs b/consensus/src/intercom/broadcast/broadcast_filter.rs index 9152c0ee2..2e5d41fd2 100644 --- a/consensus/src/intercom/broadcast/broadcast_filter.rs +++ b/consensus/src/intercom/broadcast/broadcast_filter.rs @@ -1,3 +1,4 @@ +use std::cmp::Ordering; use std::collections::BTreeMap; use std::sync::Arc; @@ -7,11 +8,12 @@ use tycho_network::PeerId; use tycho_util::FastDashMap; use super::dto::ConsensusEvent; -use crate::dag::Verifier; +use crate::dag::{DagRound, Verifier}; +use crate::dyn_event; use crate::effects::{AltFormat, CurrentRoundContext, Effects}; use crate::engine::MempoolConfig; use crate::intercom::dto::PeerState; -use crate::intercom::PeerSchedule; +use crate::intercom::{Downloader, PeerSchedule}; use crate::models::{DagPoint, Digest, Location, NodeCount, Point, PointId, Round}; #[derive(Clone)] @@ -36,19 +38,24 @@ impl BroadcastFilter { pub fn add( &self, - peer_id: &PeerId, - point: &Arc, - top_dag_round: Round, + sender: &PeerId, + point: &Point, + top_dag_round: &DagRound, + downloader: &Downloader, effects: &Effects, ) { - let point_round = point.body.location.round; - if self.inner.add(peer_id, point, top_dag_round, effects) { - self.inner.advance_round(point_round, effects); - } + self.inner + .add(sender, point, top_dag_round, downloader, effects); } - pub fn advance_round(&self, new_round: Round, round_effects: &Effects) { - self.inner.advance_round(new_round, round_effects); + pub fn advance_round( + &self, + top_dag_round: &DagRound, + downloader: &Downloader, + round_effects: &Effects, + ) { + self.inner + .advance_round(top_dag_round, downloader, round_effects); } pub async fn clear_cache(self) -> ! { @@ -79,11 +86,11 @@ impl BroadcastFilter { } } -type SimpleDagLocations = BTreeMap>>; +type SimpleDagLocations = BTreeMap>; struct BroadcastFilterInner { // defend from spam from future rounds: // should keep rounds greater than current dag round - last_by_peer: FastDashMap, + last_by_peer: FastDashMap, // very much like DAG structure, but without dependency check; // just to determine reliably that consensus advanced without current node by_round: FastDashMap, @@ -103,11 +110,12 @@ impl BroadcastFilterInner { /// returns `true` if round should be advanced; fn add( &self, - peer_id: &PeerId, - point: &Arc, - engine_round: Round, + sender: &PeerId, + point: &Point, + top_dag_round: &DagRound, + downloader: &Downloader, effects: &Effects, - ) -> bool { + ) { // for any node @ r+0, its DAG always contains [r-DAG_DEPTH-N; r+1] rounds, where N>=2 let PointId { @@ -115,10 +123,10 @@ impl BroadcastFilterInner { digest, } = point.id(); - let verified = if peer_id != author { + let verified = if sender != author { tracing::error!( parent: effects.span(), - peer = display(peer_id.alt()), + sender = display(sender.alt()), author = display(author.alt()), round = round.0, digest = display(digest.alt()), @@ -138,40 +146,59 @@ impl BroadcastFilterInner { }) }; - if round <= engine_round { - let event = verified.map_or_else(ConsensusEvent::Invalid, |_| { - ConsensusEvent::Verified(point.clone()) - }); - self.output.send(event).ok(); - return false; + if round <= top_dag_round.round() { + let Some(point_round) = top_dag_round.scan(point.body().location.round) else { + return; // too old point + }; + match verified { + Ok(()) => self.send_validating(&point_round, point, downloader, effects), + Err(invalid) => point_round.insert_invalid_exact(sender, &invalid), + }; + return; } // else: either consensus moved forward without us, // or we shouldn't accept the point yet, or it's a spam - let last_peer_round = *self + let (last_peer_round, duplicates) = *self .last_by_peer .entry(author) - .and_modify(|last_by_peer| { - if *last_by_peer < round { - *last_by_peer = round; + .and_modify(|(last_by_peer, duplcates)| { + match round.cmp(last_by_peer) { + Ordering::Less => {} // do nothing, blame later + Ordering::Equal => *duplcates += 1, + Ordering::Greater => { + *last_by_peer = round; + *duplcates = 0; + } } }) - .or_insert(round); - if last_peer_round > round { + .or_insert((round, 0)); + if verified.is_err() || round < last_peer_round || duplicates > 0 { // we should ban a peer that broadcasts its rounds out of order, // though we cannot prove this decision for other nodes - tracing::error!( + let level = if verified.is_err() || round < last_peer_round || duplicates >= 3 { + // that's severe errors, that require ban + tracing::Level::ERROR + } else { + // sender could have not received our response, thus decides to retry + tracing::Level::WARN + }; + dyn_event!( parent: effects.span(), + level, + malformed = verified.is_err(), + out_of_order = last_peer_round > round, + duplicated = duplicates > 0, last_round = last_peer_round.0, + duplicates = duplicates, author = display(author.alt()), round = round.0, digest = display(digest.alt()), - "out of order" + "misbehaving broadcast" ); - return false; + // do not determine next round by garbage points; it's a ban + return; }; - if verified.is_err() { - return false; // do not determine next round by garbage points; it's a ban - } + let is_threshold_reached = { // note: lock guard inside result let try_by_round_entry = self.by_round.entry(round).or_try_insert_with(|| { @@ -190,33 +217,42 @@ impl BroadcastFilterInner { .entry(author) .or_default() .insert(digest.clone(), point.clone()); - same_round.len() >= node_count.reliable_minority() + // send `Forward` signal inside `add` just once per round, not for every point + same_round.len() == node_count.reliable_minority() } } }; - if !is_threshold_reached { + if is_threshold_reached { + self.output + .send(ConsensusEvent::Forward(round)) + .expect("channel from filter to collector closed"); + } else { tracing::trace!( parent: effects.span(), - engine_round = engine_round.0, author = display(author.alt()), round = round.0, digest = display(digest.alt()), "threshold not reached yet" ); } - is_threshold_reached } /// drop everything up to the new round (inclusive), channelling cached points; - fn advance_round(&self, new_round: Round, effects: &Effects) { + fn advance_round( + &self, + top_dag_round: &DagRound, + downloader: &Downloader, + effects: &Effects, + ) { // concurrent callers may break historical order of messages in channel // until new_round is channeled, and Collector can cope with it; // but engine_round must be set to the greatest value under lock + let top_round = top_dag_round.round(); let mut rounds = self .by_round .iter() .map(|el| *el.key()) - .filter(|key| *key <= new_round) + .filter(|key| *key <= top_round) .collect::>(); rounds.sort(); if rounds.is_empty() { @@ -233,19 +269,21 @@ impl BroadcastFilterInner { missed.push(round.0); continue; }; + let Some(point_round) = top_dag_round.scan(round) else { + missed.push(round.0); + continue; + }; self.output .send(ConsensusEvent::Forward(round)) - .expect("channel from filter to broadcaster closed"); - for (_, point) in points.into_iter().flat_map(|(_, v)| v.into_iter()) { - self.output - .send(ConsensusEvent::Verified(point)) - .expect("channel from filter to broadcaster closed"); + .expect("channel from filter to collector closed"); + for (_, point) in points.iter().flat_map(|(_, v)| v.iter()) { + self.send_validating(&point_round, point, downloader, effects); } released.push(round.0); } tracing::debug!( parent: effects.span(), - to = new_round.0, + to = top_round.0, released = debug(released), missed = debug(missed), "advance round" @@ -254,7 +292,27 @@ impl BroadcastFilterInner { // TODO there must be some config value - when node needs to sync; // values too far in the future are some garbage, must ban authors self.by_round.retain(|round, _| { - new_round < *round && round.0 <= new_round.0 + MempoolConfig::COMMIT_DEPTH as u32 + top_round < *round && round.0 <= top_round.0 + MempoolConfig::COMMIT_DEPTH as u32 }); } + + fn send_validating( + &self, + point_round: &DagRound, + point: &Point, + downloader: &Downloader, + effects: &Effects, + ) { + // this may be not the first insert into DAG - in case some node received it earlier, + // and we've already received its point that references current broadcast + if let Some(first_state_fut) = point_round.add_broadcast_exact(point, downloader, effects) { + let validating = ConsensusEvent::Validating { + point_id: point.id(), + task: first_state_fut, + }; + self.output + .send(validating) + .expect("channel from filter to collector closed"); + } + } } diff --git a/consensus/src/intercom/broadcast/broadcaster.rs b/consensus/src/intercom/broadcast/broadcaster.rs index d843e8410..b756f6089 100644 --- a/consensus/src/intercom/broadcast/broadcaster.rs +++ b/consensus/src/intercom/broadcast/broadcaster.rs @@ -1,24 +1,22 @@ use std::mem; +use anyhow::Result; use futures_util::future::BoxFuture; use futures_util::stream::FuturesUnordered; use futures_util::StreamExt; use tokio::sync::broadcast::error::RecvError; -use tokio::sync::broadcast::{self}; -use tokio::sync::mpsc; +use tokio::sync::{broadcast, mpsc, oneshot}; use tycho_network::PeerId; use tycho_util::{FastHashMap, FastHashSet}; use crate::dyn_event; use crate::effects::{AltFormat, CurrentRoundContext, Effects, EffectsContext}; use crate::intercom::broadcast::collector::CollectorSignal; -use crate::intercom::dto::{PeerState, SignatureResponse}; +use crate::intercom::broadcast::utils::QueryResponses; +use crate::intercom::dto::{BroadcastResponse, PeerState, SignatureResponse}; use crate::intercom::{Dispatcher, PeerSchedule}; use crate::models::{Digest, NodeCount, Point, Round, Signature}; -type BcastResult = anyhow::Result<()>; -type SigResult = anyhow::Result; - #[derive(Copy, Clone, Debug)] pub enum BroadcasterSignal { Ok, @@ -28,14 +26,15 @@ pub enum BroadcasterSignal { pub struct Broadcaster { dispatcher: Dispatcher, // do not throw away unfinished broadcasts from previous round - bcasts_outdated: FuturesUnordered>, + bcast_outdated: Option>, } impl Broadcaster { pub fn new(dispatcher: &Dispatcher) -> Self { Self { dispatcher: dispatcher.clone(), - bcasts_outdated: FuturesUnordered::new(), + // will be replaced with `None` during task execution + bcast_outdated: Some(QueryResponses::default()), } } pub async fn run( @@ -43,21 +42,46 @@ impl Broadcaster { round_effects: &Effects, point: &Point, peer_schedule: &PeerSchedule, - bcaster_signal: mpsc::Sender, + bcaster_signal: oneshot::Sender, collector_signal: mpsc::UnboundedReceiver, ) -> FastHashMap { - let mut task = BroadcasterTask::new( - Effects::::new(round_effects, &point.digest), - point, - &self.dispatcher, - peer_schedule, - bcaster_signal, + let signers = peer_schedule + .peers_for(point.body().location.round.next()) + .iter() + .map(|(peer_id, _)| *peer_id) + .collect::>(); + let signers_count = NodeCount::new(signers.len()); + + let bcast_outdated = mem::take(&mut self.bcast_outdated).expect("cannot be unset"); + let mut bcast_peers = peer_schedule.all_resolved(); + bcast_peers.retain(|peer| !bcast_outdated.contains(peer)); + + let mut task = BroadcasterTask { + effects: Effects::::new(round_effects, point.digest()), + dispatcher: self.dispatcher.clone(), + current_round: point.body().location.round, + point_digest: point.digest().clone(), + bcaster_signal: Some(bcaster_signal), collector_signal, - mem::take(&mut self.bcasts_outdated), - ); - task.run().await; + + peer_updates: peer_schedule.updates(), + signers, + signers_count, + removed_peers: Default::default(), + rejections: Default::default(), + signatures: Default::default(), + + bcast_request: Dispatcher::broadcast_request(point), + bcast_current: QueryResponses::default(), + bcast_outdated, + + sig_request: Dispatcher::signature_request(point.body().location.round), + sig_peers: FastHashSet::default(), + sig_current: FuturesUnordered::default(), + }; + task.run(bcast_peers).await; // preserve only broadcasts from the last round and drop older ones as hung up - self.bcasts_outdated.extend(task.bcast_futs); + self.bcast_outdated = Some(task.bcast_current); task.signatures } } @@ -65,11 +89,10 @@ impl Broadcaster { struct BroadcasterTask { effects: Effects, dispatcher: Dispatcher, - bcasts_outdated: FuturesUnordered>, current_round: Round, point_digest: Digest, /// Receiver may be closed (collector finished), so do not require `Ok` on send - bcaster_signal: mpsc::Sender, + bcaster_signal: Option>, collector_signal: mpsc::UnboundedReceiver, peer_updates: broadcast::Receiver<(PeerId, PeerState)>, @@ -82,61 +105,17 @@ struct BroadcasterTask { signatures: FastHashMap, bcast_request: tycho_network::Request, - bcast_peers: FastHashSet, - bcast_futs: FuturesUnordered>, + bcast_current: QueryResponses, + bcast_outdated: QueryResponses, + sig_request: tycho_network::Request, sig_peers: FastHashSet, - sig_futs: FuturesUnordered>, + sig_current: FuturesUnordered)>>, } impl BroadcasterTask { - fn new( - effects: Effects, - point: &Point, - dispatcher: &Dispatcher, - peer_schedule: &PeerSchedule, - bcaster_signal: mpsc::Sender, - collector_signal: mpsc::UnboundedReceiver, - bcasts_outdated: FuturesUnordered>, - ) -> Self { - let _guard = effects.span().clone().entered(); - let peer_updates = peer_schedule.updates(); - let signers = peer_schedule - .peers_for(point.body.location.round.next()) - .iter() - .map(|(peer_id, _)| *peer_id) - .collect::>(); - let signers_count = NodeCount::new(signers.len()); - let collectors = peer_schedule.all_resolved(); - let bcast_request = Dispatcher::broadcast_request(point); - let sig_request = Dispatcher::signature_request(point.body.location.round); - Self { - effects, - dispatcher: dispatcher.clone(), - bcasts_outdated, - current_round: point.body.location.round, - point_digest: point.digest.clone(), - bcaster_signal, - collector_signal, - - peer_updates, - signers, - signers_count, - removed_peers: Default::default(), - rejections: Default::default(), - signatures: Default::default(), - - bcast_request, - bcast_peers: collectors, - bcast_futs: FuturesUnordered::new(), - - sig_request, - sig_peers: Default::default(), - sig_futs: FuturesUnordered::new(), - } - } /// returns evidence for broadcast point - pub async fn run(&mut self) { + pub async fn run(&mut self, bcast_peers: FastHashSet) { // how this was supposed to work: // * in short: broadcast to all and gather signatures from those who accepted the point // * both broadcast and signature tasks have their own retry loop for every peer @@ -155,26 +134,29 @@ impl BroadcasterTask { // * successfully signed our point and dequeued tracing::debug!( parent: self.effects.span(), - collectors_count = self.bcast_peers.len(), + current_peers = bcast_peers.len(), + outdated_peers = self.bcast_outdated.len(), "start", ); - for peer_id in mem::take(&mut self.bcast_peers) { - self.broadcast(&peer_id); + for peer in bcast_peers { + self.broadcast(&peer); } loop { tokio::select! { - Some(_) = self.bcasts_outdated.next() => {} // let them complete + Some((peer, _)) = self.bcast_outdated.next() => { + self.broadcast(&peer); + } + Some((peer_id, result)) = self.bcast_current.next() => { + self.match_broadcast_result(&peer_id, result); + }, + Some((peer_id, result)) = self.sig_current.next() => { + self.match_signature_result(&peer_id, result); + }, Some(collector_signal) = self.collector_signal.recv() => { - if self.should_finish(collector_signal).await { + if self.should_finish(collector_signal) { break; } } - Some((peer_id, result)) = self.bcast_futs.next() => { - self.match_broadcast_result(peer_id, result); - }, - Some((peer_id, result)) = self.sig_futs.next() => { - self.match_signature_result(peer_id, result); - }, update = self.peer_updates.recv() => { self.match_peer_updates(update); } @@ -186,23 +168,24 @@ impl BroadcasterTask { } } - async fn should_finish(&mut self, collector_signal: CollectorSignal) -> bool { + fn should_finish(&mut self, collector_signal: CollectorSignal) -> bool { let result = match collector_signal { // though we return successful result, it will be discarded on Err CollectorSignal::Finish | CollectorSignal::Err => true, CollectorSignal::Retry => { if self.rejections.len() >= self.signers_count.reliable_minority() { - _ = self.bcaster_signal.send(BroadcasterSignal::Err).await; + if let Some(sender) = mem::take(&mut self.bcaster_signal) { + _ = sender.send(BroadcasterSignal::Err); + }; true } else { if self.signatures.len() >= self.signers_count.majority_of_others() { - _ = self.bcaster_signal.send(BroadcasterSignal::Ok).await; + if let Some(sender) = mem::take(&mut self.bcaster_signal) { + _ = sender.send(BroadcasterSignal::Ok); + }; } - for peer_id in mem::take(&mut self.sig_peers) { - self.request_signature(&peer_id); - } - for peer_id in mem::take(&mut self.bcast_peers) { - self.broadcast(&peer_id); + for peer in mem::take(&mut self.sig_peers) { + self.request_signature(&peer); } false } @@ -221,12 +204,10 @@ impl BroadcasterTask { result } - fn match_broadcast_result(&mut self, peer_id: PeerId, result: BcastResult) { + fn match_broadcast_result(&mut self, peer_id: &PeerId, result: Result) { match result { Err(error) => { - // TODO distinguish timeouts from models incompatibility etc - // self.bcast_peers.push(peer_id); // let it retry - self.sig_peers.insert(peer_id); // lighter weight retry loop + self.sig_peers.insert(*peer_id); // lighter weight retry loop tracing::error!( parent: self.effects.span(), peer = display(peer_id.alt()), @@ -234,22 +215,22 @@ impl BroadcasterTask { "failed to send broadcast to" ); } - Ok(_) => { + Ok(BroadcastResponse) => { + // self.sig_peers.insert(*peer_id); // give some time to validate + self.request_signature(peer_id); // fast nodes may have delivered it as a dependency tracing::trace!( parent: self.effects.span(), peer = display(peer_id.alt()), "finished broadcast to" ); - self.request_signature(&peer_id); } } } - fn match_signature_result(&mut self, peer_id: PeerId, result: SigResult) { + fn match_signature_result(&mut self, peer_id: &PeerId, result: Result) { match result { Err(error) => { - // TODO distinguish timeouts from models incompatibility etc - self.sig_peers.insert(peer_id); // let it retry + self.sig_peers.insert(*peer_id); // let it retry tracing::error!( parent: self.effects.span(), peer = display(peer_id.alt()), @@ -258,10 +239,9 @@ impl BroadcasterTask { ); } Ok(response) => { - let level = if response == SignatureResponse::Rejected { - tracing::Level::WARN - } else { - tracing::Level::DEBUG + let level = match response { + SignatureResponse::Rejected(_) => tracing::Level::WARN, + _ => tracing::Level::DEBUG, }; dyn_event!( parent: self.effects.span(), @@ -272,21 +252,21 @@ impl BroadcasterTask { ); match response { SignatureResponse::Signature(signature) => { - if self.signers.contains(&peer_id) { - if signature.verifies(&peer_id, &self.point_digest) { - self.signatures.insert(peer_id, signature); + if self.signers.contains(peer_id) { + if signature.verifies(peer_id, &self.point_digest) { + self.signatures.insert(*peer_id, signature); } else { // any invalid signature lowers our chances // to successfully finish current round - self.rejections.insert(peer_id); + self.rejections.insert(*peer_id); } } } - SignatureResponse::NoPoint => self.broadcast(&peer_id), - SignatureResponse::TryLater => _ = self.sig_peers.insert(peer_id), - SignatureResponse::Rejected => { - if self.signers.contains(&peer_id) { - self.rejections.insert(peer_id); + SignatureResponse::NoPoint => self.broadcast(peer_id), // send data immediately + SignatureResponse::TryLater => _ = self.sig_peers.insert(*peer_id), + SignatureResponse::Rejected(_) => { + if self.signers.contains(peer_id) { + self.rejections.insert(*peer_id); } } } @@ -296,8 +276,8 @@ impl BroadcasterTask { fn broadcast(&mut self, peer_id: &PeerId) { if self.removed_peers.is_empty() || !self.removed_peers.remove(peer_id) { - self.bcast_futs - .push(self.dispatcher.send(peer_id, &self.bcast_request)); + self.bcast_current + .push(peer_id, self.dispatcher.query(peer_id, &self.bcast_request)); tracing::trace!( parent: self.effects.span(), peer = display(peer_id.alt()), @@ -314,7 +294,7 @@ impl BroadcasterTask { fn request_signature(&mut self, peer_id: &PeerId) { if self.removed_peers.is_empty() || !self.removed_peers.remove(peer_id) { - self.sig_futs + self.sig_current .push(self.dispatcher.query(peer_id, &self.sig_request)); tracing::trace!( parent: self.effects.span(), diff --git a/consensus/src/intercom/broadcast/collector.rs b/consensus/src/intercom/broadcast/collector.rs index 412440f92..9f0f85c84 100644 --- a/consensus/src/intercom/broadcast/collector.rs +++ b/consensus/src/intercom/broadcast/collector.rs @@ -58,14 +58,14 @@ impl Collector { next_dag_round: DagRound, // r+1 own_point_state: oneshot::Receiver, collector_signal: mpsc::UnboundedSender, - bcaster_signal: mpsc::Receiver, + bcaster_signal: oneshot::Receiver, ) -> Self { let effects = Effects::::new(&round_effects); let span_guard = effects.span().clone().entered(); let current_dag_round = next_dag_round .prev() - .get() + .upgrade() .expect("current DAG round must be linked into DAG chain"); let includes = mem::take(&mut self.next_includes); includes.push( @@ -104,12 +104,11 @@ impl Collector { next_includes: FuturesUnordered::new(), collector_signal, - bcaster_signal, is_bcaster_ready_ok: false, }; drop(span_guard); - let result = task.run(&mut self.from_bcast_filter).await; + let result = task.run(&mut self.from_bcast_filter, bcaster_signal).await; match result { Ok(includes) => self.next_includes = includes, Err(round) => self.next_round = round, @@ -139,7 +138,6 @@ struct CollectorTask { next_includes: FuturesUnordered>, /// Receiver may be closed (bcaster finished), so do not require `Ok` on send collector_signal: mpsc::UnboundedSender, - bcaster_signal: mpsc::Receiver, is_bcaster_ready_ok: bool, } @@ -150,11 +148,13 @@ impl CollectorTask { async fn run( mut self, from_bcast_filter: &mut mpsc::UnboundedReceiver, + bcaster_signal: oneshot::Receiver, ) -> Result>, Round> { let mut retry_interval = tokio::time::interval(MempoolConfig::RETRY_INTERVAL); + let mut bcaster_signal = std::pin::pin!(bcaster_signal); loop { tokio::select! { - Some(bcaster_signal) = self.bcaster_signal.recv() => { + Ok(bcaster_signal) = &mut bcaster_signal, if !self.is_bcaster_ready_ok => { if self.should_fail(bcaster_signal) { // has to jump over one round return Err(self.next_dag_round.round().next()) @@ -174,7 +174,7 @@ impl CollectorTask { }, filtered = from_bcast_filter.recv() => match filtered { Some(consensus_event) => { - if let Err(round) = self.match_filtered(&consensus_event) { + if let Err(round) = self.match_filtered(consensus_event) { _ = self.collector_signal.send(CollectorSignal::Err); return Err(round) } @@ -198,7 +198,6 @@ impl CollectorTask { let result = match signal { BroadcasterSignal::Ok => { self.is_bcaster_ready_ok = true; - self.bcaster_signal.close(); false } BroadcasterSignal::Err => true, @@ -234,7 +233,7 @@ impl CollectorTask { fn jump_up(&mut self, state: InclusionState) -> Option> { // its ok to discard invalid state from `next_includes` queue - let point_round = state.point()?.valid()?.point.body.location.round; + let point_round = state.point()?.valid()?.point.body().location.round; // will be signed on the next round self.next_includes .push(futures_util::future::ready(state).boxed()); @@ -271,7 +270,7 @@ impl CollectorTask { result } - fn match_filtered(&self, consensus_event: &ConsensusEvent) -> Result<(), Round> { + fn match_filtered(&self, consensus_event: ConsensusEvent) -> Result<(), Round> { match consensus_event { ConsensusEvent::Forward(consensus_round) => { #[allow(clippy::match_same_arms)] @@ -291,72 +290,35 @@ impl CollectorTask { dyn_event!( parent: self.effects.span(), level, - event = display(consensus_event.alt()), + event = display("Forward"), round = consensus_round.0, "from bcast filter", ); if should_fail { - return Err(*consensus_round); + return Err(consensus_round); } } - ConsensusEvent::Verified(point) => { - let is_first = match point.body.location.round { - x if x > self.next_dag_round.round() => { - let _guard = self.effects.span().enter(); - panic!( - "Coding error: broadcast filter advanced \ - while collector left behind; event: {} {:?}", - consensus_event.alt(), - point.id() - ) - } - x if x == self.next_dag_round.round() => self - .next_dag_round - .add(point, &self.downloader, self.effects.span()) - .map(|task| self.next_includes.push(task)) - .is_some(), - x if x == self.current_round.round() => self - .current_round - .add(point, &self.downloader, self.effects.span()) - .map(|task| self.includes.push(task)) - .is_some(), - _ => self - .current_round - .add(point, &self.downloader, self.effects.span()) - // maybe other's dependency, but too old to be included - .is_some(), - }; - tracing::debug!( - parent: self.effects.span(), - event = display(consensus_event.alt()), - new = is_first, - author = display(point.body.location.author.alt()), - round = point.body.location.round.0, - digest = display(point.digest.alt()), - "from bcast filter", - ); - } - ConsensusEvent::Invalid(dag_point) => { - if dag_point.location().round > self.next_dag_round.round() { + ConsensusEvent::Validating { point_id, task } => { + if point_id.location.round > self.next_dag_round.round() { let _guard = self.effects.span().enter(); panic!( "Coding error: broadcast filter advanced \ - while collector left behind; event: {} {:?}", - consensus_event.alt(), - dag_point.id() + while collector left behind; Validating {:?}", + point_id.alt() ) - } else { - let is_first = self.next_dag_round.insert_invalid(dag_point).is_some(); - tracing::warn!( - parent: self.effects.span(), - event = display(consensus_event.alt()), - new = is_first, - author = display(dag_point.location().author.alt()), - round = dag_point.location().round.0, - digest = display(dag_point.digest().alt()), - "from bcast filter", - ); - } + } else if point_id.location.round == self.next_dag_round.round() { + self.next_includes.push(task); + } else if point_id.location.round == self.current_round.round() { + self.includes.push(task); + } // else maybe other's dependency, but too old to be included + tracing::debug!( + parent: self.effects.span(), + event = display("Validating"), + author = display(point_id.location.author.alt()), + round = point_id.location.round.0, + digest = display(point_id.digest.alt()), + "from bcast filter", + ); } }; Ok(()) @@ -393,6 +355,7 @@ impl CollectorTask { dyn_event!( parent: self.effects.span(), level, + result = display(dag_point.alt()), author = display(dag_point.location().author.alt()), round = dag_point.location().round.0, digest = display(dag_point.digest().alt()), diff --git a/consensus/src/intercom/broadcast/dto.rs b/consensus/src/intercom/broadcast/dto.rs index b370c2579..e03835989 100644 --- a/consensus/src/intercom/broadcast/dto.rs +++ b/consensus/src/intercom/broadcast/dto.rs @@ -1,49 +1,14 @@ -use std::sync::Arc; -use std::{cmp, fmt}; +use futures_util::future::BoxFuture; -use cmp::Ordering; +use crate::dag::InclusionState; +use crate::models::{PointId, Round}; -use crate::effects::{AltFmt, AltFormat}; -use crate::models::{DagPoint, Point, Round}; - -#[derive(Debug)] pub enum ConsensusEvent { // allows not to peek but poll the channel when local dag is not ready yet Forward(Round), // well-formed, but not yet validated against DAG - Verified(Arc), - Invalid(DagPoint), -} - -impl ConsensusEvent { - pub fn priority(a: &Self, b: &Self) -> Ordering { - #[allow(clippy::match_same_arms)] - match (a, b) { - // all forwards first - (ConsensusEvent::Forward(a), ConsensusEvent::Forward(b)) => a.cmp(b), - (ConsensusEvent::Forward(_), _) => Ordering::Greater, - // then all invalid ones - by round - (ConsensusEvent::Invalid(_), ConsensusEvent::Forward(_)) => Ordering::Less, - (ConsensusEvent::Invalid(a), ConsensusEvent::Invalid(b)) => { - a.location().round.cmp(&b.location().round) - } - (ConsensusEvent::Invalid(_), ConsensusEvent::Verified(_)) => Ordering::Greater, - // then all valid ones - by round - (ConsensusEvent::Verified(a), ConsensusEvent::Verified(b)) => { - a.body.location.round.cmp(&b.body.location.round) - } - (ConsensusEvent::Verified(_), _) => Ordering::Less, - } - } -} - -impl AltFormat for ConsensusEvent {} -impl fmt::Display for AltFmt<'_, ConsensusEvent> { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.write_str(match AltFormat::unpack(self) { - ConsensusEvent::Forward(_) => "Forward", - ConsensusEvent::Verified(_) => "Verified", - ConsensusEvent::Invalid(_) => "Invalid", - }) - } + Validating { + point_id: PointId, + task: BoxFuture<'static, InclusionState>, + }, } diff --git a/consensus/src/intercom/broadcast/mod.rs b/consensus/src/intercom/broadcast/mod.rs index 8b173ca85..a95541833 100644 --- a/consensus/src/intercom/broadcast/mod.rs +++ b/consensus/src/intercom/broadcast/mod.rs @@ -11,3 +11,4 @@ mod broadcaster; mod collector; mod dto; mod signer; +mod utils; diff --git a/consensus/src/intercom/broadcast/signer.rs b/consensus/src/intercom/broadcast/signer.rs index 85c3e0d69..958832936 100644 --- a/consensus/src/intercom/broadcast/signer.rs +++ b/consensus/src/intercom/broadcast/signer.rs @@ -2,7 +2,7 @@ use tycho_network::PeerId; use crate::dag::DagRound; use crate::effects::{AltFormat, CurrentRoundContext, Effects}; -use crate::intercom::dto::SignatureResponse; +use crate::intercom::dto::{SignatureRejectedReason, SignatureResponse}; use crate::models::Round; use crate::{dyn_event, MempoolConfig}; @@ -15,10 +15,9 @@ impl Signer { effects: &Effects, ) -> SignatureResponse { let response = Self::make_signature_response(round, author, next_dag_round); - let level = if response == SignatureResponse::Rejected { - tracing::Level::WARN - } else { - tracing::Level::TRACE + let level = match response { + SignatureResponse::Rejected(_) => tracing::Level::WARN, + _ => tracing::Level::TRACE, }; dyn_event!( parent: effects.span(), @@ -45,13 +44,13 @@ impl Signer { let engine_round = next_dag_round.round().prev(); if round.next() < engine_round { // lagged too far from consensus and us, will sign only 1 round behind current; - return SignatureResponse::Rejected; + return SignatureResponse::Rejected(SignatureRejectedReason::TooOldRound); } let Some(point_dag_round) = next_dag_round.scan(round) else { // may happen on init from genesis, when DAG is not yet populated with points, // but a misbehaving node requests a point older than genesis, which cannot be signed - return SignatureResponse::Rejected; + return SignatureResponse::Rejected(SignatureRejectedReason::NoDagRound); }; // TODO do not state().clone() - mutating closure on location is easily used; // need to remove inner locks from InclusionState and leave it guarded by DashMap; @@ -63,7 +62,7 @@ impl Signer { if let Some(signable) = state.signable() { let is_witness = round.next() == engine_round; let current_dag_round_bind = if is_witness { - next_dag_round.prev().get() + next_dag_round.prev().upgrade() } else { None }; @@ -84,7 +83,7 @@ impl Signer { } match state.signed() { Some(Ok(signed)) => SignatureResponse::Signature(signed.with.clone()), - Some(Err(())) => SignatureResponse::Rejected, + Some(Err(())) => SignatureResponse::Rejected(SignatureRejectedReason::CannotSign), None => SignatureResponse::TryLater, } } diff --git a/consensus/src/intercom/broadcast/utils.rs b/consensus/src/intercom/broadcast/utils.rs new file mode 100644 index 000000000..5cc9f75e4 --- /dev/null +++ b/consensus/src/intercom/broadcast/utils.rs @@ -0,0 +1,42 @@ +use anyhow::Result; +use futures_util::future::BoxFuture; +use futures_util::stream::FuturesUnordered; +use futures_util::StreamExt; +use tycho_network::PeerId; +use tycho_util::FastHashSet; + +/// Encapsulates futures and corresponding peer ids for consistency +pub struct QueryResponses { + futures: FuturesUnordered)>>, + peers: FastHashSet, +} + +impl Default for QueryResponses { + fn default() -> Self { + Self { + futures: Default::default(), + peers: Default::default(), + } + } +} + +impl QueryResponses { + pub fn push(&mut self, peer_id: &PeerId, fut: BoxFuture<'static, (PeerId, Result)>) { + assert!(self.peers.insert(*peer_id), "insert duplicated response"); + self.futures.push(fut); + } + + pub fn len(&self) -> usize { + self.peers.len() + } + + pub fn contains(&self, peer_id: &PeerId) -> bool { + self.peers.contains(peer_id) + } + + pub async fn next(&mut self) -> Option<(PeerId, Result)> { + let (peer_id, result) = self.futures.next().await?; + assert!(self.peers.remove(&peer_id), "detected duplicated response"); + Some((peer_id, result)) + } +} diff --git a/consensus/src/intercom/core/dispatcher.rs b/consensus/src/intercom/core/dispatcher.rs index 307c6dac1..ba8bc9f48 100644 --- a/consensus/src/intercom/core/dispatcher.rs +++ b/consensus/src/intercom/core/dispatcher.rs @@ -46,6 +46,10 @@ impl Dispatcher { (&MPQuery::Signature(round)).into() } + pub fn broadcast_request(point: &Point) -> tycho_network::Request { + (&MPQuery::Broadcast(point.clone())).into() + } + pub fn query( &self, peer_id: &PeerId, @@ -71,26 +75,4 @@ impl Dispatcher { } .boxed() } - - pub fn broadcast_request(point: &Point) -> tycho_network::Request { - point.into() - } - - pub fn send( - &self, - peer_id: &PeerId, - request: &tycho_network::Request, - ) -> BoxFuture<'static, (PeerId, Result<()>)> { - let peer_id = *peer_id; - let request = request.clone(); - let overlay = self.overlay.clone(); - let network = self.network.clone(); - async move { - overlay - .send(&network, &peer_id, request) - .map(move |response| (peer_id, response)) - .await - } - .boxed() - } } diff --git a/consensus/src/intercom/core/dto.rs b/consensus/src/intercom/core/dto.rs index 6198fcbfa..888369e63 100644 --- a/consensus/src/intercom/core/dto.rs +++ b/consensus/src/intercom/core/dto.rs @@ -3,8 +3,15 @@ use bytes::Bytes; use serde::{Deserialize, Serialize}; use tycho_network::{Response, ServiceRequest, Version}; -use crate::intercom::dto::{PointByIdResponse, SignatureResponse}; +use crate::intercom::dto::{BroadcastResponse, PointByIdResponse, SignatureResponse}; use crate::models::{Point, PointId, Round}; +use crate::MempoolConfig; + +// 65535 bytes is a rough estimate for the largest point with more than 250 validators in set, +// as it contains 2 mappings of 32 (peer_id) to 32 (digest) valuable bytes (includes and witness), +// and 1 mapping of 32 (peer_id) to 64 (signature) valuable bytes (evidence); +// the size of other data is fixed, and estimate is more than enough to handle `Bytes` encoding +const LARGEST_DATA_BYTES: usize = u16::MAX as usize + MempoolConfig::PAYLOAD_BATCH_BYTES; // broadcast uses simple send_message with () return value impl From<&Point> for tycho_network::Request { @@ -18,12 +25,12 @@ impl From<&Point> for tycho_network::Request { #[derive(Serialize, Deserialize, Debug)] pub enum MPQuery { + Broadcast(Point), PointById(PointId), Signature(Round), } impl From<&MPQuery> for tycho_network::Request { - // TODO: move MPRequest et al to TL - won't need to copy Point fn from(value: &MPQuery) -> Self { tycho_network::Request { version: Version::V1, @@ -36,12 +43,16 @@ impl TryFrom<&ServiceRequest> for MPQuery { type Error = anyhow::Error; fn try_from(request: &ServiceRequest) -> Result { + if request.body.len() > LARGEST_DATA_BYTES { + anyhow::bail!("too large request: {} bytes", request.body.len()) + } Ok(bincode::deserialize::(&request.body)?) } } #[derive(Serialize, Deserialize, Debug)] pub enum MPResponse { + Broadcast, PointById(PointByIdResponse), Signature(SignatureResponse), } @@ -62,6 +73,9 @@ impl TryFrom<&Response> for MPResponse { type Error = anyhow::Error; fn try_from(response: &Response) -> Result { + if response.body.len() > LARGEST_DATA_BYTES { + anyhow::bail!("too large response: {} bytes", response.body.len()) + } match bincode::deserialize::(&response.body) { Ok(response) => Ok(response), Err(e) => Err(anyhow!("failed to deserialize: {e:?}")), @@ -73,10 +87,9 @@ impl TryFrom for PointByIdResponse { type Error = anyhow::Error; fn try_from(response: MPResponse) -> Result { - if let MPResponse::PointById(response) = response { - Ok(response) - } else { - Err(anyhow!("wrapper mismatch, expected PointById")) + match response { + MPResponse::PointById(response) => Ok(response), + _ => Err(anyhow!("wrapper mismatch, expected PointById")), } } } @@ -85,10 +98,20 @@ impl TryFrom for SignatureResponse { type Error = anyhow::Error; fn try_from(response: MPResponse) -> Result { - if let MPResponse::Signature(response) = response { - Ok(response) - } else { - Err(anyhow!("wrapper mismatch, expected Signature")) + match response { + MPResponse::Signature(response) => Ok(response), + _ => Err(anyhow!("wrapper mismatch, expected Signature")), + } + } +} + +impl TryFrom for BroadcastResponse { + type Error = anyhow::Error; + + fn try_from(response: MPResponse) -> Result { + match response { + MPResponse::Broadcast => Ok(BroadcastResponse), + _ => Err(anyhow!("wrapper mismatch, expected Broadcast")), } } } diff --git a/consensus/src/intercom/core/responder.rs b/consensus/src/intercom/core/responder.rs index 8d5bced79..4477085f9 100644 --- a/consensus/src/intercom/core/responder.rs +++ b/consensus/src/intercom/core/responder.rs @@ -8,8 +8,7 @@ use crate::effects::{AltFormat, CurrentRoundContext, Effects}; use crate::intercom::broadcast::Signer; use crate::intercom::core::dto::{MPQuery, MPResponse}; use crate::intercom::dto::{PointByIdResponse, SignatureResponse}; -use crate::intercom::{BroadcastFilter, Uploader}; -use crate::models::Point; +use crate::intercom::{BroadcastFilter, Downloader, Uploader}; #[derive(Clone, Default)] pub struct Responder(Arc>); @@ -18,6 +17,7 @@ struct ResponderInner { // state and storage components go here broadcast_filter: BroadcastFilter, top_dag_round: DagRound, + downloader: Downloader, effects: Effects, } @@ -26,12 +26,15 @@ impl Responder { &self, broadcast_filter: &BroadcastFilter, top_dag_round: &DagRound, + downloader: &Downloader, round_effects: &Effects, ) { + broadcast_filter.advance_round(top_dag_round, downloader, round_effects); self.0.store(Some(Arc::new(ResponderInner { broadcast_filter: broadcast_filter.clone(), - effects: round_effects.clone(), top_dag_round: top_dag_round.clone(), + downloader: downloader.clone(), + effects: round_effects.clone(), }))); } } @@ -48,8 +51,7 @@ impl Service for Responder { } #[inline] - fn on_message(&self, req: ServiceRequest) -> Self::OnMessageFuture { - self.handle_broadcast(&req); + fn on_message(&self, _req: ServiceRequest) -> Self::OnMessageFuture { futures_util::future::ready(()) } @@ -73,6 +75,19 @@ impl Responder { .ok()?; // malformed request is a reason to ignore it let inner = self.0.load_full(); let response = match body { + MPQuery::Broadcast(point) => { + match inner { + None => {} // do nothing: sender has retry loop via signature request + Some(inner) => inner.broadcast_filter.add( + &req.metadata.peer_id, + &Arc::new(point), + &inner.top_dag_round, + &inner.downloader, + &inner.effects, + ), + }; + MPResponse::Broadcast + } MPQuery::PointById(point_id) => MPResponse::PointById(match inner { None => PointByIdResponse(None), Some(inner) => { @@ -91,30 +106,5 @@ impl Responder { }; Some(Response::try_from(&response).expect("should serialize own response")) } - - fn handle_broadcast(&self, req: &ServiceRequest) { - let Some(inner) = self.0.load_full() else { - return; - }; - match bincode::deserialize::(&req.body) { - Ok(point) => { - let point = Arc::new(point); - inner.broadcast_filter.add( - &req.metadata.peer_id, - &point, - inner.top_dag_round.round(), - &inner.effects, - ); - } - Err(e) => { - // malformed request is a reason to ignore it - tracing::error!( - parent: inner.effects.span(), - "cannot parse broadcast from {:?}: {e:?}", - req.metadata.peer_id - ); - } - }; - } } // ResponderContext is meaningless without metrics diff --git a/consensus/src/intercom/dependency/downloader.rs b/consensus/src/intercom/dependency/downloader.rs index 69ae9cb0a..7461366e4 100644 --- a/consensus/src/intercom/dependency/downloader.rs +++ b/consensus/src/intercom/dependency/downloader.rs @@ -1,17 +1,16 @@ -use std::iter; +use std::collections::hash_map::Entry; use std::sync::Arc; use futures_util::future::BoxFuture; use futures_util::stream::FuturesUnordered; use futures_util::{FutureExt, StreamExt}; -use rand::prelude::{IteratorRandom, SmallRng}; -use rand::SeedableRng; +use rand::{thread_rng, RngCore}; use tokio::sync::broadcast::error::RecvError; -use tokio::sync::{broadcast, watch}; +use tokio::sync::{broadcast, mpsc}; use tycho_network::PeerId; -use tycho_util::{FastHashMap, FastHashSet}; +use tycho_util::FastHashMap; -use crate::dag::{Verifier, WeakDagRound}; +use crate::dag::{DagRound, Verifier, WeakDagRound}; use crate::dyn_event; use crate::effects::{AltFormat, Effects, EffectsContext, ValidateContext}; use crate::engine::MempoolConfig; @@ -31,6 +30,18 @@ struct DownloaderInner { peer_schedule: PeerSchedule, } +#[derive(Debug)] +struct PeerStatus { + state: PeerState, + failed_attempts: usize, + /// `true` for peers that depend on current point, i.e. included it directly; + /// requests are made without waiting for next attempt; + /// entries are never deleted, because they may be not resolved at the moment of insertion + is_depender: bool, + /// has uncompleted request just now + is_in_flight: bool, +} + impl Downloader { pub fn new(dispatcher: &Dispatcher, peer_schedule: &PeerSchedule) -> Self { Self { @@ -44,66 +55,74 @@ impl Downloader { pub async fn run( self, point_id: PointId, - point_round: WeakDagRound, - // TODO it would be great to increase the number of dependants in-flight, - // but then the DAG needs to store some sort of updatable state machine - // instead of opaque Shared> - dependant: PeerId, + // Download task holds weak reference to containing round and does not prevent its drop, + // while passes weak ref to validate; so Verifier is able to break recursive validation + // (trust consensus on `DAG_DEPTH` at least) and does not require too deep points + // to be checked against their dependencies (if dag round is removed from DAG). + // The task will be dropped in case DAG round is dropped and no validation waits this point. + // Do not pass `WeakDagRound` here as it would be incorrect to return `DagPoint::NotExists` + // if we need to download at a very deep round - let the start of this task hold strong ref. + point_dag_round_strong: DagRound, + dependers: mpsc::UnboundedReceiver, parent_effects: Effects, ) -> DagPoint { let effects = Effects::::new(&parent_effects, &point_id); let span_guard = effects.span().enter(); let peer_schedule = &self.inner.peer_schedule; - let Some(point_round_temp) = point_round.get() else { - return DagPoint::NotExists(Arc::new(point_id)); - }; assert_eq!( point_id.location.round, - point_round_temp.round(), + point_dag_round_strong.round(), "point and DAG round mismatch" ); - // request point from its signers (any dependant is among them as point is already verified) - let all_peers = peer_schedule - .peers_for(point_round_temp.round().next()) + // request point from its signers (any depender is among them as point is already verified) + let mut undone_peers = peer_schedule + .peers_for(point_dag_round_strong.round().next()) .iter() - .map(|(peer_id, state)| (*peer_id, *state)) - .collect::>(); - let Ok(node_count) = NodeCount::try_from(all_peers.len()) else { - return DagPoint::NotExists(Arc::new(point_id)); - }; + .map(|(peer_id, state)| { + (*peer_id, PeerStatus { + state: *state, + failed_attempts: 0, + is_depender: peer_id == point_id.location.author, + is_in_flight: false, + }) + }) + .collect::>(); + let node_count = NodeCount::try_from(undone_peers.len()) + .expect("validator set is unknown, must keep prev epoch's set for DAG_DEPTH rounds"); // query author no matter if it is in the next round, but that can't affect 3F+1 - let completed = match !all_peers.contains_key(&point_id.location.author) - && peer_schedule.is_resolved(&point_id.location.author) - { - true => -1, - false => 0, - }; - if all_peers.is_empty() { - return DagPoint::NotExists(Arc::new(point_id)); + let done_peers = match undone_peers.entry(point_id.location.author) { + Entry::Occupied(_) => 0, + Entry::Vacant(vacant) => { + vacant.insert(PeerStatus { + state: peer_schedule + .peer_state(&point_id.location.author) + .unwrap_or(PeerState::Unknown), + failed_attempts: 0, + is_depender: true, + is_in_flight: false, + }); + -1 + } }; - let mandatory = iter::once(dependant) - .chain(iter::once(point_id.location.author)) - .collect(); - let (has_resolved_tx, has_resolved_rx) = watch::channel(false); let updates = peer_schedule.updates(); - // do not leak strong ref across unlimited await - drop(point_round_temp); + let point_dag_round = point_dag_round_strong.downgrade(); + // do not leak span and strong round ref across await + drop(point_dag_round_strong); drop(span_guard); DownloadTask { + parent: self, effects, - weak_dag_round: point_round, + point_dag_round, node_count, request: Dispatcher::point_by_id_request(&point_id), point_id, + undone_peers, + done_peers, + downloading: FuturesUnordered::new(), + dependers, updates, - has_resolved_tx, - has_resolved_rx, - in_flight: FuturesUnordered::new(), - completed, - mandatory, - all_peers, - parent: self, attempt: 0, + skip_next_attempt: false, } .run() .await @@ -114,81 +133,121 @@ struct DownloadTask { parent: Downloader, effects: Effects, - weak_dag_round: WeakDagRound, + point_dag_round: WeakDagRound, node_count: NodeCount, request: tycho_network::Request, point_id: PointId, - all_peers: FastHashMap, - mandatory: FastHashSet, + undone_peers: FastHashMap, + done_peers: i16, + downloading: FuturesUnordered)>>, + + /// populated by waiting validation tasks, source of [`mandatory`] set + dependers: mpsc::UnboundedReceiver, updates: broadcast::Receiver<(PeerId, PeerState)>, - has_resolved_tx: watch::Sender, - has_resolved_rx: watch::Receiver, - in_flight: FuturesUnordered)>>, - completed: i16, + attempt: u8, + /// skip time-driven attempt if an attempt was init by empty task queue + skip_next_attempt: bool, } impl DownloadTask { // point's author is a top priority; fallback priority is (any) dependent point's author // recursively: every dependency is expected to be signed by 2/3+1 - pub async fn run(mut self) -> DagPoint { - self.download_mandatory(); - self.download(); - let mut interval = tokio::time::interval(MempoolConfig::DOWNLOAD_SPAWN_INTERVAL); - loop { + pub async fn run(&mut self) -> DagPoint { + // always ask the author + let author = self.point_id.location.author; + self.add_depender(&author); + self.download_random(true); + let mut interval = tokio::time::interval(MempoolConfig::DOWNLOAD_INTERVAL); + let dag_point = loop { tokio::select! { - Some((peer_id, resolved)) = self.in_flight.next() => - match self.match_resolved(peer_id, resolved).await { + Some((peer_id, downloaded)) = self.downloading.next() => + // de-schedule current task if point is verified and wait for validation + match self.match_downloaded(peer_id, downloaded).await { Some(dag_point) => break dag_point, None => continue }, - _ = interval.tick() => self.download(), + Some(depender) = self.dependers.recv() => self.add_depender(&depender), + _ = interval.tick() => self.download_random(false), update = self.updates.recv() => self.match_peer_updates(update), } - } + }; + // clean the channel, it will stay in `DagPointFuture` that owns current task + self.dependers.close(); + dag_point } - fn download_mandatory(&mut self) { - let mandatory = self - .mandatory - .iter() - .filter(|p| { - self.all_peers - .get(p) - .map_or(false, |&s| s == PeerState::Resolved) - }) - .cloned() - .collect::>(); - for peer_id in mandatory { - self.all_peers.remove_entry(&peer_id); - self.download_one(&peer_id); + fn add_depender(&mut self, peer_id: &PeerId) { + let is_suitable = match self.undone_peers.get_mut(peer_id) { + Some(state) if !state.is_depender => { + state.is_depender = true; + !state.is_in_flight + && state.state == PeerState::Resolved + // do not re-download immediately if already requested + && state.failed_attempts == 0 + } + _ => false, // either already marked or requested and removed, no panic + }; + if is_suitable { + // request immediately just once + self.download_one(peer_id); } } - fn download(&mut self) { - self.attempt += 1; // FIXME panics on 100% load + fn download_random(&mut self, force: bool) { + if self.skip_next_attempt { + // reset `skip_attempt` flag; do nothing, if not forced + self.skip_next_attempt = false; + if !force { + return; + } + } + self.attempt = self.attempt.wrapping_add(1); let count = (MempoolConfig::DOWNLOAD_PEERS as usize) .saturating_mul(self.attempt as usize) - .min(self.all_peers.len()); + .min(self.undone_peers.len()); - for peer_id in self - .all_peers + let mut filtered = self + .undone_peers .iter() - .filter(|(_, &p)| p == PeerState::Resolved) - .choose_multiple(&mut SmallRng::from_entropy(), count) - .into_iter() - .map(|(peer_id, _)| *peer_id) - .collect::>() - { - self.all_peers.remove_entry(&peer_id); - self.download_one(&peer_id); + .filter(|(_, p)| p.state == PeerState::Resolved && !p.is_in_flight) + .map(|(peer_id, status)| { + ( + *peer_id, + ( + // try every peer, until all are tried the same amount of times + status.failed_attempts, + // try mandatory peers before others each loop + u8::from(!status.is_depender), + // randomise within group + thread_rng().next_u32(), + ), + ) + }) + .collect::>(); + filtered.sort_unstable_by_key(|kv| kv.1); + + for (peer_id, _) in filtered.iter().take(count) { + self.download_one(peer_id); } } fn download_one(&mut self, peer_id: &PeerId) { - self.in_flight.push( + let status = self + .undone_peers + .get_mut(peer_id) + .unwrap_or_else(|| panic!("Coding error: peer not in map {}", peer_id.alt())); + assert!( + !status.is_in_flight, + "already downloading from peer {} status {:?}", + peer_id.alt(), + status + ); + status.is_in_flight = true; + + self.downloading.push( self.parent .inner .dispatcher @@ -197,127 +256,160 @@ impl DownloadTask { ); } - async fn match_resolved( + async fn match_downloaded( &mut self, peer_id: PeerId, resolved: anyhow::Result, ) -> Option { match resolved { Err(network_err) => { - tracing::error!( + let status = self + .undone_peers + .get_mut(&peer_id) + .unwrap_or_else(|| panic!("Coding error: peer not in map {}", peer_id.alt())); + status.is_in_flight = false; + status.failed_attempts += 1; + tracing::warn!( parent: self.effects.span(), - peer_id = display(peer_id.alt()), + peer = display(peer_id.alt()), error = display(network_err), "network error", ); } Ok(PointByIdResponse(None)) => { - if self.mandatory.remove(&peer_id) { - // it's a ban in case permanent storage is used, - // the other way - peer can could have advanced on full DAG_DEPTH already - tracing::error!( - parent: self.effects.span(), - peer_id = display(peer_id.alt()), - "must have returned", - ); - } else { - tracing::debug!( - parent: self.effects.span(), - peer_id = display(peer_id.alt()), - "didn't return", - ); + self.done_peers += 1; + match self.undone_peers.remove(&peer_id) { + Some(state) if state.is_depender => { + // if points are persisted in storage - it's a ban; + // else - peer evicted this point from its cache, as the point + // is at least DAG_DEPTH rounds older than current consensus round + tracing::warn!( + parent: self.effects.span(), + peer = display(peer_id.alt()), + "must have returned", + ); + } + Some(_) => { + tracing::debug!( + parent: self.effects.span(), + peer = display(peer_id.alt()), + "didn't return", + ); + } + None => { + let _guard = self.effects.span().enter(); + panic!("already removed peer {}", peer_id.alt()) + } } } + Ok(PointByIdResponse(Some(point))) if point.id() != self.point_id => { + self.done_peers += 1; + self.undone_peers.remove(&peer_id); + // it's a ban + tracing::error!( + parent: self.effects.span(), + peer_id = display(peer_id.alt()), + author = display(point.body().location.author.alt()), + round = point.body().location.round.0, + digest = display(point.digest().alt()), + "returned wrong point", + ); + } Ok(PointByIdResponse(Some(point))) => { - if point.id() != self.point_id { - // it's a ban - tracing::error!( - parent: self.effects.span(), - peer_id = display(peer_id.alt()), - author = display(self.point_id.location.author.alt()), - round = self.point_id.location.round.0, - digest = display(self.point_id.digest.alt()), - "returned wrong point", - ); - } - let validated = match Verifier::verify(&point, &self.parent.inner.peer_schedule) { + self.undone_peers.remove(&peer_id); + match Verifier::verify(&point, &self.parent.inner.peer_schedule) { Ok(()) => { - Verifier::validate( + tracing::trace!( + parent: self.effects.span(), + peer = display(peer_id.alt()), + "downloaded, now validating", + ); + let dag_point = Verifier::validate( point, - self.weak_dag_round.clone(), + self.point_dag_round.clone(), self.parent.clone(), self.effects.span().clone(), ) - .await + // this is the only `await` in the task, that resolves the download + .await; + let level = if dag_point.trusted().is_some() { + tracing::Level::DEBUG + } else if dag_point.valid().is_some() { + tracing::Level::WARN + } else { + tracing::Level::ERROR + }; + dyn_event!( + parent: self.effects.span(), + level, + result = display(dag_point.alt()), + "validated", + ); + return Some(dag_point); } - Err(dag_point) => dag_point, - }; - let level = if validated.trusted().is_some() { - tracing::Level::DEBUG - } else if validated.valid().is_some() { - tracing::Level::WARN - } else { - tracing::Level::ERROR - }; - dyn_event!( - parent: self.effects.span(), - level, - result = display(validated.alt()), - "downloaded", - ); - match validated { - DagPoint::NotExists(_) => { - // author not reliable, it's a ban; continue + Err(dag_point) => { + // reliable peer won't return unverifiable point + self.done_peers += 1; + assert!( + dag_point.valid().is_none(), + "Coding error: verify() cannot result into a valid point" + ); + tracing::error!( + parent: self.effects.span(), + result = display(dag_point.alt()), + peer = display(peer_id.alt()), + "downloaded", + ); } - dag_point => return Some(dag_point), - } + }; } }; - // the point does not exist when only 1F left unqueried, - // assuming author and dependant are queried or unavailable - self.completed += 1; - if self.completed >= self.node_count.majority() as i16 { - return Some(DagPoint::NotExists(Arc::new(self.point_id.clone()))); + self.maybe_not_downloaded() + } + + fn maybe_not_downloaded(&mut self) -> Option { + if self.done_peers >= self.node_count.majority() as i16 { + // the only normal case to resolve into `NotExists` + tracing::warn!( + parent: self.effects.span(), + "not downloaded from majority", + ); + Some(DagPoint::NotExists(Arc::new(self.point_id.clone()))) + } else { + if self.downloading.is_empty() { + self.download_random(true); + self.skip_next_attempt = true; + } + None } - if self.in_flight.is_empty() { - self.has_resolved_tx.send(false).ok(); - self.download(); - }; - if self.in_flight.is_empty() { - // mempool inclusion guarantees must be satisfied if less than 2F+1 nodes are online; - // so we should stall, waiting for peers to connect - if let Err(e) = self.has_resolved_rx.wait_for(|is| *is).await { - let _span = self.effects.span().enter(); - panic!("Downloader waiting for new resolved peer {e}"); - }; - self.download(); - }; - None } fn match_peer_updates(&mut self, result: Result<(PeerId, PeerState), RecvError>) { match result { Ok((peer_id, new)) => { - let mut is_resolved = false; - self.all_peers.entry(peer_id).and_modify(|old| { - if *old == PeerState::Unknown && new == PeerState::Resolved { - is_resolved = true; - } - *old = new; + let mut is_suitable = false; + self.undone_peers.entry(peer_id).and_modify(|status| { + is_suitable = !status.is_in_flight + && status.is_depender + && status.failed_attempts == 0 + && status.state == PeerState::Unknown + && new == PeerState::Resolved; + status.state = new; }); - if is_resolved { - self.has_resolved_tx.send(true).ok(); + if is_suitable { + self.download_one(&peer_id); } } Err(err @ RecvError::Lagged(_)) => { tracing::error!( parent: self.effects.span(), - "Downloader peer updates {err}" + error = display(err), + "peer updates" ); } Err(err @ RecvError::Closed) => { let _span = self.effects.span().enter(); - panic!("Downloader peer updates {err}") + panic!("peer updates {err}") } } } diff --git a/consensus/src/intercom/dependency/uploader.rs b/consensus/src/intercom/dependency/uploader.rs index efc5da2ae..17a47db4f 100644 --- a/consensus/src/intercom/dependency/uploader.rs +++ b/consensus/src/intercom/dependency/uploader.rs @@ -28,21 +28,26 @@ impl Uploader { .flatten() }); let task_found = found.is_some(); - let ready = found - .and_then(|shared| shared.now_or_never()) - .map(|(dag_point, _)| dag_point); + let ready = found.and_then(|shared| shared.now_or_never()); let level = if let Some(dag_point) = ready.as_ref() { if dag_point.trusted().is_some() { + // just fine tracing::Level::TRACE } else { + // either some misbehaving author produced truly invalid point, + // or we trail some point marked as `Invalid` through our history + // and don't trust newer consensus signatures tracing::Level::ERROR } + } else if point_id.location.round >= top_dag_round.round().prev() { + // peer is from future round, we lag behind consensus and can see it from other logs + tracing::Level::TRACE } else if dag_round.is_some() { + // we have the round, but not the point - it is very weakly included into global DAG tracing::Level::DEBUG - } else if point_id.location.round > top_dag_round.round() { - tracing::Level::TRACE } else { - tracing::Level::WARN + // dag round not found - we detected that peer lags more than `DAG_DEPTH` from us + tracing::Level::DEBUG }; dyn_event!( parent: effects.span(), diff --git a/consensus/src/intercom/dto.rs b/consensus/src/intercom/dto.rs index dd8973a60..5cfbf2ded 100644 --- a/consensus/src/intercom/dto.rs +++ b/consensus/src/intercom/dto.rs @@ -1,33 +1,25 @@ use std::fmt::{Display, Formatter}; -use std::sync::Arc; -use serde::{Deserialize, Deserializer, Serialize, Serializer}; +use serde::{Deserialize, Serialize}; use crate::effects::{AltFmt, AltFormat}; use crate::models::{Point, Signature}; -#[derive(Debug)] -pub struct PointByIdResponse(pub Option>); -impl Serialize for PointByIdResponse { - fn serialize(&self, serializer: S) -> Result - where - S: Serializer, - { - self.0.as_deref().serialize(serializer) - } -} +#[derive(Debug, Serialize, Deserialize)] +pub struct PointByIdResponse(pub Option); -impl<'de> Deserialize<'de> for PointByIdResponse { - fn deserialize(deserializer: D) -> Result - where - D: Deserializer<'de>, - { - let opt = Option::::deserialize(deserializer)?; - Ok(PointByIdResponse(opt.map(Arc::new))) - } +/// Denotes that broadcasts should be done via network query, not send message. +/// Because initiator must not duplicate its broadcasts, thus should wait for receiver to respond. +pub struct BroadcastResponse; + +#[derive(Serialize, Deserialize, Debug)] +pub enum SignatureRejectedReason { + TooOldRound, + NoDagRound, + CannotSign, } -#[derive(Serialize, Deserialize, PartialEq, Debug)] +#[derive(Serialize, Deserialize, Debug)] pub enum SignatureResponse { Signature(Signature), /// peer dropped its state or just reached point's round @@ -42,18 +34,18 @@ pub enum SignatureResponse { /// * invalid dependency /// * signer is more than 1 round in front of us /// * signer's clock are too far in the future (probably consensus stalled for long) - Rejected, + Rejected(SignatureRejectedReason), } impl AltFormat for SignatureResponse {} impl Display for AltFmt<'_, SignatureResponse> { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - f.write_str(match AltFormat::unpack(self) { - SignatureResponse::Signature(_) => "Signature", - SignatureResponse::NoPoint => "NoPoint", - SignatureResponse::TryLater => "TryLater", - SignatureResponse::Rejected => "Rejected", - }) + match AltFormat::unpack(self) { + SignatureResponse::Signature(_) => f.write_str("Signature"), + SignatureResponse::NoPoint => f.write_str("NoPoint"), + SignatureResponse::TryLater => f.write_str("TryLater"), + SignatureResponse::Rejected(reason) => f.debug_tuple("Rejected").field(reason).finish(), + } } } diff --git a/consensus/src/intercom/peer_schedule/peer_schedule.rs b/consensus/src/intercom/peer_schedule/peer_schedule.rs index b33866684..e3569d4ae 100644 --- a/consensus/src/intercom/peer_schedule/peer_schedule.rs +++ b/consensus/src/intercom/peer_schedule/peer_schedule.rs @@ -25,8 +25,7 @@ use crate::models::Round; pub struct PeerSchedule { // FIXME remove mutex ( parking_lot ! ) // and just restart updater when new peers or epoch start are known; - // use copy-on-write to replace Inner as a whole; - // maybe store schedule-per-round inside DAG round, but how to deal with download tasks then? + // use ArcSwap to replace whole content; find and remove Arc inner: Arc>, // Connection to self is always "Added" // Updates are Resolved or Removed, sent single time @@ -79,37 +78,52 @@ impl PeerSchedule { /// Consensus progress is not guaranteed without witness (because of evidence requirement), /// but we don't care if the consensus of an ending epoch stalls at its last round. pub fn local_keys(&self, round: Round) -> Option> { - if self.peers_for(round).contains_key(&self.local_id()) { + let inner = self.inner.lock(); + if inner + .peers_for_index_plus_one(inner.index_plus_one(round)) + .contains_key(&self.local_id()) + { Some(self.local_keys.clone()) } else { None } } + /// local peer id is always kept as not resolved, so always excluded from result pub fn all_resolved(&self) -> FastHashSet { let inner = self.inner.lock(); - inner.all_resolved(&self.local_id()) + inner.all_resolved() } - pub fn is_resolved(&self, peer_id: &PeerId) -> bool { + /// local peer id is always kept as not resolved + pub fn peer_state(&self, peer_id: &PeerId) -> Option { let inner = self.inner.lock(); - inner.is_resolved(peer_id) + inner.peer_state(peer_id) } + /// local peer id is always kept as not resolved pub fn peers_for(&self, round: Round) -> Arc> { let inner = self.inner.lock(); - inner.peers_for_index_plus_one(inner.index_plus_one(round)) + inner + .peers_for_index_plus_one(inner.index_plus_one(round)) + .clone() } + /// local peer id is always kept as not resolved pub fn peers_for_array( &self, rounds: [Round; N], ) -> [Arc>; N] { let inner = self.inner.lock(); - array::from_fn(|i| inner.peers_for_index_plus_one(inner.index_plus_one(rounds[i]))) + array::from_fn(|i| { + inner + .peers_for_index_plus_one(inner.index_plus_one(rounds[i])) + .clone() + }) } - /// does not return empty maps + /// does not return empty maps; + /// local peer id is always shown as not resolved, if it is found pub fn peers_for_range(&self, rounds: &Range) -> Vec>> { if rounds.end <= rounds.start { return vec![]; @@ -123,6 +137,7 @@ impl PeerSchedule { (first..=last) .map(|i| inner.peers_for_index_plus_one(i)) .filter(|m| !m.is_empty()) + .cloned() .collect() } @@ -189,28 +204,27 @@ impl PeerSchedule { next.extend(peers); } - /// Returns [true] if update was successfully applied - pub(super) fn set_resolved(&self, peer_id: &PeerId, resolved: bool) -> bool { + /// Returns [true] if update was successfully applied. + /// Always keeps local id as [`PeerState::Unknown`] + pub(super) fn set_state(&self, peer_id: &PeerId, state: PeerState) -> bool { let mut is_applied = false; - let new_state = if resolved && peer_id != self.local_id() { - PeerState::Resolved - } else { - PeerState::Unknown + if peer_id == self.local_id() { + return false; }; { let mut inner = self.inner.lock(); for i in 0..inner.peers_resolved.len() { - let Some(b) = Arc::make_mut(&mut inner.peers_resolved[i]).get_mut(peer_id) else { + let Some(old) = Arc::make_mut(&mut inner.peers_resolved[i]).get_mut(peer_id) else { continue; }; - if *b != new_state { - *b = new_state; + if *old != state { + *old = state; is_applied = true; } } } if is_applied { - _ = self.updates.send((*peer_id, new_state)); + _ = self.updates.send((*peer_id, state)); } is_applied } @@ -248,30 +262,31 @@ impl PeerScheduleInner { } } - fn peers_for_index_plus_one(&self, index: u8) -> Arc> { + fn peers_for_index_plus_one(&self, index: u8) -> &'_ Arc> { match index { - 0 => self.empty.clone(), - x if x <= 3 => self.peers_resolved[x as usize - 1].clone(), + 0 => &self.empty, + x if x <= 3 => &self.peers_resolved[x as usize - 1], _ => unreachable!(), } } - fn all_resolved(&self, local_id: &PeerId) -> FastHashSet { + fn all_resolved(&self) -> FastHashSet { self.peers_resolved[0] .iter() .chain(self.peers_resolved[1].iter()) .chain(self.peers_resolved[2].iter()) - .filter(|(peer_id, state)| *state == &PeerState::Resolved && peer_id != local_id) - .map(|(peer_id, _)| *peer_id) + .filter(|(_, state)| *state == &PeerState::Resolved) + .map(|(peer_id, _)| peer_id) + .copied() .collect() } - fn is_resolved(&self, peer_id: &PeerId) -> bool { + fn peer_state(&self, peer_id: &PeerId) -> Option { // used only in Downloader, such order fits its needs self.peers_resolved[0] .get(peer_id) .or_else(|| self.peers_resolved[2].get(peer_id)) .or_else(|| self.peers_resolved[1].get(peer_id)) - .map_or(false, |state| *state == PeerState::Resolved) + .copied() } } diff --git a/consensus/src/intercom/peer_schedule/peer_schedule_updater.rs b/consensus/src/intercom/peer_schedule/peer_schedule_updater.rs index c42d5fbde..f3ab9d1a9 100644 --- a/consensus/src/intercom/peer_schedule/peer_schedule_updater.rs +++ b/consensus/src/intercom/peer_schedule/peer_schedule_updater.rs @@ -4,8 +4,7 @@ use std::sync::Arc; use futures_util::stream::FuturesUnordered; use futures_util::StreamExt; use parking_lot::Mutex; -use rand::prelude::SmallRng; -use rand::SeedableRng; +use rand::thread_rng; use tokio::sync::broadcast::error::RecvError; use tokio::task::AbortHandle; use tycho_network::{ @@ -13,6 +12,8 @@ use tycho_network::{ PrivateOverlayEntriesReadGuard, PrivateOverlayEntriesWriteGuard, }; +use crate::effects::{AltFmt, AltFormat}; +use crate::intercom::dto::PeerState; use crate::intercom::PeerSchedule; #[derive(Clone)] @@ -33,7 +34,7 @@ impl PeerScheduleUpdater { pub async fn run(self) -> ! { tracing::info!("started peer schedule updater"); - self.respawn_resolve_task(self.resolved_waiters(self.overlay.read_entries())); + self.restart_resolve_task(self.resolved_waiters(self.overlay.read_entries())); self.listen().await } @@ -43,7 +44,7 @@ impl PeerScheduleUpdater { for peer_id in peers { entries.insert(peer_id); } - self.respawn_resolve_task(self.resolved_waiters(entries.downgrade())); + self.restart_resolve_task(self.resolved_waiters(entries.downgrade())); } self.peer_schedule.set_next_peers(peers, &self.overlay); } @@ -57,7 +58,7 @@ impl PeerScheduleUpdater { // Note: set_next_peers() and respawn_resolve_task() will not deadlock // although peer_schedule.inner is locked in two opposite orders // because only read read lock on overlay entries is taken - for entry in entries.choose_multiple(&mut SmallRng::from_entropy(), entries.len()) { + for entry in entries.choose_multiple(&mut thread_rng(), entries.len()) { // skip updates on self if !(entry.peer_id == local_id || entry.resolver_handle.is_resolved()) { let handle = entry.resolver_handle.clone(); @@ -67,21 +68,21 @@ impl PeerScheduleUpdater { fut } - fn respawn_resolve_task( + fn restart_resolve_task( &self, mut resolved_waiters: FuturesUnordered< impl Future + Sized + Send + 'static, >, ) { - let local_id = self.peer_schedule.local_id(); - tracing::info!("{local_id:.4?} respawn_resolve_task"); + tracing::info!("restart resolve task"); let new_abort_handle = if resolved_waiters.is_empty() { None } else { let peer_schedule = self.peer_schedule.clone(); let join = tokio::spawn(async move { while let Some(known_peer_handle) = resolved_waiters.next().await { - _ = peer_schedule.set_resolved(&known_peer_handle.peer_info().id, true); + _ = peer_schedule + .set_state(&known_peer_handle.peer_info().id, PeerState::Resolved); } }); Some(join.abort_handle()) @@ -95,34 +96,55 @@ impl PeerScheduleUpdater { async fn listen(self) -> ! { let local_id = self.peer_schedule.local_id(); - tracing::info!("{local_id:.4?} listen peer updates"); + tracing::info!("listen peer updates"); let mut rx = self.overlay.read_entries().subscribe(); loop { match rx.recv().await { - Ok(ref event @ PrivateOverlayEntriesEvent::Removed(node)) if node != local_id => { - tracing::info!("{local_id:.4?} got {event:?}"); - if self.peer_schedule.set_resolved(&node, false) { + Ok(ref event @ PrivateOverlayEntriesEvent::Removed(peer)) if peer != local_id => { + let restart = self.peer_schedule.set_state(&peer, PeerState::Unknown); + if restart { // respawn resolve task with fewer peers to await - self.respawn_resolve_task( + self.restart_resolve_task( self.resolved_waiters(self.overlay.read_entries()), ); - } else { - tracing::info!("{local_id:.4?} Skipped {event:?}"); } + tracing::info!( + event = display(event.alt()), + peer = display(peer.alt()), + resolve_restarted = restart, + "peer schedule update" + ); } Err(RecvError::Closed) => { panic!("peer info updates channel closed, cannot maintain node connectivity") } - Err(RecvError::Lagged(qnt)) => { - tracing::warn!( - "Skipped {qnt} peer info updates, node connectivity may suffer. \ + Err(RecvError::Lagged(amount)) => { + tracing::error!( + amount = amount, + "Lagged peer info updates, node connectivity may suffer. \ Consider increasing channel capacity." ); } - Ok(a) => { - tracing::warn!("{local_id:.4?} peer schedule updater missed {a:?}"); + Ok( + ref event @ (PrivateOverlayEntriesEvent::Added(peer_id) + | PrivateOverlayEntriesEvent::Removed(peer_id)), + ) => { + tracing::debug!( + event = display(event.alt()), + peer = display(peer_id.alt()), + "peer schedule update ignored" + ); } } } } } +impl AltFormat for PrivateOverlayEntriesEvent {} +impl std::fmt::Display for AltFmt<'_, PrivateOverlayEntriesEvent> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(match AltFormat::unpack(self) { + PrivateOverlayEntriesEvent::Added(_) => "Added", + PrivateOverlayEntriesEvent::Removed(_) => "Removed", + }) + } +} diff --git a/consensus/src/models/dag_point.rs b/consensus/src/models/dag_point.rs index e9c12ebc9..a0cb109bd 100644 --- a/consensus/src/models/dag_point.rs +++ b/consensus/src/models/dag_point.rs @@ -5,12 +5,12 @@ use crate::models::point::{Digest, Location, Point, PointId}; #[derive(Clone, Debug)] pub struct ValidPoint { - pub point: Arc, + pub point: Point, pub is_committed: Arc, } impl ValidPoint { - pub fn new(point: Arc) -> Self { + pub fn new(point: Point) -> Self { Self { point, is_committed: Arc::new(AtomicBool::new(false)), @@ -27,7 +27,8 @@ pub enum DagPoint { /// consensus will decide whether to sign its proof or not; we shall ban the author anyway Suspicious(ValidPoint), /// invalidates dependent point; needed to blame equivocation - Invalid(Arc), + Invalid(Point), + /// point hash or signature mismatch, not well-formed, download failed - i.e. unusable point; /// invalidates dependent point; blame author of dependent point NotExists(Arc), } @@ -75,9 +76,9 @@ impl DagPoint { pub fn location(&self) -> &'_ Location { #[allow(clippy::match_same_arms)] match self { - Self::Trusted(valid) => &valid.point.body.location, - Self::Suspicious(valid) => &valid.point.body.location, - Self::Invalid(point) => &point.body.location, + Self::Trusted(valid) => &valid.point.body().location, + Self::Suspicious(valid) => &valid.point.body().location, + Self::Invalid(point) => &point.body().location, Self::NotExists(id) => &id.location, } } @@ -85,9 +86,9 @@ impl DagPoint { pub fn digest(&self) -> &'_ Digest { #[allow(clippy::match_same_arms)] match self { - Self::Trusted(valid) => &valid.point.digest, - Self::Suspicious(valid) => &valid.point.digest, - Self::Invalid(point) => &point.digest, + Self::Trusted(valid) => valid.point.digest(), + Self::Suspicious(valid) => valid.point.digest(), + Self::Invalid(point) => point.digest(), Self::NotExists(id) => &id.digest, } } diff --git a/consensus/src/models/point.rs b/consensus/src/models/point.rs index 48b7072a2..4e410b6bb 100644 --- a/consensus/src/models/point.rs +++ b/consensus/src/models/point.rs @@ -5,7 +5,7 @@ use std::sync::Arc; use bytes::Bytes; use everscale_crypto::ed25519::KeyPair; -use serde::{Deserialize, Serialize}; +use serde::{Deserialize, Deserializer, Serialize, Serializer}; use sha2::{Digest as Sha2Digest, Sha256}; use tycho_network::PeerId; @@ -27,7 +27,7 @@ impl Display for Digest { impl Debug for Digest { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { f.write_str("Digest(")?; - std::fmt::Display::fmt(self, f)?; + Display::fmt(self, f)?; f.write_str(")") } } @@ -56,7 +56,7 @@ impl Display for Signature { impl Debug for Signature { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { f.write_str("Signature(")?; - std::fmt::Display::fmt(self, f)?; + Display::fmt(self, f)?; f.write_str(")") } } @@ -219,39 +219,122 @@ pub enum LinkField { Proof, } -// Todo: Arc => Point(Arc<...{...}>) -#[derive(Clone, Serialize, Deserialize, Debug)] -pub struct Point { - pub body: PointBody, - // hash of the point's body (includes author peer id) - pub digest: Digest, - // author's signature for the digest - pub signature: Signature, +#[derive(Clone)] +pub struct Point(Arc); + +impl Serialize for Point { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + self.0.as_ref().serialize(serializer) + } +} + +impl<'de> Deserialize<'de> for Point { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + Ok(Point(Arc::new(PointInner::deserialize(deserializer)?))) + } +} + +impl Debug for Point { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + f.debug_struct("Point") + .field("body", self.body()) + .field("digest", self.digest()) + .field("signature", self.signature()) + .finish() + } } impl Point { - pub fn new(local_keypair: &KeyPair, point_body: PointBody) -> Arc { + pub fn new(local_keypair: &KeyPair, point_body: PointBody) -> Self { assert_eq!( point_body.location.author, PeerId::from(local_keypair.public_key), "produced point author must match local key pair" ); let digest = Digest::new(&point_body); - Arc::new(Point { + Self(Arc::new(PointInner { body: point_body, signature: Signature::new(local_keypair, &digest), digest, - }) + })) + } + + pub fn body(&self) -> &'_ PointBody { + &self.0.body + } + + pub fn digest(&self) -> &'_ Digest { + &self.0.digest + } + + pub fn signature(&self) -> &'_ Signature { + &self.0.signature } pub fn id(&self) -> PointId { + self.0.id() + } + + pub fn prev_id(&self) -> Option { + self.0.prev_id() + } + + /// Failed integrity means the point may be created by someone else. + /// blame every dependent point author and the sender of this point, + /// do not use the author from point's body + pub fn is_integrity_ok(&self) -> bool { + self.0.is_integrity_ok() + } + + /// blame author and every dependent point's author + /// must be checked right after integrity, before any manipulations with the point + pub fn is_well_formed(&self) -> bool { + self.0.is_well_formed() + } + + pub fn anchor_link(&self, link_field: LinkField) -> &'_ Link { + self.0.anchor_link(link_field) + } + + pub fn anchor_round(&self, link_field: LinkField) -> Round { + self.0.anchor_round(link_field) + } + + /// the final destination of an anchor link + pub fn anchor_id(&self, link_field: LinkField) -> PointId { + self.0.anchor_id(link_field) + } + + /// next point in path from `&self` to the anchor + pub fn anchor_link_id(&self, link_field: LinkField) -> PointId { + self.0.anchor_link_id(link_field) + } +} + +#[derive(Serialize, Deserialize, Debug)] +struct PointInner { + body: PointBody, + // hash of the point's body (includes author peer id) + digest: Digest, + // author's signature for the digest + signature: Signature, +} + +impl PointInner { + fn id(&self) -> PointId { PointId { location: self.body.location, digest: self.digest.clone(), } } - pub fn prev_id(&self) -> Option { + fn prev_id(&self) -> Option { let digest = self.body.proof.as_ref().map(|p| &p.digest)?; Some(PointId { location: Location { @@ -262,18 +345,13 @@ impl Point { }) } - /// Failed integrity means the point may be created by someone else. - /// blame every dependent point author and the sender of this point, - /// do not use the author from point's body - pub fn is_integrity_ok(&self) -> bool { + fn is_integrity_ok(&self) -> bool { self.signature .verifies(&self.body.location.author, &self.digest) && self.digest == Digest::new(&self.body) } - /// blame author and every dependent point's author - /// must be checked right after integrity, before any manipulations with the point - pub fn is_well_formed(&self) -> bool { + fn is_well_formed(&self) -> bool { // any genesis is suitable, round number may be taken from configs let author = &self.body.location.author; let is_special_ok = match self.body.location.round { @@ -282,6 +360,7 @@ impl Point { && self.body.witness.is_empty() && self.body.payload.is_empty() && self.body.proof.is_none() + && self.body.time == self.body.anchor_time && self.body.anchor_proof == Link::ToSelf && self.body.anchor_trigger == Link::ToSelf } @@ -338,14 +417,14 @@ impl Point { } } - pub fn anchor_link(&self, link_field: LinkField) -> &'_ Link { + fn anchor_link(&self, link_field: LinkField) -> &'_ Link { match link_field { LinkField::Trigger => &self.body.anchor_trigger, LinkField::Proof => &self.body.anchor_proof, } } - pub fn anchor_round(&self, link_field: LinkField) -> Round { + fn anchor_round(&self, link_field: LinkField) -> Round { match self.anchor_link(link_field) { Link::ToSelf => self.body.location.round, Link::Direct(Through::Includes(_)) => self.body.location.round.prev(), @@ -354,45 +433,180 @@ impl Point { } } - /// the final destination of an anchor link - pub fn anchor_id(&self, link_field: LinkField) -> PointId { + fn anchor_id(&self, link_field: LinkField) -> PointId { match self.anchor_link(link_field) { Link::Indirect { to, .. } => to.clone(), _direct => self.anchor_link_id(link_field), } } - /// next point in path from `&self` to the anchor - pub fn anchor_link_id(&self, link_field: LinkField) -> PointId { - #[allow(clippy::match_same_arms)] - let (peer, is_in_includes) = match self.anchor_link(link_field) { + fn anchor_link_id(&self, link_field: LinkField) -> PointId { + let (digest, location) = match self.anchor_link(link_field) { Link::ToSelf => return self.id(), - Link::Direct(Through::Includes(peer)) => (peer, true), - Link::Direct(Through::Witness(peer)) => (peer, false), - Link::Indirect { + Link::Direct(Through::Includes(peer)) + | Link::Indirect { path: Through::Includes(peer), .. - } => (peer, true), - Link::Indirect { + } => (self.body.includes.get(peer), Location { + author: *peer, + round: self.body.location.round.prev(), + }), + Link::Direct(Through::Witness(peer)) + | Link::Indirect { path: Through::Witness(peer), .. - } => (peer, false), - }; - - let (map, round) = if is_in_includes { - (&self.body.includes, self.body.location.round.prev()) - } else { - (&self.body.witness, self.body.location.round.prev().prev()) + } => (self.body.witness.get(peer), Location { + author: *peer, + round: self.body.location.round.prev().prev(), + }), }; PointId { - location: Location { - round, - author: *peer, - }, - digest: map - .get(peer) + location, + digest: digest .expect("Coding error: usage of ill-formed point") .clone(), } } } + +#[cfg(test)] +mod tests { + use std::time::Instant; + + use everscale_crypto::ed25519::SecretKey; + use rand::{thread_rng, RngCore}; + use tycho_util::sync::rayon_run; + + use super::*; + + const PEERS: usize = 100; + const MSG_COUNT: usize = 1000; + const MSG_BYTES: usize = 2 * 1000; + + fn new_key_pair() -> KeyPair { + let mut secret_bytes: [u8; 32] = [0; 32]; + thread_rng().fill_bytes(&mut secret_bytes); + KeyPair::from(&SecretKey::from_bytes(secret_bytes)) + } + + fn point_body(key_pair: &KeyPair) -> PointBody { + let mut payload = Vec::with_capacity(MSG_COUNT); + for _ in 0..MSG_COUNT { + let mut data = vec![0; MSG_BYTES]; + thread_rng().fill_bytes(data.as_mut_slice()); + payload.push(Bytes::from(data)); + } + + let mut includes = BTreeMap::default(); + for _ in 0..PEERS { + let key_pair = new_key_pair(); + let peer_id = PeerId::from(key_pair.public_key); + let digest = Digest(*key_pair.secret_key.nonce()); + includes.insert(peer_id, digest); + } + + PointBody { + location: Location { + round: Round(thread_rng().next_u32()), + author: PeerId::from(key_pair.public_key), + }, + time: UnixTime::now(), + payload, + proof: None, + includes, + witness: Default::default(), + anchor_trigger: Link::ToSelf, + anchor_proof: Link::ToSelf, + anchor_time: UnixTime::now(), + } + } + fn sig_data() -> (Digest, Vec<(PeerId, Signature)>) { + let digest = Digest([12; 32]); + let mut data = Vec::with_capacity(PEERS); + for _ in 0..PEERS { + let key_pair = new_key_pair(); + let sig = Signature::new(&key_pair, &digest); + let peer_id = PeerId::from(key_pair.public_key); + data.push((peer_id, sig)); + } + (digest, data) + } + + #[test] + pub fn check_sig() { + let (digest, data) = sig_data(); + + let timer = Instant::now(); + for (peer_id, sig) in data.iter() { + assert!(sig.verifies(peer_id, &digest), "invalid signature"); + } + let elapsed = timer.elapsed(); + println!( + "check {PEERS} sigs took {}", + humantime::format_duration(elapsed) + ); + } + + #[tokio::test] + pub async fn check_sig_on_rayon() { + let (digest, data) = sig_data(); + rayon_run(|| ()).await; + + let timer = Instant::now(); + let fut = rayon_run(move || { + for (peer_id, sig) in data.iter() { + assert!(sig.verifies(peer_id, &digest), "invalid signature"); + } + }); + let elapsed_start = timer.elapsed(); + fut.await; + let elapsed_run = timer.elapsed(); + + println!( + "init rayon took {}", + humantime::format_duration(elapsed_start) + ); + println!( + "check {PEERS} with rayon took {}", + humantime::format_duration(elapsed_run) + ); + } + + #[test] + pub fn check_new_point() { + let point_key_pair = new_key_pair(); + let point_body = point_body(&point_key_pair); + let point = Point::new(&point_key_pair, point_body.clone()); + + let timer = Instant::now(); + let body = bincode::serialize(&point_body).expect("shouldn't happen"); + let bincode_elapsed = timer.elapsed(); + + let timer = Instant::now(); + let mut hasher = Sha256::new(); + hasher.update(body.as_slice()); + let digest = Digest(hasher.finalize().into()); + let sha_elapsed = timer.elapsed(); + assert_eq!(&digest, point.digest(), "point digest"); + + let timer = Instant::now(); + let sig = Signature::new(&point_key_pair, &digest); + let sig_elapsed = timer.elapsed(); + assert_eq!(&sig, point.signature(), "point signature"); + + println!( + "bincode {} bytes of point with {} bytes payload took {}", + body.len(), + point_body + .payload + .iter() + .fold(0, |acc, bytes| acc + bytes.len()), + humantime::format_duration(bincode_elapsed) + ); + println!("sha256 took {}", humantime::format_duration(sha_elapsed)); + println!( + "total {}", + humantime::format_duration(bincode_elapsed + sha_elapsed + sig_elapsed) + ) + } +} diff --git a/consensus/src/test_utils.rs b/consensus/src/test_utils.rs deleted file mode 100644 index 68c2f7305..000000000 --- a/consensus/src/test_utils.rs +++ /dev/null @@ -1,202 +0,0 @@ -use std::collections::hash_map::Entry::{Occupied, Vacant}; -use std::sync::Arc; - -use everscale_crypto::ed25519::{KeyPair, PublicKey, SecretKey}; -use tokio::sync::mpsc::UnboundedReceiver; -use tokio::sync::Mutex; -use tycho_network::{ - Address, DhtClient, DhtConfig, DhtService, Network, NetworkConfig, OverlayService, PeerId, - PeerInfo, Router, ToSocket, -}; -use tycho_util::time::now_sec; -use tycho_util::FastHashMap; - -use crate::effects::AltFormat; -use crate::engine::MempoolConfig; -use crate::models::{Link, Location, Point, PointBody, PointId, Round, UnixTime}; - -const GENESIS_SECRET_KEY_BYTES: [u8; 32] = [0xAE; 32]; -const GENESIS_MILLIS: u64 = 1713225727398; - -pub fn genesis() -> Arc { - let genesis_keys = KeyPair::from(&SecretKey::from_bytes(GENESIS_SECRET_KEY_BYTES)); - - Point::new(&genesis_keys, PointBody { - location: Location { - round: MempoolConfig::GENESIS_ROUND, - author: genesis_keys.public_key.into(), - }, - time: UnixTime::from_millis(GENESIS_MILLIS), - payload: vec![], - proof: None, - includes: Default::default(), - witness: Default::default(), - anchor_trigger: Link::ToSelf, - anchor_proof: Link::ToSelf, - anchor_time: UnixTime::from_millis(GENESIS_MILLIS), - }) -} - -pub fn make_peer_info(keypair: &KeyPair, address_list: Vec
, ttl: Option) -> PeerInfo { - let peer_id = PeerId::from(keypair.public_key); - - let now = now_sec(); - let mut peer_info = PeerInfo { - id: peer_id, - address_list: address_list.into_boxed_slice(), - created_at: now, - expires_at: ttl.unwrap_or(u32::MAX), - signature: Box::new([0; 64]), - }; - *peer_info.signature = keypair.sign(&peer_info); - peer_info -} - -// TODO receive configured services from general node, -// move current setup to tests as it provides acceptable timing -// This dependencies should be passed from validator module to init mempool -pub fn from_validator( - bind_address: T, - secret_key: &SecretKey, - remote_addr: Option
, - dht_config: DhtConfig, - network_config: NetworkConfig, -) -> (DhtClient, OverlayService) { - let local_id = PeerId::from(PublicKey::from(secret_key)); - - let (dht_tasks, dht_service) = DhtService::builder(local_id) - .with_config(dht_config) - .build(); - - let (overlay_tasks, overlay_service) = OverlayService::builder(local_id) - .with_dht_service(dht_service.clone()) - .build(); - - let router = Router::builder() - .route(dht_service.clone()) - .route(overlay_service.clone()) - .build(); - - let mut network_builder = Network::builder() - .with_config(network_config) - .with_private_key(secret_key.to_bytes()) - .with_service_name("mempool-test-network-service"); - if let Some(remote_addr) = remote_addr { - network_builder = network_builder.with_remote_addr(remote_addr); - } - - let network = network_builder.build(bind_address, router).unwrap(); - - dht_tasks.spawn(&network); - overlay_tasks.spawn(&network); - - (dht_service.make_client(&network), overlay_service) -} - -type PeerToAnchor = FastHashMap; -pub async fn check_anchors( - mut committed: UnboundedReceiver<(Arc, Vec>)>, - peer_id: PeerId, - anchors_hashmap: Arc>>, - known_round_refs: Arc>>>, - nodes_count: usize, -) { - loop { - let (anchor, refs) = committed - .recv() - .await - .expect("committed anchor reader must be alive"); - let anchor_id = anchor.id(); - - let anchor_round = anchor.body.location.round; - let mut guard = anchors_hashmap.lock().await; - - // get last previous anchor round and check if we don't have previous - guard.iter().for_each(|(key, value)| { - if key < &anchor_round { - tracing::info!( - "Checking consistency of prev round {:?} for node {}", - &key, - peer_id.alt() - ); - if value.get(&peer_id).is_none() { - panic!( - "Missing anchor for node {} at {:?} but received newer anchor {:?}", - peer_id.alt(), - key, - &anchor_round - ); - } - } - }); - - match guard.entry(anchor_round) { - Occupied(mut round_peers_entry) => { - let round_peers = round_peers_entry.get_mut(); - if round_peers.len() >= nodes_count { - panic!("We have more anchors than nodes at round: {anchor_round:?}"); - } - - if let Some(point) = round_peers.get(&peer_id) { - panic!( - "We have already anchor {:?} for round: {anchor_round:?} and node {}", - point.location, - peer_id.alt() - ); - } - - round_peers.insert(peer_id, anchor_id); - } - Vacant(entry) => { - let mut peer_map = FastHashMap::default(); - peer_map.insert(peer_id, anchor_id); - entry.insert(peer_map); - } - } - - let mut refs_guard = known_round_refs.lock().await; - match refs_guard.get(&anchor_round) { - Some(round) => { - if round.len() != refs.len() { - panic!("Commited points size differs for round: {anchor_round:?}"); - } - - for (rr, rf) in round.iter().zip(refs.iter()) { - if rr.digest != rf.digest { - panic!( - "Points are not equal or order is different for round {anchor_round:?}" - ) - } - } - } - None => { - let point_refs = refs.iter().map(|x| x.id()).collect::>(); - refs_guard.insert(anchor_round, point_refs); - } - } - - guard.retain(|key, value| { - if value.len() == nodes_count { - refs_guard.remove(key); - false - } else { - true - } - }); - - tracing::debug!("Anchor hashmap len: {}", guard.len()); - tracing::debug!("Refs hashmap ken: {}", refs_guard.len()); - - drop(guard); - drop(refs_guard); - } -} - -pub async fn drain_anchors(mut committed: UnboundedReceiver<(Arc, Vec>)>) { - loop { - _ = committed - .recv() - .await - .expect("committed anchor reader must be alive"); - } -} diff --git a/consensus/src/test_utils/anchor_consumer.rs b/consensus/src/test_utils/anchor_consumer.rs new file mode 100644 index 000000000..51282ea00 --- /dev/null +++ b/consensus/src/test_utils/anchor_consumer.rs @@ -0,0 +1,133 @@ +use std::collections::hash_map::Entry::{Occupied, Vacant}; + +use tokio::sync::mpsc::UnboundedReceiver; +use tokio_stream::wrappers::UnboundedReceiverStream; +use tokio_stream::{StreamExt, StreamMap}; +use tycho_network::PeerId; +use tycho_util::FastHashMap; + +use crate::effects::AltFormat; +use crate::models::{PointId, Round}; +use crate::Point; + +#[derive(Default)] +pub struct AnchorConsumer { + streams: StreamMap)>>, + // all committers must share the same sequence of anchor points + anchors: FastHashMap>, + // all committers must share the same anchor history (linearized inclusion dag) for each anchor + history: FastHashMap>, +} + +impl AnchorConsumer { + pub fn add(&mut self, committer: PeerId, committed: UnboundedReceiver<(Point, Vec)>) { + self.streams + .insert(committer, UnboundedReceiverStream::new(committed)); + } + + pub async fn drain(mut self) { + loop { + _ = self + .streams + .next() + .await + .expect("committed anchor reader must be alive"); + } + } + + pub async fn check(mut self) { + loop { + let (peer_id, (anchor, history)) = self + .streams + .next() + .await + .expect("committed anchor reader must be alive"); + let anchor_id = anchor.id(); + + let anchor_round = anchor.body().location.round; + + // get last previous anchor round and check if we don't have previous + for (prev_anchor_round, committers) in self + .anchors + .iter() + .filter(|(prev_anchor_round, _)| **prev_anchor_round < anchor_round) + { + assert!( + committers.get(&peer_id).is_some(), + "Missing anchor for node {} at {:?}, it committed {:?}", + peer_id.alt(), + prev_anchor_round, + anchor_id.alt(), + ); + } + + match self.anchors.entry(anchor_round) { + Occupied(mut stored_anchor) => { + let committers = stored_anchor.get_mut(); + assert!( + committers.len() < self.streams.len(), + "Broken test: we can't store {} committers for total {} peers at round {}", + committers.len(), + self.streams.len(), + anchor_round.0, + ); + assert!( + committers.get(&peer_id).is_none(), + "Peer {} already committed {:?}, now committed {:?}", + peer_id.alt(), + committers.get(&peer_id).map(|old_anchor| old_anchor.alt()), + anchor_id.alt() + ); + + committers.insert(peer_id, anchor_id); + } + Vacant(entry) => { + let mut peer_map = FastHashMap::default(); + peer_map.insert(peer_id, anchor_id); + entry.insert(peer_map); + } + } + + match self.history.get(&anchor_round) { + Some(stored_history) => { + assert_eq!( + stored_history.len(), + history.len(), + "Commited points size differs for {} at round: {}", + peer_id.alt(), + anchor_round.0, + ); + + for (left, right) in stored_history.iter().zip(history.iter()) { + assert_eq!( + &left.digest, + right.digest(), + "Points are not equal or order is different for round {anchor_round:?}" + ); + } + } + None => { + let point_refs = history.iter().map(|x| x.id()).collect::>(); + self.history.insert(anchor_round, point_refs); + } + } + + let mut common_anchors = vec![]; + self.anchors.retain(|key, value| { + if value.len() == self.streams.len() { + self.history.remove(key); + common_anchors.push(key.0); + false + } else { + true + } + }); + + tracing::debug!("Anchor hashmap len: {}", self.anchors.len()); + tracing::debug!("Refs hashmap ken: {}", self.history.len()); + if !common_anchors.is_empty() { + tracing::info!("all nodes commited anchors for rounds {:?}", common_anchors); + } + } + } +} diff --git a/consensus/src/test_utils/bootstrap.rs b/consensus/src/test_utils/bootstrap.rs new file mode 100644 index 000000000..50cfc4eb4 --- /dev/null +++ b/consensus/src/test_utils/bootstrap.rs @@ -0,0 +1,62 @@ +use everscale_crypto::ed25519::{KeyPair, PublicKey, SecretKey}; +use tycho_network::{ + Address, DhtClient, DhtConfig, DhtService, Network, NetworkConfig, OverlayService, PeerId, + PeerInfo, Router, ToSocket, +}; +use tycho_util::time::now_sec; + +pub fn make_peer_info(keypair: &KeyPair, address_list: Vec
, ttl: Option) -> PeerInfo { + let peer_id = PeerId::from(keypair.public_key); + + let now = now_sec(); + let mut peer_info = PeerInfo { + id: peer_id, + address_list: address_list.into_boxed_slice(), + created_at: now, + expires_at: ttl.unwrap_or(u32::MAX), + signature: Box::new([0; 64]), + }; + *peer_info.signature = keypair.sign(&peer_info); + peer_info +} + +// TODO receive configured services from general node, +// move current setup to tests as it provides acceptable timing +// This dependencies should be passed from validator module to init mempool +pub fn from_validator( + bind_address: T, + secret_key: &SecretKey, + remote_addr: Option
, + dht_config: DhtConfig, + network_config: NetworkConfig, +) -> (DhtClient, OverlayService) { + let local_id = PeerId::from(PublicKey::from(secret_key)); + + let (dht_tasks, dht_service) = DhtService::builder(local_id) + .with_config(dht_config) + .build(); + + let (overlay_tasks, overlay_service) = OverlayService::builder(local_id) + .with_dht_service(dht_service.clone()) + .build(); + + let router = Router::builder() + .route(dht_service.clone()) + .route(overlay_service.clone()) + .build(); + + let mut network_builder = Network::builder() + .with_config(network_config) + .with_private_key(secret_key.to_bytes()) + .with_service_name("mempool-test-network-service"); + if let Some(remote_addr) = remote_addr { + network_builder = network_builder.with_remote_addr(remote_addr); + } + + let network = network_builder.build(bind_address, router).unwrap(); + + dht_tasks.spawn(&network); + overlay_tasks.spawn(&network); + + (dht_service.make_client(&network), overlay_service) +} diff --git a/consensus/src/test_utils/genesis.rs b/consensus/src/test_utils/genesis.rs new file mode 100644 index 000000000..90285e7e8 --- /dev/null +++ b/consensus/src/test_utils/genesis.rs @@ -0,0 +1,31 @@ +use everscale_crypto::ed25519::{KeyPair, SecretKey}; + +use crate::engine::MempoolConfig; +use crate::models::{Link, Location, Point, PointBody, PointId, UnixTime}; + +const GENESIS_SECRET_KEY_BYTES: [u8; 32] = [0xAE; 32]; +const GENESIS_MILLIS: u64 = 1713225727398; + +// TODO this must be passed via config file +pub fn genesis_point_id() -> PointId { + genesis().id() +} + +pub fn genesis() -> Point { + let genesis_keys = KeyPair::from(&SecretKey::from_bytes(GENESIS_SECRET_KEY_BYTES)); + + Point::new(&genesis_keys, PointBody { + location: Location { + round: MempoolConfig::GENESIS_ROUND, + author: genesis_keys.public_key.into(), + }, + time: UnixTime::from_millis(GENESIS_MILLIS), + payload: vec![], + proof: None, + includes: Default::default(), + witness: Default::default(), + anchor_trigger: Link::ToSelf, + anchor_proof: Link::ToSelf, + anchor_time: UnixTime::from_millis(GENESIS_MILLIS), + }) +} diff --git a/consensus/src/test_utils/mod.rs b/consensus/src/test_utils/mod.rs new file mode 100644 index 000000000..45efb2c62 --- /dev/null +++ b/consensus/src/test_utils/mod.rs @@ -0,0 +1,12 @@ +#[cfg(feature = "test")] +pub use anchor_consumer::*; +#[cfg(feature = "test")] +pub use bootstrap::*; +// TODO hide the whole mod under feature flag after configs are implemented +pub(crate) use genesis::*; + +#[cfg(feature = "test")] +mod anchor_consumer; +#[cfg(feature = "test")] +mod bootstrap; +mod genesis; diff --git a/util/src/sync/rayon.rs b/util/src/sync/rayon.rs index cc4a96af7..e62dfd929 100644 --- a/util/src/sync/rayon.rs +++ b/util/src/sync/rayon.rs @@ -1,8 +1,12 @@ pub async fn rayon_run(f: impl 'static + Send + FnOnce() -> T) -> T { let (send, recv) = tokio::sync::oneshot::channel(); + let span = tracing::Span::current(); rayon::spawn(move || { if send.send(f()).is_err() { - tracing::warn!("rayon_run has been aborted"); + tracing::warn!( + parent: &span, + "rayon_run has been aborted" + ); } }); recv.await.unwrap()