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

[Consensus][RingCT] Correctly track and test RingCT spends. #972

Merged
merged 1 commit into from
Mar 13, 2022
Merged

[Consensus][RingCT] Correctly track and test RingCT spends. #972

merged 1 commit into from
Mar 13, 2022

Conversation

Zannick
Copy link
Collaborator

@Zannick Zannick commented Oct 19, 2021 •
edited
Loading

Problem

Creating multiple ringct spend transactions in a batch (as part of #970) does not work because of these two similar issues.

  1. When creating a transaction, the transaction record has dummy inputs which don't correspond to any actual output, so we don't actually track the pending spends.
  2. When verifying that we aren't double-spending any ringct output, we add every keyimage we see to the set to check against. But if we're testing that a transaction could be accepted to the mempool, then committing that transaction, the commit fails (until the next block).

Solution

  1. Extract the spent coins' txo records to be marked as pending spend.
  2. Don't track the inputs permanently when testing that a transaction could be accepted to mempool. We still track within the transaction.

Tested

On regtest with other changes for #970.

@Zannick Zannick added Component: Consensus Part of the core cryptocurrency consensus protocol Tag: Mempool Coin Type: RingCT Specifically related to RingCT transactions Tag: Transaction Creation Related to the transaction creation process. Tag: Waiting For Code Review Waiting for code review from a core developer Tag: Validation Related to the validation of blocks and transactions. labels Oct 19, 2021
@Zannick Zannick self-assigned this Oct 19, 2021
@WetOne
Copy link
Collaborator

WetOne commented Oct 21, 2021

utACK e8c32ca

@Zannick
Correctly track and test RingCT spends.
ec15117 
1.  When creating a transaction, the record has dummy inputs, but we
    want to mark the actual txo's as pending spend so later transactions
    don't attempt to spend them.
2.  When verifying that we aren't double-spending, if we're only testing
    that a transaction can be accepted, don't track the inputs as though
    we're trying to commit the transaction. This way we can test
    accepting a transaction and later commit it (e.g. as part of a batch).
codeofalltrades
codeofalltrades approved these changes Mar 13, 2022
View reviewed changes
Copy link
Collaborator

@codeofalltrades codeofalltrades left a comment

Choose a reason for hiding this comment

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

ACK ec15117

@codeofalltrades codeofalltrades added Code Review: Passed and removed Tag: Waiting For Code Review Waiting for code review from a core developer labels Mar 13, 2022
@codeofalltrades codeofalltrades merged commit 5230e52 into Veil-Project:master Mar 13, 2022
@Zannick Zannick deleted the ringct branch March 20, 2022 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@codeofalltrades codeofalltrades codeofalltrades approved these changes

@WetOne WetOne WetOne approved these changes

@CaveSpectre11 CaveSpectre11 Awaiting requested review from CaveSpectre11

Assignees

@Zannick Zannick

Labels
Code Review: Passed Coin Type: RingCT Specifically related to RingCT transactions Component: Consensus Part of the core cryptocurrency consensus protocol Tag: Mempool Tag: Transaction Creation Related to the transaction creation process. Tag: Validation Related to the validation of blocks and transactions.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Zannick @WetOne @codeofalltrades