Skip to content

pay dispatch fee at target chain#911

Merged
svyatonik merged 16 commits intomasterfrom
pay-dispatch-fee-at-target-chain
Jun 18, 2021
Merged

pay dispatch fee at target chain#911
svyatonik merged 16 commits intomasterfrom
pay-dispatch-fee-at-target-chain

Conversation

@svyatonik
Copy link
Copy Markdown
Contributor

@svyatonik svyatonik commented Apr 19, 2021

closes #377
closes #561

Things to do in this PR (quite a lot - opening just to show some progress):

  • actually return unspent weight from delivery call;
  • test that transfer actually happens when using our messages adapter;
  • update benchmarks - the worst delivery case is now when fee is paid at the target chain
  • (most probably) weight of pay-dispatch-fee-at-target must not be paid by submitter when message is prepaid at source + we should refund relayer with this fee if message is already prepaid in delivery tx. This would require additional constant in chain primitives, which is a bit unfortunate.

Things to do in follow-up PRs:

  • support pay-at-target-chain option in encode-call, send-message and other subcommands;
  • actually generate pay-at-target-chain messages in test deployments.

@svyatonik svyatonik self-assigned this Apr 19, 2021
Copy link
Copy Markdown
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.

Nice!

Comment on lines +110 to +112
/// 1) if message has been dispatched successfully, but post-dispatch weight is less than
/// the weight, declared by the message sender;
/// 2) if message has not been dispatched at all.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I was thinking a bit about burning part of that weight, or returning part of the fee to the message sender, but I concluded that it doesn't make much sense for now. Leaving the comment here for posterity though.

  1. In case of target-chain fee payment, messages senders are heavily incentivised to specify the lowest possible weight, cause they pay up-front. Any unused weight is benefiting the relayers now by increasing their reward for accepting target-chain fee payment, which inherently increases the risk for them. So that seems to be quite aligned.
  2. In case of source-chain fee payment, the situation is quite similar - in such case relayers risk is increased by price fluctuation though.
  3. Hence message senders does not really benefit directly from having the dispatch consume less weight than declared, cause they pay for dispatch upfront. Burning part of the unused weight would only reduce reward for the relayers (which are exposed to a bunch of risks already) and would unnecessarily inflate block consumption.
  4. Returning part of unused fee to senders reduces their incentive to calculate the fee correctly and increases risk for the relayers.

Overall we might consider doing a more detailed analysis, perhaps there is some fair split between returning/burning/leaving to the relayer, but taking into account real-world data will be a great help for this, so I'd rather collect it first :)
My gut feeling is that relayers are more on the upside here, but I think it's acceptable for the first deployment.

@svyatonik
Copy link
Copy Markdown
Contributor Author

Some notes about weight of the delivery transaction - it has been increased from 1_000_000_000 to 1_500_000_000 (DEFAULT_MESSAGE_DELIVERY_TX_WEIGHT) in this PR. That's because of additional Currency::transfer call. This is the cost of worst-case single message delivery tx, where dispatch fee is paid at the target chain. However, the actual post-dispatch fee that the delivery call now returns is decreased by this additional cost (PAY_INBOUND_DISPATCH_FEE_WEIGHT) if fee has already been paid at the source chain.

As a result of ^^^, we now able to deliver only 704 instead of 1020 messages in single delivery tx. This may be 'fixed' by adding another parameter (something like number_of_pay_fee_calls) to the delivery tx, but it'll complicate a relayer code a bit (which is probably ok) + will add another pre-dispatch check in the call (which is also probably ok). The milestone of this fix actually depends on the expected use-case at real chains - if we'll have most/all messages that are paid at the target chain, then it's ok to leave as is for now + fix later. If we expect mixed/pay-at-the-source-chain usage, then it is better to fix before deployments.

@svyatonik svyatonik marked this pull request as ready for review April 21, 2021 10:47
Copy link
Copy Markdown
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 to me! Couple of small grumbles and questions. I'm super sorry it took my so long to take a look at this.

@svyatonik svyatonik merged commit fc5cf7d into master Jun 18, 2021
@svyatonik svyatonik deleted the pay-dispatch-fee-at-target-chain branch June 18, 2021 09:35
svyatonik pushed a commit that referenced this pull request Jul 17, 2023
* Add the uniques migration for statemine

* return weight from on_runtime_upgrade (#914)

need to return the weight from on_runtimie_upgrade().

* cargo update -p sp-io

* cargo update -p polkadot-primitives

Co-authored-by: hamidra <[email protected]>
serban300 pushed a commit to serban300/parity-bridges-common that referenced this pull request Mar 27, 2024
* pay dispatch fee at target chain

* refund unspent dispatch weight to messages relayer

* test that transfer actually happens

* pay-at-target-cchain benchmarks + fix previous benchmarks (invalid signature)

* include/exclude pay-dispatch-fee weight from delivery_and_dispatch_fee/delivery tx cost

* remvoe some redundant traces

* enum DispatchFeePayment {}

* typo

* update docs

* (revert removal of valid check)

* Update modules/messages/src/benchmarking.rs

Co-authored-by: Tomasz Drwięga <[email protected]>

* Update modules/messages/src/benchmarking.rs

Co-authored-by: Tomasz Drwięga <[email protected]>

* Update modules/messages/src/benchmarking.rs

Co-authored-by: Tomasz Drwięga <[email protected]>

* Update modules/messages/src/benchmarking.rs

Co-authored-by: Tomasz Drwięga <[email protected]>

Co-authored-by: Tomasz Drwięga <[email protected]>
Co-authored-by: Tomasz Drwięga <[email protected]>
serban300 pushed a commit to serban300/parity-bridges-common that referenced this pull request Apr 8, 2024
* pay dispatch fee at target chain

* refund unspent dispatch weight to messages relayer

* test that transfer actually happens

* pay-at-target-cchain benchmarks + fix previous benchmarks (invalid signature)

* include/exclude pay-dispatch-fee weight from delivery_and_dispatch_fee/delivery tx cost

* remvoe some redundant traces

* enum DispatchFeePayment {}

* typo

* update docs

* (revert removal of valid check)

* Update modules/messages/src/benchmarking.rs

Co-authored-by: Tomasz Drwięga <[email protected]>

* Update modules/messages/src/benchmarking.rs

Co-authored-by: Tomasz Drwięga <[email protected]>

* Update modules/messages/src/benchmarking.rs

Co-authored-by: Tomasz Drwięga <[email protected]>

* Update modules/messages/src/benchmarking.rs

Co-authored-by: Tomasz Drwięga <[email protected]>

Co-authored-by: Tomasz Drwięga <[email protected]>
Co-authored-by: Tomasz Drwięga <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P-Message Dispatch PR-audit-needed A PR has to be audited before going live.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Pay dispatch fee on the target chain. Refund relayer with 'weight' that remains after calls are dispatched

2 participants