-
Notifications
You must be signed in to change notification settings - Fork 131
Refund extra proof bytes in message delivery transaction #1864
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
Changes from 1 commit
2400817
3e184ab
e3fe021
e4916f6
1a719ef
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -303,6 +303,13 @@ pub mod pallet { | |||||||||||||||||||||||||
| for (lane_id, lane_data) in messages { | ||||||||||||||||||||||||||
| let mut lane = inbound_lane::<T, I>(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<T: Config<I>, I: 'static = ()> { | |||||||||||||||||||||||||
| _phantom: PhantomData<I>, | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| impl<T: Config<I>, I: 'static> RuntimeInboundLaneStorage<T, I> { | ||||||||||||||||||||||||||
| /// 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::<T, I>::max_encoded_len(); | ||||||||||||||||||||||||||
| let relayers_count = self.data().relayers.len(); | ||||||||||||||||||||||||||
| let actual_encoded_len = | ||||||||||||||||||||||||||
| InboundLaneData::<T::InboundRelayer>::encoded_size_hint(relayers_count) | ||||||||||||||||||||||||||
| .unwrap_or(usize::MAX); | ||||||||||||||||||||||||||
| max_encoded_len.saturating_sub(actual_encoded_len) as _ | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| impl<T: Config<I>, I: 'static> InboundLaneStorage for RuntimeInboundLaneStorage<T, I> { | ||||||||||||||||||||||||||
| 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,14 +1609,92 @@ 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); | ||||||||||||||||||||||||||
| assert_eq!(post.ref_time(), pre.ref_time() - 1); | ||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| #[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 = | ||||||||||||||||||||||||||
| <TestRuntime as Config>::WeightInfo::receive_messages_proof_weight( | ||||||||||||||||||||||||||
| &proof, | ||||||||||||||||||||||||||
| messages_count, | ||||||||||||||||||||||||||
| REGULAR_PAYLOAD.declared_weight, | ||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||
| InboundLanes::<TestRuntime>::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::<TestRuntime>::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::<TestRuntime>::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::<TestRuntime>::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<TestRelayer> { | ||||||||||||||||||||||||||
| UnrewardedRelayer { relayer: 42u64, messages: DeliveredMessages { begin: 0, end: 100 } } | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| fn storage(relayer_entries: usize) -> RuntimeInboundLaneStorage<TestRuntime, ()> { | ||||||||||||||||||||||||||
| 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 | ||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||
|
Comment on lines
+2108
to
+2111
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit1: I hope nit2: I would add another one for extra safety (especially since previous test only verifies generic
Suggested change
(above could also be a loop, but testing at least two values is good enough imo)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I'm not sure exactly what you mean here, but both expressions in the
Done |
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // when we have more than `MaxUnrewardedRelayerEntriesAtInboundLane` unrewarded relayers | ||||||||||||||||||||||||||
| // (shall not happen in practice) | ||||||||||||||||||||||||||
| assert_eq!(storage(max_entries + 1).extra_proof_size_bytes(), 0); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.