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.

Sign up for GitHub

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

Jump to bottom

fix: enforce unique commitments in utxo set #3173

Merged
merged 1 commit into from
Aug 17, 2021
Merged

fix: enforce unique commitments in utxo set #3173

merged 1 commit into from
Aug 17, 2021

Conversation

sdbondi
Copy link
Member

@sdbondi sdbondi commented Aug 9, 2021 •
edited
Loading

Description

Adds a unique commitment db index for the UTXO set as well as unique
commitment check in the block validator.

Motivation and Context

Prevents the a consensus disagreement between pruned and unpruned nodes by
reusing a commitment.

How Has This Been Tested?

  • New unit test that tests adding a UTXO with the same commitment, but with different maturity to avoid a hash collision.
    After that UTXO is spent, the duplicate UTXO is allowed to be included in a block.
  • Archival sync
  • Pruned sync
  • Sent a number of transactions
  • Mined some blocks
  • rewind-blockchain command

Checklist:

  • I'm merging against the development branch.
  • I have squashed my commits into a single commit.

@sdbondi sdbondi force-pushed the core-unique-utxo-commitments branch 3 times, most recently from c8fcb29 to 99e11a3 Compare August 9, 2021 14:50
SWvheerden
SWvheerden reviewed Aug 10, 2021
View reviewed changes
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.

Looks good, a few small nits, and a question.

base_layer/core/src/chain_storage/lmdb_db/lmdb_db.rs Outdated Show resolved Hide resolved
base_layer/core/src/chain_storage/lmdb_db/lmdb_db.rs Outdated Show resolved Hide resolved
base_layer/core/src/validation/transaction_validators.rs Outdated Show resolved Hide resolved
base_layer/core/tests/helpers/block_builders.rs Outdated Show resolved Hide resolved
base_layer/core/tests/helpers/block_builders.rs Outdated Show resolved Hide resolved
@sdbondi sdbondi marked this pull request as draft August 10, 2021 09:19
@sdbondi sdbondi changed the title fix: enforce unique commitments in utxo set DNM fix: enforce unique commitments in utxo set Aug 10, 2021
@sdbondi sdbondi force-pushed the core-unique-utxo-commitments branch 6 times, most recently from cca28e0 to 5dbd6ae Compare August 10, 2021 14:00
@sdbondi sdbondi self-assigned this Aug 10, 2021
@sdbondi sdbondi marked this pull request as ready for review August 10, 2021 14:00
@sdbondi sdbondi changed the title DNM fix: enforce unique commitments in utxo set fix: enforce unique commitments in utxo set Aug 10, 2021
@sdbondi sdbondi removed their assignment Aug 10, 2021
@sdbondi sdbondi force-pushed the core-unique-utxo-commitments branch from 5dbd6ae to a6e3096 Compare August 10, 2021 18:26
SWvheerden
SWvheerden previously approved these changes Aug 11, 2021
View reviewed changes
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.

LGTM

@sdbondi sdbondi force-pushed the core-unique-utxo-commitments branch 2 times, most recently from 90b5e09 to 88b8c06 Compare August 12, 2021 07:05
stringhandler
stringhandler previously approved these changes Aug 12, 2021
View reviewed changes
@aviator-app
Copy link
Contributor

aviator-app bot commented Aug 12, 2021

PR queued successfully. Your position in queue is: 2

@aviator-app
Copy link
Contributor

aviator-app bot commented Aug 12, 2021

PR is on top of the queue now

@aviator-app
Copy link
Contributor

aviator-app bot commented Aug 12, 2021

PR failed to merge with reason: Some CI status(es) failed
Failed CI(s): ci/circleci: run-integration-tests

@sdbondi
fix: enforce unique commitments in utxo set
7c30d65 
Adds a unique commitment db index for the UTXO set as well as unique
commitment check in the block validator.
@sdbondi sdbondi force-pushed the core-unique-utxo-commitments branch from cca6b4b to 7c30d65 Compare August 15, 2021 05:47
@aviator-app
Copy link
Contributor

aviator-app bot commented Aug 15, 2021

Waiting for approval before queuing

@stringhandler stringhandler merged commit 23a7d64 into tari-project:development Aug 17, 2021
@sdbondi sdbondi deleted the core-unique-utxo-commitments branch August 17, 2021 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stringhandler stringhandler stringhandler approved these changes

@SWvheerden SWvheerden SWvheerden left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sdbondi @SWvheerden @stringhandler