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

[Cleanup] The zerocoin garbage collector is in town once more. #2425

Merged

Conversation

furszy
Copy link

@furszy furszy commented Jun 16, 2021

Built on top of #2391.

Made the following changes:

  1. Removed dead mintpool.h/cpp.
  2. Removed dead zerocoin.h/cpp.
  3. Removed unneeded hashBlock arg from ContextualCheckZerocoinSpend and ContextualCheckZerocoinSpendNoSerialCheck.
  4. Validation cleanup: Inside ConnectBlock, moved coinSpend parse and validation to zerocoin_verify files.

@furszy furszy self-assigned this Jun 16, 2021
@furszy furszy added this to the 6.0.0 milestone Jun 16, 2021
@furszy furszy added the Cleanup label Jun 16, 2021
@furszy furszy force-pushed the 2021_cleaning_zerocoin_further branch from a3a8f9f to 65a2271 Compare June 28, 2021 14:14
@furszy
Copy link
Author

furszy commented Jun 28, 2021

Rebased, ready to go.

@furszy furszy force-pushed the 2021_cleaning_zerocoin_further branch from 65a2271 to 017ee3e Compare July 1, 2021 15:37
Copy link

@random-zebra random-zebra left a comment

Choose a reason for hiding this comment

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

utACK 017ee3e

@random-zebra random-zebra merged commit 48ff3af into PIVX-Project:master Jul 12, 2021
furszy added a commit that referenced this pull request Jul 28, 2021
… sporks

a5a20e8 [Tests] Add check for tip != pblock in zerocoin_rejection_tests (random-zebra)
f7a93cd [Refactoring] Cleanup and simplify ParseAndValidateZerocoinSpends (random-zebra)
b943f2c [Refactoring] Remove zerocoin disabling check in ATMP and CheckBlock (random-zebra)
f91e4df [Refactoring] Skip expensive ContainsZerocoins/HasZerocoinSpendInputs (random-zebra)
dd784bb [Cleanup] Deprecate SPORK 16 (zerocoin maintenance) (random-zebra)
179662c [Cleanup] Deprecate SPORK 18 (random-zebra)
f91161f [Validation] Reject under-minting blocks (random-zebra)
6c5884c [BUG] Remove double counted nValueIn for zerocoin txes (random-zebra)
78a5da8 [BUG] Remove spork-based CheckPublicCoinSpendVersion (random-zebra)
10db9bf [BUG] Fix ParseAndValidateZerocoinSpend(s) (random-zebra)

Pull request description:

  #2425 introduced two bugs which are preventing a successful chain sync from scratch on `master`:

  1. It abstracted a piece of code in `ParseAndValidateZerocoinSpend`, which is only parsing/validating a single zerocoin spend per tx, while a transaction could contain multiple spends. Fix it by returning an optional list of `CoinSpendValue`, instead of a singleton.

  2. It re-introduced in the validation the (unused-at-the-time) function `CheckPublicCoinSpendVersion`, which is now failing at the first block with (version 3) public spends.
  As all spork-based checks, this was supposed to be skipped when `fInitialBlockDownload`.
  Zerocoin transactions are no longer accepted (in both nets) in the mempool, as well as inside blocks (after v5 enforcement), so we can just remove this obsolete function.

  The third commit fixes a very old bug: a zerocoin spend tx `nValueIn` is counted twice in ConnectBlock (first directly, after the coinspend parsing/validation, and then via `CCoinsViewCache::GetValueIn`).

  Then, since we are at it, we can deprecate `SPORK_16_ZEROCOIN_MAINTENANCE_MODE` and `SPORK_18_ZEROCOIN_PUBLICSPEND_V4`, which are now unused/unneeded.

  The last three commits are performance optimizations to avoid multiple calls to `ContainsZerocoins()`/`HasZerocoinSpendInputs()` in `ConnectBlock`, when possible.

ACKs for top commit:
  Fuzzbawls:
    ACK a5a20e8
  furszy:
    ACK a5a20e8

Tree-SHA512: 961a755fd36cb24a83d9f0382253cb6f6a94d06534104a3d3328db9047ff769a9ae83c27a4d538ce38c1d9dc25bd4da21ee4387c8ea6251b03d83f81270cd4cc
@furszy furszy deleted the 2021_cleaning_zerocoin_further branch November 29, 2022 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants