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

fix(core)!: include issuer public key in contract id hash #4239

Conversation

sdbondi
Copy link
Member

@sdbondi sdbondi commented Jun 28, 2022

Description

  • includes issuer public key and commitment in contract hash
  • update command "wizard" code to fetch contract id from resulting transaction
  • simplify FixedString code
  • removes unused Hashable trait impls

Motivation and Context

The contract ID should be a hash of all contract definition data. Adding the commitment allows contracts with the same name label to exist on the blockchain at the same time.
The contract name functions as a label and does not have to be globally unique. This PR essentially makes the
<issuer_pk, contract_name> pair unique.

How Has This Been Tested?

Existing tests, manually

@sdbondi sdbondi force-pushed the core-contract-id-hash-incl-def-issuer branch 4 times, most recently from de48edb to c10059e Compare June 28, 2022 16:09
@sdbondi sdbondi changed the title fix(core): include commitment in contract id hash fix(core)!: include commitment in contract id hash Jun 28, 2022
@sdbondi sdbondi force-pushed the core-contract-id-hash-incl-def-issuer branch 5 times, most recently from ef36e04 to d9b4a43 Compare June 30, 2022 14:05
Copy link
Collaborator

@SWvheerden SWvheerden left a comment

Choose a reason for hiding this comment

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

I have my questions in allowing duplicate contracts.
If this is all the same:

contract_name
contract_issuer
contract_spec

Is it not essentially the same asset, just with a different owner? Do we want to allow this? I am thinking no, why do we want to allow copes of an asset to existing? If you change the name or the spec, it's going to be unique?

Another question I have is, is the contract def UTXO always going to stay static and never change? If it is, it kinda becomes hard to validate contract_id

@sdbondi
Copy link
Member Author

sdbondi commented Jul 5, 2022

@SWvheerden There are a few ways to do this, but I dont think we want to tie the contract_id just to the contract_name as that functions as more of a label. This PR is more about tying the contract to a specific UTXO. i.e. you publish a contract utxo and you get a new unique contract_id on-chain. Also removing the need for the issuer to generate a new public key if a contract with the same "label" (contract_name) is used. However, the issuer generating a new key is not unreasonable.

I think I will change the PR to only include the definition UTXO data for now as that is not controversial.

@sdbondi sdbondi force-pushed the core-contract-id-hash-incl-def-issuer branch from d9b4a43 to 7acae6a Compare July 5, 2022 11:48
@sdbondi sdbondi changed the title fix(core)!: include commitment in contract id hash fix(core)!: include issuer public key in contract id hash Jul 5, 2022
@sdbondi sdbondi force-pushed the core-contract-id-hash-incl-def-issuer branch from 7acae6a to dfc70a8 Compare July 5, 2022 11:59
@aviator-app aviator-app bot merged commit ef62c00 into tari-project:development Jul 6, 2022
@sdbondi sdbondi deleted the core-contract-id-hash-incl-def-issuer branch July 6, 2022 09:41
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.

3 participants