Make some headers submissions free

[svyatonik]

related to #2871 - look there for overview of what happens here

Better to review commit-by-commit:

• e358816: adds FreeHeadersInterval parameter to the pallet-bridge-grandpa. Every FreeHeadersIntervalth bridged header submission is free for the submitter;
• 824b8b9: adds is_free_execution_expected flag to the recently added submit_finality_proof_ex. If it is set to true by the caller and during block creation (not checked during tx validation) there are no more entries left for the free headers in the block (it is limited by the MaxFreeHeadersPerBlock parameter of the pallet-bridge-grandpa), then transaction won't be included into the block and the submitter won't lose any fees;
• e901f46: just moves everything related to our extensions to the separate folder;
• 2f13251: some refactoring required for the next commit;
• efcb990: adds new RefundBridgedMessages transaction extension, used to check for obsolete standalone message transactions AND bump delivery priority for registered relayers. Previous extensions (RefundBridgedParachainMessages and RefundBridgedGrandpaMessages) also refund/bump batch transactions, but we do not need that for our new approach. Because we want transactions to be free (Pays::No) instead of doing refunds (using pallet-bridge-relayers) and batch transactions are incompatible with that;
• 186a73b: check_obsolete_submit_finality_proof now returns TransactionValidity with bumped priority;
• 0298318: adds CheckAndBoostBridgeGrandpaTransactions wrapper for the pallet-bridge-grandpa, which actually boost standalone submit_finality_proof (and submit_finality_proof_ex) transactions using stuff introduced in previous commit;
• 1541ba5: relayer can now be slashed to explicit account. Previously we've been computing destination account from RewardsAccountParams struct. But this struct is specific to messages (lanes) and when we deal with GRANDPA submissions, we do not have any lanes and funds will likely go to some treasury account;
• 64418ac: adds slashing support to the CheckAndBoostBridgeGrandpaTransactions;
• 48bce25: allows free parachain head updates. Update may be free if filter from pallet-bridge-parachains returns true;
• 8003f8e: adds filter implementation FreeParachainUpdateForFreeRelayHeader, which allows single free parachain head update for every free relay chain header.

I think that's all changes we need in the runtime, but we'll need some changes to relayer (and maybe runtime APIs). But I was trying to wrap it by the end of the day, so will double check on friday.

[svyatonik]

Leaving this comment as a guide on how we'll be integrating that into test (R<>W) bridge (test matters here - see details below):

1. MaxFreeHeadersPerBlock is now set to 2 there. Let's maybe increase it a bit, e.g. to 4 - it should be below 25% of block weight (to avoid increasing fee multiplier);
2. let's set FreeHeadersInterval to 50 - it is one free relay chain header ~ every 5 minutes, which is imo good enough for the bridge;
3. let's replace RefundBridgedParachainMessages tx extension with new RefundBridgedMessages extension. By doing that, we no longer refund batch transactions with messages delivery/confirmation - only standalone successful message delivery and confirmation transactions will be refunded;
4. let's replace BridgeWestendGrandpa and BridgeRococoGrandpa in bridge_runtime_common::generate_bridge_reject_obsolete_headers_and_messages call with CheckAndBoostBridgeGrandpaTransactions<BridgeWestendGrandpa> and CheckAndBoostBridgeGrandpaTransactions<BridgeRococoGrandpa> accordingly. This will boost priority of registered relayer transactions + slash this relayer if transaction fails
5. let's set FreeHeadsUpdateFilter to FreeParachainUpdateForFreeRelayHeader. It'd allow us to get one free bridged BH header for every free bridged relay chain header;
6. let's also deduct finality costs from the bridge fee, paid by the sender.

We can restart relayer (actually run 3 relayers instead of one) after that - it may lose some funds if we keep running old relayer after this ^^^ upgrade.

---

For P<>K bridge we need to take care of tokens :) So while we may apply the same changes ^^^ there, we need to have a schedule that takes into account:

1. the fact that Kusama and Polkadot parachains may have different upgrade schedules - e.g. Polkadot.AH may be upgraded at t, Kusama.BH at t + N and so on;
2. the fact that we have a relayer running, so we need to sync parachains and relayer upgrade.

Maybe we should do the following:

1. make sure that we do (1), (2), (4), (5) from above in first upgrade - we don't care if upgrades will be synchronized or not + we may keep running relayer meanwhile;
2. once it is live on both bridge hubs, start 3 standalone relayers instead of one: one to relay GRANDPA proofs, one for parachain proofs and last for message. First two should not spend any tokens at all (Pays::No) and 3rd one will work in a previous mode (i.e. its refunds + rewards are registered in the relayers pallet). Relayers, running in previous complex mode (if any) will still get receive their refunds+rewards to the relayers pallet;
3. in next upgrade we need to do remaining (3) and (6) - it'll disable refunds for complex relayers (hope all are upgraded) and decrease the bridge fee

[acatangiu]

just a note:

let's set FreeHeadersInterval to 50 - it is one free relay chain header ~ every 5 minutes, which is imo good enough for the bridge;

I've seen in #2883 a description along the lines "header free for numbers divisible by some N"

