Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Introduce submit_finality_proof_ex call to bridges GRANDPA pallet #3225

Merged
merged 3 commits into from
Feb 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
268 changes: 266 additions & 2 deletions bridges/bin/runtime-common/src/refund_relayer_extension.rs

Large diffs are not rendered by default.

10 changes: 5 additions & 5 deletions bridges/modules/grandpa/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,11 @@ There are two main things in GRANDPA that help building light clients:

## Pallet Operations

The main entrypoint of the pallet is the `submit_finality_proof` call. It has two arguments - the finalized
headers and associated GRANDPA justification. The call simply verifies the justification using current
validators set and checks if header is better than the previous best header. If both checks are passed, the
header (only its useful fields) is inserted into the runtime storage and may be used by other pallets to verify
storage proofs.
The main entrypoint of the pallet is the `submit_finality_proof_ex` call. It has three arguments - the finalized
headers, associated GRANDPA justification and ID of the authority set that has generated this justification. The
call simply verifies the justification using current validators set and checks if header is better than the
previous best header. If both checks are passed, the header (only its useful fields) is inserted into the runtime
storage and may be used by other pallets to verify storage proofs.

The submitter pays regular fee for submitting all headers, except for the mandatory header. Since it is
required for the pallet operations, submitting such header is free. So if you're ok with session-length
Expand Down
5 changes: 3 additions & 2 deletions bridges/modules/grandpa/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@

