Skip to content

Commit

Permalink
Integrate Grandpa Proof Checker into Substrate Pallet (paritytech#375)
Browse files Browse the repository at this point in the history 
* Remove the Substrate primitives crate

The types here were only used in one place, the pallet itself. If other
components start using these types we can considering moving them back
into a standalone crate.

* Start trying to integrate justification module

* Make Substrate blocks configurable in Pallet

* WIP: Try and generalize justification test helpers

* Fix tests which use "real" justifications

* Put common test helpers alongside mock code

* Use common helper for creating headers

* Remove usage of UintAuthorityId

This change favours the use of the Ed25519Keyring authorities
in order to keep things consistent with the tests.

* Add documentation around config trait types

* Make test header, hash, and number types consistent

* Update modules/substrate/src/verifier.rs

Co-authored-by: Svyatoslav Nikolsky <svyatonik@gmail.com>

* Update modules/substrate/src/lib.rs

Co-authored-by: Tomasz Drwięga <tomusdrw@users.noreply.github.com>

* Update modules/substrate/Cargo.toml

Co-authored-by: Svyatoslav Nikolsky <svyatonik@gmail.com>

* Derive `RuntimeDebug` instead of `Debug`

* Add `Paramter` as a trait constraint on config types

Since we use these types as part of the dispatchable functions
we should explicitly require this.

* Enforce that hasher output matches expected hash type

* Accept headers over indexes when making test justifications

* Check that authority sets are valid

* Make Clippy happy

* Apply correct Clippy fix

* Move justification code into primitives module

* Use new module in verifier code

* Add primitives module for Substrate test helpers

* WIP

* Move justification generation into test_helpers

* Revert commits which move `justification` into primitives

This reverts commit 03a381f0bc4a8dbe4785c30d42ab252a06ba876c.

Co-authored-by: Svyatoslav Nikolsky <svyatonik@gmail.com>
Co-authored-by: Tomasz Drwięga <tomusdrw@users.noreply.github.com>
  • Loading branch information
@HCastano @svyatonik
@tomusdrw @serban300
3 people authored and serban300 committed Apr 8, 2024
1 parent b20b169 commit b2db3ac
Show file tree
Hide file tree
Showing 6 changed files with 366 additions and 174 deletions.
3 changes: 2 additions & 1 deletion bridges/modules/substrate/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ license = "GPL-3.0-or-later WITH Classpath-exception-2.0"

[dependencies]
bp-header-chain = { path = "../../primitives/header-chain", default-features = false }
bp-substrate = { path = "../../primitives/substrate", default-features = false }
finality-grandpa = { version = "0.12.3", default-features = false }
hash-db = { version = "0.15.2", default-features = false }
serde = { version = "1.0", optional = true }
num-traits = { version = "0.2", default-features = false }

[dependencies.codec]
package = "parity-scale-codec"
Expand Down Expand Up @@ -89,6 +89,7 @@ std = [
"finality-grandpa/std",
"frame-support/std",
"frame-system/std",
"num-traits/std",
"serde",
"sp-finality-grandpa/std",
"sp-runtime/std",
Expand Down
140 changes: 68 additions & 72 deletions bridges/modules/substrate/src/justification.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,11 @@
// You should have received a copy of the GNU General Public License
// along with Parity Bridges Common. If not, see <http://www.gnu.org/licenses/>.

//! Module for checking Grandpa Finality Proofs.
//!
//! Adapted copy of substrate/client/finality-grandpa/src/justification.rs. If origin
//! will ever be moved to the sp_finality_grandpa, we should reuse that implementation.

// TODO: remove on actual use
#![allow(dead_code)]

use codec::Decode;
use finality_grandpa::{voter_set::VoterSet, Chain, Error as GrandpaError};
use frame_support::RuntimeDebug;
Expand Down Expand Up @@ -115,16 +114,21 @@ where
Ok(())
}

/// GRANDPA justification of the bridged chain
/// A Grandpa Justification is a proof that a given header was finalized
/// at a certain height and with a certain set of authorities.
///
/// This particular proof is used to prove that headers on a bridged chain
/// (so not our chain) have been finalized correctly.
#[derive(Decode, RuntimeDebug)]
#[cfg_attr(test, derive(codec::Encode))]
struct GrandpaJustification<Header: HeaderT> {
pub(crate) struct GrandpaJustification<Header: HeaderT> {
round: u64,
commit: finality_grandpa::Commit<Header::Hash, Header::Number, AuthoritySignature, AuthorityId>,
votes_ancestries: Vec<Header>,
}

/// A utility trait implementing `finality_grandpa::Chain` using a given set of headers.
#[derive(RuntimeDebug)]
struct AncestryChain<Header: HeaderT> {
ancestry: BTreeMap<Header::Hash, Header::Hash>,
}
Expand Down Expand Up @@ -170,56 +174,30 @@ where
}

#[cfg(test)]
mod tests {
pub(crate) mod tests {
use super::*;
use crate::mock::helpers::*;
use codec::Encode;
use sp_core::H256;
use sp_finality_grandpa::{AuthorityId, AuthorityWeight};
use sp_keyring::Ed25519Keyring;
use sp_runtime::traits::BlakeTwo256;

const TEST_GRANDPA_ROUND: u64 = 1;
const TEST_GRANDPA_SET_ID: SetId = 1;

type TestHeader = sp_runtime::generic::Header<u64, BlakeTwo256>;

fn header(index: u8) -> TestHeader {
TestHeader::new(
index as _,
Default::default(),
Default::default(),
if index == 0 {
Default::default()
} else {
header(index - 1).hash()
},
Default::default(),
)
}

fn header_id(index: u8) -> (H256, u64) {
(header(index).hash(), index as _)
}

fn authorities_set() -> VoterSet<AuthorityId> {
VoterSet::new(vec![
(Ed25519Keyring::Alice.public().into(), 1),
(Ed25519Keyring::Bob.public().into(), 1),
(Ed25519Keyring::Charlie.public().into(), 1),
])
.unwrap()
}

fn signed_precommit(
pub(crate) fn signed_precommit(
signer: Ed25519Keyring,
target: (H256, u64),
target: HeaderId,
round: u64,
set_id: SetId,
) -> finality_grandpa::SignedPrecommit<H256, u64, AuthoritySignature, AuthorityId> {
let precommit = finality_grandpa::Precommit {
target_hash: target.0,
target_number: target.1,
};
let encoded = sp_finality_grandpa::localized_payload(
TEST_GRANDPA_ROUND,
TEST_GRANDPA_SET_ID,
round,
set_id,
&finality_grandpa::Message::Precommit(precommit.clone()),
);
let signature = signer.sign(&encoded[..]).into();
Expand All @@ -230,26 +208,59 @@ mod tests {
}
}

fn make_justification_for_header_1() -> GrandpaJustification<TestHeader> {
pub(crate) fn make_justification_for_header(
header: &TestHeader,
round: u64,
set_id: SetId,
authorities: &[(AuthorityId, AuthorityWeight)],
) -> GrandpaJustification<TestHeader> {
let (target_hash, target_number) = (header.hash(), *header.number());
let mut precommits = vec![];
let mut votes_ancestries = vec![];

// We want to make sure that the header included in the vote ancestries
// is actually related to our target header
let mut precommit_header = test_header(target_number + 1);
precommit_header.parent_hash = target_hash;

// I'm using the same header for all the voters since it doesn't matter as long
// as they all vote on blocks _ahead_ of the one we're interested in finalizing
for (id, _weight) in authorities.iter() {
let signer = extract_keyring(&id);
let precommit = signed_precommit(
signer,
(precommit_header.hash(), *precommit_header.number()),
round,
set_id,
);
precommits.push(precommit);
votes_ancestries.push(precommit_header.clone());
}

GrandpaJustification {
round: TEST_GRANDPA_ROUND,
round,
commit: finality_grandpa::Commit {
target_hash: header_id(1).0,
target_number: header_id(1).1,
precommits: vec![
signed_precommit(Ed25519Keyring::Alice, header_id(2)),
signed_precommit(Ed25519Keyring::Bob, header_id(3)),
signed_precommit(Ed25519Keyring::Charlie, header_id(4)),
],
target_hash,
target_number,
precommits,
},
votes_ancestries: vec![header(2), header(3), header(4)],
votes_ancestries,
}
}

pub(crate) fn make_justification_for_header_1() -> GrandpaJustification<TestHeader> {
make_justification_for_header(
&test_header(1),
TEST_GRANDPA_ROUND,
TEST_GRANDPA_SET_ID,
&authority_list(),
)
}

#[test]
fn justification_with_invalid_encoding_rejected() {
assert_eq!(
verify_justification::<TestHeader>(header_id(1), TEST_GRANDPA_SET_ID, authorities_set(), &[],),
verify_justification::<TestHeader>(header_id(1), TEST_GRANDPA_SET_ID, voter_set(), &[],),
Err(Error::JustificationDecode),
);
}
Expand All @@ -260,7 +271,7 @@ mod tests {
verify_justification::<TestHeader>(
header_id(2),
TEST_GRANDPA_SET_ID,
authorities_set(),
voter_set(),
&make_justification_for_header_1().encode(),
),
Err(Error::InvalidJustificationTarget),
Expand All @@ -273,12 +284,7 @@ mod tests {
justification.commit.precommits.clear();

assert_eq!(
verify_justification::<TestHeader>(
header_id(1),
TEST_GRANDPA_SET_ID,
authorities_set(),
&justification.encode(),
),
verify_justification::<TestHeader>(header_id(1), TEST_GRANDPA_SET_ID, voter_set(), &justification.encode(),),
Err(Error::InvalidJustificationCommit),
);
}
Expand All @@ -289,28 +295,18 @@ mod tests {
justification.commit.precommits[0].signature = Default::default();

assert_eq!(
verify_justification::<TestHeader>(
header_id(1),
TEST_GRANDPA_SET_ID,
authorities_set(),
&justification.encode(),
),
verify_justification::<TestHeader>(header_id(1), TEST_GRANDPA_SET_ID, voter_set(), &justification.encode(),),
Err(Error::InvalidAuthoritySignature),
);
}

#[test]
fn justification_with_invalid_precommit_ancestry() {
let mut justification = make_justification_for_header_1();
justification.votes_ancestries.push(header(10));
justification.votes_ancestries.push(test_header(10));

assert_eq!(
verify_justification::<TestHeader>(
header_id(1),
TEST_GRANDPA_SET_ID,
authorities_set(),
&justification.encode(),
),
verify_justification::<TestHeader>(header_id(1), TEST_GRANDPA_SET_ID, voter_set(), &justification.encode(),),
Err(Error::InvalidPrecommitAncestries),
);
}
Expand All @@ -321,7 +317,7 @@ mod tests {
verify_justification::<TestHeader>(
header_id(1),
TEST_GRANDPA_SET_ID,
authorities_set(),
voter_set(),
&make_justification_for_header_1().encode(),
),
Ok(()),
Expand Down

0 comments on commit b2db3ac

Please sign in to comment.