Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feedbacks and questions #1

Closed
xlc opened this issue Jun 14, 2020 · 14 comments
Closed

Feedbacks and questions #1

xlc opened this issue Jun 14, 2020 · 14 comments

Comments

@xlc
Copy link
Contributor

xlc commented Jun 14, 2020

I have many questions and I will put all of them in here for now to avoid creating too many issues at once.

For questions worth further discussion, could spawn a new issue once needed.


Can we have less acronyms?.

It makes reading this really hard especially when some acronyms appear before the definitions.

The full name, (e.g. Universal Destination Indicator) gives me enough context to continue the reading whereas UDI means I need to search for full name first to understand the sentence that I was reading.


XCM version: Compact<u32>

What's reason of using compact? Wouldn't a u8 be enough? I can see we have > 64 versions at some stage but less likely > 256? and even if that's the case, it is easy to make 0xff to be a meta version and extend version into two bytes.

Same for type: Compact<u32>, a variable length instructions format could be more space efficient and also better typing with Rust implementation like

enum Message {
  FAX { asset: MultiDest, dest: MultiDest, source: MultiDest },
  FAC { ... },
  SomeSubMessageFamily(SubMessage),
}

Nullability

source: MultiDest for FAX / FAC/TA seems nullable by looking at the description. Better to make that explicitly.


FAX

A type 3 (multi-level) ID indicates that an FAC message is needed.

Does it means an additional FAC message is needed to transfer from the primary account to sub destination?


How does tx fee works for those message / execution / storage usage?

Presumably spec-confirming relaychains and parachains should have some on chain state used to store the MultiAsset representation for native and foreign assets and process the messages will require some execution time. So someone will need to pay for those to avoid DOS issue and provide incentives to validators/collators. So who pays the fee and how does the fee get charged when multiple chains are involved?

I guess for example FAC, the standard extrinsic tx fee will be charged on origin chain and the recipient chain can charge some portion of the asset been credited as tx fee.

All existential deposit & dust thing will make it bit more complicated.


MultiAsset

version: Compact<u32>: unsure why it is Compact<u32> over u8. I just cannot think how it can be more than 256 asset meta types.

id: [u8; 3]: I feel three bytes is not enough. Also while it is readable, I can see a lot of troubles of naming clashes. For example, multiple teams are working on BTC bridges and each of their cross chain BTC are not the same. Now who get to use BTC? Make it four bytes can help that each of them have a variant of BTC. e.g. XBTC for ChainX and IBTC for Interlay or RBTC for RenVM. Four bytes can also be viewed as u32 so people can also to choose numeric values.

But still, how is class id allocated? People will be able to issue their own tokens via smart contract / pallet-generic-asset and their asset id could be u32 or a H256 contract address or whatever types. It just won't be possible to map that into some fixed number bytes.


AssetInstance

The SCALE encoded representation of Compact<u16> / Compact<u128> are identical, it is only when decoding may overflow. I see no reason to introduce new variant to differentiate them.

I feel it will be useful to generalize this bit more to allow this format to be reused. For example, asset id / class id type can also be this format as well.


MultiDest

It seems like a relative path, which required a context to be able t resolve to a concrete destination. e.g. Type 1 Account Index will represent different accounts if resolved on different networks.

To make things less confusing, I will like to see an absolute unambiguous representation of a destination. e.g. an unambiguous way to represent an address on Kusama vs Polkadot.

Should this be able to represent BTC address? If yes, how?


Fungible/Non-fungible Asset should always have a canonical owner (the entity controls the definition of this asset, the rules, the permissions), which is usually a para/relay/solo chain, but can also be a smart contract live inside a chain. For some assets that may not have a canonical chain (e.g. USDT), it still have canonical owner controlling it, which may be represented as a DID.

I think we need a better way to uniquely and universally address an asset without unambiguity. To optimize storage space and performance, alias or registry could be used to provide short representation.

@apopiak
Copy link

apopiak commented Jun 15, 2020

+1 for reducing Acronyms and/or explaining them earlier/better.

@jak-pan
Copy link

jak-pan commented Jun 17, 2020

+1 for insufficient asset identifier.

As AMM parachain, we will be issuing share tokens, and possibly other kinds of tokens for the users. If they are trade-able on different exchange chains, I can quickly see this not being enough.

If there are other financial instruments on top of it, also issuing tokens, this will be used up very quickly.

I'd also imagine that it would hold information about its original issuing chain, since the token could have special functionality on that chain and it could partially prevent collisions and / or confusion. - Somehow missed source on the first read.

@gavofyork
Copy link
Contributor

Thanks for the feedback - once things settle with CC1 I'll make some revisions.

@gavofyork
Copy link
Contributor

I can see we have > 64 versions at some stage but less likely > 256? and even if that's the case, it is easy to make 0xff to be a meta version and extend version into two bytes.

I don't want to try to predict how many versions we'll have and at this stage I don't think there's enough of a distinction between the limit of 64 and 256 to stray from standardised and well-supported datatypes like Compact and create a new kind of Compact that is more than one byte only when >254.

@gavofyork
Copy link
Contributor

Does it means an additional FAC message is needed to transfer from the primary account to sub destination?

Yes, but I'll be revising this shortly anyway, so I've removed it for now.

@gavofyork
Copy link
Contributor

source: MultiDest for FAX / FAC/TA seems nullable by looking at the description. Better to make that explicitly.

Removed, since it should be clear now without.

@gavofyork
Copy link
Contributor

version: Compact: unsure why it is Compact over u8. I just cannot think how it can be more than 256 asset meta types.

My desire is to keep the standard lightweight in terms of both spec and impl by reusing pre-existing (SCALE) standards, while also guaranteeing it to be futureproof.

@gavofyork
Copy link
Contributor

I feel three bytes is not enough.

Fixed

@gavofyork
Copy link
Contributor

gavofyork commented Jun 23, 2020

I see no reason to introduce new variant to differentiate them

This is mostly because we have the capacity to do so and it allows the XCM implementations the ability to discard input data that they don't support early on without having to try and potentially fail to decode at the scale/Compact stage.

In the case of a single-byte index, it also saves one byte from the encoded data if the value is 64 or above.

@gavofyork
Copy link
Contributor

asset id / class id type can also be this format as well

asset id and class id do not have any context in which an index could be evaluated and would need to make sense across all possible chains/consensus systems on which this message could be received. an instance id has already been restricted to an NFT asset-class which may identify its NFTs e.g. by index.

@gavofyork
Copy link
Contributor

How does tx fee works for those message / execution / storage usage?

This will be addressed in later revisions, but basically it'll be up to the sender to ensure that its sovereign account on the receiver has sufficient funds to pay for any associated fees. I'll be introducing new proposals to allow for message batching and temporary accounts in order to facilitate this.

@gavofyork
Copy link
Contributor

I think we need a better way to uniquely and universally address an asset without unambiguity.

For sure some sort of registry is required, since we are dealing with a global namespace. For the present I'll leave it as a Vec<u8> and the problem can be considered in isolation.

@gavofyork
Copy link
Contributor

To make things less confusing, I will like to see an absolute unambiguous representation of a destination. e.g. an unambiguous way to represent an address on Kusama vs Polkadot.

Should this be able to represent BTC address? If yes, how?

Should all be addressed with the latest revisions.

@xlc
Copy link
Contributor Author

xlc commented Jun 24, 2020

Thanks for answering!

@xlc xlc closed this as completed Jun 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants