warp sync hardening

changes/added/ledger-warp-sync.md

@@ -2,8 +2,14 @@
 
 Adds support for syncing ledger state, while warp syncing. Should enable Substrate warp sync.
 Non-validator nodes serve ledger snapshots to warp-syncing peers; validators don't by default,
-but can opt in via the new `--serve-warp-ledger-sync` flag (off by default). Nodes can warp-sync
-as clients regardless of the flag.
+but can opt in via the new `--serve-warp-ledger-sync` flag (off by default), and any node can opt
+out with `--no-serve-warp-ledger-sync`. Nodes can warp-sync as clients regardless of either flag.
+
+Serving is bounded against abuse: snapshots are memoized in a small LRU, peers that replay an
+identical byte range are penalised, and each peer has a budget for how many full-arena
+serializations it may induce, charged before the work rather than after. On the client side each
+range request has its own timeout and the whole per-peer transfer has a throughput floor, so one
+slow peer cannot hold arena recovery open indefinitely.
 
 PR: https://github.com/midnightntwrk/midnight-node/pull/1650
-Issue: https://github.com/midnightntwrk/midnight-node/issues/1648
+Issue: https://github.com/midnightntwrk/midnight-node/issues/1648

node/Cargo.toml

@@ -27,6 +27,7 @@ async-trait.workspace = true
 futures.workspace = true
 # Must match the version sc-network's `request_response_config` inbound queue expects (1.9).
 async-channel = "1.9"
+lru.workspace = true
 
 sc-cli.workspace = true
 sc-chain-spec.workspace = true

node/src/cli.rs

@@ -50,12 +50,21 @@ pub struct RunMidnight {
 
 	/// Serve Midnight ledger snapshots to warp-syncing peers even when running as a validator.
 	///
-	/// Non-validator nodes always serve. Validators don't by default — serializing the ledger
-	/// arena is the warp ledger-sync protocol's most CPU-expensive operation and competes with
+	/// Non-validator nodes serve by default. Validators don't — serializing the ledger arena is the
+	/// warp ledger-sync protocol's most CPU-expensive operation and competes with
 	/// authoring/finality duties — but can opt in with this flag, e.g. on small or local networks
 	/// that have no non-validator nodes. Nodes can warp-sync as clients regardless of this flag.
 	#[arg(long)]
 	pub serve_warp_ledger_sync: bool,
+
+	/// Never serve Midnight ledger snapshots to warp-syncing peers.
+	///
+	/// The opt-out counterpart of `--serve-warp-ledger-sync`, for non-validators that must not
+	/// spend CPU on other nodes' warp sync — public RPC endpoints and bootnodes, whose exposure to
+	/// arbitrary peers is highest. Overrides the serve-by-default for non-validators; passing both
+	/// flags is rejected rather than silently resolved. The node can still warp-sync as a client.
+	#[arg(long, conflicts_with = "serve_warp_ledger_sync")]
+	pub no_serve_warp_ledger_sync: bool,
 }
 
 #[derive(Debug, clap::Parser)]
@@ -538,3 +547,44 @@ impl std::fmt::Display for NotImplementedError {
 	}
 }
 impl core::error::Error for NotImplementedError {}
+
+#[cfg(test)]
+mod tests {
+	use super::*;
+	use clap::Parser;
+
+	/// Serving is off by default for validators (they must opt in) and the opt-out is separate, so
+	/// neither flag may be set unless the operator asked for it.
+	#[test]
+	fn warp_ledger_sync_flags_default_off() {
+		let run = RunMidnight::try_parse_from(["midnight-node"]).expect("bare run parses");
+		assert!(!run.serve_warp_ledger_sync);
+		assert!(!run.no_serve_warp_ledger_sync);
+	}
+
+	#[test]
+	fn warp_ledger_sync_flags_parse_individually() {
+		let opt_in = RunMidnight::try_parse_from(["midnight-node", "--serve-warp-ledger-sync"])
+			.expect("opt-in parses");
+		assert!(opt_in.serve_warp_ledger_sync);
+		assert!(!opt_in.no_serve_warp_ledger_sync);
+
+		let opt_out = RunMidnight::try_parse_from(["midnight-node", "--no-serve-warp-ledger-sync"])
+			.expect("opt-out parses");
+		assert!(opt_out.no_serve_warp_ledger_sync);
+		assert!(!opt_out.serve_warp_ledger_sync);
+	}
+
+	/// "Serve" and "never serve" together is operator error, not something to resolve by silent
+	/// precedence — the operator should be told which one they meant.
+	#[test]
+	fn warp_ledger_sync_flags_conflict() {
+		let err = RunMidnight::try_parse_from([
+			"midnight-node",
+			"--serve-warp-ledger-sync",
+			"--no-serve-warp-ledger-sync",
+		])
+		.expect_err("both flags together must be rejected");
+		assert_eq!(err.kind(), clap::error::ErrorKind::ArgumentConflict);
+	}
+}

node/src/command.rs

@@ -336,6 +336,7 @@ fn run_node(cfg: Cfg) -> sc_cli::Result<()> {
 			tx_filter_config,
 			run_midnight.rpc_max_finality_subscriptions,
 			run_midnight.serve_warp_ledger_sync,
+			run_midnight.no_serve_warp_ledger_sync,
 		)
 		.await
 		.map_err(sc_cli::Error::Service)?;
@@ -422,6 +423,8 @@ fn run_subcommand(subcommand: Subcommand, cfg: Cfg) -> sc_cli::Result<()> {
 					midnight_cfg,
 					storage_config,
 					tx_filter_config,
+					// Subcommands never serve warp sync.
+					false,
 				)?;
 				Ok((client, task_manager, other.5.authority_selection))
 			};
@@ -443,6 +446,8 @@ fn run_subcommand(subcommand: Subcommand, cfg: Cfg) -> sc_cli::Result<()> {
 						cfg.midnight_cfg.clone(),
 						storage_config,
 						tx_filter_config,
+						// Subcommands never serve warp sync.
+						false,
 					)?;
 				Ok((cmd.run(client, import_queue), task_manager))
 			})
@@ -458,6 +463,8 @@ fn run_subcommand(subcommand: Subcommand, cfg: Cfg) -> sc_cli::Result<()> {
 					cfg.midnight_cfg.clone(),
 					storage_config,
 					tx_filter_config,
+					// Subcommands never serve warp sync.
+					false,
 				)?;
 				Ok((cmd.run(client, config.database), task_manager))
 			})
@@ -472,6 +479,8 @@ fn run_subcommand(subcommand: Subcommand, cfg: Cfg) -> sc_cli::Result<()> {
 					cfg.midnight_cfg.clone(),
 					storage_config,
 					tx_filter_config,
+					// Subcommands never serve warp sync.
+					false,
 				)?;
 				Ok((cmd.run(client, config.chain_spec), task_manager))
 			})
@@ -488,6 +497,8 @@ fn run_subcommand(subcommand: Subcommand, cfg: Cfg) -> sc_cli::Result<()> {
 						cfg.midnight_cfg.clone(),
 						storage_config,
 						tx_filter_config,
+						// Subcommands never serve warp sync.
+						false,
 					)?;
 				Ok((cmd.run(client, import_queue), task_manager))
 			})
