paritytech / polkadot-sdk

The Parity Polkadot Blockchain SDK
https://polkadot.com/
1.91k stars 704 forks source link

Relayer rewards paid to specified location account #6578

Open claravanstaden opened 1 day ago

claravanstaden commented 1 day ago

Adds the required functionality for relayers to accumulate and claim rewards.

@bkontur and @acatangiu, I just lifted and shifted the code that was in the snowbridge rewards pallet into the bridge-relayers pallet. I looked at the existing code (to see if I can reuse it) and I think the main shortcoming of the existing claim_rewards method is the ability to claim the rewards to destination account location on AH (instead of using the extrinsic origin as destination account). And we won't use rewards_account_params, since we mint Weth on AH. Let me know your thoughts, and which option your prefer:

1) Use this code as I am submitting it now (with some cleanup - I'll wrap all the newly added pallet configs in a trait) 2) Add a parameter to claim_rewards to specify a target account location

bkontur commented 4 hours ago

@claravanstaden Nice! I’m glad you’re starting with this. I’m marking it as part of [this issue](https://github.com/orgs/paritytech/projects/145/views/8?pane=issue&itemId=84899273).

I have some ideas on how to make this pallet more generic to cover all our use cases (collecting and claiming rewards regardless of where the messaging queues/pallets are deployed—whether on AH or BH). I believe a single generic RelayerRewards map with a "reward_discriminator" and one extrinsic for claiming (plus a migration for P/K rewards) should suffice. Regarding P/K rewards or Snowbridge rewards, it should just be a matter of configuring the pallet where it’s deployed.

I’d like to prepare a more detailed final design for this. How time-sensitive is this for you? What is your deadline?

claravanstaden commented 4 hours ago

Thanks @bkontur, that sounds great! Would reward_discriminator describe the reward asset time.

We need this functionality in stable-2412. Hope that is feasible for you. I can aid in the implementation too. :)

acatangiu commented 3 hours ago

We need this functionality in stable-2412. Hope that is feasible for you. I can aid in the implementation too. :)

stable-2412 has already been branched off since early Nov (see cutoff date here), it is now in QA and audit period. The changes/functionality you are adding with this PR should also be tested and audited so it should go in before QA and audit period. Meaning you missed stable-2412.

If push comes to shove and it is critical to make this release you could force it in, but then I would not change existing code to avoid introducing regressions that we don't have time to test or catch.

vgeddes commented 2 hours ago

@claravanstaden actually we are aiming for the stable2503 release, soft code-freeze on Dec 26.