paritytech / polkadot

Polkadot Node Implementation
GNU General Public License v3.0
7.12k stars 1.58k forks source link

Confusing documentation for xcm ConcrateFungible #2547

Open h4x3rotab opened 3 years ago

h4x3rotab commented 3 years ago

As pointed out here:

/// Throughout XCM, messages are authored such that *when interpreted from the receiver's point of view* they will
/// have the desired meaning/effect. This means that relative paths should always by constructed to be read from the
/// point of view of the receiving system, *which may be have a completely different meaning in the authoring system*.

It clearly said the XCM should be interpreted from the receiver's point of view. Does it mean the sender should construct the XCM in a way that the asset MultiLocation should be related to the receiver, or to the sender?

Let me take an example. If I'm at Acala parachain, and want to send 10 ACA to Phala parachain back and forth. I will need to construct two different messages DepositReserveAsset on the Acala side, and InitiateReserveWithdraw on the Phala side. The following are the two different understanding. Please advise which one is correct.

Understanding 1: AssetId is constructed from sender's point of view

Message Sender Receiver Constructed AssetId Received AssetId
DepositReserveAsset Acala Phala "ACA" Parent/Parachain(Acala)/"ACA"
InitiateReserveWithdraw Phala Acala Parent/Parachain(Acala)/"ACA" "ACA"

We tried this with the default xcm-executor, xcm-handler provided by Cumulus, and this construction worked. However by definition it's conflicted with the description from the XCM documentations.

Understanding 2: AssetId is constructed from receivers's point of view

Message Sender Receiver Constructed AssetId Received AssetId
DepositReserveAsset Acala Phala Parent/Parachain(Acala)/"ACA" Parent/Parachain(Acala)/"ACA"
InitiateReserveWithdraw Phala Acala "ACA" "ACA"

(Not tried yet)

xlc commented 3 years ago

Sender's point of view. And xcm will reanchor it so receiver will receive it on the receivers' point of view

https://github.com/paritytech/polkadot/blob/master/xcm/xcm-executor/src/assets.rs#L30

h4x3rotab commented 3 years ago

@xlc Thanks. I also think it makes sense because that's how we were using it before. However the comments in the ConcrateFungible is written in a way conflicted with the reanchor logic you posted.

I guess the intention of the comments is like below, but we need the confirmation from polkadot team:

tolak commented 3 years ago

@h4x3rotab pallet xcm-handle in cumulus repo actually do nothing but help to wrap XCM message to HRMP message and impl DownwardMessageHandler/HrmpMessageHandler . Only XCM message itself and xcm-executor are real components of XCM system. So i think xcm-exector should be the only component that executes XCM and decide where to send them to. However some logic within xcm-handler acts as a XCM router, that's a little bit confuse

shaunxw commented 3 years ago

@h4x3rotab IMO there is no problem in documentation. My understanding is that in your Understanding 1, the XCM message with DepositReserveAsset is not sent to receiver(Phala), instead it is sent to self parachain(Acala) and got executed. Let's say it's scenario 1.

In comparison, if the sending parachain doesn't send XCM to itself, but sends it to receiver directly, the message would be a different one: obviously there would be no DepositReserveAsset, but Xcm::ReserveAssetDeposit to notify the receiver about reserve account deposit, along with an order of deposit to recipient account. In this case, the relative path would be Parent/Parachain(Acala)/"ACA". In this case the sending parachain needs to manually handle withdraw and reserve account deposit. Let's say it's scenario 2.

So as we see above, messages are different based on whom it is sent to. In both cases the relative path is the point view of receivers. Additionally, in scenario 1, xcm-executor helps to convert message to the one in scenario 2 in execute_effect (and also re-anchor as @xlc mentioned), and then send it to receiver parachain.