Skip to content

Conversation

@random-zebra
Copy link

@random-zebra random-zebra commented Nov 30, 2020

Problem:
Starting the wallet with --reindex (w/wo --zapwallettxes) causes a crash if the wallet has shield transactions.

Cause:
g_IsSaplingActive is used to select the proper serialization for v2 txes (with or without sapData).
During init, g_IsSaplingActive is set to true in LoadBlockIndexDB when connecting a post-enforcement block.
When --reindexing, though, LoadBlockIndexDB is skipped in LoadBlockIndex, and the global flag remains false.
Therefore g_IsSaplingActive is false when init gets to InitLoadWallet and the deserialization of shield transactions throws.

Solution:
Set g_IsSaplingActive to true when --reindex.
This way the wallet can be loaded, and then the flag switches back to false as soon as the first pre-enforcement block is connected (ThreadImport starts only after the wallet has been loaded).

Note: A wallet would now crash during a reindex, if containing a pre-enforcement v2 transaction (old serialization, no sapData).
But such transactions are non-standard (afaik there's only one on the whole main-net blockchain).
A wallet with this transaction (if it still exists) would not be able to use --reindex and would be forced to --resync.

@random-zebra random-zebra added this to the 5.0.0 milestone Nov 30, 2020
@random-zebra random-zebra self-assigned this Nov 30, 2020
@random-zebra random-zebra force-pushed the 202011_BUG_reindex-sapling branch from 8ae43c0 to 6a59566 Compare December 1, 2020 16:29
@random-zebra random-zebra force-pushed the 202011_BUG_reindex-sapling branch from 6a59566 to f943cbb Compare December 1, 2020 16:45
Copy link

@furszy furszy left a comment

Choose a reason for hiding this comment

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

tested ACK f943cbb.

Moving forward, we can clean this flag and directly guard any pre-enforcement v2 tx.

@furszy furszy requested a review from Fuzzbawls December 2, 2020 02:58
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 f943cbb

@furszy furszy merged commit 547d606 into PIVX-Project:master Dec 2, 2020
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