Skip to content

Commit 29a0c4a

Browse files
sistemdgithub-actions[bot]iulianbarbu
authored
store headers and justifications during warp sync (#9424)
Closes #2738. Still need to add tests for this - but I think the easiest way might be after the zombienet tests are converted to Rust, in the warp sync test maybe we can just request the headers (and justifications?) from JSON-RPC? Though I'm not sure there is an API for the justifications. But in any case we can in theory make a P2P justifications request as well and the node should be able to respond. Let me know if anybody has some better ideas. --------- Signed-off-by: sistemd <[email protected]> Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Iulian Barbu <[email protected]>
1 parent 5bb3afc commit 29a0c4a

File tree

6 files changed

+170
-24
lines changed

6 files changed

+170
-24
lines changed

prdoc/pr_9424.prdoc

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
title: 'feat(warp sync): store headers and justifications during warp sync'
2+
doc:
3+
- audience: Node Operator
4+
description: |-
5+
Store justifications and headers for authority set changes blocks during warp sync to be able to respond to warp sync requests from peers even without gap sync. In a follow-up PR, gap sync will be disabled if block pruning is enabled. This PR ensures that warp sync still works across the network, even on pruned nodes.
6+
crates:
7+
- name: cumulus-client-pov-recovery
8+
bump: patch
9+
- name: sc-client-api
10+
bump: patch
11+
- name: sc-consensus-grandpa
12+
bump: patch
13+
- name: sc-client-db
14+
bump: patch
15+
- name: sc-network-sync
16+
bump: patch
17+
- name: sc-rpc-spec-v2
18+
bump: patch
19+
- name: sc-service
20+
bump: patch
21+
- name: sp-runtime
22+
bump: patch

substrate/client/consensus/grandpa/src/warp_proof.rs

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ use sp_consensus_grandpa::{AuthorityList, SetId, GRANDPA_ENGINE_ID};
3030
use sp_runtime::{
3131
generic::BlockId,
3232
traits::{Block as BlockT, Header as HeaderT, NumberFor, One},
33+
Justifications,
3334
};
3435

3536
use std::{collections::HashMap, sync::Arc};
@@ -311,13 +312,28 @@ where
311312
.ok_or_else(|| "Empty proof".to_string())?;
312313
let (next_set_id, next_authorities) =
313314
proof.verify(set_id, authorities, &self.hard_forks).map_err(Box::new)?;
315+
let justifications = proof
316+
.proofs
317+
.into_iter()
318+
.map(|p| {
319+
let justifications =
320+
Justifications::new(vec![(GRANDPA_ENGINE_ID, p.justification.encode())]);
321+
(p.header, justifications)
322+
})
323+
.collect::<Vec<_>>();
314324
if proof.is_finished {
315-
Ok(VerificationResult::<Block>::Complete(next_set_id, next_authorities, last_header))
325+
Ok(VerificationResult::<Block>::Complete(
326+
next_set_id,
327+
next_authorities,
328+
last_header,
329+
justifications,
330+
))
316331
} else {
317332
Ok(VerificationResult::<Block>::Partial(
318333
next_set_id,
319334
next_authorities,
320335
last_header.hash(),
336+
justifications,
321337
))
322338
}
323339
}

substrate/client/network/sync/src/strategy/warp.rs

Lines changed: 123 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818

1919
//! Warp syncing strategy. Bootstraps chain by downloading warp proofs and state.
2020
21+
use sc_consensus::IncomingBlock;
22+
use sp_consensus::BlockOrigin;
2123
pub use sp_consensus_grandpa::{AuthorityList, SetId};
2224

2325
use crate::{
@@ -61,9 +63,9 @@ pub struct WarpProofRequest<B: BlockT> {
6163
/// Proof verification result.
6264
pub enum VerificationResult<Block: BlockT> {
6365
/// Proof is valid, but the target was not reached.
64-
Partial(SetId, AuthorityList, Block::Hash),
66+
Partial(SetId, AuthorityList, Block::Hash, Vec<(Block::Header, Justifications)>),
6567
/// Target finality is proved.
66-
Complete(SetId, AuthorityList, Block::Header),
68+
Complete(SetId, AuthorityList, Block::Header, Vec<(Block::Header, Justifications)>),
6769
}
6870

6971
/// Warp sync backend. Handles retrieving and verifying warp sync proofs.
@@ -403,20 +405,43 @@ where
403405
return
404406
};
405407

408+
let proof_to_incoming_block =
409+
|(header, justifications): (B::Header, Justifications)| -> IncomingBlock<B> {
410+
IncomingBlock {
411+
hash: header.hash(),
412+
header: Some(header),
413+
body: None,
414+
indexed_body: None,
415+
justifications: Some(justifications),
416+
origin: Some(*peer_id),
417+
// We are still in warp sync, so we don't have the state. This means
418+
// we also can't execute the block.
419+
allow_missing_state: true,
420+
skip_execution: true,
421+
// Shouldn't already exist in the database.
422+
import_existing: false,
423+
state: None,
424+
}
425+
};
426+
406427
match warp_sync_provider.verify(&response, *set_id, authorities.clone()) {
407428
Err(e) => {
408429
debug!(target: LOG_TARGET, "Bad warp proof response: {}", e);
409430
self.actions
410431
.push(SyncingAction::DropPeer(BadPeer(*peer_id, rep::BAD_WARP_PROOF)))
411432
},
412-
Ok(VerificationResult::Partial(new_set_id, new_authorities, new_last_hash)) => {
433+
Ok(VerificationResult::Partial(new_set_id, new_authorities, new_last_hash, proofs)) => {
413434
log::debug!(target: LOG_TARGET, "Verified partial proof, set_id={:?}", new_set_id);
414435
*set_id = new_set_id;
415436
*authorities = new_authorities;
416437
*last_hash = new_last_hash;
417438
self.total_proof_bytes += response.0.len() as u64;
439+
self.actions.push(SyncingAction::ImportBlocks {
440+
origin: BlockOrigin::NetworkInitialSync,
441+
blocks: proofs.into_iter().map(proof_to_incoming_block).collect(),
442+
});
418443
},
419-
Ok(VerificationResult::Complete(new_set_id, _, header)) => {
444+
Ok(VerificationResult::Complete(new_set_id, _, header, proofs)) => {
420445
log::debug!(
421446
target: LOG_TARGET,
422447
"Verified complete proof, set_id={:?}. Continuing with target block download: {} ({}).",
@@ -426,6 +451,10 @@ where
426451
);
427452
self.total_proof_bytes += response.0.len() as u64;
428453
self.phase = Phase::TargetBlock(header);
454+
self.actions.push(SyncingAction::ImportBlocks {
455+
origin: BlockOrigin::NetworkInitialSync,
456+
blocks: proofs.into_iter().map(proof_to_incoming_block).collect(),
457+
});
429458
},
430459
}
431460
}
@@ -744,7 +773,7 @@ mod test {
744773
use crate::{mock::MockBlockDownloader, service::network::NetworkServiceProvider};
745774
use sc_block_builder::BlockBuilderBuilder;
746775
use sp_blockchain::{BlockStatus, Error as BlockchainError, HeaderBackend, Info};
747-
use sp_consensus_grandpa::{AuthorityList, SetId};
776+
use sp_consensus_grandpa::{AuthorityList, SetId, GRANDPA_ENGINE_ID};
748777
use sp_core::H256;
749778
use sp_runtime::traits::{Block as BlockT, Header as HeaderT, NumberFor};
750779
use std::{io::ErrorKind, sync::Arc};
@@ -1230,19 +1259,38 @@ mod test {
12301259

12311260
#[test]
12321261
fn partial_warp_proof_doesnt_advance_phase() {
1233-
let client = mock_client_without_state();
1262+
let client = Arc::new(TestClientBuilder::new().set_no_genesis().build());
12341263
let mut provider = MockWarpSyncProvider::<Block>::new();
12351264
provider
12361265
.expect_current_authorities()
12371266
.once()
12381267
.return_const(AuthorityList::default());
1268+
let target_block = BlockBuilderBuilder::new(&*client)
1269+
.on_parent_block(client.chain_info().best_hash)
1270+
.with_parent_block_number(client.chain_info().best_number)
1271+
.build()
1272+
.unwrap()
1273+
.build()
1274+
.unwrap()
1275+
.block;
1276+
let target_header = target_block.header().clone();
1277+
let justifications = Justifications::new(vec![(GRANDPA_ENGINE_ID, vec![1, 2, 3, 4, 5])]);
12391278
// Warp proof is partial.
1240-
provider.expect_verify().return_once(|_proof, set_id, authorities| {
1241-
Ok(VerificationResult::Partial(set_id, authorities, Hash::random()))
1242-
});
1279+
{
1280+
let target_header = target_header.clone();
1281+
let justifications = justifications.clone();
1282+
provider.expect_verify().return_once(move |_proof, set_id, authorities| {
1283+
Ok(VerificationResult::Partial(
1284+
set_id,
1285+
authorities,
1286+
target_header.hash(),
1287+
vec![(target_header, justifications)],
1288+
))
1289+
});
1290+
}
12431291
let config = WarpSyncConfig::WithProvider(Arc::new(provider));
12441292
let mut warp_sync = WarpSync::new(
1245-
Arc::new(client),
1293+
client,
12461294
config,
12471295
Some(ProtocolName::Static("")),
12481296
Arc::new(MockBlockDownloader::new()),
@@ -1267,7 +1315,29 @@ mod test {
12671315

12681316
warp_sync.on_warp_proof_response(&request_peer_id, EncodedProof(Vec::new()));
12691317

1270-
assert!(warp_sync.actions.is_empty(), "No extra actions generated");
1318+
assert_eq!(warp_sync.actions.len(), 1);
1319+
let SyncingAction::ImportBlocks { origin, mut blocks } = warp_sync.actions.pop().unwrap()
1320+
else {
1321+
panic!("Expected `ImportBlocks` action.");
1322+
};
1323+
assert_eq!(origin, BlockOrigin::NetworkInitialSync);
1324+
assert_eq!(blocks.len(), 1);
1325+
let import_block = blocks.pop().unwrap();
1326+
assert_eq!(
1327+
import_block,
1328+
IncomingBlock {
1329+
hash: target_header.hash(),
1330+
header: Some(target_header),
1331+
body: None,
1332+
indexed_body: None,
1333+
justifications: Some(justifications),
1334+
origin: Some(request_peer_id),
1335+
allow_missing_state: true,
1336+
skip_execution: true,
1337+
import_existing: false,
1338+
state: None,
1339+
}
1340+
);
12711341
assert!(matches!(warp_sync.phase, Phase::WarpProof { .. }));
12721342
}
12731343

@@ -1288,10 +1358,20 @@ mod test {
12881358
.unwrap()
12891359
.block;
12901360
let target_header = target_block.header().clone();
1361+
let justifications = Justifications::new(vec![(GRANDPA_ENGINE_ID, vec![1, 2, 3, 4, 5])]);
12911362
// Warp proof is complete.
1292-
provider.expect_verify().return_once(move |_proof, set_id, authorities| {
1293-
Ok(VerificationResult::Complete(set_id, authorities, target_header))
1294-
});
1363+
{
1364+
let target_header = target_header.clone();
1365+
let justifications = justifications.clone();
1366+
provider.expect_verify().return_once(move |_proof, set_id, authorities| {
1367+
Ok(VerificationResult::Complete(
1368+
set_id,
1369+
authorities,
1370+
target_header.clone(),
1371+
vec![(target_header, justifications)],
1372+
))
1373+
});
1374+
}
12951375
let config = WarpSyncConfig::WithProvider(Arc::new(provider));
12961376
let mut warp_sync = WarpSync::new(
12971377
client,
@@ -1319,7 +1399,29 @@ mod test {
13191399

13201400
warp_sync.on_warp_proof_response(&request_peer_id, EncodedProof(Vec::new()));
13211401

1322-
assert!(warp_sync.actions.is_empty(), "No extra actions generated.");
1402+
assert_eq!(warp_sync.actions.len(), 1);
1403+
let SyncingAction::ImportBlocks { origin, mut blocks } = warp_sync.actions.pop().unwrap()
1404+
else {
1405+
panic!("Expected `ImportBlocks` action.");
1406+
};
1407+
assert_eq!(origin, BlockOrigin::NetworkInitialSync);
1408+
assert_eq!(blocks.len(), 1);
1409+
let import_block = blocks.pop().unwrap();
1410+
assert_eq!(
1411+
import_block,
1412+
IncomingBlock {
1413+
hash: target_header.hash(),
1414+
header: Some(target_header),
1415+
body: None,
1416+
indexed_body: None,
1417+
justifications: Some(justifications),
1418+
origin: Some(request_peer_id),
1419+
allow_missing_state: true,
1420+
skip_execution: true,
1421+
import_existing: false,
1422+
state: None,
1423+
}
1424+
);
13231425
assert!(
13241426
matches!(warp_sync.phase, Phase::TargetBlock(header) if header == *target_block.header())
13251427
);
@@ -1372,7 +1474,7 @@ mod test {
13721474
let target_header = target_block.header().clone();
13731475
// Warp proof is complete.
13741476
provider.expect_verify().return_once(move |_proof, set_id, authorities| {
1375-
Ok(VerificationResult::Complete(set_id, authorities, target_header))
1477+
Ok(VerificationResult::Complete(set_id, authorities, target_header, Default::default()))
13761478
});
13771479
let config = WarpSyncConfig::WithProvider(Arc::new(provider));
13781480
let mut warp_sync =
@@ -1446,7 +1548,7 @@ mod test {
14461548
let target_header = target_block.header().clone();
14471549
// Warp proof is complete.
14481550
provider.expect_verify().return_once(move |_proof, set_id, authorities| {
1449-
Ok(VerificationResult::Complete(set_id, authorities, target_header))
1551+
Ok(VerificationResult::Complete(set_id, authorities, target_header, Default::default()))
14501552
});
14511553
let config = WarpSyncConfig::WithProvider(Arc::new(provider));
14521554
let mut warp_sync =
@@ -1485,7 +1587,7 @@ mod test {
14851587
let target_header = target_block.header().clone();
14861588
// Warp proof is complete.
14871589
provider.expect_verify().return_once(move |_proof, set_id, authorities| {
1488-
Ok(VerificationResult::Complete(set_id, authorities, target_header))
1590+
Ok(VerificationResult::Complete(set_id, authorities, target_header, Default::default()))
14891591
});
14901592
let config = WarpSyncConfig::WithProvider(Arc::new(provider));
14911593
let mut warp_sync =
@@ -1540,7 +1642,7 @@ mod test {
15401642
let target_header = target_block.header().clone();
15411643
// Warp proof is complete.
15421644
provider.expect_verify().return_once(move |_proof, set_id, authorities| {
1543-
Ok(VerificationResult::Complete(set_id, authorities, target_header))
1645+
Ok(VerificationResult::Complete(set_id, authorities, target_header, Default::default()))
15441646
});
15451647
let config = WarpSyncConfig::WithProvider(Arc::new(provider));
15461648
let mut warp_sync =
@@ -1618,7 +1720,7 @@ mod test {
16181720
let target_header = target_block.header().clone();
16191721
// Warp proof is complete.
16201722
provider.expect_verify().return_once(move |_proof, set_id, authorities| {
1621-
Ok(VerificationResult::Complete(set_id, authorities, target_header))
1723+
Ok(VerificationResult::Complete(set_id, authorities, target_header, Default::default()))
16221724
});
16231725
let config = WarpSyncConfig::WithProvider(Arc::new(provider));
16241726
let mut warp_sync =
@@ -1672,7 +1774,7 @@ mod test {
16721774
let target_header = target_block.header().clone();
16731775
// Warp proof is complete.
16741776
provider.expect_verify().return_once(move |_proof, set_id, authorities| {
1675-
Ok(VerificationResult::Complete(set_id, authorities, target_header))
1777+
Ok(VerificationResult::Complete(set_id, authorities, target_header, Default::default()))
16761778
});
16771779
let config = WarpSyncConfig::WithProvider(Arc::new(provider));
16781780
let mut warp_sync =

substrate/client/network/test/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -673,7 +673,7 @@ impl<B: BlockT> WarpSyncProvider<B> for TestWarpSyncProvider<B> {
673673
) -> Result<VerificationResult<B>, Box<dyn std::error::Error + Send + Sync>> {
674674
let EncodedProof(encoded) = proof;
675675
let header = B::Header::decode(&mut encoded.as_slice()).unwrap();
676-
Ok(VerificationResult::Complete(0, Default::default(), header))
676+
Ok(VerificationResult::Complete(0, Default::default(), header, Default::default()))
677677
}
678678
fn current_authorities(&self) -> AuthorityList {
679679
Default::default()

substrate/client/rpc-spec-v2/src/chain_head/test_utils.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,7 @@ impl<Block: BlockT, Client: BlockBackend<Block>> BlockBackend<Block>
289289
fn block_indexed_body(&self, hash: Block::Hash) -> sp_blockchain::Result<Option<Vec<Vec<u8>>>> {
290290
self.client.block_indexed_body(hash)
291291
}
292+
292293
fn requires_full_sync(&self) -> bool {
293294
self.client.requires_full_sync()
294295
}

substrate/primitives/runtime/src/lib.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,10 +156,15 @@ pub type EncodedJustification = Vec<u8>;
156156
/// Collection of justifications for a given block, multiple justifications may
157157
/// be provided by different consensus engines for the same block.
158158
#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
159-
#[derive(Debug, Clone, PartialEq, Eq, Encode, Decode)]
159+
#[derive(Default, Debug, Clone, PartialEq, Eq, Encode, Decode)]
160160
pub struct Justifications(Vec<Justification>);
161161

162162
impl Justifications {
163+
/// Create a new `Justifications` instance with the given justifications.
164+
pub fn new(justifications: Vec<Justification>) -> Self {
165+
Self(justifications)
166+
}
167+
163168
/// Return an iterator over the justifications.
164169
pub fn iter(&self) -> impl Iterator<Item = &Justification> {
165170
self.0.iter()

0 commit comments

Comments
 (0)