Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No longer discard justifications of blocks that were already known #2800

Merged
merged 3 commits into from
Oct 4, 2022
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions bin/wasm-node/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -5,6 +5,7 @@
### Fixed

- Syncing no longer stalls if the gap between the finalized and latest block is more than 100 blocks. ([#2801](https://github.com/paritytech/smoldot/pull/2801))
- 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))
- CPU-heavy operations such as verifying finality proofs or compiling the runtime will now better respect the CPU rate limit. ([#2803](https://github.com/paritytech/smoldot/pull/2803))

## 0.7.0 - 2022-09-28
24 changes: 18 additions & 6 deletions src/sync/all_forks.rs
Original file line number Diff line number Diff line change
@@ -1129,6 +1129,12 @@ impl<TBl, TRq, TSrc> FinishAncestrySearch<TBl, TRq, TSrc> {
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::<Vec<_>>();

// If the block is already part of the local tree of blocks, nothing more to do.
if self
.inner
@@ -1139,6 +1145,7 @@ impl<TBl, TRq, TSrc> FinishAncestrySearch<TBl, TRq, TSrc> {
inner: self,
decoded_header: decoded_header.into(),
is_verified: true,
justifications,
}));
}

@@ -1171,15 +1178,14 @@ impl<TBl, TRq, TSrc> FinishAncestrySearch<TBl, TRq, TSrc> {
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::<Vec<_>>(),
justifications,
}))
} else {
Ok(AddBlock::AlreadyPending(AddBlockOccupied {
inner: self,
decoded_header: decoded_header.into(),
is_verified: false,
justifications,
}))
}
}
@@ -1231,6 +1237,7 @@ pub struct AddBlockOccupied<TBl, TRq, TSrc> {
inner: FinishAncestrySearch<TBl, TRq, TSrc>,
decoded_header: header::Header,
is_verified: bool,
justifications: Vec<([u8; 4], Vec<u8>)>,
}

impl<TBl, TRq, TSrc> AddBlockOccupied<TBl, TRq, TSrc> {
@@ -1319,9 +1326,14 @@ impl<TBl, TRq, TSrc> AddBlockOccupied<TBl, TRq, TSrc> {
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