Skip to content

Commit

Permalink
Remove redundant data from InboundLaneData (paritytech#638)
Browse files Browse the repository at this point in the history 
* Remove latest_*_nonce.

* cargo fmt --all

* Fix tests.

* cargo fmt --all

* Fix benchmarking.

* Update docs.
  • Loading branch information
@tomusdrw @serban300
tomusdrw authored and serban300 committed Apr 9, 2024
1 parent d43da82 commit e551023
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 63 deletions.
18 changes: 6 additions & 12 deletions bridges/modules/message-lane/src/benchmarking.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -311,8 +311,7 @@ benchmarks_instance! {
lane: bench_lane_id(),
inbound_lane_data: InboundLaneData {
relayers: vec![(1, 1, relayer_id.clone())].into_iter().collect(),
latest_received_nonce: 1,
latest_confirmed_nonce: 0,
last_confirmed_nonce: 0,
}
});
}: receive_messages_delivery_proof(RawOrigin::Signed(relayer_id.clone()), proof, relayers_state)
Expand Down Expand Up @@ -349,8 +348,7 @@ benchmarks_instance! {
lane: bench_lane_id(),
inbound_lane_data: InboundLaneData {
relayers: vec![(1, 2, relayer_id.clone())].into_iter().collect(),
latest_received_nonce: 2,
latest_confirmed_nonce: 0,
last_confirmed_nonce: 0,
}
});
}: receive_messages_delivery_proof(RawOrigin::Signed(relayer_id.clone()), proof, relayers_state)
Expand Down Expand Up @@ -392,8 +390,7 @@ benchmarks_instance! {
(1, 1, relayer1_id.clone()),
(2, 2, relayer2_id.clone()),
].into_iter().collect(),
latest_received_nonce: 2,
latest_confirmed_nonce: 0,
last_confirmed_nonce: 0,
}
});
}: receive_messages_delivery_proof(RawOrigin::Signed(relayer1_id.clone()), proof, relayers_state)
Expand Down Expand Up @@ -552,8 +549,7 @@ benchmarks_instance! {
lane: bench_lane_id(),
inbound_lane_data: InboundLaneData {
relayers: vec![(1, i as MessageNonce, relayer_id.clone())].into_iter().collect(),
latest_received_nonce: i as MessageNonce,
latest_confirmed_nonce: 0,
last_confirmed_nonce: 0,
}
});
}: receive_messages_delivery_proof(RawOrigin::Signed(relayer_id.clone()), proof, relayers_state)
Expand Down Expand Up @@ -601,8 +597,7 @@ benchmarks_instance! {
.enumerate()
.map(|(j, relayer_id)| (j as MessageNonce + 1, j as MessageNonce + 1, relayer_id.clone()))
.collect(),
latest_received_nonce: i as MessageNonce,
latest_confirmed_nonce: 0,
last_confirmed_nonce: 0,
}
});
}: receive_messages_delivery_proof(RawOrigin::Signed(confirmation_relayer_id), proof, relayers_state)
Expand Down Expand Up @@ -637,7 +632,6 @@ fn receive_messages<T: Config<I>, I: Instance>(nonce: MessageNonce) {
let mut inbound_lane_storage = inbound_lane_storage::<T, I>(bench_lane_id());
inbound_lane_storage.set_data(InboundLaneData {
relayers: vec![(1, nonce, T::bridged_relayer_id())].into_iter().collect(),
latest_received_nonce: nonce,
latest_confirmed_nonce: 0,
last_confirmed_nonce: 0,
});
}
42 changes: 21 additions & 21 deletions bridges/modules/message-lane/src/inbound_lane.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,29 +55,32 @@ impl<S: InboundLaneStorage> InboundLane<S> {
/// Receive state of the corresponding outbound lane.
pub fn receive_state_update(&mut self, outbound_lane_data: OutboundLaneData) -> Option<MessageNonce> {
let mut data = self.storage.data();
if outbound_lane_data.latest_received_nonce > data.latest_received_nonce {
let last_delivered_nonce = data.last_delivered_nonce();

if outbound_lane_data.latest_received_nonce > last_delivered_nonce {
// this is something that should never happen if proofs are correct
return None;
}
if outbound_lane_data.latest_received_nonce <= data.latest_confirmed_nonce {
if outbound_lane_data.latest_received_nonce <= data.last_confirmed_nonce {
return None;
}

data.latest_confirmed_nonce = outbound_lane_data.latest_received_nonce;
let new_confirmed_nonce = outbound_lane_data.latest_received_nonce;
data.last_confirmed_nonce = new_confirmed_nonce;
// Firstly, remove all of the records where higher nonce <= new confirmed nonce
while data
.relayers
.front()
.map(|(_, nonce_high, _)| *nonce_high <= data.latest_confirmed_nonce)
.map(|(_, nonce_high, _)| *nonce_high <= new_confirmed_nonce)
.unwrap_or(false)
{
data.relayers.pop_front();
}
// Secondly, update the next record with lower nonce equal to new confirmed nonce if needed.
// Note: There will be max. 1 record to update as we don't allow messages from relayers to overlap.
match data.relayers.front_mut() {
Some((nonce_low, _, _)) if *nonce_low < data.latest_confirmed_nonce => {
*nonce_low = data.latest_confirmed_nonce + 1;
Some((nonce_low, _, _)) if *nonce_low < new_confirmed_nonce => {
*nonce_low = new_confirmed_nonce + 1;
}
_ => {}
}
Expand All @@ -94,7 +97,7 @@ impl<S: InboundLaneStorage> InboundLane<S> {
message_data: DispatchMessageData<P::DispatchPayload, S::MessageFee>,
) -> bool {
let mut data = self.storage.data();
let is_correct_message = nonce == data.latest_received_nonce + 1;
let is_correct_message = nonce == data.last_delivered_nonce() + 1;
if !is_correct_message {
return false;
}
Expand All @@ -105,13 +108,11 @@ impl<S: InboundLaneStorage> InboundLane<S> {
}

// if there are more unconfirmed messages than we may accept, reject this message
let unconfirmed_messages_count = nonce.saturating_sub(data.latest_confirmed_nonce);
let unconfirmed_messages_count = nonce.saturating_sub(data.last_confirmed_nonce);
if unconfirmed_messages_count > self.storage.max_unconfirmed_messages() {
return false;
}

data.latest_received_nonce = nonce;

let push_new = match data.relayers.back_mut() {
Some((_, nonce_high, last_relayer)) if last_relayer == &relayer => {
*nonce_high = nonce;
Expand Down Expand Up @@ -173,7 +174,7 @@ mod tests {
None,
);

assert_eq!(lane.storage.data().latest_confirmed_nonce, 0);
assert_eq!(lane.storage.data().last_confirmed_nonce, 0);
});
}

Expand All @@ -191,7 +192,7 @@ mod tests {
}),
Some(3),
);
assert_eq!(lane.storage.data().latest_confirmed_nonce, 3);
assert_eq!(lane.storage.data().last_confirmed_nonce, 3);

assert_eq!(
lane.receive_state_update(OutboundLaneData {
Expand All @@ -200,7 +201,7 @@ mod tests {
}),
None,
);
assert_eq!(lane.storage.data().latest_confirmed_nonce, 3);
assert_eq!(lane.storage.data().last_confirmed_nonce, 3);
});
}

Expand All @@ -211,7 +212,7 @@ mod tests {
receive_regular_message(&mut lane, 1);
receive_regular_message(&mut lane, 2);
receive_regular_message(&mut lane, 3);
assert_eq!(lane.storage.data().latest_confirmed_nonce, 0);
assert_eq!(lane.storage.data().last_confirmed_nonce, 0);
assert_eq!(lane.storage.data().relayers, vec![(1, 3, TEST_RELAYER_A)]);

assert_eq!(
Expand All @@ -221,7 +222,7 @@ mod tests {
}),
Some(2),
);
assert_eq!(lane.storage.data().latest_confirmed_nonce, 2);
assert_eq!(lane.storage.data().last_confirmed_nonce, 2);
assert_eq!(lane.storage.data().relayers, vec![(3, 3, TEST_RELAYER_A)]);

assert_eq!(
Expand All @@ -231,7 +232,7 @@ mod tests {
}),
Some(3),
);
assert_eq!(lane.storage.data().latest_confirmed_nonce, 3);
assert_eq!(lane.storage.data().last_confirmed_nonce, 3);
assert_eq!(lane.storage.data().relayers, vec![]);
});
}
Expand All @@ -242,8 +243,7 @@ mod tests {
let mut lane = inbound_lane::<TestRuntime, _>(TEST_LANE_ID);
let mut seed_storage_data = lane.storage.data();
// Prepare data
seed_storage_data.latest_confirmed_nonce = 0;
seed_storage_data.latest_received_nonce = 5;
seed_storage_data.last_confirmed_nonce = 0;
seed_storage_data.relayers.push_back((1, 1, TEST_RELAYER_A));
// Simulate messages batch (2, 3, 4) from relayer #2
seed_storage_data.relayers.push_back((2, 4, TEST_RELAYER_B));
Expand All @@ -257,7 +257,7 @@ mod tests {
}),
Some(3),
);
assert_eq!(lane.storage.data().latest_confirmed_nonce, 3);
assert_eq!(lane.storage.data().last_confirmed_nonce, 3);
assert_eq!(
lane.storage.data().relayers,
vec![(4, 4, TEST_RELAYER_B), (5, 5, TEST_RELAYER_C)]
Expand All @@ -274,7 +274,7 @@ mod tests {
10,
message_data(REGULAR_PAYLOAD).into()
));
assert_eq!(lane.storage.data().latest_received_nonce, 0);
assert_eq!(lane.storage.data().last_delivered_nonce(), 0);
});
}

