Skip to content

Commit

Permalink
beefy: initialize voter from genesis and fix initial sync (#11959)
Browse files Browse the repository at this point in the history
* client/beefy: use backend instead of client where possible

* client/beefy: initialize voter from genesis

Now that we have justifications import, we can drop the "lean beefy"
behaviour and start building justifications chain from Genesis with
containing all past sessions' mandatory blocks justifications.

* client/beefy: walk finality tree_route to catch session changes

* client/beefy: fix block import

During initial block import blocks are not finalized, so trying to
validate and append justifications within block import fails (for
initial network sync imported blocks).

Changes:

- Move justification validation to _after_ `inner.block_import()`,
  so block is imported in backend and runtime api can be called to
  get the BEEFY authorities for said block.
- Move append-to-backend for imported BEEFY justification to voter,
  because it already has the required logic to BEEFY-finalize blocks
  only after GRANDPA finalized them.
- Mark voting rounds as concluded when finalizing through
  imported justifications as well as when finalizing through voting.

* client/beefy: valid justifications are one per block number

The only way we'd get _different_ _validated_ justifications for same
block number is if authorities are double voting, which will be handled
later.

* client/beefy: process incoming justifs during major sync

* client/beefy: correct voter initialization

BEEFY voter should resume voting from either:
  - last BEEFY finalized block,
  - session start,
whichever is closest to head.

* client/beefy: test voter initialization

* client/beefy: impl review suggestions

Signed-off-by: acatangiu <[email protected]>
  • Loading branch information
acatangiu authored Sep 5, 2022
1 parent 1a15071 commit 58d3bc6
Show file tree
Hide file tree
Showing 4 changed files with 342 additions and 147 deletions.
75 changes: 20 additions & 55 deletions client/beefy/src/import.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,11 @@
// along with this program. If not, see <https://www.gnu.org/licenses/>.

use beefy_primitives::{BeefyApi, BEEFY_ENGINE_ID};
use codec::Encode;
use log::error;
use log::debug;
use std::{collections::HashMap, sync::Arc};

use sp_api::{ProvideRuntimeApi, TransactionFor};
use sp_blockchain::{well_known_cache_keys, HeaderBackend};
use sp_blockchain::well_known_cache_keys;
use sp_consensus::Error as ConsensusError;
use sp_runtime::{
generic::BlockId,
Expand Down Expand Up @@ -97,29 +96,6 @@ where

decode_and_verify_finality_proof::<Block>(&encoded[..], number, &validator_set)
}

/// Import BEEFY justification: Send it to worker for processing and also append it to backend.
///
/// This function assumes:
/// - `justification` is verified and valid,
/// - the block referred by `justification` has been imported _and_ finalized.
fn import_beefy_justification_unchecked(
&self,
number: NumberFor<Block>,
justification: BeefyVersionedFinalityProof<Block>,
) {
// Append the justification to the block in the backend.
if let Err(e) = self.backend.append_justification(
BlockId::Number(number),
(BEEFY_ENGINE_ID, justification.encode()),
) {
error!(target: "beefy", "🥩 Error {:?} on appending justification: {:?}", e, justification);
}
// Send the justification to the BEEFY voter for processing.
self.justification_sender
.notify(|| Ok::<_, ()>(justification))
.expect("forwards closure result; the closure always returns Ok; qed.");
}
}

#[async_trait::async_trait]
Expand Down Expand Up @@ -147,42 +123,31 @@ where
let hash = block.post_hash();
let number = *block.header.number();

let beefy_proof = block
.justifications
.as_mut()
.and_then(|just| {
let decoded = just
.get(BEEFY_ENGINE_ID)
.map(|encoded| self.decode_and_verify(encoded, number, hash));
// Remove BEEFY justification from the list before giving to `inner`;
// we will append it to backend ourselves at the end if all goes well.
just.remove(BEEFY_ENGINE_ID);
decoded
})
.transpose()
.unwrap_or(None);
let beefy_encoded = block.justifications.as_mut().and_then(|just| {
let encoded = just.get(BEEFY_ENGINE_ID).cloned();
// Remove BEEFY justification from the list before giving to `inner`; we send it to the
// voter (beefy-gadget) and it will append it to the backend after block is finalized.
just.remove(BEEFY_ENGINE_ID);
encoded
});

// Run inner block import.
let inner_import_result = self.inner.import_block(block, new_cache).await?;

