[R-W Bridge] (Fees) Redeploy to synch Rococo and Wococo with fees/weights/rewards

[EmmanuellNorbertTulbure]

check and setup weight/fees everywhere

• BridgeHub extrinsics (finality, messages)
• BridgeHub xcm execution (ParentAsUmp for bridge-hub-kusama/rococo)
• Rockmine/Statemine XCM send export_message
• Wockmint/Statemint XCM receive execution
• hints from Slava
  • Backport xcm bridging extensions to the bridge repo #1813 (comment)
  • Backport xcm bridging extensions to the bridge repo #1813 (comment)
• design some tests Add test cases to Cumulus which simulates real e2e message delivery #2078
• type OnChargeTransaction change () -> DealWithFees<Runtime>
• setup reward constants - checking constants will be a part of Add test cases to Cumulus which simulates real e2e message delivery #2078

   pub storage DeliveryRewardInBalance: u64 = 1_000_000;
   pub storage RequiredStakeForStakeAndSlash: Balance = 1_000_000;
  

[bkontur]

One delivered message (local setup) BridgeHubRococo -> BridgeHubWococo:

on BridgeHubRococo:

submitFinalityProof:		0 (Pays=No) (multiple)
submitParachainHeads: 		0.0000505074 	ROC (fee for extrinsic)
submitParachainHeads: 		0.000050478224	ROC (fee for extrinsic)
submitParachainHeads: 		0.000050507133	ROC (fee for extrinsic)
receiveMessagesDeliveryProof:	0.000033512269	ROC (fee for extrinsic)
---------------------------------------------------------------------
Relayer expenses:		0.000185005	ROC (fees)


RewardRegistered:		0.0000001 	ROC `by DeliveryConfirmationPaymentsAdapter<100_000, 10_000>`
claimRewards:			0.000005074699	ROC (fee for extrinsic)

on BridgeHubWococo:

submitFinalityProof:		0 (Pays=No) (multiple)
submitParachainHeads:		0.000050536633	ROC (fee for extrinsic)
submitParachainHeads:		0.0000505368	ROC (fee for extrinsic)
submitParachainHeads:		0.0000505368	ROC (fee for extrinsic) - (block was pruned meantime, but was similar)
receiveMessagesProof:		0.000038495791	ROC (fee for extrinsic)
---------------------------------------------------------------------
Relayer expenses:		0,000190106	ROC  (fees)	


RewardRegistered:		0.000038495791	ROC `by RefundRelayerForMessagesFromParachain`
claimRewards:			0.000005075256	ROC (fee for extrinsic)

[bkontur]

@svyatonik
RefundRelayerForMessagesFromParachain does some reward = compute_fee(call), which is the same as pallet_transaction_payment::ChargeTransactionPayment does, so it makes sense why fee for extrinsic == RewardRegistered.

One thing, I see here that reward is much lower than paid fees on BridgeHubWococo, is it ok?

And second thing, I am just thinking, how to setup those fee consts for DeliveryConfirmationPaymentsAdapter.
I am not sure about this hard-coded constants, maybe this is something which should be able to change by governance (dedicated extrinsic?).

On the sending side we have constants for rewards and on the receiving side we have reward as fee calculation from extrinsic call.

I am just thinking, what if we have the same system on both sides:
receiveMessagesProof: calculate reward from extrinsic call + hard-coded/governance-managed constant (we do now)
receiveMessagesDeliveryProof: calculate reward from extrinsic call (we do now) + hard-coded/governance-managed constant

[svyatonik]

First of all, RBH<>WBH realyer is not using batch transactions now, so the numbers are incorrect at least at BridgeHubWococo side. So just ignore that now. Also - why you're counting multiple parachain heads transactions? Is it how it really works now? To deliver single message, it submits 3 parachain heads? If so, then it is an issue

And second thing, I am just thinking, how to setup those fee consts for DeliveryConfirmationPaymentsAdapter.
I am not sure about this hard-coded constants, maybe this is something which should be able to change by governance (dedicated extrinsic?).

Yep - we may add another signed ext for that (to refund confirmation transaction), so that it'll be consistent on both sides. @serban300 @acatangiu wdyt - do we need to do that/do that now/do that after initial deployment?

Just a brief intro on how it works now:

Right now at the source chain there are two constants to configure relayer rewards - DeliveryReward and ConfirmationReward. So rewards are computed as follows:

