XDM: Close init channels and take protocol fee when channel being closed is in init state

[vedhavyas]

PR allows closing init channels so that user can get their reserve back as raised by audit here - #2804. Since the channel is in init state, protocol takes a portion of the reserve fee to dis-incentivize users opening channels in init state and gives it the dst_chain which does not have the src_chain in the allowlist

I also took the opportunity to ensure ChainAllowlist is only updated if the domain is already instantiated.

@dariolina I have currently set the reserver portion protocol gets to 20%. Let me know if the percentage needs to be changed.

Closes: #2654

Code contributor checklist:

• I have read, understood and followed contributing guide

[NingLin-P]

I'm confused, is "the end to end flow" mean even if the src chain is not in the allowlist the dst chain still accepts the init_channel req and then waits for the src chain to close the channel manually?

IMHO, in this case, ideally the dst chain should return an error XDM response and the src chain closes the init status channel when processing that response, so there is no need to close the init status channel manually. Or maybe I missed something.

[vedhavyas]

Right now, dst_chain will return the Err through XDM and src_chain will not open the channel. I have let the closing channel to be a manual process since there is a reserve fee and channel initiator will have the need to unlock their funds. This ensures they not only pay the portion of reserve fee but also needs to pay for the closing channel fee as well. If the src_chain closing it automatically, they no one is paying for it instead

[NingLin-P]

If the src_chain closing it automatically, they no one is paying for it instead

This seems like a broader problem for any XDM response, e.g. for transporter if the request is rejected by the dst chain, the src chain needs to refund the user when handling the XDM response and the user should also pay for such action.

[vedhavyas]

Yes that is the current behavior. What Am i missing ?

[NingLin-P]

I mean if the XDM resp tx fee is a problem then it also exists in the endpoint XDM, if we follow the same approach as the close channel XDM to fix it, we will need the user manually submit extrinsic to get refund, which is obvious a bad UX. We should consider a better solution like pre-charge the XDM resp tx fee when the user send the close channel/transporter XDM.

[NingLin-P]

From your comment the reason we need to manually close channel here is we want the XDM response tx fee to get paid, but the same situation exists in the XDM response of transporter and to collect the tx fee of the XDM response of transporter we will require the user manually submit extrinsic to handle the transporter XDM response, which is obvious a bad UX. So I proposed other solutions like pre-charge the XDM resp tx fee when the user sends the close channel/transporter XDM.

[vedhavyas]

no, that is not the case. Transporter automatically reverts when it receives the err from dst_chain and fees are already acounted for. There is a test case for this specifically. For Closing channel, user needs to manually submit the close request request

[NingLin-P]

For Closing channel, user needs to manually submit the close request request

There are 2 scenarios:

1. The channel is open before the src chain is removed from the allowlist
   • In this scenario, the user does need to manually submit extrinsic to close the channel as we agreed in XDM: Close channels when the chain is removed from the Allowlist #2815
2. The src chain is removed from the allowlist (or the src chain is never added to the allowlist at all) and then an open channel request comes
   • In this PR, even if the src chain is not in the allowlist, the dst chain will still accept the request and init the channel in its state then wait for the src chain to close the channel manually

My question is for scenario 2 why "For Closing channel, user needs to manually submit the close request request" is necessary? if it is all about the XDM response tx fee it can be handled just like the transporter i.e. the dst chain rejected the open channel request and returns an error XDM response, so the channel is never initialized is the dst chain.

[vedhavyas]

if we keep disregard the message then relayer would end up trying for however long the message exisits on source chain. Instead we init the channel once and no messages are accepted until user submits the close channel and cleanups all the storage. I like the second option because its completes the flow and relayer can move on

[NingLin-P]

if we keep disregard the message then relayer would end up trying for however long the message exisits on source chain.

Okay, it makes sense in this perspective

[vedhavyas]

Apologies for the force push. I wanted to handle this audit issue - #2807
in this PR since the changes are mostly co-related

[dariolina]

20% sounds fine as first approximation. I have added it to spec subspace/protocol-specs#39

[NingLin-P]

If the src_chain closing it automatically, they no one is paying for it instead

This seems like a broader problem for any XDM response, e.g. for transporter if the request is rejected by the dst chain, the src chain needs to refund the user when handling the XDM response and the user should also pay for such action.

[NingLin-P]

This may be another DDoS attack vector where the channel is closed in the dst chain but the src chain can still keep sending XDM and force the dst chain to handle.

[vedhavyas]

This is only valid when src_chain has not closed the channel yet but dst_chain did. This is the expected change to ensure in transit messages are processed.

Once the channel is closed, src_chain should not initiate new messages

[NingLin-P]

If the src domain is compromised i.e. controlled by a single entity then they can filter the close channel response when constructing bundle to keep the channel open on the src domain and keep sending XDM to the dst domain.

[vedhavyas]

This problem does not exist if the src_domain is sending messages since it wont be able to send any messages after the channel is closed. Any messages sent before channel is closed will be processed and responses are executed.

If the dst_chain closes the channel they will not be able to send any messages to src_chain but sure the operator of the src_chain may exclude the Channel close message from the dst_chain in the bundlee. Any messages sent from src_chain to dst_chain, since src_chain did not receive the message to close channel, will be responded with Err and necessary fee is taken by the relayer. So there is a cost associated with this.

The reason for this change it to ensure following
dst_chain closes the channel but simultaneouly src_Chain sends messages. These messages wont be accepted and relayer will continue to try and fail. Allowing messages to land then would unblock the relayer while they still get their fee.

IMHO, while they can send messages and pay for the relay fee, this will not harm the dst_chain since it always reponds Err.

If you have a better approach that can solve the audit issue mentioned above, please mention them