From 76d3814e907826b1b73fe86fd95606f80706a492 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bastian=20K=C3=B6cher?= <bkchr@users.noreply.github.com>
Date: Wed, 24 Feb 2021 08:03:05 +0100
Subject: [PATCH] Make `on_slot` return the block with the post header (#8188)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* Make `on_slot` return the block with the post header
Before this pr `on_slot` returned the pre block. However this is wrong,
because adding some post digest changes the hash of the header. Thus,
we need to make sure to return the correct block that uses the post
header.
* Update primitives/consensus/common/src/block_import.rs
Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com>
Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com>
---
substrate/client/consensus/aura/src/lib.rs | 53 +++++++++++++++++--
substrate/client/consensus/slots/src/lib.rs | 7 +--
.../consensus/common/src/block_import.rs | 23 ++++----
3 files changed, 68 insertions(+), 15 deletions(-)
diff --git a/substrate/client/consensus/aura/src/lib.rs b/substrate/client/consensus/aura/src/lib.rs
index 47ce364cb66..29c4a401551 100644
--- a/substrate/client/consensus/aura/src/lib.rs
+++ b/substrate/client/consensus/aura/src/lib.rs
@@ -683,7 +683,7 @@ impl<B: BlockT, C, P, CAW> Verifier<B> for AuraVerifier<C, P, CAW> where
}
fn initialize_authorities_cache<A, B, C>(client: &C) -> Result<(), ConsensusError> where
- A: Codec,
+ A: Codec + Debug,
B: BlockT,
C: ProvideRuntimeApi<B> + BlockOf + ProvideCache<B>,
C::Api: AuraApi<B, A>,
@@ -719,7 +719,7 @@ fn initialize_authorities_cache<A, B, C>(client: &C) -> Result<(), ConsensusErro
#[allow(deprecated)]
fn authorities<A, B, C>(client: &C, at: &BlockId<B>) -> Result<Vec<A>, ConsensusError> where
- A: Codec,
+ A: Codec + Debug,
B: BlockT,
C: ProvideRuntimeApi<B> + BlockOf + ProvideCache<B>,
C::Api: AuraApi<B, A>,
@@ -886,7 +886,7 @@ mod tests {
use sc_client_api::BlockchainEvents;
use sp_consensus_aura::sr25519::AuthorityPair;
use sc_consensus_slots::{SimpleSlotWorker, BackoffAuthoringOnFinalizedHeadLagging};
- use std::task::Poll;
+ use std::{task::Poll, time::Instant};
use sc_block_builder::BlockBuilderProvider;
use sp_runtime::traits::Header as _;
use substrate_test_runtime_client::{TestClient, runtime::{Header, H256}};
@@ -1124,4 +1124,51 @@ mod tests {
assert!(worker.claim_slot(&head, 6.into(), &authorities).is_none());
assert!(worker.claim_slot(&head, 7.into(), &authorities).is_some());
}
+
+ #[test]
+ fn on_slot_returns_correct_block() {
+ let net = AuraTestNet::new(4);
+
+ let keystore_path = tempfile::tempdir().expect("Creates keystore path");
+ let keystore = LocalKeystore::open(keystore_path.path(), None)
+ .expect("Creates keystore.");
+ SyncCryptoStore::sr25519_generate_new(
+ &keystore,
+ AuthorityPair::ID, Some(&Keyring::Alice.to_seed()),
+ ).expect("Key should be created");
+
+ let net = Arc::new(Mutex::new(net));
+
+ let mut net = net.lock();
+ let peer = net.peer(3);
+ let client = peer.client().as_full().expect("full clients are created").clone();
+ let environ = DummyFactory(client.clone());
+
+ let mut worker = AuraWorker {
+ client: client.clone(),
+ block_import: Arc::new(Mutex::new(client.clone())),
+ env: environ,
+ keystore: keystore.into(),
+ sync_oracle: DummyOracle.clone(),
+ force_authoring: false,
+ backoff_authoring_blocks: Option::<()>::None,
+ _key_type: PhantomData::<AuthorityPair>,
+ };
+
+ let head = client.header(&BlockId::Number(0)).unwrap().unwrap();
+
+ let res = futures::executor::block_on(worker.on_slot(
+ head,
+ SlotInfo {
+ slot: 0.into(),
+ timestamp: 0,
+ ends_at: Instant::now() + Duration::from_secs(100),
+ inherent_data: InherentData::new(),
+ duration: 1000,
+ },
+ )).unwrap();
+
+ // The returned block should be imported and we should be able to get its header by now.
+ assert!(client.header(&BlockId::Hash(res.block.hash())).unwrap().is_some());
+ }
}
diff --git a/substrate/client/consensus/slots/src/lib.rs b/substrate/client/consensus/slots/src/lib.rs
index 62b6b452eb4..1df8378d514 100644
--- a/substrate/client/consensus/slots/src/lib.rs
+++ b/substrate/client/consensus/slots/src/lib.rs
@@ -334,7 +334,7 @@ pub trait SimpleSlotWorker<B: BlockT> {
proposal_work.and_then(move |(proposal, claim)| async move {
let (block, storage_proof) = (proposal.block, proposal.proof);
- let (header, body) = block.clone().deconstruct();
+ let (header, body) = block.deconstruct();
let header_num = *header.number();
let header_hash = header.hash();
let parent_hash = *header.parent_hash();
@@ -342,7 +342,7 @@ pub trait SimpleSlotWorker<B: BlockT> {
let block_import_params = block_import_params_maker(
header,
&header_hash,
- body,
+ body.clone(),
proposal.storage_changes,
claim,
epoch_data,
@@ -361,6 +361,7 @@ pub trait SimpleSlotWorker<B: BlockT> {
"hash_previously" => ?header_hash,
);
+ let header = block_import_params.post_header();
if let Err(err) = block_import.lock().import_block(block_import_params, Default::default()) {
warn!(
target: logging_target,
@@ -376,7 +377,7 @@ pub trait SimpleSlotWorker<B: BlockT> {
);
}
- Ok(SlotResult { block, storage_proof })
+ Ok(SlotResult { block: B::new(header, body), storage_proof })
}).then(|r| async move {
r.map_err(|e| warn!(target: "slots", "Encountered consensus error: {:?}", e)).ok()
}).boxed()
diff --git a/substrate/primitives/consensus/common/src/block_import.rs b/substrate/primitives/consensus/common/src/block_import.rs
index 41b5f391f65..00f84501dbb 100644
--- a/substrate/primitives/consensus/common/src/block_import.rs
+++ b/substrate/primitives/consensus/common/src/block_import.rs
@@ -193,16 +193,21 @@ impl<Block: BlockT, Transaction> BlockImportParams<Block, Transaction> {
if let Some(hash) = self.post_hash {
hash
} else {
- if self.post_digests.is_empty() {
- self.header.hash()
- } else {
- let mut hdr = self.header.clone();
- for digest_item in &self.post_digests {
- hdr.digest_mut().push(digest_item.clone());
- }
-
- hdr.hash()
+ self.post_header().hash()
+ }
+ }
+
+ /// Get the post header.
+ pub fn post_header(&self) -> Block::Header {
+ if self.post_digests.is_empty() {
+ self.header.clone()
+ } else {
+ let mut hdr = self.header.clone();
+ for digest_item in &self.post_digests {
+ hdr.digest_mut().push(digest_item.clone());
}
+
+ hdr
}
}
--
GitLab