Skip to content
This repository has been archived by the owner on Feb 29, 2024. It is now read-only.

Redesign fee market payment #169

Merged
merged 22 commits into from
Aug 1, 2022
Merged

Redesign fee market payment #169

merged 22 commits into from
Aug 1, 2022

Conversation

boundless-forest
Copy link
Member

@boundless-forest boundless-forest commented Jul 22, 2022

TODO

  • (Update tests)
  • (Remove RewardBook)

@boundless-forest boundless-forest marked this pull request as ready for review July 25, 2022 08:56
@hackfisher hackfisher requested a review from hujw77 July 26, 2022 03:48
Copy link
Contributor

@hackfisher hackfisher left a comment

Choose a reason for hiding this comment

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

Have kind of difficulty to read the code from my point as a reviewer. There might be still spaces for the code to be more simplified/clean.

And it's a chance to review and simplify other business logic.

Request for better naming and docs.

@hujw77 @wuminzhe please review.

reward_item: &mut RewardItem<T::AccountId, BalanceOf<T, I>>,
) {
// MessageRelayersRewardRatio total reward => message relayer
let message_reward = T::MessageRelayersRewardRatio::get() * total_reward;
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe in another PR, need to handle the dispatch fee (message.weight)

https://github.com/darwinia-network/darwinia-messages-substrate/issues/142

modules/fee-market/src/s2s/payment.rs Outdated Show resolved Hide resolved
modules/fee-market/src/s2s/payment.rs Outdated Show resolved Hide resolved
modules/fee-market/src/s2s/payment.rs Outdated Show resolved Hide resolved
modules/fee-market/src/s2s/payment.rs Outdated Show resolved Hide resolved
modules/fee-market/src/s2s/payment.rs Outdated Show resolved Hide resolved
modules/fee-market/src/s2s/payment.rs Outdated Show resolved Hide resolved
modules/fee-market/src/s2s/payment.rs Outdated Show resolved Hide resolved
modules/fee-market/src/s2s/payment.rs Show resolved Hide resolved
@boundless-forest boundless-forest marked this pull request as draft July 26, 2022 05:39
@boundless-forest boundless-forest marked this pull request as ready for review July 26, 2022 07:07
@hackfisher
Copy link
Contributor

V3:
{
    confirm_relayer 落在哪个Slot Index, Slot price is the relayer ask for Slot Index.

    Message surplus = Message Fee - Slot price.

    1. 
      A: Slot Offensive Slash: ?For each assigned relayer's slot index < Slot index(Confirm), slash: a) collateral_const * 20% and no reward or b) slash function(t) in expriy time (> last slot)

    Message Reward = Slot price + Slot Offensive Slash

      B. Slot Duty Reward = Message surplus * 20% -> assigned relayers which >= Slot index(Confirm)  (If no relayers match the condition, Ask reward is zero)

    Message surplus -= Slot Duty Reward

    2. Message Reward -> (delivery_relayer, confirm_relayer)

    3. Treasury: Message surplus
}

@boundless-forest boundless-forest marked this pull request as draft July 28, 2022 04:55
@boundless-forest boundless-forest changed the title Add assigned relayers slash Redesign fee market payment Jul 28, 2022
@boundless-forest
Copy link
Member Author

boundless-forest commented Jul 29, 2022

V3:
{
    confirm_relayer 落在哪个Slot Index, Slot price is the relayer ask for Slot Index.

    Message surplus = Message Fee - Slot price.

    1. 
      A: Slot Offensive Slash: ?For each assigned relayer's slot index < Slot index(Confirm), slash: a) collateral_const * 20% and no reward or b) slash function(t) in expriy time (> last slot)

    Message Reward = Slot price + Slot Offensive Slash

      B. Slot Duty Reward = Message surplus * 20% -> assigned relayers which >= Slot index(Confirm)  (If no relayers match the condition, Ask reward is zero)

    Message surplus -= Slot Duty Reward

    2. Message Reward -> (delivery_relayer, confirm_relayer)

    3. Treasury: Message surplus
}

I find a issue while doing tests:

Assuming the fee market with 3 assigned relayers A, B, C with quota QA, QB, QC. If there is a order confirmed at the third slot, the message fee paied by user is QC in this case, the slot price is also QC when calculate rewards. Thus the message_surplus is 0, the slot duty reward for C is zero as well. That's seems make no sense. Confirmed in the third slot is still considered in-time delivery, the slot duty reward for C should not be zero. @hackfisher @hujw77

@hackfisher
Copy link
Contributor

the slot duty reward for C is zero as well. That's seems make no sense

This looks good to me, C still will get incentives from the message reward.

@boundless-forest
Copy link
Member Author

This looks good to me, C still will get incentives from the message reward.

It may not be the case, C may not be a message deliver or confirm relayer. Check out this test case: https://github.com/darwinia-network/darwinia-messages-substrate/blob/bear-slash-empty-relayers/modules/fee-market/src/tests.rs#L1027-L1060

@boundless-forest boundless-forest marked this pull request as ready for review July 29, 2022 09:40
@hackfisher hackfisher merged commit 78d02d4 into main Aug 1, 2022
@hackfisher hackfisher deleted the bear-slash-empty-relayers branch August 1, 2022 16:17
boundless-forest added a commit that referenced this pull request Aug 10, 2022
* Add basic solution

* Refactor Slash Report

* Refactor Slash Report 2

* Add other changes

* Add tests

* Self review

* Rename to `message_and_confirm_reward`

* Rename to `previous_relayers`

* Rename to AssignedRelayer

* Add more comments

* Add more comments

* Add new reward implementation

* Rename and clean the code, needs more test

* Add more docs here

* Prepare for tests

* Fix broken tests

* Refactor

* Rename

* Remove RewardBook

* Self review

* Save one storage

* Try fix ci
boundless-forest added a commit that referenced this pull request Aug 10, 2022
boundless-forest added a commit that referenced this pull request Sep 21, 2022
* Add basic solution

* Refactor Slash Report

* Refactor Slash Report 2

* Add other changes

* Add tests

* Self review

* Rename to `message_and_confirm_reward`

* Rename to `previous_relayers`

* Rename to AssignedRelayer

* Add more comments

* Add more comments

* Add new reward implementation

* Rename and clean the code, needs more test

* Add more docs here

* Prepare for tests

* Fix broken tests

* Refactor

* Rename

* Remove RewardBook

* Self review

* Save one storage

* Try fix ci
jiguantong pushed a commit that referenced this pull request Sep 21, 2022
* Add basic solution

* Refactor Slash Report

* Refactor Slash Report 2

* Add other changes

* Add tests

* Self review

* Rename to `message_and_confirm_reward`

* Rename to `previous_relayers`

* Rename to AssignedRelayer

* Add more comments

* Add more comments

* Add new reward implementation

* Rename and clean the code, needs more test

* Add more docs here

* Prepare for tests

* Fix broken tests

* Refactor

* Rename

* Remove RewardBook

* Self review

* Save one storage

* Try fix ci
jiguantong pushed a commit that referenced this pull request Sep 29, 2022
* Add basic solution

* Refactor Slash Report

* Refactor Slash Report 2

* Add other changes

* Add tests

* Self review

* Rename to `message_and_confirm_reward`

* Rename to `previous_relayers`

* Rename to AssignedRelayer

* Add more comments

* Add more comments

* Add new reward implementation

* Rename and clean the code, needs more test

* Add more docs here

* Prepare for tests

* Fix broken tests

* Refactor

* Rename

* Remove RewardBook

* Self review

* Save one storage

* Try fix ci
jiguantong added a commit to darwinia-network/darwinia-parachain that referenced this pull request Sep 30, 2022
jiguantong added a commit to darwinia-network/darwinia-parachain that referenced this pull request Sep 30, 2022
wuminzhe pushed a commit that referenced this pull request Sep 30, 2022
* Update market after order is created or comfirmed (#94)

* Update order capacity

* FIX CI

* Update comment

* Redesign fee market payment (#169)

* Add basic solution

* Refactor Slash Report

* Refactor Slash Report 2

* Add other changes

* Add tests

* Self review

* Rename to `message_and_confirm_reward`

* Rename to `previous_relayers`

* Rename to AssignedRelayer

* Add more comments

* Add more comments

* Add new reward implementation

* Rename and clean the code, needs more test

* Add more docs here

* Prepare for tests

* Fix broken tests

* Refactor

* Rename

* Remove RewardBook

* Self review

* Save one storage

* Try fix ci

* Handle Zero `CollateralPerOrder` (#176)

* Companion 189 for darwinia-parachain (#194)

* Update fee-market terminology (#192) (#199)

* Support pangolin <> pangolin parachain alpha bridge (#190)

* Cherry-pick #201 to `darwinia-parachain` (#203)

* Del useless related to fee calculation (#201)

* Delete estimate_delivery_transaction

* Delete transaction_payment

* Delete MessageTransaction

* Code clean

* Fix compile

* Anchor polkadot-v0.9.27

* remove unused import

* Fix test problem (#172)

* fix ci

* Add necessary reward event (#89)

* Refactor the total reward cal

* Code Clean

* Fix test

* Add OrderCreated event

* Self review

* Update comment

* Update OrderCreate event to include relayers

* Fix review

* Enhance dispatch module (#121)

* rm shift session manager pallet and clean useless imports (#128)

* `Pre-dispatch` validate for main branch (#130)

* Adjust the traits

* Try fix compile

* Fix tests

* Code clean

* Avoid duplicate evm transact fees (#136)

* cherry pick #137 (#140)

* cherry pick #137

* pick #141

Co-authored-by: bear <[email protected]>

* Drop error in `pre_dispatch` (#152) (#153)

* Migrate to new s2s bridge (#149)

* Add client-darwinia/client-crab/client-crab-parachain

* Update CI

* rever substrate commit

* fix compile

* remove transactional

* Fix

* Bump finality-grandpa to v0.16.0

* fix wrong import

* make change follow paritytech/substrate#11415

* Add runtime-common to workspace and try fix test

* Make changes follow paritytech/substrate#10242

* Code clean

* More ci check

* Remove integrity

Co-authored-by: bear <[email protected]>

* Companion for #155 (#157)

* Update market after order is created or comfirmed (#94)

* Update order capacity

* FIX CI

* Update comment

* Redesign fee market payment (#169)

* Add basic solution

* Refactor Slash Report

* Refactor Slash Report 2

* Add other changes

* Add tests

* Self review

* Rename to `message_and_confirm_reward`

* Rename to `previous_relayers`

* Rename to AssignedRelayer

* Add more comments

* Add more comments

* Add new reward implementation

* Rename and clean the code, needs more test

* Add more docs here

* Prepare for tests

* Fix broken tests

* Refactor

* Rename

* Remove RewardBook

* Self review

* Save one storage

* Try fix ci

* Handle Zero `CollateralPerOrder` (#176)

* Update Fee market docs (#178)

* Update doc

* Update example

* Move to `dev-dependencies` (#182)

* Sync part.1 (#184)

* Remove fee relates (#186)

* Sync missing changes from different branches (#191)

* Support pangolin <> pangolin parachain alpha bridge (#190)

* Integrate `fee-market` to `FromThisChainMessageVerifier` (#189)

* Run ignored crate

* Fix tests

* Add features

* Fix compile

* Update fee-market terminology (#192)

* Del useless related to fee calculation (#201)

* Delete estimate_delivery_transaction

* Delete transaction_payment

* Delete MessageTransaction

* Code clean

* master > polkadot-v0.9.27

* roll back bp-parachain

Co-authored-by: bear <[email protected]>
Co-authored-by: Xavier Lau <[email protected]>
Co-authored-by: Aki Wu <[email protected]>
Co-authored-by: HackFisher <[email protected]>
Co-authored-by: HackFisher <[email protected]>
Co-authored-by: fewensa <[email protected]>
AurevoirXavier added a commit to darwinia-network/darwinia-parachain that referenced this pull request Oct 12, 2022
* Anchor polkadot-v0.9.27

* Companion for paritytech/cumulus#1436

* Companion for paritytech/cumulus#1421

* Companion for paritytech/cumulus#1350

* Companion for paritytech/cumulus#1456

* Use messages-substrate prepare-polkadot-v0.9.27 for debug

* Companion for darwinia-network/darwinia-messages-substrate#169

* Companion for darwinia-network/darwinia-messages-substrate#192

* Companion for darwinia-network/darwinia-messages-substrate#186

* Companion for darwinia-network/darwinia-messages-substrate#201

* Missing part companion for darwinia-network/darwinia-messages-substrate#169

* format

* replace prepare-polkadot-v0.9.27 -> polkadot-v0.9.27

* use `PANGOLIN_PANGOLIN_PARACHAIN_LANE` from `bridge_runtime_common` (#151)

* format

* Bump dependencies

Co-authored-by: Xavier Lau <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
No open projects
Status: Done/No Companion
2 participants