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

[Test] Coverage for zc transactions rejection in mempool and blocks. #2364

Merged
merged 2 commits into from
May 5, 2021
Merged

[Test] Coverage for zc transactions rejection in mempool and blocks. #2364

merged 2 commits into from
May 5, 2021

Conversation

furszy
Copy link

@furszy furszy commented May 5, 2021

Safe-belt for the post-v5 zerocoin cleanup that is being done. Unit test ensuring that we are rejecting zc txs in the mempool and inside new blocks.

@furszy furszy self-assigned this May 5, 2021
@furszy furszy added this to the 6.0.0 milestone May 5, 2021
@furszy furszy added the Tests label May 5, 2021
random-zebra added a commit that referenced this pull request May 5, 2021
@random-zebra
Merge #2320: [Cleanup] Remove zerocoin mints databasing
a971094 
529ff1f [Refactoring] Stop writing zerocoin mints in the DB (random-zebra)
6b51e1c [Cleanup] Remove unused ReadCoinMint/EraseCoinMint (key 'm') in zc DB (random-zebra)

Pull request description:

  We should have never databased the "public coins" in the first place, and it's beyond useless now.

  With this simple removal we get a 20~25% speedup during they sync of the interested blocks [`863787` -> `1880000`], and, as bonus, we save 60MB of disk space.

ACKs for top commit:
  Fuzzbawls:
    ACK 529ff1f
  furszy:
    Now that we have #2298 and #2364 test, ACK 529ff1f .

Tree-SHA512: 9d053538158cd271e64ab70bc3aa913256d3a4386c02e961c2776b45b0694221d3f0919a5992c8e652271783fc2d81729df48fbbb11d0c3f1ba9704e0654c893
random-zebra
random-zebra approved these changes May 5, 2021
View reviewed changes
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.

Nice. ACK 981e943

This will conflict with #2360 (we shouldn't be changing the chainparams inside the test, but use a different fixture). Anyway, since this is only adding a new test case, I'm going to merge it, and fix it directly inside #2360.

@furszy
Copy link
Author

furszy commented May 5, 2021 •
edited
Loading

we shouldn't be changing the chainparams inside the test, but use a different fixture

I know, i told you that :p. Didn't do it here because i know that you changed it inside #2360 ☕ .

@random-zebra random-zebra merged commit d57c950 into PIVX-Project:master May 5, 2021
@random-zebra random-zebra mentioned this pull request May 5, 2021
Merged
@furszy furszy deleted the 2021_test_zc_rejection branch May 27, 2023 01:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@random-zebra random-zebra random-zebra approved these changes

Assignees

@furszy furszy

Labels
Tests
Projects
None yet
Milestone
5.3.0
Development

Successfully merging this pull request may close these issues.
2 participants
@furszy @random-zebra