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

bip-taro: modify asset ID generation, require unique commitments #21

Merged
merged 1 commit into from
Apr 29, 2022

Conversation

Roasbeef
Copy link
Owner

This commit contains a few important changes:

  • The asset ID now includes the output index, which ensures they're
    unique and also allows for created assets to live in distinct outputs.
  • A concrete requirement (handwavy statement was there before) that only
    a single taro commitment exists in a script tree at any given time. To
    achieve this, we take advantage of the BIP 340 tagged hash contract,
    to ensure that the commitment is either the only element, or is the
    leftmost/rightmost element.
  • Building off the above, we then detail a deterministic way to burn
    assets, as well as prove to others that assets have been burnt. The
    asset burning will play a role in the upcoming re-genesis protocol
    feature.

One additional change during transfers is for parties to reveal the
sibling hash of the taro root commitment so uniqueness verification can
be done on that level. In addition, a similar reveal occurs for the
other outputs (if they exist) that don't explicitly commit (as far as
one party is aware) to any other assets.

@Roasbeef
Copy link
Owner Author

cc @dstadulis @jharveyb

@Roasbeef
Copy link
Owner Author

I still need to propagate this change to what needs to be revealed during swap transactions, which will also propagate to the PSBT BIP.

I also reference creating a BIP specifically for a deterministic algorithm for consisting a compliant commitment and tapscript tree based on either the set of leaf elements (the normal scripts) or the existing root hash (created via some other algo).

bip-taro.mediawiki Outdated Show resolved Hide resolved
bip-taro.mediawiki Show resolved Hide resolved
bip-taro.mediawiki Outdated Show resolved Hide resolved
bip-taro.mediawiki Outdated Show resolved Hide resolved
bip-taro.mediawiki Outdated Show resolved Hide resolved
bip-taro.mediawiki Outdated Show resolved Hide resolved
@Roasbeef
Copy link
Owner Author

Pushed up a new commit that tightens up the specification and adds the usual psuedo-code language in my made up language (beefcaml).

bip-taro.mediawiki Outdated Show resolved Hide resolved
bip-taro.mediawiki Show resolved Hide resolved
bip-taro.mediawiki Outdated Show resolved Hide resolved
The following algorithm can be used to verify that an output elsewhere in the
transaction doesn't commit to a duplicate Taro commitment:
<source lang="python">
verify_no_taro_up_my_sleaves(genesis_tx: Tx, taro_root: []byte) -> bool

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😂

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: s/sleaves/sleeves

bip-taro.mediawiki Outdated Show resolved Hide resolved
bip-taro.mediawiki Outdated Show resolved Hide resolved
@Roasbeef
Copy link
Owner Author

Thanks for the review @wpaulino! Pushed up a fixup commit addressing the comments.

@Roasbeef
Copy link
Owner Author

Pushed up the final beefcaml code for asset burning+verification. Will also rebase now so the fixup commits are folded in.

bip-taro.mediawiki Outdated Show resolved Hide resolved
bip-taro.mediawiki Show resolved Hide resolved
The following algorithm can be used to verify that an output elsewhere in the
transaction doesn't commit to a duplicate Taro commitment:
<source lang="python">
verify_no_taro_up_my_sleaves(genesis_tx: Tx, taro_root: []byte) -> bool

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: s/sleaves/sleeves

In addition to verifying that a commitment is unique within the constraints of
a single taproot tree, we also need to verify that there is no similar
commitment across the remaining outputs in the genesis transaction. We can do
this by asserting that:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC this requires having the preimages for every (single tapscript element or top-level tapscript branch) to cover each output in the TX. This seems worth mentioning explicitly.

This also seems like it would interfere with TXs built interactively with multiple parties (for each output added to a PSBT, we would also need to propagate tapscript preimages). Not sure if we can avoid that though.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC this requires having the preimages for every (single tapscript element or top-level tapscript branch) to cover each output in the TX

