-
Notifications
You must be signed in to change notification settings - Fork 221
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
feat(wallet): add command to create owner proof #6240
Conversation
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(); |
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 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.
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.
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
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.
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.
Test Results (Integration tests)0 tests 0 ✅ 0s ⏱️ Results for commit bb4a5a1. |
Test Results (CI) 2 files 80 suites 23m 31s ⏱️ Results for commit bb4a5a1. |
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. |
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