diff --git a/substrate/client/network/src/block_request_handler.rs b/substrate/client/network/src/block_request_handler.rs
index 332635dbe79018d2af0901b9d3191ab1e4162a38..633b6b5935edc3224ad6c2cae7c0df2cfe667b72 100644
--- a/substrate/client/network/src/block_request_handler.rs
+++ b/substrate/client/network/src/block_request_handler.rs
@@ -80,6 +80,7 @@ struct SeenRequestsKey<B: BlockT> {
max_blocks: usize,
direction: Direction,
attributes: BlockAttributes,
+ support_multiple_justifications: bool,
}
impl<B: BlockT> Hash for SeenRequestsKey<B> {
@@ -180,12 +181,15 @@ impl<B: BlockT> BlockRequestHandler<B> {
let attributes = BlockAttributes::from_be_u32(request.fields)?;
+ let support_multiple_justifications = request.support_multiple_justifications;
+
let key = SeenRequestsKey {
peer: *peer,
max_blocks,
direction,
from: from_block_id.clone(),
attributes,
+ support_multiple_justifications,
};
let mut reputation_changes = Vec::new();
@@ -221,6 +225,7 @@ impl<B: BlockT> BlockRequestHandler<B> {
from_block_id,
direction,
max_blocks,
+ support_multiple_justifications,
)?;
// If any of the blocks contains nay data, we can consider it as successful request.
@@ -259,6 +264,7 @@ impl<B: BlockT> BlockRequestHandler<B> {
mut block_id: BlockId<B>,
direction: Direction,
max_blocks: usize,
+ support_multiple_justifications: bool,
) -> Result<BlockResponse, HandleRequestError> {
let get_header = attributes.contains(BlockAttributes::HEADER);
let get_body = attributes.contains(BlockAttributes::BODY);
@@ -277,22 +283,33 @@ impl<B: BlockT> BlockRequestHandler<B> {
None
};
- // TODO: In a follow up PR tracked by https://github.com/paritytech/substrate/issues/8172
- // we want to send/receive all justifications.
- // For now we keep compatibility by selecting precisely the GRANDPA one, and not just
- // the first one. When sending we could have just taken the first one, since we don't
- // expect there to be any other kind currently, but when receiving we need to add the
- // engine ID tag.
- // The ID tag is hardcoded here to avoid depending on the GRANDPA crate, and will be
- // removed when resolving the above issue.
- let justification = justifications.and_then(|just| just.into_justification(*b"FRNK"));
-
- let is_empty_justification = justification
- .as_ref()
- .map(|j| j.is_empty())
- .unwrap_or(false);
-
- let justification = justification.unwrap_or_default();
+ let (justifications, justification, is_empty_justification) =
+ if support_multiple_justifications {
+ let justifications = match justifications {
+ Some(v) => v.encode(),
+ None => Vec::new(),
+ };
+ (justifications, Vec::new(), false)
+ } else {
+ // For now we keep compatibility by selecting precisely the GRANDPA one, and not just
+ // the first one. When sending we could have just taken the first one, since we don't
+ // expect there to be any other kind currently, but when receiving we need to add the
+ // engine ID tag.
+ // The ID tag is hardcoded here to avoid depending on the GRANDPA crate, and will be
+ // removed once we remove the backwards compatibility.
+ // See: https://github.com/paritytech/substrate/issues/8172
+ let justification =
+ justifications.and_then(|just| just.into_justification(*b"FRNK"));
+
+ let is_empty_justification = justification
+ .as_ref()
+ .map(|j| j.is_empty())
+ .unwrap_or(false);
+
+ let justification = justification.unwrap_or_default();
+
+ (Vec::new(), justification, is_empty_justification)
+ };
let body = if get_body {
match self.client.block_body(&BlockId::Hash(hash))? {
@@ -320,6 +337,7 @@ impl<B: BlockT> BlockRequestHandler<B> {
message_queue: Vec::new(),
justification,
is_empty_justification,
+ justifications,
};
total_size += block_data.body.len();
diff --git a/substrate/client/network/src/light_client_requests/sender.rs b/substrate/client/network/src/light_client_requests/sender.rs
index 652f465d6f2503c214f9eb098f7daba9abbf686b..bf832ea13aedfe81b279ba7e1a9c25822aad24d6 100644
--- a/substrate/client/network/src/light_client_requests/sender.rs
+++ b/substrate/client/network/src/light_client_requests/sender.rs
@@ -722,6 +722,7 @@ impl<B: Block> Request<B> {
to_block: Default::default(),
direction: schema::v1::Direction::Ascending as i32,
max_blocks: 1,
+ support_multiple_justifications: true,
};
let mut buf = Vec::with_capacity(rq.encoded_len());
diff --git a/substrate/client/network/src/protocol.rs b/substrate/client/network/src/protocol.rs
index 94980947e2a856447bbb6fd103176da1baccc9a5..ff64b9d599c04162b7088ad45e6acaf90a2d3207 100644
--- a/substrate/client/network/src/protocol.rs
+++ b/substrate/client/network/src/protocol.rs
@@ -580,6 +580,11 @@ impl<B: BlockT> Protocol<B> {
} else {
None
},
+ justifications: if !block_data.justifications.is_empty() {
+ Some(DecodeAll::decode_all(&mut block_data.justifications.as_ref())?)
+ } else {
+ None
+ },
})
}).collect::<Result<Vec<_>, codec::Error>>();
@@ -908,6 +913,7 @@ impl<B: BlockT> Protocol<B> {
receipt: None,
message_queue: None,
justification: None,
+ justifications: None,
},
],
},
@@ -1123,6 +1129,7 @@ fn prepare_block_request<B: BlockT>(
to_block: request.to.map(|h| h.encode()).unwrap_or_default(),
direction: request.direction as i32,
max_blocks: request.max.unwrap_or(0),
+ support_multiple_justifications: true,
};
CustomMessageOutcome::BlockRequest {
diff --git a/substrate/client/network/src/protocol/message.rs b/substrate/client/network/src/protocol/message.rs
index 7564804400fb301bd6994e09d3f134dba2ab22c6..dc6beac99aa0127dbf047db519acac122c0b51b2 100644
--- a/substrate/client/network/src/protocol/message.rs
+++ b/substrate/client/network/src/protocol/message.rs
@@ -168,7 +168,7 @@ impl<H: HeaderT> generic::BlockAnnounce<H> {
pub mod generic {
use bitflags::bitflags;
use codec::{Encode, Decode, Input, Output};
- use sp_runtime::EncodedJustification;
+ use sp_runtime::{EncodedJustification, Justifications};
use super::{
RemoteReadResponse, Transactions, Direction,
RequestId, BlockAttributes, RemoteCallResponse, ConsensusEngineId,
@@ -254,6 +254,8 @@ pub mod generic {
pub message_queue: Option<Vec<u8>>,
/// Justification if requested.
pub justification: Option<EncodedJustification>,
+ /// Justifications if requested.
+ pub justifications: Option<Justifications>,
}
/// Identifies starting point of a block sequence.
diff --git a/substrate/client/network/src/protocol/sync.rs b/substrate/client/network/src/protocol/sync.rs
index 6e07bd4c9697149beda8885527c4b71edb8adb6e..d3ab2912a9dc5a237b9ca2ed1b24af7209339fa6 100644
--- a/substrate/client/network/src/protocol/sync.rs
+++ b/substrate/client/network/src/protocol/sync.rs
@@ -833,8 +833,9 @@ impl<B: BlockT> ChainSync<B> {
.drain(self.best_queued_number + One::one())
.into_iter()
.map(|block_data| {
- let justifications =
- legacy_justification_mapping(block_data.block.justification);
+ let justifications = block_data.block.justifications.or(
+ legacy_justification_mapping(block_data.block.justification)
+ );
IncomingBlock {
hash: block_data.block.hash,
header: block_data.block.header,
@@ -854,11 +855,14 @@ impl<B: BlockT> ChainSync<B> {
}
validate_blocks::<B>(&blocks, who, Some(request))?;
blocks.into_iter().map(|b| {
+ let justifications = b.justifications.or(
+ legacy_justification_mapping(b.justification)
+ );
IncomingBlock {
hash: b.hash,
header: b.header,
body: b.body,
- justifications: legacy_justification_mapping(b.justification),
+ justifications,
origin: Some(who.clone()),
allow_missing_state: true,
import_existing: false,
@@ -963,11 +967,14 @@ impl<B: BlockT> ChainSync<B> {
// When request.is_none() this is a block announcement. Just accept blocks.
validate_blocks::<B>(&blocks, who, None)?;
blocks.into_iter().map(|b| {
+ let justifications = b.justifications.or(
+ legacy_justification_mapping(b.justification)
+ );
IncomingBlock {
hash: b.hash,
header: b.header,
body: b.body,
- justifications: legacy_justification_mapping(b.justification),
+ justifications,
origin: Some(who.clone()),
allow_missing_state: true,
import_existing: false,
@@ -1043,7 +1050,7 @@ impl<B: BlockT> ChainSync<B> {
return Err(BadPeer(who, rep::BAD_JUSTIFICATION));
}
- block.justification
+ block.justifications.or(legacy_justification_mapping(block.justification))
} else {
// we might have asked the peer for a justification on a block that we assumed it
// had but didn't (regardless of whether it had a justification for it or not).
@@ -1058,7 +1065,7 @@ impl<B: BlockT> ChainSync<B> {
if let Some((peer, hash, number, j)) = self
.extra_justifications
- .on_response(who, legacy_justification_mapping(justification))
+ .on_response(who, justification)
{
return Ok(OnBlockJustification::Import { peer, hash, number, justifications: j })
}
@@ -1655,7 +1662,7 @@ impl<B: BlockT> ChainSync<B> {
// This is purely during a backwards compatible transitionary period and should be removed
// once we can assume all nodes can send and receive multiple Justifications
// The ID tag is hardcoded here to avoid depending on the GRANDPA crate.
-// TODO: https://github.com/paritytech/substrate/issues/8172
+// See: https://github.com/paritytech/substrate/issues/8172
fn legacy_justification_mapping(justification: Option<EncodedJustification>) -> Option<Justifications> {
justification.map(|just| (*b"FRNK", just).into())
}
@@ -2163,6 +2170,7 @@ mod test {
receipt: None,
message_queue: None,
justification: None,
+ justifications: None,
}
).collect(),
}
diff --git a/substrate/client/network/src/protocol/sync/blocks.rs b/substrate/client/network/src/protocol/sync/blocks.rs
index 60492f24ed8c3a333aa71ff43c2d3da0c03cd011..81f9cffacaab4917d4db640caf5896be102292fb 100644
--- a/substrate/client/network/src/protocol/sync/blocks.rs
+++ b/substrate/client/network/src/protocol/sync/blocks.rs
@@ -228,6 +228,7 @@ mod test {
message_queue: None,
receipt: None,
justification: None,
+ justifications: None,
}).collect()
}
diff --git a/substrate/client/network/src/schema/api.v1.proto b/substrate/client/network/src/schema/api.v1.proto
index a933c5811c109616c7adec903b17293ddebaeab7..23d585b05e9cda606f2f7eee7299f9c5ffe9c6b2 100644
--- a/substrate/client/network/src/schema/api.v1.proto
+++ b/substrate/client/network/src/schema/api.v1.proto
@@ -29,6 +29,10 @@ message BlockRequest {
Direction direction = 5;
// Maximum number of blocks to return. An implementation defined maximum is used when unspecified.
uint32 max_blocks = 6; // optional
+ // Indicate to the receiver that we support multiple justifications. If the responder also
+ // supports this it will populate the multiple justifications field in `BlockData` instead of
+ // the single justification field.
+ bool support_multiple_justifications = 7; // optional
}
// Response to `BlockRequest`
@@ -56,5 +60,11 @@ message BlockData {
// doesn't make in possible to differentiate between a lack of justification and an empty
// justification.
bool is_empty_justification = 7; // optional, false if absent
+ // Justifications if requested.
+ // Unlike the field for a single justification, this field does not required an associated
+ // boolean to differentiate between the lack of justifications and empty justification(s). This
+ // is because empty justifications, like all justifications, are paired with a non-empty
+ // consensus engine ID.
+ bytes justifications = 8; // optional
}