From c00a47d5cae33f7f27e4bfe07aa854bd1ce6e04d Mon Sep 17 00:00:00 2001
From: Hernando Castano <HCastano@users.noreply.github.com>
Date: Fri, 26 Feb 2021 06:43:05 -0500
Subject: [PATCH] Stop counting invalid requests towards rate limit (#765)
---
bridges/modules/finality-verifier/src/lib.rs | 39 +++++++++++++++++++-
1 file changed, 37 insertions(+), 2 deletions(-)
diff --git a/bridges/modules/finality-verifier/src/lib.rs b/bridges/modules/finality-verifier/src/lib.rs
index a87bd75b992..4e05c5754d9 100644
--- a/bridges/modules/finality-verifier/src/lib.rs
+++ b/bridges/modules/finality-verifier/src/lib.rs
@@ -75,6 +75,8 @@ pub mod pallet {
/// The upper bound on the number of requests allowed by the pallet.
///
+ /// A request refers to an action which writes a header to storage.
+ ///
/// Once this bound is reached the pallet will not allow any dispatchables to be called
/// until the request count has decreased.
#[pallet::constant]
@@ -117,7 +119,6 @@ pub mod pallet {
Self::request_count() < T::MaxRequests::get(),
<Error<T>>::TooManyRequests
);
- <RequestCount<T>>::mutate(|count| *count += 1);
frame_support::debug::trace!("Going to try and finalize header {:?}", finality_target);
@@ -144,11 +145,13 @@ pub mod pallet {
T::HeaderChain::append_header(finality_target);
frame_support::debug::info!("Succesfully imported finalized header with hash {:?}!", hash);
+ <RequestCount<T>>::mutate(|count| *count += 1);
+
Ok(().into())
}
}
- /// The current number of requests for calling dispatchables.
+ /// The current number of requests which have written to storage.
///
/// If the `RequestCount` hits `MaxRequests`, no more calls will be allowed to the pallet until
/// the request capacity is increased.
@@ -340,6 +343,38 @@ mod tests {
})
}
+ #[test]
+ fn invalid_requests_do_not_count_towards_request_count() {
+ run_test(|| {
+ let submit_invalid_request = || {
+ let child = test_header(1);
+ let header = test_header(2);
+
+ let invalid_justification = vec![4, 2, 4, 2].encode();
+ let ancestry_proof = vec![child, header.clone()];
+
+ Module::<TestRuntime>::submit_finality_proof(
+ Origin::signed(1),
+ header,
+ invalid_justification,
+ ancestry_proof,
+ )
+ };
+
+ initialize_substrate_bridge();
+
+ for _ in 0..<TestRuntime as Config>::MaxRequests::get() + 1 {
+ // Notice that the error here *isn't* `TooManyRequests`
+ assert_err!(submit_invalid_request(), <Error<TestRuntime>>::InvalidJustification);
+ }
+
+ // Can still submit `MaxRequests` requests afterwards
+ assert_ok!(submit_finality_proof());
+ assert_ok!(submit_finality_proof());
+ assert_err!(submit_finality_proof(), <Error<TestRuntime>>::TooManyRequests);
+ })
+ }
+
#[test]
fn allows_request_after_new_block_has_started() {
run_test(|| {
--
GitLab