Revert dispatch-results (#2048)

* Revert "Reintroduce msg dispatch status reporting (#2027)"

This reverts commit 38bb051e3d778ee2f5e9451f89cf479a71fd68f8.

* post-revert fix
This commit is contained in:
Svyatoslav Nikolsky
2023-04-17 12:14:34 +03:00
committed by Bastian Köcher
parent c2e68484de
commit 512d43fabe
17 changed files with 132 additions and 315 deletions
+6 -6
View File
@@ -301,7 +301,7 @@ benchmarks_instance_pallet! {
inbound_lane_data: InboundLaneData {
relayers: vec![UnrewardedRelayer {
relayer: relayer_id.clone(),
messages: DeliveredMessages::new(1, true),
messages: DeliveredMessages::new(1),
}].into_iter().collect(),
last_confirmed_nonce: 0,
},
@@ -333,8 +333,8 @@ benchmarks_instance_pallet! {
total_messages: 2,
last_delivered_nonce: 2,
};
let mut delivered_messages = DeliveredMessages::new(1, true);
delivered_messages.note_dispatched_message(true);
let mut delivered_messages = DeliveredMessages::new(1);
delivered_messages.note_dispatched_message();
let proof = T::prepare_message_delivery_proof(MessageDeliveryProofParams {
lane: T::bench_lane_id(),
inbound_lane_data: InboundLaneData {
@@ -379,11 +379,11 @@ benchmarks_instance_pallet! {
relayers: vec![
UnrewardedRelayer {
relayer: relayer1_id.clone(),
messages: DeliveredMessages::new(1, true),
messages: DeliveredMessages::new(1),
},
UnrewardedRelayer {
relayer: relayer2_id.clone(),
messages: DeliveredMessages::new(2, true),
messages: DeliveredMessages::new(2),
},
].into_iter().collect(),
last_confirmed_nonce: 0,
@@ -451,7 +451,7 @@ fn receive_messages<T: Config<I>, I: 'static>(nonce: MessageNonce) {
inbound_lane_storage.set_data(InboundLaneData {
relayers: vec![UnrewardedRelayer {
relayer: T::bridged_relayer_id(),
messages: DeliveredMessages::new(nonce, true),
messages: DeliveredMessages::new(nonce),
}]
.into_iter()
.collect(),
+11 -16
View File
@@ -101,7 +101,6 @@ impl<T: Config<I>, I: 'static> MaxEncodedLen for StoredInboundLaneData<T, I> {
fn max_encoded_len() -> usize {
InboundLaneData::<T::InboundRelayer>::encoded_size_hint(
T::MaxUnrewardedRelayerEntriesAtInboundLane::get() as usize,
T::MaxUnconfirmedMessagesAtInboundLane::get() as usize,
)
.unwrap_or(usize::MAX)
}
@@ -155,9 +154,6 @@ impl<S: InboundLaneStorage> InboundLane<S> {
// overlap.
match data.relayers.front_mut() {
Some(entry) if entry.messages.begin < new_confirmed_nonce => {
entry.messages.dispatch_results = entry.messages.dispatch_results
[(new_confirmed_nonce + 1 - entry.messages.begin) as usize..]
.to_bitvec();
entry.messages.begin = new_confirmed_nonce + 1;
},
_ => {},
@@ -174,7 +170,7 @@ impl<S: InboundLaneStorage> InboundLane<S> {
relayer_at_this_chain: &AccountId,
nonce: MessageNonce,
message_data: DispatchMessageData<Dispatch::DispatchPayload>,
) -> ReceivalResult<Dispatch::DispatchError> {
) -> ReceivalResult<Dispatch::DispatchLevelResult> {
let mut data = self.storage.data();
let is_correct_message = nonce == data.last_delivered_nonce() + 1;
if !is_correct_message {
@@ -202,20 +198,19 @@ impl<S: InboundLaneStorage> InboundLane<S> {
);
// now let's update inbound lane storage
match data.relayers.back_mut() {
let push_new = match data.relayers.back_mut() {
Some(entry) if entry.relayer == *relayer_at_bridged_chain => {
entry.messages.note_dispatched_message(dispatch_result.dispatch_result.is_ok());
},
_ => {
data.relayers.push_back(UnrewardedRelayer {
relayer: relayer_at_bridged_chain.clone(),
messages: DeliveredMessages::new(
nonce,
dispatch_result.dispatch_result.is_ok(),
),
});
entry.messages.note_dispatched_message();
false
},
_ => true,
};
if push_new {
data.relayers.push_back(UnrewardedRelayer {
relayer: (*relayer_at_bridged_chain).clone(),
messages: DeliveredMessages::new(nonce),
});
}
self.storage.set_data(data);
ReceivalResult::Dispatched(dispatch_result)
+45 -66
View File
@@ -66,7 +66,7 @@ use bp_messages::{
use bp_runtime::{BasicOperatingMode, ChainId, OwnedBridgeModule, PreComputedSize, Size};
use codec::{Decode, Encode, MaxEncodedLen};
use frame_support::{dispatch::PostDispatchInfo, ensure, fail, traits::Get};
use sp_runtime::{traits::UniqueSaturatedFrom, SaturatedConversion};
use sp_runtime::traits::UniqueSaturatedFrom;
use sp_std::{cell::RefCell, marker::PhantomData, prelude::*};
mod inbound_lane;
@@ -547,7 +547,7 @@ pub mod pallet {
MessagesReceived(
Vec<
ReceivedMessages<
<T::MessageDispatch as MessageDispatch<T::AccountId>>::DispatchError,
<T::MessageDispatch as MessageDispatch<T::AccountId>>::DispatchLevelResult,
>,
>,
),
@@ -827,19 +827,15 @@ impl<T: Config<I>, I: 'static> RuntimeInboundLaneStorage<T, I> {
/// `receive_messages_proof` call, because the actual inbound lane state is smaller than the
/// maximal configured.
///
/// Maximal inbound lane state size is computed using the
/// `MaxUnrewardedRelayerEntriesAtInboundLane` and `MaxUnconfirmedMessagesAtInboundLane`
/// constants from the pallet configuration. The PoV of the call includes the maximal size
/// of the inbound lane state. If the actual size is smaller, we may subtract extra bytes
/// from this component.
/// 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 messages_count = self.data().relayers.iter().fold(0usize, |sum, relayer| {
sum.saturating_add(relayer.messages.total_messages().saturated_into::<usize>())
});
let actual_encoded_len =
InboundLaneData::<T::InboundRelayer>::encoded_size_hint(relayers_count, messages_count)
InboundLaneData::<T::InboundRelayer>::encoded_size_hint(relayers_count)
.unwrap_or(usize::MAX);
max_encoded_len.saturating_sub(actual_encoded_len) as _
}
@@ -971,7 +967,6 @@ mod tests {
};
use frame_system::{EventRecord, Pallet as System, Phase};
use sp_runtime::DispatchError;
use std::collections::VecDeque;
fn get_ready_for_events() {
System::<TestRuntime>::set_block_number(1);
@@ -1029,7 +1024,7 @@ mod tests {
last_confirmed_nonce: 1,
relayers: vec![UnrewardedRelayer {
relayer: 0,
messages: DeliveredMessages::new(1, true),
messages: DeliveredMessages::new(1),
}]
.into_iter()
.collect(),
@@ -1049,44 +1044,13 @@ mod tests {
phase: Phase::Initialization,
event: TestEvent::Messages(Event::MessagesDelivered {
lane_id: TEST_LANE_ID,
messages: DeliveredMessages::new(1, true),
messages: DeliveredMessages::new(1),
}),
topics: vec![],
}],
);
}
fn unrewarded_relayer_entry(msg_count: usize) -> UnrewardedRelayer<TestRelayer> {
UnrewardedRelayer {
relayer: 42u64,
messages: DeliveredMessages {
begin: 0,
end: msg_count as MessageNonce - 1,
dispatch_results: FromIterator::from_iter(vec![true; msg_count]),
},
}
}
fn unrewarded_relayers_vec(
entry_count: usize,
msg_count: usize,
) -> VecDeque<UnrewardedRelayer<TestRelayer>> {
if entry_count > msg_count {
panic!("unrewarded_relayers_vec(): expecting msg_count to be >= entry_count");
}
let mut unrewarded_relayers = vec![];
let mut available_msg_count = msg_count;
for _ in 0..entry_count - 1 {
unrewarded_relayers
.push(unrewarded_relayer_entry(std::cmp::min(1, available_msg_count)));
available_msg_count -= 1
}
unrewarded_relayers.push(unrewarded_relayer_entry(available_msg_count));
unrewarded_relayers.into_iter().collect()
}
#[test]
fn pallet_rejects_transactions_if_halted() {
run_test(|| {
@@ -1719,7 +1683,6 @@ mod tests {
fn proof_size_refund_from_receive_messages_proof_works() {
run_test(|| {
let max_entries = crate::mock::MaxUnrewardedRelayerEntriesAtInboundLane::get() as usize;
let max_msgs = crate::mock::MaxUnconfirmedMessagesAtInboundLane::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
@@ -1734,7 +1697,15 @@ mod tests {
InboundLanes::<TestRuntime>::insert(
TEST_LANE_ID,
StoredInboundLaneData(InboundLaneData {
relayers: unrewarded_relayers_vec(max_entries, max_msgs),
relayers: vec![
UnrewardedRelayer {
relayer: 42,
messages: DeliveredMessages { begin: 0, end: 100 }
};
max_entries
]
.into_iter()
.collect(),
last_confirmed_nonce: 0,
}),
);
@@ -1755,7 +1726,15 @@ mod tests {
InboundLanes::<TestRuntime>::insert(
TEST_LANE_ID,
StoredInboundLaneData(InboundLaneData {
relayers: unrewarded_relayers_vec(max_entries - 1, max_msgs),
relayers: vec![
UnrewardedRelayer {
relayer: 42,
messages: DeliveredMessages { begin: 0, end: 100 }
};
max_entries - 1
]
.into_iter()
.collect(),
last_confirmed_nonce: 0,
}),
);
@@ -1771,7 +1750,7 @@ mod tests {
.unwrap();
assert!(
post_dispatch_weight.proof_size() < pre_dispatch_weight.proof_size(),
"Expected post-dispatch PoV {} to be < than pre-dispatch PoV {}",
"Expected post-dispatch PoV {} to be less than pre-dispatch PoV {}",
post_dispatch_weight.proof_size(),
pre_dispatch_weight.proof_size(),
);
@@ -1787,8 +1766,8 @@ mod tests {
// messages 1+2 are confirmed in 1 tx, message 3 in a separate tx
// dispatch of message 2 has failed
let mut delivered_messages_1_and_2 = DeliveredMessages::new(1, true);
delivered_messages_1_and_2.note_dispatched_message(true);
let mut delivered_messages_1_and_2 = DeliveredMessages::new(1);
delivered_messages_1_and_2.note_dispatched_message();
let messages_1_and_2_proof = Ok((
TEST_LANE_ID,
InboundLaneData {
@@ -1801,7 +1780,7 @@ mod tests {
.collect(),
},
));
let delivered_message_3 = DeliveredMessages::new(3, true);
let delivered_message_3 = DeliveredMessages::new(3);
let messages_3_proof = Ok((
TEST_LANE_ID,
InboundLaneData {
@@ -2092,7 +2071,7 @@ mod tests {
last_confirmed_nonce: 1,
relayers: vec![UnrewardedRelayer {
relayer: 0,
messages: DeliveredMessages::new(1, true),
messages: DeliveredMessages::new(1),
}]
.into_iter()
.collect(),
@@ -2152,39 +2131,39 @@ mod tests {
#[test]
fn inbound_storage_extra_proof_size_bytes_works() {
let max_entries = crate::mock::MaxUnrewardedRelayerEntriesAtInboundLane::get() as usize;
let max_msgs = crate::mock::MaxUnconfirmedMessagesAtInboundLane::get() as usize;
fn relayer_entry() -> UnrewardedRelayer<TestRelayer> {
UnrewardedRelayer { relayer: 42u64, messages: DeliveredMessages { begin: 0, end: 100 } }
}
fn storage(
entry_count: usize,
msg_count: usize,
) -> RuntimeInboundLaneStorage<TestRuntime, ()> {
fn storage(relayer_entries: usize) -> RuntimeInboundLaneStorage<TestRuntime, ()> {
RuntimeInboundLaneStorage {
lane_id: Default::default(),
cached_data: RefCell::new(Some(InboundLaneData {
relayers: unrewarded_relayers_vec(entry_count, msg_count),
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, max_msgs).extra_proof_size_bytes(), 0);
assert_eq!(storage(max_entries).extra_proof_size_bytes(), 0);
// when we have less than `MaxUnrewardedRelayerEntriesAtInboundLane` unrewarded relayers
assert_eq!(
storage(max_entries - 1, max_msgs).extra_proof_size_bytes(),
unrewarded_relayer_entry(1).encoded_size() as u64
storage(max_entries - 1).extra_proof_size_bytes(),
relayer_entry().encode().len() as u64
);
assert_eq!(
storage(max_entries - 2, max_msgs).extra_proof_size_bytes(),
2 * unrewarded_relayer_entry(1).encoded_size() as u64
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)
assert_eq!(storage(max_entries + 1, max_msgs).extra_proof_size_bytes(), 0);
assert_eq!(storage(max_entries + 1).extra_proof_size_bytes(), 0);
}
#[test]
+9 -15
View File
@@ -19,7 +19,6 @@
use crate::Config;
use bitvec::prelude::*;
use bp_messages::{
calc_relayers_rewards,
source_chain::{DeliveryConfirmationPayments, LaneMessageVerifier, TargetHeaderChain},
@@ -63,13 +62,13 @@ pub struct TestPayload {
///
/// Note: in correct code `dispatch_result.unspent_weight` will always be <= `declared_weight`,
/// but for test purposes we'll be making it larger than `declared_weight` sometimes.
pub dispatch_result: MessageDispatchResult<TestDispatchError>,
pub dispatch_result: MessageDispatchResult<TestDispatchLevelResult>,
/// Extra bytes that affect payload size.
pub extra: Vec<u8>,
}
pub type TestMessageFee = u64;
pub type TestRelayer = u64;
pub type TestDispatchError = ();
pub type TestDispatchLevelResult = ();
type Block = frame_system::mocking::MockBlock<TestRuntime>;
type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic<TestRuntime>;
@@ -419,7 +418,7 @@ pub struct TestMessageDispatch;
impl MessageDispatch<AccountId> for TestMessageDispatch {
type DispatchPayload = TestPayload;
type DispatchError = TestDispatchError;
type DispatchLevelResult = TestDispatchLevelResult;
fn dispatch_weight(message: &mut DispatchMessage<TestPayload>) -> Weight {
match message.data.payload.as_ref() {
@@ -431,7 +430,7 @@ impl MessageDispatch<AccountId> for TestMessageDispatch {
fn dispatch(
_relayer_account: &AccountId,
message: DispatchMessage<TestPayload>,
) -> MessageDispatchResult<TestDispatchError> {
) -> MessageDispatchResult<TestDispatchLevelResult> {
match message.data.payload.as_ref() {
Ok(payload) => payload.dispatch_result.clone(),
Err(_) => dispatch_result(0),
@@ -466,10 +465,12 @@ pub const fn message_payload(id: u64, declared_weight: u64) -> TestPayload {
}
/// Returns message dispatch result with given unspent weight.
pub const fn dispatch_result(unspent_weight: u64) -> MessageDispatchResult<TestDispatchError> {
pub const fn dispatch_result(
unspent_weight: u64,
) -> MessageDispatchResult<TestDispatchLevelResult> {
MessageDispatchResult {
unspent_weight: Weight::from_parts(unspent_weight, 0),
dispatch_result: Ok(()),
dispatch_level_result: (),
}
}
@@ -479,14 +480,7 @@ pub fn unrewarded_relayer(
end: MessageNonce,
relayer: TestRelayer,
) -> UnrewardedRelayer<TestRelayer> {
UnrewardedRelayer {
relayer,
messages: DeliveredMessages {
begin,
end,
dispatch_results: bitvec![u8, Msb0; 1; (end + 1).saturating_sub(begin) as _],
},
}
UnrewardedRelayer { relayer, messages: DeliveredMessages { begin, end } }
}
/// Return test externalities to use in tests.
+17 -71
View File
@@ -18,10 +18,8 @@
use crate::Config;
use bitvec::prelude::*;
use bp_messages::{
DeliveredMessages, DispatchResultsBitVec, LaneId, MessageNonce, MessagePayload,
OutboundLaneData, UnrewardedRelayer,
DeliveredMessages, LaneId, MessageNonce, MessagePayload, OutboundLaneData, UnrewardedRelayer,
};
use frame_support::{
weights::{RuntimeDbWeight, Weight},
@@ -67,9 +65,6 @@ pub enum ReceivalConfirmationResult {
/// The unrewarded relayers vec contains non-consecutive entries. May be a result of invalid
/// bridged chain storage.
NonConsecutiveUnrewardedRelayerEntries,
/// The unrewarded relayers vec contains entry with mismatched number of dispatch results. May
/// be a result of invalid bridged chain storage.
InvalidNumberOfDispatchResults,
/// The chain has more messages that need to be confirmed than there is in the proof.
TryingToConfirmMoreMessagesThanExpected(MessageNonce),
}
@@ -129,14 +124,9 @@ impl<S: OutboundLaneStorage> OutboundLane<S> {
)
}
let dispatch_results = match extract_dispatch_results(
data.latest_received_nonce,
latest_delivered_nonce,
relayers,
) {
Ok(dispatch_results) => dispatch_results,
Err(extract_error) => return extract_error,
};
if let Err(e) = ensure_unrewarded_relayers_are_correct(latest_delivered_nonce, relayers) {
return e
}
let prev_latest_received_nonce = data.latest_received_nonce;
data.latest_received_nonce = latest_delivered_nonce;
@@ -145,7 +135,6 @@ impl<S: OutboundLaneStorage> OutboundLane<S> {
ReceivalConfirmationResult::ConfirmedMessages(DeliveredMessages {
begin: prev_latest_received_nonce + 1,
end: latest_delivered_nonce,
dispatch_results,
})
}
@@ -180,34 +169,30 @@ impl<S: OutboundLaneStorage> OutboundLane<S> {
}
}
/// Extract new dispatch results from the unrewarded relayers vec.
/// Verifies unrewarded relayers vec.
///
/// Returns `Err(_)` if unrewarded relayers vec contains invalid data, meaning that the bridged
/// chain has invalid runtime storage.
fn extract_dispatch_results<RelayerId>(
prev_latest_received_nonce: MessageNonce,
fn ensure_unrewarded_relayers_are_correct<RelayerId>(
latest_received_nonce: MessageNonce,
relayers: &VecDeque<UnrewardedRelayer<RelayerId>>,
) -> Result<DispatchResultsBitVec, ReceivalConfirmationResult> {
// the only caller of this functions checks that the
// prev_latest_received_nonce..=latest_received_nonce is valid, so we're ready to accept
// messages in this range => with_capacity call must succeed here or we'll be unable to receive
// confirmations at all
let mut received_dispatch_result =
BitVec::with_capacity((latest_received_nonce - prev_latest_received_nonce + 1) as _);
let mut expected_entry_begin = relayers.front().map(|entry| entry.messages.begin);
) -> Result<(), ReceivalConfirmationResult> {
let mut last_entry_end: Option<MessageNonce> = None;
for entry in relayers {
// unrewarded relayer entry must have at least 1 unconfirmed message
// (guaranteed by the `InboundLane::receive_message()`)
if entry.messages.total_messages() == 0 {
if entry.messages.end < entry.messages.begin {
return Err(ReceivalConfirmationResult::EmptyUnrewardedRelayerEntry)
}
// every entry must confirm range of messages that follows previous entry range
// (guaranteed by the `InboundLane::receive_message()`)
if expected_entry_begin != Some(entry.messages.begin) {
return Err(ReceivalConfirmationResult::NonConsecutiveUnrewardedRelayerEntries)
if let Some(last_entry_end) = last_entry_end {
let expected_entry_begin = last_entry_end.checked_add(1);
if expected_entry_begin != Some(entry.messages.begin) {
return Err(ReceivalConfirmationResult::NonConsecutiveUnrewardedRelayerEntries)
}
}
expected_entry_begin = entry.messages.end.checked_add(1);
last_entry_end = Some(entry.messages.end);
// entry can't confirm messages larger than `inbound_lane_data.latest_received_nonce()`
// (guaranteed by the `InboundLane::receive_message()`)
if entry.messages.end > latest_received_nonce {
@@ -216,30 +201,9 @@ fn extract_dispatch_results<RelayerId>(
// this is detected now
return Err(ReceivalConfirmationResult::FailedToConfirmFutureMessages)
}
// entry must have single dispatch result for every message
// (guaranteed by the `InboundLane::receive_message()`)
if entry.messages.dispatch_results.len() as MessageNonce != entry.messages.total_messages()
{
return Err(ReceivalConfirmationResult::InvalidNumberOfDispatchResults)
}
// now we know that the entry is valid
// => let's check if it brings new confirmations
let new_messages_begin =
sp_std::cmp::max(entry.messages.begin, prev_latest_received_nonce + 1);
if entry.messages.end < new_messages_begin {
continue
}
// now we know that entry brings new confirmations
// => let's extract dispatch results
received_dispatch_result.extend_from_bitslice(
&entry.messages.dispatch_results
[(new_messages_begin - entry.messages.begin) as usize..],
);
}
Ok(received_dispatch_result)
Ok(())
}
#[cfg(test)]
@@ -264,11 +228,7 @@ mod tests {
}
fn delivered_messages(nonces: RangeInclusive<MessageNonce>) -> DeliveredMessages {
DeliveredMessages {
begin: *nonces.start(),
end: *nonces.end(),
dispatch_results: bitvec![u8, Msb0; 1; (nonces.end() - nonces.start() + 1) as _],
}
DeliveredMessages { begin: *nonces.start(), end: *nonces.end() }
}
fn assert_3_messages_confirmation_fails(
@@ -401,20 +361,6 @@ mod tests {
);
}
#[test]
fn confirm_delivery_fails_if_number_of_dispatch_results_in_entry_is_invalid() {
let mut relayers: VecDeque<_> = unrewarded_relayers(1..=1)
.into_iter()
.chain(unrewarded_relayers(2..=2).into_iter())
.chain(unrewarded_relayers(3..=3).into_iter())
.collect();
relayers[0].messages.dispatch_results.clear();
assert_eq!(
assert_3_messages_confirmation_fails(3, &relayers),
ReceivalConfirmationResult::InvalidNumberOfDispatchResults,
);
}
#[test]
fn prune_messages_works() {
run_test(|| {