match (beefy_proof, &inner_import_result) {
(Some(proof), ImportResult::Imported(_)) => {
let status = self.backend.blockchain().info();
if number <= status.finalized_number &&
Some(hash) ==
self.backend
.blockchain()
.hash(number)
.map_err(|e| ConsensusError::ClientImport(e.to_string()))?
{
match (beefy_encoded, &inner_import_result) {
(Some(encoded), ImportResult::Imported(_)) => {
if let Ok(proof) = self.decode_and_verify(&encoded, number, hash) {
// The proof is valid and the block is imported and final, we can import.
self.import_beefy_justification_unchecked(number, proof);
debug!(target: "beefy", "🥩 import justif {:?} for block number {:?}.", proof, number);
// Send the justification to the BEEFY voter for processing.
self.justification_sender
.notify(|| Ok::<_, ()>(proof))
.expect("forwards closure result; the closure always returns Ok; qed.");
} else {
error!(
debug!(
target: "beefy",
"🥩 Cannot import justification: {:?} for, not yet final, block number {:?}",
proof,
number,
"🥩 error decoding justification: {:?} for imported block {:?}",
encoded, number,
);
}
},
Expand Down
28 changes: 18 additions & 10 deletions client/beefy/src/round.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,13 +147,8 @@ where
trace!(target: "beefy", "🥩 Round #{} done: {}", round.1, done);

if done {
// remove this and older (now stale) rounds
let signatures = self.rounds.remove(round)?.votes;
self.rounds.retain(|&(_, number), _| number > round.1);
self.mandatory_done = self.mandatory_done || round.1 == self.session_start;
self.best_done = self.best_done.max(Some(round.1));
debug!(target: "beefy", "🥩 Concluded round #{}", round.1);

self.conclude(round.1);
Some(
self.validators()
.iter()
Expand All @@ -165,9 +160,12 @@ where
}
}

#[cfg(test)]
pub(crate) fn test_set_mandatory_done(&mut self, done: bool) {
self.mandatory_done = done;
pub(crate) fn conclude(&mut self, round_num: NumberFor<B>) {
// Remove this and older (now stale) rounds.
self.rounds.retain(|&(_, number), _| number > round_num);
self.mandatory_done = self.mandatory_done || round_num == self.session_start;
self.best_done = self.best_done.max(Some(round_num));
debug!(target: "beefy", "🥩 Concluded round #{}", round_num);
}
}

Expand All @@ -178,9 +176,19 @@ mod tests {

use beefy_primitives::{crypto::Public, ValidatorSet};

use super::{threshold, RoundTracker, Rounds};
use super::{threshold, Block as BlockT, Hash, RoundTracker, Rounds};
use crate::keystore::tests::Keyring;

impl<P, B> Rounds<P, B>
where
P: Ord + Hash + Clone,
B: BlockT,
{
pub(crate) fn test_set_mandatory_done(&mut self, done: bool) {
self.mandatory_done = done;
}
}

#[test]
fn round_tracker() {
let mut rt = RoundTracker::default();
Expand Down
12 changes: 5 additions & 7 deletions client/beefy/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ impl BeefyTestNet {
})
}

pub(crate) fn generate_blocks(
pub(crate) fn generate_blocks_and_sync(
&mut self,
count: usize,
session_length: u64,
Expand All @@ -168,6 +168,7 @@ impl BeefyTestNet {

block
});
self.block_until_sync();
}
}

Expand Down Expand Up @@ -528,8 +529,7 @@ fn beefy_finalizing_blocks() {
runtime.spawn(initialize_beefy(&mut net, beefy_peers, min_block_delta));

// push 42 blocks including `AuthorityChange` digests every 10 blocks.
net.generate_blocks(42, session_len, &validator_set, true);
net.block_until_sync();
net.generate_blocks_and_sync(42, session_len, &validator_set, true);

let net = Arc::new(Mutex::new(net));

Expand Down Expand Up @@ -567,8 +567,7 @@ fn lagging_validators() {
runtime.spawn(initialize_beefy(&mut net, beefy_peers, min_block_delta));

// push 62 blocks including `AuthorityChange` digests every 30 blocks.
net.generate_blocks(62, session_len, &validator_set, true);
net.block_until_sync();
net.generate_blocks_and_sync(62, session_len, &validator_set, true);

let net = Arc::new(Mutex::new(net));

Expand Down Expand Up @@ -644,8 +643,7 @@ fn correct_beefy_payload() {
runtime.spawn(initialize_beefy(&mut net, bad_peers, min_block_delta));

// push 10 blocks
net.generate_blocks(12, session_len, &validator_set, false);
net.block_until_sync();
net.generate_blocks_and_sync(12, session_len, &validator_set, false);

let net = Arc::new(Mutex::new(net));
// with 3 good voters and 1 bad one, consensus should happen and best blocks produced.
Expand Down
Loading

0 comments on commit 58d3bc6

Please sign in to comment.