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

feat(wallet): add command to create owner proof #6240

Closed

Conversation

stringhandler
Copy link
Collaborator

Description

Create a wallet command to create a proof (commitment signature) that you own a commitment

Motivation and Context

In some scenarios you want to prove that you own or can spend a commitment on chain, without actually spending it.

How Has This Been Tested?

Tested with cargo run --release --bin minotari_console_wallet -- -b . create-owner-proof 6234f3569a9dac7acf690348468d1fd87f59db7273f3851641c12417d4efea2b hello

What process can a PR reviewer use to test or verify this change?

as above

Breaking Changes

  • None
  • Requires data directory on base node to be deleted
  • Requires hard fork
  • Other - Please specify

Comment on lines +453 to +457
let challenge = Blake2b::<U64>::new()
.chain_update(commitment.as_bytes())
.chain_update(nonce_commitment.as_bytes())
.chain_update(format!("create_owner_proof::{}", message).as_bytes())
.finalize();
Copy link
Collaborator

Choose a reason for hiding this comment

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

I realize this PR is still a draft, but I'd recommend using the existing domain-separated hash functionality instead of this message formatting approach if possible.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah I got this working with what was available. This code is actually copied from the examples in taricrypto. I'm not really happy with it, but this is the API that ComSig exposes. Ideally the ComSig signing api should be updated to match the schnorr api, where the domain is part of the call

Copy link
Collaborator

Choose a reason for hiding this comment

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

Right, and I know there's a longstanding issue you filed to improve the API that was sidelined due to not using that signature until now. Even so, you should be able to generate the challenge hash using a domain-separated hasher anyway, no? Then you can ensure it'll take care of things like domain separation and input collisions automagically. I suspect it could even be done such that a future API update would retain full compatibility with existing signatures.

@ghpbot-tari-project ghpbot-tari-project added P-acks_required Process - Requires more ACKs or utACKs P-reviews_required Process - Requires a review from a lead maintainer to be merged labels Mar 27, 2024
Copy link

Test Results (Integration tests)

0 tests   0 ✅  0s ⏱️
0 suites  0 💤
0 files    0 ❌

Results for commit bb4a5a1.

Copy link

Test Results (CI)

    2 files     80 suites   23m 31s ⏱️
1 269 tests 1 269 ✅ 0 💤 0 ❌
2 538 runs  2 538 ✅ 0 💤 0 ❌

Results for commit bb4a5a1.

@AaronFeickert AaronFeickert self-assigned this Apr 12, 2024
@AaronFeickert
Copy link
Collaborator

AaronFeickert commented Apr 12, 2024

After speaking with @stringhandler, it sounds like the use case of interest here is to prove two things: that you can open the commitment, and that the commitment binds to at least a specified value.

This means you don't need a commitment signature, but can get away with using a minimum-value range proof instead. The cryptographic plumbing already exists for this, and the Bulletproofs+ API (pending a new version tag) supports safe transcripting operations that allow for this to be done very safely. The proof is much larger (by several hundreds of bytes) than a commitment signature, but it handles both assertions simultaneously without the need to overhaul the commitment signature API.

I'll create a new PR that does this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-acks_required Process - Requires more ACKs or utACKs P-reviews_required Process - Requires a review from a lead maintainer to be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants