PIVX-Project / PIVX

Protected Instant Verified Transactions - Core wallet.
https://www.pivx.org
MIT License
528 stars 716 forks source link

Assertion issues on mainnet staking wallets #841

Closed CryptoDev-Project closed 5 years ago

CryptoDev-Project commented 5 years ago

Describe the issue

It appears that the changes committed here for regtest:

https://github.com/PIVX-Project/PIVX/commit/158bd2b215599eaca2d1292fa1d5034dbb03a032#diff-4a59b408ad3778278c3aeffa7da33c3c

Can cause intermittent wallet crashes on execution of the following statement on mainnet staking wallets:

https://github.com/PIVX-Project/PIVX/blob/master/src/main.cpp#L4907

It appears that the very early locking of cs_main will result in such an error any time the chain moves on and a staker attempts to process, what would traditionally become, an orphan block.

Can you reliably reproduce the issue?

Not on the PIVX mainnet network with insufficient PIV to stake. But achievable by running a separate chain and staking for some time.

Actual behavior

Wallet crashes with assertion error

What version of PIVX Core are you using?

Self compiled from master with changes to run separate chain, thus allowing testing of large staking wallets

Machine specs:

Any extra information that might be useful in the debugging process.

2019-03-21 20:36:20 CreateCoinStake : kernel found 2019-03-21 20:36:20 CreateNewBlock(): total size 1000 2019-03-21 20:36:20 CPUMiner : proof-of-stake block found 4dfff747c3f45b285fe8525c09ed9ecbcfb212608eaeb2d1ea146245f40b8a81 GUI: QObject::connect: Cannot connect (null)::triggered() to BitcoinGUI::showNormalIfMinimized() 2019-03-21 20:36:37 GUI: "registerShutdownBlockReason: Successfully registered: PIVX Core didn't yet exit safely..." 2019-03-21 20:36:37

Warrows commented 5 years ago

I guess we can either extend the lock or replace the assert with an error and abort the block creation if a new block arrived during the process. The later choice might decrease the orphan rate.

furszy commented 5 years ago

as far as I know, fuzz has this solved on top of precomputing and he is only waiting for 837 merge to push it there.

Fuzzbawls commented 5 years ago

yes, this is addressed in #842

Fuzzbawls commented 5 years ago

closing, #842 addresses this issue