From 5e0843e5a0fb601016dc1ccd46451a96049600e9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bastian=20K=C3=B6cher?= <git@kchr.de>
Date: Sat, 19 Oct 2024 01:05:44 +0200
Subject: [PATCH] sync: Remove checking of the extrinsics root (#5686)

With the introduction of `system_version` in
https://github.com/paritytech/polkadot-sdk/pull/4257 the extrinsic root
may also use the `V1` layout. At this point in the sync code it would
require some special handling to find out the `system_version`. So, this
pull request is removing it. The extrinsics root is checked when
executing the block later, so that at least no invalid block gets
imported.
---
 prdoc/pr_5686.prdoc                           | 15 +++++++++++++
 .../network/sync/src/strategy/chain_sync.rs   | 22 +------------------
 2 files changed, 16 insertions(+), 21 deletions(-)
 create mode 100644 prdoc/pr_5686.prdoc

diff --git a/prdoc/pr_5686.prdoc b/prdoc/pr_5686.prdoc
new file mode 100644
index 00000000000..3f0da912a34
--- /dev/null
+++ b/prdoc/pr_5686.prdoc
@@ -0,0 +1,15 @@
+title: "sync: Remove checking of the extrinsics root"
+
+doc:
+  - audience: Node Dev
+    description: |
+      Remove checking the extrinsics root as part of the sync code. 
+      With the introduction of `system_version` and the possibility to use the `V1`
+      layout for the trie when calculating the extrinsics root, it would require the
+      sync code to fetch the runtime version first before knowing which layout to use
+      when building the extrinsic root.
+      The extrinsics root is still checked when executing a block on chain.
+
+crates:
+  - name: sc-network-sync
+    bump: patch
diff --git a/substrate/client/network/sync/src/strategy/chain_sync.rs b/substrate/client/network/sync/src/strategy/chain_sync.rs
index b0e28d00f64..202033e8e00 100644
--- a/substrate/client/network/sync/src/strategy/chain_sync.rs
+++ b/substrate/client/network/sync/src/strategy/chain_sync.rs
@@ -42,7 +42,6 @@ use crate::{
 	LOG_TARGET,
 };
 
-use codec::Encode;
 use log::{debug, error, info, trace, warn};
 use prometheus_endpoint::{register, Gauge, PrometheusError, Registry, U64};
 use sc_client_api::{blockchain::BlockGap, BlockBackend, ProofProvider};
@@ -57,8 +56,7 @@ use sp_blockchain::{Error as ClientError, HeaderBackend, HeaderMetadata};
 use sp_consensus::{BlockOrigin, BlockStatus};
 use sp_runtime::{
 	traits::{
-		Block as BlockT, CheckedSub, Hash, HashingFor, Header as HeaderT, NumberFor, One,
-		SaturatedConversion, Zero,
+		Block as BlockT, CheckedSub, Header as HeaderT, NumberFor, One, SaturatedConversion, Zero,
 	},
 	EncodedJustification, Justifications,
 };
@@ -2305,24 +2303,6 @@ pub fn validate_blocks<Block: BlockT>(
 				return Err(BadPeer(*peer_id, rep::BAD_BLOCK));
 			}
 		}
-		if let (Some(header), Some(body)) = (&b.header, &b.body) {
-			let expected = *header.extrinsics_root();
-			let got = HashingFor::<Block>::ordered_trie_root(
-				body.iter().map(Encode::encode).collect(),
-				sp_runtime::StateVersion::V0,
-			);
-			if expected != got {
-				debug!(
-					target: LOG_TARGET,
-					"Bad extrinsic root for a block {} received from {}. Expected {:?}, got {:?}",
-					b.hash,
-					peer_id,
-					expected,
-					got,
-				);
-				return Err(BadPeer(*peer_id, rep::BAD_BLOCK));
-			}
-		}
 	}
 
 	Ok(blocks.first().and_then(|b| b.header.as_ref()).map(|h| *h.number()))
-- 
GitLab