Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CheckBridgedBlockNumber signed extension to reject duplicate header-submit transactions #1352

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions bin/millau/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,12 @@ construct_runtime!(
}
);

pallet_bridge_grandpa::declare_check_bridged_block_number_ext! {
Runtime,
Call::BridgeRialtoGrandpa => RialtoGrandpaInstance,
Call::BridgeWestendGrandpa => WestendGrandpaInstance
}

/// The address format for describing accounts.
pub type Address = AccountId;
/// Block header type as expected by this runtime.
Expand All @@ -520,6 +526,7 @@ pub type SignedExtra = (
frame_system::CheckNonce<Runtime>,
frame_system::CheckWeight<Runtime>,
pallet_transaction_payment::ChargeTransactionPayment<Runtime>,
CheckBridgedBlockNumber,
);
/// The payload being signed in transactions.
pub type SignedPayload = generic::SignedPayload<Call, SignedExtra>;
Expand Down
20 changes: 17 additions & 3 deletions deployments/bridges/westend-millau/docker-compose.yml
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
# Exposed ports: 10616
# Exposed ports: 10616, 10617

version: '3.5'
services:
relay-headers-westend-to-millau:
relay-headers-westend-to-millau-1:
image: paritytech/substrate-relay
entrypoint: /entrypoints/relay-headers-westend-to-millau-entrypoint.sh
volumes:
Expand All @@ -14,6 +14,19 @@ services:
depends_on:
- millau-node-alice

relay-headers-westend-to-millau-2:
image: paritytech/substrate-relay
entrypoint: /entrypoints/relay-headers-westend-to-millau-entrypoint.sh
volumes:
- ./bridges/westend-millau/entrypoints:/entrypoints
environment:
RUST_LOG: rpc=trace,bridge=trace
EXT_RELAY_ACCOUNT: //Harry
ports:
- "10617:9616"
depends_on:
- millau-node-alice

# Note: These are being overridden from the top level `monitoring` compose file.
grafana-dashboard:
environment:
Expand All @@ -28,4 +41,5 @@ services:
volumes:
- ./bridges/westend-millau/dashboard/prometheus/targets.yml:/etc/prometheus/targets-westend-millau.yml
depends_on:
- relay-headers-westend-to-millau
- relay-headers-westend-to-millau-1
- relay-headers-westend-to-millau-2
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@ set -xeu

sleep 15

