From 4c3cc6b6e20ab640e4ead46e1cf55b7e45901c6a Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Tue, 29 Nov 2022 15:58:55 +0300 Subject: [PATCH 01/29] batch transactions in message relay: API prototype --- relays/messages/src/message_lane_loop.rs | 68 +++++++++++++++++-- relays/messages/src/message_race_delivery.rs | 11 +-- relays/messages/src/message_race_loop.rs | 24 ++++++- relays/messages/src/message_race_receiving.rs | 8 ++- 4 files changed, 99 insertions(+), 12 deletions(-) diff --git a/relays/messages/src/message_lane_loop.rs b/relays/messages/src/message_lane_loop.rs index 6b28dcbaa6..dd79aca7d0 100644 --- a/relays/messages/src/message_lane_loop.rs +++ b/relays/messages/src/message_lane_loop.rs @@ -109,6 +109,38 @@ pub struct NoncesSubmitArtifacts { pub tx_tracker: T, } +/// Batch transaction that already submit some headers and needs to be extended with +/// messages/delivery proof before sending. +#[async_trait] +pub trait BatchTransaction { + /// Append proof and send transaction to the connected node. + async fn append_proof_and_send( + self, + generated_at_header: HeaderId, + proof: Proof, + ) -> Result; +} + +/// Boxed batch transaction that is sent to the source client. +pub type BatchTransactionOfSourceClient = Box< + dyn BatchTransaction< + TargetHeaderIdOf

, +

::MessagesReceivingProof, + >::TransactionTracker, + ::Error, + >, +>; + +/// Boxed batch transaction that is sent to the target client. +pub type BatchTransactionOfTargetClient = Box< + dyn BatchTransaction< + SourceHeaderIdOf

, +

::MessagesProof, + >::TransactionTracker, + ::Error, + >, +>; + /// Source client trait. #[async_trait] pub trait SourceClient: RelayClient { @@ -156,7 +188,17 @@ pub trait SourceClient: RelayClient { ) -> Result; /// We need given finalized target header on source to continue synchronization. - async fn require_target_header_on_source(&self, id: TargetHeaderIdOf

); + /// + /// The client may return `Some(_)`, which means that nothing has happened yet and + /// the caller must generate and append message receiving proof to the batch transaction + /// to actually send it (along with required header) to the node. + /// + /// If function has returned `None`, it means that the caller now must wait for the + /// appearance of the target header `id` at the source client. + async fn require_target_header_on_source( + &self, + id: TargetHeaderIdOf

, + ) -> Option>; } /// Target client trait. @@ -201,7 +243,17 @@ pub trait TargetClient: RelayClient { ) -> Result, Self::Error>; /// We need given finalized source header on target to continue synchronization. - async fn require_source_header_on_target(&self, id: SourceHeaderIdOf

); + /// + /// The client may return `Some(_)`, which means that nothing has happened yet and + /// the caller must generate and append messages proof to the batch transaction + /// to actually send it (along with required header) to the node. + /// + /// If function has returned `None`, it means that the caller now must wait for the + /// appearance of the source header `id` at the target client. + async fn require_source_header_on_target( + &self, + id: SourceHeaderIdOf

, + ) -> Option>; } /// State of the client. @@ -684,12 +736,16 @@ pub(crate) mod tests { Ok(TestTransactionTracker(data.source_tracked_transaction_status)) } - async fn require_target_header_on_source(&self, id: TargetHeaderIdOf) { + async fn require_target_header_on_source( + &self, + id: TargetHeaderIdOf, + ) -> Option> { let mut data = self.data.lock(); data.target_to_source_header_required = Some(id); data.target_to_source_header_requirements.push(id); (self.tick)(&mut data); (self.post_tick)(&mut data); + None } } @@ -814,12 +870,16 @@ pub(crate) mod tests { }) } - async fn require_source_header_on_target(&self, id: SourceHeaderIdOf) { + async fn require_source_header_on_target( + &self, + id: SourceHeaderIdOf, + ) -> Option> { let mut data = self.data.lock(); data.source_to_target_header_required = Some(id); data.source_to_target_header_requirements.push(id); (self.tick)(&mut data); (self.post_tick)(&mut data); + None } } diff --git a/relays/messages/src/message_race_delivery.rs b/relays/messages/src/message_race_delivery.rs index b49a05dac5..ca61965014 100644 --- a/relays/messages/src/message_race_delivery.rs +++ b/relays/messages/src/message_race_delivery.rs @@ -24,9 +24,9 @@ use relay_utils::FailedClient; use crate::{ message_lane::{MessageLane, SourceHeaderIdOf, TargetHeaderIdOf}, message_lane_loop::{ - MessageDeliveryParams, MessageDetailsMap, MessageProofParameters, NoncesSubmitArtifacts, - SourceClient as MessageLaneSourceClient, SourceClientState, - TargetClient as MessageLaneTargetClient, TargetClientState, + BatchTransactionOfTargetClient, MessageDeliveryParams, MessageDetailsMap, + MessageProofParameters, NoncesSubmitArtifacts, SourceClient as MessageLaneSourceClient, + SourceClientState, TargetClient as MessageLaneTargetClient, TargetClientState, }, message_race_limits::{MessageRaceLimits, RelayMessagesBatchReference}, message_race_loop::{ @@ -173,7 +173,10 @@ where type TargetNoncesData = DeliveryRaceTargetNoncesData; type TransactionTracker = C::TransactionTracker; - async fn require_source_header(&self, id: SourceHeaderIdOf

) { + async fn require_source_header( + &self, + id: SourceHeaderIdOf

, + ) -> Option> { self.client.require_source_header_on_target(id).await } diff --git a/relays/messages/src/message_race_loop.rs b/relays/messages/src/message_race_loop.rs index 4f59b635ae..8c2e14590c 100644 --- a/relays/messages/src/message_race_loop.rs +++ b/relays/messages/src/message_race_loop.rs @@ -20,7 +20,7 @@ //! associated data - like messages, lane state, etc) to the target node by //! generating and submitting proof. -use crate::message_lane_loop::{ClientState, NoncesSubmitArtifacts}; +use crate::message_lane_loop::{BatchTransaction, ClientState, NoncesSubmitArtifacts}; use async_trait::async_trait; use bp_messages::MessageNonce; @@ -132,7 +132,27 @@ pub trait TargetClient { /// Ask headers relay to relay finalized headers up to (and including) given header /// from race source to race target. - async fn require_source_header(&self, id: P::SourceHeaderId); + /// + /// The client may return `Some(_)`, which means that nothing has happened yet and + /// the caller must generate and append proof to the batch transaction + /// to actually send it (along with required header) to the node. + /// + /// If function has returned `None`, it means that the caller now must wait for the + /// appearance of the required header `id` at the target client. + #[must_use] + async fn require_source_header( + &self, + id: P::SourceHeaderId, + ) -> Option< + Box< + dyn BatchTransaction< + P::SourceHeaderId, + P::Proof, + Self::TransactionTracker, + Self::Error, + >, + >, + >; /// Return nonces that are known to the target client. async fn nonces( diff --git a/relays/messages/src/message_race_receiving.rs b/relays/messages/src/message_race_receiving.rs index c3d65d0e86..a0f4f66ed1 100644 --- a/relays/messages/src/message_race_receiving.rs +++ b/relays/messages/src/message_race_receiving.rs @@ -16,7 +16,8 @@ use crate::{ message_lane::{MessageLane, SourceHeaderIdOf, TargetHeaderIdOf}, message_lane_loop::{ - NoncesSubmitArtifacts, SourceClient as MessageLaneSourceClient, SourceClientState, + BatchTransactionOfSourceClient, NoncesSubmitArtifacts, + SourceClient as MessageLaneSourceClient, SourceClientState, TargetClient as MessageLaneTargetClient, TargetClientState, }, message_race_loop::{ @@ -157,7 +158,10 @@ where type TargetNoncesData = (); type TransactionTracker = C::TransactionTracker; - async fn require_source_header(&self, id: TargetHeaderIdOf

) { + async fn require_source_header( + &self, + id: TargetHeaderIdOf

, + ) -> Option> { self.client.require_target_header_on_source(id).await } From e140e119a8615bf12afa81a627a1d2cf4e67d9da Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Wed, 30 Nov 2022 12:56:15 +0300 Subject: [PATCH 02/29] get rid of Box and actually submit it --- relays/messages/src/message_lane_loop.rs | 89 +++++++++++++------ relays/messages/src/message_race_delivery.rs | 12 ++- relays/messages/src/message_race_loop.rs | 62 ++++++++----- relays/messages/src/message_race_receiving.rs | 9 +- 4 files changed, 111 insertions(+), 61 deletions(-) diff --git a/relays/messages/src/message_lane_loop.rs b/relays/messages/src/message_lane_loop.rs index dd79aca7d0..f3a3d4b982 100644 --- a/relays/messages/src/message_lane_loop.rs +++ b/relays/messages/src/message_lane_loop.rs @@ -112,38 +112,28 @@ pub struct NoncesSubmitArtifacts { /// Batch transaction that already submit some headers and needs to be extended with /// messages/delivery proof before sending. #[async_trait] -pub trait BatchTransaction { +pub trait BatchTransaction: Send { + /// Header that was required in the original call and which is bundled within this + /// batch transaction. + fn required_header_id(&self) -> HeaderId; + /// Append proof and send transaction to the connected node. async fn append_proof_and_send( self, - generated_at_header: HeaderId, proof: Proof, - ) -> Result; + ) -> Result, Error>; } -/// Boxed batch transaction that is sent to the source client. -pub type BatchTransactionOfSourceClient = Box< - dyn BatchTransaction< - TargetHeaderIdOf

, -

::MessagesReceivingProof, - >::TransactionTracker, - ::Error, - >, ->; - -/// Boxed batch transaction that is sent to the target client. -pub type BatchTransactionOfTargetClient = Box< - dyn BatchTransaction< - SourceHeaderIdOf

, -

::MessagesProof, - >::TransactionTracker, - ::Error, - >, ->; - /// Source client trait. #[async_trait] pub trait SourceClient: RelayClient { + /// Type of batch transaction that submits finality and message receiving proof. + type BatchTransaction: BatchTransaction< + TargetHeaderIdOf

, + P::MessagesReceivingProof, + Self::TransactionTracker, + Self::Error, + >; /// Transaction tracker to track submitted transactions. type TransactionTracker: TransactionTracker>; @@ -198,12 +188,19 @@ pub trait SourceClient: RelayClient { async fn require_target_header_on_source( &self, id: TargetHeaderIdOf

, - ) -> Option>; + ) -> Option; } /// Target client trait. #[async_trait] pub trait TargetClient: RelayClient { + /// Type of batch transaction that submits finality and messages proof. + type BatchTransaction: BatchTransaction< + SourceHeaderIdOf

, + P::MessagesProof, + Self::TransactionTracker, + Self::Error, + >; /// Transaction tracker to track submitted transactions. type TransactionTracker: TransactionTracker>; @@ -253,7 +250,7 @@ pub trait TargetClient: RelayClient { async fn require_source_header_on_target( &self, id: SourceHeaderIdOf

, - ) -> Option>; + ) -> Option; } /// State of the client. @@ -535,6 +532,32 @@ pub(crate) mod tests { type TargetHeaderHash = TestTargetHeaderHash; } + pub struct TestBatchTransaction( + std::marker::PhantomData<(HeaderId, Proof, TransactionTracker, Error)>, + ); + + #[async_trait] + impl + BatchTransaction + for TestBatchTransaction + where + HeaderId: Send, + Proof: Send, + TransactionTracker: Send, + Error: Send, + { + fn required_header_id(&self) -> HeaderId { + unimplemented!("TODO") + } + + async fn append_proof_and_send( + self, + _proof: Proof, + ) -> Result, Error> { + unimplemented!("TODO") + } + } + #[derive(Clone, Debug)] pub struct TestTransactionTracker(TrackedTransactionStatus); @@ -640,6 +663,12 @@ pub(crate) mod tests { #[async_trait] impl SourceClient for TestSourceClient { + type BatchTransaction = TestBatchTransaction< + TargetHeaderIdOf, + TestMessagesReceivingProof, + TestTransactionTracker, + TestError, + >; type TransactionTracker = TestTransactionTracker; async fn state(&self) -> Result, TestError> { @@ -739,7 +768,7 @@ pub(crate) mod tests { async fn require_target_header_on_source( &self, id: TargetHeaderIdOf, - ) -> Option> { + ) -> Option { let mut data = self.data.lock(); data.target_to_source_header_required = Some(id); data.target_to_source_header_requirements.push(id); @@ -783,6 +812,12 @@ pub(crate) mod tests { #[async_trait] impl TargetClient for TestTargetClient { + type BatchTransaction = TestBatchTransaction< + SourceHeaderIdOf, + TestMessagesProof, + TestTransactionTracker, + TestError, + >; type TransactionTracker = TestTransactionTracker; async fn state(&self) -> Result, TestError> { @@ -873,7 +908,7 @@ pub(crate) mod tests { async fn require_source_header_on_target( &self, id: SourceHeaderIdOf, - ) -> Option> { + ) -> Option { let mut data = self.data.lock(); data.source_to_target_header_required = Some(id); data.source_to_target_header_requirements.push(id); diff --git a/relays/messages/src/message_race_delivery.rs b/relays/messages/src/message_race_delivery.rs index ca61965014..bdb89dd6fe 100644 --- a/relays/messages/src/message_race_delivery.rs +++ b/relays/messages/src/message_race_delivery.rs @@ -24,9 +24,9 @@ use relay_utils::FailedClient; use crate::{ message_lane::{MessageLane, SourceHeaderIdOf, TargetHeaderIdOf}, message_lane_loop::{ - BatchTransactionOfTargetClient, MessageDeliveryParams, MessageDetailsMap, - MessageProofParameters, NoncesSubmitArtifacts, SourceClient as MessageLaneSourceClient, - SourceClientState, TargetClient as MessageLaneTargetClient, TargetClientState, + MessageDeliveryParams, MessageDetailsMap, MessageProofParameters, NoncesSubmitArtifacts, + SourceClient as MessageLaneSourceClient, SourceClientState, + TargetClient as MessageLaneTargetClient, TargetClientState, }, message_race_limits::{MessageRaceLimits, RelayMessagesBatchReference}, message_race_loop::{ @@ -171,12 +171,10 @@ where { type Error = C::Error; type TargetNoncesData = DeliveryRaceTargetNoncesData; + type BatchTransaction = C::BatchTransaction; type TransactionTracker = C::TransactionTracker; - async fn require_source_header( - &self, - id: SourceHeaderIdOf

