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: allow bullet proof value only rewinding off one-sided transaction #3587

Conversation

SWvheerden
Copy link
Collaborator

Description

This PR allows us to do bulletproof value-only rewinding on one-sided transactions.

Motivation and Context

Currently, it is not possible to do bulletproof rewinding on the value only on a one-sided transaction utxo due to the way the commitment blinding factor and bulletproof rewinding keys are created.
Currently, the two bulletproof rewinding keys are created as:

 let rewind_key = PrivateKey::from_bytes(&hash_secret_key(&commitment_blinding_factor))?;
 let blinding_key = PrivateKey::from_bytes(&hash_secret_key(&rewind_key))?;

This means that if you share the rewind key, which is used to do value only rewinding, that a person can calculate the blinding key which is used to do full rewinding and expose the commitment blinding factor. by changing the calculation order we prevent this and only allow full rewinding by something who needs to be able to do this.

 let rewind_key = PrivateKey::from_bytes(&hash_secret_key(&blinding_key ))?;
 let blinding_key = PrivateKey::from_bytes(&hash_secret_key(&commitment_blinding_factor))?;

How Has This Been Tested?

All current test pass

@SWvheerden SWvheerden added the W-consensus_breaking Warn - A change requiring a hard fork to be activated label Nov 18, 2021
Copy link
Member

@sdbondi sdbondi left a comment

Choose a reason for hiding this comment

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

Nice, makes sense

This isn't so much a consensus change as something that could make previously 'undiscovered' one-side funds not redeemable. Not a huge deal in testnet, so think this can go in

Copy link
Contributor

@philipr-za philipr-za left a comment

Choose a reason for hiding this comment

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

Ja this looks good and almost no one is using one-side transactions at the moment so the breaking impact is very small.

@stringhandler stringhandler changed the title fix!: allow bullet proof value only rewinding off one-sided transaction fix: allow bullet proof value only rewinding off one-sided transaction Nov 19, 2021
@aviator-app aviator-app bot merged commit f32a38f into tari-project:development Nov 19, 2021
@SWvheerden SWvheerden deleted the sw_allow_bp_value_onlyrewind_1-sided branch November 22, 2021 10:50
sdbondi added a commit to sdbondi/tari that referenced this pull request Nov 22, 2021
* development:
  feat: language detection for mnemonic seed words (tari-project#3590)
  chore: minor clippy fixes (tari-project#3576)
  fix: be more permissive of responses for the incorrect request_id (tari-project#3588)
  feat: track ping failures and disconnect (tari-project#3597)
  chore: upgrade tokio deps tari-project#3581 (tari-project#3595)
  feat: standardize output hash for unblinded output, transaction output and transaction input (tari-project#3592)
  fix: allow bullet proof value only rewinding off one-sided transaction (tari-project#3587)
  refactor: update miningcore repository links (tari-project#3593)
  refactor: clean up unwraps in wallet_ffi (tari-project#3585)
  fix: update daily test start times and seed phrase (tari-project#3584)
  fix: allow bullet proof value only rewinding in atomic swaps (tari-project#3586)
  v0.21.2
  fix: remove delay from last request latency call
sdbondi added a commit to sdbondi/tari that referenced this pull request Nov 23, 2021
* development: (46 commits)
  refactor: remove tari_common dependency from tari_comms (tari-project#3580)
  feat: language detection for mnemonic seed words (tari-project#3590)
  chore: minor clippy fixes (tari-project#3576)
  fix: be more permissive of responses for the incorrect request_id (tari-project#3588)
  feat: track ping failures and disconnect (tari-project#3597)
  chore: upgrade tokio deps tari-project#3581 (tari-project#3595)
  feat: standardize output hash for unblinded output, transaction output and transaction input (tari-project#3592)
  fix: allow bullet proof value only rewinding off one-sided transaction (tari-project#3587)
  refactor: update miningcore repository links (tari-project#3593)
  refactor: clean up unwraps in wallet_ffi (tari-project#3585)
  fix: update daily test start times and seed phrase (tari-project#3584)
  fix: allow bullet proof value only rewinding in atomic swaps (tari-project#3586)
  v0.21.2
  feat: add atomic swap refund transaction handling (tari-project#3573)
  feat: improve wallet connectivity status for console wallet (tari-project#3577)
  v0.21.1
  feat: add error codes to LibWallet for CipherSeed errors (tari-project#3578)
  ci: split cucumber job into two (tari-project#3583)
  feat(wallet): import utxo’s as EncumberedToBeReceived rather than Unspent (tari-project#3575)
  docs: rfc 0250_Covenants (tari-project#3574)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
W-consensus_breaking Warn - A change requiring a hard fork to be activated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants