-
Notifications
You must be signed in to change notification settings - Fork 220
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
docs: rfc 0310 asset implementation #3340
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good, found a few spelling/grammar points but the approach seems good to me
Co-authored-by: Byron Hambly <[email protected]>
Co-authored-by: Byron Hambly <[email protected]>
Co-authored-by: Byron Hambly <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good so far, few minor texts things.
The one thing I either don't yet understand properly is the "lockedBlock" as it seems like it goes against anything blockchain.
|
||
New fields added to `OutputFeatures` | ||
|
||
Note: this replaces some information in [RFC-0311](RFC-0310_AssetImplementation.md) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps we should add a cross-reference table to make it easy to see which fields are required for what flags.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the bit fields are set up correctly, it should be easy to make clear definitions in the code (and this RFC).
NFT_REGISTRATION = NON_FUNGIBLE | REGISTER
NFT_CHECKPOINT = NON_FUNGIBLE | CHECKPOiNT
etc
The bitflags crate actually has this functionality built right in.
confirm that block in `base_layer_block` is still present in the base node that they are connected to. | ||
|
||
If a validator node has reached the Commit stage, it must prevent the base layer from reorging by issuing a GRPC call to | ||
`LockBlock` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to properly think how this is implemented. This does open up a lot of attack vectors for a call like this.
Should we not handle this the same way we handle a double spend or normal transaction for that matter. It's the validator node that cares that the checkpoint is in the main chain. A specific baseNode won't care about "Jhon's private checkpoint". If it gets reorged out they need to re-add it. Keeping old forked orphan chains breaks a lot of the block_chain ideas and needs special rules to cover those.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets take this example:
There are 2 NFTs, A
and B
.
At block height 1 Pete has 1 utxo with funds.
At block height 2 Pete buys a NFT A
with those funds.
At block height 3 we have a checkpoint confirming Pete has NFT A
Now the block chain reorgs back to height 1
At block height 2-1 Pete buys a NFT B
with those funds.
At block height 3-1 we have a checkpoint confirming Pete has NFT B
So now at block height 3-1 what does Pete own? NFT A
or B
because according to the "LockedBlock" he owns A
, but according to the main chain he owns B
. Or does he own both and he just claimed a free NFT? When you want to buy NFT A
, who owns it?
According to blockchain rules, NFT A
is the person who owned A
at height 1. As in the history Pete never bought A
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me expand a bit on the rational of it. The problem it is trying to solve is the fact that all committee members must have the same view of the blockchain when they are deciding if an instruction is valid or not. You could remove this probability by requiring commitments to be confirmed by X blocks before they can be used as a proof, but once a Hotstuff node commits to executing a set of instructions, it can't have the landscape underneath it change. It has already told the rest of the committee it will execute the instructions, so there has to be some sort of concept of locking.
Perhaps one way of doing it is to validate and pre-execute all the instructions in the Commit or PreCommit states and then the Decide phase is simply updating the state. I do think that the block that is at the tip should be communicated to the other committee nodes, so that they can confirm their view of the chain is the same
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without an in-depth/practical understanding of hotstuff, doesn't the hotstuff protocol essentially allow for this? If the nodes do not agree on the next state in the Decide
phase, there is still a valid way to abandon the view via the NextView
interrupt. Should the tip change for n - f
committee members, causing the current view to become invalid, nodes could broadcast NextView
and the committee proceeds to the next view. If this comes too late and the view is passed, the reorg would need to be detected and instructions re-proposed for the next view i.e. pulled from the unconfirmed instruction mempool in a same/similar way to base layer transactions.
If there is a temporary network partition, the chain may fork (up to a depth of three), but the protocol guarantees safety via the voting mechanism, and the chain will reconcile once the partition resolves.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The lock_base_node command feels unnecessary.
We could require that as part of the COMMIT step, the Hotstuff committee members include the hash of the block they're building on; and Hotstuff consensus can be reached or declined in the usual way, but I'm not really sure that even this is necessary, as others have also mentioned.
If the chain re-orgs between the time that the COMMIT step is done and a transaction is broadcast, so be it,
it is akin to a wallet broadcasting a transaction based on a slightly outdated view of the chain. The transaction is either still fine, or it gets rejected and the wallet reacts accordingly.
lock_base_node doesn't add any value in my view, because all it does is allow the committee members to have the same (possibly incorrect) view of the chain, and by broadcast time you have the same effect, since the base node is still allowed to reorg.
If there are persistent Byzantine failures due to different members having different views of the base chain, then we could have some sort of "resync" instruction that tries to get all committee members to converge on the same view.
If a committee transaction is rejected, the correct course of action for the committee would depend on the context and would need to be spec'd out a bit more. In any event, the committee is not waiting for the tx to be mined. It will immediately move onto the next instruction batch.
| NON_FUNGIBLE | 0b1000_1000 | This UTXO contains Non Fungile data and must not be combined with other fungible UTXOs | | ||
| ASSET_REGISTRATION | 0b1100_1000 | This UTXO contains registration information for a new asset | | ||
| MINT |0b0100_0000 | This UTXO represents the creation of a new NFT, and is used during the | | ||
| BURN | 0b0010_0000 | The `unique_id` in this UTXO can be spent as a normal input and must not appear in the output set unless accompanied by a `MINT` flag | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This description is a bit confusing, is the mint
supposed to be BURN
?
| asset_registration | struct | Data for registering of an asset. This value **MUST** be present if ASSET_REGISTRATION flag is set | | ||
| asset_registration.name | bytes(64) | utf8 name of the asset. Optional | | ||
| asset_registration.template_ids | Vec<bytes(32)> |The templates that this asset exposes. Usually, at least one will be specified | | ||
| asset_registration.creator_sig | bytes(64) (32 Nonce + 32 sig) | A signature of the UTXO's `commitment` and `script` using the public key in `unique_id` to prove this registration was created in this UTXO | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this imply that unique_id
is a public key? It is not mentioned above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see it is mentioned below in more detail so perhaps just a mention of its public key nature in the table.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the example, it's just a hash though. So not always a pubkey
| checkpoint.merkle_root | bytes(32) | Merkle root of the sidechain data. The format and meaning of this data is specific to the sidechain implementation | | ||
|
||
|
||
Note: `unique_id` is not always required to be a public key |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Elaborate? My reading is that its a public key for asset registration but any random ID for a token?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this was also unclear for me.
|
||
**New Consensus Rule** | ||
|
||
When receiving a UTXO with the `MINT` and `NON_FUNGIBLE` flags set and the `ASSET_REGISTRATION` unset, the `minting_proof_signature` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minting_proof_signature
is not mentioned anywhere else. Where does it live?
|
||
### Transferring a unique token | ||
|
||
The `MINT` flag is only set on the first UTXO a token appears in. When it is transferred to a new UTXO, it should only have the `NON_FUNGIBLE` flag set. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we need some kind of consensus rule here to enforce that the unique_id
s of any NON_FUNGIBLE
flagged inputs must appear in the outputs of this block?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found you mention this down in the example but still think we should have a formal New Consensus Rule section here.
A token can be burnt by setting the `BURN` flag and then spending the UTXO. A token with the `BURN` set should be considered as valid as long as it is in the unspent set | ||
and is only destroyed once the UTXO is spent. | ||
|
||
**New Consensus Rule** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we perhaps need another consensus rule that enforces the burn that means you cannot undo the burn by spending to another output without the BURN flag?
|
||
The unique_id can be a specificly chosen id, but can also be a random public key. | ||
|
||
There is no limit to the number of different sidechain checkpoint tokens an asset has, but it will usually be one. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with this, I don't think there is any value in trying to use consensus to enforce anything about the checkpoints. They are purely there as on-chain commitments that users of the sidechain can point to and cannot be used for nay kind of enforcement by the main chain nodes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, think this gives a good picture of the proposed solution. Mainly think that some clarification on the dual usage of unique_id
is needed.
|
||
| Name | Type | Description | | ||
| --- | --- | --- | | ||
| unique_id | bytes(32) | A unique id representing a token. This value must be present only when the NON_FUNGIBLE flag is set. The pair (parent_pub_key, unique_id) will be unique across the unspent set as described below | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| unique_id | bytes(32) | A unique id representing a token. This value must be present only when the NON_FUNGIBLE flag is set. The pair (parent_pub_key, unique_id) will be unique across the unspent set as described below | | |
| unique_id | bytes(32) | A unique id representing a token. This value MUST be present only when the NON_FUNGIBLE flag is set. The pair (parent_pub_key, unique_id) will be unique across the unspent set as described below | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd love us to look for ways to conserve precious block space. The additional fields on OutputFeatures alone run to over 384 bytes (by very quick mental math) depending on the specifics.
There are a couple of signatures and fields here that I believe, with careful consideration, could be folded into the current UTXO fields, and offer some privacy for assets to boot.
E.g. The unique id might be added to the commitment as C = kG + vH + tT for instance.
|
||
### Registering an asset | ||
|
||
A new asset is registered by creating a UTXO with the ASSET_REGISTRATION flag set, a new public key in `unique_id`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe expand on the usage of unique_id. If I understand correctly it can represent two types of data within the protocol. On registration, it is the public key of the registered asset and MUST be placed into the parent_public_key
of the minted child token UTXOs. On a token UTXO it can be any sequence of bytes unique to the parent_public_key
namespace. Any reason we don't use a separate field for asset "class" public key?
When receiving a UTXO with the `MINT` and `NON_FUNGIBLE` flags set, either the `parent_public_key` must be populated, | ||
or the `ASSET_REGISTRATON` flag must also be set. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: ASSET_REGISTRATON
I found this a little clearer
When receiving a UTXO with the `MINT` and `NON_FUNGIBLE` flags set, either the `parent_public_key` must be populated, | |
or the `ASSET_REGISTRATON` flag must also be set. | |
When receiving a UTXO with the `MINT` and `NON_FUNGIBLE` flags set, the `parent_public_key` and/or the `ASSET_REGISTRATION` flag must be set. |
confirm that block in `base_layer_block` is still present in the base node that they are connected to. | ||
|
||
If a validator node has reached the Commit stage, it must prevent the base layer from reorging by issuing a GRPC call to | ||
`LockBlock` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without an in-depth/practical understanding of hotstuff, doesn't the hotstuff protocol essentially allow for this? If the nodes do not agree on the next state in the Decide
phase, there is still a valid way to abandon the view via the NextView
interrupt. Should the tip change for n - f
committee members, causing the current view to become invalid, nodes could broadcast NextView
and the committee proceeds to the next view. If this comes too late and the view is passed, the reorg would need to be detected and instructions re-proposed for the next view i.e. pulled from the unconfirmed instruction mempool in a same/similar way to base layer transactions.
If there is a temporary network partition, the chain may fork (up to a depth of three), but the protocol guarantees safety via the voting mechanism, and the chain will reconcile once the partition resolves.
"flags": "0b1101_1000", | ||
// Special unique id, | ||
"unique_id": "0x0000000000000000", | ||
"parent_public_key": "<yacht_clicker_pub_key", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor:
"parent_public_key": "<yacht_clicker_pub_key", | |
"parent_public_key": "<yacht_clicker_pub_key>", |
Missing in a few other places
| Name | Type | Description | | ||
| --- | --- | --- | | ||
| unique_id | bytes(32) | A unique id representing a token. This value must be present only when the NON_FUNGIBLE flag is set. The pair (parent_pub_key, unique_id) will be unique across the unspent set as described below | | ||
| parent_pub_key | bytes(32) | A namespacing field that also allows a heirachy of assets. In most cases this will be the asset that a token belongs to, but future uses may be possible as well | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| parent_pub_key | bytes(32) | A namespacing field that also allows a heirachy of assets. In most cases this will be the asset that a token belongs to, but future uses may be possible as well | | |
| parent_pub_key | bytes(32) | A namespacing field that also allows a hierarchy of assets. In most cases this will be the asset that a token belongs to, but future uses may be possible as well | |
Co-authored-by: Philip Robinson <[email protected]>
Co-authored-by: Byron Hambly <[email protected]>
Co-authored-by: Byron Hambly <[email protected]>
Co-authored-by: Byron Hambly <[email protected]>
Co-authored-by: Byron Hambly <[email protected]>
# Conflicts: # RFC/src/RFC-0310_AssetImplementation.md
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Besides the specific comments left in the RFC, my overriding thinking while digesting this RFC is will this implementation scale to handle millions of assets, each with millions of tokens and hundreds or thousands of descriptors / state variables / metadata, with millions of these tokens updating state many times a minute?
And I think while this RFC has some great specific suggestions and sits on the back of a concrete, working PoC my sense is that in general, the answer is "not yet".
How do we scale to millions of assets, billions of tokens and millions of state changes a day?
One way to look at this is -- 1 MB/block * 30 / hr * 24 hr/day = 720 MB per day is the max data that every node on the network will be adding. (It's roughly equivalent to 900k new standard MW UTXOs a day). This does not equate to millions of token mints and tens of millions of state changes unless that state, and the tokens1 are kept completely off chain.
What can the Tari Base layer handle in terms of assets? I would say that the full extent of the base layer's responsibility is to act as the primary ledger for Tari token transactions, a global register for assets (just the asset, not the tokens) and a notary service for sidechains managing the assets.
All state, including tokens, must effectively live off chain.
1: Tari must allow token holders to "eject" tokens from side chains and keep them in cold storage, or as a means of transferring them to a different side chain, assuming direct SC<->SC transfers aren't possible. However, this is optional, and we should find a way to do this within the current framework of Tari base layer transactions, including the use TariScript. In addition, we should disincentivise token holders to do this by making it relatively expensive to move a token on-chain by means of overweighted transaction fees for token storage; not because it's not a valid use case, but because the base layer simply cannot scale fast enough to make this something you "just do for the hell of it".
Like subatomic particles, tokens might pop into existence on a side-chain, move around a bit, and then get annihilated without the main chain ever being aware of it. This is a vital characteristic to achieve the scaling goals of Tari, and precludes the requirement that all mints must be recorded on the base layer.
Furthermore, tokens will need to be able to be minted at rates and in volumes that far exceed the block sizes and intervals of the main chain, further underlying the need for them to be created off-chain.
Coming slightly closer to home, I think the asset registration transaction described in this RFC is a really good foundation to work from. I suspect we can trim things down a little, but it's 100% in the right direction imho.
Notarisation / checkpointing is another key part of the base layer functionality. Again, given how little the base layer knows about the assets on side chains, the checkpoint transaction can likely also be shrunk significantly. There are also many details to iron out, for example what to do with cheats (and how we even define cheats). Some of the other RFCs need to be dusted off and revisited in this regard, since we've learnt a lot since those were first drafted.
I also understand that it's out of the scope of this RFC, but the topic of micropayments for validator nodes, atomic swaps for token purchases (99.999% of which should also be done off chain) and asset maintenance funding need to be covered.
There's a comment about decentralisation and censorship resistance (D&CR). This does not mandate everything to live on chain. It's perfectly doable to achieve D&CR on a suitably distributed side chain; one that's focussed on delivering D&CR for the asset that cares about that. Meanwhile, other assets that don't care as much (e.g. my grannie's biscuit shop's loyalty token) can sit on cheap, fast side-chains, comprising as few as one node.
In summary, this RFC is extremely valuable as a proof of concept, and for reigniting the conversation about how to achieve Tari's primary aim. Perhaps the RFC should place a link to this PR (how meta!) to preserve all the comments and discussion, and then I think it should be merged and added to the "canon".
|
||
New fields added to `OutputFeatures` | ||
|
||
Note: this replaces some information in [RFC-0311](RFC-0310_AssetImplementation.md) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the bit fields are set up correctly, it should be easy to make clear definitions in the code (and this RFC).
NFT_REGISTRATION = NON_FUNGIBLE | REGISTER
NFT_CHECKPOINT = NON_FUNGIBLE | CHECKPOiNT
etc
The bitflags crate actually has this functionality built right in.
| checkpoint.merkle_root | bytes(32) | Merkle root of the sidechain data. The format and meaning of this data is specific to the sidechain implementation | | ||
|
||
|
||
Note: `unique_id` is not always required to be a public key |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this was also unclear for me.
confirm that block in `base_layer_block` is still present in the base node that they are connected to. | ||
|
||
If a validator node has reached the Commit stage, it must prevent the base layer from reorging by issuing a GRPC call to | ||
`LockBlock` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The lock_base_node command feels unnecessary.
We could require that as part of the COMMIT step, the Hotstuff committee members include the hash of the block they're building on; and Hotstuff consensus can be reached or declined in the usual way, but I'm not really sure that even this is necessary, as others have also mentioned.
If the chain re-orgs between the time that the COMMIT step is done and a transaction is broadcast, so be it,
it is akin to a wallet broadcasting a transaction based on a slightly outdated view of the chain. The transaction is either still fine, or it gets rejected and the wallet reacts accordingly.
lock_base_node doesn't add any value in my view, because all it does is allow the committee members to have the same (possibly incorrect) view of the chain, and by broadcast time you have the same effect, since the base node is still allowed to reorg.
If there are persistent Byzantine failures due to different members having different views of the base chain, then we could have some sort of "resync" instruction that tries to get all committee members to converge on the same view.
If a committee transaction is rejected, the correct course of action for the committee would depend on the context and would need to be spec'd out a bit more. In any event, the committee is not waiting for the tx to be mined. It will immediately move onto the next instruction batch.
|
||
| Name | Type | Description | | ||
| --- | --- | --- | | ||
| unique_id | bytes(32) | A unique id representing a token. This value must be present only when the NON_FUNGIBLE flag is set. The pair (parent_pub_key, unique_id) will be unique across the unspent set as described below | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd love us to look for ways to conserve precious block space. The additional fields on OutputFeatures alone run to over 384 bytes (by very quick mental math) depending on the specifics.
There are a couple of signatures and fields here that I believe, with careful consideration, could be folded into the current UTXO fields, and offer some privacy for assets to boot.
E.g. The unique id might be added to the commitment as C = kG + vH + tT for instance.
| asset_registration | struct | Data for registering of an asset. This value **MUST** be present if ASSET_REGISTRATION flag is set | | ||
| asset_registration.name | bytes(64) | utf8 name of the asset. Optional | | ||
| asset_registration.template_ids | Vec<bytes(32)> |The templates that this asset exposes. Usually, at least one will be specified | | ||
| asset_registration.creator_sig | bytes(64) (32 Nonce + 32 sig) | A signature of the UTXO's `commitment` and `script` using the public key in `unique_id` to prove this registration was created in this UTXO | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the example, it's just a hash though. So not always a pubkey
"metadata": [ | ||
protobuf | ||
serialized | ||
template | ||
data | ||
as | ||
Vec<u8> | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"metadata": [ | |
protobuf | |
serialized | |
template | |
data | |
as | |
Vec<u8> | |
] | |
"metadata": [..] // protobuf serialized template data as Vec<u8> |
and others
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
metadata
isn't mentioned anywhere prior to this. Is this the serialisation of the fields
field above?
If so, it represents a potential scaling problem. Ignoring the fact that I'm limited by the base layer block size limit for now, what if I want to initialise a thousand fields for my asset?
"features": { | ||
"flags": "0b1100_1000", // ASSET_REGISTRATION | ||
"unique_id": [ | ||
generated_pubkey |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this the same as <yacht_clicker_pub_key>
below?
Co-authored-by: SW van Heerden <[email protected]>
Co-authored-by: SW van Heerden <[email protected]>
WIP of RFC for Asset Implementation