Expand Down Expand Up @@ -391,7 +391,7 @@ mod tests {
run_test(|| {
let mut lane = inbound_lane::<TestRuntime, _>(TEST_LANE_ID);
receive_regular_message(&mut lane, 1);
assert_eq!(lane.storage.data().latest_received_nonce, 1);
assert_eq!(lane.storage.data().last_delivered_nonce(), 1);
});
}
}
35 changes: 18 additions & 17 deletions bridges/modules/message-lane/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,8 @@ decl_module! {

// mark messages as delivered
let mut lane = outbound_lane::<T, I>(lane_id);
let received_range = lane.confirm_delivery(lane_data.latest_received_nonce);
let last_delivered_nonce = lane_data.last_delivered_nonce();
let received_range = lane.confirm_delivery(last_delivered_nonce);
if let Some(received_range) = received_range {
Self::deposit_event(RawEvent::MessagesDelivered(lane_id, received_range.0, received_range.1));

Expand Down Expand Up @@ -499,7 +500,7 @@ decl_module! {

frame_support::debug::trace!(
"Received messages delivery proof up to (and including) {} at lane {:?}",
lane_data.latest_received_nonce,
last_delivered_nonce,
lane_id,
);

Expand All @@ -526,12 +527,12 @@ impl<T: Config<I>, I: Instance> Module<T, I> {

/// Get nonce of latest received message at given inbound lane.
pub fn inbound_latest_received_nonce(lane: LaneId) -> MessageNonce {
InboundLanes::<T, I>::get(&lane).latest_received_nonce
InboundLanes::<T, I>::get(&lane).last_delivered_nonce()
}

/// Get nonce of latest confirmed message at given inbound lane.
pub fn inbound_latest_confirmed_nonce(lane: LaneId) -> MessageNonce {
InboundLanes::<T, I>::get(&lane).latest_confirmed_nonce
InboundLanes::<T, I>::get(&lane).last_confirmed_nonce
}

/// Get state of unrewarded relayers set.
Expand Down Expand Up @@ -795,7 +796,7 @@ mod tests {
Ok((
TEST_LANE_ID,
InboundLaneData {
latest_received_nonce: 1,
last_confirmed_nonce: 1,
..Default::default()
},
)),
Expand Down Expand Up @@ -905,7 +906,7 @@ mod tests {
Ok((
TEST_LANE_ID,
InboundLaneData {
latest_received_nonce: 1,
last_confirmed_nonce: 1,
..Default::default()
},
)),
Expand Down Expand Up @@ -977,7 +978,7 @@ mod tests {
REGULAR_PAYLOAD.1,
));

assert_eq!(InboundLanes::<TestRuntime>::get(TEST_LANE_ID).latest_received_nonce, 1);
assert_eq!(InboundLanes::<TestRuntime>::get(TEST_LANE_ID).last_delivered_nonce(), 1);
});
}

Expand All @@ -988,8 +989,7 @@ mod tests {
InboundLanes::<TestRuntime, DefaultInstance>::insert(
TEST_LANE_ID,
InboundLaneData {
latest_confirmed_nonce: 8,
latest_received_nonce: 10,
last_confirmed_nonce: 8,
relayers: vec![(9, 9, TEST_RELAYER_A), (10, 10, TEST_RELAYER_B)]
.into_iter()
.collect(),
Expand Down Expand Up @@ -1022,11 +1022,10 @@ mod tests {
assert_eq!(
InboundLanes::<TestRuntime>::get(TEST_LANE_ID),
InboundLaneData {
last_confirmed_nonce: 9,
relayers: vec![(10, 10, TEST_RELAYER_B), (11, 11, TEST_RELAYER_A)]
.into_iter()
.collect(),
latest_received_nonce: 11,
latest_confirmed_nonce: 9,
},
);
assert_eq!(
Expand Down Expand Up @@ -1108,7 +1107,6 @@ mod tests {
TEST_LANE_ID,
InboundLaneData {
relayers: vec![(1, 1, TEST_RELAYER_A)].into_iter().collect(),
latest_received_nonce: 1,
..Default::default()
}
)),
Expand Down Expand Up @@ -1136,7 +1134,6 @@ mod tests {
relayers: vec![(1, 1, TEST_RELAYER_A), (2, 2, TEST_RELAYER_B)]
.into_iter()
.collect(),
latest_received_nonce: 2,
..Default::default()
}
)),
Expand Down Expand Up @@ -1180,7 +1177,6 @@ mod tests {
relayers: vec![(1, 1, TEST_RELAYER_A), (2, 2, TEST_RELAYER_B)]
.into_iter()
.collect(),
latest_received_nonce: 2,
..Default::default()
}
)),
Expand All @@ -1203,7 +1199,6 @@ mod tests {
relayers: vec![(1, 1, TEST_RELAYER_A), (2, 2, TEST_RELAYER_B)]
.into_iter()
.collect(),
latest_received_nonce: 2,
..Default::default()
}
)),
Expand Down Expand Up @@ -1232,7 +1227,10 @@ mod tests {
0, // weight may be zero in this case (all messages are improperly encoded)
),);

assert_eq!(InboundLanes::<TestRuntime>::get(&TEST_LANE_ID).latest_received_nonce, 1,);
assert_eq!(
InboundLanes::<TestRuntime>::get(&TEST_LANE_ID).last_delivered_nonce(),
1,
);
});
}

Expand All @@ -1255,7 +1253,10 @@ mod tests {
REGULAR_PAYLOAD.1 + REGULAR_PAYLOAD.1,
),);

assert_eq!(InboundLanes::<TestRuntime>::get(&TEST_LANE_ID).latest_received_nonce, 3,);
assert_eq!(
InboundLanes::<TestRuntime>::get(&TEST_LANE_ID).last_delivered_nonce(),
3,
);
});
}

Expand Down
Loading

0 comments on commit e551023

Please sign in to comment.