Skip to content

Commit

Permalink
Replace BATCH_CALL_SUPPORTED (paritytech#1733)
Browse files Browse the repository at this point in the history
* Simplify submit_and_watch_signed_extrinsic

The way submit_and_watch_signed_extrinsic is used now, we can always
derive the SignParam from other params. If in the future we need more
customization possibilities, we can define a new method.

* Simplify submit_signed_extrinsic

* Send maybe_batch_tx as a parameter

Send `maybe_batch_tx` as a parameter to `submit_proof()`. This way we
can deduplicate the logic that submits the extrinsic for
`messages_source and `messages_target` and we can simplify the logic in
the race loop a bit.

* Define BatchProofTransaction

Deduplicate BatchConfirmationTransaction and BatchDeliveryTransaction by
replacing both of them with BatchProofTransaction

* Define ChainWithUtilityPallet and BatchCallBuilderConstructor

- Define `ChainWithUtilityPallet` in order to be able to associate the
  batching functionality with chains
- Defining `BatchCallBuilderConstructor` in order to have a more reliable
  way of checking whether an end of a messages pipeline supports batching
  or no. `BatchCallBuilderConstructor::new_builder()` returns an
  `Option<BatchCallBuilder>`.This is a bit safer because each time a caller
  tries to start creating a batch call, it will call `new_builder()` and
  will be required to handle the returned `Option`. Before we only had a
  bool `BATCH_CALL_SUPPORTED` the caller could have forgetten to check.
  • Loading branch information
serban300 committed Apr 9, 2024
1 parent 0199b36 commit 48d9c6e
Show file tree
Hide file tree
Showing 20 changed files with 288 additions and 492 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use substrate_relay_helper::{
DirectReceiveMessagesDeliveryProofCallBuilder, DirectReceiveMessagesProofCallBuilder,
SubstrateMessageLane,
},
BundledBatchCallBuilder,
UtilityPalletBatchCallBuilder,
};

/// Description of RialtoParachain -> Millau messages bridge.
Expand All @@ -46,5 +46,5 @@ impl SubstrateMessageLane for RialtoParachainMessagesToMillau {
>;

type SourceBatchCallBuilder = ();
type TargetBatchCallBuilder = BundledBatchCallBuilder<millau_runtime::Runtime>;
type TargetBatchCallBuilder = UtilityPalletBatchCallBuilder<Millau>;
}
11 changes: 2 additions & 9 deletions bridges/relays/bin-substrate/src/cli/init_bridge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use crate::{
cli::{bridge::CliBridgeBase, chain_schema::*},
};
use bp_runtime::Chain as ChainBase;
use relay_substrate_client::{AccountKeyPairOf, Chain, SignParam, UnsignedTransaction};
use relay_substrate_client::{AccountKeyPairOf, Chain, UnsignedTransaction};
use sp_core::Pair;
use structopt::StructOpt;
use strum::{EnumString, EnumVariantNames, VariantNames};
Expand Down Expand Up @@ -83,17 +83,10 @@ where
let target_sign = data.target_sign.to_keypair::<Self::Target>()?;
let dry_run = data.dry_run;

let (spec_version, transaction_version) = target_client.simple_runtime_version().await?;
substrate_relay_helper::finality::initialize::initialize::<Self::Engine, _, _, _>(
source_client,
target_client.clone(),
target_sign.public().into(),
SignParam {
spec_version,
transaction_version,
genesis_hash: *target_client.genesis_hash(),
signer: target_sign,
},
target_sign,
move |transaction_nonce, initialization_data| {
let call = Self::encode_init_bridge(initialization_data);
log::info!(
Expand Down
61 changes: 13 additions & 48 deletions bridges/relays/bin-substrate/src/cli/register_parachain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use polkadot_runtime_common::{
paras_registrar::Call as ParaRegistrarCall, slots::Call as ParaSlotsCall,
};
use polkadot_runtime_parachains::paras::ParaLifecycle;
use relay_substrate_client::{AccountIdOf, CallOf, Chain, Client, SignParam, UnsignedTransaction};
use relay_substrate_client::{AccountIdOf, CallOf, Chain, Client, UnsignedTransaction};
use relay_utils::{TrackedTransactionStatus, TransactionTracker};
use rialto_runtime::SudoCall;
use sp_core::{
Expand Down Expand Up @@ -108,28 +108,16 @@ impl RegisterParachain {
log::info!(target: "bridge", "Going to reserve parachain id: {:?}", para_id);

// step 1: reserve a parachain id
let relay_genesis_hash = *relay_client.genesis_hash();
let relay_sudo_account: AccountIdOf<Relaychain> = relay_sign.public().into();
let reserve_parachain_id_call: CallOf<Relaychain> =
ParaRegistrarCall::reserve {}.into();
let reserve_parachain_signer = relay_sign.clone();
let (spec_version, transaction_version) = relay_client.simple_runtime_version().await?;
let reserve_result = relay_client
.submit_and_watch_signed_extrinsic(
relay_sudo_account.clone(),
SignParam::<Relaychain> {
spec_version,
transaction_version,
genesis_hash: relay_genesis_hash,
signer: reserve_parachain_signer,
},
move |_, transaction_nonce| {
Ok(UnsignedTransaction::new(
reserve_parachain_id_call.into(),
transaction_nonce,
))
},
)
.submit_and_watch_signed_extrinsic(&relay_sign, move |_, transaction_nonce| {
Ok(UnsignedTransaction::new(
reserve_parachain_id_call.into(),
transaction_nonce,
))
})
.await?
.wait()
.await;
Expand Down Expand Up @@ -162,23 +150,10 @@ impl RegisterParachain {
validation_code: ParaValidationCode(para_code),
}
.into();
let register_parathread_signer = relay_sign.clone();
let register_result = relay_client
.submit_and_watch_signed_extrinsic(
relay_sudo_account.clone(),
SignParam::<Relaychain> {
spec_version,
transaction_version,
genesis_hash: relay_genesis_hash,
signer: register_parathread_signer,
},
move |_, transaction_nonce| {
Ok(UnsignedTransaction::new(
register_parathread_call.into(),
transaction_nonce,
))
},
)
.submit_and_watch_signed_extrinsic(&relay_sign, move |_, transaction_nonce| {
Ok(UnsignedTransaction::new(register_parathread_call.into(), transaction_nonce))
})
.await?
.wait()
.await;
Expand Down Expand Up @@ -227,20 +202,10 @@ impl RegisterParachain {
),
}
.into();
let force_lease_signer = relay_sign.clone();
relay_client
.submit_signed_extrinsic(
relay_sudo_account,
SignParam::<Relaychain> {
spec_version,
transaction_version,
genesis_hash: relay_genesis_hash,
signer: force_lease_signer,
},
move |_, transaction_nonce| {
Ok(UnsignedTransaction::new(force_lease_call.into(), transaction_nonce))
},
)
.submit_signed_extrinsic(&relay_sign, move |_, transaction_nonce| {
Ok(UnsignedTransaction::new(force_lease_call.into(), transaction_nonce))
})
.await?;
log::info!(target: "bridge", "Registered parachain leases: {:?}. Waiting for onboarding", para_id);

Expand Down
34 changes: 11 additions & 23 deletions bridges/relays/bin-substrate/src/cli/send_message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,7 @@ use crate::{
use async_trait::async_trait;
use codec::{Decode, Encode};
use relay_substrate_client::{
AccountIdOf, AccountKeyPairOf, Chain, ChainBase, ChainWithTransactions, SignParam,
UnsignedTransaction,
AccountIdOf, AccountKeyPairOf, Chain, ChainBase, ChainWithTransactions, UnsignedTransaction,
};
use sp_core::Pair;
use sp_runtime::AccountId32;
Expand Down Expand Up @@ -80,28 +79,17 @@ where
data.bridge.bridge_instance_index(),
)?;

let source_genesis_hash = *source_client.genesis_hash();
let (spec_version, transaction_version) = source_client.simple_runtime_version().await?;
source_client
.submit_signed_extrinsic(
source_sign.public().into(),
SignParam::<Self::Source> {
spec_version,
transaction_version,
genesis_hash: source_genesis_hash,
signer: source_sign.clone(),
},
move |_, transaction_nonce| {
let unsigned = UnsignedTransaction::new(send_message_call, transaction_nonce);
log::info!(
target: "bridge",
"Sending message to {}. Size: {}",
Self::Target::NAME,
payload_len,
);
Ok(unsigned)
},
)
.submit_signed_extrinsic(&source_sign, move |_, transaction_nonce| {
let unsigned = UnsignedTransaction::new(send_message_call, transaction_nonce);
log::info!(
target: "bridge",
"Sending message to {}. Size: {}",
Self::Target::NAME,
payload_len,
);
Ok(unsigned)
})
.await?;

Ok(())
Expand Down
8 changes: 6 additions & 2 deletions bridges/relays/client-millau/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ use bp_messages::MessageNonce;
use codec::{Compact, Decode, Encode};
use relay_substrate_client::{
BalanceOf, Chain, ChainWithBalances, ChainWithGrandpa, ChainWithMessages,
ChainWithTransactions, Error as SubstrateError, IndexOf, SignParam, UnderlyingChainProvider,
UnsignedTransaction,
ChainWithTransactions, ChainWithUtilityPallet, Error as SubstrateError,
FullRuntimeUtilityPallet, IndexOf, SignParam, UnderlyingChainProvider, UnsignedTransaction,
};
use sp_core::{storage::StorageKey, Pair};
use sp_runtime::{generic::SignedPayload, traits::IdentifyAccount};
Expand Down Expand Up @@ -149,6 +149,10 @@ impl ChainWithTransactions for Millau {
}
}

impl ChainWithUtilityPallet for Millau {
type UtilityPallet = FullRuntimeUtilityPallet<millau_runtime::Runtime>;
}

/// Millau signing params.
pub type SigningParams = sp_core::sr25519::Pair;

Expand Down
1 change: 1 addition & 0 deletions bridges/relays/client-substrate/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ frame-system = { git = "https://github.com/paritytech/substrate", branch = "mast
pallet-balances = { git = "https://github.com/paritytech/substrate", branch = "master" }
pallet-transaction-payment = { git = "https://github.com/paritytech/substrate", branch = "master" }
pallet-transaction-payment-rpc-runtime-api = { git = "https://github.com/paritytech/substrate", branch = "master" }
pallet-utility = { git = "https://github.com/paritytech/substrate", branch = "master" }
sc-chain-spec = { git = "https://github.com/paritytech/substrate", branch = "master" }
sc-rpc-api = { git = "https://github.com/paritytech/substrate", branch = "master" }
sc-transaction-pool-api = { git = "https://github.com/paritytech/substrate", branch = "master" }
Expand Down
28 changes: 28 additions & 0 deletions bridges/relays/client-substrate/src/chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,3 +234,31 @@ impl<Block: BlockT> BlockWithJustification<Block::Header> for SignedBlock<Block>
self.justifications.as_ref().and_then(|j| j.get(engine_id))
}
}

/// Trait that provides functionality defined inside `pallet-utility`
pub trait UtilityPallet<C: Chain> {
/// Create batch call from given calls vector.
fn build_batch_call(calls: Vec<C::Call>) -> C::Call;
}

/// Structure that implements `UtilityPalletProvider` based on a full runtime.
pub struct FullRuntimeUtilityPallet<R> {
_phantom: std::marker::PhantomData<R>,
}

impl<C, R> UtilityPallet<C> for FullRuntimeUtilityPallet<R>
where
C: Chain,
R: pallet_utility::Config<RuntimeCall = C::Call>,
<R as pallet_utility::Config>::RuntimeCall: From<pallet_utility::Call<R>>,
{
fn build_batch_call(calls: Vec<C::Call>) -> C::Call {
pallet_utility::Call::batch_all { calls }.into()
}
}

/// Substrate-based chain that uses `pallet-utility`.
pub trait ChainWithUtilityPallet: Chain {
/// The utility pallet provider.
type UtilityPallet: UtilityPallet<Self>;
}
33 changes: 24 additions & 9 deletions bridges/relays/client-substrate/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ use crate::{
SubstrateFrameSystemClient, SubstrateStateClient, SubstrateSystemClient,
SubstrateTransactionPaymentClient,
},
transaction_stall_timeout, ConnectionParams, Error, HashOf, HeaderIdOf, Result, SignParam,
TransactionTracker, UnsignedTransaction,
transaction_stall_timeout, AccountKeyPairOf, ConnectionParams, Error, HashOf, HeaderIdOf,
Result, SignParam, TransactionTracker, UnsignedTransaction,
};

use async_std::sync::{Arc, Mutex};
Expand All @@ -43,7 +43,7 @@ use pallet_transaction_payment::InclusionFee;
use relay_utils::{relay_loop::RECONNECT_DELAY, STALL_TIMEOUT};
use sp_core::{
storage::{StorageData, StorageKey},
Bytes, Hasher,
Bytes, Hasher, Pair,
};
use sp_runtime::{
traits::Header as HeaderT,
Expand Down Expand Up @@ -426,6 +426,19 @@ impl<C: Chain> Client<C> {
.await
}

async fn build_sign_params(&self, signer: AccountKeyPairOf<C>) -> Result<SignParam<C>>
where
C: ChainWithTransactions,
{
let (spec_version, transaction_version) = self.simple_runtime_version().await?;
Ok(SignParam::<C> {
spec_version,
transaction_version,
genesis_hash: self.genesis_hash,
signer,
})
}

/// Submit an extrinsic signed by given account.
///
/// All calls of this method are synchronized, so there can't be more than one active
Expand All @@ -435,18 +448,19 @@ impl<C: Chain> Client<C> {
/// Note: The given transaction needs to be SCALE encoded beforehand.
pub async fn submit_signed_extrinsic(
&self,
extrinsic_signer: C::AccountId,
signing_data: SignParam<C>,
signer: &AccountKeyPairOf<C>,
prepare_extrinsic: impl FnOnce(HeaderIdOf<C>, C::Index) -> Result<UnsignedTransaction<C>>
+ Send
+ 'static,
) -> Result<C::Hash>
where
C: ChainWithTransactions,
C::AccountId: From<<C::AccountKeyPair as Pair>::Public>,
{
let _guard = self.submit_signed_extrinsic_lock.lock().await;
let transaction_nonce = self.next_account_index(extrinsic_signer).await?;
let transaction_nonce = self.next_account_index(signer.public().into()).await?;
let best_header = self.best_header().await?;
let signing_data = self.build_sign_params(signer.clone()).await?;

// By using parent of best block here, we are protecing again best-block reorganizations.
// E.g. transaction may have been submitted when the best block was `A[num=100]`. Then it
Expand Down Expand Up @@ -475,18 +489,19 @@ impl<C: Chain> Client<C> {
/// after submission.
pub async fn submit_and_watch_signed_extrinsic(
&self,
extrinsic_signer: C::AccountId,
signing_data: SignParam<C>,
signer: &AccountKeyPairOf<C>,
prepare_extrinsic: impl FnOnce(HeaderIdOf<C>, C::Index) -> Result<UnsignedTransaction<C>>
+ Send
+ 'static,
) -> Result<TransactionTracker<C, Self>>
where
C: ChainWithTransactions,
C::AccountId: From<<C::AccountKeyPair as Pair>::Public>,
{
let self_clone = self.clone();
let signing_data = self.build_sign_params(signer.clone()).await?;
let _guard = self.submit_signed_extrinsic_lock.lock().await;
let transaction_nonce = self.next_account_index(extrinsic_signer).await?;
let transaction_nonce = self.next_account_index(signer.public().into()).await?;
let best_header = self.best_header().await?;
let best_header_id = best_header.id();
let (sender, receiver) = futures::channel::mpsc::channel(MAX_SUBSCRIPTION_CAPACITY);
Expand Down
5 changes: 3 additions & 2 deletions bridges/relays/client-substrate/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,9 @@ use std::time::Duration;
pub use crate::{
chain::{
AccountKeyPairOf, BlockWithJustification, CallOf, Chain, ChainWithBalances,
ChainWithGrandpa, ChainWithMessages, ChainWithTransactions, Parachain, RelayChain,
SignParam, TransactionStatusOf, UnsignedTransaction,
ChainWithGrandpa, ChainWithMessages, ChainWithTransactions, ChainWithUtilityPallet,
FullRuntimeUtilityPallet, Parachain, RelayChain, SignParam, TransactionStatusOf,
UnsignedTransaction, UtilityPallet,
},
client::{
ChainRuntimeVersion, Client, OpaqueGrandpaAuthoritiesSet, Subscription,
Expand Down
Loading

0 comments on commit 48d9c6e

Please sign in to comment.