Should we do some interval: "free if current_imported - last_imported >= N", instead of using exact block numbers? Is there a reason you prefer the latter?

[svyatonik]

just a note:

let's set FreeHeadersInterval to 50 - it is one free relay chain header ~ every 5 minutes, which is imo good enough for the bridge;

I've seen in #2883 a description along the lines "header free for numbers divisible by some N"

Should we do some interval: "free if current_imported - last_imported >= N", instead of using exact block numbers? Is there a reason you prefer the latter?

My idea was that with fixed interval for free relay chain headers, we simplify everything - we could make almost everything by only checking block numbers. Without that, we would need to do some non-trivial computations to verify that header has been submitted for free. E.g. "free" parachain header (in current impl) is parachain header that is "proved" at "free" relay chain header. So we now can check if proof.at_relay_block.number % N == 0 instead of reading several storage entries that are making ring buffer in pallet-bridge-grandpa.

So it's mostly for simplicity of both onchain and offchain code. I could also imagine that your option is better if we'll keep using "complex" relayers, which are looking at the messages pallet and if there's something to relay, they start relaying parachain headers required to prove that "something", which in turn triggers relay chain header relaying to prove that parachain header. Then, if using intervals, it could sleep when bridge is not actually used and only do transactions when there's something to deliver. But then some malicious actor may front-run us by submittig his own transaction that proves block B - 1 (we need B) for free and thus cause a delivery delay of at least that configured interval N (because we'll be able to prove B or its descendant for free, only after waiting for N new headers of the target chain).

I was thinking about running standalone finality and parachain relays. The idea is that they're running and always submitting headers with a steady interval (hoping that relay chain also steadily produces its blocks). Ideally we better to use slots, because it is a measure of time (and users will be measuring bridge delays in time, not in blocks), but again - I thought the simplicity is better here.

TLDR: we could do intervals, but it'll make some things more complex and yet there's no much gain from that. Could change the implementation if there's a consensus if intervals are better.

[acatangiu]

Thank you for the explanation Slava! I am now aligned with you in favor of using block numbers for simplicity.

[serban300]

Did a first iteration. Left only some nits, mostly related to renamings so far. But as a high-level comment I was wondering if it would make sense to work on this on a separate feature branch in order to be able to have a security review on it if needed for example, before merging into polkadot-staging.

[acatangiu]

Did a first iteration. Left only some nits, mostly related to renamings so far. But as a high-level comment I was wondering if it would make sense to work on this on a separate feature branch in order to be able to have a security review on it if needed for example, before merging into polkadot-staging.

We can do security review even after merging to polkadot-sdk as long as it's before deploying to Polkadot (before fellowship/runtimes integration + release + upgrade). So, I think we can just work on polkadot-staging, we don't have any other features development here now anyway

[svyatonik]

3a6d946: we now allow free submission of initial parachain heads. That is not a big deal for real bridges, because by the time this change lands there, we'll already have some bridged para heads there. But it should make our zombienet tests easier (in theory)

[svyatonik]

85dfd1b: when validating tx from pool (not durin block import or creation), on_initialize is not called and hence FreeHeadersRemaining is set to default value (zero). So transactions with is_free_execution_expected = true are immediately treated invalid. This commit changes FreeHeadersRemaining to Option<u32>, so if it is None, we know that we are outside of tx body, which is probably txpool verification => we pass such transaction. If relayer will ever submit call with is_free_execution_expected = true using delayed execution, it won't be free

[serban300]

Sorry, just left a lot of nits.

[acatangiu]

LGTM! 💯

[svyatonik]

I'm gonna start moving it to polkadot-sdk, starting with a separate PR for that: #2873 (comment)

[bkontur]

lgtm and the last question, this PR adds free execution by default, but what if anybody wants really the full paid execution without this free stuff?

I mean, what about adding something like type AllowFreeExecution: Get<bool>; to the pallet configurations?

E.g. when the runtime specifies:

• type AllowFreeExecution = ConstBool<false> - everything will work as before this PR
• type AllowFreeExecution = ConstBool<true> - activates this free execution feature

[svyatonik]

lgtm and the last question, this PR adds free execution by default, but what if anybody wants really the full paid execution without this free stuff?

I mean, what about adding something like type AllowFreeExecution: Get<bool>; to the pallet configurations?

E.g. when the runtime specifies:

• type AllowFreeExecution = ConstBool<false> - everything will work as before this PR
• type AllowFreeExecution = ConstBool<true> - activates this free execution feature

It should be disabled by default if we set type FreeHeadersInterval = None iirc. Mind that FreeHeadersInterval is Get<Option<u32>>, not the Get<u32>.

[svyatonik]

I've copied this PR to polkadot-sdk: paritytech/polkadot-sdk#4102 . Feel free to keep reviewing here or in polkadot-sdk - I'll be responding in both PRs. Had some conflicts (because TransactionExtension -> SignedExtension PR has been reverted), so bridge-runtime-common is the only crate that may have changed during this PR move.

[svyatonik]

Superseded by paritytech/polkadot-sdk#4102