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

[Consensus] v5.3 network upgrade. #2492

Merged
merged 5 commits into from
Aug 10, 2021
Merged

Conversation

furszy
Copy link

@furszy furszy commented Aug 1, 2021

Grouped the consensus changes for the coming v5.3 upgrade.

There is no enforcement height defined to not set it until everything is ready but.. we aren't far from be able to enforce it on testnet and start testing the release deeply, only have to merge #2411 and can be done.

@furszy furszy self-assigned this Aug 1, 2021
@furszy furszy added this to the 5.3.0 milestone Aug 2, 2021
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.

The reason for the crash in the unit test is the assertion at the beginning of CWallet::AddToSpends().
Since we have added the coinbase key without rescanning, mapWallet doesn't contain the coinstake prevout transactions (the actual coinbase transactions).

Aside from this, the guard for negative nMinted in IsBlockValueValid can be set with UPGRADE_V5_3 as well.

EDIT: As emerged in our DM convo: resync is not needed as the blocks are created after importing the key. The issue here is the validation queue not flushed before un-registering the interface in the fixture destructor.

src/wallet/test/pos_validations_tests.cpp Outdated Show resolved Hide resolved
src/Makefile.test.include Show resolved Hide resolved
@furszy
Copy link
Author

furszy commented Aug 3, 2021

kk good 👍 , God knows why my environment isn't throwing that assertion.. will do a git clean to see if it was ccache or something similar...

@furszy furszy force-pushed the 2021_v5.3_NU branch 3 times, most recently from d0f94e5 to e77a004 Compare August 3, 2021 19:46
@furszy
Copy link
Author

furszy commented Aug 3, 2021

Updated. Added the IsBlockValueValid change to the v5.3 enforcement as well.

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 1355b20

furszy referenced this pull request in furszy/bitcoin-core Aug 8, 2021
So upgraded peers don't create, accept nor broadcast an mnb containing a new addr v2 to non-upgraded peers before the new NU enforcement (introduced in bitcoin#2492).
furszy referenced this pull request in furszy/bitcoin-core Aug 8, 2021
So upgraded peers don't create, accept nor broadcast an mnb containing a new addr v2 to non-upgraded peers before the new NU enforcement (introduced in bitcoin#2492).
furszy referenced this pull request in furszy/bitcoin-core Aug 9, 2021
So upgraded peers don't create, accept nor broadcast an mnb containing a new addr v2 to non-upgraded peers before the new NU enforcement (introduced in bitcoin#2492).
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 1355b20

@furszy furszy merged commit acdbcf8 into PIVX-Project:master Aug 10, 2021
furszy referenced this pull request in furszy/bitcoin-core Aug 10, 2021
So upgraded peers don't create, accept nor broadcast an mnb containing a new addr v2 to non-upgraded peers before the new NU enforcement (introduced in bitcoin#2492).
furszy referenced this pull request in furszy/bitcoin-core Aug 10, 2021
So upgraded peers don't create, accept nor broadcast an mnb containing a new addr v2 to non-upgraded peers before the new NU enforcement (introduced in bitcoin#2492).
@furszy furszy deleted the 2021_v5.3_NU branch May 27, 2023 01:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants