From 24008176ba28c578bbdb87c0c22b0f11611067e9 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Wed, 8 Feb 2023 14:12:44 +0300 Subject: [PATCH 1/3] refund extra proof bytes in message delivery transaction --- modules/messages/src/inbound_lane.rs | 5 + modules/messages/src/lib.rs | 147 ++++++++++++++++++++++++++- 2 files changed, 147 insertions(+), 5 deletions(-) diff --git a/modules/messages/src/inbound_lane.rs b/modules/messages/src/inbound_lane.rs index 00c63b5d67..3f64ab765b 100644 --- a/modules/messages/src/inbound_lane.rs +++ b/modules/messages/src/inbound_lane.rs @@ -117,6 +117,11 @@ impl InboundLane { InboundLane { storage } } + /// Returns storage reference. + pub fn storage(&self) -> &S { + &self.storage + } + /// Receive state of the corresponding outbound lane. pub fn receive_state_update( &mut self, diff --git a/modules/messages/src/lib.rs b/modules/messages/src/lib.rs index eaa681df38..d60c2a65e7 100644 --- a/modules/messages/src/lib.rs +++ b/modules/messages/src/lib.rs @@ -303,6 +303,13 @@ pub mod pallet { for (lane_id, lane_data) in messages { let mut lane = inbound_lane::(lane_id); + // subtract extra storage proof bytes from the actual PoV size - there may be + // less unrewarded relayers than the maximal configured value + let lane_extra_proof_size_bytes = lane.storage().extra_proof_size_bytes(); + actual_weight = actual_weight.set_proof_size( + actual_weight.proof_size().saturating_sub(lane_extra_proof_size_bytes), + ); + if let Some(lane_state) = lane_data.lane_state { let updated_latest_confirmed_nonce = lane.receive_state_update(lane_state); if let Some(updated_latest_confirmed_nonce) = updated_latest_confirmed_nonce { @@ -781,6 +788,25 @@ struct RuntimeInboundLaneStorage, I: 'static = ()> { _phantom: PhantomData, } +impl, I: 'static> RuntimeInboundLaneStorage { + /// Returns number of bytes that may be subtracted from the PoV component of + /// `receive_messages_proof` call, because the actual inbound lane state is smaller than the + /// maximal configured. + /// + /// Maximal inbound lane state set size is configured by the + /// `MaxUnrewardedRelayerEntriesAtInboundLane` constant from the pallet configuration. The PoV + /// of the call includes the maximal size of inbound lane state. If the actual size is smaller, + /// we may subtract extra bytes from this component. + pub fn extra_proof_size_bytes(&self) -> u64 { + let max_encoded_len = StoredInboundLaneData::::max_encoded_len(); + let relayers_count = self.data().relayers.len(); + let actual_encoded_len = + InboundLaneData::::encoded_size_hint(relayers_count) + .unwrap_or(usize::MAX); + max_encoded_len.saturating_sub(actual_encoded_len) as _ + } +} + impl, I: 'static> InboundLaneStorage for RuntimeInboundLaneStorage { type Relayer = T::InboundRelayer; @@ -891,9 +917,9 @@ mod tests { use crate::mock::{ message, message_payload, run_test, unrewarded_relayer, AccountId, DbWeight, RuntimeEvent as TestEvent, RuntimeOrigin, TestDeliveryConfirmationPayments, - TestDeliveryPayments, TestMessagesDeliveryProof, TestMessagesProof, TestRuntime, - MAX_OUTBOUND_PAYLOAD_SIZE, PAYLOAD_REJECTED_BY_TARGET_CHAIN, REGULAR_PAYLOAD, TEST_LANE_ID, - TEST_LANE_ID_2, TEST_LANE_ID_3, TEST_RELAYER_A, TEST_RELAYER_B, + TestDeliveryPayments, TestMessagesDeliveryProof, TestMessagesProof, TestRelayer, + TestRuntime, MAX_OUTBOUND_PAYLOAD_SIZE, PAYLOAD_REJECTED_BY_TARGET_CHAIN, REGULAR_PAYLOAD, + TEST_LANE_ID, TEST_LANE_ID_2, TEST_LANE_ID_3, TEST_RELAYER_A, TEST_RELAYER_B, }; use bp_messages::{BridgeMessagesCall, UnrewardedRelayer, UnrewardedRelayersState}; use bp_test_utils::generate_owned_bridge_module_tests; @@ -1528,7 +1554,7 @@ mod tests { } #[test] - fn weight_refund_from_receive_messages_proof_works() { + fn ref_time_refund_from_receive_messages_proof_works() { run_test(|| { fn submit_with_unspent_weight( nonce: MessageNonce, @@ -1583,7 +1609,7 @@ mod tests { // when there's no unspent weight let (pre, post) = submit_with_unspent_weight(4, 0); - assert_eq!(post, pre); + assert_eq!(post.ref_time(), pre.ref_time()); // when dispatch is returning `unspent_weight < declared_weight` let (pre, post) = submit_with_unspent_weight(5, 1); @@ -1591,6 +1617,84 @@ mod tests { }); } + #[test] + fn proof_size_refund_from_receive_messages_proof_works() { + run_test(|| { + let max_entries = crate::mock::MaxUnrewardedRelayerEntriesAtInboundLane::get() as usize; + + // if there's maximal number of unrewarded relayer entries at the inbound lane, then + // `proof_size` is unchanged in post-dispatch weight + let proof: TestMessagesProof = Ok(vec![message(101, REGULAR_PAYLOAD)]).into(); + let messages_count = 1; + let pre_dispatch_weight = + ::WeightInfo::receive_messages_proof_weight( + &proof, + messages_count, + REGULAR_PAYLOAD.declared_weight, + ); + InboundLanes::::insert( + TEST_LANE_ID, + StoredInboundLaneData(InboundLaneData { + relayers: vec![ + UnrewardedRelayer { + relayer: 42, + messages: DeliveredMessages { begin: 0, end: 100 } + }; + max_entries + ] + .into_iter() + .collect(), + last_confirmed_nonce: 0, + }), + ); + let post_dispatch_weight = Pallet::::receive_messages_proof( + RuntimeOrigin::signed(1), + TEST_RELAYER_A, + proof.clone(), + messages_count, + REGULAR_PAYLOAD.declared_weight, + ) + .unwrap() + .actual_weight + .unwrap(); + assert_eq!(post_dispatch_weight.proof_size(), pre_dispatch_weight.proof_size()); + + // if cound of unrewarded relayer entries is less than maximal, then some `proof_size` + // must be refunded + InboundLanes::::insert( + TEST_LANE_ID, + StoredInboundLaneData(InboundLaneData { + relayers: vec![ + UnrewardedRelayer { + relayer: 42, + messages: DeliveredMessages { begin: 0, end: 100 } + }; + max_entries - 1 + ] + .into_iter() + .collect(), + last_confirmed_nonce: 0, + }), + ); + let post_dispatch_weight = Pallet::::receive_messages_proof( + RuntimeOrigin::signed(1), + TEST_RELAYER_A, + proof, + messages_count, + REGULAR_PAYLOAD.declared_weight, + ) + .unwrap() + .actual_weight + .unwrap(); + assert!( + post_dispatch_weight.proof_size() < pre_dispatch_weight.proof_size(), + "Expected post-dispatch PoV {} to be less than pre-dispatch PoV {}", + post_dispatch_weight.proof_size(), + pre_dispatch_weight.proof_size(), + ); + }); + } + #[test] fn messages_delivered_callbacks_are_called() { run_test(|| { @@ -1962,4 +2066,37 @@ mod tests { MessagesOperatingMode::Basic(BasicOperatingMode::Normal), MessagesOperatingMode::Basic(BasicOperatingMode::Halted) ); + + #[test] + fn inbound_storage_extra_proof_size_bytes_works() { + fn relayer_entry() -> UnrewardedRelayer { + UnrewardedRelayer { relayer: 42u64, messages: DeliveredMessages { begin: 0, end: 100 } } + } + + fn storage(relayer_entries: usize) -> RuntimeInboundLaneStorage { + RuntimeInboundLaneStorage { + lane_id: Default::default(), + cached_data: RefCell::new(Some(InboundLaneData { + relayers: vec![relayer_entry(); relayer_entries].into_iter().collect(), + last_confirmed_nonce: 0, + })), + _phantom: Default::default(), + } + } + + let max_entries = crate::mock::MaxUnrewardedRelayerEntriesAtInboundLane::get() as usize; + + // when we have exactly `MaxUnrewardedRelayerEntriesAtInboundLane` unrewarded relayers + assert_eq!(storage(max_entries).extra_proof_size_bytes(), 0); + + // when we have less than `MaxUnrewardedRelayerEntriesAtInboundLane` unrewarded relayers + assert_eq!( + storage(max_entries - 1).extra_proof_size_bytes(), + relayer_entry().encode().len() as u64 + ); + + // when we have more than `MaxUnrewardedRelayerEntriesAtInboundLane` unrewarded relayers + // (shall not happen in practice) + assert_eq!(storage(max_entries + 1).extra_proof_size_bytes(), 0); + } } From 3e184ab55746dc1e1457657153d9177edc43c855 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Thu, 9 Feb 2023 10:45:01 +0300 Subject: [PATCH 2/3] Update modules/messages/src/lib.rs Co-authored-by: Adrian Catangiu --- modules/messages/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/messages/src/lib.rs b/modules/messages/src/lib.rs index d60c2a65e7..7f8f708e7c 100644 --- a/modules/messages/src/lib.rs +++ b/modules/messages/src/lib.rs @@ -1659,7 +1659,7 @@ mod tests { .unwrap(); assert_eq!(post_dispatch_weight.proof_size(), pre_dispatch_weight.proof_size()); - // if cound of unrewarded relayer entries is less than maximal, then some `proof_size` + // if count of unrewarded relayer entries is less than maximal, then some `proof_size` // must be refunded InboundLanes::::insert( TEST_LANE_ID, From e3fe021e1a01ecfaef16370f1bcfb759b9c058a1 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Thu, 9 Feb 2023 11:03:53 +0300 Subject: [PATCH 3/3] more tests --- modules/messages/src/lib.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/modules/messages/src/lib.rs b/modules/messages/src/lib.rs index d60c2a65e7..295a698fb0 100644 --- a/modules/messages/src/lib.rs +++ b/modules/messages/src/lib.rs @@ -2094,6 +2094,10 @@ mod tests { storage(max_entries - 1).extra_proof_size_bytes(), relayer_entry().encode().len() as u64 ); + assert_eq!( + storage(max_entries - 2).extra_proof_size_bytes(), + 2 * relayer_entry().encode().len() as u64 + ); // when we have more than `MaxUnrewardedRelayerEntriesAtInboundLane` unrewarded relayers // (shall not happen in practice)