Correct. In the typical case, I expect genesis transactions to just be 1-in-1 transactions, as there's no need to make a change output. If there is a change output, then we only require that opening.

Added a sentence below this mentioning that. This is intended to be covered in the code fragment below where we examine the commitment opening.

** If top level key was derived using BIP 86, then verify the key derivation.
** If the key commits to a script path:
*** If only a single element is in the tree, verify that isn't a duplicate commitment.
*** If the tree contains more than one element, then verify that the pre-image of the two branches isn't a taro commitment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we checking only for duplicate commitments, or any other commitments? Here (and in the pseudocode) we allow non-duplicate commitments if they are the only tapscript element, but disallow them at depth 1.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So it's sort of implied by the commitment_expected call that we're mainly ensuring that none of the "expected" outputs have a commitment here. As far as allowing non-duplicate commitments (other valid taro commitments), that's accepted here as long as it isn't committing to that specific taro root.

I think I don't fully understand your questions however?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or actually, I think you're getting at the fact that assuming commitment_expected knows which outputs to expect a valid commitment, of we also need to check for the marker byte prefix here of this leaf hash?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was confused about if we are allowing multiple taro commitments in the same TX in general. Regardless of if we are or aren't, the rules / checks should be the same at depth 0 and depth 1.

Here the checks are written as:
-At depth 0:
--verify that isn't a duplicate commitment
-At depth 1:
--verify that the pre-image of the two branches isn't a taro commitment

If we want to allow multiple valid taro commitments, then we should relax the checks at depth 1 / in the MultiLeaf case to only assert that both branch preimages are not duplicate taro commitments.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was confused about if we are allowing multiple taro commitments in the same TX in general

A single output should only have a single commitment. Other outputs may have commitments as well depending on the on chain event (like a transfer for example will produce a transaction with multiple commitments).

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For asset creation, the asset ID includes the output index, so this check here is just wanting to ensure that the verifier is aware of any relevant assets being issued in the transaction.

For transfers, this is needed to ensure that they aren't committing to a history where the transfer didn't happen.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cleared this up offline, will be more explicit here w.r.t what we're trying to verify and also the intent of commitment_expected.

** For each output, the first depth of the control block leaf reveal must be presented:
*** If the output key was created via BIP 86, then simply verify the mapping based on the internal key. In this case there is no script tree being committed to.
*** If the proof is of depth 0, meaning only a single value is committed to, then assert the item isn't a valid Taro asset root.
*** If the proof is of depth 1, then assert that either both elements are tap branches, or one of the items is a leaf that isn't a valid Taro asset root.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we disallow ANY other Taro asset roots in the TX vs. above; IIUC this implies that an asset burn must be the only Taro-related activity in a given TX? (Cannot issue new assets owned by another output, or perform a transfer to another output, etc.).

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC this implies that an asset burn must be the only Taro-related activity in a given TX? (Cannot issue new assets owned by another output, or perform a transfer to another output, etc.).

