Skip to content

Relay basic single-bit message dispatch results back to the source chain #935

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.

Sign up for GitHub

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

Merged
svyatonik merged 10 commits into from
Jun 21, 2021
Merged

Relay basic single-bit message dispatch results back to the source chain #935

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
f7171f1
relay dispatch result flags back to the source chain
svyatonik Apr 27, 2021
61ec89e
OnMessagesDelivered callback
svyatonik Apr 28, 2021
2110b9a
add lane id to OnDeliveredMessages callback
svyatonik Apr 28, 2021
7377557
fix benchmarks && upate weights
svyatonik Jun 18, 2021
b43bc21
clippy
svyatonik Jun 18, 2021
71be9ed
clippy
svyatonik Jun 18, 2021
bd88510
clipy another try
svyatonik Jun 18, 2021
15b3acf
OnMessagesDelivered -> OnDeliveryConfirmed
svyatonik Jun 21, 2021
d231c82
Update primitives/messages/src/source_chain.rs
svyatonik Jun 21, 2021
858e170
Merge branch 'relay-basic-dispatch-results' of https://github.com/par…
svyatonik Jun 21, 2021
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
Prev Previous commit
Next Next commit
add lane id to OnDeliveredMessages callback
  • Loading branch information
@svyatonik
svyatonik committed Jun 18, 2021
commit 2110b9afb42263f63c305192cb5603ca5c4c4a8c
10 changes: 5 additions & 5 deletions modules/messages/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -641,7 +641,7 @@ decl_module! {
};
if let Some(confirmed_messages) = confirmed_messages {
// handle messages delivery
T::OnMessagesDelivered::on_messages_delivered(&confirmed_messages);
T::OnMessagesDelivered::on_messages_delivered(&lane_id, &confirmed_messages);

// emit 'delivered' event
let received_range = confirmed_messages.begin..=confirmed_messages.end;
Expand Down Expand Up @@ -1909,10 +1909,10 @@ mod tests {
));

// ensure that both callbacks have been called twice: for 1+2, then for 3
crate::mock::TestOnMessagesDelivered1::ensure_called(&delivered_messages_1_and_2);
crate::mock::TestOnMessagesDelivered1::ensure_called(&delivered_message_3);
crate::mock::TestOnMessagesDelivered2::ensure_called(&delivered_messages_1_and_2);
crate::mock::TestOnMessagesDelivered2::ensure_called(&delivered_message_3);
crate::mock::TestOnMessagesDelivered1::ensure_called(&TEST_LANE_ID, &delivered_messages_1_and_2);
crate::mock::TestOnMessagesDelivered1::ensure_called(&TEST_LANE_ID, &delivered_message_3);
crate::mock::TestOnMessagesDelivered2::ensure_called(&TEST_LANE_ID, &delivered_messages_1_and_2);
crate::mock::TestOnMessagesDelivered2::ensure_called(&TEST_LANE_ID, &delivered_message_3);
});
}
}
16 changes: 8 additions & 8 deletions modules/messages/src/mock.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,15 +354,15 @@ pub struct TestOnMessagesDelivered1;

impl TestOnMessagesDelivered1 {
/// Verify that the callback has been called with given delivered messages.
pub fn ensure_called(messages: &DeliveredMessages) {
let key = (b"TestOnMessagesDelivered1", messages).encode();
pub fn ensure_called(lane: &LaneId, messages: &DeliveredMessages) {
let key = (b"TestOnMessagesDelivered1", lane, messages).encode();
assert_eq!(frame_support::storage::unhashed::get(&key), Some(true));
}
}

impl OnMessagesDelivered for TestOnMessagesDelivered1 {
fn on_messages_delivered(messages: &DeliveredMessages) {
let key = (b"TestOnMessagesDelivered1", messages).encode();
fn on_messages_delivered(lane: &LaneId, messages: &DeliveredMessages) {
let key = (b"TestOnMessagesDelivered1", lane, messages).encode();
frame_support::storage::unhashed::put(&key, &true);
}
}
Expand All @@ -373,15 +373,15 @@ pub struct TestOnMessagesDelivered2;

impl TestOnMessagesDelivered2 {
/// Verify that the callback has been called with given delivered messages.
pub fn ensure_called(messages: &DeliveredMessages) {
let key = (b"TestOnMessagesDelivered2", messages).encode();
pub fn ensure_called(lane: &LaneId, messages: &DeliveredMessages) {
let key = (b"TestOnMessagesDelivered2", lane, messages).encode();
assert_eq!(frame_support::storage::unhashed::get(&key), Some(true));
}
}

impl OnMessagesDelivered for TestOnMessagesDelivered2 {
fn on_messages_delivered(messages: &DeliveredMessages) {
let key = (b"TestOnMessagesDelivered2", messages).encode();
fn on_messages_delivered(lane: &LaneId, messages: &DeliveredMessages) {
let key = (b"TestOnMessagesDelivered2", lane, messages).encode();
frame_support::storage::unhashed::put(&key, &true);
}
}
Expand Down
2 changes: 1 addition & 1 deletion primitives/messages/src/source_chain.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ pub trait OnMessagesDelivered {
/// Called when we receive confirmation that our messages have been delivered to the
/// target chain. The confirmation aso has single bit dispatch result for every
/// confirmed message (see `DeliveredMessages` for details).
fn on_messages_delivered(_messages: &DeliveredMessages) {}
fn on_messages_delivered(_lane: &LaneId, _messages: &DeliveredMessages) {}
Copy link
Contributor Author

@svyatonik svyatonik Apr 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This'll need to do something with weights - I'm not yet sure how to deal with this. Change benchmarks to worst-case result and then refund? But what is worst case when there are e.g. multiple apps using different lanes for their purposes. And because of refund policy we'll need to decrease max number of messages in the confirmation tx. Probably just reserve single read and single write for the every registered callback? I'm not yet sure - will leave it for future PRs as well

Copy link
Contributor

@tomusdrw tomusdrw Jun 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think the general sentiment about these kind of handlers is that they should be as simple as possible. Having one read + one write seems reasonable. In future we can prepare some tooling to kind of "subscribe" to interesting messages and for instance have them processed in on_initialize of the pallet or some manual trigger (extrinsic).

Might be good to add this assumption to the docs though, so that implementers are aware of that.

Copy link
Contributor Author

@svyatonik svyatonik Jun 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#1012

}

/// Structure that may be used in place of `TargetHeaderChain`, `LaneMessageVerifier` and
Expand Down