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

Reward relayers for dispatching messages #385

Merged
merged 16 commits into from
Oct 20, 2020
Merged

Reward relayers for dispatching messages #385

merged 16 commits into from
Oct 20, 2020

Conversation

svyatonik
Copy link
Contributor

related #316
follow up of #339 (II)

This PR introduces scheme which rewards relayers that have delivered messages to the target chain. There are no any transfers - reward logic is hidden behind MessageDeliveryAndDispatchPayment trait, so both "the winner takes all" and election scheme from #316 can be applied.

Main change is that inbound lane state now holds set of relayers which have delivered last messages. Since proof-of-delivery is that state, the set is included into the "delivery race" and is relayed back to the source chain.

The main drawback of this approach is that if there are too may relayed messages, this set may grow indefinitely. This is solved by introducing additional config param MaxUnconfirmedMessagesAtInboundLane. If relayers are not confirming delivery back to the source chain for too long, and there are MaxUnconfirmedMessagesAtInboundLane unconfirmed messages in the inbound lane, then it stops accepting new messages && waits until proof-of-outbound-lane-state will be included into the messages-proof. When updated outbound lane state is received, all rewarded relayers ids are removed from the relayers set. This would require minimal changes into "delivery race" logic of the relayer - I'll add those changes in separate PR.

Copy link
Contributor

@HCastano HCastano left a comment

Choose a reason for hiding this comment

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

Looks fine, but I'd wait until Tomek takes a look before merging

}