1. the "base reward" for delivering messages for given relayer is N * DeliveryReward, where N is number of delivered messages;
2. relayer, which has submitted the confirmation transaction gets portion of other relayer rewards. The "confirmation reward value" is N * ConfirmationReward. So the final reward is N * DeliveryReward - N * ConfirmationReward for relayer that IS NOT submitter of the confirmation transaction and N * DeliveryReward + Sum(N) * ConfirmationReward for relayer that has submitted the confirmation transaction.

The code is here: https://github.com/paritytech/parity-bridges-common/blob/master/modules/relayers/src/payment_adapter.rs#L68-L102

[serban300]

Yep - we may add another signed ext for that (to refund confirmation transaction), so that it'll be consistent on both sides. @serban300 @acatangiu wdyt - do we need to do that/do that now/do that after initial deployment?

Having a consistent approach on both sides sounds good. But I just want to take a look on the code to make sure I understand everything correctly. I'll get back with a more documented opinion.

[serban300]

@svyatonik @bkontur

I looked on the code. I suppose that if the delivery and confirmation fees are configured correctly, they will always cover the costs of submitting the extrinsics, but I think it would make sense to refund the receive_messages_delivery_proof explicitly. It's good for consistency also, but first of all I think it would be more fair to the relayers. Also I think we could implement it in the current refund signed extension. I don't think it would require big changes.

Regarding timing, if we're sure we want to do it, I would do it now, in order to avoid bugs after the initial deployment. I can take the issue if you want. However you prefer.

Another small thing, related to the fees. IIUC right now the ConfirmationReward is just the confirmation reward, while the DeliveryReward is more of a sum of confirmation reward + delivery reward. So for example for

	type DeliveryConfirmationPayments = pallet_bridge_relayers::DeliveryConfirmationPaymentsAdapter<
		Runtime,
		frame_support::traits::ConstU64<100_000>, /// DeliveryReward
		frame_support::traits::ConstU64<10_000>, /// ConfirmationReward
	>;

the confirmation reward is 10000, while the actual delivery reward is (100000 - 10000) = 90000.

I was a bit confused by this. Could we rename the DeliveryReward to something like ConfirmedDeliveryReward ? Or change the DeliveryReward to be just the delivery reward (in this case 90000) ? I think the second option would be better since it would completely prevent situations where the confirmation reward > confirmed delivery reward, which seems like a wrong configuration.

[svyatonik]

I don't mind if you want to take it, of course :) But probably we want a separate signed extension for that - it is for an opposite bridge direction. So it needs to be explored.

If we gonna do it now, then:

1. we'll need audit for that new code. Which is probably fine, because we need an audit for latest changes + fixes as well;
2. this confusion you've mentioned should gone - we should leave single constDeliveryReward in a DeliveryConfirmationPaymentsAdapter implementation. Because we'll handle confirmation reward on signed extension level

[serban300]

this confusion you've mentioned should gone - we should leave single constDeliveryReward in a DeliveryConfirmationPaymentsAdapter implementation. Because we'll handle confirmation reward on signed extension level

I hope I understood correctly. Just checking. So you're saying that we'll replace the confirmation fee with a refund of the batch of calls containing the receive_messages_delivery_proof ?

[svyatonik]

Let's consider one-direction bridge. So we have source and target chains + some lane. Right now we have everything ready to get rewards/compensations at the target chain. So right now we have everything ready to handle that at target chain:

• we refund for message delivery transactions
• we refund for parachain heads ++ message delivery transactions batch;
• we refund for relay chain GRANDPa finality ++ parachain heads ++ message delivery transactions batch;
• we do not refund for separate finality transactions.

Once messages are delivered (delivery transaction is sent to the target chain), the relayer submits delivery confirmation transaction to the source chain. We have no batch transactions there (at the source chain) and right now we don't have any plans to support that! Right now we have this DeliveryConfirmationPaymentsAdapter adapter that is called and it splits the whole reward amount (Count(delivered-messages) * DeliveryReward) between relayers who have submitted delivery transactions (at target chain) and relayer that has submitted confirmation transaction (at the source chain). What Branislav suggests (iiuc) is to refund relayer that submits confirmation transaction at the signed extension level. So that in DeliveryConfirmationPaymentsAdapter we'll only need to reward relayers who have actually delivered messages (who have submitted transactions to the target chain).

I hope I understood correctly. Just checking. So you're saying that we'll replace the confirmation fee with a refund of the batch of calls containing the receive_messages_delivery_proof ?

So yes - we're replacing the ConfirmationReward from DeliveryConfirmationPaymentsAdapter with signed extensions. But we don't need to support any batch calls right now in this signed extension - just a plain receive_messages_delivery_proof transaction