Great question! The burn operation is allowed when batched with any other taro events (transfer, etc). The specific event here is defined by removal of a commitment from the sub-asset tree and therefore the main taro commitment. The burn verification routine has access to both the non-inclusion proof (verify it's no longer committed to in the simple case of a collectible), and also the new taro root (to verify that this is placed in a unique location).

Copy link
Owner Author

@Roasbeef Roasbeef Apr 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two additional rules are more or less summed up in the taro_commitment_is_valid routine, but I wanted to spell them out here as well.

I made another updated to make this more explicit in that right now it sort of only allows a total burn (no assets committed to at all), vs a selective burn (some assets not committed to, or the committed amounts should be reduced):

For each output, that doesn't commit to the asset tree the non-inclusion proof is rooted at

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a selective burn (some assets not committed to, or the committed amounts should be reduced)

Is it correct to equate:
"some assets not committed to" = burning an entire class of an asset (my collectible, or all my beefbux)
"he committed amounts should be reduced" = Of my 100 beefbux, I burn 20 for a net result of 80 beefbux

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it correct to equate:

Correct.

@Roasbeef Roasbeef force-pushed the bip-taro-uniqueness branch 2 times, most recently from ff93833 to 7ae1e0b Compare April 22, 2022 01:06
Copy link

@dstadulis dstadulis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good, a few questions about some of the pseudo-code, a nit.

bip-taro.mediawiki Outdated Show resolved Hide resolved
bip-taro.mediawiki Outdated Show resolved Hide resolved
Comment on lines 278 to 279
# The pre-image is a leaf, so verify the hashes match up.
case LeafReveal(leaf_bytes):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# The pre-image is a leaf, so verify the hashes match up.
case LeafReveal(leaf_bytes):
# The pre-image is a non-taro leaf, so verify the hashes match up.
case LeafReveal(leaf_bytes):

Is this code verifying that the hashes match up (equates to something) or that their characteristics conform to what aught to be (no conflicts)

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC, both: we verify that the pre-image is indeed a tapleaf (as bound by the tagged hash output) and we can re-derive that to arrive at the same sibling hash contained in the normal control block proof.

bip-taro.mediawiki Outdated Show resolved Hide resolved
bip-taro.mediawiki Show resolved Hide resolved
** For each output, the first depth of the control block leaf reveal must be presented:
*** If the output key was created via BIP 86, then simply verify the mapping based on the internal key. In this case there is no script tree being committed to.
*** If the proof is of depth 0, meaning only a single value is committed to, then assert the item isn't a valid Taro asset root.
*** If the proof is of depth 1, then assert that either both elements are tap branches, or one of the items is a leaf that isn't a valid Taro asset root.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a selective burn (some assets not committed to, or the committed amounts should be reduced)

Is it correct to equate:
"some assets not committed to" = burning an entire class of an asset (my collectible, or all my beefbux)
"he committed amounts should be reduced" = Of my 100 beefbux, I burn 20 for a net result of 80 beefbux

bip-taro.mediawiki Show resolved Hide resolved
bip-taro.mediawiki Outdated Show resolved Hide resolved
bip-taro.mediawiki Outdated Show resolved Hide resolved
bip-taro.mediawiki Outdated Show resolved Hide resolved
bip-taro.mediawiki Outdated Show resolved Hide resolved
This commit contains a few important changes:

* The asset ID now includes the output index, which ensures they're
  unique and also allows for created assets to live in distinct outputs.
* A concrete requirement (handwavy statement was there before) that only
  a single taro commitment exists in a script tree at any given time. To
  achieve this, we take advantage of the BIP 340 tagged hash contract,
  to ensure that the commitment is either the only element, or is the
  leftmost/rightmost element.
* Building off the above, we then detail a deterministic way to burn
  assets, as well as prove to others that assets have been burnt. The
  asset burning will play a role in the upcoming re-genesis protocol
  feature.

One additional change during transfers is for parties to reveal the
sibling hash of the taro root commitment so uniqueness verification can
be done on that level. In addition, a similar reveal occurs for the
other outputs (if they exist) that don't explicitly commit (as far as
one party is aware) to any other assets.
@Roasbeef
Copy link
Owner Author

Roasbeef commented Apr 29, 2022

Pushed a new commit with the following updates:

  • During asset creation, we indeed only care that no other duplicate commitments for that asset exist.
  • For asset burning, we validate the normal non-inclusion proof, but for each other output that can commit to a to commitment, we verify that either it doesn't has a taro commitment or it has one and the asset is no longer committed to.

Here's the comparison: https://github.com/Roasbeef/bips/compare/ec30a546bcd0187141aca8988d7595337418a7e7..70fb13a4a76d9f20d0c3c8dac55cfb10df850e3b

@Roasbeef
Copy link
Owner Author

Working on a similar PR to tighten up the bip-taro-vm doc with more precise algorithms.

@Roasbeef Roasbeef merged commit 70bbfc5 into bip-taro Apr 29, 2022
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

Successfully merging this pull request may close these issues.

4 participants