From b1514156a20d6b91119f514eff70c7c7631e2bb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Thu, 12 Sep 2024 10:52:36 +0200 Subject: [PATCH 1/3] sync: Remove checking of the extrinsics root 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. --- .../network/sync/src/strategy/chain_sync.rs | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/substrate/client/network/sync/src/strategy/chain_sync.rs b/substrate/client/network/sync/src/strategy/chain_sync.rs index a8ba5558d1bc..9993c6d5e59c 100644 --- a/substrate/client/network/sync/src/strategy/chain_sync.rs +++ b/substrate/client/network/sync/src/strategy/chain_sync.rs @@ -2285,24 +2285,6 @@ pub fn validate_blocks( 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::::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())) From 48109aca7e295b73355dd8513989dd0a08f9fdb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Thu, 12 Sep 2024 14:38:36 +0200 Subject: [PATCH 2/3] PRDOC --- prdoc/pr_5686.prdoc | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 prdoc/pr_5686.prdoc diff --git a/prdoc/pr_5686.prdoc b/prdoc/pr_5686.prdoc new file mode 100644 index 000000000000..3f0da912a34c --- /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 From 14db308aa4c4c610c51cb17018c744432aa9f1cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Tue, 24 Sep 2024 15:44:14 +0200 Subject: [PATCH 3/3] Make clippy happy --- substrate/client/network/sync/src/strategy/chain_sync.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/substrate/client/network/sync/src/strategy/chain_sync.rs b/substrate/client/network/sync/src/strategy/chain_sync.rs index 9993c6d5e59c..bc8b5b9377b0 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}; @@ -56,8 +55,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, };