//! Benchmarks for the GRANDPA Pallet.
//!
//! The main dispatchable for the GRANDPA pallet is `submit_finality_proof`, so these benchmarks are
//! based around that. There are to main factors which affect finality proof verification:
//! The main dispatchable for the GRANDPA pallet is `submit_finality_proof_ex`. Our benchmarks
//! are based around `submit_finality_proof`, though - from weight PoV they are the same calls.
//! There are to main factors which affect finality proof verification:
//!
//! 1. The number of `votes-ancestries` in the justification
//! 2. The number of `pre-commits` in the justification
Expand Down
115 changes: 105 additions & 10 deletions bridges/modules/grandpa/src/call_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,18 @@
// 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::{weights::WeightInfo, BridgedBlockNumber, BridgedHeader, Config, Error, Pallet};
use crate::{
weights::WeightInfo, BridgedBlockNumber, BridgedHeader, Config, CurrentAuthoritySet, Error,
Pallet,
};
use bp_header_chain::{
justification::GrandpaJustification, max_expected_submit_finality_proof_arguments_size,
ChainWithGrandpa, GrandpaConsensusLogReader,
};
use bp_runtime::{BlockNumberOf, OwnedBridgeModule};
use codec::Encode;
use frame_support::{dispatch::CallableCallFor, traits::IsSubType, weights::Weight};
use sp_consensus_grandpa::SetId;
use sp_runtime::{
traits::{Header, Zero},
transaction_validity::{InvalidTransaction, TransactionValidity, ValidTransaction},
Expand All @@ -33,6 +37,9 @@ use sp_runtime::{
pub struct SubmitFinalityProofInfo<N> {
/// Number of the finality target.
pub block_number: N,
/// An identifier of the validators set that has signed the submitted justification.
/// It might be `None` if deprecated version of the `submit_finality_proof` is used.
pub current_set_id: Option<SetId>,
/// Extra weight that we assume is included in the call.
///
/// We have some assumptions about headers and justifications of the bridged chain.
Expand Down Expand Up @@ -61,9 +68,11 @@ pub struct SubmitFinalityProofHelper<T: Config<I>, I: 'static> {

impl<T: Config<I>, I: 'static> SubmitFinalityProofHelper<T, I> {
/// Check that the GRANDPA head provided by the `SubmitFinalityProof` is better than the best
/// one we know.
/// one we know. Additionally, checks if `current_set_id` matches the current authority set
/// id, if specified.
pub fn check_obsolete(
finality_target: BlockNumberOf<T::BridgedChain>,
current_set_id: Option<SetId>,
) -> Result<(), Error<T, I>> {
let best_finalized = crate::BestFinalized::<T, I>::get().ok_or_else(|| {
log::trace!(
Expand All @@ -85,6 +94,20 @@ impl<T: Config<I>, I: 'static> SubmitFinalityProofHelper<T, I> {
return Err(Error::<T, I>::OldHeader)
}

if let Some(current_set_id) = current_set_id {
let actual_set_id = <CurrentAuthoritySet<T, I>>::get().set_id;
if current_set_id != actual_set_id {
log::trace!(
target: crate::LOG_TARGET,
"Cannot finalize header signed by unknown authority set: bundled {:?}, best {:?}",
current_set_id,
actual_set_id,
);

return Err(Error::<T, I>::InvalidAuthoritySetId)
}
}

Ok(())
}

Expand All @@ -111,6 +134,18 @@ pub trait CallSubType<T: Config<I, RuntimeCall = Self>, I: 'static>:
return Some(submit_finality_proof_info_from_args::<T, I>(
finality_target,
justification,
None,
))
} else if let Some(crate::Call::<T, I>::submit_finality_proof_ex {
finality_target,
justification,
current_set_id,
}) = self.is_sub_type()
{
return Some(submit_finality_proof_info_from_args::<T, I>(
finality_target,
justification,
Some(*current_set_id),
))
}

Expand All @@ -133,7 +168,10 @@ pub trait CallSubType<T: Config<I, RuntimeCall = Self>, I: 'static>:
return InvalidTransaction::Call.into()
}

match SubmitFinalityProofHelper::<T, I>::check_obsolete(finality_target.block_number) {
match SubmitFinalityProofHelper::<T, I>::check_obsolete(
finality_target.block_number,
finality_target.current_set_id,
) {
Ok(_) => Ok(ValidTransaction::default()),
Err(Error::<T, I>::OldHeader) => InvalidTransaction::Stale.into(),
Err(_) => InvalidTransaction::Call.into(),
Expand All @@ -150,6 +188,7 @@ impl<T: Config<I>, I: 'static> CallSubType<T, I> for T::RuntimeCall where
pub(crate) fn submit_finality_proof_info_from_args<T: Config<I>, I: 'static>(
finality_target: &BridgedHeader<T, I>,
justification: &GrandpaJustification<BridgedHeader<T, I>>,
current_set_id: Option<SetId>,
) -> SubmitFinalityProofInfo<BridgedBlockNumber<T, I>> {
let block_number = *finality_target.number();

Expand Down Expand Up @@ -191,28 +230,32 @@ pub(crate) fn submit_finality_proof_info_from_args<T: Config<I>, I: 'static>(
);
let extra_size = actual_call_size.saturating_sub(max_expected_call_size);

SubmitFinalityProofInfo { block_number, extra_weight, extra_size }
SubmitFinalityProofInfo { block_number, current_set_id, extra_weight, extra_size }
}

#[cfg(test)]
mod tests {
use crate::{
call_ext::CallSubType,
mock::{run_test, test_header, RuntimeCall, TestBridgedChain, TestNumber, TestRuntime},
BestFinalized, Config, PalletOperatingMode, WeightInfo,
BestFinalized, Config, CurrentAuthoritySet, PalletOperatingMode, StoredAuthoritySet,
SubmitFinalityProofInfo, WeightInfo,
};
use bp_header_chain::ChainWithGrandpa;
use bp_runtime::{BasicOperatingMode, HeaderId};
use bp_test_utils::{
make_default_justification, make_justification_for_header, JustificationGeneratorParams,
TEST_GRANDPA_SET_ID,
};
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 {
let bridge_grandpa_call = crate::Call::<TestRuntime, ()>::submit_finality_proof_ex {
finality_target: Box::new(test_header(num)),
justification: make_default_justification(&test_header(num)),
// not initialized => zero
current_set_id: 0,
};
RuntimeCall::check_obsolete_submit_finality_proof(&RuntimeCall::Grandpa(
bridge_grandpa_call,
Expand Down Expand Up @@ -256,6 +299,18 @@ mod tests {
});
}

#[test]
fn extension_rejects_new_header_if_set_id_is_invalid() {
run_test(|| {
// when set id is different from the passed one => tx is rejected
sync_to_header_10();
let next_set = StoredAuthoritySet::<TestRuntime, ()>::try_new(vec![], 0x42).unwrap();
CurrentAuthoritySet::<TestRuntime, ()>::put(next_set);

assert!(!validate_block_submit(15));
});
}

#[test]
fn extension_accepts_new_header() {
run_test(|| {
Expand All @@ -266,6 +321,42 @@ mod tests {
});
}

#[test]
fn submit_finality_proof_info_is_parsed() {
// when `submit_finality_proof` is used, `current_set_id` is set to `None`
let deprecated_call =
RuntimeCall::Grandpa(crate::Call::<TestRuntime, ()>::submit_finality_proof {
finality_target: Box::new(test_header(42)),
justification: make_default_justification(&test_header(42)),
});
assert_eq!(
deprecated_call.submit_finality_proof_info(),
Some(SubmitFinalityProofInfo {
block_number: 42,
current_set_id: None,
extra_weight: Weight::zero(),
extra_size: 0,
})
);

// when `submit_finality_proof_ex` is used, `current_set_id` is set to `Some`
let deprecated_call =
RuntimeCall::Grandpa(crate::Call::<TestRuntime, ()>::submit_finality_proof_ex {
finality_target: Box::new(test_header(42)),
justification: make_default_justification(&test_header(42)),
current_set_id: 777,
});
assert_eq!(
deprecated_call.submit_finality_proof_info(),
Some(SubmitFinalityProofInfo {
block_number: 42,
current_set_id: Some(777),
extra_weight: Weight::zero(),
extra_size: 0,
})
);
}

#[test]
fn extension_returns_correct_extra_size_if_call_arguments_are_too_large() {
// when call arguments are below our limit => no refund
Expand All @@ -275,9 +366,10 @@ mod tests {
..Default::default()
};
let small_justification = make_justification_for_header(justification_params);
let small_call = RuntimeCall::Grandpa(crate::Call::submit_finality_proof {
let small_call = RuntimeCall::Grandpa(crate::Call::submit_finality_proof_ex {
finality_target: Box::new(small_finality_target),
justification: small_justification,
current_set_id: TEST_GRANDPA_SET_ID,
});
assert_eq!(small_call.submit_finality_proof_info().unwrap().extra_size, 0);

Expand All @@ -291,9 +383,10 @@ mod tests {
..Default::default()
};
let large_justification = make_justification_for_header(justification_params);
let large_call = RuntimeCall::Grandpa(crate::Call::submit_finality_proof {
let large_call = RuntimeCall::Grandpa(crate::Call::submit_finality_proof_ex {
finality_target: Box::new(large_finality_target),
justification: large_justification,
current_set_id: TEST_GRANDPA_SET_ID,
});
assert_ne!(large_call.submit_finality_proof_info().unwrap().extra_size, 0);
}
Expand All @@ -309,9 +402,10 @@ mod tests {

// 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 {
let call = RuntimeCall::Grandpa(crate::Call::submit_finality_proof_ex {
finality_target: Box::new(finality_target.clone()),
justification,
current_set_id: TEST_GRANDPA_SET_ID,
});
assert_eq!(call.submit_finality_proof_info().unwrap().extra_weight, Weight::zero());

Expand All @@ -322,9 +416,10 @@ mod tests {
justification.commit.precommits.len().saturated_into(),
justification.votes_ancestries.len().saturated_into(),
);
let call = RuntimeCall::Grandpa(crate::Call::submit_finality_proof {
let call = RuntimeCall::Grandpa(crate::Call::submit_finality_proof_ex {
finality_target: Box::new(finality_target),
justification,
current_set_id: TEST_GRANDPA_SET_ID,
});
assert_eq!(call.submit_finality_proof_info().unwrap().extra_weight, call_weight);
}
Expand Down
Loading
Loading