@@ -507,6 +518,8 @@ fn run_subcommand(subcommand: Subcommand, cfg: Cfg) -> sc_cli::Result<()> {
 					cfg.midnight_cfg.clone(),
 					storage_config,
 					tx_filter_config,
+					// Subcommands never serve warp sync.
+					false,
 				)?;
 				let aux_revert = Box::new(|client, _, blocks| {
 					sc_consensus_grandpa::revert(client, blocks)?;
@@ -547,6 +560,8 @@ fn run_subcommand(subcommand: Subcommand, cfg: Cfg) -> sc_cli::Result<()> {
 							cfg.midnight_cfg.clone(),
 							storage_config,
 							tx_filter_config,
+							// Subcommands never serve warp sync.
+							false,
 						)?;
 
 						cmd.run(partial.client)
@@ -567,6 +582,8 @@ fn run_subcommand(subcommand: Subcommand, cfg: Cfg) -> sc_cli::Result<()> {
 							cfg.midnight_cfg.clone(),
 							storage_config,
 							tx_filter_config,
+							// Subcommands never serve warp sync.
+							false,
 						)?;
 						let db = partial.backend.expose_db();
 						let storage = partial.backend.expose_storage();
@@ -583,6 +600,8 @@ fn run_subcommand(subcommand: Subcommand, cfg: Cfg) -> sc_cli::Result<()> {
 							cfg.midnight_cfg.clone(),
 							storage_config,
 							tx_filter_config,
+							// Subcommands never serve warp sync.
+							false,
 						)?;
 						let ext_builder = RemarkBuilder::new(partial.client.clone());
 
@@ -605,6 +624,8 @@ fn run_subcommand(subcommand: Subcommand, cfg: Cfg) -> sc_cli::Result<()> {
 							cfg.midnight_cfg.clone(),
 							storage_config,
 							tx_filter_config,
+							// Subcommands never serve warp sync.
+							false,
 						)?;
 						// Register the *Remark* and *TKA* builders.
 						let ext_factory = ExtrinsicFactory(vec![Box::new(RemarkBuilder::new(

node/src/service.rs

@@ -288,6 +288,7 @@ pub fn new_partial(
 	midnight_cfg: MidnightCfg,
 	storage_config: StorageInit,
 	tx_filter_config: TxFilterConfig,
+	serve_warp_ledger_sync: bool,
 ) -> Result<MidnightService, ServiceError> {
 	let mc_follower_metrics = register_metrics_warn_errors(config.prometheus_registry());
 	let midnight_metrics =
@@ -327,6 +328,22 @@ pub fn new_partial(
 	let executor = sc_service::new_wasm_executor(&config.executor);
 
 	let mut db_config = config.db_config();
+
+	// Serving warp sync needs the GRANDPA justifications at authority-set changes to survive
+	// `--blocks-pruning`; without the filter a block-pruned node silently stops being a viable
+	// warp-sync server, because the warp proof is built from exactly those blocks. Costs nothing
+	// under the default `archive-canonical`, where no bodies are pruned at all.
+	//
+	// Conditional on serving, so a node that never serves does not retain blocks it has no use
+	// for. NOTE: the filter only protects blocks pruned *from here on*. Enabling serving later on
+	// a node that has already run block-pruned leaves holes in its justification history that
+	// this cannot repair.
+	if serve_warp_ledger_sync {
+		db_config
+			.pruning_filters
+			.push(std::sync::Arc::new(sc_consensus_grandpa::GrandpaPruningFilter));
+	}
+
 	let DatabaseSource::ParityDb { path: db_path } = db_config.source else {
 		panic!("Midnight node support only parity-db as a backend");
 	};
@@ -540,13 +557,35 @@ pub async fn new_full<Network: sc_network::NetworkBackend<Block, <Block as Block
 	tx_filter_config: TxFilterConfig,
 	max_finality_subscriptions: u32,
 	serve_warp_ledger_sync: bool,
+	no_serve_warp_ledger_sync: bool,
 ) -> Result<(TaskManager, Arc<FullBackend>), ServiceError> {
 	let database_source = config.database.clone();
 	// Captured before `storage_config` is moved into `new_partial`: selects the ParityDb layout the
 	// warp ledger-sync server/importer dispatch to.
 	let warp_ledger_unified = matches!(storage_config.separation, StorageSeparation::Unified);
-	let new_partial_components =
-		new_partial(&config, epoch_config.clone(), midnight_cfg, storage_config, tx_filter_config)?;
+
+	// Decided here rather than at the point of use: `new_partial` needs it too, to know whether to
+	// install the GRANDPA pruning filter that keeps this node able to serve warp proofs. One
+	// computation, both consumers.
+	//
+	// Non-validators serve by default. Validators don't — arena serialization is the protocol's
+	// most CPU-expensive operation and must not compete with authoring/finality (a remote DoS
+	// vector) — unless the operator opts in via `--serve-warp-ledger-sync` (for small or local
+	// networks with no non-validator nodes to serve). `--no-serve-warp-ledger-sync` opts out
+	// outright, for non-validators whose CPU must not be spent on other nodes' warp sync (public
+	// RPC endpoints, bootnodes). Every node still registers the protocol as a client, so it can
+	// warp-sync and recover its own arena regardless. Clap rejects both flags together.
+	let serve_ledger_sync =
+		!no_serve_warp_ledger_sync && (!config.role.is_authority() || serve_warp_ledger_sync);
+
+	let new_partial_components = new_partial(
+		&config,
+		epoch_config.clone(),
+		midnight_cfg,
+		storage_config,
+		tx_filter_config,
+		serve_ledger_sync,
+	)?;
 
 	let sc_service::PartialComponents {
 		client,
@@ -613,20 +652,20 @@ pub async fn new_full<Network: sc_network::NetworkBackend<Block, <Block as Block
 	// Warp ledger-sync: register the request/response protocol that serves / recovers the
 	// Midnight ledger arena after warp+state-sync. The server handler is spawned after the network
 	// is built; `ledger_sync_protocol_name` is reused by the client driver in the monitor.
-	//
-	// Non-validators serve by default. Validators don't — arena serialization is the protocol's
-	// most CPU-expensive operation and must not compete with authoring/finality (a remote DoS
-	// vector) — unless the operator opts in via `--serve-warp-ledger-sync` (for small or local
-	// networks with no non-validator nodes to serve). Every node still registers the protocol as
-	// a client, so it can warp-sync and recover its own arena regardless.
-	let serve_ledger_sync = !config.role.is_authority() || serve_warp_ledger_sync;
+	// `serve_ledger_sync` was decided above, before `new_partial`.
 	if serve_warp_ledger_sync && config.role.is_authority() {
 		log::warn!(
 			"--serve-warp-ledger-sync is enabled on an authority: serializing ledger snapshots \
 			 for warp-syncing peers is CPU-expensive and may compete with block authoring and \
 			 finality duties"
 		);
 	}
+	if no_serve_warp_ledger_sync {
+		log::info!(
+			"--no-serve-warp-ledger-sync: this node will not serve ledger snapshots to \
+			 warp-syncing peers (it can still warp-sync as a client)"
+		);
+	}
 	let ledger_sync_protocol_name: sc_network::ProtocolName =
 		crate::warp_ledger_sync::protocol::ledger_sync_protocol_name(
 			genesis_hash,