polkadot: eradicate LeafStatus (#1565)

ordian · web-flow · commit 5ca909cc09ca · 2023-10-23T16:22:37.000+02:00
Fixes #768.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/cumulus/client/relay-chain-minimal-node/Cargo.toml b/cumulus/client/relay-chain-minimal-node/Cargo.toml
@@ -37,7 +37,6 @@ cumulus-relay-chain-rpc-interface = { path = "../relay-chain-rpc-interface" }
 cumulus-primitives-core = { path = "../../primitives/core" }
 
 array-bytes = "6.1"
-schnellru = "0.2.1"
 tracing = "0.1.37"
 async-trait = "0.1.73"
 futures = "0.3.28"
diff --git a/cumulus/client/relay-chain-minimal-node/src/collator_overseer.rs b/cumulus/client/relay-chain-minimal-node/src/collator_overseer.rs
@@ -15,7 +15,6 @@
 // along with Polkadot.  If not, see <http://www.gnu.org/licenses/>.
 
 use futures::{select, StreamExt};
-use schnellru::{ByLength, LruMap};
 use std::sync::Arc;
 
 use polkadot_availability_recovery::AvailabilityRecoverySubsystem;
@@ -36,7 +35,7 @@ use polkadot_node_network_protocol::{
 use polkadot_node_subsystem_util::metrics::{prometheus::Registry, Metrics};
 use polkadot_overseer::{
 	BlockInfo, DummySubsystem, Handle, Overseer, OverseerConnector, OverseerHandle, SpawnGlue,
-	UnpinHandle, KNOWN_LEAVES_CACHE_SIZE,
+	UnpinHandle,
 };
 use polkadot_primitives::CollatorPair;
 
@@ -156,7 +155,6 @@ fn build_overseer(
 		.span_per_active_leaf(Default::default())
 		.active_leaves(Default::default())
 		.supports_parachains(runtime_client)
-		.known_leaves(LruMap::new(ByLength::new(KNOWN_LEAVES_CACHE_SIZE)))
 		.metrics(Metrics::register(registry)?)
 		.spawner(spawner);
 
diff --git a/polkadot/node/core/bitfield-signing/src/lib.rs b/polkadot/node/core/bitfield-signing/src/lib.rs
@@ -32,8 +32,8 @@ use polkadot_node_subsystem::{
 	messages::{
 		AvailabilityStoreMessage, BitfieldDistributionMessage, RuntimeApiMessage, RuntimeApiRequest,
 	},
-	overseer, ActivatedLeaf, FromOrchestra, LeafStatus, OverseerSignal, PerLeafSpan,
-	SpawnedSubsystem, SubsystemError, SubsystemResult, SubsystemSender,
+	overseer, ActivatedLeaf, FromOrchestra, OverseerSignal, PerLeafSpan, SpawnedSubsystem,
+	SubsystemError, SubsystemResult, SubsystemSender,
 };
 use polkadot_node_subsystem_util::{self as util, Validator};
 use polkadot_primitives::{AvailabilityBitfield, CoreState, Hash, ValidatorIndex};
@@ -257,16 +257,6 @@ async fn handle_active_leaves_update<Sender>(
 where
 	Sender: overseer::BitfieldSigningSenderTrait,
 {
-	if let LeafStatus::Stale = leaf.status {
-		gum::debug!(
-			target: LOG_TARGET,
-			relay_parent = ?leaf.hash,
-			block_number =  ?leaf.number,
-			"Skip bitfield signing for stale leaf"
-		);
-		return Ok(())
-	}
-
 	let span = PerLeafSpan::new(leaf.span, "bitfield-signing");
 	let span_delay = span.child("delay");
 	let wait_until = Instant::now() + SPAWNED_TASK_DELAY;
diff --git a/polkadot/node/core/provisioner/src/lib.rs b/polkadot/node/core/provisioner/src/lib.rs
@@ -31,8 +31,8 @@ use polkadot_node_subsystem::{
 		CandidateBackingMessage, ChainApiMessage, ProspectiveParachainsMessage, ProvisionableData,
 		ProvisionerInherentData, ProvisionerMessage, RuntimeApiMessage, RuntimeApiRequest,
 	},
-	overseer, ActivatedLeaf, ActiveLeavesUpdate, FromOrchestra, LeafStatus, OverseerSignal,
-	PerLeafSpan, RuntimeApiError, SpawnedSubsystem, SubsystemError,
+	overseer, ActivatedLeaf, ActiveLeavesUpdate, FromOrchestra, OverseerSignal, PerLeafSpan,
+	RuntimeApiError, SpawnedSubsystem, SubsystemError,
 };
 use polkadot_node_subsystem_util::{
 	request_availability_cores, request_persisted_validation_data,
@@ -421,13 +421,7 @@ async fn send_inherent_data(
 		"Selected disputes"
 	);
 
-	// Only include bitfields on fresh leaves. On chain reversions, we want to make sure that
-	// there will be at least one block, which cannot get disputed, so the chain can make progress.
-	let bitfields = match leaf.status {
-		LeafStatus::Fresh =>
-			select_availability_bitfields(&availability_cores, bitfields, &leaf.hash),
-		LeafStatus::Stale => Vec::new(),
-	};
+	let bitfields = select_availability_bitfields(&availability_cores, bitfields, &leaf.hash);
 
 	gum::trace!(
 		target: LOG_TARGET,
diff --git a/polkadot/node/network/availability-distribution/src/requester/mod.rs b/polkadot/node/network/availability-distribution/src/requester/mod.rs
@@ -35,7 +35,7 @@ use futures::{
 use polkadot_node_subsystem::{
 	jaeger,
 	messages::{ChainApiMessage, RuntimeApiMessage},
-	overseer, ActivatedLeaf, ActiveLeavesUpdate, LeafStatus,
+	overseer, ActivatedLeaf, ActiveLeavesUpdate,
 };
 use polkadot_node_subsystem_util::runtime::{get_occupied_cores, RuntimeInfo};
 use polkadot_primitives::{CandidateHash, Hash, OccupiedCore, SessionIndex};
@@ -105,8 +105,7 @@ impl Requester {
 	) -> Result<()> {
 		gum::trace!(target: LOG_TARGET, ?update, "Update fetching heads");
 		let ActiveLeavesUpdate { activated, deactivated } = update;
-		// Stale leaves happen after a reversion - we don't want to re-run availability there.
-		if let Some(leaf) = activated.filter(|leaf| leaf.status == LeafStatus::Fresh) {
+		if let Some(leaf) = activated {
 			let span = spans
 				.get(&leaf.hash)
 				.map(|span| span.child("update-fetching-heads"))
diff --git a/polkadot/node/overseer/Cargo.toml b/polkadot/node/overseer/Cargo.toml
@@ -18,7 +18,6 @@ polkadot-node-metrics = { path = "../metrics" }
 polkadot-primitives = { path = "../../primitives" }
 orchestra = { version = "0.3.3", default-features = false, features=["futures_channel"] }
 gum = { package = "tracing-gum", path = "../gum" }
-schnellru = "0.2.1"
 sp-core = { path = "../../../substrate/primitives/core" }
 async-trait = "0.1.57"
 tikv-jemalloc-ctl = { version = "0.5.0", optional = true }
diff --git a/polkadot/node/overseer/src/dummy.rs b/polkadot/node/overseer/src/dummy.rs
@@ -17,11 +17,9 @@
 use crate::{
 	prometheus::Registry, HeadSupportsParachains, InitializedOverseerBuilder, MetricsTrait,
 	Overseer, OverseerMetrics, OverseerSignal, OverseerSubsystemContext, SpawnGlue,
-	KNOWN_LEAVES_CACHE_SIZE,
 };
 use orchestra::{FromOrchestra, SpawnedSubsystem, Subsystem, SubsystemContext};
 use polkadot_node_subsystem_types::{errors::SubsystemError, messages::*};
-use schnellru::{ByLength, LruMap};
 // Generated dummy messages
 use crate::messages::*;
 
@@ -193,7 +191,6 @@ where
 		.activation_external_listeners(Default::default())
 		.span_per_active_leaf(Default::default())
 		.active_leaves(Default::default())
-		.known_leaves(LruMap::new(ByLength::new(KNOWN_LEAVES_CACHE_SIZE)))
 		.spawner(SpawnGlue(spawner))
 		.metrics(metrics)
 		.supports_parachains(supports_parachains);
diff --git a/polkadot/node/overseer/src/lib.rs b/polkadot/node/overseer/src/lib.rs
@@ -70,7 +70,6 @@ use std::{
 };
 
 use futures::{channel::oneshot, future::BoxFuture, select, Future, FutureExt, StreamExt};
-use schnellru::LruMap;
 
 use client::{BlockImportNotification, BlockchainEvents, FinalityNotification};
 use polkadot_primitives::{Block, BlockNumber, Hash};
@@ -88,8 +87,8 @@ use polkadot_node_subsystem_types::messages::{
 
 pub use polkadot_node_subsystem_types::{
 	errors::{SubsystemError, SubsystemResult},
-	jaeger, ActivatedLeaf, ActiveLeavesUpdate, LeafStatus, OverseerSignal,
-	RuntimeApiSubsystemClient, UnpinHandle,
+	jaeger, ActivatedLeaf, ActiveLeavesUpdate, OverseerSignal, RuntimeApiSubsystemClient,
+	UnpinHandle,
 };
 
 pub mod metrics;
@@ -112,10 +111,6 @@ pub use orchestra::{
 	SubsystemSender, TimeoutExt, ToOrchestra, TrySendError,
 };
 
-/// Store 2 days worth of blocks, not accounting for forks,
-/// in the LRU cache. Assumes a 6-second block time.
-pub const KNOWN_LEAVES_CACHE_SIZE: u32 = 2 * 24 * 3600 / 6;
-
 #[cfg(any(target_os = "linux", feature = "jemalloc-allocator"))]
 mod memory_stats;
 #[cfg(test)]
@@ -283,6 +278,11 @@ impl From<FinalityNotification<Block>> for BlockInfo {
 /// as the substrate framework or user interaction.
 pub enum Event {
 	/// A new block was imported.
+	///
+	/// This event is not sent if the block was already known
+	/// and we reorged to it e.g. due to a reversion.
+	///
+	/// Also, these events are not sent during a major sync.
 	BlockImported(BlockInfo),
 	/// A block was finalized with i.e. babe or another consensus algorithm.
 	BlockFinalized(BlockInfo),
@@ -641,9 +641,6 @@ pub struct Overseer<SupportsParachains> {
 	/// An implementation for checking whether a header supports parachain consensus.
 	pub supports_parachains: SupportsParachains,
 
-	/// An LRU cache for keeping track of relay-chain heads that have already been seen.
-	pub known_leaves: LruMap<Hash, ()>,
-
 	/// Various Prometheus metrics.
 	pub metrics: OverseerMetrics,
 }
@@ -802,10 +799,9 @@ where
 		};
 
 		let mut update = match self.on_head_activated(&block.hash, Some(block.parent_hash)).await {
-			Some((span, status)) => ActiveLeavesUpdate::start_work(ActivatedLeaf {
+			Some(span) => ActiveLeavesUpdate::start_work(ActivatedLeaf {
 				hash: block.hash,
 				number: block.number,
-				status,
 				unpin_handle: block.unpin_handle,
 				span,
 			}),
@@ -864,7 +860,7 @@ where
 		&mut self,
 		hash: &Hash,
 		parent_hash: Option<Hash>,
-	) -> Option<(Arc<jaeger::Span>, LeafStatus)> {
+	) -> Option<Arc<jaeger::Span>> {
 		if !self.supports_parachains.head_supports_parachains(hash).await {
 			return None
 		}
@@ -891,14 +887,7 @@ where
 		let span = Arc::new(span);
 		self.span_per_active_leaf.insert(*hash, span.clone());
 
-		let status = if self.known_leaves.get(hash).is_some() {
-			LeafStatus::Stale
-		} else {
-			self.known_leaves.insert(*hash, ());
-			LeafStatus::Fresh
-		};
-
-		Some((span, status))
+		Some(span)
 	}
 
 	fn on_head_deactivated(&mut self, hash: &Hash) {
diff --git a/polkadot/node/service/Cargo.toml b/polkadot/node/service/Cargo.toml
@@ -86,7 +86,6 @@ parity-db = { version = "0.4.8", optional = true }
 codec = { package = "parity-scale-codec", version = "3.6.1" }
 
 async-trait = "0.1.57"
-schnellru = "0.2.1"
 log = "0.4.17"
 is_executable = "1.0.1"
 
diff --git a/polkadot/node/service/src/overseer.rs b/polkadot/node/service/src/overseer.rs
@@ -40,7 +40,6 @@ use polkadot_overseer::{
 	metrics::Metrics as OverseerMetrics, InitializedOverseerBuilder, MetricsTrait, Overseer,
 	OverseerConnector, OverseerHandle, SpawnGlue,
 };
-use schnellru::{ByLength, LruMap};
 
 use polkadot_primitives::runtime_api::ParachainHost;
 use sc_authority_discovery::Service as AuthorityDiscoveryService;
@@ -342,7 +341,6 @@ where
 		.span_per_active_leaf(Default::default())
 		.active_leaves(Default::default())
 		.supports_parachains(runtime_api_client)
-		.known_leaves(LruMap::new(ByLength::new(KNOWN_LEAVES_CACHE_SIZE)))
 		.metrics(metrics)
 		.spawner(spawner);
 
@@ -380,8 +378,6 @@ pub trait OverseerGen {
 	// as consequence make this rather annoying to implement and use.
 }
 
-use polkadot_overseer::KNOWN_LEAVES_CACHE_SIZE;
-
 /// The regular set of subsystems.
 pub struct RealOverseerGen;
 
diff --git a/polkadot/node/subsystem-test-helpers/src/mock.rs b/polkadot/node/subsystem-test-helpers/src/mock.rs
@@ -16,7 +16,7 @@
 
 use std::sync::Arc;
 
-use polkadot_node_subsystem::{jaeger, ActivatedLeaf, LeafStatus};
+use polkadot_node_subsystem::{jaeger, ActivatedLeaf};
 use sc_client_api::UnpinHandle;
 use sc_keystore::LocalKeystore;
 use sc_utils::mpsc::tracing_unbounded;
@@ -55,7 +55,6 @@ pub fn new_leaf(hash: Hash, number: BlockNumber) -> ActivatedLeaf {
 	ActivatedLeaf {
 		hash,
 		number,
-		status: LeafStatus::Fresh,
 		unpin_handle: dummy_unpin_handle(hash),
 		span: Arc::new(jaeger::Span::Disabled),
 	}
diff --git a/polkadot/node/subsystem-types/src/lib.rs b/polkadot/node/subsystem-types/src/lib.rs
@@ -51,44 +51,13 @@ pub use polkadot_node_jaeger as jaeger;
 /// If there are greater than this number of slots, then we fall back to a heap vector.
 const ACTIVE_LEAVES_SMALLVEC_CAPACITY: usize = 8;
 
-/// The status of an activated leaf.
-#[derive(Clone, Debug, PartialEq)]
-pub enum LeafStatus {
-	/// A leaf is fresh when it's the first time the leaf has been encountered.
-	/// Most leaves should be fresh.
-	Fresh,
-	/// A leaf is stale when it's encountered for a subsequent time. This will happen
-	/// when the chain is reverted or the fork-choice rule abandons some chain.
-	Stale,
-}
-
-impl LeafStatus {
-	/// Returns a `bool` indicating fresh status.
-	pub fn is_fresh(&self) -> bool {
-		match *self {
-			LeafStatus::Fresh => true,
-			LeafStatus::Stale => false,
-		}
-	}
-
-	/// Returns a `bool` indicating stale status.
-	pub fn is_stale(&self) -> bool {
-		match *self {
-			LeafStatus::Fresh => false,
-			LeafStatus::Stale => true,
-		}
-	}
-}
-
 /// Activated leaf.
 #[derive(Debug, Clone)]
 pub struct ActivatedLeaf {
 	/// The block hash.
 	pub hash: Hash,
 	/// The block number.
 	pub number: BlockNumber,
-	/// The status of the leaf.
-	pub status: LeafStatus,
 	/// A handle to unpin the block on drop.
 	pub unpin_handle: UnpinHandle,
 	/// An associated [`jaeger::Span`].
diff --git a/polkadot/node/subsystem/src/lib.rs b/polkadot/node/subsystem/src/lib.rs
@@ -28,7 +28,7 @@ pub use polkadot_overseer::{self as overseer, *};
 
 pub use polkadot_node_subsystem_types::{
 	errors::{self, *},
-	ActivatedLeaf, LeafStatus,
+	ActivatedLeaf,
 };
 
 /// Re-export of all messages type, including the wrapper type.
diff --git a/polkadot/roadmap/implementers-guide/src/types/overseer-protocol.md b/polkadot/roadmap/implementers-guide/src/types/overseer-protocol.md
@@ -32,18 +32,8 @@ Indicates a change in active leaves. Activated leaves should have jobs, whereas
 winding-down of work based on those leaves.
 
 ```rust
-enum LeafStatus {
-    // A leaf is fresh when it's the first time the leaf has been encountered.
-    // Most leaves should be fresh.
-    Fresh,
-    // A leaf is stale when it's encountered for a subsequent time. This will
-    // happen when the chain is reverted or the fork-choice rule abandons some
-    // chain.
-    Stale,
-}
-
 struct ActiveLeavesUpdate {
-    activated: [(Hash, Number, LeafStatus)], // in practice, these should probably be a SmallVec
+    activated: [(Hash, Number)],
     deactivated: [Hash],
 }
 ```