[serban300]

Thanks for the details ! They were helpful. And replacing ConfirmationReward with signed extension sounds good to me.

[svyatonik]

Actually - I maybe wrong here. Maybe we need to support refund for batch transactions here too. Because if we'll be only refunding confirmation transaction submitter, he'll lose funds for submitting finality transactions (unless he has previously submitted them in a single batch with other-direction delivery transactions). And malicious relayers can just block the inbound lane (at the target chain) by not submitting confirmation transaction to the source chain (and thus by not clearing the unrewarded_relayers vector). So let's support this from beginning as well. So we need to refund for:

• plain receive_messages_delivery_proof transactions;
• batch of required parachain head + receive_messages_delivery_proof transaction;
• batch of required relay chain header + parachain head + receive_messages_delivery_proof transaction;

Also we need to be sure that:

• we support such batch transactions in relayer;
• we do not refund more than required (so we need to take post_dispatch weight of the receive_messages_delivery_proof into account)

Yep, that sounds about right - sorry for misleading you, @serban300 . So if you're going to work on signed extension, then I'll work on the relayer part. Does it sounds ok to you?

[serban300]

Yep, that sounds about right - sorry for misleading you, @serban300 . So if you're going to work on signed extension, then I'll work on the relayer part. Does it sounds ok to you?

Yes, perfect. Thanks !

[serban300]

The changes were deployed (Runtimes + relay). The script for sending a remark fails:

Error: createType(Call):: Call: failed decoding polkadotXcm.send:: Struct: failed on args: {"dest":"{\"_enum\":{\"V2\":\"XcmV2MultiLocation\",\"V3\":\"XcmV3MultiLocation\"}}","message":"{\"_enum\":{\"__Unused0\":\"Null\",\"__Unused1\":\"Null\",\"V2\":\"XcmV2Xcm\",\"V3\":\"XcmV3Xcm\"}}"}:: Struct: failed on message: {"_enum":{"__Unused0":"Null","__Unused1":"Null","V2":"XcmV2Xcm","V3":"XcmV3Xcm"}}:: Cannot map Enum JSON, unable to find '' in __unused0, __unused1, v2, v3
    at createTypeUnsafe (/home/serban/.nvm/versions/node/v18.13.0/lib/node_modules/@polkadot/api-cli/node_modules/@polkadot/types-create/cjs/create/type.js:73:18)
    at TypeRegistry.createTypeUnsafe (/home/serban/.nvm/versions/node/v18.13.0/lib/node_modules/@polkadot/api-cli/node_modules/@polkadot/types/cjs/create/registry.js:353:46)
    at extrinsicFn (/home/serban/.nvm/versions/node/v18.13.0/lib/node_modules/@polkadot/api-cli/node_modules/@polkadot/types/cjs/metadata/decorate/extrinsics/createUnchecked.js:31:21)
    at decorated (/home/serban/.nvm/versions/node/v18.13.0/lib/node_modules/@polkadot/api-cli/node_modules/@polkadot/api/cjs/base/Decorate.js:616:22)
    at makeTx (/home/serban/.nvm/versions/node/v18.13.0/lib/node_modules/@polkadot/api-cli/cjs/runcli.js:199:16)
    at main (/home/serban/.nvm/versions/node/v18.13.0/lib/node_modules/@polkadot/api-cli/cjs/runcli.js:239:12)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)

Not sure what this failure is related to. This also fails on local.

The script for sending a trap works, the message reaches BHR and the relayer fails to send it to BHW:

2023-03-02 10:12:14 | [BridgeHubRococo_to_BridgeHubWococo_MessageLane_00000001] 2023-03-02 08:12:14 +00 DEBUG bridge Received nonces from BridgeHubRococo::MessagesDelivery: SourceClientNonces { new_nonces: {13: MessageDetails { dispatch_weight: Weight { ref_time: 0, proof_size: 0 }, size: 56, reward: 0 }, 14: MessageDetails { dispatch_weight: Weight { ref_time: 0, proof_size: 0 }, size: 56, reward: 0 }, 15: MessageDetails { dispatch_weight: Weight { ref_time: 0, proof_size: 0 }, size: 56, reward: 0 }, 16: MessageDetails { dispatch_weight: Weight { ref_time: 0, proof_size: 0 }, size: 56, reward: 0 }}, confirmed_nonce: Some(12) }

