Merkleized metadata #78
Conversation
|
Maybe we can have a reference test section with some test data? We already have PoC so it shouldn't be hard to make some just by taking from existing tests. |
Can you clarify what you mean by test data? What should this section show? |
|
A random example: https://eips.ethereum.org/EIPS/eip-145#test-cases In this case, I am expecting something like Input: An example metadata JSON |
The metadata is multiple 100KB's big. I don't want to paste this into the document 🙈 I mean I see what you want, but I don't really see on what this brings us here? |
|
I have a couple of questions:
struct BitSequence {
bit_store_type: TypeRef,
bit_order_type: TypeRef,
}Which I personally find it quite annoying because the The proposed struct BitSequence {
num_bytes: u8,
least_significant_bit_first: bool,
}However, unless I'm mistaken, this change won't make it possible to generate the
If that's the case, wouldn't it be beneficial to also mention in the spec what's the canonical way for accessing those field values? Furthermore, it would probably be a good idea to state what the canonical way is for accessing the values for the fields |
https://github.com/bkchr/merkelized-metadata/blob/f73707f4a0d0dea8cf2c22f632eed828f244f377/src/from_frame_metadata.rs#L332-L359 this is the implementation. There are no other reasonable values for
I think the current assumption is that these values are just copied from the chain spec. I mean I left as an unresolved question in the RFC that this isn't really perfect. This goes into the direction of semantic information, which we are not supporting at all. It also ignores chains like asset-hub where you can pay with one of the "unlimited" number of assets. All in all not a great solution. I'm open for suggestions.
Yeah, as you said |
|
No need to put a real metadata. Just a hand crafted minimal one would do. |
|
There a quite a lot of different types and combinations of them that would be required to make any useful test case. I mean I'm in for ensuring that future implementation are compatible, but we should ensure this by testing these implementation against each. |
- Include `type_id` in the `Type` directly. - Change `variant` to `Compact<u32>`
Co-authored-by: Josep M Sobrepere <jm.sobrepere@gmail.com>
Co-authored-by: Josep M Sobrepere <jm.sobrepere@gmail.com>
|
Current implementation: https://github.com/bkchr/merkleized-metadata |
| struct EnumerationVariant { | ||
| name: String, | ||
| fields: Vec<Field>, | ||
| index: Compact<u32>, |
There was a problem hiding this comment.
This should be a u8, right? Or I'm missing something?
There was a problem hiding this comment.
This was done on purpose. From time to time the idea pops up to change this index in an enum to a Compact. So, far no one has done this 🙈
For most of the enums this will be u8 any way when using Compact.
text/0078-merkleized-metadata.md
Outdated
|
|
||
| The `MetadataDigest` itself is represented as an `enum`. This is done to make it future proof, because a `SCALE` encoded `enum` is prefixed by the `index` of the variant. This `index` represents the version of the digest. As seen above, there is no `index` zero and it starts directly with one. Version one of the digest contains the following elements: | ||
|
|
||
| - `type_information_tree_root`: The root of the [Merkleized type information](#merkleizing-type-information) tree. |
There was a problem hiding this comment.
Should the link point to "Building the Merkle Tree Root?"
text/0078-merkleized-metadata.md
Outdated
| } | ||
| ``` | ||
|
|
||
| To begin with, `TypeRef`. This is a unique identifier for a type as found in the type information. Using this `TypeRef`, it is possible to look up the type in the type information tree. More details on this process can be found in the section [Merkleizing type information](#merkleizing-type-information). |
There was a problem hiding this comment.
Same as above about to which section the link refers to.
Co-authored-by: Santi Balaguer <santiago.balaguer@gmail.com>
Co-authored-by: Santi Balaguer <santiago.balaguer@gmail.com>
Co-authored-by: Santi Balaguer <santiago.balaguer@gmail.com>
Co-authored-by: Carlo Sala <carlosalag@protonmail.com>
|
|
||
| The `Hash` is 32 bytes long and `blake3` is used for calculating it. The hash of the `MetadataDigest` is calculated by `blake3(SCALE(MetadataDigest))`. Therefore, `MetadataDigest` is at first `SCALE` encoded, and then those bytes are hashed. | ||
|
|
||
| The `MetadataDigest` itself is represented as an `enum`. This is done to make it future proof, because a `SCALE` encoded `enum` is prefixed by the `index` of the variant. This `index` represents the version of the digest. As seen above, there is no `index` zero and it starts directly with one. Version one of the digest contains the following elements: |
There was a problem hiding this comment.
Since the enum variant might be encoded as a compact, I'd clarify that it's a u8 in this case (or a compact, I don't really care). Just let's clarify it to make it future-proof
There was a problem hiding this comment.
We can not change SCALE that easily any way. So, I would not mention this here yet.
| CompactU16, | ||
| CompactU32, | ||
| CompactU64, | ||
| CompactU128, |
There was a problem hiding this comment.
Shall we add CompactU256 to make it 100% future-proof? It might happen at some point.
Co-authored-by: Carlo Sala <carlosalag@protonmail.com>
…ws/RFCs into bkchr-merkelized-metadata
| - `address_ty`: The address type used by the chain. | ||
| - `call_ty`: The `call` type used by the chain. The `call` in FRAME based runtimes represents the type of transaction being executed on chain. It references the actual function to execute and the parameters of this function. | ||
| - `signature_ty`: The signature type used by the chain. | ||
| - `signed_extensions`: FRAME based runtimes can extend the base extrinsic with extra information. This extra information that is put into an extrinsic is called "signed extensions". These extensions offer the runtime developer the possibility to include data directly into the extrinsic, like `nonce`, `tip`, amongst others. This means that the this data is sent alongside the extrinsic to the runtime. The other possibility these extensions offer is to include extra information only in the signed data that is signed by the sender. This means that this data needs to be known by both sides, the signing side and the verification side. An example for this kind of data is the *genesis hash* that ensures that extrinsics are unique per chain. Another example is the *metadata hash* itself that will also be included in the signed data. The offline wallets need to know which signed extensions are present in the chain and this is communicated to them using this field. |
There was a problem hiding this comment.
Should the terminology "signed extension" now become "transaction extension" since paritytech/polkadot-sdk#2280
text/0078-merkleized-metadata.md
Outdated
|
|
||
| The `TypeDef` variants have the following meaning: | ||
|
|
||
| - `Composite`: A `struct` like type that is composed of multiple different fields. Each `Field` can have its own type. A `Composite` with no fields is expressed as primitive type `Void`. |
There was a problem hiding this comment.
Maybe mention the order of the fields is significant
| The `TypeDef` variants have the following meaning: | ||
|
|
||
| - `Composite`: A `struct` like type that is composed of multiple different fields. Each `Field` can have its own type. A `Composite` with no fields is expressed as primitive type `Void`. | ||
| - `Enumeration`: Stores a `EnumerationVariant`. A `EnumerationVariant` is a struct that is described by a name, an index and a vector of `Field`s, each of which can have it's own type. Typically `Enumeration`s have more than just one variant, and in those cases `Enumeration` will appear multiple times, each time with a different variant, in the type information. `Enumeration`s can become quite large, yet usually for decoding a type only one variant is required, therefore this design brings optimizations and helps reduce the size of the proof. An `Enumeration` with no variants is expressed as primitive type `Void`. |
There was a problem hiding this comment.
I think it is worth mentioning, as below that each of the Enumeration instances will have the same TypeRef lookup id. This detail helped me understand how this worked.
For
Enumerations all variants have the same unique identifier, while they are represented as multiple type information. All variants need to have the same unique identifier as the reference doesn't know which variant will appear in the actual encoded data.
There was a problem hiding this comment.
I mean this is explained in Generating TypeRef.
| ## Future Directions and Related Material | ||
|
|
||
| - Does it work with all kind of offline wallets? | ||
| - Generic types currently appear multiple times in the metadata with each instantiation. It could be may be useful to have generic type only once in the metadata and declare the generic parameters at their instantiation. |
There was a problem hiding this comment.
I once tried to do this at the scale-info level, see paritytech/scale-info#12, but abandoned it.
It would be considerably easier to do as a second pass over the already generated type information.
|
/rfc propose |
1 similar comment
|
/rfc propose |
|
Hey @bkchr, here is a link you can use to create the referendum aiming to approve this RFC number 0078. Instructions
It is based on commit hash b42ba43600f1ff18e9bfb74bcd042cdea2c983fb. The proposed remark text is: |
|
Voting for this referenda is ongoing. Vote for it here |
|
|
||
| - Does it work with all kind of offline wallets? | ||
| - Generic types currently appear multiple times in the metadata with each instantiation. It could be may be useful to have generic type only once in the metadata and declare the generic parameters at their instantiation. | ||
| - The metadata doesn't contain any kind of semantic information. This means that the offline wallet for example doesn't know what is a balance etc. The current solution for this problem is to match on the type name, but this isn't a sustainable solution. |
There was a problem hiding this comment.
Indeed. My main complaint about cold wallets at this point is that the rendering is sometimes not helpful. If e.g. an account address is rendered as a decimal or hexadecimal array ... that's not helpful in checking the correctness.
|
PR can be merged. Write the following command to trigger the bot
|
|
/rfc process 0x5945e40d7cc049b8176a4de631ab20d7e2bb8ac9576fa4de2e620ec7660a0272 |
|
The on-chain referendum has approved the RFC. |
No description provided.