Skip to content

[test] fix coinstake input selection on secondary chains. #2634

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
Merged
furszy merged 3 commits into from
Nov 19, 2021
Merged

[test] fix coinstake input selection on secondary chains. #2634

furszy merged 3 commits into from
Nov 19, 2021

Conversation

@furszy
Copy link

@furszy furszy commented Nov 9, 2021 •
edited
Loading

Fixing an inconsistency in the PoS validations unit tests, specifically in the secondary chain branches acceptance test cases.

As the wallet is not prepared to follow several chains at the same time, need to manually remove from the stakeable utxo set the coinstake inputs that were used on the previous blocks of the parallel chain so they are not used again in the block that is being crafted.

@furszy furszy self-assigned this Nov 9, 2021
@furszy furszy added this to the 5.4.0 milestone Nov 9, 2021
@furszy furszy added the Tests label Nov 9, 2021
@furszy furszy mentioned this pull request Nov 10, 2021
Merged
@furszy
test: fix secondary chains acceptance coinstake selection on forks in…
6c48493 
…consistencies.

As the wallet is not prepared to follow several chains at the same time,
need to manually remove from the stakeable utxo set every already used
coinstake inputs on the previous blocks of the parallel chain so they
are not used again.
@furszy furszy force-pushed the 2021_improve_secondary_chains_test branch from 8b2d597 to 6c48493 Compare November 11, 2021 13:52
@random-zebra
Copy link

random-zebra commented Nov 12, 2021 •
edited
Loading

I've spent a bit of time with this test, to debug the last intermittent failures.
There are two more issues, aside from the one fixed here.

  1. failures due to bad-txns-premature-spend-of-coinbase-coinstake:
    when the wallet checks the maturity of a coin, the depth is calculated considering the current tip. Such coin could result immature if spent in a forked block, at a lower height (either inside a regular tx, or a coinstake).
    Fix --> random-zebra@0957885

  2. (a bit harder and fun to track) failures due to bad-txns-inputs-spent-fork-post-split in G3.
    This is because when the input to spend in gTx (to be mined in block G) is selected, this input could be already spent either in block D3 or E3 (which are accepted but not connected yet).
    When gTx is duplicated in G3, in such cases, the longest chain could not be connected anymore, as it would contain a double spend.
    Fix --> random-zebra@68ff1ae

@furszy
Copy link
Author

furszy commented Nov 13, 2021

Awesome findings zebra 👌👌, would love to clone you.
Cherry-picking them.. 🥃 .

random-zebra
random-zebra approved these changes Nov 16, 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.

ACK 47448c8

@furszy furszy requested a review from Fuzzbawls November 17, 2021 17:24
Fuzzbawls
Fuzzbawls approved these changes Nov 19, 2021
View reviewed changes
Copy link
Collaborator

@Fuzzbawls Fuzzbawls left a comment

Choose a reason for hiding this comment

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

ACK 47448c8

@furszy furszy merged commit c0c1a10 into PIVX-Project:master Nov 19, 2021
@furszy furszy deleted the 2021_improve_secondary_chains_test branch November 29, 2022 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Fuzzbawls Fuzzbawls Fuzzbawls approved these changes

+1 more reviewer

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

Reviewers whose approvals may not affect merge requirements

Assignees

@furszy furszy

Labels

Tests

Projects

None yet

Milestone

5.4.0

Development

Successfully merging this pull request may close these issues.

3 participants

@furszy @random-zebra @Fuzzbawls