Skip to content
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

Remove dispatch_result field #1660

Merged
merged 5 commits into from
Nov 22, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion bin/millau/runtime/src/rialto_messages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,6 @@ mod tests {
let max_incoming_inbound_lane_data_proof_size =
bp_messages::InboundLaneData::<()>::encoded_size_hint_u32(
bp_millau::MAX_UNREWARDED_RELAYERS_IN_CONFIRMATION_TX as _,
bp_millau::MAX_UNCONFIRMED_MESSAGES_IN_CONFIRMATION_TX as _,
);
pallet_bridge_messages::ensure_able_to_receive_confirmation::<Weights>(
bp_millau::Millau::max_extrinsic_size(),
Expand Down
1 change: 0 additions & 1 deletion bin/millau/runtime/src/xcm_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,6 @@ mod tests {
assert_eq!(
dispatch_result,
MessageDispatchResult {
dispatch_result: true,
unspent_weight: frame_support::weights::Weight::from_ref_time(0),
dispatch_fee_paid_during_dispatch: false,
}
Expand Down
1 change: 0 additions & 1 deletion bin/rialto-parachain/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -895,7 +895,6 @@ mod tests {
assert_eq!(
dispatch_result,
MessageDispatchResult {
dispatch_result: true,
unspent_weight: frame_support::weights::Weight::from_ref_time(0),
dispatch_fee_paid_during_dispatch: false,
}
Expand Down
1 change: 0 additions & 1 deletion bin/rialto/runtime/src/millau_messages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,6 @@ mod tests {
let max_incoming_inbound_lane_data_proof_size =
bp_messages::InboundLaneData::<()>::encoded_size_hint_u32(
bp_rialto::MAX_UNREWARDED_RELAYERS_IN_CONFIRMATION_TX as _,
bp_rialto::MAX_UNCONFIRMED_MESSAGES_IN_CONFIRMATION_TX as _,
);
pallet_bridge_messages::ensure_able_to_receive_confirmation::<Weights>(
bp_rialto::Rialto::max_extrinsic_size(),
Expand Down
1 change: 0 additions & 1 deletion bin/rialto/runtime/src/xcm_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,6 @@ mod tests {
assert_eq!(
dispatch_result,
MessageDispatchResult {
dispatch_result: true,
unspent_weight: frame_support::weights::Weight::from_ref_time(0),
dispatch_fee_paid_during_dispatch: false,
}
Expand Down
1 change: 0 additions & 1 deletion bin/runtime-common/src/messages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,6 @@ pub mod target {
xcm_outcome,
);
MessageDispatchResult {
dispatch_result: true,
unspent_weight: Weight::zero(),
dispatch_fee_paid_during_dispatch: false,
}
Expand Down
1 change: 0 additions & 1 deletion modules/messages/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ edition = "2021"
license = "GPL-3.0-or-later WITH Classpath-exception-2.0"

[dependencies]
bitvec = { version = "1", default-features = false, features = ["alloc"] }
codec = { package = "parity-scale-codec", version = "3.1.5", default-features = false }
log = { version = "0.4.17", default-features = false }
num-traits = { version = "0.2", default-features = false }
Expand Down
9 changes: 2 additions & 7 deletions modules/messages/src/inbound_lane.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -150,10 +149,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
.split_off((new_confirmed_nonce + 1 - entry.messages.begin) as _);
entry.messages.begin = new_confirmed_nonce + 1;
},
_ => {},
Expand Down Expand Up @@ -200,15 +195,15 @@ impl<S: InboundLaneStorage> InboundLane<S> {
// now let's update inbound lane storage
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);
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, dispatch_result.dispatch_result),
messages: DeliveredMessages::new(nonce),
});
}
self.storage.set_data(data);
Expand Down
10 changes: 5 additions & 5 deletions modules/messages/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -987,7 +987,7 @@ mod tests {
last_confirmed_nonce: 1,
relayers: vec![UnrewardedRelayer {
relayer: 0,
messages: DeliveredMessages::new(1, true),
messages: DeliveredMessages::new(1),
}]
.into_iter()
.collect(),
Expand All @@ -1007,7 +1007,7 @@ 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![],
}],
Expand Down Expand Up @@ -1622,8 +1622,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(false);
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 {
Expand All @@ -1636,7 +1636,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 {
Expand Down
15 changes: 1 addition & 14 deletions modules/messages/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

use crate::{calc_relayers_rewards, Config};

use bitvec::prelude::*;
use bp_messages::{
source_chain::{LaneMessageVerifier, MessageDeliveryAndDispatchPayment, TargetHeaderChain},
target_chain::{
Expand Down Expand Up @@ -394,7 +393,6 @@ 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 {
MessageDispatchResult {
dispatch_result: true,
unspent_weight: Weight::from_ref_time(unspent_weight),
dispatch_fee_paid_during_dispatch: true,
}
Expand All @@ -406,18 +404,7 @@ pub fn unrewarded_relayer(
end: MessageNonce,
relayer: TestRelayer,
) -> UnrewardedRelayer<TestRelayer> {
UnrewardedRelayer {
relayer,
messages: DeliveredMessages {
begin,
end,
dispatch_results: if end >= begin {
bitvec![u8, Msb0; 1; (end - begin + 1) as _]
} else {
Default::default()
},
},
}
UnrewardedRelayer { relayer, messages: DeliveredMessages { begin, end } }
}

/// Run pallet test.
Expand Down
78 changes: 9 additions & 69 deletions modules/messages/src/outbound_lane.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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),
}
Expand Down Expand Up @@ -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;
Expand All @@ -145,7 +135,6 @@ impl<S: OutboundLaneStorage> OutboundLane<S> {
ReceivalConfirmationResult::ConfirmedMessages(DeliveredMessages {
begin: prev_latest_received_nonce + 1,
end: latest_delivered_nonce,
dispatch_results,
})
}

Expand Down Expand Up @@ -180,21 +169,14 @@ 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 _);
) -> Result<(), ReceivalConfirmationResult> {
let mut last_entry_end: Option<MessageNonce> = None;
for entry in relayers {
// unrewarded relayer entry must have at least 1 unconfirmed message
Expand All @@ -219,33 +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.end - entry.messages.begin + 1
{
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);
let new_messages_end = sp_std::cmp::min(entry.messages.end, latest_received_nonce);
let new_messages_range = new_messages_begin..=new_messages_end;
if new_messages_range.is_empty() {
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)]
Expand All @@ -270,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(
Expand Down Expand Up @@ -407,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(|| {
Expand Down
2 changes: 0 additions & 2 deletions primitives/messages/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ edition = "2021"
license = "GPL-3.0-or-later WITH Classpath-exception-2.0"

[dependencies]
bitvec = { version = "1", default-features = false, features = ["alloc"] }
codec = { package = "parity-scale-codec", version = "3.1.5", default-features = false, features = ["derive", "bit-vec"] }
impl-trait-for-tuples = "0.2"
scale-info = { version = "2.1.1", default-features = false, features = ["bit-vec", "derive"] }
Expand All @@ -30,7 +29,6 @@ hex-literal = "0.3"
[features]
default = ["std"]
std = [
"bitvec/std",
"bp-runtime/std",
"codec/std",
"frame-support/std",
Expand Down
Loading