Skip to content

Commit

Permalink
Backport two recent PRs from polkadot-staging branch (#2828)
Browse files Browse the repository at this point in the history
* Add submit_finality_proof_ex call to the GRANDPA pallet (#2821)

* depreacte submit_finality_proof and introduce submit_finality_proof_ex instead

* propagate changes to the rest of runtime crates

* tests for new call

* suppress deprecation warning

* revert changes to benchmarks to avoid unnecessary compilation issues when integrating to other repos

* Pass finality proof verification context to the call builder (#2823)

* pass verification context to the build_submit_finality_proof_call

* current_set_id -> context
  • Loading branch information
svyatonik authored Feb 7, 2024
1 parent 4d042b9 commit d9f11f7
Show file tree
Hide file tree
Showing 12 changed files with 574 additions and 116 deletions.
10 changes: 5 additions & 5 deletions 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 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
114 changes: 103 additions & 11 deletions 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, SubmitFinalityProofInfo,
};
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,
transaction_validity::{InvalidTransaction, TransactionValidity, ValidTransaction},
Expand All @@ -35,9 +39,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 @@ -59,6 +65,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 @@ -85,6 +105,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 @@ -107,7 +139,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 @@ -124,6 +159,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 @@ -165,28 +201,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,
WeightInfo,
};
use bp_header_chain::ChainWithGrandpa;
use bp_header_chain::{ChainWithGrandpa, SubmitFinalityProofInfo};
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 @@ -230,6 +270,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 @@ -240,6 +292,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 @@ -249,9 +337,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 @@ -265,9 +354,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 @@ -283,9 +373,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 @@ -296,9 +387,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

0 comments on commit d9f11f7

Please sign in to comment.