Skip to content

Commit 211364d

Browse files
authored
Check that downloaded sender chain blocks are relevant. (#4026)
## Motivation When trying to find "received certificates", faulty validators might send us certificates that didn't actually send anything to the chain in question, causing the client to unnecessarily process valid but irrelevant blocks. ## Proposal Check that they actually sent something. ## Test Plan Since this only defends against some spam, isn't critical for security, and testing it would require considerably extending the test validators, I suggest not adding a test for this. CI will check if it _breaks_ anything (i.e. yields false positives). ## Release Plan - Nothing to do / These changes follow the usual release cycle. ## Links - Closes #4025. - [reviewer checklist](https://github.com/linera-io/linera-protocol/blob/main/CONTRIBUTING.md#reviewer-checklist)
1 parent 27929e1 commit 211364d

File tree

1 file changed

+24
-8
lines changed

1 file changed

+24
-8
lines changed

linera-core/src/client/mod.rs

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -803,9 +803,9 @@ impl<Env: Environment> Client<Env> {
803803
let remote_certificates = remote_node
804804
.download_certificates(certificate_hashes)
805805
.await?;
806+
let mut certificates_by_height_by_chain = BTreeMap::new();
806807

807808
// Check the signatures and keep only the ones that are valid.
808-
let mut certificates = Vec::new();
809809
for confirmed_block_certificate in remote_certificates {
810810
let block_header = &confirmed_block_certificate.inner().block().header;
811811
let sender_chain_id = block_header.chain_id;
@@ -828,31 +828,47 @@ impl<Env: Environment> Client<Env> {
828828
warn!("Skipping received certificate from past epoch {epoch:?}");
829829
}
830830
CheckCertificateResult::New => {
831-
downloaded_heights
831+
certificates_by_height_by_chain
832832
.entry(sender_chain_id)
833-
.and_modify(|h| *h = height.max(*h))
834-
.or_insert(height);
835-
certificates.push(confirmed_block_certificate);
833+
.or_insert_with(BTreeMap::new)
834+
.insert(height, confirmed_block_certificate.clone());
836835
}
837836
}
838837
}
839838

840839
// Increase the tracker up to the first position we haven't downloaded.
841840
for entry in remote_log {
842-
if downloaded_heights
841+
if certificates_by_height_by_chain
843842
.get(&entry.chain_id)
844-
.is_some_and(|h| *h >= entry.height)
843+
.is_some_and(|certs| certs.contains_key(&entry.height))
845844
{
846845
tracker += 1;
847846
} else {
848847
break;
849848
}
850849
}
851850

851+
for (sender_chain_id, certs) in &mut certificates_by_height_by_chain {
852+
if !certs
853+
.values()
854+
.last()
855+
.is_some_and(|cert| cert.block().recipients().contains(&chain_id))
856+
{
857+
warn!(
858+
"Skipping received certificates from chain {sender_chain_id:.8}:
859+
No messages for {chain_id:.8}."
860+
);
861+
certs.clear();
862+
}
863+
}
864+
852865
Ok(ReceivedCertificatesFromValidator {
853866
public_key: remote_node.public_key,
854867
tracker,
855-
certificates,
868+
certificates: certificates_by_height_by_chain
869+
.into_values()
870+
.flat_map(BTreeMap::into_values)
871+
.collect(),
856872
other_sender_chains,
857873
})
858874
}

0 commit comments

Comments
 (0)