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(base_layer)!: new output field for minimum value range proof #4319

Merged
merged 15 commits into from
Jul 20, 2022

Conversation

mrnaveira
Copy link
Contributor

@mrnaveira mrnaveira commented Jul 18, 2022

Warning: this PR includes consensus breaking changes

Description

  • New field minimum_value_promise for:
    • Struct UnblindedOutput, the range proof gets calculated automatically
    • Struct TransactionOutput, the range proof for the minimum value is verified in the existing verify_range_proof function
  • Adding the new field cascades into the following changes:
    • Created a new TransactionOutputVersion::V2 to isolate the breaking changes of the new field. Used to differentiate when calculating the consensus hashing.
    • Updated the gRPC definitions
    • Updated the lmdb block storage
    • For the wallet:
      • Updated multiple services
      • Updated the Sqlite storage (including a new migration)
      • Changed the FFI function wallet_import_external_utxo_as_non_rewindable with the new parameter
    • Used the default value whenever necessary: genesis block, faucet, etc.
    • Updated multiple test helpers across the project

Motivation and Context

The new Bulletproof+ allows us to create range proofs to demonstrate that a commitment value is greater than a particular value. This is very useful for future stake validation.

The purpose of this PR is to include a convenience new field (minimum_value_promise) in transaction outputs to specify a minimum value for range proofs. Range proofs should be automatically created and verified with minimum changes to existing code. The default value should be 0 so it behaves as current proofs.

How Has This Been Tested?

  • New unit test that builds a TransactionOutput from a UnblindedOutput with the new field, and verifies the range proof, metadata and mask

BREAKING CHANGE: Will not sync with existing dibbler genesis block

@stringhandler stringhandler changed the title feat(base_layer): new output field for minimum value range proof feat(base_layer)!: new output field for minimum value range proof Jul 18, 2022
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.

Looks good - two minor comments

sdbondi
sdbondi previously approved these changes Jul 18, 2022
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!

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.

This is going to break on running the actual test net.
We can default the min_output_value everywhere.
We have to properly map it in.
Currently, its mapped to some outputs, but the inputs are all default. This will cause a hash imbalance and the outputs will not be recognised.

base_layer/core/src/proto/transaction.rs Outdated Show resolved Hide resolved
base_layer/wallet/src/output_manager_service/service.rs Outdated Show resolved Hide resolved
base_layer/wallet/src/wallet.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@hansieodendaal hansieodendaal left a comment

Choose a reason for hiding this comment

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

Hi there,

Looking good, except some changes required where the range proofs are constructed and verified.

Regards

@aviator-app aviator-app bot merged commit 5cff7a9 into tari-project:development Jul 20, 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.

5 participants