From 0bcb82e30af5d8bddb9831a3b8b6b1b3d017567c Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Thu, 29 Sep 2022 07:53:43 +0200 Subject: [PATCH 1/2] No longer discard justifications of blocks that were already known --- bin/wasm-node/CHANGELOG.md | 4 ++++ src/sync/all_forks.rs | 24 ++++++++++++++++++------ 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/bin/wasm-node/CHANGELOG.md b/bin/wasm-node/CHANGELOG.md index c2a8eb7ad5..41e72dcb0c 100644 --- a/bin/wasm-node/CHANGELOG.md +++ b/bin/wasm-node/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Fixed + +- No longer silently discard justifications when receive a block from the network that was already known locally. + ## 0.7.0 - 2022-09-28 ### Removed diff --git a/src/sync/all_forks.rs b/src/sync/all_forks.rs index 071b8dfb35..67ac7772a3 100644 --- a/src/sync/all_forks.rs +++ b/src/sync/all_forks.rs @@ -1129,6 +1129,12 @@ impl FinishAncestrySearch { return Err((AncestrySearchResponseError::TooOld, self.finish())); } + // Convert the justifications in an "owned" format, because we're likely going to store + // them. + let justifications = scale_encoded_justifications + .map(|(e, j)| (e, j.as_ref().to_owned())) + .collect::>(); + // If the block is already part of the local tree of blocks, nothing more to do. if self .inner @@ -1139,6 +1145,7 @@ impl FinishAncestrySearch { inner: self, decoded_header: decoded_header.into(), is_verified: true, + justifications, })); } @@ -1171,15 +1178,14 @@ impl FinishAncestrySearch { Ok(AddBlock::UnknownBlock(AddBlockVacant { inner: self, decoded_header: decoded_header.into(), - justifications: scale_encoded_justifications - .map(|(e, j)| (e, j.as_ref().to_owned())) - .collect::>(), + justifications, })) } else { Ok(AddBlock::AlreadyPending(AddBlockOccupied { inner: self, decoded_header: decoded_header.into(), is_verified: false, + justifications, })) } } @@ -1231,6 +1237,7 @@ pub struct AddBlockOccupied { inner: FinishAncestrySearch, decoded_header: header::Header, is_verified: bool, + justifications: Vec<([u8; 4], Vec)>, } impl AddBlockOccupied { @@ -1319,9 +1326,14 @@ impl AddBlockOccupied { mem::replace(&mut block_user_data.user_data, user_data) }; - // TODO: what if the pending block already contains a justification and it is not the - // same as here? since justifications aren't immediately verified, it is possible - // for a malicious peer to send us bad justifications + if !self.justifications.is_empty() { + self.inner.inner.inner.blocks[self.inner.source_id] + .unverified_finality_proofs + .insert( + self.decoded_header.number, + FinalityProofs::Justifications(self.justifications), + ); + } // Update the state machine for the next iteration. // Note: this can't be reached if `expected_next_height` is 0, because that should have From 40b2d117049150c563d11f27623612a1e798fafa Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Thu, 29 Sep 2022 07:56:31 +0200 Subject: [PATCH 2/2] PR number --- bin/wasm-node/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/wasm-node/CHANGELOG.md b/bin/wasm-node/CHANGELOG.md index 41e72dcb0c..df13247e98 100644 --- a/bin/wasm-node/CHANGELOG.md +++ b/bin/wasm-node/CHANGELOG.md @@ -4,7 +4,7 @@ ### Fixed -- No longer silently discard justifications when receive a block from the network that was already known locally. +- No longer silently discard justifications when receive a block from the network that was already known locally. ([#2800](https://github.com/paritytech/smoldot/pull/2800)) ## 0.7.0 - 2022-09-28