Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Commit

Permalink
Squashed 'bridges/' changes from 5b5627e90..3c15c3645
Browse files Browse the repository at this point in the history
3c15c3645 get rid of ChainWithMessages::WeightInfo, because we can't have exact weights for "external chains" (#1899)
8ccaa0213 Wrap confirmation and finality transactions into batch_all in Millau -> RialtoParachain bridge (#1898)
9b7285edb Weight+size limits for bridge GRANDPA pallet calls (#1882)

git-subtree-dir: bridges
git-subtree-split: 3c15c36455f2ad944df6a492a8d82f7e0aaf7e9f
  • Loading branch information
bkontur committed Feb 22, 2023
1 parent 1afb13d commit 0fda54d
Show file tree
Hide file tree
Showing 21 changed files with 670 additions and 192 deletions.
98 changes: 89 additions & 9 deletions bin/runtime-common/src/refund_relayer_extension.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,12 @@ use codec::{Decode, Encode};
use frame_support::{
dispatch::{CallableCallFor, DispatchInfo, Dispatchable, PostDispatchInfo},
traits::IsSubType,
weights::Weight,
CloneNoBound, DefaultNoBound, EqNoBound, PartialEqNoBound, RuntimeDebugNoBound,
};
use pallet_bridge_grandpa::{CallSubType as GrandpaCallSubType, SubmitFinalityProofHelper};
use pallet_bridge_grandpa::{
CallSubType as GrandpaCallSubType, SubmitFinalityProofHelper, SubmitFinalityProofInfo,
};
use pallet_bridge_messages::Config as MessagesConfig;
use pallet_bridge_parachains::{
BoundedBridgeGrandpaConfig, CallSubType as ParachainsCallSubType, Config as ParachainsConfig,
Expand Down Expand Up @@ -140,7 +143,11 @@ pub struct PreDispatchData<AccountId> {
#[derive(RuntimeDebugNoBound, PartialEq)]
pub enum CallInfo {
/// Relay chain finality + parachain finality + message delivery calls.
AllFinalityAndDelivery(RelayBlockNumber, SubmitParachainHeadsInfo, ReceiveMessagesProofInfo),
AllFinalityAndDelivery(
SubmitFinalityProofInfo<RelayBlockNumber>,
SubmitParachainHeadsInfo,
ReceiveMessagesProofInfo,
),
/// Parachain finality + message delivery calls.
ParachainFinalityAndDelivery(SubmitParachainHeadsInfo, ReceiveMessagesProofInfo),
/// Standalone message delivery call.
Expand All @@ -149,7 +156,7 @@ pub enum CallInfo {

impl CallInfo {
/// Returns the pre-dispatch `finality_target` sent to the `SubmitFinalityProof` call.
fn submit_finality_proof_info(&self) -> Option<RelayBlockNumber> {
fn submit_finality_proof_info(&self) -> Option<SubmitFinalityProofInfo<RelayBlockNumber>> {
match *self {
Self::AllFinalityAndDelivery(info, _, _) => Some(info),
_ => None,
Expand Down Expand Up @@ -318,6 +325,9 @@ where
len: usize,
result: &DispatchResult,
) -> Result<(), TransactionValidityError> {
let mut extra_weight = Weight::zero();
let mut extra_size = 0;

// We don't refund anything if the transaction has failed.
if result.is_err() {
return Ok(())
Expand All @@ -330,8 +340,10 @@ where
};

// check if relay chain state has been updated
if let Some(relay_block_number) = call_info.submit_finality_proof_info() {
if !SubmitFinalityProofHelper::<Runtime, Runtime::BridgesGrandpaPalletInstance>::was_successful(relay_block_number) {
if let Some(finality_proof_info) = call_info.submit_finality_proof_info() {
if !SubmitFinalityProofHelper::<Runtime, Runtime::BridgesGrandpaPalletInstance>::was_successful(
finality_proof_info.block_number,
) {
// we only refund relayer if all calls have updated chain state
return Ok(())
}
Expand All @@ -342,6 +354,11 @@ where
// `utility.batchAll` transaction always requires payment. But in both cases we'll
// refund relayer - either explicitly here, or using `Pays::No` if he's choosing
// to submit dedicated transaction.

// submitter has means to include extra weight/bytes in the `submit_finality_proof`
// call, so let's subtract extra weight/size to avoid refunding for this extra stuff
extra_weight = finality_proof_info.extra_weight;
extra_size = finality_proof_info.extra_size;
}

// check if parachain state has been updated
Expand Down Expand Up @@ -370,8 +387,15 @@ where
// cost of this attack is nothing. Hence we use zero as tip here.
let tip = Zero::zero();

// decrease post-dispatch weight/size using extra weight/size that we know now
let post_info_len = len.saturating_sub(extra_size as usize);
let mut post_info = *post_info;
post_info.actual_weight =
Some(post_info.actual_weight.unwrap_or(info.weight).saturating_sub(extra_weight));

// compute the relayer refund
let refund = Refund::compute_refund(info, post_info, len, tip);
let refund = Refund::compute_refund(info, &post_info, post_info_len, tip);

// finally - register refund in relayers pallet
RelayersPallet::<Runtime>::register_relayer_reward(Msgs::Id::get(), &relayer, refund);

Expand All @@ -397,9 +421,9 @@ mod tests {
use bp_parachains::{BestParaHeadHash, ParaInfo};
use bp_polkadot_core::parachains::{ParaHash, ParaHeadsProof, ParaId};
use bp_runtime::HeaderId;
use bp_test_utils::make_default_justification;
use bp_test_utils::{make_default_justification, test_keyring};
use frame_support::{assert_storage_noop, parameter_types, weights::Weight};
use pallet_bridge_grandpa::Call as GrandpaCall;
use pallet_bridge_grandpa::{Call as GrandpaCall, StoredAuthoritySet};
use pallet_bridge_messages::Call as MessagesCall;
use pallet_bridge_parachains::{Call as ParachainsCall, RelayBlockHash};
use sp_runtime::{
Expand Down Expand Up @@ -434,7 +458,11 @@ mod tests {
parachain_head_hash: ParaHash,
best_delivered_message: MessageNonce,
) {
let authorities = test_keyring().into_iter().map(|(a, w)| (a.into(), w)).collect();
let best_relay_header = HeaderId(best_relay_header_number, RelayBlockHash::default());
pallet_bridge_grandpa::CurrentAuthoritySet::<TestRuntime>::put(
StoredAuthoritySet::try_new(authorities, 0).unwrap(),
);
pallet_bridge_grandpa::BestFinalized::<TestRuntime>::put(best_relay_header);

let para_id = ParaId(TestParachain::get());
Expand Down Expand Up @@ -524,7 +552,11 @@ mod tests {
PreDispatchData {
relayer: relayer_account_at_this_chain(),
call_info: CallInfo::AllFinalityAndDelivery(
200,
SubmitFinalityProofInfo {
block_number: 200,
extra_weight: Weight::zero(),
extra_size: 0,
},
SubmitParachainHeadsInfo {
at_relay_block_number: 200,
para_id: ParaId(TestParachain::get()),
Expand Down Expand Up @@ -823,6 +855,54 @@ mod tests {
});
}

#[test]
fn post_dispatch_refunds_relayer_in_all_finality_batch_with_extra_weight() {
run_test(|| {
initialize_environment(200, 200, [1u8; 32].into(), 200);

let mut dispatch_info = dispatch_info();
dispatch_info.weight = Weight::from_ref_time(
frame_support::weights::constants::WEIGHT_REF_TIME_PER_SECOND * 2,
);

// without any size/weight refund: we expect regular reward
let pre_dispatch_data = all_finality_pre_dispatch_data();
let regular_reward = expected_reward();
run_post_dispatch(Some(pre_dispatch_data), Ok(()));
assert_eq!(
RelayersPallet::<TestRuntime>::relayer_reward(
relayer_account_at_this_chain(),
TestLaneId::get()
),
Some(regular_reward),
);

// now repeat the same with size+weight refund: we expect smaller reward
let mut pre_dispatch_data = all_finality_pre_dispatch_data();
match pre_dispatch_data.call_info {
CallInfo::AllFinalityAndDelivery(ref mut info, ..) => {
info.extra_weight.set_ref_time(
frame_support::weights::constants::WEIGHT_REF_TIME_PER_SECOND,
);
info.extra_size = 32;
},
_ => unreachable!(),
}
run_post_dispatch(Some(pre_dispatch_data), Ok(()));
let reward_after_two_calls = RelayersPallet::<TestRuntime>::relayer_reward(
relayer_account_at_this_chain(),
TestLaneId::get(),
)
.unwrap();
assert!(
reward_after_two_calls < 2 * regular_reward,
"{} must be < 2 * {}",
reward_after_two_calls,
2 * regular_reward,
);
});
}

#[test]
fn post_dispatch_refunds_relayer_in_all_finality_batch() {
run_test(|| {
Expand Down
172 changes: 160 additions & 12 deletions modules/grandpa/src/call_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,46 @@
// You should have received a copy of the GNU General Public License
// along with Parity Bridges Common. If not, see <http://www.gnu.org/licenses/>.

use crate::{Config, Error, Pallet};
use crate::{weights::WeightInfo, BridgedBlockNumber, BridgedHeader, Config, Error, Pallet};
use bp_header_chain::{justification::GrandpaJustification, ChainWithGrandpa};
use bp_runtime::BlockNumberOf;
use frame_support::{dispatch::CallableCallFor, traits::IsSubType};
use codec::Encode;
use frame_support::{dispatch::CallableCallFor, traits::IsSubType, weights::Weight, RuntimeDebug};
use sp_runtime::{
traits::Header,
traits::{Header, Zero},
transaction_validity::{InvalidTransaction, TransactionValidity, ValidTransaction},
SaturatedConversion,
};

/// Info about a `SubmitParachainHeads` call which tries to update a single parachain.
#[derive(Copy, Clone, PartialEq, RuntimeDebug)]
pub struct SubmitFinalityProofInfo<N> {
/// Number of the finality target.
pub block_number: N,
/// Extra weight that we assume is included in the call.
///
/// We have some assumptions about headers and justifications of the bridged chain.
/// We know that if our assumptions are correct, then the call must not have the
/// weight above some limit. The fee paid for weight above that limit, is never refunded.
pub extra_weight: Weight,
/// Extra size (in bytes) that we assume are included in the call.
///
/// We have some assumptions about headers and justifications of the bridged chain.
/// We know that if our assumptions are correct, then the call must not have the
/// weight above some limit. The fee paid for bytes above that limit, is never refunded.
pub extra_size: u32,
}

impl<N> SubmitFinalityProofInfo<N> {
/// Returns `true` if call size/weight is below our estimations for regular calls.
pub fn fits_limits(&self) -> bool {
self.extra_weight.is_zero() && self.extra_size.is_zero()
}
}

/// Helper struct that provides methods for working with the `SubmitFinalityProof` call.
pub struct SubmitFinalityProofHelper<T: Config<I>, I: 'static> {
pub _phantom_data: sp_std::marker::PhantomData<(T, I)>,
_phantom_data: sp_std::marker::PhantomData<(T, I)>,
}

impl<T: Config<I>, I: 'static> SubmitFinalityProofHelper<T, I> {
Expand Down Expand Up @@ -69,12 +98,17 @@ impl<T: Config<I>, I: 'static> SubmitFinalityProofHelper<T, I> {
pub trait CallSubType<T: Config<I, RuntimeCall = Self>, I: 'static>:
IsSubType<CallableCallFor<Pallet<T, I>, T>>
{
/// Extract the finality target from a `SubmitParachainHeads` call.
fn submit_finality_proof_info(&self) -> Option<BlockNumberOf<T::BridgedChain>> {
if let Some(crate::Call::<T, I>::submit_finality_proof { finality_target, .. }) =
/// Extract finality proof info from a runtime call.
fn submit_finality_proof_info(
&self,
) -> Option<SubmitFinalityProofInfo<BridgedBlockNumber<T, I>>> {
if let Some(crate::Call::<T, I>::submit_finality_proof { finality_target, justification }) =
self.is_sub_type()
{
return Some(*finality_target.number())
return Some(submit_finality_proof_info_from_args::<T, I>(
finality_target,
justification,
))
}

None
Expand All @@ -92,7 +126,7 @@ pub trait CallSubType<T: Config<I, RuntimeCall = Self>, I: 'static>:
_ => return Ok(ValidTransaction::default()),
};

match SubmitFinalityProofHelper::<T, I>::check_obsolete(finality_target) {
match SubmitFinalityProofHelper::<T, I>::check_obsolete(finality_target.block_number) {
Ok(_) => Ok(ValidTransaction::default()),
Err(Error::<T, I>::OldHeader) => InvalidTransaction::Stale.into(),
Err(_) => InvalidTransaction::Call.into(),
Expand All @@ -105,15 +139,66 @@ impl<T: Config<I>, I: 'static> CallSubType<T, I> for T::RuntimeCall where
{
}

/// Extract finality proof info from the submitted header and justification.
pub(crate) fn submit_finality_proof_info_from_args<T: Config<I>, I: 'static>(
finality_target: &BridgedHeader<T, I>,
justification: &GrandpaJustification<BridgedHeader<T, I>>,
) -> SubmitFinalityProofInfo<BridgedBlockNumber<T, I>> {
let block_number = *finality_target.number();

// the `submit_finality_proof` call will reject justifications with invalid, duplicate,
// unknown and extra signatures. It'll also reject justifications with less than necessary
// signatures. So we do not care about extra weight because of additional signatures here.
let precommits_len = justification.commit.precommits.len().saturated_into();
let required_precommits = precommits_len;

// We do care about extra weight because of more-than-expected headers in the votes
// ancestries. But we have problems computing extra weight for additional headers (weight of
// additional header is too small, so that our benchmarks aren't detecting that). So if there
// are more than expected headers in votes ancestries, we will treat the whole call weight
// as an extra weight.
let votes_ancestries_len = justification.votes_ancestries.len().saturated_into();
let extra_weight =
if votes_ancestries_len > T::BridgedChain::REASONABLE_HEADERS_IN_JUSTIFICATON_ANCESTRY {
T::WeightInfo::submit_finality_proof(precommits_len, votes_ancestries_len)
} else {
Weight::zero()
};

// we can estimate extra call size easily, without any additional significant overhead
let actual_call_size: u32 = finality_target
.encoded_size()
.saturating_add(justification.encoded_size())
.saturated_into();
let max_expected_call_size = max_expected_call_size::<T, I>(required_precommits);
let extra_size = actual_call_size.saturating_sub(max_expected_call_size);

SubmitFinalityProofInfo { block_number, extra_weight, extra_size }
}

/// Returns maximal expected size of `submit_finality_proof` call arguments.
fn max_expected_call_size<T: Config<I>, I: 'static>(required_precommits: u32) -> u32 {
let max_expected_justification_size =
GrandpaJustification::max_reasonable_size::<T::BridgedChain>(required_precommits);

// call arguments are header and justification
T::BridgedChain::MAX_HEADER_SIZE.saturating_add(max_expected_justification_size)
}

#[cfg(test)]
mod tests {
use crate::{
call_ext::CallSubType,
mock::{run_test, test_header, RuntimeCall, TestNumber, TestRuntime},
BestFinalized,
mock::{run_test, test_header, RuntimeCall, TestBridgedChain, TestNumber, TestRuntime},
BestFinalized, Config, WeightInfo,
};
use bp_header_chain::ChainWithGrandpa;
use bp_runtime::HeaderId;
use bp_test_utils::make_default_justification;
use bp_test_utils::{
make_default_justification, make_justification_for_header, JustificationGeneratorParams,
};
use frame_support::weights::Weight;
use sp_runtime::{testing::DigestItem, traits::Header as _, SaturatedConversion};

fn validate_block_submit(num: TestNumber) -> bool {
let bridge_grandpa_call = crate::Call::<TestRuntime, ()>::submit_finality_proof {
Expand Down Expand Up @@ -160,4 +245,67 @@ mod tests {
assert!(validate_block_submit(15));
});
}

#[test]
fn extension_returns_correct_extra_size_if_call_arguments_are_too_large() {
// when call arguments are below our limit => no refund
let small_finality_target = test_header(1);
let justification_params = JustificationGeneratorParams {
header: small_finality_target.clone(),
..Default::default()
};
let small_justification = make_justification_for_header(justification_params);
let small_call = RuntimeCall::Grandpa(crate::Call::submit_finality_proof {
finality_target: Box::new(small_finality_target),
justification: small_justification,
});
assert_eq!(small_call.submit_finality_proof_info().unwrap().extra_size, 0);

// when call arguments are too large => partial refund
let mut large_finality_target = test_header(1);
large_finality_target
.digest_mut()
.push(DigestItem::Other(vec![42u8; 1024 * 1024]));
let justification_params = JustificationGeneratorParams {
header: large_finality_target.clone(),
..Default::default()
};
let large_justification = make_justification_for_header(justification_params);
let large_call = RuntimeCall::Grandpa(crate::Call::submit_finality_proof {
finality_target: Box::new(large_finality_target),
justification: large_justification,
});
assert_ne!(large_call.submit_finality_proof_info().unwrap().extra_size, 0);
}

#[test]
fn extension_returns_correct_extra_weight_if_there_are_too_many_headers_in_votes_ancestry() {
let finality_target = test_header(1);
let mut justification_params = JustificationGeneratorParams {
header: finality_target.clone(),
ancestors: TestBridgedChain::REASONABLE_HEADERS_IN_JUSTIFICATON_ANCESTRY,
..Default::default()
};

// when there are `REASONABLE_HEADERS_IN_JUSTIFICATON_ANCESTRY` headers => no refund
let justification = make_justification_for_header(justification_params.clone());
let call = RuntimeCall::Grandpa(crate::Call::submit_finality_proof {
finality_target: Box::new(finality_target.clone()),
justification,
});
assert_eq!(call.submit_finality_proof_info().unwrap().extra_weight, Weight::zero());

// when there are `REASONABLE_HEADERS_IN_JUSTIFICATON_ANCESTRY + 1` headers => full refund
justification_params.ancestors += 1;
let justification = make_justification_for_header(justification_params);
let call_weight = <TestRuntime as Config>::WeightInfo::submit_finality_proof(
justification.commit.precommits.len().saturated_into(),
justification.votes_ancestries.len().saturated_into(),
);
let call = RuntimeCall::Grandpa(crate::Call::submit_finality_proof {
finality_target: Box::new(finality_target),
justification,
});
assert_eq!(call.submit_finality_proof_info().unwrap().extra_weight, call_weight);
}
}
Loading

0 comments on commit 0fda54d

Please sign in to comment.