, - ) -> Option> { + async fn require_source_header(&self, id: SourceHeaderIdOf

) -> Option { self.client.require_source_header_on_target(id).await } diff --git a/relays/messages/src/message_race_loop.rs b/relays/messages/src/message_race_loop.rs index 8c2e14590c..13dcc2612e 100644 --- a/relays/messages/src/message_race_loop.rs +++ b/relays/messages/src/message_race_loop.rs @@ -127,6 +127,13 @@ pub trait TargetClient { type Error: std::fmt::Debug + MaybeConnectionError; /// Type of the additional data from the target client, used by the race. type TargetNoncesData: std::fmt::Debug; + /// Type of batch transaction that submits finality and proof to the target node. + type BatchTransaction: BatchTransaction< + P::SourceHeaderId, + P::Proof, + Self::TransactionTracker, + Self::Error, + >; /// Transaction tracker to track submitted transactions. type TransactionTracker: TransactionTracker; @@ -140,19 +147,7 @@ pub trait TargetClient { /// If function has returned `None`, it means that the caller now must wait for the /// appearance of the required header `id` at the target client. #[must_use] - async fn require_source_header( - &self, - id: P::SourceHeaderId, - ) -> Option< - Box< - dyn BatchTransaction< - P::SourceHeaderId, - P::Proof, - Self::TransactionTracker, - Self::Error, - >, - >, - >; + async fn require_source_header(&self, id: P::SourceHeaderId) -> Option; /// Return nonces that are known to the target client. async fn nonces( @@ -270,6 +265,7 @@ pub async fn run, TC: TargetClient

>( let mut target_client_is_online = true; let mut target_best_nonces_required = false; let mut target_finalized_nonces_required = false; + let mut target_batch_transaction = None; let target_best_nonces = futures::future::Fuse::terminated(); let target_finalized_nonces = futures::future::Fuse::terminated(); let target_submit_proof = futures::future::Fuse::terminated(); @@ -349,9 +345,11 @@ pub async fn run, TC: TargetClient

>( let required_source_header_id = race_state .best_finalized_source_header_id_at_best_target .as_ref() - .and_then(|best|strategy.required_source_header_at_target(best)); + .and_then(|best| strategy.required_source_header_at_target(best)); if let Some(required_source_header_id) = required_source_header_id { - race_target.require_source_header(required_source_header_id).await; + target_batch_transaction = race_target + .require_source_header(required_source_header_id) + .await; } }, nonces = target_best_nonces => { @@ -429,6 +427,7 @@ pub async fn run, TC: TargetClient

>( P::target_name(), ); + target_batch_transaction = None; race_state.nonces_to_submit = None; race_state.nonces_submitted = Some(artifacts.nonces); target_tx_tracker.set(artifacts.tx_tracker.wait().fuse()); @@ -499,8 +498,23 @@ pub async fn run, TC: TargetClient

>( if source_client_is_online { source_client_is_online = false; + // if we've started to submit batch transaction, let's prioritize it + let expected_race_state = + if let Some(ref target_batch_transaction) = target_batch_transaction { + // when selecting nonces for the batch transaction, we assume that the required + // source header is already at the target chain + let required_source_header_at_target = + target_batch_transaction.required_header_id(); + let mut expected_race_state = race_state.clone(); + expected_race_state.best_finalized_source_header_id_at_best_target = + Some(required_source_header_at_target); + expected_race_state + } else { + race_state.clone() + }; + let nonces_to_deliver = - select_nonces_to_deliver(race_state.clone(), &mut strategy).await; + select_nonces_to_deliver(expected_race_state, &mut strategy).await; let best_at_source = strategy.best_at_source(); if let Some((at_block, nonces_range, proof_parameters)) = nonces_to_deliver { @@ -544,11 +558,17 @@ pub async fn run, TC: TargetClient

>( nonces_range, P::target_name(), ); - target_submit_proof.set( - race_target - .submit_proof(at_block.clone(), nonces_range.clone(), proof.clone()) - .fuse(), - ); + + if let Some(target_batch_transaction) = target_batch_transaction.take() { + target_submit_proof + .set(target_batch_transaction.append_proof_and_send(proof.clone()).fuse()); + } else { + target_submit_proof.set( + race_target + .submit_proof(at_block.clone(), nonces_range.clone(), proof.clone()) + .fuse(), + ); + } } else if target_best_nonces_required { log::debug!(target: "bridge", "Asking {} about best message nonces", P::target_name()); let at_block = race_state diff --git a/relays/messages/src/message_race_receiving.rs b/relays/messages/src/message_race_receiving.rs index a0f4f66ed1..2fbef5defa 100644 --- a/relays/messages/src/message_race_receiving.rs +++ b/relays/messages/src/message_race_receiving.rs @@ -16,8 +16,7 @@ use crate::{ message_lane::{MessageLane, SourceHeaderIdOf, TargetHeaderIdOf}, message_lane_loop::{ - BatchTransactionOfSourceClient, NoncesSubmitArtifacts, - SourceClient as MessageLaneSourceClient, SourceClientState, + NoncesSubmitArtifacts, SourceClient as MessageLaneSourceClient, SourceClientState, TargetClient as MessageLaneTargetClient, TargetClientState, }, message_race_loop::{ @@ -156,12 +155,10 @@ where { type Error = C::Error; type TargetNoncesData = (); + type BatchTransaction = C::BatchTransaction; type TransactionTracker = C::TransactionTracker; - async fn require_source_header( - &self, - id: TargetHeaderIdOf

, - ) -> Option> { + async fn require_source_header(&self, id: TargetHeaderIdOf

) -> Option { self.client.require_target_header_on_source(id).await } From feba6fa3c44dd7155904c06d56e634d1bd564d06 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Wed, 30 Nov 2022 14:24:25 +0300 Subject: [PATCH 03/29] test batch transactions --- relays/messages/src/message_lane_loop.rs | 113 ++++++++++++++--------- 1 file changed, 71 insertions(+), 42 deletions(-) diff --git a/relays/messages/src/message_lane_loop.rs b/relays/messages/src/message_lane_loop.rs index f3a3d4b982..0ec274573d 100644 --- a/relays/messages/src/message_lane_loop.rs +++ b/relays/messages/src/message_lane_loop.rs @@ -532,29 +532,58 @@ pub(crate) mod tests { type TargetHeaderHash = TestTargetHeaderHash; } - pub struct TestBatchTransaction( - std::marker::PhantomData<(HeaderId, Proof, TransactionTracker, Error)>, - ); + pub struct TestMessagesBatchTransaction { + data: Arc>, + required_header_id: TestSourceHeaderId, + tx_tracker: TestTransactionTracker, + } #[async_trait] - impl - BatchTransaction - for TestBatchTransaction - where - HeaderId: Send, - Proof: Send, - TransactionTracker: Send, - Error: Send, + impl BatchTransaction + for TestMessagesBatchTransaction + { + fn required_header_id(&self) -> TestSourceHeaderId { + self.required_header_id + } + + async fn append_proof_and_send( + self, + proof: TestMessagesProof, + ) -> Result, TestError> { + let nonces = proof.0.clone(); + let mut data = self.data.lock(); + data.receive_messages(proof); + Ok(NoncesSubmitArtifacts { nonces, tx_tracker: self.tx_tracker }) + } + } + + pub struct TestConfirmationBatchTransaction { + data: Arc>, + required_header_id: TestTargetHeaderId, + tx_tracker: TestTransactionTracker, + } + + #[async_trait] + impl + BatchTransaction< + TestTargetHeaderId, + TestMessagesReceivingProof, + TestTransactionTracker, + TestError, + > for TestConfirmationBatchTransaction { - fn required_header_id(&self) -> HeaderId { - unimplemented!("TODO") + fn required_header_id(&self) -> TestTargetHeaderId { + self.required_header_id } async fn append_proof_and_send( self, - _proof: Proof, - ) -> Result, Error> { - unimplemented!("TODO") + proof: TestMessagesReceivingProof, + ) -> Result, TestError> { + let nonces = proof..=proof; + let mut data = self.data.lock(); + data.receive_messages_delivery_proof(proof); + Ok(NoncesSubmitArtifacts { nonces, tx_tracker: self.tx_tracker }) } } @@ -629,6 +658,28 @@ pub(crate) mod tests { } } + impl TestClientData { + fn receive_messages(&mut self, proof: TestMessagesProof) { + self.target_state.best_self = + HeaderId(self.target_state.best_self.0 + 1, self.target_state.best_self.1 + 1); + self.target_state.best_finalized_self = self.target_state.best_self; + self.target_latest_received_nonce = *proof.0.end(); + if let Some(target_latest_confirmed_received_nonce) = proof.1 { + self.target_latest_confirmed_received_nonce = + target_latest_confirmed_received_nonce; + } + self.submitted_messages_proofs.push(proof); + } + + fn receive_messages_delivery_proof(&mut self, proof: TestMessagesReceivingProof) { + self.source_state.best_self = + HeaderId(self.source_state.best_self.0 + 1, self.source_state.best_self.1 + 1); + self.source_state.best_finalized_self = self.source_state.best_self; + self.submitted_messages_receiving_proofs.push(proof); + self.source_latest_confirmed_received_nonce = proof; + } + } + #[derive(Clone)] pub struct TestSourceClient { data: Arc>, @@ -663,12 +714,7 @@ pub(crate) mod tests { #[async_trait] impl SourceClient for TestSourceClient { - type BatchTransaction = TestBatchTransaction< - TargetHeaderIdOf, - TestMessagesReceivingProof, - TestTransactionTracker, - TestError, - >; + type BatchTransaction = TestConfirmationBatchTransaction; type TransactionTracker = TestTransactionTracker; async fn state(&self) -> Result, TestError> { @@ -756,11 +802,7 @@ pub(crate) mod tests { ) -> Result { let mut data = self.data.lock(); (self.tick)(&mut data); - data.source_state.best_self = - HeaderId(data.source_state.best_self.0 + 1, data.source_state.best_self.1 + 1); - data.source_state.best_finalized_self = data.source_state.best_self; - data.submitted_messages_receiving_proofs.push(proof); - data.source_latest_confirmed_received_nonce = proof; + data.receive_messages_delivery_proof(proof); (self.post_tick)(&mut data); Ok(TestTransactionTracker(data.source_tracked_transaction_status)) } @@ -812,12 +854,7 @@ pub(crate) mod tests { #[async_trait] impl TargetClient for TestTargetClient { - type BatchTransaction = TestBatchTransaction< - SourceHeaderIdOf, - TestMessagesProof, - TestTransactionTracker, - TestError, - >; + type BatchTransaction = TestMessagesBatchTransaction; type TransactionTracker = TestTransactionTracker; async fn state(&self) -> Result, TestError> { @@ -889,15 +926,7 @@ pub(crate) mod tests { if data.is_target_fails { return Err(TestError) } - data.target_state.best_self = - HeaderId(data.target_state.best_self.0 + 1, data.target_state.best_self.1 + 1); - data.target_state.best_finalized_self = data.target_state.best_self; - data.target_latest_received_nonce = *proof.0.end(); - if let Some(target_latest_confirmed_received_nonce) = proof.1 { - data.target_latest_confirmed_received_nonce = - target_latest_confirmed_received_nonce; - } - data.submitted_messages_proofs.push(proof); + data.receive_messages(proof); (self.post_tick)(&mut data); Ok(NoncesSubmitArtifacts { nonces, From e372820e22066721bb1a87884c8e05e24aae2103 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Wed, 30 Nov 2022 15:15:03 +0300 Subject: [PATCH 04/29] message_lane_loop_works_with_batch_transactions --- relays/messages/src/message_lane_loop.rs | 109 ++++++++++++++++++++--- relays/messages/src/message_race_loop.rs | 22 +++-- 2 files changed, 111 insertions(+), 20 deletions(-) diff --git a/relays/messages/src/message_lane_loop.rs b/relays/messages/src/message_lane_loop.rs index 0ec274573d..e37b2acbb7 100644 --- a/relays/messages/src/message_lane_loop.rs +++ b/relays/messages/src/message_lane_loop.rs @@ -532,6 +532,7 @@ pub(crate) mod tests { type TargetHeaderHash = TestTargetHeaderHash; } + #[derive(Clone, Debug)] pub struct TestMessagesBatchTransaction { data: Arc>, required_header_id: TestSourceHeaderId, @@ -557,6 +558,7 @@ pub(crate) mod tests { } } + #[derive(Clone, Debug)] pub struct TestConfirmationBatchTransaction { data: Arc>, required_header_id: TestTargetHeaderId, @@ -621,8 +623,10 @@ pub(crate) mod tests { target_latest_confirmed_received_nonce: MessageNonce, target_tracked_transaction_status: TrackedTransactionStatus, submitted_messages_proofs: Vec, + target_to_source_batch_transaction: Option, target_to_source_header_required: Option, target_to_source_header_requirements: Vec, + source_to_target_batch_transaction: Option, source_to_target_header_required: Option, source_to_target_header_requirements: Vec, } @@ -650,8 +654,10 @@ pub(crate) mod tests { Default::default(), )), submitted_messages_proofs: Vec::new(), + target_to_source_batch_transaction: None, target_to_source_header_required: None, target_to_source_header_requirements: Vec::new(), + source_to_target_batch_transaction: None, source_to_target_header_required: None, source_to_target_header_requirements: Vec::new(), } @@ -816,7 +822,11 @@ pub(crate) mod tests { data.target_to_source_header_requirements.push(id); (self.tick)(&mut data); (self.post_tick)(&mut data); - None + + data.target_to_source_batch_transaction.take().map(|mut tx| { + tx.required_header_id = id; + tx + }) } } @@ -943,12 +953,16 @@ pub(crate) mod tests { data.source_to_target_header_requirements.push(id); (self.tick)(&mut data); (self.post_tick)(&mut data); - None + + data.source_to_target_batch_transaction.take().map(|mut tx| { + tx.required_header_id = id; + tx + }) } } fn run_loop_test( - data: TestClientData, + data: Arc>, source_tick: Arc, source_post_tick: Arc, target_tick: Arc, @@ -956,8 +970,6 @@ pub(crate) mod tests { exit_signal: impl Future + 'static + Send, ) -> TestClientData { async_std::task::block_on(async { - let data = Arc::new(Mutex::new(data)); - let source_client = TestSourceClient { data: data.clone(), tick: source_tick, @@ -1000,7 +1012,7 @@ pub(crate) mod tests { // able to deliver messages. let (exit_sender, exit_receiver) = unbounded(); let result = run_loop_test( - TestClientData { + Arc::new(Mutex::new(TestClientData { is_source_fails: true, source_state: ClientState { best_self: HeaderId(0, 0), @@ -1017,7 +1029,7 @@ pub(crate) mod tests { }, target_latest_received_nonce: 0, ..Default::default() - }, + })), Arc::new(|data: &mut TestClientData| { if data.is_source_reconnected { data.is_source_fails = false; @@ -1053,7 +1065,7 @@ pub(crate) mod tests { let (source_exit_sender, exit_receiver) = unbounded(); let target_exit_sender = source_exit_sender.clone(); let result = run_loop_test( - TestClientData { + Arc::new(Mutex::new(TestClientData { source_state: ClientState { best_self: HeaderId(0, 0), best_finalized_self: HeaderId(0, 0), @@ -1071,7 +1083,7 @@ pub(crate) mod tests { target_latest_received_nonce: 0, target_tracked_transaction_status: TrackedTransactionStatus::Lost, ..Default::default() - }, + })), Arc::new(move |data: &mut TestClientData| { if data.is_source_reconnected { data.source_tracked_transaction_status = @@ -1104,7 +1116,7 @@ pub(crate) mod tests { // their corresponding nonce won't be udpated => reconnect will happen let (exit_sender, exit_receiver) = unbounded(); let result = run_loop_test( - TestClientData { + Arc::new(Mutex::new(TestClientData { source_state: ClientState { best_self: HeaderId(0, 0), best_finalized_self: HeaderId(0, 0), @@ -1120,7 +1132,7 @@ pub(crate) mod tests { }, target_latest_received_nonce: 0, ..Default::default() - }, + })), Arc::new(move |data: &mut TestClientData| { // blocks are produced on every tick data.source_state.best_self = @@ -1178,7 +1190,7 @@ pub(crate) mod tests { fn message_lane_loop_works() { let (exit_sender, exit_receiver) = unbounded(); let result = run_loop_test( - TestClientData { + Arc::new(Mutex::new(TestClientData { source_state: ClientState { best_self: HeaderId(10, 10), best_finalized_self: HeaderId(10, 10), @@ -1194,7 +1206,7 @@ pub(crate) mod tests { }, target_latest_received_nonce: 0, ..Default::default() - }, + })), Arc::new(|data: &mut TestClientData| { // blocks are produced on every tick data.source_state.best_self = @@ -1257,4 +1269,75 @@ pub(crate) mod tests { assert!(!result.target_to_source_header_requirements.is_empty()); assert!(!result.source_to_target_header_requirements.is_empty()); } + + #[test] + fn message_lane_loop_works_with_batch_transactions() { + let _ = env_logger::try_init(); + let (exit_sender, exit_receiver) = unbounded(); + let original_data = Arc::new(Mutex::new(TestClientData { + source_state: ClientState { + best_self: HeaderId(10, 10), + best_finalized_self: HeaderId(10, 10), + best_finalized_peer_at_best_self: HeaderId(0, 0), + actual_best_finalized_peer_at_best_self: HeaderId(0, 0), + }, + source_latest_generated_nonce: 10, + target_state: ClientState { + best_self: HeaderId(0, 0), + best_finalized_self: HeaderId(0, 0), + best_finalized_peer_at_best_self: HeaderId(0, 0), + actual_best_finalized_peer_at_best_self: HeaderId(0, 0), + }, + target_latest_received_nonce: 0, + ..Default::default() + })); + let target_original_data = original_data.clone(); + let source_original_data = original_data.clone(); + let result = run_loop_test( + original_data, + Arc::new(|_| {}), + Arc::new(move |data: &mut TestClientData| { + if let Some(target_to_source_header_required) = + data.target_to_source_header_required.take() + { + data.target_to_source_batch_transaction = + Some(TestConfirmationBatchTransaction { + data: source_original_data.clone(), + required_header_id: target_to_source_header_required, + tx_tracker: TestTransactionTracker::default(), + }) + } + }), + Arc::new(|_| {}), + Arc::new(move |data: &mut TestClientData| { + if let Some(source_to_target_header_required) = + data.source_to_target_header_required.take() + { + data.source_to_target_batch_transaction = Some(TestMessagesBatchTransaction { + data: target_original_data.clone(), + required_header_id: source_to_target_header_required, + tx_tracker: TestTransactionTracker::default(), + }) + } + + if data.source_latest_confirmed_received_nonce == 10 { + exit_sender.unbounded_send(()).unwrap(); + } + }), + exit_receiver.into_future().map(|(_, _)| ()), + ); + + // there are no strict restrictions on when reward confirmation should come + // (because `max_unconfirmed_nonces_at_target` is `100` in tests and this confirmation + // depends on the state of both clients) + // => we do not check it here + assert_eq!(result.submitted_messages_proofs[0].0, 1..=4); + assert_eq!(result.submitted_messages_proofs[1].0, 5..=8); + assert_eq!(result.submitted_messages_proofs[2].0, 9..=10); + assert!(!result.submitted_messages_receiving_proofs.is_empty()); + + // check that we have at least once required new source->target or target->source headers + assert!(!result.target_to_source_header_requirements.is_empty()); + assert!(!result.source_to_target_header_requirements.is_empty()); + } } diff --git a/relays/messages/src/message_race_loop.rs b/relays/messages/src/message_race_loop.rs index 13dcc2612e..36ebd48712 100644 --- a/relays/messages/src/message_race_loop.rs +++ b/relays/messages/src/message_race_loop.rs @@ -552,17 +552,25 @@ pub async fn run, TC: TargetClient

>( target_client_is_online = false; if let Some((at_block, nonces_range, proof)) = race_state.nonces_to_submit.as_ref() { - log::debug!( - target: "bridge", - "Going to submit proof of messages in range {:?} to {} node", - nonces_range, - P::target_name(), - ); - if let Some(target_batch_transaction) = target_batch_transaction.take() { + log::debug!( + target: "bridge", + "Going to submit batch transaction with header {:?} and proof of messages in range {:?} to {} node", + target_batch_transaction.required_header_id(), + nonces_range, + P::target_name(), + ); + target_submit_proof .set(target_batch_transaction.append_proof_and_send(proof.clone()).fuse()); } else { + log::debug!( + target: "bridge", + "Going to submit proof of messages in range {:?} to {} node", + nonces_range, + P::target_name(), + ); + target_submit_proof.set( race_target .submit_proof(at_block.clone(), nonces_range.clone(), proof.clone()) From 824fc23c0717e3c7b59038c18d4e9a67c9ec70e4 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Wed, 30 Nov 2022 15:48:27 +0300 Subject: [PATCH 05/29] removed logger --- relays/messages/src/message_lane_loop.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/relays/messages/src/message_lane_loop.rs b/relays/messages/src/message_lane_loop.rs index e37b2acbb7..add568da01 100644 --- a/relays/messages/src/message_lane_loop.rs +++ b/relays/messages/src/message_lane_loop.rs @@ -1272,7 +1272,6 @@ pub(crate) mod tests { #[test] fn message_lane_loop_works_with_batch_transactions() { - let _ = env_logger::try_init(); let (exit_sender, exit_receiver) = unbounded(); let original_data = Arc::new(Mutex::new(TestClientData { source_state: ClientState { From f8730aa6a258e135690ca9ca1fc9e4ede99c3b48 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Thu, 1 Dec 2022 10:33:18 +0300 Subject: [PATCH 06/29] BatchConfirmationTransaction + BatchDeliveryTransaction --- .../lib-substrate-relay/src/messages_lane.rs | 2 +- .../src/messages_source.rs | 35 +++++++++++++++++-- .../src/messages_target.rs | 33 +++++++++++++++-- 3 files changed, 64 insertions(+), 6 deletions(-) diff --git a/relays/lib-substrate-relay/src/messages_lane.rs b/relays/lib-substrate-relay/src/messages_lane.rs index da138a3d12..7617f7121a 100644 --- a/relays/lib-substrate-relay/src/messages_lane.rs +++ b/relays/lib-substrate-relay/src/messages_lane.rs @@ -59,7 +59,7 @@ pub trait SubstrateMessageLane: 'static + Clone + Debug + Send + Sync { /// Adapter that allows all `SubstrateMessageLane` to act as `MessageLane`. #[derive(Clone, Debug)] -pub(crate) struct MessageLaneAdapter { +pub struct MessageLaneAdapter { _phantom: PhantomData

, } diff --git a/relays/lib-substrate-relay/src/messages_source.rs b/relays/lib-substrate-relay/src/messages_source.rs index bf3779b3b4..e53408ae49 100644 --- a/relays/lib-substrate-relay/src/messages_source.rs +++ b/relays/lib-substrate-relay/src/messages_source.rs @@ -41,8 +41,8 @@ use frame_support::weights::Weight; use messages_relay::{ message_lane::{MessageLane, SourceHeaderIdOf, TargetHeaderIdOf}, message_lane_loop::{ - ClientState, MessageDetails, MessageDetailsMap, MessageProofParameters, SourceClient, - SourceClientState, + BatchTransaction, ClientState, MessageDetails, MessageDetailsMap, MessageProofParameters, NoncesSubmitArtifacts, + SourceClient, SourceClientState, }, }; use num_traits::Zero; @@ -140,6 +140,7 @@ impl SourceClient> for SubstrateM where AccountIdOf: From< as Pair>::Public>, { + type BatchTransaction = BatchConfirmationTransaction

; type TransactionTracker = TransactionTracker>; async fn state(&self) -> Result>, SubstrateError> { @@ -360,10 +361,38 @@ where .await } - async fn require_target_header_on_source(&self, id: TargetHeaderIdOf>) { + async fn require_target_header_on_source(&self, id: TargetHeaderIdOf>) -> Option { if let Some(ref target_to_source_headers_relay) = self.target_to_source_headers_relay { target_to_source_headers_relay.require_more_headers(id.0).await; + // TODO: return batch transaction if possible } + + None + } +} + +/// Batch transaction that brings target headers + and delivery confirmations to the source node. +pub struct BatchConfirmationTransaction { + source_client: SubstrateMessagesSource

, + required_target_header_on_source: TargetHeaderIdOf>, +} + +#[async_trait] +impl BatchTransaction< + TargetHeaderIdOf>, + as MessageLane>::MessagesReceivingProof, + TransactionTracker>, + SubstrateError, +> for BatchConfirmationTransaction

{ + fn required_header_id(&self) -> TargetHeaderIdOf> { + self.required_target_header_on_source.clone() + } + + async fn append_proof_and_send( + self, + _proof: as MessageLane>::MessagesReceivingProof, + ) -> Result>>, SubstrateError> { + unimplemented!("TODO") } } diff --git a/relays/lib-substrate-relay/src/messages_target.rs b/relays/lib-substrate-relay/src/messages_target.rs index 22a50acf37..affcc8f79c 100644 --- a/relays/lib-substrate-relay/src/messages_target.rs +++ b/relays/lib-substrate-relay/src/messages_target.rs @@ -34,7 +34,7 @@ use bp_messages::{ use bridge_runtime_common::messages::source::FromBridgedChainMessagesDeliveryProof; use messages_relay::{ message_lane::{MessageLane, SourceHeaderIdOf, TargetHeaderIdOf}, - message_lane_loop::{NoncesSubmitArtifacts, TargetClient, TargetClientState}, + message_lane_loop::{BatchTransaction, NoncesSubmitArtifacts, TargetClient, TargetClientState}, }; use relay_substrate_client::{ AccountIdOf, AccountKeyPairOf, BalanceOf, BlockNumberOf, Chain, ChainWithMessages, Client, @@ -132,6 +132,7 @@ where AccountIdOf: From< as Pair>::Public>, BalanceOf: TryFrom>, { + type BatchTransaction = BatchDeliveryTransaction

; type TransactionTracker = TransactionTracker>; async fn state(&self) -> Result>, SubstrateError> { @@ -267,10 +268,38 @@ where Ok(NoncesSubmitArtifacts { nonces, tx_tracker }) } - async fn require_source_header_on_target(&self, id: SourceHeaderIdOf>) { + async fn require_source_header_on_target(&self, id: SourceHeaderIdOf>) -> Option { if let Some(ref source_to_target_headers_relay) = self.source_to_target_headers_relay { source_to_target_headers_relay.require_more_headers(id.0).await; + // TODO: return batch transaction if possible } + + None + } +} + +/// Batch transaction that brings target headers + and delivery confirmations to the source node. +pub struct BatchDeliveryTransaction { + target_client: SubstrateMessagesTarget

, + required_source_header_on_target: SourceHeaderIdOf>, +} + +#[async_trait] +impl BatchTransaction< +SourceHeaderIdOf>, + as MessageLane>::MessagesProof, + TransactionTracker>, + SubstrateError, +> for BatchDeliveryTransaction

{ + fn required_header_id(&self) -> SourceHeaderIdOf> { + self.required_source_header_on_target.clone() + } + + async fn append_proof_and_send( + self, + _proof: as MessageLane>::MessagesProof, + ) -> Result>>, SubstrateError> { + unimplemented!("TODO") } } From 9cc4fe35b1b86dbb527ce19020e161bf572135c0 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Thu, 1 Dec 2022 12:15:59 +0300 Subject: [PATCH 07/29] more prototyping --- .../lib-substrate-relay/src/messages_lane.rs | 14 +++++ .../src/messages_source.rs | 59 ++++++++++++++++--- .../src/messages_target.rs | 19 ++++-- relays/messages/src/message_lane_loop.rs | 8 +-- relays/messages/src/message_race_loop.rs | 10 +++- 5 files changed, 90 insertions(+), 20 deletions(-) diff --git a/relays/lib-substrate-relay/src/messages_lane.rs b/relays/lib-substrate-relay/src/messages_lane.rs index 7617f7121a..4ef47ea79a 100644 --- a/relays/lib-substrate-relay/src/messages_lane.rs +++ b/relays/lib-substrate-relay/src/messages_lane.rs @@ -55,6 +55,20 @@ pub trait SubstrateMessageLane: 'static + Clone + Debug + Send + Sync { type ReceiveMessagesProofCallBuilder: ReceiveMessagesProofCallBuilder; /// How receive messages delivery proof call is built? type ReceiveMessagesDeliveryProofCallBuilder: ReceiveMessagesDeliveryProofCallBuilder; + + /// How batch calls are built at the source chain? + type SourceBatchCallBuilder: BatchCallBuilder; + /// How batch calls are built at the target chain? + type TargetBatchCallBuilder: BatchCallBuilder; +} + +/// Batch call builder. +pub trait BatchCallBuilder { + /// If `true`, then batch calls are supported at the chain. + const BATCH_CALL_SUPPORTED: bool; + + /// Create batch call from given calls vector. + fn build_batch_call(calls: Vec>) -> CallOf; } /// Adapter that allows all `SubstrateMessageLane` to act as `MessageLane`. diff --git a/relays/lib-substrate-relay/src/messages_source.rs b/relays/lib-substrate-relay/src/messages_source.rs index e53408ae49..d86d18476e 100644 --- a/relays/lib-substrate-relay/src/messages_source.rs +++ b/relays/lib-substrate-relay/src/messages_source.rs @@ -20,7 +20,7 @@ use crate::{ messages_lane::{ - MessageLaneAdapter, ReceiveMessagesDeliveryProofCallBuilder, SubstrateMessageLane, + BatchCallBuilder, MessageLaneAdapter, ReceiveMessagesDeliveryProofCallBuilder, SubstrateMessageLane, }, messages_target::SubstrateMessagesDeliveryProof, on_demand::OnDemandRelay, @@ -41,7 +41,7 @@ use frame_support::weights::Weight; use messages_relay::{ message_lane::{MessageLane, SourceHeaderIdOf, TargetHeaderIdOf}, message_lane_loop::{ - BatchTransaction, ClientState, MessageDetails, MessageDetailsMap, MessageProofParameters, NoncesSubmitArtifacts, + BatchTransaction, ClientState, MessageDetails, MessageDetailsMap, MessageProofParameters, SourceClient, SourceClientState, }, }; @@ -361,10 +361,19 @@ where .await } - async fn require_target_header_on_source(&self, id: TargetHeaderIdOf>) -> Option { + async fn require_target_header_on_source( + &self, + id: TargetHeaderIdOf>, + ) -> Option { if let Some(ref target_to_source_headers_relay) = self.target_to_source_headers_relay { + if P::SourceBatchCallBuilder::BATCH_CALL_SUPPORTED { + return Some(BatchConfirmationTransaction::

{ + messages_source: self.clone(), + required_target_header_on_source: id, + }); + } + target_to_source_headers_relay.require_more_headers(id.0).await; - // TODO: return batch transaction if possible } None @@ -373,7 +382,7 @@ where /// Batch transaction that brings target headers + and delivery confirmations to the source node. pub struct BatchConfirmationTransaction { - source_client: SubstrateMessagesSource

, + messages_source: SubstrateMessagesSource

, required_target_header_on_source: TargetHeaderIdOf>, } @@ -383,16 +392,48 @@ impl BatchTransaction< as MessageLane>::MessagesReceivingProof, TransactionTracker>, SubstrateError, -> for BatchConfirmationTransaction

{ +> for BatchConfirmationTransaction

where + AccountIdOf: From< as Pair>::Public>, +{ fn required_header_id(&self) -> TargetHeaderIdOf> { self.required_target_header_on_source.clone() } async fn append_proof_and_send( self, - _proof: as MessageLane>::MessagesReceivingProof, - ) -> Result>>, SubstrateError> { - unimplemented!("TODO") + proof: as MessageLane>::MessagesReceivingProof, + ) -> Result>, SubstrateError> { + let mut calls = self + .messages_source + .target_to_source_headers_relay + .as_ref() + .expect("BatchConfirmationTransaction is only created when target_to_source_headers_relay is Some; qed") + .prove_header(self.required_target_header_on_source) + .await?; + calls.push(P::ReceiveMessagesDeliveryProofCallBuilder::build_receive_messages_delivery_proof_call( + proof, + false, + )); + + let genesis_hash = *self.messages_source.source_client.genesis_hash(); + let transaction_params = self.messages_source.transaction_params.clone(); + let (spec_version, transaction_version) = + self.messages_source.source_client.simple_runtime_version().await?; + self.messages_source.source_client + .submit_and_watch_signed_extrinsic( + self.messages_source.transaction_params.signer.public().into(), + SignParam:: { + spec_version, + transaction_version, + genesis_hash, + signer: self.messages_source.transaction_params.signer.clone(), + }, + move |best_block_id, transaction_nonce| { + Ok(UnsignedTransaction::new(calls.into(), transaction_nonce) + .era(TransactionEra::new(best_block_id, self.messages_source.transaction_params.mortality))) + }, + ) + .await } } diff --git a/relays/lib-substrate-relay/src/messages_target.rs b/relays/lib-substrate-relay/src/messages_target.rs index affcc8f79c..73c8da8fc7 100644 --- a/relays/lib-substrate-relay/src/messages_target.rs +++ b/relays/lib-substrate-relay/src/messages_target.rs @@ -19,7 +19,7 @@ //! chain. use crate::{ - messages_lane::{MessageLaneAdapter, ReceiveMessagesProofCallBuilder, SubstrateMessageLane}, + messages_lane::{BatchCallBuilder, MessageLaneAdapter, ReceiveMessagesProofCallBuilder, SubstrateMessageLane}, messages_source::{ensure_messages_pallet_active, read_client_state, SubstrateMessagesProof}, on_demand::OnDemandRelay, TransactionParams, @@ -268,10 +268,19 @@ where Ok(NoncesSubmitArtifacts { nonces, tx_tracker }) } - async fn require_source_header_on_target(&self, id: SourceHeaderIdOf>) -> Option { + async fn require_source_header_on_target( + &self, + id: SourceHeaderIdOf>, + ) -> Option { if let Some(ref source_to_target_headers_relay) = self.source_to_target_headers_relay { + if P::TargetBatchCallBuilder::BATCH_CALL_SUPPORTED { + return Some(BatchDeliveryTransaction::

{ + messages_target: self.clone(), + required_source_header_on_target: id, + }); + } + source_to_target_headers_relay.require_more_headers(id.0).await; - // TODO: return batch transaction if possible } None @@ -280,7 +289,7 @@ where /// Batch transaction that brings target headers + and delivery confirmations to the source node. pub struct BatchDeliveryTransaction { - target_client: SubstrateMessagesTarget

, + messages_target: SubstrateMessagesTarget

, required_source_header_on_target: SourceHeaderIdOf>, } @@ -298,7 +307,7 @@ SourceHeaderIdOf>, async fn append_proof_and_send( self, _proof: as MessageLane>::MessagesProof, - ) -> Result>>, SubstrateError> { + ) -> Result>, SubstrateError> { unimplemented!("TODO") } } diff --git a/relays/messages/src/message_lane_loop.rs b/relays/messages/src/message_lane_loop.rs index add568da01..736a06e7e3 100644 --- a/relays/messages/src/message_lane_loop.rs +++ b/relays/messages/src/message_lane_loop.rs @@ -121,7 +121,7 @@ pub trait BatchTransaction: Send { async fn append_proof_and_send( self, proof: Proof, - ) -> Result, Error>; + ) -> Result; } /// Source client trait. @@ -551,10 +551,9 @@ pub(crate) mod tests { self, proof: TestMessagesProof, ) -> Result, TestError> { - let nonces = proof.0.clone(); let mut data = self.data.lock(); data.receive_messages(proof); - Ok(NoncesSubmitArtifacts { nonces, tx_tracker: self.tx_tracker }) + self.tx_tracker } } @@ -582,10 +581,9 @@ pub(crate) mod tests { self, proof: TestMessagesReceivingProof, ) -> Result, TestError> { - let nonces = proof..=proof; let mut data = self.data.lock(); data.receive_messages_delivery_proof(proof); - Ok(NoncesSubmitArtifacts { nonces, tx_tracker: self.tx_tracker }) + self.tx_tracker } } diff --git a/relays/messages/src/message_race_loop.rs b/relays/messages/src/message_race_loop.rs index 36ebd48712..02a87e3f84 100644 --- a/relays/messages/src/message_race_loop.rs +++ b/relays/messages/src/message_race_loop.rs @@ -561,8 +561,15 @@ pub async fn run, TC: TargetClient

>( P::target_name(), ); + let nonces = nonces_range.clone(); target_submit_proof - .set(target_batch_transaction.append_proof_and_send(proof.clone()).fuse()); + .set(target_batch_transaction + .append_proof_and_send(proof.clone()) + .map(|result| result.map(|tx_tracker| { + NoncesSubmitArtifacts { nonces, tx_tracker } + })) + .left_future() + .fuse()); } else { log::debug!( target: "bridge", @@ -574,6 +581,7 @@ pub async fn run, TC: TargetClient

>( target_submit_proof.set( race_target .submit_proof(at_block.clone(), nonces_range.clone(), proof.clone()) + .right_future() .fuse(), ); } From 14c1aa868f2e74a1e4e3cd784680fa9fe76aefba Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Thu, 1 Dec 2022 12:16:08 +0300 Subject: [PATCH 08/29] fmt --- .../src/messages_source.rs | 39 ++++++++++++------- .../src/messages_target.rs | 20 ++++++---- relays/messages/src/message_lane_loop.rs | 5 +-- relays/messages/src/message_race_loop.rs | 14 ++++--- 4 files changed, 45 insertions(+), 33 deletions(-) diff --git a/relays/lib-substrate-relay/src/messages_source.rs b/relays/lib-substrate-relay/src/messages_source.rs index d86d18476e..8cf7016875 100644 --- a/relays/lib-substrate-relay/src/messages_source.rs +++ b/relays/lib-substrate-relay/src/messages_source.rs @@ -20,7 +20,8 @@ use crate::{ messages_lane::{ - BatchCallBuilder, MessageLaneAdapter, ReceiveMessagesDeliveryProofCallBuilder, SubstrateMessageLane, + BatchCallBuilder, MessageLaneAdapter, ReceiveMessagesDeliveryProofCallBuilder, + SubstrateMessageLane, }, messages_target::SubstrateMessagesDeliveryProof, on_demand::OnDemandRelay, @@ -370,7 +371,7 @@ where return Some(BatchConfirmationTransaction::

{ messages_source: self.clone(), required_target_header_on_source: id, - }); + }) } target_to_source_headers_relay.require_more_headers(id.0).await; @@ -387,12 +388,14 @@ pub struct BatchConfirmationTransaction { } #[async_trait] -impl BatchTransaction< - TargetHeaderIdOf>, - as MessageLane>::MessagesReceivingProof, - TransactionTracker>, - SubstrateError, -> for BatchConfirmationTransaction

where +impl + BatchTransaction< + TargetHeaderIdOf>, + as MessageLane>::MessagesReceivingProof, + TransactionTracker>, + SubstrateError, + > for BatchConfirmationTransaction

+where AccountIdOf: From< as Pair>::Public>, { fn required_header_id(&self) -> TargetHeaderIdOf> { @@ -410,16 +413,18 @@ impl BatchTransaction< .expect("BatchConfirmationTransaction is only created when target_to_source_headers_relay is Some; qed") .prove_header(self.required_target_header_on_source) .await?; - calls.push(P::ReceiveMessagesDeliveryProofCallBuilder::build_receive_messages_delivery_proof_call( - proof, - false, - )); + calls.push( + P::ReceiveMessagesDeliveryProofCallBuilder::build_receive_messages_delivery_proof_call( + proof, false, + ), + ); let genesis_hash = *self.messages_source.source_client.genesis_hash(); let transaction_params = self.messages_source.transaction_params.clone(); let (spec_version, transaction_version) = self.messages_source.source_client.simple_runtime_version().await?; - self.messages_source.source_client + self.messages_source + .source_client .submit_and_watch_signed_extrinsic( self.messages_source.transaction_params.signer.public().into(), SignParam:: { @@ -429,8 +434,12 @@ impl BatchTransaction< signer: self.messages_source.transaction_params.signer.clone(), }, move |best_block_id, transaction_nonce| { - Ok(UnsignedTransaction::new(calls.into(), transaction_nonce) - .era(TransactionEra::new(best_block_id, self.messages_source.transaction_params.mortality))) + Ok(UnsignedTransaction::new(calls.into(), transaction_nonce).era( + TransactionEra::new( + best_block_id, + self.messages_source.transaction_params.mortality, + ), + )) }, ) .await diff --git a/relays/lib-substrate-relay/src/messages_target.rs b/relays/lib-substrate-relay/src/messages_target.rs index 73c8da8fc7..e6f4c061cb 100644 --- a/relays/lib-substrate-relay/src/messages_target.rs +++ b/relays/lib-substrate-relay/src/messages_target.rs @@ -19,7 +19,9 @@ //! chain. use crate::{ - messages_lane::{BatchCallBuilder, MessageLaneAdapter, ReceiveMessagesProofCallBuilder, SubstrateMessageLane}, + messages_lane::{ + BatchCallBuilder, MessageLaneAdapter, ReceiveMessagesProofCallBuilder, SubstrateMessageLane, + }, messages_source::{ensure_messages_pallet_active, read_client_state, SubstrateMessagesProof}, on_demand::OnDemandRelay, TransactionParams, @@ -277,7 +279,7 @@ where return Some(BatchDeliveryTransaction::

{ messages_target: self.clone(), required_source_header_on_target: id, - }); + }) } source_to_target_headers_relay.require_more_headers(id.0).await; @@ -294,12 +296,14 @@ pub struct BatchDeliveryTransaction { } #[async_trait] -impl BatchTransaction< -SourceHeaderIdOf>, - as MessageLane>::MessagesProof, - TransactionTracker>, - SubstrateError, -> for BatchDeliveryTransaction

{ +impl + BatchTransaction< + SourceHeaderIdOf>, + as MessageLane>::MessagesProof, + TransactionTracker>, + SubstrateError, + > for BatchDeliveryTransaction

+{ fn required_header_id(&self) -> SourceHeaderIdOf> { self.required_source_header_on_target.clone() } diff --git a/relays/messages/src/message_lane_loop.rs b/relays/messages/src/message_lane_loop.rs index 736a06e7e3..b648389053 100644 --- a/relays/messages/src/message_lane_loop.rs +++ b/relays/messages/src/message_lane_loop.rs @@ -118,10 +118,7 @@ pub trait BatchTransaction: Send { fn required_header_id(&self) -> HeaderId; /// Append proof and send transaction to the connected node. - async fn append_proof_and_send( - self, - proof: Proof, - ) -> Result; + async fn append_proof_and_send(self, proof: Proof) -> Result; } /// Source client trait. diff --git a/relays/messages/src/message_race_loop.rs b/relays/messages/src/message_race_loop.rs index 02a87e3f84..df3939e44a 100644 --- a/relays/messages/src/message_race_loop.rs +++ b/relays/messages/src/message_race_loop.rs @@ -562,14 +562,16 @@ pub async fn run, TC: TargetClient

>( ); let nonces = nonces_range.clone(); - target_submit_proof - .set(target_batch_transaction + target_submit_proof.set( + target_batch_transaction .append_proof_and_send(proof.clone()) - .map(|result| result.map(|tx_tracker| { - NoncesSubmitArtifacts { nonces, tx_tracker } - })) + .map(|result| { + result + .map(|tx_tracker| NoncesSubmitArtifacts { nonces, tx_tracker }) + }) .left_future() - .fuse()); + .fuse(), + ); } else { log::debug!( target: "bridge", From 418d6e488b0734ef60ab732cbdea5f77b239ce28 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Thu, 1 Dec 2022 13:30:39 +0300 Subject: [PATCH 09/29] continue with batch calls --- .../src/cli/relay_headers_and_messages/mod.rs | 8 +++--- .../parachain_to_parachain.rs | 5 ++-- .../relay_to_parachain.rs | 5 ++-- .../relay_to_relay.rs | 5 ++-- .../lib-substrate-relay/src/messages_lane.rs | 4 +-- .../src/messages_source.rs | 14 +++++----- .../src/messages_target.rs | 6 ++--- .../src/on_demand/headers.rs | 22 ++++++++++----- .../lib-substrate-relay/src/on_demand/mod.rs | 13 +++++++-- .../src/on_demand/parachains.rs | 27 ++++++++++++------- 10 files changed, 67 insertions(+), 42 deletions(-) diff --git a/relays/bin-substrate/src/cli/relay_headers_and_messages/mod.rs b/relays/bin-substrate/src/cli/relay_headers_and_messages/mod.rs index f84174563e..27fc64141f 100644 --- a/relays/bin-substrate/src/cli/relay_headers_and_messages/mod.rs +++ b/relays/bin-substrate/src/cli/relay_headers_and_messages/mod.rs @@ -166,8 +166,8 @@ where /// Returns message relay parameters. fn messages_relay_params( &self, - source_to_target_headers_relay: Arc>>, - target_to_source_headers_relay: Arc>>, + source_to_target_headers_relay: Arc>, + target_to_source_headers_relay: Arc>, lane_id: LaneId, ) -> MessagesRelayParams { MessagesRelayParams { @@ -242,8 +242,8 @@ trait Full2WayBridgeBase: Sized + Send + Sync { async fn start_on_demand_headers_relayers( &mut self, ) -> anyhow::Result<( - Arc>>, - Arc>>, + Arc>, + Arc>, )>; } diff --git a/relays/bin-substrate/src/cli/relay_headers_and_messages/parachain_to_parachain.rs b/relays/bin-substrate/src/cli/relay_headers_and_messages/parachain_to_parachain.rs index e7328a837e..577de85b85 100644 --- a/relays/bin-substrate/src/cli/relay_headers_and_messages/parachain_to_parachain.rs +++ b/relays/bin-substrate/src/cli/relay_headers_and_messages/parachain_to_parachain.rs @@ -25,7 +25,6 @@ use crate::cli::{ CliChain, }; use bp_polkadot_core::parachains::ParaHash; -use bp_runtime::BlockNumberOf; use pallet_bridge_parachains::{RelayBlockHash, RelayBlockHasher, RelayBlockNumber}; use relay_substrate_client::{AccountIdOf, AccountKeyPairOf, Chain, ChainWithTransactions, Client}; use sp_core::Pair; @@ -208,8 +207,8 @@ where async fn start_on_demand_headers_relayers( &mut self, ) -> anyhow::Result<( - Arc>>, - Arc>>, + Arc>, + Arc>, )> { self.common.left.accounts.push(TaggedAccount::Headers { id: self.right_headers_to_left_transaction_params.signer.public().into(), diff --git a/relays/bin-substrate/src/cli/relay_headers_and_messages/relay_to_parachain.rs b/relays/bin-substrate/src/cli/relay_headers_and_messages/relay_to_parachain.rs index 4070783df0..ea9e776b50 100644 --- a/relays/bin-substrate/src/cli/relay_headers_and_messages/relay_to_parachain.rs +++ b/relays/bin-substrate/src/cli/relay_headers_and_messages/relay_to_parachain.rs @@ -26,7 +26,6 @@ use crate::cli::{ CliChain, }; use bp_polkadot_core::parachains::ParaHash; -use bp_runtime::BlockNumberOf; use pallet_bridge_parachains::{RelayBlockHash, RelayBlockHasher, RelayBlockNumber}; use relay_substrate_client::{AccountIdOf, AccountKeyPairOf, Chain, ChainWithTransactions, Client}; use sp_core::Pair; @@ -194,8 +193,8 @@ where async fn start_on_demand_headers_relayers( &mut self, ) -> anyhow::Result<( - Arc>>, - Arc>>, + Arc>, + Arc>, )> { self.common.left.accounts.push(TaggedAccount::Headers { id: self.right_headers_to_left_transaction_params.signer.public().into(), diff --git a/relays/bin-substrate/src/cli/relay_headers_and_messages/relay_to_relay.rs b/relays/bin-substrate/src/cli/relay_headers_and_messages/relay_to_relay.rs index bda532a2af..85fe0d5ac3 100644 --- a/relays/bin-substrate/src/cli/relay_headers_and_messages/relay_to_relay.rs +++ b/relays/bin-substrate/src/cli/relay_headers_and_messages/relay_to_relay.rs @@ -22,7 +22,6 @@ use crate::cli::{ relay_headers_and_messages::{Full2WayBridgeBase, Full2WayBridgeCommonParams}, CliChain, }; -use bp_runtime::BlockNumberOf; use relay_substrate_client::{AccountIdOf, AccountKeyPairOf, ChainWithTransactions}; use sp_core::Pair; use substrate_relay_helper::{ @@ -149,8 +148,8 @@ where async fn start_on_demand_headers_relayers( &mut self, ) -> anyhow::Result<( - Arc>>, - Arc>>, + Arc>, + Arc>, )> { self.common.right.accounts.push(TaggedAccount::Headers { id: self.left_to_right_transaction_params.signer.public().into(), diff --git a/relays/lib-substrate-relay/src/messages_lane.rs b/relays/lib-substrate-relay/src/messages_lane.rs index 4ef47ea79a..f96944bbe8 100644 --- a/relays/lib-substrate-relay/src/messages_lane.rs +++ b/relays/lib-substrate-relay/src/messages_lane.rs @@ -104,10 +104,10 @@ pub struct MessagesRelayParams { pub target_transaction_params: TransactionParams>, /// Optional on-demand source to target headers relay. pub source_to_target_headers_relay: - Option>>>, + Option>>, /// Optional on-demand target to source headers relay. pub target_to_source_headers_relay: - Option>>>, + Option>>, /// Identifier of lane that needs to be served. pub lane_id: LaneId, /// Metrics parameters. diff --git a/relays/lib-substrate-relay/src/messages_source.rs b/relays/lib-substrate-relay/src/messages_source.rs index 8cf7016875..f679cd88ef 100644 --- a/relays/lib-substrate-relay/src/messages_source.rs +++ b/relays/lib-substrate-relay/src/messages_source.rs @@ -69,7 +69,7 @@ pub struct SubstrateMessagesSource { target_client: Client, lane_id: LaneId, transaction_params: TransactionParams>, - target_to_source_headers_relay: Option>>>, + target_to_source_headers_relay: Option>>, } impl SubstrateMessagesSource

{ @@ -80,7 +80,7 @@ impl SubstrateMessagesSource

{ lane_id: LaneId, transaction_params: TransactionParams>, target_to_source_headers_relay: Option< - Arc>>, + Arc>, >, ) -> Self { SubstrateMessagesSource { @@ -411,7 +411,7 @@ where .target_to_source_headers_relay .as_ref() .expect("BatchConfirmationTransaction is only created when target_to_source_headers_relay is Some; qed") - .prove_header(self.required_target_header_on_source) + .prove_header(self.required_target_header_on_source.0) .await?; calls.push( P::ReceiveMessagesDeliveryProofCallBuilder::build_receive_messages_delivery_proof_call( @@ -419,8 +419,8 @@ where ), ); - let genesis_hash = *self.messages_source.source_client.genesis_hash(); - let transaction_params = self.messages_source.transaction_params.clone(); + let batch_call = P::SourceBatchCallBuilder::build_batch_call(calls); + let (spec_version, transaction_version) = self.messages_source.source_client.simple_runtime_version().await?; self.messages_source @@ -430,11 +430,11 @@ where SignParam:: { spec_version, transaction_version, - genesis_hash, + genesis_hash: *self.messages_source.source_client.genesis_hash(), signer: self.messages_source.transaction_params.signer.clone(), }, move |best_block_id, transaction_nonce| { - Ok(UnsignedTransaction::new(calls.into(), transaction_nonce).era( + Ok(UnsignedTransaction::new(batch_call.into(), transaction_nonce).era( TransactionEra::new( best_block_id, self.messages_source.transaction_params.mortality, diff --git a/relays/lib-substrate-relay/src/messages_target.rs b/relays/lib-substrate-relay/src/messages_target.rs index e6f4c061cb..8884cd61c4 100644 --- a/relays/lib-substrate-relay/src/messages_target.rs +++ b/relays/lib-substrate-relay/src/messages_target.rs @@ -39,7 +39,7 @@ use messages_relay::{ message_lane_loop::{BatchTransaction, NoncesSubmitArtifacts, TargetClient, TargetClientState}, }; use relay_substrate_client::{ - AccountIdOf, AccountKeyPairOf, BalanceOf, BlockNumberOf, Chain, ChainWithMessages, Client, + AccountIdOf, AccountKeyPairOf, BalanceOf, Chain, ChainWithMessages, Client, Error as SubstrateError, HashOf, HeaderIdOf, IndexOf, SignParam, TransactionEra, TransactionTracker, UnsignedTransaction, }; @@ -58,7 +58,7 @@ pub struct SubstrateMessagesTarget { lane_id: LaneId, relayer_id_at_source: AccountIdOf, transaction_params: TransactionParams>, - source_to_target_headers_relay: Option>>>, + source_to_target_headers_relay: Option>>, } impl SubstrateMessagesTarget

{ @@ -70,7 +70,7 @@ impl SubstrateMessagesTarget

{ relayer_id_at_source: AccountIdOf, transaction_params: TransactionParams>, source_to_target_headers_relay: Option< - Arc>>, + Arc>, >, ) -> Self { SubstrateMessagesTarget { diff --git a/relays/lib-substrate-relay/src/on_demand/headers.rs b/relays/lib-substrate-relay/src/on_demand/headers.rs index c0603cda8c..7171840ddb 100644 --- a/relays/lib-substrate-relay/src/on_demand/headers.rs +++ b/relays/lib-substrate-relay/src/on_demand/headers.rs @@ -22,9 +22,12 @@ use bp_header_chain::ConsensusLogReader; use futures::{select, FutureExt}; use num_traits::{One, Zero}; use sp_runtime::traits::Header; +use std::marker::PhantomData; use finality_relay::{FinalitySyncParams, TargetClient as FinalityTargetClient}; -use relay_substrate_client::{AccountIdOf, AccountKeyPairOf, BlockNumberOf, Chain, Client}; +use relay_substrate_client::{ + AccountIdOf, AccountKeyPairOf, BlockNumberOf, Chain, ChainWithTransactions, Client, +}; use relay_utils::{ metrics::MetricsParams, relay_loop::Client as RelayClient, FailedClient, MaybeConnectionError, STALL_TIMEOUT, @@ -47,16 +50,22 @@ use crate::{ /// relay) needs it to continue its regular work. When enough headers are relayed, on-demand stops /// syncing headers. #[derive(Clone)] -pub struct OnDemandHeadersRelay { +pub struct OnDemandHeadersRelay { /// Relay task name. relay_task_name: String, /// Shared reference to maximal required finalized header number. required_header_number: RequiredHeaderNumberRef, + /// Just rusty things. + _marker: PhantomData, } -impl OnDemandHeadersRelay { +impl + OnDemandHeadersRelay +{ /// Create new on-demand headers relay. - pub fn new>( + pub fn new< + P: SubstrateFinalitySyncPipeline, + >( source_client: Client, target_client: Client, target_transaction_params: TransactionParams>, @@ -70,6 +79,7 @@ impl OnDemandHeadersRelay { let this = OnDemandHeadersRelay { relay_task_name: on_demand_headers_relay_name::(), required_header_number: required_header_number.clone(), + _marker: PhantomData, }; async_std::task::spawn(async move { background_task::

( @@ -87,8 +97,8 @@ impl OnDemandHeadersRelay { } #[async_trait] -impl OnDemandRelay> - for OnDemandHeadersRelay +impl OnDemandRelay + for OnDemandHeadersRelay { async fn require_more_headers(&self, required_header: BlockNumberOf) { let mut required_header_number = self.required_header_number.lock().await; diff --git a/relays/lib-substrate-relay/src/on_demand/mod.rs b/relays/lib-substrate-relay/src/on_demand/mod.rs index 7a2dfc9c15..5582ea080f 100644 --- a/relays/lib-substrate-relay/src/on_demand/mod.rs +++ b/relays/lib-substrate-relay/src/on_demand/mod.rs @@ -18,18 +18,27 @@ //! on-demand pipelines. use async_trait::async_trait; +use relay_substrate_client::{BlockNumberOf, CallOf, Chain, Error as SubstrateError}; pub mod headers; pub mod parachains; /// On-demand headers relay that is relaying finalizing headers only when requested. #[async_trait] -pub trait OnDemandRelay: Send + Sync { +pub trait OnDemandRelay: Send + Sync { /// Ask relay to relay source header with given number to the target chain. /// /// Depending on implementation, on-demand relay may also relay `required_header` ancestors /// (e.g. if they're mandatory), or its descendants. The request is considered complete if /// the best avbailable header at the target chain has number that is larger than or equal /// to the `required_header`. - async fn require_more_headers(&self, required_header: SourceHeaderNumber); + async fn require_more_headers(&self, required_header: BlockNumberOf); + + /// Ask relay to prove source `required_header` to the `TargetChain`. + async fn prove_header( + &self, + required_header: BlockNumberOf, + ) -> Result>, SubstrateError> { + unimplemented!("TODO") + } } diff --git a/relays/lib-substrate-relay/src/on_demand/parachains.rs b/relays/lib-substrate-relay/src/on_demand/parachains.rs index 35ef8244ae..f7b010d81e 100644 --- a/relays/lib-substrate-relay/src/on_demand/parachains.rs +++ b/relays/lib-substrate-relay/src/on_demand/parachains.rs @@ -38,12 +38,13 @@ use num_traits::Zero; use pallet_bridge_parachains::{RelayBlockHash, RelayBlockHasher, RelayBlockNumber}; use parachains_relay::parachains_loop::{AvailableHeader, ParachainSyncParams, TargetClient}; use relay_substrate_client::{ - AccountIdOf, AccountKeyPairOf, BlockNumberOf, Chain, Client, Error as SubstrateError, HashOf, + AccountIdOf, AccountKeyPairOf, BlockNumberOf, Chain, ChainWithTransactions, Client, + Error as SubstrateError, HashOf, }; use relay_utils::{ metrics::MetricsParams, relay_loop::Client as RelayClient, FailedClient, HeaderId, }; -use std::fmt::Debug; +use std::{fmt::Debug, marker::PhantomData}; /// On-demand Substrate <-> Substrate parachain finality relay. /// @@ -51,25 +52,31 @@ use std::fmt::Debug; /// (e.g. messages relay) needs it to continue its regular work. When enough parachain headers /// are relayed, on-demand stops syncing headers. #[derive(Clone)] -pub struct OnDemandParachainsRelay { +pub struct OnDemandParachainsRelay { /// Relay task name. relay_task_name: String, /// Channel used to communicate with background task and ask for relay of parachain heads. required_header_number_sender: Sender>, + /// Just rusty things. + _marker: PhantomData, } -impl OnDemandParachainsRelay { +impl + OnDemandParachainsRelay +{ /// Create new on-demand parachains relay. /// /// Note that the argument is the source relay chain client, not the parachain client. /// That's because parachain finality is determined by the relay chain and we don't /// need to connect to the parachain itself here. - pub fn new>( + pub fn new< + P: SubstrateParachainsPipeline, + >( source_relay_client: Client, target_client: Client, target_transaction_params: TransactionParams>, on_demand_source_relay_to_target_headers: Arc< - dyn OnDemandRelay>, + dyn OnDemandRelay, >, ) -> Self where @@ -83,6 +90,7 @@ impl OnDemandParachainsRelay { let this = OnDemandParachainsRelay { relay_task_name: on_demand_parachains_relay_name::(), required_header_number_sender, + _marker: PhantomData, }; async_std::task::spawn(async move { background_task::

( @@ -100,10 +108,11 @@ impl OnDemandParachainsRelay { } #[async_trait] -impl OnDemandRelay> - for OnDemandParachainsRelay +impl OnDemandRelay + for OnDemandParachainsRelay where SourceParachain: Chain, + TargetChain: Chain, { async fn require_more_headers(&self, required_header: BlockNumberOf) { if let Err(e) = self.required_header_number_sender.send(required_header).await { @@ -125,7 +134,7 @@ async fn background_task( target_client: Client, target_transaction_params: TransactionParams>, on_demand_source_relay_to_target_headers: Arc< - dyn OnDemandRelay>, + dyn OnDemandRelay, >, required_parachain_header_number_receiver: Receiver>, ) where From 0dcdec8b2364db8cdeea98ab2f6fe82590ee8bf3 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Thu, 1 Dec 2022 15:16:08 +0300 Subject: [PATCH 10/29] impl BatchCallBuilder for () --- ...ub_rococo_messages_to_bridge_hub_wococo.rs | 3 ++ ...ub_wococo_messages_to_bridge_hub_rococo.rs | 3 ++ .../src/chains/millau_messages_to_rialto.rs | 3 ++ .../millau_messages_to_rialto_parachain.rs | 3 ++ .../src/chains/rialto_messages_to_millau.rs | 3 ++ .../rialto_parachain_messages_to_millau.rs | 3 ++ .../src/cli/relay_headers_and_messages/mod.rs | 2 +- .../lib-substrate-relay/src/messages_lane.rs | 30 +++++++++++++------ 8 files changed, 40 insertions(+), 10 deletions(-) diff --git a/relays/bin-substrate/src/chains/bridge_hub_rococo_messages_to_bridge_hub_wococo.rs b/relays/bin-substrate/src/chains/bridge_hub_rococo_messages_to_bridge_hub_wococo.rs index 6f9f05e663..339be92063 100644 --- a/relays/bin-substrate/src/chains/bridge_hub_rococo_messages_to_bridge_hub_wococo.rs +++ b/relays/bin-substrate/src/chains/bridge_hub_rococo_messages_to_bridge_hub_wococo.rs @@ -59,4 +59,7 @@ impl SubstrateMessageLane for BridgeHubRococoMessagesToBridgeHubWococoMessageLan BridgeHubRococoMessagesToBridgeHubWococoMessageLaneReceiveMessagesProofCallBuilder; type ReceiveMessagesDeliveryProofCallBuilder = BridgeHubRococoMessagesToBridgeHubWococoMessageLaneReceiveMessagesDeliveryProofCallBuilder; + + type SourceBatchCallBuilder = (); + type TargetBatchCallBuilder = (); } diff --git a/relays/bin-substrate/src/chains/bridge_hub_wococo_messages_to_bridge_hub_rococo.rs b/relays/bin-substrate/src/chains/bridge_hub_wococo_messages_to_bridge_hub_rococo.rs index 2b38eccbcb..3bb2aabde0 100644 --- a/relays/bin-substrate/src/chains/bridge_hub_wococo_messages_to_bridge_hub_rococo.rs +++ b/relays/bin-substrate/src/chains/bridge_hub_wococo_messages_to_bridge_hub_rococo.rs @@ -59,4 +59,7 @@ impl SubstrateMessageLane for BridgeHubWococoMessagesToBridgeHubRococoMessageLan BridgeHubWococoMessagesToBridgeHubRococoMessageLaneReceiveMessagesProofCallBuilder; type ReceiveMessagesDeliveryProofCallBuilder = BridgeHubWococoMessagesToBridgeHubRococoMessageLaneReceiveMessagesDeliveryProofCallBuilder; + + type SourceBatchCallBuilder = (); + type TargetBatchCallBuilder = (); } diff --git a/relays/bin-substrate/src/chains/millau_messages_to_rialto.rs b/relays/bin-substrate/src/chains/millau_messages_to_rialto.rs index b9920db53d..e6a2ef1a85 100644 --- a/relays/bin-substrate/src/chains/millau_messages_to_rialto.rs +++ b/relays/bin-substrate/src/chains/millau_messages_to_rialto.rs @@ -41,4 +41,7 @@ impl SubstrateMessageLane for MillauMessagesToRialto { millau_runtime::Runtime, millau_runtime::WithRialtoMessagesInstance, >; + + type SourceBatchCallBuilder = (); + type TargetBatchCallBuilder = (); } diff --git a/relays/bin-substrate/src/chains/millau_messages_to_rialto_parachain.rs b/relays/bin-substrate/src/chains/millau_messages_to_rialto_parachain.rs index 70cb887fa3..0b1d3afb79 100644 --- a/relays/bin-substrate/src/chains/millau_messages_to_rialto_parachain.rs +++ b/relays/bin-substrate/src/chains/millau_messages_to_rialto_parachain.rs @@ -41,4 +41,7 @@ impl SubstrateMessageLane for MillauMessagesToRialtoParachain { millau_runtime::Runtime, millau_runtime::WithRialtoParachainMessagesInstance, >; + + type SourceBatchCallBuilder = (); + type TargetBatchCallBuilder = (); } diff --git a/relays/bin-substrate/src/chains/rialto_messages_to_millau.rs b/relays/bin-substrate/src/chains/rialto_messages_to_millau.rs index 80b6b9fdbc..b45239fb9a 100644 --- a/relays/bin-substrate/src/chains/rialto_messages_to_millau.rs +++ b/relays/bin-substrate/src/chains/rialto_messages_to_millau.rs @@ -41,4 +41,7 @@ impl SubstrateMessageLane for RialtoMessagesToMillau { rialto_runtime::Runtime, rialto_runtime::WithMillauMessagesInstance, >; + + type SourceBatchCallBuilder = (); + type TargetBatchCallBuilder = (); } diff --git a/relays/bin-substrate/src/chains/rialto_parachain_messages_to_millau.rs b/relays/bin-substrate/src/chains/rialto_parachain_messages_to_millau.rs index 5cca26105b..18de144750 100644 --- a/relays/bin-substrate/src/chains/rialto_parachain_messages_to_millau.rs +++ b/relays/bin-substrate/src/chains/rialto_parachain_messages_to_millau.rs @@ -41,4 +41,7 @@ impl SubstrateMessageLane for RialtoParachainMessagesToMillau { rialto_parachain_runtime::Runtime, rialto_parachain_runtime::WithMillauMessagesInstance, >; + + type SourceBatchCallBuilder = (); + type TargetBatchCallBuilder = (); } diff --git a/relays/bin-substrate/src/cli/relay_headers_and_messages/mod.rs b/relays/bin-substrate/src/cli/relay_headers_and_messages/mod.rs index 27fc64141f..3a53510b22 100644 --- a/relays/bin-substrate/src/cli/relay_headers_and_messages/mod.rs +++ b/relays/bin-substrate/src/cli/relay_headers_and_messages/mod.rs @@ -59,7 +59,7 @@ use crate::{ declare_chain_cli_schema, }; use bp_messages::LaneId; -use bp_runtime::{BalanceOf, BlockNumberOf}; +use bp_runtime::BalanceOf; use relay_substrate_client::{ AccountIdOf, AccountKeyPairOf, Chain, ChainWithBalances, ChainWithTransactions, Client, }; diff --git a/relays/lib-substrate-relay/src/messages_lane.rs b/relays/lib-substrate-relay/src/messages_lane.rs index f96944bbe8..dd4f13b014 100644 --- a/relays/lib-substrate-relay/src/messages_lane.rs +++ b/relays/lib-substrate-relay/src/messages_lane.rs @@ -62,15 +62,6 @@ pub trait SubstrateMessageLane: 'static + Clone + Debug + Send + Sync { type TargetBatchCallBuilder: BatchCallBuilder; } -/// Batch call builder. -pub trait BatchCallBuilder { - /// If `true`, then batch calls are supported at the chain. - const BATCH_CALL_SUPPORTED: bool; - - /// Create batch call from given calls vector. - fn build_batch_call(calls: Vec>) -> CallOf; -} - /// Adapter that allows all `SubstrateMessageLane` to act as `MessageLane`. #[derive(Clone, Debug)] pub struct MessageLaneAdapter { @@ -210,6 +201,27 @@ where .map_err(Into::into) } +/// Batch call builder. +pub trait BatchCallBuilder { + /// If `true`, then batch calls are supported at the chain. + const BATCH_CALL_SUPPORTED: bool; + + /// Create batch call from given calls vector. + fn build_batch_call(_calls: Vec>) -> CallOf; +} + +impl BatchCallBuilder for () { + const BATCH_CALL_SUPPORTED: bool = false; + + fn build_batch_call(calls: Vec>) -> CallOf { + unreachable!( + "only called if `BATCH_CALL_SUPPORTED` is true;\ + `BATCH_CALL_SUPPORTED` is false;\ + qed" + ) + } +} + /// Different ways of building `receive_messages_proof` calls. pub trait ReceiveMessagesProofCallBuilder { /// Given messages proof, build call of `receive_messages_proof` function of bridge From d32fb4d0396b9b6cff50800e825ed2329e7c9b42 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Thu, 1 Dec 2022 15:29:03 +0300 Subject: [PATCH 11/29] BatchDeliveryTransaction impl --- .../lib-substrate-relay/src/messages_lane.rs | 2 +- .../src/messages_source.rs | 1 - .../src/messages_target.rs | 70 ++++++++++++++++--- 3 files changed, 60 insertions(+), 13 deletions(-) diff --git a/relays/lib-substrate-relay/src/messages_lane.rs b/relays/lib-substrate-relay/src/messages_lane.rs index dd4f13b014..c684b77b38 100644 --- a/relays/lib-substrate-relay/src/messages_lane.rs +++ b/relays/lib-substrate-relay/src/messages_lane.rs @@ -213,7 +213,7 @@ pub trait BatchCallBuilder { impl BatchCallBuilder for () { const BATCH_CALL_SUPPORTED: bool = false; - fn build_batch_call(calls: Vec>) -> CallOf { + fn build_batch_call(_calls: Vec>) -> CallOf { unreachable!( "only called if `BATCH_CALL_SUPPORTED` is true;\ `BATCH_CALL_SUPPORTED` is false;\ diff --git a/relays/lib-substrate-relay/src/messages_source.rs b/relays/lib-substrate-relay/src/messages_source.rs index f679cd88ef..e1a777718a 100644 --- a/relays/lib-substrate-relay/src/messages_source.rs +++ b/relays/lib-substrate-relay/src/messages_source.rs @@ -418,7 +418,6 @@ where proof, false, ), ); - let batch_call = P::SourceBatchCallBuilder::build_batch_call(calls); let (spec_version, transaction_version) = diff --git a/relays/lib-substrate-relay/src/messages_target.rs b/relays/lib-substrate-relay/src/messages_target.rs index 8884cd61c4..87bd1005e7 100644 --- a/relays/lib-substrate-relay/src/messages_target.rs +++ b/relays/lib-substrate-relay/src/messages_target.rs @@ -39,7 +39,7 @@ use messages_relay::{ message_lane_loop::{BatchTransaction, NoncesSubmitArtifacts, TargetClient, TargetClientState}, }; use relay_substrate_client::{ - AccountIdOf, AccountKeyPairOf, BalanceOf, Chain, ChainWithMessages, Client, + AccountIdOf, AccountKeyPairOf, BalanceOf, CallOf, Chain, ChainWithMessages, Client, Error as SubstrateError, HashOf, HeaderIdOf, IndexOf, SignParam, TransactionEra, TransactionTracker, UnsignedTransaction, }; @@ -303,6 +303,8 @@ impl TransactionTracker>, SubstrateError, > for BatchDeliveryTransaction

+where + AccountIdOf: From< as Pair>::Public>, { fn required_header_id(&self) -> SourceHeaderIdOf> { self.required_source_header_on_target.clone() @@ -310,31 +312,77 @@ impl async fn append_proof_and_send( self, - _proof: as MessageLane>::MessagesProof, + proof: as MessageLane>::MessagesProof, ) -> Result>, SubstrateError> { - unimplemented!("TODO") + let mut calls = self + .messages_target + .source_to_target_headers_relay + .as_ref() + .expect("BatchDeliveryTransaction is only created when source_to_target_headers_relay is Some; qed") + .prove_header(self.required_source_header_on_target.0) + .await?; + calls.push(make_messages_delivery_call::

( + self.messages_target.relayer_id_at_source, + proof.1.nonces_start..=proof.1.nonces_end, + proof, + false, + )); + let batch_call = P::TargetBatchCallBuilder::build_batch_call(calls); + + let (spec_version, transaction_version) = + self.messages_target.target_client.simple_runtime_version().await?; + self.messages_target + .target_client + .submit_and_watch_signed_extrinsic( + self.messages_target.transaction_params.signer.public().into(), + SignParam:: { + spec_version, + transaction_version, + genesis_hash: *self.messages_target.target_client.genesis_hash(), + signer: self.messages_target.transaction_params.signer.clone(), + }, + move |best_block_id, transaction_nonce| { + Ok(UnsignedTransaction::new(batch_call.into(), transaction_nonce).era( + TransactionEra::new( + best_block_id, + self.messages_target.transaction_params.mortality, + ), + )) + }, + ) + .await } } -/// Make messages delivery transaction from given proof. -fn make_messages_delivery_transaction( - target_transaction_params: &TransactionParams>, - target_best_block_id: HeaderIdOf, - transaction_nonce: IndexOf, +/// Make messages delivery call from given proof. +fn make_messages_delivery_call( relayer_id_at_source: AccountIdOf, nonces: RangeInclusive, proof: SubstrateMessagesProof, trace_call: bool, -) -> Result, SubstrateError> { +) -> CallOf { let messages_count = nonces.end() - nonces.start() + 1; let dispatch_weight = proof.0; - let call = P::ReceiveMessagesProofCallBuilder::build_receive_messages_proof_call( + P::ReceiveMessagesProofCallBuilder::build_receive_messages_proof_call( relayer_id_at_source, proof, messages_count as _, dispatch_weight, trace_call, - ); + ) +} + +/// Make messages delivery transaction from given proof. +fn make_messages_delivery_transaction( + target_transaction_params: &TransactionParams>, + target_best_block_id: HeaderIdOf, + transaction_nonce: IndexOf, + relayer_id_at_source: AccountIdOf, + nonces: RangeInclusive, + proof: SubstrateMessagesProof, + trace_call: bool, +) -> Result, SubstrateError> { + let call = make_messages_delivery_call::

(relayer_id_at_source, nonces, proof, trace_call); Ok(UnsignedTransaction::new(call.into(), transaction_nonce) .era(TransactionEra::new(target_best_block_id, target_transaction_params.mortality))) } From b2659494bc53fe8c7ba369eef8468d6caaebd08b Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Mon, 12 Dec 2022 09:26:30 +0300 Subject: [PATCH 12/29] BundledBatchCallBuilder --- Cargo.lock | 1 + relays/lib-substrate-relay/Cargo.toml | 1 + relays/lib-substrate-relay/src/lib.rs | 39 +++++++++++++++++++ .../lib-substrate-relay/src/messages_lane.rs | 27 ++----------- .../src/messages_source.rs | 5 +-- .../src/messages_target.rs | 6 +-- relays/messages/src/message_lane_loop.rs | 8 ++-- 7 files changed, 52 insertions(+), 35 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e4707af571..9f4d8bb6cf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11668,6 +11668,7 @@ dependencies = [ "pallet-bridge-messages", "pallet-bridge-parachains", "pallet-transaction-payment", + "pallet-utility", "parachains-relay", "parity-scale-codec", "relay-rialto-client", diff --git a/relays/lib-substrate-relay/Cargo.toml b/relays/lib-substrate-relay/Cargo.toml index bdf49d42ea..1dd7cb7b37 100644 --- a/relays/lib-substrate-relay/Cargo.toml +++ b/relays/lib-substrate-relay/Cargo.toml @@ -41,6 +41,7 @@ bp-messages = { path = "../../primitives/messages" } frame-support = { git = "https://github.com/paritytech/substrate", branch = "master" } frame-system = { git = "https://github.com/paritytech/substrate", branch = "master" } pallet-balances = { git = "https://github.com/paritytech/substrate", branch = "master" } +pallet-utility = { git = "https://github.com/paritytech/substrate", branch = "master" } sp-core = { git = "https://github.com/paritytech/substrate", branch = "master" } sp-finality-grandpa = { git = "https://github.com/paritytech/substrate", branch = "master" } sp-runtime = { git = "https://github.com/paritytech/substrate", branch = "master" } diff --git a/relays/lib-substrate-relay/src/lib.rs b/relays/lib-substrate-relay/src/lib.rs index 62ae756e00..e0d6a915c7 100644 --- a/relays/lib-substrate-relay/src/lib.rs +++ b/relays/lib-substrate-relay/src/lib.rs @@ -18,6 +18,8 @@ #![warn(missing_docs)] +use std::marker::PhantomData; + pub mod error; pub mod finality; pub mod messages_lane; @@ -96,3 +98,40 @@ impl TaggedAccount { } } } + +/// Batch call builder. +pub trait BatchCallBuilder { + /// If `true`, then batch calls are supported at the chain. + const BATCH_CALL_SUPPORTED: bool; + + /// Create batch call from given calls vector. + fn build_batch_call(_calls: Vec) -> Call; +} + +impl BatchCallBuilder for () { + const BATCH_CALL_SUPPORTED: bool = false; + + fn build_batch_call(_calls: Vec) -> Call { + unreachable!( + "only called if `BATCH_CALL_SUPPORTED` is true;\ + `BATCH_CALL_SUPPORTED` is false;\ + qed" + ) + } +} + +/// Batch call builder for bundled runtimes. +pub struct BundledBatchCallBuilder(PhantomData); + +impl BatchCallBuilder<::RuntimeCall> for BundledBatchCallBuilder +where + R: pallet_utility::Config::RuntimeCall>, +{ + const BATCH_CALL_SUPPORTED: bool = true; + + fn build_batch_call( + calls: Vec<::RuntimeCall>, + ) -> ::RuntimeCall { + unimplemented!("TODO") + } +} diff --git a/relays/lib-substrate-relay/src/messages_lane.rs b/relays/lib-substrate-relay/src/messages_lane.rs index c684b77b38..e3da85b3ab 100644 --- a/relays/lib-substrate-relay/src/messages_lane.rs +++ b/relays/lib-substrate-relay/src/messages_lane.rs @@ -20,7 +20,7 @@ use crate::{ messages_source::{SubstrateMessagesProof, SubstrateMessagesSource}, messages_target::{SubstrateMessagesDeliveryProof, SubstrateMessagesTarget}, on_demand::OnDemandRelay, - TransactionParams, + BatchCallBuilder, TransactionParams, }; use async_std::sync::Arc; @@ -57,9 +57,9 @@ pub trait SubstrateMessageLane: 'static + Clone + Debug + Send + Sync { type ReceiveMessagesDeliveryProofCallBuilder: ReceiveMessagesDeliveryProofCallBuilder; /// How batch calls are built at the source chain? - type SourceBatchCallBuilder: BatchCallBuilder; + type SourceBatchCallBuilder: BatchCallBuilder>; /// How batch calls are built at the target chain? - type TargetBatchCallBuilder: BatchCallBuilder; + type TargetBatchCallBuilder: BatchCallBuilder>; } /// Adapter that allows all `SubstrateMessageLane` to act as `MessageLane`. @@ -201,27 +201,6 @@ where .map_err(Into::into) } -/// Batch call builder. -pub trait BatchCallBuilder { - /// If `true`, then batch calls are supported at the chain. - const BATCH_CALL_SUPPORTED: bool; - - /// Create batch call from given calls vector. - fn build_batch_call(_calls: Vec>) -> CallOf; -} - -impl BatchCallBuilder for () { - const BATCH_CALL_SUPPORTED: bool = false; - - fn build_batch_call(_calls: Vec>) -> CallOf { - unreachable!( - "only called if `BATCH_CALL_SUPPORTED` is true;\ - `BATCH_CALL_SUPPORTED` is false;\ - qed" - ) - } -} - /// Different ways of building `receive_messages_proof` calls. pub trait ReceiveMessagesProofCallBuilder { /// Given messages proof, build call of `receive_messages_proof` function of bridge diff --git a/relays/lib-substrate-relay/src/messages_source.rs b/relays/lib-substrate-relay/src/messages_source.rs index e1a777718a..c4fd1c3bfe 100644 --- a/relays/lib-substrate-relay/src/messages_source.rs +++ b/relays/lib-substrate-relay/src/messages_source.rs @@ -20,12 +20,11 @@ use crate::{ messages_lane::{ - BatchCallBuilder, MessageLaneAdapter, ReceiveMessagesDeliveryProofCallBuilder, - SubstrateMessageLane, + MessageLaneAdapter, ReceiveMessagesDeliveryProofCallBuilder, SubstrateMessageLane, }, messages_target::SubstrateMessagesDeliveryProof, on_demand::OnDemandRelay, - TransactionParams, + BatchCallBuilder, TransactionParams, }; use async_std::sync::Arc; diff --git a/relays/lib-substrate-relay/src/messages_target.rs b/relays/lib-substrate-relay/src/messages_target.rs index 87bd1005e7..b8e729ba4e 100644 --- a/relays/lib-substrate-relay/src/messages_target.rs +++ b/relays/lib-substrate-relay/src/messages_target.rs @@ -19,12 +19,10 @@ //! chain. use crate::{ - messages_lane::{ - BatchCallBuilder, MessageLaneAdapter, ReceiveMessagesProofCallBuilder, SubstrateMessageLane, - }, + messages_lane::{MessageLaneAdapter, ReceiveMessagesProofCallBuilder, SubstrateMessageLane}, messages_source::{ensure_messages_pallet_active, read_client_state, SubstrateMessagesProof}, on_demand::OnDemandRelay, - TransactionParams, + BatchCallBuilder, TransactionParams, }; use async_std::sync::Arc; diff --git a/relays/messages/src/message_lane_loop.rs b/relays/messages/src/message_lane_loop.rs index b648389053..3cc21b2972 100644 --- a/relays/messages/src/message_lane_loop.rs +++ b/relays/messages/src/message_lane_loop.rs @@ -547,10 +547,10 @@ pub(crate) mod tests { async fn append_proof_and_send( self, proof: TestMessagesProof, - ) -> Result, TestError> { + ) -> Result { let mut data = self.data.lock(); data.receive_messages(proof); - self.tx_tracker + Ok(self.tx_tracker) } } @@ -577,10 +577,10 @@ pub(crate) mod tests { async fn append_proof_and_send( self, proof: TestMessagesReceivingProof, - ) -> Result, TestError> { + ) -> Result { let mut data = self.data.lock(); data.receive_messages_delivery_proof(proof); - self.tx_tracker + Ok(self.tx_tracker) } } From ac59ecc882406698734a81f005008bc256711c45 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Mon, 12 Dec 2022 09:43:50 +0300 Subject: [PATCH 13/29] proper impl of BundledBatchCallBuilder + use it in RialtoParachain -> Millau --- .../src/chains/rialto_parachain_messages_to_millau.rs | 11 +++++++---- relays/lib-substrate-relay/src/lib.rs | 3 ++- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/relays/bin-substrate/src/chains/rialto_parachain_messages_to_millau.rs b/relays/bin-substrate/src/chains/rialto_parachain_messages_to_millau.rs index 18de144750..8400157b9d 100644 --- a/relays/bin-substrate/src/chains/rialto_parachain_messages_to_millau.rs +++ b/relays/bin-substrate/src/chains/rialto_parachain_messages_to_millau.rs @@ -18,9 +18,12 @@ use relay_millau_client::Millau; use relay_rialto_parachain_client::RialtoParachain; -use substrate_relay_helper::messages_lane::{ - DirectReceiveMessagesDeliveryProofCallBuilder, DirectReceiveMessagesProofCallBuilder, - SubstrateMessageLane, +use substrate_relay_helper::{ + messages_lane::{ + DirectReceiveMessagesDeliveryProofCallBuilder, DirectReceiveMessagesProofCallBuilder, + SubstrateMessageLane, + }, + BundledBatchCallBuilder, }; /// Description of RialtoParachain -> Millau messages bridge. @@ -43,5 +46,5 @@ impl SubstrateMessageLane for RialtoParachainMessagesToMillau { >; type SourceBatchCallBuilder = (); - type TargetBatchCallBuilder = (); + type TargetBatchCallBuilder = BundledBatchCallBuilder; } diff --git a/relays/lib-substrate-relay/src/lib.rs b/relays/lib-substrate-relay/src/lib.rs index e0d6a915c7..bbff815720 100644 --- a/relays/lib-substrate-relay/src/lib.rs +++ b/relays/lib-substrate-relay/src/lib.rs @@ -126,12 +126,13 @@ pub struct BundledBatchCallBuilder(PhantomData); impl BatchCallBuilder<::RuntimeCall> for BundledBatchCallBuilder where R: pallet_utility::Config::RuntimeCall>, + ::RuntimeCall: From>, { const BATCH_CALL_SUPPORTED: bool = true; fn build_batch_call( calls: Vec<::RuntimeCall>, ) -> ::RuntimeCall { - unimplemented!("TODO") + pallet_utility::Call::batch_all { calls }.into() } } From cfa95c17d661d516fed4e9ad1a5c2fcd88c73dd7 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Mon, 12 Dec 2022 10:56:58 +0300 Subject: [PATCH 14/29] impl prove_header in OnDemandHeadersRelay --- .../parachain_to_parachain.rs | 4 +- .../relay_to_parachain.rs | 4 +- .../relay_to_relay.rs | 4 +- .../src/finality/source.rs | 22 +++++++++ .../src/on_demand/headers.rs | 46 ++++++++++++------- .../lib-substrate-relay/src/on_demand/mod.rs | 4 +- .../src/on_demand/parachains.rs | 10 +++- relays/messages/src/message_race_loop.rs | 4 ++ 8 files changed, 71 insertions(+), 27 deletions(-) diff --git a/relays/bin-substrate/src/cli/relay_headers_and_messages/parachain_to_parachain.rs b/relays/bin-substrate/src/cli/relay_headers_and_messages/parachain_to_parachain.rs index 577de85b85..9c91ec7373 100644 --- a/relays/bin-substrate/src/cli/relay_headers_and_messages/parachain_to_parachain.rs +++ b/relays/bin-substrate/src/cli/relay_headers_and_messages/parachain_to_parachain.rs @@ -241,14 +241,14 @@ where .await?; let left_relay_to_right_on_demand_headers = - OnDemandHeadersRelay::new::<::RelayFinality>( + OnDemandHeadersRelay::<::RelayFinality>::new( self.left_relay.clone(), self.common.right.client.clone(), self.left_headers_to_right_transaction_params.clone(), self.common.shared.only_mandatory_headers, ); let right_relay_to_left_on_demand_headers = - OnDemandHeadersRelay::new::<::RelayFinality>( + OnDemandHeadersRelay::<::RelayFinality>::new( self.right_relay.clone(), self.common.left.client.clone(), self.right_headers_to_left_transaction_params.clone(), diff --git a/relays/bin-substrate/src/cli/relay_headers_and_messages/relay_to_parachain.rs b/relays/bin-substrate/src/cli/relay_headers_and_messages/relay_to_parachain.rs index ea9e776b50..7391075159 100644 --- a/relays/bin-substrate/src/cli/relay_headers_and_messages/relay_to_parachain.rs +++ b/relays/bin-substrate/src/cli/relay_headers_and_messages/relay_to_parachain.rs @@ -223,14 +223,14 @@ where .await?; let left_to_right_on_demand_headers = - OnDemandHeadersRelay::new::<::Finality>( + OnDemandHeadersRelay::<::Finality>::new( self.common.left.client.clone(), self.common.right.client.clone(), self.left_headers_to_right_transaction_params.clone(), self.common.shared.only_mandatory_headers, ); let right_relay_to_left_on_demand_headers = - OnDemandHeadersRelay::new::<::RelayFinality>( + OnDemandHeadersRelay::<::RelayFinality>::new( self.right_relay.clone(), self.common.left.client.clone(), self.right_headers_to_left_transaction_params.clone(), diff --git a/relays/bin-substrate/src/cli/relay_headers_and_messages/relay_to_relay.rs b/relays/bin-substrate/src/cli/relay_headers_and_messages/relay_to_relay.rs index 85fe0d5ac3..625f1e6632 100644 --- a/relays/bin-substrate/src/cli/relay_headers_and_messages/relay_to_relay.rs +++ b/relays/bin-substrate/src/cli/relay_headers_and_messages/relay_to_relay.rs @@ -174,14 +174,14 @@ where .await?; let left_to_right_on_demand_headers = - OnDemandHeadersRelay::new::<::Finality>( + OnDemandHeadersRelay::<::Finality>::new( self.common.left.client.clone(), self.common.right.client.clone(), self.left_to_right_transaction_params.clone(), self.common.shared.only_mandatory_headers, ); let right_to_left_on_demand_headers = - OnDemandHeadersRelay::new::<::Finality>( + OnDemandHeadersRelay::<::Finality>::new( self.common.right.client.clone(), self.common.left.client.clone(), self.right_to_left_transaction_params.clone(), diff --git a/relays/lib-substrate-relay/src/finality/source.rs b/relays/lib-substrate-relay/src/finality/source.rs index e75862a822..fa81822419 100644 --- a/relays/lib-substrate-relay/src/finality/source.rs +++ b/relays/lib-substrate-relay/src/finality/source.rs @@ -23,6 +23,7 @@ use async_trait::async_trait; use codec::Decode; use finality_relay::SourceClient; use futures::stream::{unfold, Stream, StreamExt}; +use num_traits::One; use relay_substrate_client::{ BlockNumberOf, BlockWithJustification, Chain, Client, Error, HeaderOf, }; @@ -70,6 +71,27 @@ impl SubstrateFinalitySource

{ // target node may be missing proofs that are already available at the source self.client.best_finalized_header_number().await } + + /// Return header and its justification of the given block or its earlier descendant that + /// has a GRANDPA justification. + pub async fn prove_block_finality( + &self, + mut block_number: BlockNumberOf, + ) -> Result< + (relay_substrate_client::SyncHeader>, SubstrateFinalityProof

), + Error, + > { + loop { + let (header, maybe_justification) = + self.header_and_finality_proof(block_number).await?; + match maybe_justification { + Some(justification) => return Ok((header, justification)), + None => { + block_number += One::one(); + }, + } + } + } } impl Clone for SubstrateFinalitySource

{ diff --git a/relays/lib-substrate-relay/src/on_demand/headers.rs b/relays/lib-substrate-relay/src/on_demand/headers.rs index 7171840ddb..f273103279 100644 --- a/relays/lib-substrate-relay/src/on_demand/headers.rs +++ b/relays/lib-substrate-relay/src/on_demand/headers.rs @@ -16,17 +16,18 @@ //! On-demand Substrate -> Substrate header finality relay. +use crate::finality::SubmitFinalityProofCallBuilder; + use async_std::sync::{Arc, Mutex}; use async_trait::async_trait; use bp_header_chain::ConsensusLogReader; use futures::{select, FutureExt}; use num_traits::{One, Zero}; use sp_runtime::traits::Header; -use std::marker::PhantomData; use finality_relay::{FinalitySyncParams, TargetClient as FinalityTargetClient}; use relay_substrate_client::{ - AccountIdOf, AccountKeyPairOf, BlockNumberOf, Chain, ChainWithTransactions, Client, + AccountIdOf, AccountKeyPairOf, BlockNumberOf, CallOf, Chain, Client, Error as SubstrateError, }; use relay_utils::{ metrics::MetricsParams, relay_loop::Client as RelayClient, FailedClient, MaybeConnectionError, @@ -50,22 +51,18 @@ use crate::{ /// relay) needs it to continue its regular work. When enough headers are relayed, on-demand stops /// syncing headers. #[derive(Clone)] -pub struct OnDemandHeadersRelay { +pub struct OnDemandHeadersRelay { /// Relay task name. relay_task_name: String, /// Shared reference to maximal required finalized header number. - required_header_number: RequiredHeaderNumberRef, - /// Just rusty things. - _marker: PhantomData, + required_header_number: RequiredHeaderNumberRef, + /// Client of the source chain. + source_client: Client, } -impl - OnDemandHeadersRelay -{ +impl OnDemandHeadersRelay

{ /// Create new on-demand headers relay. - pub fn new< - P: SubstrateFinalitySyncPipeline, - >( + pub fn new( source_client: Client, target_client: Client, target_transaction_params: TransactionParams>, @@ -79,7 +76,7 @@ impl let this = OnDemandHeadersRelay { relay_task_name: on_demand_headers_relay_name::(), required_header_number: required_header_number.clone(), - _marker: PhantomData, + source_client: source_client.clone(), }; async_std::task::spawn(async move { background_task::

( @@ -97,23 +94,38 @@ impl } #[async_trait] -impl OnDemandRelay - for OnDemandHeadersRelay +impl OnDemandRelay + for OnDemandHeadersRelay

{ - async fn require_more_headers(&self, required_header: BlockNumberOf) { + async fn require_more_headers(&self, required_header: BlockNumberOf) { let mut required_header_number = self.required_header_number.lock().await; if required_header > *required_header_number { log::trace!( target: "bridge", "[{}] More {} headers required. Going to sync up to the {}", self.relay_task_name, - SourceChain::NAME, + P::SourceChain::NAME, required_header, ); *required_header_number = required_header; } } + + async fn prove_header( + &self, + required_header: BlockNumberOf, + ) -> Result>, SubstrateError> { + // first find proper header (either `required_header`) or its descendant + let finality_source = SubstrateFinalitySource::

::new(self.source_client.clone(), None); + let (header, proof) = finality_source.prove_block_finality(required_header).await?; + + // and then craft the submit-proof call + let call = + P::SubmitFinalityProofCallBuilder::build_submit_finality_proof_call(header, proof); + + Ok(vec![call]) + } } /// Background task that is responsible for starting headers relay. diff --git a/relays/lib-substrate-relay/src/on_demand/mod.rs b/relays/lib-substrate-relay/src/on_demand/mod.rs index 5582ea080f..97a0424712 100644 --- a/relays/lib-substrate-relay/src/on_demand/mod.rs +++ b/relays/lib-substrate-relay/src/on_demand/mod.rs @@ -38,7 +38,5 @@ pub trait OnDemandRelay: Send + Sync { async fn prove_header( &self, required_header: BlockNumberOf, - ) -> Result>, SubstrateError> { - unimplemented!("TODO") - } + ) -> Result>, SubstrateError>; } diff --git a/relays/lib-substrate-relay/src/on_demand/parachains.rs b/relays/lib-substrate-relay/src/on_demand/parachains.rs index f7b010d81e..3261e9063f 100644 --- a/relays/lib-substrate-relay/src/on_demand/parachains.rs +++ b/relays/lib-substrate-relay/src/on_demand/parachains.rs @@ -38,7 +38,7 @@ use num_traits::Zero; use pallet_bridge_parachains::{RelayBlockHash, RelayBlockHasher, RelayBlockNumber}; use parachains_relay::parachains_loop::{AvailableHeader, ParachainSyncParams, TargetClient}; use relay_substrate_client::{ - AccountIdOf, AccountKeyPairOf, BlockNumberOf, Chain, ChainWithTransactions, Client, + AccountIdOf, AccountKeyPairOf, BlockNumberOf, CallOf, Chain, ChainWithTransactions, Client, Error as SubstrateError, HashOf, }; use relay_utils::{ @@ -126,6 +126,14 @@ where ); } } + + /// Ask relay to prove source `required_header` to the `TargetChain`. + async fn prove_header( + &self, + required_header: BlockNumberOf, + ) -> Result>, SubstrateError> { + unimplemented!("TODO") + } } /// Background task that is responsible for starting parachain headers relay. diff --git a/relays/messages/src/message_race_loop.rs b/relays/messages/src/message_race_loop.rs index df3939e44a..6e465a6cf3 100644 --- a/relays/messages/src/message_race_loop.rs +++ b/relays/messages/src/message_race_loop.rs @@ -525,6 +525,10 @@ pub async fn run, TC: TargetClient

>( nonces_range, at_block, ); + + unimplemented!("TODO"); + unimplemented!("TODO: using at_block here is wrong - it may happen that the batch transaction would contain the descendant of `at_block` e.g. if GRANPA justificaiton is missing for `at_block`"); + source_generate_proof.set( race_source.generate_proof(at_block, nonces_range, proof_parameters).fuse(), ); From 9e850d2e27f251a71c99274b395b703130efa1e0 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Mon, 12 Dec 2022 15:28:20 +0300 Subject: [PATCH 15/29] impl OnDemandParachainsRelay::prove_header (needs extensive tests) --- Cargo.lock | 1 + .../parachain_to_parachain.rs | 8 +- .../relay_to_parachain.rs | 4 +- relays/client-substrate/Cargo.toml | 1 + relays/client-substrate/src/error.rs | 4 + .../src/on_demand/parachains.rs | 148 ++++++++++++++---- relays/messages/src/message_lane_loop.rs | 2 + relays/utils/src/lib.rs | 2 +- 8 files changed, 135 insertions(+), 35 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bd7e97114c..e4b4a509bd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8564,6 +8564,7 @@ dependencies = [ "async-trait", "bp-header-chain", "bp-messages", + "bp-polkadot-core", "bp-runtime", "finality-relay", "frame-support", diff --git a/relays/bin-substrate/src/cli/relay_headers_and_messages/parachain_to_parachain.rs b/relays/bin-substrate/src/cli/relay_headers_and_messages/parachain_to_parachain.rs index 9c91ec7373..aaaf31fd2e 100644 --- a/relays/bin-substrate/src/cli/relay_headers_and_messages/parachain_to_parachain.rs +++ b/relays/bin-substrate/src/cli/relay_headers_and_messages/parachain_to_parachain.rs @@ -255,17 +255,17 @@ where self.common.shared.only_mandatory_headers, ); - let left_to_right_on_demand_parachains = OnDemandParachainsRelay::new::< + let left_to_right_on_demand_parachains = OnDemandParachainsRelay::< ::ParachainFinality, - >( + >::new( self.left_relay.clone(), self.common.right.client.clone(), self.left_parachains_to_right_transaction_params.clone(), Arc::new(left_relay_to_right_on_demand_headers), ); - let right_to_left_on_demand_parachains = OnDemandParachainsRelay::new::< + let right_to_left_on_demand_parachains = OnDemandParachainsRelay::< ::ParachainFinality, - >( + >::new( self.right_relay.clone(), self.common.left.client.clone(), self.right_parachains_to_left_transaction_params.clone(), diff --git a/relays/bin-substrate/src/cli/relay_headers_and_messages/relay_to_parachain.rs b/relays/bin-substrate/src/cli/relay_headers_and_messages/relay_to_parachain.rs index 7391075159..f4e6084405 100644 --- a/relays/bin-substrate/src/cli/relay_headers_and_messages/relay_to_parachain.rs +++ b/relays/bin-substrate/src/cli/relay_headers_and_messages/relay_to_parachain.rs @@ -236,9 +236,9 @@ where self.right_headers_to_left_transaction_params.clone(), self.common.shared.only_mandatory_headers, ); - let right_to_left_on_demand_parachains = OnDemandParachainsRelay::new::< + let right_to_left_on_demand_parachains = OnDemandParachainsRelay::< ::ParachainFinality, - >( + >::new( self.right_relay.clone(), self.common.left.client.clone(), self.right_parachains_to_left_transaction_params.clone(), diff --git a/relays/client-substrate/Cargo.toml b/relays/client-substrate/Cargo.toml index 60ef1c67a2..3f5156b765 100644 --- a/relays/client-substrate/Cargo.toml +++ b/relays/client-substrate/Cargo.toml @@ -21,6 +21,7 @@ thiserror = "1.0.26" bp-header-chain = { path = "../../primitives/header-chain" } bp-messages = { path = "../../primitives/messages" } +bp-polkadot-core = { path = "../../primitives/polkadot-core" } bp-runtime = { path = "../../primitives/runtime" } pallet-bridge-messages = { path = "../../modules/messages" } finality-relay = { path = "../finality" } diff --git a/relays/client-substrate/src/error.rs b/relays/client-substrate/src/error.rs index 9323b75722..b0fd659c7a 100644 --- a/relays/client-substrate/src/error.rs +++ b/relays/client-substrate/src/error.rs @@ -16,6 +16,7 @@ //! Substrate node RPC errors. +use bp_polkadot_core::parachains::ParaId; use jsonrpsee::core::Error as RpcError; use relay_utils::MaybeConnectionError; use sc_rpc_api::system::Health; @@ -45,6 +46,9 @@ pub enum Error { /// Runtime storage is missing some mandatory value. #[error("Mandatory storage value is missing from the runtime storage.")] MissingMandatoryStorageValue, + /// Required parachain head is not present at the relay chain. + #[error("Parachain {0:?} head {1} is missing from the relay chain storage.")] + MissingRequiredParachainHead(ParaId, u64), /// The client we're connected to is not synced, so we can't rely on its state. #[error("Substrate client is not synced {0}.")] ClientNotSynced(Health), diff --git a/relays/lib-substrate-relay/src/on_demand/parachains.rs b/relays/lib-substrate-relay/src/on_demand/parachains.rs index 3261e9063f..d32989ea17 100644 --- a/relays/lib-substrate-relay/src/on_demand/parachains.rs +++ b/relays/lib-substrate-relay/src/on_demand/parachains.rs @@ -21,7 +21,7 @@ use crate::{ on_demand::OnDemandRelay, parachains::{ source::ParachainsSource, target::ParachainsTarget, ParachainsPipelineAdapter, - SubstrateParachainsPipeline, + SubmitParachainHeadsCallBuilder, SubstrateParachainsPipeline, }, TransactionParams, }; @@ -31,20 +31,23 @@ use async_std::{ sync::{Arc, Mutex}, }; use async_trait::async_trait; -use bp_polkadot_core::parachains::ParaHash; +use bp_polkadot_core::parachains::{ParaHash, ParaId}; use bp_runtime::HeaderIdProvider; use futures::{select, FutureExt}; -use num_traits::Zero; +use num_traits::{Saturating, Zero}; use pallet_bridge_parachains::{RelayBlockHash, RelayBlockHasher, RelayBlockNumber}; -use parachains_relay::parachains_loop::{AvailableHeader, ParachainSyncParams, TargetClient}; +use parachains_relay::parachains_loop::{ + AvailableHeader, ParachainSyncParams, SourceClient, TargetClient, +}; use relay_substrate_client::{ - AccountIdOf, AccountKeyPairOf, BlockNumberOf, CallOf, Chain, ChainWithTransactions, Client, - Error as SubstrateError, HashOf, + AccountIdOf, AccountKeyPairOf, BlockNumberOf, CallOf, Chain, Client, Error as SubstrateError, + HashOf, }; use relay_utils::{ metrics::MetricsParams, relay_loop::Client as RelayClient, FailedClient, HeaderId, + UniqueSaturatedFrom, UniqueSaturatedInto, }; -use std::{fmt::Debug, marker::PhantomData}; +use std::fmt::Debug; /// On-demand Substrate <-> Substrate parachain finality relay. /// @@ -52,26 +55,27 @@ use std::{fmt::Debug, marker::PhantomData}; /// (e.g. messages relay) needs it to continue its regular work. When enough parachain headers /// are relayed, on-demand stops syncing headers. #[derive(Clone)] -pub struct OnDemandParachainsRelay { +pub struct OnDemandParachainsRelay { /// Relay task name. relay_task_name: String, /// Channel used to communicate with background task and ask for relay of parachain heads. - required_header_number_sender: Sender>, - /// Just rusty things. - _marker: PhantomData, + required_header_number_sender: Sender>, + /// Source relay chain client. + source_relay_client: Client, + /// Target chain client. + target_client: Client, + /// On-demand relay chain relay. + on_demand_source_relay_to_target_headers: + Arc>, } -impl - OnDemandParachainsRelay -{ +impl OnDemandParachainsRelay

{ /// Create new on-demand parachains relay. /// /// Note that the argument is the source relay chain client, not the parachain client. /// That's because parachain finality is determined by the relay chain and we don't /// need to connect to the parachain itself here. - pub fn new< - P: SubstrateParachainsPipeline, - >( + pub fn new( source_relay_client: Client, target_client: Client, target_transaction_params: TransactionParams>, @@ -88,9 +92,13 @@ impl { let (required_header_number_sender, required_header_number_receiver) = unbounded(); let this = OnDemandParachainsRelay { - relay_task_name: on_demand_parachains_relay_name::(), + relay_task_name: on_demand_parachains_relay_name::( + ), required_header_number_sender, - _marker: PhantomData, + source_relay_client: source_relay_client.clone(), + target_client: target_client.clone(), + on_demand_source_relay_to_target_headers: on_demand_source_relay_to_target_headers + .clone(), }; async_std::task::spawn(async move { background_task::

( @@ -108,19 +116,18 @@ impl } #[async_trait] -impl OnDemandRelay - for OnDemandParachainsRelay +impl OnDemandRelay + for OnDemandParachainsRelay

where - SourceParachain: Chain, - TargetChain: Chain, + P::SourceParachain: Chain, { - async fn require_more_headers(&self, required_header: BlockNumberOf) { + async fn require_more_headers(&self, required_header: BlockNumberOf) { if let Err(e) = self.required_header_number_sender.send(required_header).await { log::trace!( target: "bridge", "[{}] Failed to request {} header {:?}: {:?}", self.relay_task_name, - SourceParachain::NAME, + P::SourceParachain::NAME, required_header, e, ); @@ -130,9 +137,94 @@ where /// Ask relay to prove source `required_header` to the `TargetChain`. async fn prove_header( &self, - required_header: BlockNumberOf, - ) -> Result>, SubstrateError> { - unimplemented!("TODO") + required_header: BlockNumberOf, + ) -> Result>, SubstrateError> { + // parachains proof also requires relay header proof. Let's first select relay block + // number that we'll be dealing with + let parachains_source = ParachainsSource::

::new( + self.source_relay_client.clone(), + Arc::new(Mutex::new(AvailableHeader::Missing)), + ); + let best_finalized_relay_block_at_source = + self.source_relay_client.best_finalized_header().await?.id(); + let best_finalized_relay_block_at_target = + crate::messages_source::read_client_state::( + &self.target_client, + None, + P::SourceRelayChain::BEST_FINALIZED_HEADER_ID_METHOD, + ) + .await? + .best_finalized_peer_at_best_self; + + // if we can't prove `required_header` even using `best_finalized_relay_block_at_source`, we + // can't do anything here + // (this shall not actually happen, given current code, because we only require finalized + // headers) + let para_id = ParaId(P::SOURCE_PARACHAIN_PARA_ID); + let best_possible_parachain_block = parachains_source + .on_chain_para_head_id(best_finalized_relay_block_at_target, para_id) + .await?; + let best_possible_parachain_block = match best_possible_parachain_block { + Some(best_possible_parachain_block) + if best_possible_parachain_block.number() >= required_header => + best_possible_parachain_block, + _ => + return Err(SubstrateError::MissingRequiredParachainHead( + para_id, + required_header.unique_saturated_into(), + )), + }; + + // now let's check if `required_header` may be proved using + // `best_finalized_relay_block_at_target` + let available_parachain_block = parachains_source + .on_chain_para_head_id(best_finalized_relay_block_at_target, para_id) + .await?; + let can_use_available_relay_header = available_parachain_block + .as_ref() + .map(|available_parachain_block| available_parachain_block.number() >= required_header) + .unwrap_or(false); + + // we don't require source node to be archive, so we can't craft storage proofs using + // ancient headers. So if the `best_finalized_relay_block_at_target` is too ancient, we + // can't craft storage proofs using it + let difference = best_finalized_relay_block_at_source + .number() + .saturating_sub(best_finalized_relay_block_at_target.number()); + let can_use_available_relay_header = can_use_available_relay_header && + difference < BlockNumberOf::::unique_saturated_from(100u64); // TODO: extract const + + // ok - now we have everything ready to select which headers we need on the target chain + let (selected_relay_block, selected_parachain_block) = if can_use_available_relay_header { + ( + best_finalized_relay_block_at_target, + available_parachain_block.expect("TODO").number(), + ) + } else { + (best_finalized_relay_block_at_source, best_possible_parachain_block.number()) + }; + + // now let's prove relay chain block (if needed) + let mut calls = Vec::new(); + if selected_relay_block != best_finalized_relay_block_at_target { + calls.extend( + self.on_demand_source_relay_to_target_headers + .prove_header(selected_relay_block.number()) + .await?, + ); + } + + // and finally - prove parachain head + let (para_proof, para_hashes) = parachains_source + .prove_parachain_heads(selected_relay_block, &[para_id]) + .await?; + calls.push(P::SubmitParachainHeadsCallBuilder::build_submit_parachain_heads_call( + selected_relay_block, + para_hashes.into_iter().map(|h| (para_id, h)).collect(), + para_proof, + )); + + Ok(calls) } } diff --git a/relays/messages/src/message_lane_loop.rs b/relays/messages/src/message_lane_loop.rs index 3cc21b2972..5bbe413310 100644 --- a/relays/messages/src/message_lane_loop.rs +++ b/relays/messages/src/message_lane_loop.rs @@ -176,6 +176,8 @@ pub trait SourceClient: RelayClient { /// We need given finalized target header on source to continue synchronization. /// + /// We assume that the absence of header `id` has already been checked by caller. + /// /// The client may return `Some(_)`, which means that nothing has happened yet and /// the caller must generate and append message receiving proof to the batch transaction /// to actually send it (along with required header) to the node. diff --git a/relays/utils/src/lib.rs b/relays/utils/src/lib.rs index 16e1a18bab..428ee33494 100644 --- a/relays/utils/src/lib.rs +++ b/relays/utils/src/lib.rs @@ -19,7 +19,7 @@ pub use bp_runtime::HeaderId; pub use error::Error; pub use relay_loop::{relay_loop, relay_metrics}; -pub use sp_runtime::traits::UniqueSaturatedInto; +pub use sp_runtime::traits::{UniqueSaturatedFrom, UniqueSaturatedInto}; use async_trait::async_trait; use backoff::{backoff::Backoff, ExponentialBackoff}; From b0a277b3aac9514f784a71ef6c5a61a4656f2e02 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Mon, 12 Dec 2022 16:17:07 +0300 Subject: [PATCH 16/29] added a couple of TODOs --- relays/lib-substrate-relay/src/messages_source.rs | 5 ++++- relays/lib-substrate-relay/src/messages_target.rs | 5 ++++- relays/lib-substrate-relay/src/on_demand/headers.rs | 5 +++-- relays/lib-substrate-relay/src/on_demand/mod.rs | 5 ++++- relays/lib-substrate-relay/src/on_demand/parachains.rs | 7 ++++--- 5 files changed, 19 insertions(+), 8 deletions(-) diff --git a/relays/lib-substrate-relay/src/messages_source.rs b/relays/lib-substrate-relay/src/messages_source.rs index c4fd1c3bfe..687c065b00 100644 --- a/relays/lib-substrate-relay/src/messages_source.rs +++ b/relays/lib-substrate-relay/src/messages_source.rs @@ -405,13 +405,16 @@ where self, proof: as MessageLane>::MessagesReceivingProof, ) -> Result>, SubstrateError> { + unimplemented!("TODO"); + unimplemented!("TODO: move prove_header call to constructor and return proper header id from required_header_id method"); let mut calls = self .messages_source .target_to_source_headers_relay .as_ref() .expect("BatchConfirmationTransaction is only created when target_to_source_headers_relay is Some; qed") .prove_header(self.required_target_header_on_source.0) - .await?; + .await? + .1; calls.push( P::ReceiveMessagesDeliveryProofCallBuilder::build_receive_messages_delivery_proof_call( proof, false, diff --git a/relays/lib-substrate-relay/src/messages_target.rs b/relays/lib-substrate-relay/src/messages_target.rs index b8e729ba4e..b045446df1 100644 --- a/relays/lib-substrate-relay/src/messages_target.rs +++ b/relays/lib-substrate-relay/src/messages_target.rs @@ -312,13 +312,16 @@ where self, proof: as MessageLane>::MessagesProof, ) -> Result>, SubstrateError> { + unimplemented!("TODO"); + unimplemented!("TODO: move prove_header call to constructor and return proper header id from required_header_id method"); let mut calls = self .messages_target .source_to_target_headers_relay .as_ref() .expect("BatchDeliveryTransaction is only created when source_to_target_headers_relay is Some; qed") .prove_header(self.required_source_header_on_target.0) - .await?; + .await? + .1; calls.push(make_messages_delivery_call::

( self.messages_target.relayer_id_at_source, proof.1.nonces_start..=proof.1.nonces_end, diff --git a/relays/lib-substrate-relay/src/on_demand/headers.rs b/relays/lib-substrate-relay/src/on_demand/headers.rs index f273103279..8b22d55a58 100644 --- a/relays/lib-substrate-relay/src/on_demand/headers.rs +++ b/relays/lib-substrate-relay/src/on_demand/headers.rs @@ -115,16 +115,17 @@ impl OnDemandRelay, - ) -> Result>, SubstrateError> { + ) -> Result<(BlockNumberOf, Vec>), SubstrateError> { // first find proper header (either `required_header`) or its descendant let finality_source = SubstrateFinalitySource::

::new(self.source_client.clone(), None); let (header, proof) = finality_source.prove_block_finality(required_header).await?; + let header_number = *header.number(); // and then craft the submit-proof call let call = P::SubmitFinalityProofCallBuilder::build_submit_finality_proof_call(header, proof); - Ok(vec![call]) + Ok((header_number, vec![call])) } } diff --git a/relays/lib-substrate-relay/src/on_demand/mod.rs b/relays/lib-substrate-relay/src/on_demand/mod.rs index 97a0424712..25dd2931dc 100644 --- a/relays/lib-substrate-relay/src/on_demand/mod.rs +++ b/relays/lib-substrate-relay/src/on_demand/mod.rs @@ -35,8 +35,11 @@ pub trait OnDemandRelay: Send + Sync { async fn require_more_headers(&self, required_header: BlockNumberOf); /// Ask relay to prove source `required_header` to the `TargetChain`. + /// + /// Returns number of header that is proved (it may be the `required_header` or one of its + /// descendants) and calls for delivering the proof. async fn prove_header( &self, required_header: BlockNumberOf, - ) -> Result>, SubstrateError>; + ) -> Result<(BlockNumberOf, Vec>), SubstrateError>; } diff --git a/relays/lib-substrate-relay/src/on_demand/parachains.rs b/relays/lib-substrate-relay/src/on_demand/parachains.rs index d32989ea17..71cc7432ce 100644 --- a/relays/lib-substrate-relay/src/on_demand/parachains.rs +++ b/relays/lib-substrate-relay/src/on_demand/parachains.rs @@ -138,7 +138,7 @@ where async fn prove_header( &self, required_header: BlockNumberOf, - ) -> Result>, SubstrateError> { + ) -> Result<(BlockNumberOf, Vec>), SubstrateError> { // parachains proof also requires relay header proof. Let's first select relay block // number that we'll be dealing with let parachains_source = ParachainsSource::

::new( @@ -210,7 +210,8 @@ where calls.extend( self.on_demand_source_relay_to_target_headers .prove_header(selected_relay_block.number()) - .await?, + .await? + .1, ); } @@ -224,7 +225,7 @@ where para_proof, )); - Ok(calls) + Ok((selected_parachain_block, calls)) } } From a1c50d6c2070312f8f92bd457651d42b7790723b Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Tue, 13 Dec 2022 09:47:32 +0300 Subject: [PATCH 17/29] return Result> when asking for more headers --- relays/messages/src/message_lane_loop.rs | 16 +++---- relays/messages/src/message_race_delivery.rs | 5 ++- relays/messages/src/message_race_loop.rs | 45 ++++++++++++++----- relays/messages/src/message_race_receiving.rs | 5 ++- 4 files changed, 50 insertions(+), 21 deletions(-) diff --git a/relays/messages/src/message_lane_loop.rs b/relays/messages/src/message_lane_loop.rs index 5bbe413310..f4f91d50dd 100644 --- a/relays/messages/src/message_lane_loop.rs +++ b/relays/messages/src/message_lane_loop.rs @@ -187,7 +187,7 @@ pub trait SourceClient: RelayClient { async fn require_target_header_on_source( &self, id: TargetHeaderIdOf

, - ) -> Option; + ) -> Result, Self::Error>; } /// Target client trait. @@ -249,7 +249,7 @@ pub trait TargetClient: RelayClient { async fn require_source_header_on_target( &self, id: SourceHeaderIdOf

, - ) -> Option; + ) -> Result, Self::Error>; } /// State of the client. @@ -813,17 +813,17 @@ pub(crate) mod tests { async fn require_target_header_on_source( &self, id: TargetHeaderIdOf, - ) -> Option { + ) -> Result, Self::Error> { let mut data = self.data.lock(); data.target_to_source_header_required = Some(id); data.target_to_source_header_requirements.push(id); (self.tick)(&mut data); (self.post_tick)(&mut data); - data.target_to_source_batch_transaction.take().map(|mut tx| { + Ok(data.target_to_source_batch_transaction.take().map(|mut tx| { tx.required_header_id = id; tx - }) + })) } } @@ -944,17 +944,17 @@ pub(crate) mod tests { async fn require_source_header_on_target( &self, id: SourceHeaderIdOf, - ) -> Option { + ) -> Result, Self::Error> { let mut data = self.data.lock(); data.source_to_target_header_required = Some(id); data.source_to_target_header_requirements.push(id); (self.tick)(&mut data); (self.post_tick)(&mut data); - data.source_to_target_batch_transaction.take().map(|mut tx| { + Ok(data.source_to_target_batch_transaction.take().map(|mut tx| { tx.required_header_id = id; tx - }) + })) } } diff --git a/relays/messages/src/message_race_delivery.rs b/relays/messages/src/message_race_delivery.rs index bdb89dd6fe..660eb333dc 100644 --- a/relays/messages/src/message_race_delivery.rs +++ b/relays/messages/src/message_race_delivery.rs @@ -174,7 +174,10 @@ where type BatchTransaction = C::BatchTransaction; type TransactionTracker = C::TransactionTracker; - async fn require_source_header(&self, id: SourceHeaderIdOf

) -> Option { + async fn require_source_header( + &self, + id: SourceHeaderIdOf

, + ) -> Result, Self::Error> { self.client.require_source_header_on_target(id).await } diff --git a/relays/messages/src/message_race_loop.rs b/relays/messages/src/message_race_loop.rs index 6e465a6cf3..0da2d3baf7 100644 --- a/relays/messages/src/message_race_loop.rs +++ b/relays/messages/src/message_race_loop.rs @@ -146,8 +146,10 @@ pub trait TargetClient { /// /// If function has returned `None`, it means that the caller now must wait for the /// appearance of the required header `id` at the target client. - #[must_use] - async fn require_source_header(&self, id: P::SourceHeaderId) -> Option; + async fn require_source_header( + &self, + id: P::SourceHeaderId, + ) -> Result, Self::Error>; /// Return nonces that are known to the target client. async fn nonces( @@ -257,6 +259,7 @@ pub async fn run, TC: TargetClient

>( let mut source_retry_backoff = retry_backoff(); let mut source_client_is_online = true; let mut source_nonces_required = false; + let mut source_required_header = None; let source_nonces = futures::future::Fuse::terminated(); let source_generate_proof = futures::future::Fuse::terminated(); let source_go_offline_future = futures::future::Fuse::terminated(); @@ -266,6 +269,7 @@ pub async fn run, TC: TargetClient

>( let mut target_best_nonces_required = false; let mut target_finalized_nonces_required = false; let mut target_batch_transaction = None; + let target_require_source_header = futures::future::Fuse::terminated(); let target_best_nonces = futures::future::Fuse::terminated(); let target_finalized_nonces = futures::future::Fuse::terminated(); let target_submit_proof = futures::future::Fuse::terminated(); @@ -278,6 +282,7 @@ pub async fn run, TC: TargetClient

>( source_generate_proof, source_go_offline_future, race_target_updated, + target_require_source_header, target_best_nonces, target_finalized_nonces, target_submit_proof, @@ -342,15 +347,10 @@ pub async fn run, TC: TargetClient

>( ).fail_if_connection_error(FailedClient::Source)?; // ask for more headers if we have nonces to deliver and required headers are missing - let required_source_header_id = race_state + source_required_header = race_state .best_finalized_source_header_id_at_best_target .as_ref() .and_then(|best| strategy.required_source_header_at_target(best)); - if let Some(required_source_header_id) = required_source_header_id { - target_batch_transaction = race_target - .require_source_header(required_source_header_id) - .await; - } }, nonces = target_best_nonces => { target_best_nonces_required = false; @@ -396,6 +396,28 @@ pub async fn run, TC: TargetClient

>( }, // proof generation and submission + maybe_batch_transaction = target_require_source_header => { + source_required_header = None; + + target_client_is_online = process_future_result( + maybe_batch_transaction, + &mut target_retry_backoff, + |maybe_batch_transaction: Option| { + log::debug!( + target: "bridge", + "Target {} client has been asked for more {} headers. Batch tx: {:?}", + P::target_name(), + P::source_name(), + maybe_batch_transaction.is_some(), + ); + + target_batch_transaction = maybe_batch_transaction; + }, + &mut target_go_offline_future, + async_std::task::sleep, + || format!("Error asking for source headers at {}", P::target_name()), + ).fail_if_connection_error(FailedClient::Target)?; + }, proof = source_generate_proof => { source_client_is_online = process_future_result( proof, @@ -526,9 +548,6 @@ pub async fn run, TC: TargetClient

>( at_block, ); - unimplemented!("TODO"); - unimplemented!("TODO: using at_block here is wrong - it may happen that the batch transaction would contain the descendant of `at_block` e.g. if GRANPA justificaiton is missing for `at_block`"); - source_generate_proof.set( race_source.generate_proof(at_block, nonces_range, proof_parameters).fuse(), ); @@ -591,6 +610,10 @@ pub async fn run, TC: TargetClient

>( .fuse(), ); } + } else if let Some(source_required_header) = source_required_header.clone() { + log::debug!(target: "bridge", "Going to require {} header {:?} at {}", P::source_name(), source_required_header, P::target_name()); + target_require_source_header + .set(race_target.require_source_header(source_required_header).fuse()); } else if target_best_nonces_required { log::debug!(target: "bridge", "Asking {} about best message nonces", P::target_name()); let at_block = race_state diff --git a/relays/messages/src/message_race_receiving.rs b/relays/messages/src/message_race_receiving.rs index 2fbef5defa..c807d76fe5 100644 --- a/relays/messages/src/message_race_receiving.rs +++ b/relays/messages/src/message_race_receiving.rs @@ -158,7 +158,10 @@ where type BatchTransaction = C::BatchTransaction; type TransactionTracker = C::TransactionTracker; - async fn require_source_header(&self, id: TargetHeaderIdOf

) -> Option { + async fn require_source_header( + &self, + id: TargetHeaderIdOf

, + ) -> Result, Self::Error> { self.client.require_target_header_on_source(id).await } From b85215833392e5cd6603805da67bd316e9d3da3f Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Tue, 13 Dec 2022 10:19:03 +0300 Subject: [PATCH 18/29] prove headers when reauire_* is called && return proper headers from required_header_id --- relays/lib-substrate-relay/src/lib.rs | 8 +++- .../src/messages_source.rs | 44 ++++++++++--------- .../src/messages_target.rs | 40 +++++++++-------- .../src/on_demand/headers.rs | 8 ++-- .../lib-substrate-relay/src/on_demand/mod.rs | 4 +- .../src/on_demand/parachains.rs | 11 ++--- 6 files changed, 63 insertions(+), 52 deletions(-) diff --git a/relays/lib-substrate-relay/src/lib.rs b/relays/lib-substrate-relay/src/lib.rs index bbff815720..c1a13cc72e 100644 --- a/relays/lib-substrate-relay/src/lib.rs +++ b/relays/lib-substrate-relay/src/lib.rs @@ -131,8 +131,12 @@ where const BATCH_CALL_SUPPORTED: bool = true; fn build_batch_call( - calls: Vec<::RuntimeCall>, + mut calls: Vec<::RuntimeCall>, ) -> ::RuntimeCall { - pallet_utility::Call::batch_all { calls }.into() + if calls.len() == 1 { + calls.remove(0) + } else { + pallet_utility::Call::batch_all { calls }.into() + } } } diff --git a/relays/lib-substrate-relay/src/messages_source.rs b/relays/lib-substrate-relay/src/messages_source.rs index 687c065b00..dc05723783 100644 --- a/relays/lib-substrate-relay/src/messages_source.rs +++ b/relays/lib-substrate-relay/src/messages_source.rs @@ -47,8 +47,8 @@ use messages_relay::{ }; use num_traits::Zero; use relay_substrate_client::{ - AccountIdOf, AccountKeyPairOf, BalanceOf, BlockNumberOf, Chain, ChainWithMessages, Client, - Error as SubstrateError, HashOf, HeaderIdOf, IndexOf, SignParam, TransactionEra, + AccountIdOf, AccountKeyPairOf, BalanceOf, BlockNumberOf, CallOf, Chain, ChainWithMessages, + Client, Error as SubstrateError, HashOf, HeaderIdOf, IndexOf, SignParam, TransactionEra, TransactionTracker, UnsignedTransaction, }; use relay_utils::{relay_loop::Client as RelayClient, HeaderId}; @@ -364,26 +364,39 @@ where async fn require_target_header_on_source( &self, id: TargetHeaderIdOf>, - ) -> Option { + ) -> Result, SubstrateError> { if let Some(ref target_to_source_headers_relay) = self.target_to_source_headers_relay { if P::SourceBatchCallBuilder::BATCH_CALL_SUPPORTED { - return Some(BatchConfirmationTransaction::

{ - messages_source: self.clone(), - required_target_header_on_source: id, - }) + return BatchConfirmationTransaction::

::new(self.clone(), id).await.map(Some) } target_to_source_headers_relay.require_more_headers(id.0).await; } - None + Ok(None) } } /// Batch transaction that brings target headers + and delivery confirmations to the source node. pub struct BatchConfirmationTransaction { messages_source: SubstrateMessagesSource

, - required_target_header_on_source: TargetHeaderIdOf>, + proved_header: TargetHeaderIdOf>, + prove_calls: Vec>, +} + +impl BatchConfirmationTransaction

{ + async fn new( + messages_source: SubstrateMessagesSource

, + required_target_header_on_source: TargetHeaderIdOf>, + ) -> Result { + let (proved_header, prove_calls) = messages_source + .target_to_source_headers_relay + .as_ref() + .expect("BatchConfirmationTransaction is only created when target_to_source_headers_relay is Some; qed") + .prove_header(required_target_header_on_source.0) + .await?; + Ok(Self { messages_source, proved_header, prove_calls }) + } } #[async_trait] @@ -398,23 +411,14 @@ where AccountIdOf: From< as Pair>::Public>, { fn required_header_id(&self) -> TargetHeaderIdOf> { - self.required_target_header_on_source.clone() + self.proved_header.clone() } async fn append_proof_and_send( self, proof: as MessageLane>::MessagesReceivingProof, ) -> Result>, SubstrateError> { - unimplemented!("TODO"); - unimplemented!("TODO: move prove_header call to constructor and return proper header id from required_header_id method"); - let mut calls = self - .messages_source - .target_to_source_headers_relay - .as_ref() - .expect("BatchConfirmationTransaction is only created when target_to_source_headers_relay is Some; qed") - .prove_header(self.required_target_header_on_source.0) - .await? - .1; + let mut calls = self.prove_calls; calls.push( P::ReceiveMessagesDeliveryProofCallBuilder::build_receive_messages_delivery_proof_call( proof, false, diff --git a/relays/lib-substrate-relay/src/messages_target.rs b/relays/lib-substrate-relay/src/messages_target.rs index b045446df1..c92ad107b3 100644 --- a/relays/lib-substrate-relay/src/messages_target.rs +++ b/relays/lib-substrate-relay/src/messages_target.rs @@ -271,26 +271,39 @@ where async fn require_source_header_on_target( &self, id: SourceHeaderIdOf>, - ) -> Option { + ) -> Result, SubstrateError> { if let Some(ref source_to_target_headers_relay) = self.source_to_target_headers_relay { if P::TargetBatchCallBuilder::BATCH_CALL_SUPPORTED { - return Some(BatchDeliveryTransaction::

{ - messages_target: self.clone(), - required_source_header_on_target: id, - }) + return BatchDeliveryTransaction::

::new(self.clone(), id).await.map(Some) } source_to_target_headers_relay.require_more_headers(id.0).await; } - None + Ok(None) } } /// Batch transaction that brings target headers + and delivery confirmations to the source node. pub struct BatchDeliveryTransaction { messages_target: SubstrateMessagesTarget

, - required_source_header_on_target: SourceHeaderIdOf>, + proved_header: SourceHeaderIdOf>, + prove_calls: Vec>, +} + +impl BatchDeliveryTransaction

{ + async fn new( + messages_target: SubstrateMessagesTarget

, + required_source_header_on_target: SourceHeaderIdOf>, + ) -> Result { + let (proved_header, prove_calls) = messages_target + .source_to_target_headers_relay + .as_ref() + .expect("BatchDeliveryTransaction is only created when source_to_target_headers_relay is Some; qed") + .prove_header(required_source_header_on_target.0) + .await?; + Ok(Self { messages_target, proved_header, prove_calls }) + } } #[async_trait] @@ -305,23 +318,14 @@ where AccountIdOf: From< as Pair>::Public>, { fn required_header_id(&self) -> SourceHeaderIdOf> { - self.required_source_header_on_target.clone() + self.proved_header.clone() } async fn append_proof_and_send( self, proof: as MessageLane>::MessagesProof, ) -> Result>, SubstrateError> { - unimplemented!("TODO"); - unimplemented!("TODO: move prove_header call to constructor and return proper header id from required_header_id method"); - let mut calls = self - .messages_target - .source_to_target_headers_relay - .as_ref() - .expect("BatchDeliveryTransaction is only created when source_to_target_headers_relay is Some; qed") - .prove_header(self.required_source_header_on_target.0) - .await? - .1; + let mut calls = self.prove_calls; calls.push(make_messages_delivery_call::

( self.messages_target.relayer_id_at_source, proof.1.nonces_start..=proof.1.nonces_end, diff --git a/relays/lib-substrate-relay/src/on_demand/headers.rs b/relays/lib-substrate-relay/src/on_demand/headers.rs index 8b22d55a58..2e5c4ca143 100644 --- a/relays/lib-substrate-relay/src/on_demand/headers.rs +++ b/relays/lib-substrate-relay/src/on_demand/headers.rs @@ -21,6 +21,7 @@ use crate::finality::SubmitFinalityProofCallBuilder; use async_std::sync::{Arc, Mutex}; use async_trait::async_trait; use bp_header_chain::ConsensusLogReader; +use bp_runtime::HeaderIdProvider; use futures::{select, FutureExt}; use num_traits::{One, Zero}; use sp_runtime::traits::Header; @@ -28,6 +29,7 @@ use sp_runtime::traits::Header; use finality_relay::{FinalitySyncParams, TargetClient as FinalityTargetClient}; use relay_substrate_client::{ AccountIdOf, AccountKeyPairOf, BlockNumberOf, CallOf, Chain, Client, Error as SubstrateError, + HeaderIdOf, }; use relay_utils::{ metrics::MetricsParams, relay_loop::Client as RelayClient, FailedClient, MaybeConnectionError, @@ -115,17 +117,17 @@ impl OnDemandRelay, - ) -> Result<(BlockNumberOf, Vec>), SubstrateError> { + ) -> Result<(HeaderIdOf, Vec>), SubstrateError> { // first find proper header (either `required_header`) or its descendant let finality_source = SubstrateFinalitySource::

::new(self.source_client.clone(), None); let (header, proof) = finality_source.prove_block_finality(required_header).await?; - let header_number = *header.number(); + let header_id = header.id(); // and then craft the submit-proof call let call = P::SubmitFinalityProofCallBuilder::build_submit_finality_proof_call(header, proof); - Ok((header_number, vec![call])) + Ok((header_id, vec![call])) } } diff --git a/relays/lib-substrate-relay/src/on_demand/mod.rs b/relays/lib-substrate-relay/src/on_demand/mod.rs index 25dd2931dc..eca7d20163 100644 --- a/relays/lib-substrate-relay/src/on_demand/mod.rs +++ b/relays/lib-substrate-relay/src/on_demand/mod.rs @@ -18,7 +18,7 @@ //! on-demand pipelines. use async_trait::async_trait; -use relay_substrate_client::{BlockNumberOf, CallOf, Chain, Error as SubstrateError}; +use relay_substrate_client::{BlockNumberOf, CallOf, Chain, Error as SubstrateError, HeaderIdOf}; pub mod headers; pub mod parachains; @@ -41,5 +41,5 @@ pub trait OnDemandRelay: Send + Sync { async fn prove_header( &self, required_header: BlockNumberOf, - ) -> Result<(BlockNumberOf, Vec>), SubstrateError>; + ) -> Result<(HeaderIdOf, Vec>), SubstrateError>; } diff --git a/relays/lib-substrate-relay/src/on_demand/parachains.rs b/relays/lib-substrate-relay/src/on_demand/parachains.rs index 71cc7432ce..8b14a30fa6 100644 --- a/relays/lib-substrate-relay/src/on_demand/parachains.rs +++ b/relays/lib-substrate-relay/src/on_demand/parachains.rs @@ -41,7 +41,7 @@ use parachains_relay::parachains_loop::{ }; use relay_substrate_client::{ AccountIdOf, AccountKeyPairOf, BlockNumberOf, CallOf, Chain, Client, Error as SubstrateError, - HashOf, + HashOf, HeaderIdOf, }; use relay_utils::{ metrics::MetricsParams, relay_loop::Client as RelayClient, FailedClient, HeaderId, @@ -138,7 +138,7 @@ where async fn prove_header( &self, required_header: BlockNumberOf, - ) -> Result<(BlockNumberOf, Vec>), SubstrateError> { + ) -> Result<(HeaderIdOf, Vec>), SubstrateError> { // parachains proof also requires relay header proof. Let's first select relay block // number that we'll be dealing with let parachains_source = ParachainsSource::

::new( @@ -196,12 +196,9 @@ where // ok - now we have everything ready to select which headers we need on the target chain let (selected_relay_block, selected_parachain_block) = if can_use_available_relay_header { - ( - best_finalized_relay_block_at_target, - available_parachain_block.expect("TODO").number(), - ) + (best_finalized_relay_block_at_target, available_parachain_block.expect("TODO")) } else { - (best_finalized_relay_block_at_source, best_possible_parachain_block.number()) + (best_finalized_relay_block_at_source, best_possible_parachain_block) }; // now let's prove relay chain block (if needed) From 38561c0010d082a97d1877b86d492359d1c2865b Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Tue, 13 Dec 2022 11:27:39 +0300 Subject: [PATCH 19/29] split parachains::prove_header and test select_headers_to_prove --- .../src/on_demand/parachains.rs | 274 ++++++++++++++---- 1 file changed, 212 insertions(+), 62 deletions(-) diff --git a/relays/lib-substrate-relay/src/on_demand/parachains.rs b/relays/lib-substrate-relay/src/on_demand/parachains.rs index 8b14a30fa6..a968fbcd51 100644 --- a/relays/lib-substrate-relay/src/on_demand/parachains.rs +++ b/relays/lib-substrate-relay/src/on_demand/parachains.rs @@ -34,7 +34,7 @@ use async_trait::async_trait; use bp_polkadot_core::parachains::{ParaHash, ParaId}; use bp_runtime::HeaderIdProvider; use futures::{select, FutureExt}; -use num_traits::{Saturating, Zero}; +use num_traits::Zero; use pallet_bridge_parachains::{RelayBlockHash, RelayBlockHasher, RelayBlockNumber}; use parachains_relay::parachains_loop::{ AvailableHeader, ParachainSyncParams, SourceClient, TargetClient, @@ -44,8 +44,8 @@ use relay_substrate_client::{ HashOf, HeaderIdOf, }; use relay_utils::{ - metrics::MetricsParams, relay_loop::Client as RelayClient, FailedClient, HeaderId, - UniqueSaturatedFrom, UniqueSaturatedInto, + metrics::MetricsParams, relay_loop::Client as RelayClient, BlockNumberBase, FailedClient, + HeaderId, }; use std::fmt::Debug; @@ -137,73 +137,20 @@ where /// Ask relay to prove source `required_header` to the `TargetChain`. async fn prove_header( &self, - required_header: BlockNumberOf, + required_parachain_header: BlockNumberOf, ) -> Result<(HeaderIdOf, Vec>), SubstrateError> { - // parachains proof also requires relay header proof. Let's first select relay block - // number that we'll be dealing with + // select headers to prove let parachains_source = ParachainsSource::

::new( self.source_relay_client.clone(), Arc::new(Mutex::new(AvailableHeader::Missing)), ); - let best_finalized_relay_block_at_source = - self.source_relay_client.best_finalized_header().await?.id(); - let best_finalized_relay_block_at_target = - crate::messages_source::read_client_state::( - &self.target_client, - None, - P::SourceRelayChain::BEST_FINALIZED_HEADER_ID_METHOD, - ) - .await? - .best_finalized_peer_at_best_self; - - // if we can't prove `required_header` even using `best_finalized_relay_block_at_source`, we - // can't do anything here - // (this shall not actually happen, given current code, because we only require finalized - // headers) - let para_id = ParaId(P::SOURCE_PARACHAIN_PARA_ID); - let best_possible_parachain_block = parachains_source - .on_chain_para_head_id(best_finalized_relay_block_at_target, para_id) - .await?; - let best_possible_parachain_block = match best_possible_parachain_block { - Some(best_possible_parachain_block) - if best_possible_parachain_block.number() >= required_header => - best_possible_parachain_block, - _ => - return Err(SubstrateError::MissingRequiredParachainHead( - para_id, - required_header.unique_saturated_into(), - )), - }; - - // now let's check if `required_header` may be proved using - // `best_finalized_relay_block_at_target` - let available_parachain_block = parachains_source - .on_chain_para_head_id(best_finalized_relay_block_at_target, para_id) - .await?; - let can_use_available_relay_header = available_parachain_block - .as_ref() - .map(|available_parachain_block| available_parachain_block.number() >= required_header) - .unwrap_or(false); - - // we don't require source node to be archive, so we can't craft storage proofs using - // ancient headers. So if the `best_finalized_relay_block_at_target` is too ancient, we - // can't craft storage proofs using it - let difference = best_finalized_relay_block_at_source - .number() - .saturating_sub(best_finalized_relay_block_at_target.number()); - let can_use_available_relay_header = can_use_available_relay_header && - difference < BlockNumberOf::::unique_saturated_from(100u64); // TODO: extract const - - // ok - now we have everything ready to select which headers we need on the target chain - let (selected_relay_block, selected_parachain_block) = if can_use_available_relay_header { - (best_finalized_relay_block_at_target, available_parachain_block.expect("TODO")) - } else { - (best_finalized_relay_block_at_source, best_possible_parachain_block) - }; + let env = (self, ¶chains_source); + let (need_to_prove_relay_block, selected_relay_block, selected_parachain_block) = + select_headers_to_prove(env, required_parachain_header).await?; // now let's prove relay chain block (if needed) let mut calls = Vec::new(); - if selected_relay_block != best_finalized_relay_block_at_target { + if need_to_prove_relay_block { calls.extend( self.on_demand_source_relay_to_target_headers .prove_header(selected_relay_block.number()) @@ -213,6 +160,7 @@ where } // and finally - prove parachain head + let para_id = ParaId(P::SOURCE_PARACHAIN_PARA_ID); let (para_proof, para_hashes) = parachains_source .prove_parachain_heads(selected_relay_block, &[para_id]) .await?; @@ -590,6 +538,132 @@ where RelayState::RelayingParaHeader(para_header_at_source) } +/// Envirnonment for the `select_headers_to_prove` call. +#[async_trait] +trait SelectHeadersToProveEnvironment { + /// Returns associated parachain id. + fn parachain_id(&self) -> ParaId; + /// Returns best finalized relay block. + async fn best_finalized_relay_block_at_source( + &self, + ) -> Result, SubstrateError>; + /// Returns best finalized relay block that is known at `P::TargetChain`. + async fn best_finalized_relay_block_at_target( + &self, + ) -> Result, SubstrateError>; + /// Returns best finalized parachain block at given source relay chain block. + async fn best_finalized_para_block_at_source( + &self, + at_relay_block: HeaderId, + ) -> Result>, SubstrateError>; +} + +#[async_trait] +impl<'a, P: SubstrateParachainsPipeline> + SelectHeadersToProveEnvironment< + BlockNumberOf, + HashOf, + BlockNumberOf, + HashOf, + > for (&'a OnDemandParachainsRelay

, &'a ParachainsSource

) +{ + fn parachain_id(&self) -> ParaId { + ParaId(P::SOURCE_PARACHAIN_PARA_ID) + } + + async fn best_finalized_relay_block_at_source( + &self, + ) -> Result, SubstrateError> { + Ok(self.0.source_relay_client.best_finalized_header().await?.id()) + } + + async fn best_finalized_relay_block_at_target( + &self, + ) -> Result, SubstrateError> { + Ok(crate::messages_source::read_client_state::( + &self.0.target_client, + None, + P::SourceRelayChain::BEST_FINALIZED_HEADER_ID_METHOD, + ) + .await? + .best_finalized_peer_at_best_self) + } + + async fn best_finalized_para_block_at_source( + &self, + at_relay_block: HeaderIdOf, + ) -> Result>, SubstrateError> { + self.1.on_chain_para_head_id(at_relay_block, self.parachain_id()).await + } +} + +/// Given request to prove `required_parachain_header`, select actual headers that need to be +/// proved. +async fn select_headers_to_prove( + env: impl SelectHeadersToProveEnvironment, + required_parachain_header: PBN, +) -> Result<(bool, HeaderId, HeaderId), SubstrateError> +where + RBH: Copy, + RBN: BlockNumberBase, + PBH: Copy, + PBN: BlockNumberBase, +{ + // parachains proof also requires relay header proof. Let's first select relay block + // number that we'll be dealing with + let best_finalized_relay_block_at_source = env.best_finalized_relay_block_at_source().await?; + let best_finalized_relay_block_at_target = env.best_finalized_relay_block_at_target().await?; + + // if we can't prove `required_header` even using `best_finalized_relay_block_at_source`, we + // can't do anything here + // (this shall not actually happen, given current code, because we only require finalized + // headers) + let best_possible_parachain_block = env + .best_finalized_para_block_at_source(best_finalized_relay_block_at_source) + .await?; + let best_possible_parachain_block = match best_possible_parachain_block { + Some(best_possible_parachain_block) + if best_possible_parachain_block.number() >= required_parachain_header => + best_possible_parachain_block, + _ => + return Err(SubstrateError::MissingRequiredParachainHead( + env.parachain_id(), + required_parachain_header.unique_saturated_into(), + )), + }; + + // now let's check if `required_header` may be proved using + // `best_finalized_relay_block_at_target` + let available_parachain_block = env + .best_finalized_para_block_at_source(best_finalized_relay_block_at_target) + .await?; + let can_use_available_relay_header = available_parachain_block + .as_ref() + .map(|available_parachain_block| { + available_parachain_block.number() >= required_parachain_header + }) + .unwrap_or(false); + + // we don't require source node to be archive, so we can't craft storage proofs using + // ancient headers. So if the `best_finalized_relay_block_at_target` is too ancient, we + // can't craft storage proofs using it + let difference = best_finalized_relay_block_at_source + .number() + .saturating_sub(best_finalized_relay_block_at_target.number()); + let can_use_available_relay_header = + can_use_available_relay_header && difference < RBN::from(100u32); // TODO: extract const + + // ok - now we have everything ready to select which headers we need on the target chain + let (need_to_prove_relay_block, selected_relay_block, selected_parachain_block) = + if can_use_available_relay_header { + (false, best_finalized_relay_block_at_target, available_parachain_block.expect("TODO")) + } else { + (true, best_finalized_relay_block_at_source, best_possible_parachain_block) + }; + + Ok((need_to_prove_relay_block, selected_relay_block, selected_parachain_block)) +} + #[cfg(test)] mod tests { use super::*; @@ -808,4 +882,80 @@ mod tests { RelayState::RelayingRelayHeader(800), ); } + + // tuple is: + // + // - best_finalized_relay_block_at_source + // - best_finalized_relay_block_at_target + // - best_finalized_para_block_at_source at best_finalized_relay_block_at_source + // - best_finalized_para_block_at_source at best_finalized_relay_block_at_target + #[async_trait] + impl SelectHeadersToProveEnvironment for (u32, u32, u32, u32) { + fn parachain_id(&self) -> ParaId { + ParaId(0) + } + + async fn best_finalized_relay_block_at_source( + &self, + ) -> Result, SubstrateError> { + Ok(HeaderId(self.0, self.0)) + } + + async fn best_finalized_relay_block_at_target( + &self, + ) -> Result, SubstrateError> { + Ok(HeaderId(self.1, self.1)) + } + + async fn best_finalized_para_block_at_source( + &self, + at_relay_block: HeaderId, + ) -> Result>, SubstrateError> { + if at_relay_block.0 == self.0 { + Ok(Some(HeaderId(self.2, self.2))) + } else if at_relay_block.0 == self.1 { + Ok(Some(HeaderId(self.3, self.3))) + } else { + Ok(None) + } + } + } + + #[async_std::test] + async fn select_headers_to_prove_returns_err_if_required_para_block_is_missing_at_source() { + assert!(matches!( + select_headers_to_prove((20_u32, 10_u32, 200_u32, 100_u32), 300_u32,).await, + Err(SubstrateError::MissingRequiredParachainHead(ParaId(0), 300_u64)), + )); + } + + #[async_std::test] + async fn select_headers_to_prove_fails_to_use_existing_ancient_relay_block() { + assert_eq!( + select_headers_to_prove((220_u32, 10_u32, 200_u32, 100_u32), 100_u32,) + .await + .map_err(drop), + Ok((true, HeaderId(220, 220), HeaderId(200, 200))), + ); + } + + #[async_std::test] + async fn select_headers_to_prove_is_able_to_use_existing_recent_relay_block() { + assert_eq!( + select_headers_to_prove((40_u32, 10_u32, 200_u32, 100_u32), 100_u32,) + .await + .map_err(drop), + Ok((false, HeaderId(10, 10), HeaderId(100, 100))), + ); + } + + #[async_std::test] + async fn select_headers_to_prove_uses_new_relay_block() { + assert_eq!( + select_headers_to_prove((20_u32, 10_u32, 200_u32, 100_u32), 200_u32,) + .await + .map_err(drop), + Ok((true, HeaderId(20, 20), HeaderId(200, 200))), + ); + } } From c1dff4b14aaadfc1a2670e73d249da5171063532 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Wed, 14 Dec 2022 12:20:05 +0300 Subject: [PATCH 20/29] more traces and leave TODOs --- .../src/finality/source.rs | 3 +++ .../src/on_demand/headers.rs | 10 ++++++++ .../src/on_demand/parachains.rs | 23 +++++++++++++++++++ 3 files changed, 36 insertions(+) diff --git a/relays/lib-substrate-relay/src/finality/source.rs b/relays/lib-substrate-relay/src/finality/source.rs index fa81822419..2c20f2d8a6 100644 --- a/relays/lib-substrate-relay/src/finality/source.rs +++ b/relays/lib-substrate-relay/src/finality/source.rs @@ -81,6 +81,9 @@ impl SubstrateFinalitySource

{ (relay_substrate_client::SyncHeader>, SubstrateFinalityProof

), Error, > { + // TODO: this is wrong - only mandatory headers have persistent justifications and they are + // rare => we need to find another way + loop { let (header, maybe_justification) = self.header_and_finality_proof(block_number).await?; diff --git a/relays/lib-substrate-relay/src/on_demand/headers.rs b/relays/lib-substrate-relay/src/on_demand/headers.rs index 2e5c4ca143..369520f0e9 100644 --- a/relays/lib-substrate-relay/src/on_demand/headers.rs +++ b/relays/lib-substrate-relay/src/on_demand/headers.rs @@ -123,6 +123,16 @@ impl OnDemandRelay so we need to call + // `on_demand_source_relay_to_target_headers.prove_header()` and then prove parachain head + // at thisreturned header, not the `selected_relay_block` + // + // i.e. `selected_parachain_block` could change even after `select_headers_to_prove` call + // now let's prove relay chain block (if needed) let mut calls = Vec::new(); if need_to_prove_relay_block { From 2d8b5006adc14ce2084276719467fab86af0ff72 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Wed, 14 Dec 2022 12:49:19 +0300 Subject: [PATCH 21/29] use finality stream in SubstrateFinalitySource::prove_block_finality --- relays/client-substrate/src/error.rs | 3 ++ .../src/finality/source.rs | 49 ++++++++++++++----- 2 files changed, 41 insertions(+), 11 deletions(-) diff --git a/relays/client-substrate/src/error.rs b/relays/client-substrate/src/error.rs index b0fd659c7a..56add8d29b 100644 --- a/relays/client-substrate/src/error.rs +++ b/relays/client-substrate/src/error.rs @@ -49,6 +49,9 @@ pub enum Error { /// Required parachain head is not present at the relay chain. #[error("Parachain {0:?} head {1} is missing from the relay chain storage.")] MissingRequiredParachainHead(ParaId, u64), + /// Failed to find finality proof for the given header. + #[error("Failed to find finality proof for header {0}.")] + FailedToFindFinalityProof(u64), /// The client we're connected to is not synced, so we can't rely on its state. #[error("Substrate client is not synced {0}.")] ClientNotSynced(Health), diff --git a/relays/lib-substrate-relay/src/finality/source.rs b/relays/lib-substrate-relay/src/finality/source.rs index 2c20f2d8a6..f02d08be77 100644 --- a/relays/lib-substrate-relay/src/finality/source.rs +++ b/relays/lib-substrate-relay/src/finality/source.rs @@ -20,6 +20,7 @@ use crate::finality::{engine::Engine, FinalitySyncPipelineAdapter, SubstrateFina use async_std::sync::{Arc, Mutex}; use async_trait::async_trait; +use bp_header_chain::FinalityProof; use codec::Decode; use finality_relay::SourceClient; use futures::stream::{unfold, Stream, StreamExt}; @@ -27,7 +28,7 @@ use num_traits::One; use relay_substrate_client::{ BlockNumberOf, BlockWithJustification, Chain, Client, Error, HeaderOf, }; -use relay_utils::relay_loop::Client as RelayClient; +use relay_utils::{relay_loop::Client as RelayClient, UniqueSaturatedInto}; use std::pin::Pin; /// Shared updatable reference to the maximal header number that we want to sync from the source. @@ -74,26 +75,52 @@ impl SubstrateFinalitySource

{ /// Return header and its justification of the given block or its earlier descendant that /// has a GRANDPA justification. + /// + /// This method is optimized for cases when `block_number` is close to the best finalized + /// chain block. pub async fn prove_block_finality( &self, - mut block_number: BlockNumberOf, + block_number: BlockNumberOf, ) -> Result< (relay_substrate_client::SyncHeader>, SubstrateFinalityProof

), Error, > { - // TODO: this is wrong - only mandatory headers have persistent justifications and they are - // rare => we need to find another way - - loop { - let (header, maybe_justification) = - self.header_and_finality_proof(block_number).await?; - match maybe_justification { - Some(justification) => return Ok((header, justification)), + // when we talk about GRANDPA finality: + // + // only mandatory headers have persistent notifications (that are returned by the + // `header_and_finality_proof` call). Since this method is supposed to work with arbitrary + // headers, we can't rely only on persistent justifications. So let's start with subscribing + // to ephemeral justifications to avoid waiting too much if we have failed to find + // persistent one. + let best_finalized_block_number = self.client.best_finalized_header_number().await?; + let mut finality_proofs = self.finality_proofs().await?; + + // start searching for persistent justificaitons + let mut current_block_number = block_number; + while current_block_number <= best_finalized_block_number { + let (header, maybe_proof) = + self.header_and_finality_proof(current_block_number).await?; + match maybe_proof { + Some(proof) => return Ok((header, proof)), None => { - block_number += One::one(); + current_block_number += One::one(); }, } } + + // we have failed to find persistent justification, so let's try with ephemeral + while let Some(proof) = finality_proofs.next().await { + // this is just for safety, in practice we shall never get notifications for earlier + // headers here (if `block_number <= best_finalized_block_number` of course) + if proof.target_header_number() < block_number { + continue + } + + let header = self.client.header_by_number(proof.target_header_number()).await?; + return Ok((header.into(), proof)) + } + + Err(Error::FailedToFindFinalityProof(block_number.unique_saturated_into())) } } From fad8069f4409c527cf8294bb15f0c6118f9bc4e8 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Wed, 14 Dec 2022 13:50:07 +0300 Subject: [PATCH 22/29] prove parachain head at block, selected by headers relay --- .../src/on_demand/parachains.rs | 62 +++++++++++++------ 1 file changed, 43 insertions(+), 19 deletions(-) diff --git a/relays/lib-substrate-relay/src/on_demand/parachains.rs b/relays/lib-substrate-relay/src/on_demand/parachains.rs index 78cd8ec7f8..3a9768eb46 100644 --- a/relays/lib-substrate-relay/src/on_demand/parachains.rs +++ b/relays/lib-substrate-relay/src/on_demand/parachains.rs @@ -45,7 +45,7 @@ use relay_substrate_client::{ }; use relay_utils::{ metrics::MetricsParams, relay_loop::Client as RelayClient, BlockNumberBase, FailedClient, - HeaderId, + HeaderId, UniqueSaturatedInto, }; use std::fmt::Debug; @@ -164,36 +164,60 @@ where }, ); - // TODO: that is wrong to assume that the `on_demand_source_relay_to_target_headers` will - // prove `selected_relay_block` => so we need to call - // `on_demand_source_relay_to_target_headers.prove_header()` and then prove parachain head - // at thisreturned header, not the `selected_relay_block` - // - // i.e. `selected_parachain_block` could change even after `select_headers_to_prove` call - // now let's prove relay chain block (if needed) let mut calls = Vec::new(); + let mut proved_relay_block = selected_relay_block; if need_to_prove_relay_block { - calls.extend( - self.on_demand_source_relay_to_target_headers - .prove_header(selected_relay_block.number()) - .await? - .1, + let (relay_block, relay_prove_call) = self + .on_demand_source_relay_to_target_headers + .prove_header(selected_relay_block.number()) + .await?; + proved_relay_block = relay_block; + calls.extend(relay_prove_call); + } + + // despite what we've selected before (in `select_headers_to_prove` call), if headers relay + // have chose the different header (e.g. because there's no GRANDPA jusstification for it), + // we need to prove parachain head available at this header + let para_id = ParaId(P::SOURCE_PARACHAIN_PARA_ID); + let mut proved_parachain_block = selected_parachain_block; + if proved_relay_block != selected_relay_block { + proved_parachain_block = parachains_source + .on_chain_para_head_id(proved_relay_block, para_id) + .await? + // this could happen e.g. if parachain has been offboarded? + .ok_or_else(|| { + SubstrateError::MissingRequiredParachainHead( + para_id, + proved_relay_block.number().unique_saturated_into(), + ) + })?; + + log::debug!( + target: "bridge", + "[{}] Selected to prove {} head {:?} and {} head {:?}. Instead proved {} head {:?} and {} head {:?}", + self.relay_task_name, + P::SourceParachain::NAME, + selected_parachain_block, + P::SourceRelayChain::NAME, + selected_relay_block, + P::SourceParachain::NAME, + proved_parachain_block, + P::SourceRelayChain::NAME, + proved_relay_block, ); } // and finally - prove parachain head - let para_id = ParaId(P::SOURCE_PARACHAIN_PARA_ID); - let (para_proof, para_hashes) = parachains_source - .prove_parachain_heads(selected_relay_block, &[para_id]) - .await?; + let (para_proof, para_hashes) = + parachains_source.prove_parachain_heads(proved_relay_block, &[para_id]).await?; calls.push(P::SubmitParachainHeadsCallBuilder::build_submit_parachain_heads_call( - selected_relay_block, + proved_relay_block, para_hashes.into_iter().map(|h| (para_id, h)).collect(), para_proof, )); - Ok((selected_parachain_block, calls)) + Ok((proved_parachain_block, calls)) } } From 5053b1630004db391fd1b62184d5d9730bc67e36 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Wed, 14 Dec 2022 16:25:22 +0300 Subject: [PATCH 23/29] const ANCIENT_BLOCK_THRESHOLD --- relays/client-substrate/src/client.rs | 12 ++++++++++++ relays/client-substrate/src/lib.rs | 5 ++++- .../lib-substrate-relay/src/on_demand/parachains.rs | 4 ++-- 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/relays/client-substrate/src/client.rs b/relays/client-substrate/src/client.rs index 4f783291ee..07293a7e65 100644 --- a/relays/client-substrate/src/client.rs +++ b/relays/client-substrate/src/client.rs @@ -57,6 +57,18 @@ const SUB_API_GRANDPA_AUTHORITIES: &str = "GrandpaApi_grandpa_authorities"; const SUB_API_TXPOOL_VALIDATE_TRANSACTION: &str = "TaggedTransactionQueue_validate_transaction"; const MAX_SUBSCRIPTION_CAPACITY: usize = 4096; +/// The difference between best block number and number of its ancestor, that is enough +/// for us to consider that ancestor an "ancient" block with dropped state. +/// +/// The relay does not assume that it is connected to the archive node, so it always tries +/// to use the best available chain state. But sometimes it still may use state of some +/// old block. If the state of that block is already dropped, relay will see errors when +/// e.g. it tries to prove something. +/// +/// By default Substrate-based nodes are storing state for last 256 blocks. We'll use +/// half of this value. +pub const ANCIENT_BLOCK_THRESHOLD: u32 = 128; + /// Opaque justifications subscription type. pub struct Subscription(pub(crate) Mutex>>); diff --git a/relays/client-substrate/src/lib.rs b/relays/client-substrate/src/lib.rs index a5e73d78b4..4bc38e3c35 100644 --- a/relays/client-substrate/src/lib.rs +++ b/relays/client-substrate/src/lib.rs @@ -37,7 +37,10 @@ pub use crate::{ ChainWithGrandpa, ChainWithMessages, ChainWithTransactions, RelayChain, SignParam, TransactionStatusOf, UnsignedTransaction, WeightToFeeOf, }, - client::{ChainRuntimeVersion, Client, OpaqueGrandpaAuthoritiesSet, Subscription}, + client::{ + ChainRuntimeVersion, Client, OpaqueGrandpaAuthoritiesSet, Subscription, + ANCIENT_BLOCK_THRESHOLD, + }, error::{Error, Result}, rpc::{SubstrateBeefyFinalityClient, SubstrateFinalityClient, SubstrateGrandpaFinalityClient}, sync_header::SyncHeader, diff --git a/relays/lib-substrate-relay/src/on_demand/parachains.rs b/relays/lib-substrate-relay/src/on_demand/parachains.rs index 3a9768eb46..00f99d723b 100644 --- a/relays/lib-substrate-relay/src/on_demand/parachains.rs +++ b/relays/lib-substrate-relay/src/on_demand/parachains.rs @@ -41,7 +41,7 @@ use parachains_relay::parachains_loop::{ }; use relay_substrate_client::{ AccountIdOf, AccountKeyPairOf, BlockNumberOf, CallOf, Chain, Client, Error as SubstrateError, - HashOf, HeaderIdOf, + HashOf, HeaderIdOf, ANCIENT_BLOCK_THRESHOLD, }; use relay_utils::{ metrics::MetricsParams, relay_loop::Client as RelayClient, BlockNumberBase, FailedClient, @@ -698,7 +698,7 @@ where .number() .saturating_sub(best_finalized_relay_block_at_target.number()); let can_use_available_relay_header = - can_use_available_relay_header && difference < RBN::from(100u32); // TODO: extract const + can_use_available_relay_header && difference < RBN::from(ANCIENT_BLOCK_THRESHOLD); // ok - now we have everything ready to select which headers we need on the target chain let (need_to_prove_relay_block, selected_relay_block, selected_parachain_block) = From b5f2623c171e2918cf2df9c74a86ce060e20ad7c Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Wed, 14 Dec 2022 16:26:19 +0300 Subject: [PATCH 24/29] TODO -> proof --- relays/lib-substrate-relay/src/on_demand/parachains.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/relays/lib-substrate-relay/src/on_demand/parachains.rs b/relays/lib-substrate-relay/src/on_demand/parachains.rs index 00f99d723b..43e24c1364 100644 --- a/relays/lib-substrate-relay/src/on_demand/parachains.rs +++ b/relays/lib-substrate-relay/src/on_demand/parachains.rs @@ -703,7 +703,15 @@ where // ok - now we have everything ready to select which headers we need on the target chain let (need_to_prove_relay_block, selected_relay_block, selected_parachain_block) = if can_use_available_relay_header { - (false, best_finalized_relay_block_at_target, available_parachain_block.expect("TODO")) + ( + false, + best_finalized_relay_block_at_target, + available_parachain_block.expect( + "can_use_available_relay_header is true;\ + can_use_available_relay_header is only true when available_parachain_block is Some;\ + qed", + ), + ) } else { (true, best_finalized_relay_block_at_source, best_possible_parachain_block) }; From aec2063d7c55d442ab2863dc3e39fc2474700bf2 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Wed, 14 Dec 2022 16:27:16 +0300 Subject: [PATCH 25/29] clippy and spelling --- relays/lib-substrate-relay/src/messages_source.rs | 2 +- relays/lib-substrate-relay/src/messages_target.rs | 2 +- relays/lib-substrate-relay/src/on_demand/parachains.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/relays/lib-substrate-relay/src/messages_source.rs b/relays/lib-substrate-relay/src/messages_source.rs index dc05723783..de2aba7bdc 100644 --- a/relays/lib-substrate-relay/src/messages_source.rs +++ b/relays/lib-substrate-relay/src/messages_source.rs @@ -411,7 +411,7 @@ where AccountIdOf: From< as Pair>::Public>, { fn required_header_id(&self) -> TargetHeaderIdOf> { - self.proved_header.clone() + self.proved_header } async fn append_proof_and_send( diff --git a/relays/lib-substrate-relay/src/messages_target.rs b/relays/lib-substrate-relay/src/messages_target.rs index c92ad107b3..95e219f961 100644 --- a/relays/lib-substrate-relay/src/messages_target.rs +++ b/relays/lib-substrate-relay/src/messages_target.rs @@ -318,7 +318,7 @@ where AccountIdOf: From< as Pair>::Public>, { fn required_header_id(&self) -> SourceHeaderIdOf> { - self.proved_header.clone() + self.proved_header } async fn append_proof_and_send( diff --git a/relays/lib-substrate-relay/src/on_demand/parachains.rs b/relays/lib-substrate-relay/src/on_demand/parachains.rs index 43e24c1364..7a4a8cb97d 100644 --- a/relays/lib-substrate-relay/src/on_demand/parachains.rs +++ b/relays/lib-substrate-relay/src/on_demand/parachains.rs @@ -585,7 +585,7 @@ where RelayState::RelayingParaHeader(para_header_at_source) } -/// Envirnonment for the `select_headers_to_prove` call. +/// Environment for the `select_headers_to_prove` call. #[async_trait] trait SelectHeadersToProveEnvironment { /// Returns associated parachain id. From 0dc01ab24cb0299795cf11c6af13d7ff66fcf5c5 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Thu, 15 Dec 2022 11:23:55 +0300 Subject: [PATCH 26/29] BatchCallBuilder::build_batch_call() returns Result --- relays/lib-substrate-relay/src/lib.rs | 22 +++++++++++++------ .../lib-substrate-relay/src/messages_lane.rs | 6 ++--- .../src/messages_source.rs | 2 +- .../src/messages_target.rs | 2 +- 4 files changed, 20 insertions(+), 12 deletions(-) diff --git a/relays/lib-substrate-relay/src/lib.rs b/relays/lib-substrate-relay/src/lib.rs index c1a13cc72e..2181f09358 100644 --- a/relays/lib-substrate-relay/src/lib.rs +++ b/relays/lib-substrate-relay/src/lib.rs @@ -18,6 +18,7 @@ #![warn(missing_docs)] +use relay_substrate_client::Error as SubstrateError; use std::marker::PhantomData; pub mod error; @@ -101,22 +102,28 @@ impl TaggedAccount { /// Batch call builder. pub trait BatchCallBuilder { + /// Associated error type. + type Error; /// If `true`, then batch calls are supported at the chain. const BATCH_CALL_SUPPORTED: bool; /// Create batch call from given calls vector. - fn build_batch_call(_calls: Vec) -> Call; + fn build_batch_call(_calls: Vec) -> Result; } impl BatchCallBuilder for () { + type Error = SubstrateError; const BATCH_CALL_SUPPORTED: bool = false; - fn build_batch_call(_calls: Vec) -> Call { - unreachable!( + fn build_batch_call(_calls: Vec) -> Result { + debug_assert!( + false, "only called if `BATCH_CALL_SUPPORTED` is true;\ `BATCH_CALL_SUPPORTED` is false;\ qed" - ) + ); + + Err(SubstrateError::Custom("<() as BatchCallBuilder>::build_batch_call() is called".into())) } } @@ -128,15 +135,16 @@ where R: pallet_utility::Config::RuntimeCall>, ::RuntimeCall: From>, { + type Error = SubstrateError; const BATCH_CALL_SUPPORTED: bool = true; fn build_batch_call( mut calls: Vec<::RuntimeCall>, - ) -> ::RuntimeCall { - if calls.len() == 1 { + ) -> Result<::RuntimeCall, SubstrateError> { + Ok(if calls.len() == 1 { calls.remove(0) } else { pallet_utility::Call::batch_all { calls }.into() - } + }) } } diff --git a/relays/lib-substrate-relay/src/messages_lane.rs b/relays/lib-substrate-relay/src/messages_lane.rs index e3da85b3ab..8ec78d0ae2 100644 --- a/relays/lib-substrate-relay/src/messages_lane.rs +++ b/relays/lib-substrate-relay/src/messages_lane.rs @@ -35,7 +35,7 @@ use messages_relay::message_lane::MessageLane; use pallet_bridge_messages::{Call as BridgeMessagesCall, Config as BridgeMessagesConfig}; use relay_substrate_client::{ transaction_stall_timeout, AccountKeyPairOf, BalanceOf, BlockNumberOf, CallOf, Chain, - ChainWithMessages, ChainWithTransactions, Client, HashOf, + ChainWithMessages, ChainWithTransactions, Client, Error as SubstrateError, HashOf, }; use relay_utils::{ metrics::{GlobalMetrics, MetricsParams, StandaloneMetric}, @@ -57,9 +57,9 @@ pub trait SubstrateMessageLane: 'static + Clone + Debug + Send + Sync { type ReceiveMessagesDeliveryProofCallBuilder: ReceiveMessagesDeliveryProofCallBuilder; /// How batch calls are built at the source chain? - type SourceBatchCallBuilder: BatchCallBuilder>; + type SourceBatchCallBuilder: BatchCallBuilder, Error = SubstrateError>; /// How batch calls are built at the target chain? - type TargetBatchCallBuilder: BatchCallBuilder>; + type TargetBatchCallBuilder: BatchCallBuilder, Error = SubstrateError>; } /// Adapter that allows all `SubstrateMessageLane` to act as `MessageLane`. diff --git a/relays/lib-substrate-relay/src/messages_source.rs b/relays/lib-substrate-relay/src/messages_source.rs index de2aba7bdc..e2876b3107 100644 --- a/relays/lib-substrate-relay/src/messages_source.rs +++ b/relays/lib-substrate-relay/src/messages_source.rs @@ -424,7 +424,7 @@ where proof, false, ), ); - let batch_call = P::SourceBatchCallBuilder::build_batch_call(calls); + let batch_call = P::SourceBatchCallBuilder::build_batch_call(calls)?; let (spec_version, transaction_version) = self.messages_source.source_client.simple_runtime_version().await?; diff --git a/relays/lib-substrate-relay/src/messages_target.rs b/relays/lib-substrate-relay/src/messages_target.rs index 95e219f961..9d80b9166c 100644 --- a/relays/lib-substrate-relay/src/messages_target.rs +++ b/relays/lib-substrate-relay/src/messages_target.rs @@ -332,7 +332,7 @@ where proof, false, )); - let batch_call = P::TargetBatchCallBuilder::build_batch_call(calls); + let batch_call = P::TargetBatchCallBuilder::build_batch_call(calls)?; let (spec_version, transaction_version) = self.messages_target.target_client.simple_runtime_version().await?; From d1254fcda6ed1bc3855d9b35801bc30ac39d6738 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Thu, 15 Dec 2022 14:51:12 +0300 Subject: [PATCH 27/29] read first proof from two streams --- .../src/finality/source.rs | 155 +++++++++++++----- 1 file changed, 113 insertions(+), 42 deletions(-) diff --git a/relays/lib-substrate-relay/src/finality/source.rs b/relays/lib-substrate-relay/src/finality/source.rs index f02d08be77..a3431109b8 100644 --- a/relays/lib-substrate-relay/src/finality/source.rs +++ b/relays/lib-substrate-relay/src/finality/source.rs @@ -23,7 +23,10 @@ use async_trait::async_trait; use bp_header_chain::FinalityProof; use codec::Decode; use finality_relay::SourceClient; -use futures::stream::{unfold, Stream, StreamExt}; +use futures::{ + select, + stream::{try_unfold, unfold, Stream, StreamExt, TryStreamExt}, +}; use num_traits::One; use relay_substrate_client::{ BlockNumberOf, BlockWithJustification, Chain, Client, Error, HeaderOf, @@ -73,7 +76,7 @@ impl SubstrateFinalitySource

{ self.client.best_finalized_header_number().await } - /// Return header and its justification of the given block or its earlier descendant that + /// Return header and its justification of the given block or its descendant that /// has a GRANDPA justification. /// /// This method is optimized for cases when `block_number` is close to the best finalized @@ -85,42 +88,97 @@ impl SubstrateFinalitySource

{ (relay_substrate_client::SyncHeader>, SubstrateFinalityProof

), Error, > { - // when we talk about GRANDPA finality: - // - // only mandatory headers have persistent notifications (that are returned by the - // `header_and_finality_proof` call). Since this method is supposed to work with arbitrary - // headers, we can't rely only on persistent justifications. So let's start with subscribing - // to ephemeral justifications to avoid waiting too much if we have failed to find - // persistent one. - let best_finalized_block_number = self.client.best_finalized_header_number().await?; - let mut finality_proofs = self.finality_proofs().await?; + // first, subscribe to proofs + let next_persistent_proof = + self.persistent_proofs_stream(block_number + One::one()).await?.fuse(); + let next_ephemeral_proof = self.ephemeral_proofs_stream(block_number).await?.fuse(); - // start searching for persistent justificaitons - let mut current_block_number = block_number; - while current_block_number <= best_finalized_block_number { - let (header, maybe_proof) = - self.header_and_finality_proof(current_block_number).await?; - match maybe_proof { - Some(proof) => return Ok((header, proof)), - None => { - current_block_number += One::one(); + // in perfect world we'll need to return justfication for the requested `block_number` + let (header, maybe_proof) = self.header_and_finality_proof(block_number).await?; + if let Some(proof) = maybe_proof { + return Ok((header, proof)) + } + + // otherwise we don't care which header to return, so let's select first + futures::pin_mut!(next_persistent_proof, next_ephemeral_proof); + loop { + select! { + maybe_header_and_proof = next_persistent_proof.next() => match maybe_header_and_proof { + Some(header_and_proof) => return header_and_proof, + None => continue, + }, + maybe_header_and_proof = next_ephemeral_proof.next() => match maybe_header_and_proof { + Some(header_and_proof) => return header_and_proof, + None => continue, }, + complete => return Err(Error::FailedToFindFinalityProof(block_number.unique_saturated_into())) } } + } - // we have failed to find persistent justification, so let's try with ephemeral - while let Some(proof) = finality_proofs.next().await { - // this is just for safety, in practice we shall never get notifications for earlier - // headers here (if `block_number <= best_finalized_block_number` of course) - if proof.target_header_number() < block_number { - continue + /// Returns stream of headers and their persistent proofs, starting from given block. + async fn persistent_proofs_stream( + &self, + block_number: BlockNumberOf, + ) -> Result< + impl Stream< + Item = Result< + ( + relay_substrate_client::SyncHeader>, + SubstrateFinalityProof

, + ), + Error, + >, + >, + Error, + > { + let client = self.client.clone(); + let best_finalized_block_number = self.client.best_finalized_header_number().await?; + Ok(try_unfold((client, block_number), move |(client, current_block_number)| async move { + // if we've passed the `best_finalized_block_number`, we no longer need persistent + // justifications + if current_block_number > best_finalized_block_number { + return Ok(None) } - let header = self.client.header_by_number(proof.target_header_number()).await?; - return Ok((header.into(), proof)) - } + let (header, maybe_proof) = + header_and_finality_proof::

(&client, current_block_number).await?; + let next_block_number = current_block_number + One::one(); + let next_state = (client, next_block_number); - Err(Error::FailedToFindFinalityProof(block_number.unique_saturated_into())) + Ok(Some((maybe_proof.map(|proof| (header, proof)), next_state))) + }) + .try_filter_map(|maybe_result| async { Ok(maybe_result) })) + } + + /// Returns stream of headers and their ephemeral proofs, starting from given block. + async fn ephemeral_proofs_stream( + &self, + block_number: BlockNumberOf, + ) -> Result< + impl Stream< + Item = Result< + ( + relay_substrate_client::SyncHeader>, + SubstrateFinalityProof

, + ), + Error, + >, + >, + Error, + > { + let client = self.client.clone(); + Ok(self.finality_proofs().await?.map(Ok).try_filter_map(move |proof| { + let client = client.clone(); + async move { + if proof.target_header_number() < block_number { + return Ok(None) + } + + let header = client.header_by_number(proof.target_header_number()).await?; + Ok(Some((header.into(), proof))) + } + })) } } @@ -171,18 +229,7 @@ impl SourceClient { - let header_hash = self.client.block_hash_by_number(number).await?; - let signed_block = self.client.get_block(Some(header_hash)).await?; - - let justification = signed_block - .justification(P::FinalityEngine::ID) - .map(|raw_justification| { - SubstrateFinalityProof::

::decode(&mut raw_justification.as_slice()) - }) - .transpose() - .map_err(Error::ResponseParseFailed)?; - - Ok((signed_block.header().into(), justification)) + header_and_finality_proof::

(&self.client, number).await } async fn finality_proofs(&self) -> Result { @@ -225,3 +272,27 @@ impl SourceClient( + client: &Client, + number: BlockNumberOf, +) -> Result< + ( + relay_substrate_client::SyncHeader>, + Option>, + ), + Error, +> { + let header_hash = client.block_hash_by_number(number).await?; + let signed_block = client.get_block(Some(header_hash)).await?; + + let justification = signed_block + .justification(P::FinalityEngine::ID) + .map(|raw_justification| { + SubstrateFinalityProof::

::decode(&mut raw_justification.as_slice()) + }) + .transpose() + .map_err(Error::ResponseParseFailed)?; + + Ok((signed_block.header().into(), justification)) +} From 78d5214760dd36219973236d93fe7ccc33775fa0 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Fri, 16 Dec 2022 13:26:02 +0300 Subject: [PATCH 28/29] FailedToFindFinalityProof -> FinalityProofNotFound --- relays/client-substrate/src/error.rs | 2 +- relays/lib-substrate-relay/src/finality/source.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/relays/client-substrate/src/error.rs b/relays/client-substrate/src/error.rs index 56add8d29b..ddea1819fb 100644 --- a/relays/client-substrate/src/error.rs +++ b/relays/client-substrate/src/error.rs @@ -51,7 +51,7 @@ pub enum Error { MissingRequiredParachainHead(ParaId, u64), /// Failed to find finality proof for the given header. #[error("Failed to find finality proof for header {0}.")] - FailedToFindFinalityProof(u64), + FinalityProofNotFound(u64), /// The client we're connected to is not synced, so we can't rely on its state. #[error("Substrate client is not synced {0}.")] ClientNotSynced(Health), diff --git a/relays/lib-substrate-relay/src/finality/source.rs b/relays/lib-substrate-relay/src/finality/source.rs index a3431109b8..c8f11d9946 100644 --- a/relays/lib-substrate-relay/src/finality/source.rs +++ b/relays/lib-substrate-relay/src/finality/source.rs @@ -111,7 +111,7 @@ impl SubstrateFinalitySource

{ Some(header_and_proof) => return header_and_proof, None => continue, }, - complete => return Err(Error::FailedToFindFinalityProof(block_number.unique_saturated_into())) + complete => return Err(Error::FinalityProofNotFound(block_number.unique_saturated_into())) } } } From 9da39c7a2e1181c7d962758df2c318a661a03708 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Fri, 16 Dec 2022 13:39:10 +0300 Subject: [PATCH 29/29] changed select_headers_to_prove to version from PR review --- .../src/on_demand/parachains.rs | 73 ++++++++----------- 1 file changed, 29 insertions(+), 44 deletions(-) diff --git a/relays/lib-substrate-relay/src/on_demand/parachains.rs b/relays/lib-substrate-relay/src/on_demand/parachains.rs index e4cabe5e9a..cfb9d9c7a9 100644 --- a/relays/lib-substrate-relay/src/on_demand/parachains.rs +++ b/relays/lib-substrate-relay/src/on_demand/parachains.rs @@ -670,56 +670,41 @@ where // headers) let best_possible_parachain_block = env .best_finalized_para_block_at_source(best_finalized_relay_block_at_source) - .await?; - let best_possible_parachain_block = match best_possible_parachain_block { - Some(best_possible_parachain_block) - if best_possible_parachain_block.number() >= required_parachain_header => - best_possible_parachain_block, - _ => - return Err(SubstrateError::MissingRequiredParachainHead( - env.parachain_id(), - required_parachain_header.unique_saturated_into(), - )), - }; + .await? + .filter(|best_possible_parachain_block| { + best_possible_parachain_block.number() >= required_parachain_header + }) + .ok_or(SubstrateError::MissingRequiredParachainHead( + env.parachain_id(), + required_parachain_header.unique_saturated_into(), + ))?; // now let's check if `required_header` may be proved using // `best_finalized_relay_block_at_target` - let available_parachain_block = env + let selection = env .best_finalized_para_block_at_source(best_finalized_relay_block_at_target) - .await?; - let can_use_available_relay_header = available_parachain_block - .as_ref() - .map(|available_parachain_block| { - available_parachain_block.number() >= required_parachain_header + .await? + .filter(|best_finalized_para_block_at_target| { + best_finalized_para_block_at_target.number() >= required_parachain_header }) - .unwrap_or(false); - - // we don't require source node to be archive, so we can't craft storage proofs using - // ancient headers. So if the `best_finalized_relay_block_at_target` is too ancient, we - // can't craft storage proofs using it - let difference = best_finalized_relay_block_at_source - .number() - .saturating_sub(best_finalized_relay_block_at_target.number()); - let can_use_available_relay_header = - can_use_available_relay_header && difference < RBN::from(ANCIENT_BLOCK_THRESHOLD); - - // ok - now we have everything ready to select which headers we need on the target chain - let (need_to_prove_relay_block, selected_relay_block, selected_parachain_block) = - if can_use_available_relay_header { - ( - false, - best_finalized_relay_block_at_target, - available_parachain_block.expect( - "can_use_available_relay_header is true;\ - can_use_available_relay_header is only true when available_parachain_block is Some;\ - qed", - ), - ) - } else { - (true, best_finalized_relay_block_at_source, best_possible_parachain_block) - }; + .map(|best_finalized_para_block_at_target| { + (false, best_finalized_relay_block_at_target, best_finalized_para_block_at_target) + }) + // we don't require source node to be archive, so we can't craft storage proofs using + // ancient headers. So if the `best_finalized_relay_block_at_target` is too ancient, we + // can't craft storage proofs using it + .filter(|(_, selected_relay_block, _)| { + let difference = best_finalized_relay_block_at_source + .number() + .saturating_sub(selected_relay_block.number()); + difference <= RBN::from(ANCIENT_BLOCK_THRESHOLD) + }); - Ok((need_to_prove_relay_block, selected_relay_block, selected_parachain_block)) + Ok(selection.unwrap_or(( + true, + best_finalized_relay_block_at_source, + best_possible_parachain_block, + ))) } #[cfg(test)]