RELAY_ACCOUNT=${EXT_RELAY_ACCOUNT:-//George}

/home/user/substrate-relay init-bridge westend-to-millau \
--source-host westend-rpc.polkadot.io \
--source-port 443 \
--source-secure \
--target-host millau-node-alice \
--target-port 9944 \
--target-signer //George
--target-signer $RELAY_ACCOUNT

# Give chain a little bit of time to process initialization transaction
sleep 6
Expand All @@ -19,6 +21,6 @@ sleep 6
--source-secure \
--target-host millau-node-alice \
--target-port 9944 \
--target-signer //George \
--target-signer $RELAY_ACCOUNT \
--target-transactions-mortality=4\
--prometheus-host=0.0.0.0
171 changes: 171 additions & 0 deletions modules/grandpa/src/extension.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
// Copyright 2021 Parity Technologies (UK) Ltd.
// This file is part of Parity Bridges Common.

// Parity Bridges Common is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.

// Parity Bridges Common is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.

// 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/>.

/// Declares a runtime-specific `CheckBridgedBlockNumber` signed extension.
///
/// ## Example
///
/// ```nocompile
/// pallet_bridge_grandpa::declare_check_bridged_block_number_ext!{
/// Runtime,
/// Call::BridgeRialtoGrandpa => RialtoGrandpaInstance,
/// Call::BridgeWestendGrandpa => WestendGrandpaInstance,
/// }
/// ```
#[macro_export]
macro_rules! declare_check_bridged_block_number_ext {
($runtime:ident, $($call:path => $instance:ty),*) => {
/// Transaction-with-obsolete-bridged-header check that will reject transaction if
/// it submits obsolete bridged header.
#[derive(Clone, codec::Decode, codec::Encode, Eq, PartialEq, frame_support::RuntimeDebug, scale_info::TypeInfo)]
pub struct CheckBridgedBlockNumber;

impl sp_runtime::traits::SignedExtension for CheckBridgedBlockNumber {
const IDENTIFIER: &'static str = "CheckBridgedBlockNumber";
type AccountId = <$runtime as frame_system::Config>::AccountId;
type Call = <$runtime as frame_system::Config>::Call;
type AdditionalSigned = ();
type Pre = ();

fn additional_signed(&self) -> sp_std::result::Result<
(),
sp_runtime::transaction_validity::TransactionValidityError,
> {
Ok(())
}

fn validate(
&self,
_who: &Self::AccountId,
call: &Self::Call,
_info: &sp_runtime::traits::DispatchInfoOf<Self::Call>,
_len: usize,
) -> sp_runtime::transaction_validity::TransactionValidity {
match *call {
$(
$call($crate::Call::<$runtime, $instance>::submit_finality_proof { ref finality_target, ..}) => {
use sp_runtime::traits::Header as HeaderT;

let bundled_block_number = *finality_target.number();

let best_finalized_hash = $crate::BestFinalized::<$runtime, $instance>::get();
let best_finalized_number = match $crate::ImportedHeaders::<
$runtime,
$instance,
>::get(best_finalized_hash) {
Some(best_finalized_header) => *best_finalized_header.number(),
None => return sp_runtime::transaction_validity::InvalidTransaction::Call.into(),
};

if best_finalized_number < bundled_block_number {
Ok(sp_runtime::transaction_validity::ValidTransaction::default())
} else {
sp_runtime::transaction_validity::InvalidTransaction::Stale.into()
}
},
)*
_ => Ok(sp_runtime::transaction_validity::ValidTransaction::default()),
}
}

fn pre_dispatch(
self,
who: &Self::AccountId,
call: &Self::Call,
info: &sp_runtime::traits::DispatchInfoOf<Self::Call>,
len: usize,
) -> Result<Self::Pre, sp_runtime::transaction_validity::TransactionValidityError> {
self.validate(who, call, info, len).map(drop)
}

fn post_dispatch(
_maybe_pre: Option<Self::Pre>,
_info: &sp_runtime::traits::DispatchInfoOf<Self::Call>,
_post_info: &sp_runtime::traits::PostDispatchInfoOf<Self::Call>,
_len: usize,
_result: &sp_runtime::DispatchResult,
) -> Result<(), sp_runtime::transaction_validity::TransactionValidityError> {
Ok(())
}
}
};
}

#[cfg(test)]
mod tests {
use crate::{
mock::{run_test, test_header, Call, TestNumber, TestRuntime},
BestFinalized, ImportedHeaders,
};
use bp_test_utils::make_default_justification;
use frame_support::weights::{DispatchClass, DispatchInfo, Pays};
use sp_runtime::traits::SignedExtension;

declare_check_bridged_block_number_ext! {
TestRuntime,
Call::Grandpa => ()
}

fn validate_block_submit(num: TestNumber) -> bool {
CheckBridgedBlockNumber
.validate(
&42,
&Call::Grandpa(crate::Call::<TestRuntime, ()>::submit_finality_proof {
finality_target: Box::new(test_header(num)),
justification: make_default_justification(&test_header(num)),
}),
&DispatchInfo { weight: 0, class: DispatchClass::Operational, pays_fee: Pays::Yes },
0,
)
.is_ok()
}

fn sync_to_header_10() {
let header10_hash = sp_core::H256::default();
BestFinalized::<TestRuntime, ()>::put(header10_hash);
ImportedHeaders::<TestRuntime, ()>::insert(header10_hash, test_header(10));
}

#[test]
fn check_bridged_block_number_rejects_obsolete_header() {
run_test(|| {
// when current best finalized is #10 and we're trying to import header#5 => tx is
// rejected
sync_to_header_10();
assert!(!validate_block_submit(5));
});
}

#[test]
fn check_bridged_block_number_rejects_same_header() {
run_test(|| {
// when current best finalized is #10 and we're trying to import header#10 => tx is
// rejected
sync_to_header_10();
assert!(!validate_block_submit(10));
});
}

#[test]
fn check_bridged_block_number_accepts_new_header() {
run_test(|| {
// when current best finalized is #10 and we're trying to import header#15 => tx is
// accepted
sync_to_header_10();
assert!(validate_block_submit(15));
});
}
}
5 changes: 3 additions & 2 deletions modules/grandpa/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,11 @@ use sp_finality_grandpa::{ConsensusLog, GRANDPA_ENGINE_ID};
use sp_runtime::traits::{BadOrigin, Header as HeaderT, Zero};
use sp_std::{boxed::Box, convert::TryInto};

mod extension;
#[cfg(test)]
mod mock;

/// Pallet containing weights for this pallet.
/// Module, containing weights for this pallet.
pub mod weights;

#[cfg(feature = "runtime-benchmarks")]
Expand Down Expand Up @@ -269,7 +270,7 @@ pub mod pallet {

/// Hash of the best finalized header.
#[pallet::storage]
pub(super) type BestFinalized<T: Config<I>, I: 'static = ()> =
pub type BestFinalized<T: Config<I>, I: 'static = ()> =
StorageValue<_, BridgedBlockHash<T, I>, ValueQuery>;

/// A ring buffer of imported hashes. Ordered by the insertion time.
Expand Down
2 changes: 1 addition & 1 deletion modules/grandpa/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ construct_runtime! {
UncheckedExtrinsic = UncheckedExtrinsic,
{
System: frame_system::{Pallet, Call, Config, Storage, Event<T>},
Grandpa: grandpa::{Pallet},
Grandpa: grandpa::{Pallet, Call},
}
}

Expand Down
1 change: 1 addition & 0 deletions relays/client-millau/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ relay-utils = { path = "../utils" }
bp-messages = { path = "../../primitives/messages" }
bp-millau = { path = "../../primitives/chain-millau" }
millau-runtime = { path = "../../bin/millau/runtime" }
pallet-bridge-grandpa = { path = "../../modules/grandpa" }

# Substrate Dependencies

Expand Down
2 changes: 2 additions & 0 deletions relays/client-millau/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ impl TransactionSignScheme for Millau {
frame_system::CheckNonce::<millau_runtime::Runtime>::from(param.unsigned.nonce),
frame_system::CheckWeight::<millau_runtime::Runtime>::new(),
pallet_transaction_payment::ChargeTransactionPayment::<millau_runtime::Runtime>::from(param.unsigned.tip),
millau_runtime::CheckBridgedBlockNumber, // TODO
),
(
(),
Expand All @@ -123,6 +124,7 @@ impl TransactionSignScheme for Millau {
(),
(),
(),
(),
),
);
let signature = raw_payload.using_encoded(|payload| param.signer.sign(payload));
Expand Down