2023-03-02 10:12:14 | [BridgeHubRococo_to_BridgeHubWococo_MessageLane_00000001] 2023-03-02 08:12:14 +00 DEBUG bridge Going to require BridgeHubRococo::MessagesDelivery header HeaderId(242729, 0x2dec3bc4101230d3f022cf903fa38ce2c8a065ea44ba3f583788ef57567e2871) at BridgeHubWococo::MessagesDelivery

2023-03-02 10:12:14 | [BridgeHubRococo_to_BridgeHubWococo_MessageLane_00000001] 2023-03-02 08:12:14 +00 ERROR bridge Error asking for source headers at BridgeHubWococo::MessagesDelivery: RpcError(RestartNeeded("Networking or low-level protocol error: WebSocket connection error: i/o error: Connection reset by peer (os error 104)")). Going to restart

Not sure why. This works locally.

[bkontur]

maybe live westmint is down, there is one call to generate remark_with_event encoded_call_data

[bkontur]

hmm, I tried now and it worked from actual bridge-hub-rococo-wococo

./scripts/bridges_rococo_wococo.sh send-remark-rococo

https://polkadot.js.org/apps/?rpc=wss%3A%2F%2Fws-rococo-rockmine2-collator-node-0.parity-testnet.parity.io#/explorer/query/0x6a707b2411c21851d0201b9601e5eb6862aac7be49ece9962480e37826048b95

@serban300
check please, ensure_polkadot_js_api(), there this fails on your PC:

    generate_hex_encoded_call_data "check" "--"
    local retVal=$?
    if [ $retVal -ne 0 ]; then
        echo ""
        echo ""
        echo "-------------------"
        echo "Installing (nodejs) sub module: ./scripts/generate_hex_encoded_call"
        pushd ./scripts/generate_hex_encoded_call
        npm install
        popd
        exit 1
    fi

this could explain, why trap works and transact not :),
becase transact is generating data from live node by generate_hex_encoded_call_data

[serban300]

@bkontur thanks ! I'll try. For the moment I'm debugging using traps since they work.

I ran the relayer locally, and I'm getting a more specific error here:

[BridgeHubRococo_to_BridgeHubWococo_MessageLane_00000001] 2023-03-02 10:23:00 +00 ERROR bridge Error submitting proof BridgeHubWococo::MessagesDelivery: RpcError(Call(Custom(ErrorObject { code: ServerError(1010), message: "Invalid Transaction", data: Some(RawValue("Inability to pay some fees (e.g. account balance too low)")) }))). Retrying in 0.368770155

Which makes sense. I'll try to fund the relayer accounts.

[EmmanuellNorbertTulbure]

@EmmanuellNorbertTulbure to redefine title and description

[svyatonik]

So I've been watching relay balances today during single message send. Here's what happening now:

Relayer accounts:
BridgeHubWococo relayer account id: e6eb4cf91678d3e6f5c59bd065e19639787b599e58043fffb424f3f6dd1fa500 (5HHUmazp...)
    Initial balance: 149,966,987,564,140
    Initial rewards:
        (0x00000001, bhro, ThisChain): 7,797,460,846
        (0x00000001, bhro, BridgedChain): 0
        (0x00000001, bhwo, ThisChain): 0
        (0x00000001, bhwo, BridgedChain): 0
    After single message roundtrip balance: 149,966,556,614,875 (difference = 430,949,265)
    After single message roundtrip rewards: 8,228,410,111 (difference = 430,949,265)
BridgeHubRococo relayer account id: f09804d2edb9680a41119069417e8376569795302131b838563e41c3c02a662a (5HWAWSum...)
    Initial balance: 99,995,311,451,112
    Initial rewards:
        (0x00000001, bhro, ThisChain): 0
        (0x00000001, bhro, BridgedChain): 0
        (0x00000001, bhwo, ThisChain): 0
        (0x00000001, bhwo, BridgedChain): 1,881,741,248
    After single message roundtrip balance: 99,995,208,354,376 (difference = 103,096,736)
    After single message roundtrip rewards: 1,985,837,984 (difference = 104,096,736)

So:

1. no extra transactions!!!
2. both signed extensions are working - at the target side we have the exact tx cost compensation for rewards. At the source side it is the compensation plus rewards
3. current delivery transaction cost (single remark message) is around 0.0004 WOC. I guess it'll be increaed, because we are not accounting (do we?) weight of message queueing
4. current confirmation transaction cost (single message) is around 0.0001 ROC. I suppose it is close to the final cost

[bkontur]

closing, we dont care about constants now, but we will provide test which will check them so it will be under controll