for message in lane_data.messages {
debug_assert_eq!(message.key.lane_id, lane_id);
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you mean to keep this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can remove. But changing to something that'll work in release isn't desired, because this group_by is actually happens in the same pallet function.

primitives/message-lane/src/lib.rs Show resolved Hide resolved
use sp_std::{collections::btree_map::BTreeMap, fmt::Debug, prelude::*};

/// Proved messages from the source chain.
pub type ProvedMessages<Message> = BTreeMap<LaneId, ProvedLaneMessages<Message>>;
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm unclear as to what a "proved" message is here. Is it one that's been proven to have been delivered, and thus one who's delivery can be acknowledged back on the source chain so we can reward the relayer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When you relay messages to the target chain, they aren't simply encoded messages. Instead we relay storage-proof of these messages. ProvedMessages are the messages that are proved by that proof (of proved to be in that proof). If you have any other identifier idea - please suggest.

modules/message-lane/src/lib.rs Outdated Show resolved Hide resolved
modules/message-lane/src/lib.rs Show resolved Hide resolved
modules/message-lane/src/lib.rs Outdated Show resolved Hide resolved
modules/message-lane/src/mock.rs Show resolved Hide resolved
@HCastano HCastano requested a review from tomusdrw September 30, 2020 21:56
svyatonik and others added 3 commits October 2, 2020 09:32
Copy link
Contributor

@tomusdrw tomusdrw left a comment

Choose a reason for hiding this comment

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

Looks good, few concerns in comments.

modules/message-lane/src/inbound_lane.rs Outdated Show resolved Hide resolved
primitives/message-lane/src/source_chain.rs Outdated Show resolved Hide resolved
modules/message-lane/src/inbound_lane.rs Outdated Show resolved Hide resolved
primitives/message-lane/src/target_chain.rs Outdated Show resolved Hide resolved
modules/message-lane/src/lib.rs Show resolved Hide resolved
modules/message-lane/src/lib.rs Show resolved Hide resolved
modules/message-lane/src/lib.rs Outdated Show resolved Hide resolved
@tomusdrw tomusdrw mentioned this pull request Oct 6, 2020
Copy link
Contributor

@tomusdrw tomusdrw left a comment

Choose a reason for hiding this comment

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

lgtm!

modules/message-lane/src/lib.rs Outdated Show resolved Hide resolved
svyatonik and others added 3 commits October 19, 2020 15:44
* Add tests for checking messages above max limit

Signed-off-by: MaciejBaj <macie.baj@gmail.com>

* Extend the relayers entry of inbound lane by additional msg nonce

Signed-off-by: MaciejBaj <macie.baj@gmail.com>

* Support additional message nonce from inbound relayers

Signed-off-by: MaciejBaj <macie.baj@gmail.com>

* Code format

Signed-off-by: MaciejBaj <macie.baj@gmail.com>

* Merge messages range for highest relayers

* Change unwrap() to ensure() while accessing relayers

* Edit rustdocs for relayers deque at inbound lane data

Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>

* Declare additional relayers A & B and use across tests consistently

Signed-off-by: MaciejBaj <macie.baj@gmail.com>

* Remove duplicates and improve naming for inbound lane tests

* Fix test checking max limit per inbound lane

* Correct relayers rewards loop after a proof is received

* Remove redundant check for messages ahead of received range

* Correct grammar at inbound lane tests rustdocs

Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>

* Improve code quality of relayers updates 💅

Co-authored-by: Tomasz Drwięga <tomusdrw@users.noreply.github.com>

* Test dispatches above max limit from same relayer

Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>
Co-authored-by: Tomasz Drwięga <tomusdrw@users.noreply.github.com>
@svyatonik svyatonik merged commit 924d764 into master Oct 20, 2020
@svyatonik svyatonik deleted the reward-relayers branch October 20, 2020 10:12
serban300 pushed a commit to serban300/parity-bridges-common that referenced this pull request Mar 27, 2024
* reward relayers for dispatching messages

* clippy

* Update modules/message-lane/src/lib.rs

Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>

* added comment

* Update modules/message-lane/src/inbound_lane.rs

Co-authored-by: Tomasz Drwięga <tomusdrw@users.noreply.github.com>

* Update modules/message-lane/src/inbound_lane.rs

Co-authored-by: Tomasz Drwięga <tomusdrw@users.noreply.github.com>

* SubmitterId + RelayerId -> AccountId

* add confirmation_relayer arg to pay_relayer_reward

* cargo fmt --all

* removed verify_and_decode_messages_proof from SourceHeaderChain

* &mut self -> RefCell

* Optimize max messages at inbound lane (paritytech#418)

* Add tests for checking messages above max limit

Signed-off-by: MaciejBaj <macie.baj@gmail.com>

* Extend the relayers entry of inbound lane by additional msg nonce

Signed-off-by: MaciejBaj <macie.baj@gmail.com>

* Support additional message nonce from inbound relayers

Signed-off-by: MaciejBaj <macie.baj@gmail.com>

* Code format

Signed-off-by: MaciejBaj <macie.baj@gmail.com>

* Merge messages range for highest relayers

* Change unwrap() to ensure() while accessing relayers

* Edit rustdocs for relayers deque at inbound lane data

Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>

* Declare additional relayers A & B and use across tests consistently

Signed-off-by: MaciejBaj <macie.baj@gmail.com>

* Remove duplicates and improve naming for inbound lane tests

* Fix test checking max limit per inbound lane

* Correct relayers rewards loop after a proof is received

* Remove redundant check for messages ahead of received range

* Correct grammar at inbound lane tests rustdocs

Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>

* Improve code quality of relayers updates 💅

Co-authored-by: Tomasz Drwięga <tomusdrw@users.noreply.github.com>

* Test dispatches above max limit from same relayer

Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>
Co-authored-by: Tomasz Drwięga <tomusdrw@users.noreply.github.com>

* Fix typo.

Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>
Co-authored-by: Tomasz Drwięga <tomusdrw@users.noreply.github.com>
Co-authored-by: Maciej Baj <macie.baj@gmail.com>
Co-authored-by: Tomasz Drwięga <tomasz@parity.io>
serban300 pushed a commit to serban300/parity-bridges-common that referenced this pull request Apr 8, 2024
* reward relayers for dispatching messages

* clippy

* Update modules/message-lane/src/lib.rs

Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>

* added comment

* Update modules/message-lane/src/inbound_lane.rs

Co-authored-by: Tomasz Drwięga <tomusdrw@users.noreply.github.com>

* Update modules/message-lane/src/inbound_lane.rs

Co-authored-by: Tomasz Drwięga <tomusdrw@users.noreply.github.com>

* SubmitterId + RelayerId -> AccountId

* add confirmation_relayer arg to pay_relayer_reward

* cargo fmt --all

* removed verify_and_decode_messages_proof from SourceHeaderChain

* &mut self -> RefCell

* Optimize max messages at inbound lane (paritytech#418)

* Add tests for checking messages above max limit

Signed-off-by: MaciejBaj <macie.baj@gmail.com>

* Extend the relayers entry of inbound lane by additional msg nonce

Signed-off-by: MaciejBaj <macie.baj@gmail.com>

* Support additional message nonce from inbound relayers

Signed-off-by: MaciejBaj <macie.baj@gmail.com>

* Code format

Signed-off-by: MaciejBaj <macie.baj@gmail.com>

* Merge messages range for highest relayers

* Change unwrap() to ensure() while accessing relayers

* Edit rustdocs for relayers deque at inbound lane data

Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>

* Declare additional relayers A & B and use across tests consistently

Signed-off-by: MaciejBaj <macie.baj@gmail.com>

* Remove duplicates and improve naming for inbound lane tests

* Fix test checking max limit per inbound lane

* Correct relayers rewards loop after a proof is received

* Remove redundant check for messages ahead of received range

* Correct grammar at inbound lane tests rustdocs

Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>

* Improve code quality of relayers updates 💅

Co-authored-by: Tomasz Drwięga <tomusdrw@users.noreply.github.com>

* Test dispatches above max limit from same relayer

Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>
Co-authored-by: Tomasz Drwięga <tomusdrw@users.noreply.github.com>

* Fix typo.

Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>
Co-authored-by: Tomasz Drwięga <tomusdrw@users.noreply.github.com>
Co-authored-by: Maciej Baj <macie.baj@gmail.com>
Co-authored-by: Tomasz Drwięga <tomasz@parity.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants