[Core][RPC][Tests] Add HD Wallet support by akshaynexus · Pull Request #956 · PIVX-Project/PIVX

akshaynexus · 2019-07-19T07:24:08Z

This code is based off of dashpay#1405 ,also adds extra functions and files that was needed in order for the changes to compile successfully

Release Notes

[Core]

Adds 4 new args to startup options/config [usehd,mnemonic,mnemonicphrase,hdseed].
Replaces references to 74 in key and other key related files to use constant BIP32_EXTKEY_SIZE instead.
Adds backported function GetScriptForRawPubKey.

[RPC]

Adds dumphdinfo,dumpwallethd[to be merged to dumpwallet instead].
Adds hdkeypath and hdchainid to validateaddress output.
Modified getwalletinfo to keypool sizes of internal and external keypool.Also adds hdchainidand hdaccountcount to outputs.

TODO

Move dumpwallethd to dumpwallet,need help in this as the dumpwallet tests fail due to the changes in formatting.

random-zebra · 2019-07-19T08:44:30Z

This is just great, @akshaynexus .
Much needed feature, thanks for the submission.
Will test and review asap.

Warrows

Concept ACK. This is a much awaited and needed improvement for PIVX. Thank you very much for submitting it.
Merging this will close #378.
If you could squash your three latest commits into one, that would be great.
And here are a few nits from my initial first glance, I'll get to testing later.

random-zebra

Few minor nits from visual inspection and a couple questions.
Will do some testing soon.

Fuzzbawls · 2019-07-27T00:22:31Z

Needs Rebase

akshaynexus · 2019-07-27T13:58:35Z

Needs Rebase

done

random-zebra · 2019-07-27T19:37:03Z

Rebase was not successful.
#949 removed all using namespace statements (including the std namespace).
So, now, for example, runtime_error (or ofstream, etc..) needs to be converted to std::runtime_error (or std::ofstream etc)

akshaynexus · 2019-07-29T12:03:15Z

Rebase was not successful.
#949 removed all using namespace statements (including the std namespace).
So, now, for example, runtime_error (or ofstream, etc..) needs to be converted to std::runtime_error (or std::ofstream etc)

will fix those shortly

yurykk · 2019-08-10T17:36:25Z

The code is not fully BIP44 compliant. While staking, the wallet will take next available address from reserved pool ( CreateNewBlockWithKey() ), but this address will never any transaction.
That can potentially cause "gap" of more than 20 addresses w/o transactions, which breaks BIP44 "account discovery" procedure.

akshaynexus · 2019-08-10T17:38:04Z

The code is not fully BIP44 compliant. While staking, the wallet will take next available address from reserved pool ( CreateNewBlockWithKey() ), but this address will never any transaction.
That can potentially cause "gap" of more than 20 addresses w/o transactions, which breaks BIP44 "account discovery" procedure.

but this address will never any transaction. not sure what you mean by that.

akshaynexus · 2019-08-10T17:47:51Z

CreateNewBlockWithKey() is calling GetReservedKey with fInternalIn by default as false, also checked on the dashpay pr and your commit to TWINS-Core ,both have the same changes, but this pr has it set by default to false in the header, if you can clarify the problem you saw in the pr changes, it would be helpful.

yurykk · 2019-08-10T18:35:22Z

CreateNewBlockWithKey() is calling GetReservedKey with fInternalIn by default as false, also checked on the dashpay pr and your commit to TWINS-Core ,both have the same changes, but this pr has it set by default to false in the header, if you can clarify the problem you saw in the pr changes, it would be helpful.

I recently discovered this issue on TWINS-Core ;-) Staking wallet will use a lot of keys from the pool, which will have no transaction. If the wallet is rarely used for actual transactions, then gap of 20 (or more) unused addresses will be created by CreateNewBlockWithKey.

It's not a problem for this specific implementation of the wallet, since it doesn't follow BIP44 procedures. The problem will appear if someone will try to restore the wallet using mnemonic phrase on a 3rd party BIP44 wallet (either mobile or desktop), which will follow BIP44 account discovery procedure and will STOP scanning for addresses & transactions after discovering 20 consecutive addresses without transactions.

Therefore, after recovering the wallet using mnemonic phrase on a 3rd party BIP44 wallet, it may not discover all the transactions/balance...

Changing fInternalIn to 'true' will not help, as the same "gap" will be created in Internal pool, instead of "external".

Probably the best way to solve it, is to create additional non-standard "technical" poll of addresses (for example, m / 44' /119' / 0' / 999 / x ) to be used specifically for CreateNewBlockWithKey, so it will not interfere with "regular" addresses...

yurykk · 2019-08-10T18:36:05Z

but this address will never any transaction. not sure what you mean by that.

This is exactly why it's a problem.

yurykk · 2019-08-10T18:39:54Z

Probably the best way to solve it, is to create additional non-standard "technical" poll of addresses (for example, m / 44' /119' / 0' / 999 / x ) to be used specifically for CreateNewBlockWithKey, so it will not interfere with "regular" addresses...

Or maybe just to derive single address "on the fly". Just need to store "count" of already used keys somewhere...

random-zebra · 2019-08-11T23:38:42Z

Hey @yurykk , thanks for the comment. Nice catch.

Actually, since PoS blocks don't need new keys (the coinstake output key is obtained from the input and the coinbase is empty), I think it would be better to just prevent calls to CreateNewBlockWithKey in that case, and use CreateNewBlock directly.
#982 does that.

74e7fc7 [Miner] Don't create new keys when generating PoS blocks (random-zebra) Pull request description: This fixes the calls to `CreateNewBlockWithKey` with PoS blocks, which are not needed. We simply call `CreateNewBlock` with an empy script for coinbase. It should prevent the problem described in #956 (comment) ACKs for top commit: CaveSpectre11: utACK 74e7fc7 akshaynexus: utACK 74e7fc7 furszy: utACK [74e7fc7](74e7fc7) Warrows: Pretty straightforward indeed, utACK 74e7fc7 Tree-SHA512: bcb9752a689a058b6c33317136d16afd694b5da66c69cdd32a8b89b71059c5b2c5dc20c88045c52429fae99d45f508ea223f1b7d1afa5db3c77eb59befd48d51

74e7fc7 [Miner] Don't create new keys when generating PoS blocks (random-zebra) Pull request description: This fixes the calls to `CreateNewBlockWithKey` with PoS blocks, which are not needed. We simply call `CreateNewBlock` with an empy script for coinbase. It should prevent the problem described in PIVX-Project/PIVX#956 (comment) ACKs for top commit: CaveSpectre11: utACK PIVX-Project/PIVX@74e7fc7 akshaynexus: utACK PIVX-Project/PIVX@74e7fc7 furszy: utACK [74e7fc7](PIVX-Project/PIVX@74e7fc7) Warrows: Pretty straightforward indeed, utACK 74e7fc7 Tree-SHA512: bcb9752a689a058b6c33317136d16afd694b5da66c69cdd32a8b89b71059c5b2c5dc20c88045c52429fae99d45f508ea223f1b7d1afa5db3c77eb59befd48d51

* Give error if pow phase is finished * Force Sync to allow for staking without mnsync * Fix error * Update rpcmisc.cpp * Update rpcmisc.cpp * more ignore * Update fundamentalnode-sync.cpp * Add gitian build python script * Update Univalue * add missing gencpp * Add missing includes * Fix Gitian build Update Univalue add missing gencpp Add missing includes * Revert "Update fundamentalnode-sync.cpp" This reverts commit 323b13f * Fix gitian build and minor updates * remove forcesync * [Model] Invalid iterator position crash fix. * background loading crash fixed. * random: fix crash on some 64bit platforms rbx needs to be stashed in a 64bit register on 64bit platforms. With this crash in particular, it was holding a stack canary which was not properly restored after the cpuid. Split out the x86+PIC case so that x86_64 doesn't have to worry about it. * random: Add fallback if getrandom syscall not available If the code was compiled with newer (>=3.17) kernel headers but executed on a system without the system call, every use of random would crash the program. Add a fallback for that case. * Don't assert if we were beaten to the block A timing window exists where a wallet could be creating a new block from within the miner thread when a new block is received to the wallet. This window will create a situation where TestBlockValidity() fails because the chain tip has changed between the time it created the new block and the time it tested the validity of the block. This situation would result in the wallet being asserted; however this is a little overkill. rather than asserting if the tip has changed, it is better to throw the block away. This problem was revealed during a testnet test of an altcoin, and very prevalent when multiple wallet existed with the exact same number of staking coins received in the same transaction; or when multiple wallets were staking the same coins via import private key. The problem happens significantly less in more normal circumstances, but was still observed in a testing environment with fast blocks. It is likely that this scenario has been encountered but never determined to be root cause, as a crashed wallet could be restarted, re-indexed and never investigated further. * [Wallet] Add some LOCK to avoid crash Github-Pull: #625 Rebased-From: ed23d0d62054156e0c2cb632fecbd65588c50e8c * testnet mining fixes * fix placement of utxo signer * dont sign twice * prevent logging twice * random files updates * Add Chacha20 crypto files * Merge #643: [Crypto] Use stronger rand for key generation b7dda924cfc2405386968b0eaa32aa0e546f322c [Log] Replace a string by the function name in a log (warrows) 977f089d0d2883ca3ca1648b14423b738c9248ef [Refactor] Use arrays instead of unic vars in Chacha20 (warrows) d8abe323bd4f5cff3ca23fc00f0093ab7b143ead [Random] Add a missing include (warrows) 27663b8f16dfaac75bae1ab0e4a0c465b838c826 Do not permit copying FastRandomContexts (Pieter Wuille) 64e03e6de1a65c8d934d8df65af48e2b92bf887a Bugfix: randbytes should seed when needed (non reachable issue) (Pieter Wuille) e8f12aa3621e98588b82533edb56ed4865645862 Check if sys/random.h is required for getentropy on OSX. (James Hilliard) de85c7ae55681f8a9d322f4c48c6e9ba15ee0cf1 Add attribute [[noreturn]] (C++11) to functions that will not return (practicalswift) df46c7ff5a2e6a71ba636fa39d77f58e018816a6 Fix resource leak (Dag Robole) d426d856d23fe6c5d6ef3d57fe2c420b9a8e4e5e Clarify entropy source (Pieter Wuille) 30a320b3d12d3529dfca1daae0e477d76acd9ae7 Use cpuid intrinsics instead of asm code (Pieter Wuille) 0c21204e6dc714de6b917d9de12b1029d0bfb67b random: fix crash on some 64bit platforms (Cory Fields) b8bbb9c68d996b1bea579956b3c91872d6d94b8f Use rdrand as entropy source on supported platforms (Pieter Wuille) 8e19443e41f748ad54d0f8aa25907bdad0815c19 [Tests] Fix compilation (warrows) f53edec73901a91a6c4f595b248e4058e8be5468 [Rand/test] scripted-diff: Use new naming style for insecure_rand* functions (warrows) 272f3a5ac83ba0ab72a799ce524b6605b3555751 [Random / tests] scripted-diff: Use randbits/bool instead of randrange (warrows) 0173ee3332990ac1eea134cca5c21cc451b81d92 Replace rand() & ((1 << N) - 1) with randbits(N) (Pieter Wuille) 250de7426e8317f86fc25fe60e960c2e9e903b69 Replace more rand() % NUM by randranges (Pieter Wuille) d6904136030d94e844074fb75b9cafda46253ce0 [Random / tests] scripted-diff: use insecure_rand256/randrange more (warrows) 4a811ff9542439d43e3df3460114d9a24f849a36 Merge test_random.h into test_bitcoin.h (Pieter Wuille) f275e638bf0a752d9f231f2e4a781432deafbfe8 Add various insecure_rand wrappers for tests (Pieter Wuille) 602af4fb3ef45a2a821f1b76d0dbd418ec852683 Add FastRandomContext::rand256() and ::randbytes() (Pieter Wuille) 90549785824f4c13ee8cf36e77f737b5d89d0f02 Add perf counter data to GetStrongRandBytes state in scheduler (Matt Corallo) 17dd13e746b567f44015fbefaa634b278010fccf Add internal method to add new random data to our internal RNG state (Matt Corallo) c7a1602879aff2ae72bdefe959d4026ee8c0aefc Use sanity check timestamps as entropy (Pieter Wuille) f671fe99994cb220dede903459307c61475aeef8 Test that GetPerformanceCounter() increments (Pieter Wuille) dcb536fb2cef719abef9bb4104242410987bfd3d Use hardware timestamps in RNG seeding (Pieter Wuille) 7c3f290b50d32a3290a2c49542d72a3a7466da58 [Random] Fix compilation (warrows) 22b78957cc90252c958bc66439c098001d47b8fc random: only use getentropy on openbsd (Cory Fields) e5750e5c9831edf01026643deed249e0efb28f0b Add a FastRandomContext::randrange and use it (Pieter Wuille) 2a0f6cd1ca2224e2ba30aea4ab312a1919dedc0c Switch FastRandomContext to ChaCha20 (Pieter Wuille) 401ca7db468939184681b73f4b346aebcfc378c6 Introduce FastRandomContext::randbool() (Pieter Wuille) 3d056d6ed478c5bbd42d7dde4f1ab0a75ffd42b1 Add ChaCha20 (Pieter Wuille) 3c97f3f2cb1b3dc2f5d642877d9a2de0a85c06bf Kill insecure_random and associated global state (Wladimir J. van der Laan) 68ba16c78593bf221844dae95fc5edacbf69ba62 Maintain state across GetStrongRandBytes calls (Pieter Wuille) c3c399e199c2d25d10e782dca2ef17f55b6939d0 random: Add fallback if getrandom syscall not available (Wladimir J. van der Laan) 7a8111f915fc7ed23aaec86cd95904bf5d5972fd sanity: Move OS random to sanity check function (Wladimir J. van der Laan) 387c2e9574b56ae941b7723c561ed39488891782 squashme: comment that NUM_OS_RANDOM_BYTES should not be changed lightly (Wladimir J. van der Laan) 9e8c266b158c26b361d68a3cccdbcfd7bee0b376 util: Specific GetOSRandom for Linux/FreeBSD/OpenBSD (Wladimir J. van der Laan) f989b865f215c797e77483a04cc7fe9026904517 Don't use assert for catching randomness failures (Pieter Wuille) a15419e13204d96416b0cce9aad2ae4e35ea5ed5 Always require OS randomness when generating secret keys (Pieter Wuille) Pull request description: Since #576 hasn't changed in over a month, here is a reworked version of it. So in this PR: -We add the memory_cleanse function from upstream, to remove a number of OpenSSL calls. -We use OS randomness in addition to OpenSSL randomness (see #576 for why it's needed). ACKs for top commit: random-zebra: ACK PIVX-Project/PIVX@b7dda92 furszy: ACK [`b7dda92`](PIVX-Project/PIVX@b7dda92) Tree-SHA512: d92cbc14d844263ced753248e646f5cd4f03ec37546f50ff1b558fc3076b6d777c7efcb899c0400bc510e21311b5bd93d9aca26d811033fedb370f8457204035 * Revert "Merge #643: [Crypto] Use stronger rand for key generation" This reverts commit ae3e256. * Revert "Add Chacha20 crypto files" This reverts commit ddd102f. * Revert "random files updates" This reverts commit f29e084. * Revert "random: fix crash on some 64bit platforms" This reverts commit 5a52414. * Revert "random: Add fallback if getrandom syscall not available" This reverts commit b46c1cb. * fix redeclaration * fix signing code * do not return in void func * remove unused funcs * remove on error * revert "Refactor miner" * fix repeat of masternode in fundamental node payment log * fix name collision * [Wallet] Look at last CoinsView block for corruption fix process * Fix crashes * Merge #941: [Refactor] Move ThreadStakeMinter out of net.cpp 42fe4040acff08450168416ccf35413abdc1747a [Refactor] Move ThreadStakeMinter out of net.cpp (Fuzzbawls) Pull request description: This moves the implementation function and thread creation to a more appropriate file (`miner.cpp`). Resolves #938 ACKs for top commit: CaveSpectre11: ACK PIVX-Project/PIVX@42fe404 random-zebra: ACK PIVX-Project/PIVX@42fe404 furszy: utACK [42fe404](PIVX-Project/PIVX@42fe404) Tree-SHA512: d3417f03cab63aa41b1ff0fb4d391d42f4448b83efe70391d4014407008af34eb265a23b5807ebecb90cf9637fb74e5b330b02c92e7038e5400a5f16608800ee * Merge #865: [Main] Don't return an invalid state when shutting down the wallet 8126729 Don't return an invalid state when shutting down the wallet (Fuzzbawls) Tree-SHA512: b7042df42af620faca5d415382ad0a6ae973e9dbad7e527957c2ac3c7d0cb5470fcca555b3fe262f5f06e334504ded657db879c2471ecc290247e0f7e2bbf5f3 * Merge #958: [Staking] Modify miner and staking thread for efficiency b7a9a9cdb20cdee869906fb24e2bc984e8a93802 [Staking] Modify miner and staking thread for efficency (Cave Spectre) Pull request description: ### **Release notes** - [Mining] Unnecessary mining threads now exit after PoS has begun - [Staking] The staking thread dormancy is more efficient during PoW - [Performance] Some unnecessary processing in the mining thread removed - [RPC] setgenerate errors if attempted to turn on after end of PoW - [Build] Wallet only routines conditionalized in miner.h - [Refactoring] Log messages with bitcoin named routines changed for easier rename of routines in the future. In order to better explain the changes in this PR, a review of the existing code would be helpful. ### **Backstory** When fixing a PIVX forked coin's transition from PoW to PoS, it was observed that mining threads go into tight infinite loops after the switch to proof of stake. This can be seen with a simple `pivx-qt -testnet -gen`, and watching the debug log. This observation triggered the below code review (re-written for the current release; some of the findings in the code originally reviewed had already been addressed in PIVX). ### **Code Review** _BitcoinMiner()_ This routine is a worker routine for both staking and mining threads; fProofOfStake is set to true when running in the staking thread, false when running in the mining thread. fGeneratedBitcoins is set when mining is started; and cleared on shutdown or when mining is stopped. _In the initial fProofOfStake section:_ ``` if ((GetTime() - nMintableLastCheck > 5 * 60)) // 5 minute check time { nMintableLastCheck = GetTime(); fMintableCoins = pwallet->MintableCoins(); } if (chainActive.Tip()->nHeight < Params().LAST_POW_BLOCK()) { MilliSleep(5000); continue; } ``` Every 5 minutes this code will scan through the wallet and look to see if mintable coins exist. After it's done that, it checks if the network is still in the proof of work phase. It doesn't make sense to scan the wallet until after the proof of work phase is ending. If it is in the proof of work phase; the staking thread sleeps for 5 seconds (`MilliSleep(5000)`). This only makes sense if the block time is 5 seconds. Technically the thread could determine the number of blocks remaining in the proof of work phase, and use that to calculate a hibernation of sorts. Using the target spacing to wait for the block to pass is a 91% reduction in the number of passes through the loop (12 vs. 1 when the target spacing is one minute), and doesn't run into issues where adjustments to the actual spacing throw the timing of a hibernation off. ``` if (!fMintableCoins) { if (GetTime() - nMintableLastCheck > 1 * 60) // 1 minute check time { nMintableLastCheck = GetTime(); fMintableCoins = pwallet->MintableCoins(); } } MilliSleep(5000); ``` We come to this section of code in the waiting while loop; if we don't have mintable coins yet, or our wallet is locked, or we're not synched; or a host of other potential things that would prevent staking. However, we check again for mintable coins; and then we wait 5 seconds after that check before we come out. Since the first time we come into this loop; we have already checked mintable coins (within the last 5 minutes); we should sleep first, and then do the check closer to the time we're actually going to go around the loop again; so we're working with the most recent data. ``` if (!fGenerateBitcoins && !fProofOfStake) continue; ``` This check is buried in a `if (fProofOfStake)` conditional, where fProofOfStake is a parameter that is passed in by the caller to BitcoinMiner(). `!fProofOfStake` will never be true; so this condition will never be true. Removing it we are left with !fGenerateBitcoins; which is unrelated to this section of code anyway. If we're down to this point, we're not in PoW mode anymore, so there shouldn't be a mining thread (more on that later). But the most compelling part of this condition,whether it passes or not, is that it "continues" the while loop it's in; and since it's at the end of the actual while loop, it's going to iterate into another round of the while loop as soon as it finishes with this conditional anyway. Long story short; the code doesn't do anything. _Overall logic_ The mining thread will run until mining is turned off (`setgenerate false`, or the mining flags taken off the invocation). However, there is no need to continue to mine with proof of work after the proof of stake phase begins. In fact, there really is no consideration of that at all; PoW will continue to try to generate blocks well after PoS has begun. Yes, it's within the users control to stop mining; but it's within the power of the code to take care of that for the user, and stop the mining thread(s) after the transition to proof of stake. ### **This PR:** The logical issues above have been corrected. The dormancy for the proof of stake thread is held for just one block rather than any extreme hibernation. Combined with moving that code to be the first thing in the while loop; it also removes the processing done to search through the wallets looking for stakeable coins. Since a significant amount of code assumes there will not be a transition from PoS to PoW; code was added to BitcoinMiner() to exit the mining threads if it's transitioned into the PoS phase. The threads will continue for a little bit, in case there is any rewind on the chain; but after 6 PoS blocks are accepted, the miner threads will exit. To prevent the PoW thread from trying to generate blocks after the PoS phase has begun, logic was added in CreateNewBlockWithKey() to return quickly. Rather than create a situation where the mining thread ends up in a tight loop in the initial phases of PoS, the mining thread will be held for 1/2 of the target spacing before returning. Lastly; Some tweaks were made to the logging; removing hardcoded references to "BitcoinMiner()", in case refactoring in the future changes the name of the routine. Similarly, since there is a log message when the mining thread starts "PIVXMiner started"; rather than reporting thread exits or errors as "ThreadBitcoinMiner", they have been changed to PIVXMiner to match. Much of this can be observed with an errant `pivx-qt -gen`. With this PR, it simply tries to start the miner, and then exits. _With PR code - `pivx-qt -gen -testnet`_ ``` 2019-07-21 20:04:22 PIVXMiner started 2019-07-21 20:04:22 BitcoinMiner: Exiting Proof of Work Mining Thread at height: 1160732 2019-07-21 20:04:22 PIVXMiner exiting ``` _Current release:_ ``` 2019-07-21 19:37:06 CreateNewBlock(): total size 1000 2019-07-21 19:37:06 ERROR: CheckProofOfWork() : hash doesn't match nBits 2019-07-21 19:37:06 ERROR: CheckBlockHeader() : proof of work failed 2019-07-21 19:37:06 ERROR: CheckBlock() : CheckBlockHeader failed 2019-07-21 19:37:06 CreateNewBlock() : TestBlockValidity failed ``` is repeated in a tight loop. ACKs for top commit: furszy: ACK [b7a9a9c](PIVX-Project/PIVX@b7a9a9c) random-zebra: ACK PIVX-Project/PIVX@b7a9a9c and merging... Tree-SHA512: 064619ce1fde75f1baf35ddb202b4573f2a8c0effc5d2939a678612e3f1d56b08d5c5d7c7bb30ce919e5a648f3f406cca35093b9a3a04c799f746fff5c6a35b6 * remove duplicate stake thread code * add missing include * remove zvitae pos validator * Refine signblock code * revert zerocoin added check * Miner reference updates * revert miner changes * fix fundamentalnode sync checker * [Refactor] Move rpc files to its own folder * update includes * fix rpcserver include to rpc/server.h * [Refactor] Move wallet files to wallet dir * [Trivial] Fix db.h path * Merge #982: [Miner] Don't create new keys when generating PoS blocks 74e7fc7d432c7f6d586d54ff276d6babad0fe505 [Miner] Don't create new keys when generating PoS blocks (random-zebra) Pull request description: This fixes the calls to `CreateNewBlockWithKey` with PoS blocks, which are not needed. We simply call `CreateNewBlock` with an empy script for coinbase. It should prevent the problem described in PIVX-Project/PIVX#956 (comment) ACKs for top commit: CaveSpectre11: utACK PIVX-Project/PIVX@74e7fc7 akshaynexus: utACK PIVX-Project/PIVX@74e7fc7 furszy: utACK [74e7fc7](PIVX-Project/PIVX@74e7fc7) Warrows: Pretty straightforward indeed, utACK 74e7fc7 Tree-SHA512: bcb9752a689a058b6c33317136d16afd694b5da66c69cdd32a8b89b71059c5b2c5dc20c88045c52429fae99d45f508ea223f1b7d1afa5db3c77eb59befd48d51 * Merge #549: [Crypto] Switch to libsecp256k1 signature verification and update the lib f10439c [Crypto] Add ctx initialisation for bip38 (warrows) 21234db [Crypto] Bring back function CKey.SetPrivKey for zPIV (warrows) 65e009a [Tests] Add new auto generated script tests (warrows) 6839f3b [Crypto] Switch from openssl to secp256k1 for consensus (warrows) 8a901f9 Squashed 'src/secp256k1/' content from commit 452d8e4d2 (warrows) d98a584 [Refactor] Delete secp256k1 folder for subtreefication (warrows) Tree-SHA512: f0f6777be57777ba86f83af1b891a6c0f384e6b059afc9249599269c71e5d3bf46a6498325488878af71b6685c6dac6cb672d0147c2ebf43b36f6d786fc38a10 * fix keystore refs * update libscepk265k1 * [RPC/REST] Migrate to libevent based httpd server * fix mempooltojson and misc erroros * Merge #769: [Main] Unify shutdown proceedure in init rather than per-app 518542f [Main] Unify shutdown proceedure in init rather than per-app (Fuzzbawls) Tree-SHA512: 6e734ce002e236c3c2c59c2e8e93d87f5fe577b3f5546804b117a71255266cc4012d847d6a1d9b019a7b763124b45987c2dc3f95294670489cac269beceb2e3a * Merge #542: [Network] Remove vfReachable and modify IsReachable to only use vfLimited. 38ed737 Remove vfReachable and modify IsReachable to only use vfLimited. (Patrick Strateman) Tree-SHA512: 8aa61f9abd8356522f3ba71b35497aac07e10333a39ff4b98bb635d1246e6b1be6b0ee0b236944fbfaa558af0e6c0cc219e540eacb2232bc05db393e4a72b088 * [Refactor] Refactor bignum header file into several files * [zerocoin] Free memory from ToString() Simple fix for a minor memory leak * Revert "[Refactor] Refactor bignum header file into several files" This reverts commit c9aa98f.

* Give error if pow phase is finished * Force Sync to allow for staking without mnsync * Fix error * Update rpcmisc.cpp * Update rpcmisc.cpp * more ignore * Update fundamentalnode-sync.cpp * Add gitian build python script * Update Univalue * add missing gencpp * Add missing includes * Fix Gitian build Update Univalue add missing gencpp Add missing includes * Revert "Update fundamentalnode-sync.cpp" This reverts commit 323b13f * Fix gitian build and minor updates * remove forcesync * [Model] Invalid iterator position crash fix. * background loading crash fixed. * random: fix crash on some 64bit platforms rbx needs to be stashed in a 64bit register on 64bit platforms. With this crash in particular, it was holding a stack canary which was not properly restored after the cpuid. Split out the x86+PIC case so that x86_64 doesn't have to worry about it. * random: Add fallback if getrandom syscall not available If the code was compiled with newer (>=3.17) kernel headers but executed on a system without the system call, every use of random would crash the program. Add a fallback for that case. * Don't assert if we were beaten to the block A timing window exists where a wallet could be creating a new block from within the miner thread when a new block is received to the wallet. This window will create a situation where TestBlockValidity() fails because the chain tip has changed between the time it created the new block and the time it tested the validity of the block. This situation would result in the wallet being asserted; however this is a little overkill. rather than asserting if the tip has changed, it is better to throw the block away. This problem was revealed during a testnet test of an altcoin, and very prevalent when multiple wallet existed with the exact same number of staking coins received in the same transaction; or when multiple wallets were staking the same coins via import private key. The problem happens significantly less in more normal circumstances, but was still observed in a testing environment with fast blocks. It is likely that this scenario has been encountered but never determined to be root cause, as a crashed wallet could be restarted, re-indexed and never investigated further. * [Wallet] Add some LOCK to avoid crash Github-Pull: #625 Rebased-From: ed23d0d62054156e0c2cb632fecbd65588c50e8c * testnet mining fixes * fix placement of utxo signer * dont sign twice * prevent logging twice * random files updates * Add Chacha20 crypto files * Merge #643: [Crypto] Use stronger rand for key generation b7dda924cfc2405386968b0eaa32aa0e546f322c [Log] Replace a string by the function name in a log (warrows) 977f089d0d2883ca3ca1648b14423b738c9248ef [Refactor] Use arrays instead of unic vars in Chacha20 (warrows) d8abe323bd4f5cff3ca23fc00f0093ab7b143ead [Random] Add a missing include (warrows) 27663b8f16dfaac75bae1ab0e4a0c465b838c826 Do not permit copying FastRandomContexts (Pieter Wuille) 64e03e6de1a65c8d934d8df65af48e2b92bf887a Bugfix: randbytes should seed when needed (non reachable issue) (Pieter Wuille) e8f12aa3621e98588b82533edb56ed4865645862 Check if sys/random.h is required for getentropy on OSX. (James Hilliard) de85c7ae55681f8a9d322f4c48c6e9ba15ee0cf1 Add attribute [[noreturn]] (C++11) to functions that will not return (practicalswift) df46c7ff5a2e6a71ba636fa39d77f58e018816a6 Fix resource leak (Dag Robole) d426d856d23fe6c5d6ef3d57fe2c420b9a8e4e5e Clarify entropy source (Pieter Wuille) 30a320b3d12d3529dfca1daae0e477d76acd9ae7 Use cpuid intrinsics instead of asm code (Pieter Wuille) 0c21204e6dc714de6b917d9de12b1029d0bfb67b random: fix crash on some 64bit platforms (Cory Fields) b8bbb9c68d996b1bea579956b3c91872d6d94b8f Use rdrand as entropy source on supported platforms (Pieter Wuille) 8e19443e41f748ad54d0f8aa25907bdad0815c19 [Tests] Fix compilation (warrows) f53edec73901a91a6c4f595b248e4058e8be5468 [Rand/test] scripted-diff: Use new naming style for insecure_rand* functions (warrows) 272f3a5ac83ba0ab72a799ce524b6605b3555751 [Random / tests] scripted-diff: Use randbits/bool instead of randrange (warrows) 0173ee3332990ac1eea134cca5c21cc451b81d92 Replace rand() & ((1 << N) - 1) with randbits(N) (Pieter Wuille) 250de7426e8317f86fc25fe60e960c2e9e903b69 Replace more rand() % NUM by randranges (Pieter Wuille) d6904136030d94e844074fb75b9cafda46253ce0 [Random / tests] scripted-diff: use insecure_rand256/randrange more (warrows) 4a811ff9542439d43e3df3460114d9a24f849a36 Merge test_random.h into test_bitcoin.h (Pieter Wuille) f275e638bf0a752d9f231f2e4a781432deafbfe8 Add various insecure_rand wrappers for tests (Pieter Wuille) 602af4fb3ef45a2a821f1b76d0dbd418ec852683 Add FastRandomContext::rand256() and ::randbytes() (Pieter Wuille) 90549785824f4c13ee8cf36e77f737b5d89d0f02 Add perf counter data to GetStrongRandBytes state in scheduler (Matt Corallo) 17dd13e746b567f44015fbefaa634b278010fccf Add internal method to add new random data to our internal RNG state (Matt Corallo) c7a1602879aff2ae72bdefe959d4026ee8c0aefc Use sanity check timestamps as entropy (Pieter Wuille) f671fe99994cb220dede903459307c61475aeef8 Test that GetPerformanceCounter() increments (Pieter Wuille) dcb536fb2cef719abef9bb4104242410987bfd3d Use hardware timestamps in RNG seeding (Pieter Wuille) 7c3f290b50d32a3290a2c49542d72a3a7466da58 [Random] Fix compilation (warrows) 22b78957cc90252c958bc66439c098001d47b8fc random: only use getentropy on openbsd (Cory Fields) e5750e5c9831edf01026643deed249e0efb28f0b Add a FastRandomContext::randrange and use it (Pieter Wuille) 2a0f6cd1ca2224e2ba30aea4ab312a1919dedc0c Switch FastRandomContext to ChaCha20 (Pieter Wuille) 401ca7db468939184681b73f4b346aebcfc378c6 Introduce FastRandomContext::randbool() (Pieter Wuille) 3d056d6ed478c5bbd42d7dde4f1ab0a75ffd42b1 Add ChaCha20 (Pieter Wuille) 3c97f3f2cb1b3dc2f5d642877d9a2de0a85c06bf Kill insecure_random and associated global state (Wladimir J. van der Laan) 68ba16c78593bf221844dae95fc5edacbf69ba62 Maintain state across GetStrongRandBytes calls (Pieter Wuille) c3c399e199c2d25d10e782dca2ef17f55b6939d0 random: Add fallback if getrandom syscall not available (Wladimir J. van der Laan) 7a8111f915fc7ed23aaec86cd95904bf5d5972fd sanity: Move OS random to sanity check function (Wladimir J. van der Laan) 387c2e9574b56ae941b7723c561ed39488891782 squashme: comment that NUM_OS_RANDOM_BYTES should not be changed lightly (Wladimir J. van der Laan) 9e8c266b158c26b361d68a3cccdbcfd7bee0b376 util: Specific GetOSRandom for Linux/FreeBSD/OpenBSD (Wladimir J. van der Laan) f989b865f215c797e77483a04cc7fe9026904517 Don't use assert for catching randomness failures (Pieter Wuille) a15419e13204d96416b0cce9aad2ae4e35ea5ed5 Always require OS randomness when generating secret keys (Pieter Wuille) Pull request description: Since #576 hasn't changed in over a month, here is a reworked version of it. So in this PR: -We add the memory_cleanse function from upstream, to remove a number of OpenSSL calls. -We use OS randomness in addition to OpenSSL randomness (see #576 for why it's needed). ACKs for top commit: random-zebra: ACK PIVX-Project/PIVX@b7dda92 furszy: ACK [`b7dda92`](PIVX-Project/PIVX@b7dda92) Tree-SHA512: d92cbc14d844263ced753248e646f5cd4f03ec37546f50ff1b558fc3076b6d777c7efcb899c0400bc510e21311b5bd93d9aca26d811033fedb370f8457204035 * Revert "Merge #643: [Crypto] Use stronger rand for key generation" This reverts commit ae3e256. * Revert "Add Chacha20 crypto files" This reverts commit ddd102f. * Revert "random files updates" This reverts commit f29e084. * Revert "random: fix crash on some 64bit platforms" This reverts commit 5a52414. * Revert "random: Add fallback if getrandom syscall not available" This reverts commit b46c1cb. * fix redeclaration * fix signing code * do not return in void func * remove unused funcs * remove on error * revert "Refactor miner" * fix repeat of masternode in fundamental node payment log * fix name collision * [Wallet] Look at last CoinsView block for corruption fix process * Fix crashes * Merge #941: [Refactor] Move ThreadStakeMinter out of net.cpp 42fe4040acff08450168416ccf35413abdc1747a [Refactor] Move ThreadStakeMinter out of net.cpp (Fuzzbawls) Pull request description: This moves the implementation function and thread creation to a more appropriate file (`miner.cpp`). Resolves #938 ACKs for top commit: CaveSpectre11: ACK PIVX-Project/PIVX@42fe404 random-zebra: ACK PIVX-Project/PIVX@42fe404 furszy: utACK [42fe404](PIVX-Project/PIVX@42fe404) Tree-SHA512: d3417f03cab63aa41b1ff0fb4d391d42f4448b83efe70391d4014407008af34eb265a23b5807ebecb90cf9637fb74e5b330b02c92e7038e5400a5f16608800ee * Merge #865: [Main] Don't return an invalid state when shutting down the wallet 8126729 Don't return an invalid state when shutting down the wallet (Fuzzbawls) Tree-SHA512: b7042df42af620faca5d415382ad0a6ae973e9dbad7e527957c2ac3c7d0cb5470fcca555b3fe262f5f06e334504ded657db879c2471ecc290247e0f7e2bbf5f3 * Merge #958: [Staking] Modify miner and staking thread for efficiency b7a9a9cdb20cdee869906fb24e2bc984e8a93802 [Staking] Modify miner and staking thread for efficency (Cave Spectre) Pull request description: ### **Release notes** - [Mining] Unnecessary mining threads now exit after PoS has begun - [Staking] The staking thread dormancy is more efficient during PoW - [Performance] Some unnecessary processing in the mining thread removed - [RPC] setgenerate errors if attempted to turn on after end of PoW - [Build] Wallet only routines conditionalized in miner.h - [Refactoring] Log messages with bitcoin named routines changed for easier rename of routines in the future. In order to better explain the changes in this PR, a review of the existing code would be helpful. ### **Backstory** When fixing a PIVX forked coin's transition from PoW to PoS, it was observed that mining threads go into tight infinite loops after the switch to proof of stake. This can be seen with a simple `pivx-qt -testnet -gen`, and watching the debug log. This observation triggered the below code review (re-written for the current release; some of the findings in the code originally reviewed had already been addressed in PIVX). ### **Code Review** _BitcoinMiner()_ This routine is a worker routine for both staking and mining threads; fProofOfStake is set to true when running in the staking thread, false when running in the mining thread. fGeneratedBitcoins is set when mining is started; and cleared on shutdown or when mining is stopped. _In the initial fProofOfStake section:_ ``` if ((GetTime() - nMintableLastCheck > 5 * 60)) // 5 minute check time { nMintableLastCheck = GetTime(); fMintableCoins = pwallet->MintableCoins(); } if (chainActive.Tip()->nHeight < Params().LAST_POW_BLOCK()) { MilliSleep(5000); continue; } ``` Every 5 minutes this code will scan through the wallet and look to see if mintable coins exist. After it's done that, it checks if the network is still in the proof of work phase. It doesn't make sense to scan the wallet until after the proof of work phase is ending. If it is in the proof of work phase; the staking thread sleeps for 5 seconds (`MilliSleep(5000)`). This only makes sense if the block time is 5 seconds. Technically the thread could determine the number of blocks remaining in the proof of work phase, and use that to calculate a hibernation of sorts. Using the target spacing to wait for the block to pass is a 91% reduction in the number of passes through the loop (12 vs. 1 when the target spacing is one minute), and doesn't run into issues where adjustments to the actual spacing throw the timing of a hibernation off. ``` if (!fMintableCoins) { if (GetTime() - nMintableLastCheck > 1 * 60) // 1 minute check time { nMintableLastCheck = GetTime(); fMintableCoins = pwallet->MintableCoins(); } } MilliSleep(5000); ``` We come to this section of code in the waiting while loop; if we don't have mintable coins yet, or our wallet is locked, or we're not synched; or a host of other potential things that would prevent staking. However, we check again for mintable coins; and then we wait 5 seconds after that check before we come out. Since the first time we come into this loop; we have already checked mintable coins (within the last 5 minutes); we should sleep first, and then do the check closer to the time we're actually going to go around the loop again; so we're working with the most recent data. ``` if (!fGenerateBitcoins && !fProofOfStake) continue; ``` This check is buried in a `if (fProofOfStake)` conditional, where fProofOfStake is a parameter that is passed in by the caller to BitcoinMiner(). `!fProofOfStake` will never be true; so this condition will never be true. Removing it we are left with !fGenerateBitcoins; which is unrelated to this section of code anyway. If we're down to this point, we're not in PoW mode anymore, so there shouldn't be a mining thread (more on that later). But the most compelling part of this condition,whether it passes or not, is that it "continues" the while loop it's in; and since it's at the end of the actual while loop, it's going to iterate into another round of the while loop as soon as it finishes with this conditional anyway. Long story short; the code doesn't do anything. _Overall logic_ The mining thread will run until mining is turned off (`setgenerate false`, or the mining flags taken off the invocation). However, there is no need to continue to mine with proof of work after the proof of stake phase begins. In fact, there really is no consideration of that at all; PoW will continue to try to generate blocks well after PoS has begun. Yes, it's within the users control to stop mining; but it's within the power of the code to take care of that for the user, and stop the mining thread(s) after the transition to proof of stake. ### **This PR:** The logical issues above have been corrected. The dormancy for the proof of stake thread is held for just one block rather than any extreme hibernation. Combined with moving that code to be the first thing in the while loop; it also removes the processing done to search through the wallets looking for stakeable coins. Since a significant amount of code assumes there will not be a transition from PoS to PoW; code was added to BitcoinMiner() to exit the mining threads if it's transitioned into the PoS phase. The threads will continue for a little bit, in case there is any rewind on the chain; but after 6 PoS blocks are accepted, the miner threads will exit. To prevent the PoW thread from trying to generate blocks after the PoS phase has begun, logic was added in CreateNewBlockWithKey() to return quickly. Rather than create a situation where the mining thread ends up in a tight loop in the initial phases of PoS, the mining thread will be held for 1/2 of the target spacing before returning. Lastly; Some tweaks were made to the logging; removing hardcoded references to "BitcoinMiner()", in case refactoring in the future changes the name of the routine. Similarly, since there is a log message when the mining thread starts "PIVXMiner started"; rather than reporting thread exits or errors as "ThreadBitcoinMiner", they have been changed to PIVXMiner to match. Much of this can be observed with an errant `pivx-qt -gen`. With this PR, it simply tries to start the miner, and then exits. _With PR code - `pivx-qt -gen -testnet`_ ``` 2019-07-21 20:04:22 PIVXMiner started 2019-07-21 20:04:22 BitcoinMiner: Exiting Proof of Work Mining Thread at height: 1160732 2019-07-21 20:04:22 PIVXMiner exiting ``` _Current release:_ ``` 2019-07-21 19:37:06 CreateNewBlock(): total size 1000 2019-07-21 19:37:06 ERROR: CheckProofOfWork() : hash doesn't match nBits 2019-07-21 19:37:06 ERROR: CheckBlockHeader() : proof of work failed 2019-07-21 19:37:06 ERROR: CheckBlock() : CheckBlockHeader failed 2019-07-21 19:37:06 CreateNewBlock() : TestBlockValidity failed ``` is repeated in a tight loop. ACKs for top commit: furszy: ACK [b7a9a9c](PIVX-Project/PIVX@b7a9a9c) random-zebra: ACK PIVX-Project/PIVX@b7a9a9c and merging... Tree-SHA512: 064619ce1fde75f1baf35ddb202b4573f2a8c0effc5d2939a678612e3f1d56b08d5c5d7c7bb30ce919e5a648f3f406cca35093b9a3a04c799f746fff5c6a35b6 * remove duplicate stake thread code * add missing include * remove zvitae pos validator * Refine signblock code * revert zerocoin added check * Miner reference updates * revert miner changes * fix fundamentalnode sync checker * [Refactor] Move rpc files to its own folder * update includes * fix rpcserver include to rpc/server.h * [Refactor] Move wallet files to wallet dir * [Trivial] Fix db.h path * Merge #982: [Miner] Don't create new keys when generating PoS blocks 74e7fc7d432c7f6d586d54ff276d6babad0fe505 [Miner] Don't create new keys when generating PoS blocks (random-zebra) Pull request description: This fixes the calls to `CreateNewBlockWithKey` with PoS blocks, which are not needed. We simply call `CreateNewBlock` with an empy script for coinbase. It should prevent the problem described in PIVX-Project/PIVX#956 (comment) ACKs for top commit: CaveSpectre11: utACK PIVX-Project/PIVX@74e7fc7 akshaynexus: utACK PIVX-Project/PIVX@74e7fc7 furszy: utACK [74e7fc7](PIVX-Project/PIVX@74e7fc7) Warrows: Pretty straightforward indeed, utACK 74e7fc7 Tree-SHA512: bcb9752a689a058b6c33317136d16afd694b5da66c69cdd32a8b89b71059c5b2c5dc20c88045c52429fae99d45f508ea223f1b7d1afa5db3c77eb59befd48d51 * Merge #549: [Crypto] Switch to libsecp256k1 signature verification and update the lib f10439c [Crypto] Add ctx initialisation for bip38 (warrows) 21234db [Crypto] Bring back function CKey.SetPrivKey for zPIV (warrows) 65e009a [Tests] Add new auto generated script tests (warrows) 6839f3b [Crypto] Switch from openssl to secp256k1 for consensus (warrows) 8a901f9 Squashed 'src/secp256k1/' content from commit 452d8e4d2 (warrows) d98a584 [Refactor] Delete secp256k1 folder for subtreefication (warrows) Tree-SHA512: f0f6777be57777ba86f83af1b891a6c0f384e6b059afc9249599269c71e5d3bf46a6498325488878af71b6685c6dac6cb672d0147c2ebf43b36f6d786fc38a10 * fix keystore refs * update libscepk265k1 * [RPC/REST] Migrate to libevent based httpd server * fix mempooltojson and misc erroros * Merge #769: [Main] Unify shutdown proceedure in init rather than per-app 518542f [Main] Unify shutdown proceedure in init rather than per-app (Fuzzbawls) Tree-SHA512: 6e734ce002e236c3c2c59c2e8e93d87f5fe577b3f5546804b117a71255266cc4012d847d6a1d9b019a7b763124b45987c2dc3f95294670489cac269beceb2e3a * Merge #542: [Network] Remove vfReachable and modify IsReachable to only use vfLimited. 38ed737 Remove vfReachable and modify IsReachable to only use vfLimited. (Patrick Strateman) Tree-SHA512: 8aa61f9abd8356522f3ba71b35497aac07e10333a39ff4b98bb635d1246e6b1be6b0ee0b236944fbfaa558af0e6c0cc219e540eacb2232bc05db393e4a72b088 * [Refactor] Refactor bignum header file into several files * [zerocoin] Free memory from ToString() Simple fix for a minor memory leak * Revert "[Refactor] Refactor bignum header file into several files" This reverts commit c9aa98f. * fix delay in mining blocks

* Give error if pow phase is finished * Force Sync to allow for staking without mnsync * Fix error * Update rpcmisc.cpp * Update rpcmisc.cpp * more ignore * Update fundamentalnode-sync.cpp * Add gitian build python script * Update Univalue * add missing gencpp * Add missing includes * Fix Gitian build Update Univalue add missing gencpp Add missing includes * Revert "Update fundamentalnode-sync.cpp" This reverts commit 323b13f * Fix gitian build and minor updates * remove forcesync * [Model] Invalid iterator position crash fix. * background loading crash fixed. * random: fix crash on some 64bit platforms rbx needs to be stashed in a 64bit register on 64bit platforms. With this crash in particular, it was holding a stack canary which was not properly restored after the cpuid. Split out the x86+PIC case so that x86_64 doesn't have to worry about it. * random: Add fallback if getrandom syscall not available If the code was compiled with newer (>=3.17) kernel headers but executed on a system without the system call, every use of random would crash the program. Add a fallback for that case. * Don't assert if we were beaten to the block A timing window exists where a wallet could be creating a new block from within the miner thread when a new block is received to the wallet. This window will create a situation where TestBlockValidity() fails because the chain tip has changed between the time it created the new block and the time it tested the validity of the block. This situation would result in the wallet being asserted; however this is a little overkill. rather than asserting if the tip has changed, it is better to throw the block away. This problem was revealed during a testnet test of an altcoin, and very prevalent when multiple wallet existed with the exact same number of staking coins received in the same transaction; or when multiple wallets were staking the same coins via import private key. The problem happens significantly less in more normal circumstances, but was still observed in a testing environment with fast blocks. It is likely that this scenario has been encountered but never determined to be root cause, as a crashed wallet could be restarted, re-indexed and never investigated further. * [Wallet] Add some LOCK to avoid crash Github-Pull: #625 Rebased-From: ed23d0d62054156e0c2cb632fecbd65588c50e8c * testnet mining fixes * fix placement of utxo signer * dont sign twice * prevent logging twice * random files updates * Add Chacha20 crypto files * Merge #643: [Crypto] Use stronger rand for key generation b7dda924cfc2405386968b0eaa32aa0e546f322c [Log] Replace a string by the function name in a log (warrows) 977f089d0d2883ca3ca1648b14423b738c9248ef [Refactor] Use arrays instead of unic vars in Chacha20 (warrows) d8abe323bd4f5cff3ca23fc00f0093ab7b143ead [Random] Add a missing include (warrows) 27663b8f16dfaac75bae1ab0e4a0c465b838c826 Do not permit copying FastRandomContexts (Pieter Wuille) 64e03e6de1a65c8d934d8df65af48e2b92bf887a Bugfix: randbytes should seed when needed (non reachable issue) (Pieter Wuille) e8f12aa3621e98588b82533edb56ed4865645862 Check if sys/random.h is required for getentropy on OSX. (James Hilliard) de85c7ae55681f8a9d322f4c48c6e9ba15ee0cf1 Add attribute [[noreturn]] (C++11) to functions that will not return (practicalswift) df46c7ff5a2e6a71ba636fa39d77f58e018816a6 Fix resource leak (Dag Robole) d426d856d23fe6c5d6ef3d57fe2c420b9a8e4e5e Clarify entropy source (Pieter Wuille) 30a320b3d12d3529dfca1daae0e477d76acd9ae7 Use cpuid intrinsics instead of asm code (Pieter Wuille) 0c21204e6dc714de6b917d9de12b1029d0bfb67b random: fix crash on some 64bit platforms (Cory Fields) b8bbb9c68d996b1bea579956b3c91872d6d94b8f Use rdrand as entropy source on supported platforms (Pieter Wuille) 8e19443e41f748ad54d0f8aa25907bdad0815c19 [Tests] Fix compilation (warrows) f53edec73901a91a6c4f595b248e4058e8be5468 [Rand/test] scripted-diff: Use new naming style for insecure_rand* functions (warrows) 272f3a5ac83ba0ab72a799ce524b6605b3555751 [Random / tests] scripted-diff: Use randbits/bool instead of randrange (warrows) 0173ee3332990ac1eea134cca5c21cc451b81d92 Replace rand() & ((1 << N) - 1) with randbits(N) (Pieter Wuille) 250de7426e8317f86fc25fe60e960c2e9e903b69 Replace more rand() % NUM by randranges (Pieter Wuille) d6904136030d94e844074fb75b9cafda46253ce0 [Random / tests] scripted-diff: use insecure_rand256/randrange more (warrows) 4a811ff9542439d43e3df3460114d9a24f849a36 Merge test_random.h into test_bitcoin.h (Pieter Wuille) f275e638bf0a752d9f231f2e4a781432deafbfe8 Add various insecure_rand wrappers for tests (Pieter Wuille) 602af4fb3ef45a2a821f1b76d0dbd418ec852683 Add FastRandomContext::rand256() and ::randbytes() (Pieter Wuille) 90549785824f4c13ee8cf36e77f737b5d89d0f02 Add perf counter data to GetStrongRandBytes state in scheduler (Matt Corallo) 17dd13e746b567f44015fbefaa634b278010fccf Add internal method to add new random data to our internal RNG state (Matt Corallo) c7a1602879aff2ae72bdefe959d4026ee8c0aefc Use sanity check timestamps as entropy (Pieter Wuille) f671fe99994cb220dede903459307c61475aeef8 Test that GetPerformanceCounter() increments (Pieter Wuille) dcb536fb2cef719abef9bb4104242410987bfd3d Use hardware timestamps in RNG seeding (Pieter Wuille) 7c3f290b50d32a3290a2c49542d72a3a7466da58 [Random] Fix compilation (warrows) 22b78957cc90252c958bc66439c098001d47b8fc random: only use getentropy on openbsd (Cory Fields) e5750e5c9831edf01026643deed249e0efb28f0b Add a FastRandomContext::randrange and use it (Pieter Wuille) 2a0f6cd1ca2224e2ba30aea4ab312a1919dedc0c Switch FastRandomContext to ChaCha20 (Pieter Wuille) 401ca7db468939184681b73f4b346aebcfc378c6 Introduce FastRandomContext::randbool() (Pieter Wuille) 3d056d6ed478c5bbd42d7dde4f1ab0a75ffd42b1 Add ChaCha20 (Pieter Wuille) 3c97f3f2cb1b3dc2f5d642877d9a2de0a85c06bf Kill insecure_random and associated global state (Wladimir J. van der Laan) 68ba16c78593bf221844dae95fc5edacbf69ba62 Maintain state across GetStrongRandBytes calls (Pieter Wuille) c3c399e199c2d25d10e782dca2ef17f55b6939d0 random: Add fallback if getrandom syscall not available (Wladimir J. van der Laan) 7a8111f915fc7ed23aaec86cd95904bf5d5972fd sanity: Move OS random to sanity check function (Wladimir J. van der Laan) 387c2e9574b56ae941b7723c561ed39488891782 squashme: comment that NUM_OS_RANDOM_BYTES should not be changed lightly (Wladimir J. van der Laan) 9e8c266b158c26b361d68a3cccdbcfd7bee0b376 util: Specific GetOSRandom for Linux/FreeBSD/OpenBSD (Wladimir J. van der Laan) f989b865f215c797e77483a04cc7fe9026904517 Don't use assert for catching randomness failures (Pieter Wuille) a15419e13204d96416b0cce9aad2ae4e35ea5ed5 Always require OS randomness when generating secret keys (Pieter Wuille) Pull request description: Since #576 hasn't changed in over a month, here is a reworked version of it. So in this PR: -We add the memory_cleanse function from upstream, to remove a number of OpenSSL calls. -We use OS randomness in addition to OpenSSL randomness (see #576 for why it's needed). ACKs for top commit: random-zebra: ACK PIVX-Project/PIVX@b7dda92 furszy: ACK [`b7dda92`](PIVX-Project/PIVX@b7dda92) Tree-SHA512: d92cbc14d844263ced753248e646f5cd4f03ec37546f50ff1b558fc3076b6d777c7efcb899c0400bc510e21311b5bd93d9aca26d811033fedb370f8457204035 * Revert "Merge #643: [Crypto] Use stronger rand for key generation" This reverts commit ae3e256. * Revert "Add Chacha20 crypto files" This reverts commit ddd102f. * Revert "random files updates" This reverts commit f29e084. * Revert "random: fix crash on some 64bit platforms" This reverts commit 5a52414. * Revert "random: Add fallback if getrandom syscall not available" This reverts commit b46c1cb. * fix redeclaration * fix signing code * do not return in void func * remove unused funcs * remove on error * revert "Refactor miner" * fix repeat of masternode in fundamental node payment log * fix name collision * [Wallet] Look at last CoinsView block for corruption fix process * Fix crashes * Merge #941: [Refactor] Move ThreadStakeMinter out of net.cpp 42fe4040acff08450168416ccf35413abdc1747a [Refactor] Move ThreadStakeMinter out of net.cpp (Fuzzbawls) Pull request description: This moves the implementation function and thread creation to a more appropriate file (`miner.cpp`). Resolves #938 ACKs for top commit: CaveSpectre11: ACK PIVX-Project/PIVX@42fe404 random-zebra: ACK PIVX-Project/PIVX@42fe404 furszy: utACK [42fe404](PIVX-Project/PIVX@42fe404) Tree-SHA512: d3417f03cab63aa41b1ff0fb4d391d42f4448b83efe70391d4014407008af34eb265a23b5807ebecb90cf9637fb74e5b330b02c92e7038e5400a5f16608800ee * Merge #865: [Main] Don't return an invalid state when shutting down the wallet 8126729 Don't return an invalid state when shutting down the wallet (Fuzzbawls) Tree-SHA512: b7042df42af620faca5d415382ad0a6ae973e9dbad7e527957c2ac3c7d0cb5470fcca555b3fe262f5f06e334504ded657db879c2471ecc290247e0f7e2bbf5f3 * Merge #958: [Staking] Modify miner and staking thread for efficiency b7a9a9cdb20cdee869906fb24e2bc984e8a93802 [Staking] Modify miner and staking thread for efficency (Cave Spectre) Pull request description: ### **Release notes** - [Mining] Unnecessary mining threads now exit after PoS has begun - [Staking] The staking thread dormancy is more efficient during PoW - [Performance] Some unnecessary processing in the mining thread removed - [RPC] setgenerate errors if attempted to turn on after end of PoW - [Build] Wallet only routines conditionalized in miner.h - [Refactoring] Log messages with bitcoin named routines changed for easier rename of routines in the future. In order to better explain the changes in this PR, a review of the existing code would be helpful. ### **Backstory** When fixing a PIVX forked coin's transition from PoW to PoS, it was observed that mining threads go into tight infinite loops after the switch to proof of stake. This can be seen with a simple `pivx-qt -testnet -gen`, and watching the debug log. This observation triggered the below code review (re-written for the current release; some of the findings in the code originally reviewed had already been addressed in PIVX). ### **Code Review** _BitcoinMiner()_ This routine is a worker routine for both staking and mining threads; fProofOfStake is set to true when running in the staking thread, false when running in the mining thread. fGeneratedBitcoins is set when mining is started; and cleared on shutdown or when mining is stopped. _In the initial fProofOfStake section:_ ``` if ((GetTime() - nMintableLastCheck > 5 * 60)) // 5 minute check time { nMintableLastCheck = GetTime(); fMintableCoins = pwallet->MintableCoins(); } if (chainActive.Tip()->nHeight < Params().LAST_POW_BLOCK()) { MilliSleep(5000); continue; } ``` Every 5 minutes this code will scan through the wallet and look to see if mintable coins exist. After it's done that, it checks if the network is still in the proof of work phase. It doesn't make sense to scan the wallet until after the proof of work phase is ending. If it is in the proof of work phase; the staking thread sleeps for 5 seconds (`MilliSleep(5000)`). This only makes sense if the block time is 5 seconds. Technically the thread could determine the number of blocks remaining in the proof of work phase, and use that to calculate a hibernation of sorts. Using the target spacing to wait for the block to pass is a 91% reduction in the number of passes through the loop (12 vs. 1 when the target spacing is one minute), and doesn't run into issues where adjustments to the actual spacing throw the timing of a hibernation off. ``` if (!fMintableCoins) { if (GetTime() - nMintableLastCheck > 1 * 60) // 1 minute check time { nMintableLastCheck = GetTime(); fMintableCoins = pwallet->MintableCoins(); } } MilliSleep(5000); ``` We come to this section of code in the waiting while loop; if we don't have mintable coins yet, or our wallet is locked, or we're not synched; or a host of other potential things that would prevent staking. However, we check again for mintable coins; and then we wait 5 seconds after that check before we come out. Since the first time we come into this loop; we have already checked mintable coins (within the last 5 minutes); we should sleep first, and then do the check closer to the time we're actually going to go around the loop again; so we're working with the most recent data. ``` if (!fGenerateBitcoins && !fProofOfStake) continue; ``` This check is buried in a `if (fProofOfStake)` conditional, where fProofOfStake is a parameter that is passed in by the caller to BitcoinMiner(). `!fProofOfStake` will never be true; so this condition will never be true. Removing it we are left with !fGenerateBitcoins; which is unrelated to this section of code anyway. If we're down to this point, we're not in PoW mode anymore, so there shouldn't be a mining thread (more on that later). But the most compelling part of this condition,whether it passes or not, is that it "continues" the while loop it's in; and since it's at the end of the actual while loop, it's going to iterate into another round of the while loop as soon as it finishes with this conditional anyway. Long story short; the code doesn't do anything. _Overall logic_ The mining thread will run until mining is turned off (`setgenerate false`, or the mining flags taken off the invocation). However, there is no need to continue to mine with proof of work after the proof of stake phase begins. In fact, there really is no consideration of that at all; PoW will continue to try to generate blocks well after PoS has begun. Yes, it's within the users control to stop mining; but it's within the power of the code to take care of that for the user, and stop the mining thread(s) after the transition to proof of stake. ### **This PR:** The logical issues above have been corrected. The dormancy for the proof of stake thread is held for just one block rather than any extreme hibernation. Combined with moving that code to be the first thing in the while loop; it also removes the processing done to search through the wallets looking for stakeable coins. Since a significant amount of code assumes there will not be a transition from PoS to PoW; code was added to BitcoinMiner() to exit the mining threads if it's transitioned into the PoS phase. The threads will continue for a little bit, in case there is any rewind on the chain; but after 6 PoS blocks are accepted, the miner threads will exit. To prevent the PoW thread from trying to generate blocks after the PoS phase has begun, logic was added in CreateNewBlockWithKey() to return quickly. Rather than create a situation where the mining thread ends up in a tight loop in the initial phases of PoS, the mining thread will be held for 1/2 of the target spacing before returning. Lastly; Some tweaks were made to the logging; removing hardcoded references to "BitcoinMiner()", in case refactoring in the future changes the name of the routine. Similarly, since there is a log message when the mining thread starts "PIVXMiner started"; rather than reporting thread exits or errors as "ThreadBitcoinMiner", they have been changed to PIVXMiner to match. Much of this can be observed with an errant `pivx-qt -gen`. With this PR, it simply tries to start the miner, and then exits. _With PR code - `pivx-qt -gen -testnet`_ ``` 2019-07-21 20:04:22 PIVXMiner started 2019-07-21 20:04:22 BitcoinMiner: Exiting Proof of Work Mining Thread at height: 1160732 2019-07-21 20:04:22 PIVXMiner exiting ``` _Current release:_ ``` 2019-07-21 19:37:06 CreateNewBlock(): total size 1000 2019-07-21 19:37:06 ERROR: CheckProofOfWork() : hash doesn't match nBits 2019-07-21 19:37:06 ERROR: CheckBlockHeader() : proof of work failed 2019-07-21 19:37:06 ERROR: CheckBlock() : CheckBlockHeader failed 2019-07-21 19:37:06 CreateNewBlock() : TestBlockValidity failed ``` is repeated in a tight loop. ACKs for top commit: furszy: ACK [b7a9a9c](PIVX-Project/PIVX@b7a9a9c) random-zebra: ACK PIVX-Project/PIVX@b7a9a9c and merging... Tree-SHA512: 064619ce1fde75f1baf35ddb202b4573f2a8c0effc5d2939a678612e3f1d56b08d5c5d7c7bb30ce919e5a648f3f406cca35093b9a3a04c799f746fff5c6a35b6 * remove duplicate stake thread code * add missing include * remove zvitae pos validator * Refine signblock code * revert zerocoin added check * Miner reference updates * revert miner changes * fix fundamentalnode sync checker * [Refactor] Move rpc files to its own folder * update includes * fix rpcserver include to rpc/server.h * [Refactor] Move wallet files to wallet dir * [Trivial] Fix db.h path * Merge #982: [Miner] Don't create new keys when generating PoS blocks 74e7fc7d432c7f6d586d54ff276d6babad0fe505 [Miner] Don't create new keys when generating PoS blocks (random-zebra) Pull request description: This fixes the calls to `CreateNewBlockWithKey` with PoS blocks, which are not needed. We simply call `CreateNewBlock` with an empy script for coinbase. It should prevent the problem described in PIVX-Project/PIVX#956 (comment) ACKs for top commit: CaveSpectre11: utACK PIVX-Project/PIVX@74e7fc7 akshaynexus: utACK PIVX-Project/PIVX@74e7fc7 furszy: utACK [74e7fc7](PIVX-Project/PIVX@74e7fc7) Warrows: Pretty straightforward indeed, utACK 74e7fc7 Tree-SHA512: bcb9752a689a058b6c33317136d16afd694b5da66c69cdd32a8b89b71059c5b2c5dc20c88045c52429fae99d45f508ea223f1b7d1afa5db3c77eb59befd48d51 * Merge #549: [Crypto] Switch to libsecp256k1 signature verification and update the lib f10439c [Crypto] Add ctx initialisation for bip38 (warrows) 21234db [Crypto] Bring back function CKey.SetPrivKey for zPIV (warrows) 65e009a [Tests] Add new auto generated script tests (warrows) 6839f3b [Crypto] Switch from openssl to secp256k1 for consensus (warrows) 8a901f9 Squashed 'src/secp256k1/' content from commit 452d8e4d2 (warrows) d98a584 [Refactor] Delete secp256k1 folder for subtreefication (warrows) Tree-SHA512: f0f6777be57777ba86f83af1b891a6c0f384e6b059afc9249599269c71e5d3bf46a6498325488878af71b6685c6dac6cb672d0147c2ebf43b36f6d786fc38a10 * fix keystore refs * update libscepk265k1 * [RPC/REST] Migrate to libevent based httpd server * fix mempooltojson and misc erroros * Merge #769: [Main] Unify shutdown proceedure in init rather than per-app 518542f [Main] Unify shutdown proceedure in init rather than per-app (Fuzzbawls) Tree-SHA512: 6e734ce002e236c3c2c59c2e8e93d87f5fe577b3f5546804b117a71255266cc4012d847d6a1d9b019a7b763124b45987c2dc3f95294670489cac269beceb2e3a * Merge #542: [Network] Remove vfReachable and modify IsReachable to only use vfLimited. 38ed737 Remove vfReachable and modify IsReachable to only use vfLimited. (Patrick Strateman) Tree-SHA512: 8aa61f9abd8356522f3ba71b35497aac07e10333a39ff4b98bb635d1246e6b1be6b0ee0b236944fbfaa558af0e6c0cc219e540eacb2232bc05db393e4a72b088 * [Refactor] Refactor bignum header file into several files * [zerocoin] Free memory from ToString() Simple fix for a minor memory leak * Revert "[Refactor] Refactor bignum header file into several files" This reverts commit c9aa98f. * fix delay in mining blocks * allow turning off mining command on testnet * fix names in reference * rename to vitae in contrib/debian * Vitaefy Debian Docs

* Give error if pow phase is finished * Force Sync to allow for staking without mnsync * Fix error * Update rpcmisc.cpp * Update rpcmisc.cpp * more ignore * Update fundamentalnode-sync.cpp * Add gitian build python script * Update Univalue * add missing gencpp * Add missing includes * Fix Gitian build Update Univalue add missing gencpp Add missing includes * Revert "Update fundamentalnode-sync.cpp" This reverts commit 323b13f * Fix gitian build and minor updates * remove forcesync * [Model] Invalid iterator position crash fix. * background loading crash fixed. * random: fix crash on some 64bit platforms rbx needs to be stashed in a 64bit register on 64bit platforms. With this crash in particular, it was holding a stack canary which was not properly restored after the cpuid. Split out the x86+PIC case so that x86_64 doesn't have to worry about it. * random: Add fallback if getrandom syscall not available If the code was compiled with newer (>=3.17) kernel headers but executed on a system without the system call, every use of random would crash the program. Add a fallback for that case. * Don't assert if we were beaten to the block A timing window exists where a wallet could be creating a new block from within the miner thread when a new block is received to the wallet. This window will create a situation where TestBlockValidity() fails because the chain tip has changed between the time it created the new block and the time it tested the validity of the block. This situation would result in the wallet being asserted; however this is a little overkill. rather than asserting if the tip has changed, it is better to throw the block away. This problem was revealed during a testnet test of an altcoin, and very prevalent when multiple wallet existed with the exact same number of staking coins received in the same transaction; or when multiple wallets were staking the same coins via import private key. The problem happens significantly less in more normal circumstances, but was still observed in a testing environment with fast blocks. It is likely that this scenario has been encountered but never determined to be root cause, as a crashed wallet could be restarted, re-indexed and never investigated further. * [Wallet] Add some LOCK to avoid crash Github-Pull: #625 Rebased-From: ed23d0d62054156e0c2cb632fecbd65588c50e8c * testnet mining fixes * fix placement of utxo signer * dont sign twice * prevent logging twice * random files updates * Add Chacha20 crypto files * Merge #643: [Crypto] Use stronger rand for key generation b7dda924cfc2405386968b0eaa32aa0e546f322c [Log] Replace a string by the function name in a log (warrows) 977f089d0d2883ca3ca1648b14423b738c9248ef [Refactor] Use arrays instead of unic vars in Chacha20 (warrows) d8abe323bd4f5cff3ca23fc00f0093ab7b143ead [Random] Add a missing include (warrows) 27663b8f16dfaac75bae1ab0e4a0c465b838c826 Do not permit copying FastRandomContexts (Pieter Wuille) 64e03e6de1a65c8d934d8df65af48e2b92bf887a Bugfix: randbytes should seed when needed (non reachable issue) (Pieter Wuille) e8f12aa3621e98588b82533edb56ed4865645862 Check if sys/random.h is required for getentropy on OSX. (James Hilliard) de85c7ae55681f8a9d322f4c48c6e9ba15ee0cf1 Add attribute [[noreturn]] (C++11) to functions that will not return (practicalswift) df46c7ff5a2e6a71ba636fa39d77f58e018816a6 Fix resource leak (Dag Robole) d426d856d23fe6c5d6ef3d57fe2c420b9a8e4e5e Clarify entropy source (Pieter Wuille) 30a320b3d12d3529dfca1daae0e477d76acd9ae7 Use cpuid intrinsics instead of asm code (Pieter Wuille) 0c21204e6dc714de6b917d9de12b1029d0bfb67b random: fix crash on some 64bit platforms (Cory Fields) b8bbb9c68d996b1bea579956b3c91872d6d94b8f Use rdrand as entropy source on supported platforms (Pieter Wuille) 8e19443e41f748ad54d0f8aa25907bdad0815c19 [Tests] Fix compilation (warrows) f53edec73901a91a6c4f595b248e4058e8be5468 [Rand/test] scripted-diff: Use new naming style for insecure_rand* functions (warrows) 272f3a5ac83ba0ab72a799ce524b6605b3555751 [Random / tests] scripted-diff: Use randbits/bool instead of randrange (warrows) 0173ee3332990ac1eea134cca5c21cc451b81d92 Replace rand() & ((1 << N) - 1) with randbits(N) (Pieter Wuille) 250de7426e8317f86fc25fe60e960c2e9e903b69 Replace more rand() % NUM by randranges (Pieter Wuille) d6904136030d94e844074fb75b9cafda46253ce0 [Random / tests] scripted-diff: use insecure_rand256/randrange more (warrows) 4a811ff9542439d43e3df3460114d9a24f849a36 Merge test_random.h into test_bitcoin.h (Pieter Wuille) f275e638bf0a752d9f231f2e4a781432deafbfe8 Add various insecure_rand wrappers for tests (Pieter Wuille) 602af4fb3ef45a2a821f1b76d0dbd418ec852683 Add FastRandomContext::rand256() and ::randbytes() (Pieter Wuille) 90549785824f4c13ee8cf36e77f737b5d89d0f02 Add perf counter data to GetStrongRandBytes state in scheduler (Matt Corallo) 17dd13e746b567f44015fbefaa634b278010fccf Add internal method to add new random data to our internal RNG state (Matt Corallo) c7a1602879aff2ae72bdefe959d4026ee8c0aefc Use sanity check timestamps as entropy (Pieter Wuille) f671fe99994cb220dede903459307c61475aeef8 Test that GetPerformanceCounter() increments (Pieter Wuille) dcb536fb2cef719abef9bb4104242410987bfd3d Use hardware timestamps in RNG seeding (Pieter Wuille) 7c3f290b50d32a3290a2c49542d72a3a7466da58 [Random] Fix compilation (warrows) 22b78957cc90252c958bc66439c098001d47b8fc random: only use getentropy on openbsd (Cory Fields) e5750e5c9831edf01026643deed249e0efb28f0b Add a FastRandomContext::randrange and use it (Pieter Wuille) 2a0f6cd1ca2224e2ba30aea4ab312a1919dedc0c Switch FastRandomContext to ChaCha20 (Pieter Wuille) 401ca7db468939184681b73f4b346aebcfc378c6 Introduce FastRandomContext::randbool() (Pieter Wuille) 3d056d6ed478c5bbd42d7dde4f1ab0a75ffd42b1 Add ChaCha20 (Pieter Wuille) 3c97f3f2cb1b3dc2f5d642877d9a2de0a85c06bf Kill insecure_random and associated global state (Wladimir J. van der Laan) 68ba16c78593bf221844dae95fc5edacbf69ba62 Maintain state across GetStrongRandBytes calls (Pieter Wuille) c3c399e199c2d25d10e782dca2ef17f55b6939d0 random: Add fallback if getrandom syscall not available (Wladimir J. van der Laan) 7a8111f915fc7ed23aaec86cd95904bf5d5972fd sanity: Move OS random to sanity check function (Wladimir J. van der Laan) 387c2e9574b56ae941b7723c561ed39488891782 squashme: comment that NUM_OS_RANDOM_BYTES should not be changed lightly (Wladimir J. van der Laan) 9e8c266b158c26b361d68a3cccdbcfd7bee0b376 util: Specific GetOSRandom for Linux/FreeBSD/OpenBSD (Wladimir J. van der Laan) f989b865f215c797e77483a04cc7fe9026904517 Don't use assert for catching randomness failures (Pieter Wuille) a15419e13204d96416b0cce9aad2ae4e35ea5ed5 Always require OS randomness when generating secret keys (Pieter Wuille) Pull request description: Since #576 hasn't changed in over a month, here is a reworked version of it. So in this PR: -We add the memory_cleanse function from upstream, to remove a number of OpenSSL calls. -We use OS randomness in addition to OpenSSL randomness (see #576 for why it's needed). ACKs for top commit: random-zebra: ACK PIVX-Project/PIVX@b7dda92 furszy: ACK [`b7dda92`](PIVX-Project/PIVX@b7dda92) Tree-SHA512: d92cbc14d844263ced753248e646f5cd4f03ec37546f50ff1b558fc3076b6d777c7efcb899c0400bc510e21311b5bd93d9aca26d811033fedb370f8457204035 * Revert "Merge #643: [Crypto] Use stronger rand for key generation" This reverts commit ae3e256. * Revert "Add Chacha20 crypto files" This reverts commit ddd102f. * Revert "random files updates" This reverts commit f29e084. * Revert "random: fix crash on some 64bit platforms" This reverts commit 5a52414. * Revert "random: Add fallback if getrandom syscall not available" This reverts commit b46c1cb. * fix redeclaration * fix signing code * do not return in void func * remove unused funcs * remove on error * revert "Refactor miner" * fix repeat of masternode in fundamental node payment log * fix name collision * [Wallet] Look at last CoinsView block for corruption fix process * Fix crashes * Merge #941: [Refactor] Move ThreadStakeMinter out of net.cpp 42fe4040acff08450168416ccf35413abdc1747a [Refactor] Move ThreadStakeMinter out of net.cpp (Fuzzbawls) Pull request description: This moves the implementation function and thread creation to a more appropriate file (`miner.cpp`). Resolves #938 ACKs for top commit: CaveSpectre11: ACK PIVX-Project/PIVX@42fe404 random-zebra: ACK PIVX-Project/PIVX@42fe404 furszy: utACK [42fe404](PIVX-Project/PIVX@42fe404) Tree-SHA512: d3417f03cab63aa41b1ff0fb4d391d42f4448b83efe70391d4014407008af34eb265a23b5807ebecb90cf9637fb74e5b330b02c92e7038e5400a5f16608800ee * Merge #865: [Main] Don't return an invalid state when shutting down the wallet 8126729 Don't return an invalid state when shutting down the wallet (Fuzzbawls) Tree-SHA512: b7042df42af620faca5d415382ad0a6ae973e9dbad7e527957c2ac3c7d0cb5470fcca555b3fe262f5f06e334504ded657db879c2471ecc290247e0f7e2bbf5f3 * Merge #958: [Staking] Modify miner and staking thread for efficiency b7a9a9cdb20cdee869906fb24e2bc984e8a93802 [Staking] Modify miner and staking thread for efficency (Cave Spectre) Pull request description: ### **Release notes** - [Mining] Unnecessary mining threads now exit after PoS has begun - [Staking] The staking thread dormancy is more efficient during PoW - [Performance] Some unnecessary processing in the mining thread removed - [RPC] setgenerate errors if attempted to turn on after end of PoW - [Build] Wallet only routines conditionalized in miner.h - [Refactoring] Log messages with bitcoin named routines changed for easier rename of routines in the future. In order to better explain the changes in this PR, a review of the existing code would be helpful. ### **Backstory** When fixing a PIVX forked coin's transition from PoW to PoS, it was observed that mining threads go into tight infinite loops after the switch to proof of stake. This can be seen with a simple `pivx-qt -testnet -gen`, and watching the debug log. This observation triggered the below code review (re-written for the current release; some of the findings in the code originally reviewed had already been addressed in PIVX). ### **Code Review** _BitcoinMiner()_ This routine is a worker routine for both staking and mining threads; fProofOfStake is set to true when running in the staking thread, false when running in the mining thread. fGeneratedBitcoins is set when mining is started; and cleared on shutdown or when mining is stopped. _In the initial fProofOfStake section:_ ``` if ((GetTime() - nMintableLastCheck > 5 * 60)) // 5 minute check time { nMintableLastCheck = GetTime(); fMintableCoins = pwallet->MintableCoins(); } if (chainActive.Tip()->nHeight < Params().LAST_POW_BLOCK()) { MilliSleep(5000); continue; } ``` Every 5 minutes this code will scan through the wallet and look to see if mintable coins exist. After it's done that, it checks if the network is still in the proof of work phase. It doesn't make sense to scan the wallet until after the proof of work phase is ending. If it is in the proof of work phase; the staking thread sleeps for 5 seconds (`MilliSleep(5000)`). This only makes sense if the block time is 5 seconds. Technically the thread could determine the number of blocks remaining in the proof of work phase, and use that to calculate a hibernation of sorts. Using the target spacing to wait for the block to pass is a 91% reduction in the number of passes through the loop (12 vs. 1 when the target spacing is one minute), and doesn't run into issues where adjustments to the actual spacing throw the timing of a hibernation off. ``` if (!fMintableCoins) { if (GetTime() - nMintableLastCheck > 1 * 60) // 1 minute check time { nMintableLastCheck = GetTime(); fMintableCoins = pwallet->MintableCoins(); } } MilliSleep(5000); ``` We come to this section of code in the waiting while loop; if we don't have mintable coins yet, or our wallet is locked, or we're not synched; or a host of other potential things that would prevent staking. However, we check again for mintable coins; and then we wait 5 seconds after that check before we come out. Since the first time we come into this loop; we have already checked mintable coins (within the last 5 minutes); we should sleep first, and then do the check closer to the time we're actually going to go around the loop again; so we're working with the most recent data. ``` if (!fGenerateBitcoins && !fProofOfStake) continue; ``` This check is buried in a `if (fProofOfStake)` conditional, where fProofOfStake is a parameter that is passed in by the caller to BitcoinMiner(). `!fProofOfStake` will never be true; so this condition will never be true. Removing it we are left with !fGenerateBitcoins; which is unrelated to this section of code anyway. If we're down to this point, we're not in PoW mode anymore, so there shouldn't be a mining thread (more on that later). But the most compelling part of this condition,whether it passes or not, is that it "continues" the while loop it's in; and since it's at the end of the actual while loop, it's going to iterate into another round of the while loop as soon as it finishes with this conditional anyway. Long story short; the code doesn't do anything. _Overall logic_ The mining thread will run until mining is turned off (`setgenerate false`, or the mining flags taken off the invocation). However, there is no need to continue to mine with proof of work after the proof of stake phase begins. In fact, there really is no consideration of that at all; PoW will continue to try to generate blocks well after PoS has begun. Yes, it's within the users control to stop mining; but it's within the power of the code to take care of that for the user, and stop the mining thread(s) after the transition to proof of stake. ### **This PR:** The logical issues above have been corrected. The dormancy for the proof of stake thread is held for just one block rather than any extreme hibernation. Combined with moving that code to be the first thing in the while loop; it also removes the processing done to search through the wallets looking for stakeable coins. Since a significant amount of code assumes there will not be a transition from PoS to PoW; code was added to BitcoinMiner() to exit the mining threads if it's transitioned into the PoS phase. The threads will continue for a little bit, in case there is any rewind on the chain; but after 6 PoS blocks are accepted, the miner threads will exit. To prevent the PoW thread from trying to generate blocks after the PoS phase has begun, logic was added in CreateNewBlockWithKey() to return quickly. Rather than create a situation where the mining thread ends up in a tight loop in the initial phases of PoS, the mining thread will be held for 1/2 of the target spacing before returning. Lastly; Some tweaks were made to the logging; removing hardcoded references to "BitcoinMiner()", in case refactoring in the future changes the name of the routine. Similarly, since there is a log message when the mining thread starts "PIVXMiner started"; rather than reporting thread exits or errors as "ThreadBitcoinMiner", they have been changed to PIVXMiner to match. Much of this can be observed with an errant `pivx-qt -gen`. With this PR, it simply tries to start the miner, and then exits. _With PR code - `pivx-qt -gen -testnet`_ ``` 2019-07-21 20:04:22 PIVXMiner started 2019-07-21 20:04:22 BitcoinMiner: Exiting Proof of Work Mining Thread at height: 1160732 2019-07-21 20:04:22 PIVXMiner exiting ``` _Current release:_ ``` 2019-07-21 19:37:06 CreateNewBlock(): total size 1000 2019-07-21 19:37:06 ERROR: CheckProofOfWork() : hash doesn't match nBits 2019-07-21 19:37:06 ERROR: CheckBlockHeader() : proof of work failed 2019-07-21 19:37:06 ERROR: CheckBlock() : CheckBlockHeader failed 2019-07-21 19:37:06 CreateNewBlock() : TestBlockValidity failed ``` is repeated in a tight loop. ACKs for top commit: furszy: ACK [b7a9a9c](PIVX-Project/PIVX@b7a9a9c) random-zebra: ACK PIVX-Project/PIVX@b7a9a9c and merging... Tree-SHA512: 064619ce1fde75f1baf35ddb202b4573f2a8c0effc5d2939a678612e3f1d56b08d5c5d7c7bb30ce919e5a648f3f406cca35093b9a3a04c799f746fff5c6a35b6 * remove duplicate stake thread code * add missing include * remove zvitae pos validator * Refine signblock code * revert zerocoin added check * Miner reference updates * revert miner changes * fix fundamentalnode sync checker * [Refactor] Move rpc files to its own folder * update includes * fix rpcserver include to rpc/server.h * [Refactor] Move wallet files to wallet dir * [Trivial] Fix db.h path * Merge #982: [Miner] Don't create new keys when generating PoS blocks 74e7fc7d432c7f6d586d54ff276d6babad0fe505 [Miner] Don't create new keys when generating PoS blocks (random-zebra) Pull request description: This fixes the calls to `CreateNewBlockWithKey` with PoS blocks, which are not needed. We simply call `CreateNewBlock` with an empy script for coinbase. It should prevent the problem described in PIVX-Project/PIVX#956 (comment) ACKs for top commit: CaveSpectre11: utACK PIVX-Project/PIVX@74e7fc7 akshaynexus: utACK PIVX-Project/PIVX@74e7fc7 furszy: utACK [74e7fc7](PIVX-Project/PIVX@74e7fc7) Warrows: Pretty straightforward indeed, utACK 74e7fc7 Tree-SHA512: bcb9752a689a058b6c33317136d16afd694b5da66c69cdd32a8b89b71059c5b2c5dc20c88045c52429fae99d45f508ea223f1b7d1afa5db3c77eb59befd48d51 * Merge #549: [Crypto] Switch to libsecp256k1 signature verification and update the lib f10439c [Crypto] Add ctx initialisation for bip38 (warrows) 21234db [Crypto] Bring back function CKey.SetPrivKey for zPIV (warrows) 65e009a [Tests] Add new auto generated script tests (warrows) 6839f3b [Crypto] Switch from openssl to secp256k1 for consensus (warrows) 8a901f9 Squashed 'src/secp256k1/' content from commit 452d8e4d2 (warrows) d98a584 [Refactor] Delete secp256k1 folder for subtreefication (warrows) Tree-SHA512: f0f6777be57777ba86f83af1b891a6c0f384e6b059afc9249599269c71e5d3bf46a6498325488878af71b6685c6dac6cb672d0147c2ebf43b36f6d786fc38a10 * fix keystore refs * update libscepk265k1 * [RPC/REST] Migrate to libevent based httpd server * fix mempooltojson and misc erroros * Merge #769: [Main] Unify shutdown proceedure in init rather than per-app 518542f [Main] Unify shutdown proceedure in init rather than per-app (Fuzzbawls) Tree-SHA512: 6e734ce002e236c3c2c59c2e8e93d87f5fe577b3f5546804b117a71255266cc4012d847d6a1d9b019a7b763124b45987c2dc3f95294670489cac269beceb2e3a * Merge #542: [Network] Remove vfReachable and modify IsReachable to only use vfLimited. 38ed737 Remove vfReachable and modify IsReachable to only use vfLimited. (Patrick Strateman) Tree-SHA512: 8aa61f9abd8356522f3ba71b35497aac07e10333a39ff4b98bb635d1246e6b1be6b0ee0b236944fbfaa558af0e6c0cc219e540eacb2232bc05db393e4a72b088 * [Refactor] Refactor bignum header file into several files * [zerocoin] Free memory from ToString() Simple fix for a minor memory leak * Revert "[Refactor] Refactor bignum header file into several files" This reverts commit c9aa98f. * fix delay in mining blocks * allow turning off mining command on testnet * fix names in reference * rename to vitae in contrib/debian * Vitaefy Debian Docs * Revert "remove forcesync" This reverts commit c404117. * Update fundamentalnode-sync.cpp

* Depencies, change to macOS and macOS version bump * Update MacOSX to macOS * Fix typo and unneeded var * Added Make Deploy for Portable Installs * Build System Fix(Gitain) * Give error if pow phase is finished * Force Sync to allow for staking without mnsync * Fix error * Update rpcmisc.cpp * Update rpcmisc.cpp * more ignore * Update fundamentalnode-sync.cpp * Add gitian build python script * Update Univalue * add missing gencpp * Add missing includes * Fix Gitian build Update Univalue add missing gencpp Add missing includes * Revert "Update fundamentalnode-sync.cpp" This reverts commit 323b13fc599b03982b0ca7c3f32342537a6b992f * Fix gitian build and minor updates * remove forcesync * Build System Fix(Gitain) (#31) * Give error if pow phase is finished * Force Sync to allow for staking without mnsync * Fix error * Update rpcmisc.cpp * Update rpcmisc.cpp * more ignore * Update fundamentalnode-sync.cpp * Add gitian build python script * Update Univalue * add missing gencpp * Add missing includes * Fix Gitian build Update Univalue add missing gencpp Add missing includes * Revert "Update fundamentalnode-sync.cpp" This reverts commit 323b13fc599b03982b0ca7c3f32342537a6b992f * Fix gitian build and minor updates * remove forcesync * Updates + crash fixes (#34) * Give error if pow phase is finished * Force Sync to allow for staking without mnsync * Fix error * Update rpcmisc.cpp * Update rpcmisc.cpp * more ignore * Update fundamentalnode-sync.cpp * Add gitian build python script * Update Univalue * add missing gencpp * Add missing includes * Fix Gitian build Update Univalue add missing gencpp Add missing includes * Revert "Update fundamentalnode-sync.cpp" This reverts commit 323b13fc599b03982b0ca7c3f32342537a6b992f * Fix gitian build and minor updates * remove forcesync * [Model] Invalid iterator position crash fix. * background loading crash fixed. * random: fix crash on some 64bit platforms rbx needs to be stashed in a 64bit register on 64bit platforms. With this crash in particular, it was holding a stack canary which was not properly restored after the cpuid. Split out the x86+PIC case so that x86_64 doesn't have to worry about it. * random: Add fallback if getrandom syscall not available If the code was compiled with newer (>=3.17) kernel headers but executed on a system without the system call, every use of random would crash the program. Add a fallback for that case. * Don't assert if we were beaten to the block A timing window exists where a wallet could be creating a new block from within the miner thread when a new block is received to the wallet. This window will create a situation where TestBlockValidity() fails because the chain tip has changed between the time it created the new block and the time it tested the validity of the block. This situation would result in the wallet being asserted; however this is a little overkill. rather than asserting if the tip has changed, it is better to throw the block away. This problem was revealed during a testnet test of an altcoin, and very prevalent when multiple wallet existed with the exact same number of staking coins received in the same transaction; or when multiple wallets were staking the same coins via import private key. The problem happens significantly less in more normal circumstances, but was still observed in a testing environment with fast blocks. It is likely that this scenario has been encountered but never determined to be root cause, as a crashed wallet could be restarted, re-indexed and never investigated further. * [Wallet] Add some LOCK to avoid crash Github-Pull: #625 Rebased-From: ed23d0d62054156e0c2cb632fecbd65588c50e8c * Sync develop with master (#35) * Build System Fix(Gitain) * Give error if pow phase is finished * Force Sync to allow for staking without mnsync * Fix error * Update rpcmisc.cpp * Update rpcmisc.cpp * more ignore * Update fundamentalnode-sync.cpp * Add gitian build python script * Update Univalue * add missing gencpp * Add missing includes * Fix Gitian build Update Univalue add missing gencpp Add missing includes * Revert "Update fundamentalnode-sync.cpp" This reverts commit 323b13fc599b03982b0ca7c3f32342537a6b992f * Fix gitian build and minor updates * remove forcesync * Fix for supply --dirty dirty fix for moneysupply * Revert "Fix for supply --dirty" This reverts commit 8060c6c1f840f477cb7803000e91be8a632aa03f. * Another attempt at syncing master and develop (#36) * Build System Fix(Gitain) * Give error if pow phase is finished * Force Sync to allow for staking without mnsync * Fix error * Update rpcmisc.cpp * Update rpcmisc.cpp * more ignore * Update fundamentalnode-sync.cpp * Add gitian build python script * Update Univalue * add missing gencpp * Add missing includes * Fix Gitian build Update Univalue add missing gencpp Add missing includes * Revert "Update fundamentalnode-sync.cpp" This reverts commit 323b13fc599b03982b0ca7c3f32342537a6b992f * Fix gitian build and minor updates * remove forcesync * Fix for supply --dirty dirty fix for moneysupply * Revert "Fix for supply --dirty" This reverts commit 8060c6c1f840f477cb7803000e91be8a632aa03f. * Mining Fixes (#37) * Give error if pow phase is finished * Force Sync to allow for staking without mnsync * Fix error * Update rpcmisc.cpp * Update rpcmisc.cpp * more ignore * Update fundamentalnode-sync.cpp * Add gitian build python script * Update Univalue * add missing gencpp * Add missing includes * Fix Gitian build Update Univalue add missing gencpp Add missing includes * Revert "Update fundamentalnode-sync.cpp" This reverts commit 323b13fc599b03982b0ca7c3f32342537a6b992f * Fix gitian build and minor updates * remove forcesync * [Model] Invalid iterator position crash fix. * background loading crash fixed. * random: fix crash on some 64bit platforms rbx needs to be stashed in a 64bit register on 64bit platforms. With this crash in particular, it was holding a stack canary which was not properly restored after the cpuid. Split out the x86+PIC case so that x86_64 doesn't have to worry about it. * random: Add fallback if getrandom syscall not available If the code was compiled with newer (>=3.17) kernel headers but executed on a system without the system call, every use of random would crash the program. Add a fallback for that case. * Don't assert if we were beaten to the block A timing window exists where a wallet could be creating a new block from within the miner thread when a new block is received to the wallet. This window will create a situation where TestBlockValidity() fails because the chain tip has changed between the time it created the new block and the time it tested the validity of the block. This situation would result in the wallet being asserted; however this is a little overkill. rather than asserting if the tip has changed, it is better to throw the block away. This problem was revealed during a testnet test of an altcoin, and very prevalent when multiple wallet existed with the exact same number of staking coins received in the same transaction; or when multiple wallets were staking the same coins via import private key. The problem happens significantly less in more normal circumstances, but was still observed in a testing environment with fast blocks. It is likely that this scenario has been encountered but never determined to be root cause, as a crashed wallet could be restarted, re-indexed and never investigated further. * [Wallet] Add some LOCK to avoid crash Github-Pull: #625 Rebased-From: ed23d0d62054156e0c2cb632fecbd65588c50e8c * testnet mining fixes * Update developer-notes.md * Update Copyright * Fix compile error (#38) * Give error if pow phase is finished * Force Sync to allow for staking without mnsync * Fix error * Update rpcmisc.cpp * Update rpcmisc.cpp * more ignore * Update fundamentalnode-sync.cpp * Add gitian build python script * Update Univalue * add missing gencpp * Add missing includes * Fix Gitian build Update Univalue add missing gencpp Add missing includes * Revert "Update fundamentalnode-sync.cpp" This reverts commit 323b13fc599b03982b0ca7c3f32342537a6b992f * Fix gitian build and minor updates * remove forcesync * [Model] Invalid iterator position crash fix. * background loading crash fixed. * random: fix crash on some 64bit platforms rbx needs to be stashed in a 64bit register on 64bit platforms. With this crash in particular, it was holding a stack canary which was not properly restored after the cpuid. Split out the x86+PIC case so that x86_64 doesn't have to worry about it. * random: Add fallback if getrandom syscall not available If the code was compiled with newer (>=3.17) kernel headers but executed on a system without the system call, every use of random would crash the program. Add a fallback for that case. * Don't assert if we were beaten to the block A timing window exists where a wallet could be creating a new block from within the miner thread when a new block is received to the wallet. This window will create a situation where TestBlockValidity() fails because the chain tip has changed between the time it created the new block and the time it tested the validity of the block. This situation would result in the wallet being asserted; however this is a little overkill. rather than asserting if the tip has changed, it is better to throw the block away. This problem was revealed during a testnet test of an altcoin, and very prevalent when multiple wallet existed with the exact same number of staking coins received in the same transaction; or when multiple wallets were staking the same coins via import private key. The problem happens significantly less in more normal circumstances, but was still observed in a testing environment with fast blocks. It is likely that this scenario has been encountered but never determined to be root cause, as a crashed wallet could be restarted, re-indexed and never investigated further. * [Wallet] Add some LOCK to avoid crash Github-Pull: #625 Rebased-From: ed23d0d62054156e0c2cb632fecbd65588c50e8c * testnet mining fixes * fix placement of utxo signer * dont sign twice * prevent logging twice * random files updates * Add Chacha20 crypto files * Merge #643: [Crypto] Use stronger rand for key generation b7dda924cfc2405386968b0eaa32aa0e546f322c [Log] Replace a string by the function name in a log (warrows) 977f089d0d2883ca3ca1648b14423b738c9248ef [Refactor] Use arrays instead of unic vars in Chacha20 (warrows) d8abe323bd4f5cff3ca23fc00f0093ab7b143ead [Random] Add a missing include (warrows) 27663b8f16dfaac75bae1ab0e4a0c465b838c826 Do not permit copying FastRandomContexts (Pieter Wuille) 64e03e6de1a65c8d934d8df65af48e2b92bf887a Bugfix: randbytes should seed when needed (non reachable issue) (Pieter Wuille) e8f12aa3621e98588b82533edb56ed4865645862 Check if sys/random.h is required for getentropy on OSX. (James Hilliard) de85c7ae55681f8a9d322f4c48c6e9ba15ee0cf1 Add attribute [[noreturn]] (C++11) to functions that will not return (practicalswift) df46c7ff5a2e6a71ba636fa39d77f58e018816a6 Fix resource leak (Dag Robole) d426d856d23fe6c5d6ef3d57fe2c420b9a8e4e5e Clarify entropy source (Pieter Wuille) 30a320b3d12d3529dfca1daae0e477d76acd9ae7 Use cpuid intrinsics instead of asm code (Pieter Wuille) 0c21204e6dc714de6b917d9de12b1029d0bfb67b random: fix crash on some 64bit platforms (Cory Fields) b8bbb9c68d996b1bea579956b3c91872d6d94b8f Use rdrand as entropy source on supported platforms (Pieter Wuille) 8e19443e41f748ad54d0f8aa25907bdad0815c19 [Tests] Fix compilation (warrows) f53edec73901a91a6c4f595b248e4058e8be5468 [Rand/test] scripted-diff: Use new naming style for insecure_rand* functions (warrows) 272f3a5ac83ba0ab72a799ce524b6605b3555751 [Random / tests] scripted-diff: Use randbits/bool instead of randrange (warrows) 0173ee3332990ac1eea134cca5c21cc451b81d92 Replace rand() & ((1 << N) - 1) with randbits(N) (Pieter Wuille) 250de7426e8317f86fc25fe60e960c2e9e903b69 Replace more rand() % NUM by randranges (Pieter Wuille) d6904136030d94e844074fb75b9cafda46253ce0 [Random / tests] scripted-diff: use insecure_rand256/randrange more (warrows) 4a811ff9542439d43e3df3460114d9a24f849a36 Merge test_random.h into test_bitcoin.h (Pieter Wuille) f275e638bf0a752d9f231f2e4a781432deafbfe8 Add various insecure_rand wrappers for tests (Pieter Wuille) 602af4fb3ef45a2a821f1b76d0dbd418ec852683 Add FastRandomContext::rand256() and ::randbytes() (Pieter Wuille) 90549785824f4c13ee8cf36e77f737b5d89d0f02 Add perf counter data to GetStrongRandBytes state in scheduler (Matt Corallo) 17dd13e746b567f44015fbefaa634b278010fccf Add internal method to add new random data to our internal RNG state (Matt Corallo) c7a1602879aff2ae72bdefe959d4026ee8c0aefc Use sanity check timestamps as entropy (Pieter Wuille) f671fe99994cb220dede903459307c61475aeef8 Test that GetPerformanceCounter() increments (Pieter Wuille) dcb536fb2cef719abef9bb4104242410987bfd3d Use hardware timestamps in RNG seeding (Pieter Wuille) 7c3f290b50d32a3290a2c49542d72a3a7466da58 [Random] Fix compilation (warrows) 22b78957cc90252c958bc66439c098001d47b8fc random: only use getentropy on openbsd (Cory Fields) e5750e5c9831edf01026643deed249e0efb28f0b Add a FastRandomContext::randrange and use it (Pieter Wuille) 2a0f6cd1ca2224e2ba30aea4ab312a1919dedc0c Switch FastRandomContext to ChaCha20 (Pieter Wuille) 401ca7db468939184681b73f4b346aebcfc378c6 Introduce FastRandomContext::randbool() (Pieter Wuille) 3d056d6ed478c5bbd42d7dde4f1ab0a75ffd42b1 Add ChaCha20 (Pieter Wuille) 3c97f3f2cb1b3dc2f5d642877d9a2de0a85c06bf Kill insecure_random and associated global state (Wladimir J. van der Laan) 68ba16c78593bf221844dae95fc5edacbf69ba62 Maintain state across GetStrongRandBytes calls (Pieter Wuille) c3c399e199c2d25d10e782dca2ef17f55b6939d0 random: Add fallback if getrandom syscall not available (Wladimir J. van der Laan) 7a8111f915fc7ed23aaec86cd95904bf5d5972fd sanity: Move OS random to sanity check function (Wladimir J. van der Laan) 387c2e9574b56ae941b7723c561ed39488891782 squashme: comment that NUM_OS_RANDOM_BYTES should not be changed lightly (Wladimir J. van der Laan) 9e8c266b158c26b361d68a3cccdbcfd7bee0b376 util: Specific GetOSRandom for Linux/FreeBSD/OpenBSD (Wladimir J. van der Laan) f989b865f215c797e77483a04cc7fe9026904517 Don't use assert for catching randomness failures (Pieter Wuille) a15419e13204d96416b0cce9aad2ae4e35ea5ed5 Always require OS randomness when generating secret keys (Pieter Wuille) Pull request description: Since #576 hasn't changed in over a month, here is a reworked version of it. So in this PR: -We add the memory_cleanse function from upstream, to remove a number of OpenSSL calls. -We use OS randomness in addition to OpenSSL randomness (see #576 for why it's needed). ACKs for top commit: random-zebra: ACK https://github.com/PIVX-Project/PIVX/pull/643/commits/b7dda924cfc2405386968b0eaa32aa0e546f322c furszy: ACK [`b7dda92`](https://github.com/PIVX-Project/PIVX/commit/b7dda924cfc2405386968b0eaa32aa0e546f322c) Tree-SHA512: d92cbc14d844263ced753248e646f5cd4f03ec37546f50ff1b558fc3076b6d777c7efcb899c0400bc510e21311b5bd93d9aca26d811033fedb370f8457204035 * Revert "Merge #643: [Crypto] Use stronger rand for key generation" This reverts commit ae3e2562220577420e37a6367feeaa3cfedc67e9. * Revert "Add Chacha20 crypto files" This reverts commit ddd102fca86499062dac0ea3a40a8d8b26d039f5. * Revert "random files updates" This reverts commit f29e0844099487811d4f3c7af7f6d089adc2f6cf. * Revert "random: fix crash on some 64bit platforms" This reverts commit 5a5241452e1515fbd554ee99cc14e552fcb98051. * Revert "random: Add fallback if getrandom syscall not available" This reverts commit b46c1cb2b466f4bce60db364bf26f2cf791cdf38. * fix redeclaration * fix signing code * do not return in void func * remove unused funcs * remove on error * revert "Refactor miner" * Fix name collision + mac crashes (#39) * Give error if pow phase is finished * Force Sync to allow for staking without mnsync * Fix error * Update rpcmisc.cpp * Update rpcmisc.cpp * more ignore * Update fundamentalnode-sync.cpp * Add gitian build python script * Update Univalue * add missing gencpp * Add missing includes * Fix Gitian build Update Univalue add missing gencpp Add missing includes * Revert "Update fundamentalnode-sync.cpp" This reverts commit 323b13fc599b03982b0ca7c3f32342537a6b992f * Fix gitian build and minor updates * remove forcesync * [Model] Invalid iterator position crash fix. * background loading crash fixed. * random: fix crash on some 64bit platforms rbx needs to be stashed in a 64bit register on 64bit platforms. With this crash in particular, it was holding a stack canary which was not properly restored after the cpuid. Split out the x86+PIC case so that x86_64 doesn't have to worry about it. * random: Add fallback if getrandom syscall not available If the code was compiled with newer (>=3.17) kernel headers but executed on a system without the system call, every use of random would crash the program. Add a fallback for that case. * Don't assert if we were beaten to the block A timing window exists where a wallet could be creating a new block from within the miner thread when a new block is received to the wallet. This window will create a situation where TestBlockValidity() fails because the chain tip has changed between the time it created the new block and the time it tested the validity of the block. This situation would result in the wallet being asserted; however this is a little overkill. rather than asserting if the tip has changed, it is better to throw the block away. This problem was revealed during a testnet test of an altcoin, and very prevalent when multiple wallet existed with the exact same number of staking coins received in the same transaction; or when multiple wallets were staking the same coins via import private key. The problem happens significantly less in more normal circumstances, but was still observed in a testing environment with fast blocks. It is likely that this scenario has been encountered but never determined to be root cause, as a crashed wallet could be restarted, re-indexed and never investigated further. * [Wallet] Add some LOCK to avoid crash Github-Pull: #625 Rebased-From: ed23d0d62054156e0c2cb632fecbd65588c50e8c * testnet mining fixes * fix placement of utxo signer * dont sign twice * prevent logging twice * random files updates * Add Chacha20 crypto files * Merge #643: [Crypto] Use stronger rand for key generation b7dda924cfc2405386968b0eaa32aa0e546f322c [Log] Replace a string by the function name in a log (warrows) 977f089d0d2883ca3ca1648b14423b738c9248ef [Refactor] Use arrays instead of unic vars in Chacha20 (warrows) d8abe323bd4f5cff3ca23fc00f0093ab7b143ead [Random] Add a missing include (warrows) 27663b8f16dfaac75bae1ab0e4a0c465b838c826 Do not permit copying FastRandomContexts (Pieter Wuille) 64e03e6de1a65c8d934d8df65af48e2b92bf887a Bugfix: randbytes should seed when needed (non reachable issue) (Pieter Wuille) e8f12aa3621e98588b82533edb56ed4865645862 Check if sys/random.h is required for getentropy on OSX. (James Hilliard) de85c7ae55681f8a9d322f4c48c6e9ba15ee0cf1 Add attribute [[noreturn]] (C++11) to functions that will not return (practicalswift) df46c7ff5a2e6a71ba636fa39d77f58e018816a6 Fix resource leak (Dag Robole) d426d856d23fe6c5d6ef3d57fe2c420b9a8e4e5e Clarify entropy source (Pieter Wuille) 30a320b3d12d3529dfca1daae0e477d76acd9ae7 Use cpuid intrinsics instead of asm code (Pieter Wuille) 0c21204e6dc714de6b917d9de12b1029d0bfb67b random: fix crash on some 64bit platforms (Cory Fields) b8bbb9c68d996b1bea579956b3c91872d6d94b8f Use rdrand as entropy source on supported platforms (Pieter Wuille) 8e19443e41f748ad54d0f8aa25907bdad0815c19 [Tests] Fix compilation (warrows) f53edec73901a91a6c4f595b248e4058e8be5468 [Rand/test] scripted-diff: Use new naming style for insecure_rand* functions (warrows) 272f3a5ac83ba0ab72a799ce524b6605b3555751 [Random / tests] scripted-diff: Use randbits/bool instead of randrange (warrows) 0173ee3332990ac1eea134cca5c21cc451b81d92 Replace rand() & ((1 << N) - 1) with randbits(N) (Pieter Wuille) 250de7426e8317f86fc25fe60e960c2e9e903b69 Replace more rand() % NUM by randranges (Pieter Wuille) d6904136030d94e844074fb75b9cafda46253ce0 [Random / tests] scripted-diff: use insecure_rand256/randrange more (warrows) 4a811ff9542439d43e3df3460114d9a24f849a36 Merge test_random.h into test_bitcoin.h (Pieter Wuille) f275e638bf0a752d9f231f2e4a781432deafbfe8 Add various insecure_rand wrappers for tests (Pieter Wuille) 602af4fb3ef45a2a821f1b76d0dbd418ec852683 Add FastRandomContext::rand256() and ::randbytes() (Pieter Wuille) 90549785824f4c13ee8cf36e77f737b5d89d0f02 Add perf counter data to GetStrongRandBytes state in scheduler (Matt Corallo) 17dd13e746b567f44015fbefaa634b278010fccf Add internal method to add new random data to our internal RNG state (Matt Corallo) c7a1602879aff2ae72bdefe959d4026ee8c0aefc Use sanity check timestamps as entropy (Pieter Wuille) f671fe99994cb220dede903459307c61475aeef8 Test that GetPerformanceCounter() increments (Pieter Wuille) dcb536fb2cef719abef9bb4104242410987bfd3d Use hardware timestamps in RNG seeding (Pieter Wuille) 7c3f290b50d32a3290a2c49542d72a3a7466da58 [Random] Fix compilation (warrows) 22b78957cc90252c958bc66439c098001d47b8fc random: only use getentropy on openbsd (Cory Fields) e5750e5c9831edf01026643deed249e0efb28f0b Add a FastRandomContext::randrange and use it (Pieter Wuille) 2a0f6cd1ca2224e2ba30aea4ab312a1919dedc0c Switch FastRandomContext to ChaCha20 (Pieter Wuille) 401ca7db468939184681b73f4b346aebcfc378c6 Introduce FastRandomContext::randbool() (Pieter Wuille) 3d056d6ed478c5bbd42d7dde4f1ab0a75ffd42b1 Add ChaCha20 (Pieter Wuille) 3c97f3f2cb1b3dc2f5d642877d9a2de0a85c06bf Kill insecure_random and associated global state (Wladimir J. van der Laan) 68ba16c78593bf221844dae95fc5edacbf69ba62 Maintain state across GetStrongRandBytes calls (Pieter Wuille) c3c399e199c2d25d10e782dca2ef17f55b6939d0 random: Add fallback if getrandom syscall not available (Wladimir J. van der Laan) 7a8111f915fc7ed23aaec86cd95904bf5d5972fd sanity: Move OS random to sanity check function (Wladimir J. van der Laan) 387c2e9574b56ae941b7723c561ed39488891782 squashme: comment that NUM_OS_RANDOM_BYTES should not be changed lightly (Wladimir J. van der Laan) 9e8c266b158c26b361d68a3cccdbcfd7bee0b376 util: Specific GetOSRandom for Linux/FreeBSD/OpenBSD (Wladimir J. van der Laan) f989b865f215c797e77483a04cc7fe9026904517 Don't use assert for catching randomness failures (Pieter Wuille) a15419e13204d96416b0cce9aad2ae4e35ea5ed5 Always require OS randomness when generating secret keys (Pieter Wuille) Pull request description: Since #576 hasn't changed in over a month, here is a reworked version of it. So in this PR: -We add the memory_cleanse function from upstream, to remove a number of OpenSSL calls. -We use OS randomness in addition to OpenSSL randomness (see #576 for why it's needed). ACKs for top commit: random-zebra: ACK https://github.com/PIVX-Project/PIVX/pull/643/commits/b7dda924cfc2405386968b0eaa32aa0e546f322c furszy: ACK [`b7dda92`](https://github.com/PIVX-Project/PIVX/commit/b7dda924cfc2405386968b0eaa32aa0e546f322c) Tree-SHA512: d92cbc14d844263ced753248e646f5cd4f03ec37546f50ff1b558fc3076b6d777c7efcb899c0400bc510e21311b5bd93d9aca26d811033fedb370f8457204035 * Revert "Merge #643: [Crypto] Use stronger rand for key generation" This reverts commit ae3e2562220577420e37a6367feeaa3cfedc67e9. * Revert "Add Chacha20 crypto files" This reverts commit ddd102fca86499062dac0ea3a40a8d8b26d039f5. * Revert "random files updates" This reverts commit f29e0844099487811d4f3c7af7f6d089adc2f6cf. * Revert "random: fix crash on some 64bit platforms" This reverts commit 5a5241452e1515fbd554ee99cc14e552fcb98051. * Revert "random: Add fallback if getrandom syscall not available" This reverts commit b46c1cb2b466f4bce60db364bf26f2cf791cdf38. * fix redeclaration * fix signing code * do not return in void func * remove unused funcs * remove on error * revert "Refactor miner" * fix repeat of masternode in fundamental node payment log * fix name collision * [Wallet] Look at last CoinsView block for corruption fix process * Fix crashes * Merge #941: [Refactor] Move ThreadStakeMinter out of net.cpp 42fe4040acff08450168416ccf35413abdc1747a [Refactor] Move ThreadStakeMinter out of net.cpp (Fuzzbawls) Pull request description: This moves the implementation function and thread creation to a more appropriate file (`miner.cpp`). Resolves #938 ACKs for top commit: CaveSpectre11: ACK https://github.com/PIVX-Project/PIVX/commit/42fe4040acff08450168416ccf35413abdc1747a random-zebra: ACK https://github.com/PIVX-Project/PIVX/pull/941/commits/42fe4040acff08450168416ccf35413abdc1747a furszy: utACK [42fe404](https://github.com/PIVX-Project/PIVX/pull/941/commits/42fe4040acff08450168416ccf35413abdc1747a) Tree-SHA512: d3417f03cab63aa41b1ff0fb4d391d42f4448b83efe70391d4014407008af34eb265a23b5807ebecb90cf9637fb74e5b330b02c92e7038e5400a5f16608800ee * Merge #865: [Main] Don't return an invalid state when shutting down the wallet 8126729 Don't return an invalid state when shutting down the wallet (Fuzzbawls) Tree-SHA512: b7042df42af620faca5d415382ad0a6ae973e9dbad7e527957c2ac3c7d0cb5470fcca555b3fe262f5f06e334504ded657db879c2471ecc290247e0f7e2bbf5f3 * Merge #958: [Staking] Modify miner and staking thread for efficiency b7a9a9cdb20cdee869906fb24e2bc984e8a93802 [Staking] Modify miner and staking thread for efficency (Cave Spectre) Pull request description: ### **Release notes** - [Mining] Unnecessary mining threads now exit after PoS has begun - [Staking] The staking thread dormancy is more efficient during PoW - [Performance] Some unnecessary processing in the mining thread removed - [RPC] setgenerate errors if attempted to turn on after end of PoW - [Build] Wallet only routines conditionalized in miner.h - [Refactoring] Log messages with bitcoin named routines changed for easier rename of routines in the future. In order to better explain the changes in this PR, a review of the existing code would be helpful. ### **Backstory** When fixing a PIVX forked coin's transition from PoW to PoS, it was observed that mining threads go into tight infinite loops after the switch to proof of stake. This can be seen with a simple `pivx-qt -testnet -gen`, and watching the debug log. This observation triggered the below code review (re-written for the current release; some of the findings in the code originally reviewed had already been addressed in PIVX). ### **Code Review** _BitcoinMiner()_ This routine is a worker routine for both staking and mining threads; fProofOfStake is set to true when running in the staking thread, false when running in the mining thread. fGeneratedBitcoins is set when mining is started; and cleared on shutdown or when mining is stopped. _In the initial fProofOfStake section:_ ``` if ((GetTime() - nMintableLastCheck > 5 * 60)) // 5 minute check time { nMintableLastCheck = GetTime(); fMintableCoins = pwallet->MintableCoins(); } if (chainActive.Tip()->nHeight < Params().LAST_POW_BLOCK()) { MilliSleep(5000); continue; } ``` Every 5 minutes this code will scan through the wallet and look to see if mintable coins exist. After it's done that, it checks if the network is still in the proof of work phase. It doesn't make sense to scan the wallet until after the proof of work phase is ending. If it is in the proof of work phase; the staking thread sleeps for 5 seconds (`MilliSleep(5000)`). This only makes sense if the block time is 5 seconds. Technically the thread could determine the number of blocks remaining in the proof of work phase, and use that to calculate a hibernation of sorts. Using the target spacing to wait for the block to pass is a 91% reduction in the number of passes through the loop (12 vs. 1 when the target spacing is one minute), and doesn't run into issues where adjustments to the actual spacing throw the timing of a hibernation off. ``` if (!fMintableCoins) { if (GetTime() - nMintableLastCheck > 1 * 60) // 1 minute check time { nMintableLastCheck = GetTime(); fMintableCoins = pwallet->MintableCoins(); } } MilliSleep(5000); ``` We come to this section of code in the waiting while loop; if we don't have mintable coins yet, or our wallet is locked, or we're not synched; or a host of other potential things that would prevent staking. However, we check again for mintable coins; and then we wait 5 seconds after that check before we come out. Since the first time we come into this loop; we have already checked mintable coins (within the last 5 minutes); we should sleep first, and then do the check closer to the time we're actually going to go around the loop again; so we're working with the most recent data. ``` if (!fGenerateBitcoins && !fProofOfStake) continue; ``` This check is buried in a `if (fProofOfStake)` conditional, where fProofOfStake is a parameter that is passed in by the caller to BitcoinMiner(). `!fProofOfStake` will never be true; so this condition will never be true. Removing it we are left with !fGenerateBitcoins; which is unrelated to this section of code anyway. If we're down to this point, we're not in PoW mode anymore, so there shouldn't be a mining thread (more on that later). But the most compelling part of this condition,whether it passes or not, is that it "continues" the while loop it's in; and since it's at the end of the actual while loop, it's going to iterate into another round of the while loop as soon as it finishes with this conditional anyway. Long story short; the code doesn't do anything. _Overall logic_ The mining thread will run until mining is turned off (`setgenerate false`, or the mining flags taken off the invocation). However, there is no need to continue to mine with proof of work after the proof of stake phase begins. In fact, there really is no consideration of that at all; PoW will continue to try to generate blocks well after PoS has begun. Yes, it's within the users control to stop mining; but it's within the power of the code to take care of that for the user, and stop the mining thread(s) after the transition to proof of stake. ### **This PR:** The logical issues above have been corrected. The dormancy for the proof of stake thread is held for just one block rather than any extreme hibernation. Combined with moving that code to be the first thing in the while loop; it also removes the processing done to search through the wallets looking for stakeable coins. Since a significant amount of code assumes there will not be a transition from PoS to PoW; code was added to BitcoinMiner() to exit the mining threads if it's transitioned into the PoS phase. The threads will continue for a little bit, in case there is any rewind on the chain; but after 6 PoS blocks are accepted, the miner threads will exit. To prevent the PoW thread from trying to generate blocks after the PoS phase has begun, logic was added in CreateNewBlockWithKey() to return quickly. Rather than create a situation where the mining thread ends up in a tight loop in the initial phases of PoS, the mining thread will be held for 1/2 of the target spacing before returning. Lastly; Some tweaks were made to the logging; removing hardcoded references to "BitcoinMiner()", in case refactoring in the future changes the name of the routine. Similarly, since there is a log message when the mining thread starts "PIVXMiner started"; rather than reporting thread exits or errors as "ThreadBitcoinMiner", they have been changed to PIVXMiner to match. Much of this can be observed with an errant `pivx-qt -gen`. With this PR, it simply tries to start the miner, and then exits. _With PR code - `pivx-qt -gen -testnet`_ ``` 2019-07-21 20:04:22 PIVXMiner started 2019-07-21 20:04:22 BitcoinMiner: Exiting Proof of Work Mining Thread at height: 1160732 2019-07-21 20:04:22 PIVXMiner exiting ``` _Current release:_ ``` 2019-07-21 19:37:06 CreateNewBlock(): total size 1000 2019-07-21 19:37:06 ERROR: CheckProofOfWork() : hash doesn't match nBits 2019-07-21 19:37:06 ERROR: CheckBlockHeader() : proof of work failed 2019-07-21 19:37:06 ERROR: CheckBlock() : CheckBlockHeader failed 2019-07-21 19:37:06 CreateNewBlock() : TestBlockValidity failed ``` is repeated in a tight loop. ACKs for top commit: furszy: ACK [b7a9a9c](https://github.com/PIVX-Project/PIVX/commit/b7a9a9cdb20cdee869906fb24e2bc984e8a93802) random-zebra: ACK https://github.com/PIVX-Project/PIVX/pull/958/commits/b7a9a9cdb20cdee869906fb24e2bc984e8a93802 and merging... Tree-SHA512: 064619ce1fde75f1baf35ddb202b4573f2a8c0effc5d2939a678612e3f1d56b08d5c5d7c7bb30ce919e5a648f3f406cca35093b9a3a04c799f746fff5c6a35b6 * remove duplicate stake thread code * add missing include * remove zvitae pos validator * Refine signblock code * revert zerocoin added check * Miner reference updates * revert miner changes * fix fundamentalnode sync checker * [Refactor] Move rpc files to its own folder * update includes * fix rpcserver include to rpc/server.h * [Refactor] Move wallet files to wallet dir * [Trivial] Fix db.h path * Merge #982: [Miner] Don't create new keys when generating PoS blocks 74e7fc7d432c7f6d586d54ff276d6babad0fe505 [Miner] Don't create new keys when generating PoS blocks (random-zebra) Pull request description: This fixes the calls to `CreateNewBlockWithKey` with PoS blocks, which are not needed. We simply call `CreateNewBlock` with an empy script for coinbase. It should prevent the problem described in https://github.com/PIVX-Project/PIVX/pull/956#issuecomment-520166966 ACKs for top commit: CaveSpectre11: utACK https://github.com/PIVX-Project/PIVX/pull/982/commits/74e7fc7d432c7f6d586d54ff276d6babad0fe505 akshaynexus: utACK https://github.com/PIVX-Project/PIVX/commit/74e7fc7d432c7f6d586d54ff276d6babad0fe505 furszy: utACK [74e7fc7](https://github.com/PIVX-Project/PIVX/pull/982/commits/74e7fc7d432c7f6d586d54ff276d6babad0fe505) Warrows: Pretty straightforward indeed, utACK 74e7fc7 Tree-SHA512: bcb9752a689a058b6c33317136d16afd694b5da66c69cdd32a8b89b71059c5b2c5dc20c88045c52429fae99d45f508ea223f1b7d1afa5db3c77eb59befd48d51 * Merge #549: [Crypto] Switch to libsecp256k1 signature verification and update the lib f10439c [Crypto] Add ctx initialisation for bip38 (warrows) 21234db [Crypto] Bring back function CKey.SetPrivKey for zPIV (warrows) 65e009a [Tests] Add new auto generated script tests (warrows) 6839f3b [Crypto] Switch from openssl to secp256k1 for consensus (warrows) 8a901f9 Squashed 'src/secp256k1/' content from commit 452d8e4d2 (warrows) d98a584 [Refactor] Delete secp256k1 folder for subtreefication (warrows) Tree-SHA512: f0f6777be57777ba86f83af1b891a6c0f384e6b059afc9249599269c71e5d3bf46a6498325488878af71b6685c6dac6cb672d0147c2ebf43b36f6d786fc38a10 * fix keystore refs * update libscepk265k1 * [RPC/REST] Migrate to libevent based httpd server * fix mempooltojson and misc erroros * Merge #769: [Main] Unify shutdown proceedure in init rather than per-app 518542f [Main] Unify shutdown proceedure in init rather than per-app (Fuzzbawls) Tree-SHA512: 6e734ce002e236c3c2c59c2e8e93d87f5fe577b3f5546804b117a71255266cc4012d847d6a1d9b019a7b763124b45987c2dc3f95294670489cac269beceb2e3a * Merge #542: [Network] Remove vfReachable and modify IsReachable to only use vfLimited. 38ed737 Remove vfReachable and modify IsReachable to only use vfLimited. (Patrick Strateman) Tree-SHA512: 8aa61f9abd8356522f3ba71b35497aac07e10333a39ff4b98bb635d1246e6b1be6b0ee0b236944fbfaa558af0e6c0cc219e540eacb2232bc05db393e4a72b088 * [Refactor] Refactor bignum header file into several files * [zerocoin] Free memory from ToString() Simple fix for a minor memory leak * Revert "[Refactor] Refactor bignum header file into several files" This reverts commit c9aa98fe451e27d2bb4564747ffb0768cce56d53. * fix delays in mining code in testnet (#40) * Give error if pow phase is finished * Force Sync to allow for staking without mnsync * Fix error * Update rpcmisc.cpp * Update rpcmisc.cpp * more ignore * Update fundamentalnode-sync.cpp * Add gitian build python script * Update Univalue * add missing gencpp * Add missing includes * Fix Gitian build Update Univalue add missing gencpp Add missing includes * Revert "Update fundamentalnode-sync.cpp" This reverts commit 323b13fc599b03982b0ca7c3f32342537a6b992f * Fix gitian build and minor updates * remove forcesync * [Model] Invalid iterator position crash fix. * background loading crash fixed. * random: fix crash on some 64bit platforms rbx needs to be stashed in a 64bit register on 64bit platforms. With this crash in particular, it was holding a stack canary which was not properly restored after the cpuid. Split out the x86+PIC case so that x86_64 doesn't have to worry about it. * random: Add fallback if getrandom syscall not available If the code was compiled with newer (>=3.17) kernel headers but executed on a system without the system call, every use of random would crash the program. Add a fallback for that case. * Don't assert if we were beaten to the block A timing window exists where a wallet could be creating a new block from within the miner thread when a new block is received to the wallet. This window will create a situation where TestBlockValidity() fails because the chain tip has changed between the time it created the new block and the time it tested the validity of the block. This situation would result in the wallet being asserted; however this is a little overkill. rather than asserting if the tip has changed, it is better to throw the block away. This problem was revealed during a testnet test of an altcoin, and very prevalent when multiple wallet existed with the exact same number of staking coins received in the same transaction; or when multiple wallets were staking the same coins via import private key. The problem happens significantly less in more normal circumstances, but was still observed in a testing environment with fast blocks. It is likely that this scenario has been encountered but never determined to be root cause, as a crashed wallet could be restarted, re-indexed and never investigated further. * [Wallet] Add some LOCK to avoid crash Github-Pull: #625 Rebased-From: ed23d0d62054156e0c2cb632fecbd65588c50e8c * testnet mining fixes * fix placement of utxo signer * dont sign twice * prevent logging twice * random files updates * Add Chacha20 crypto files * Merge #643: [Crypto] Use stronger rand for key generation b7dda924cfc2405386968b0eaa32aa0e546f322c [Log] Replace a string by the function name in a log (warrows) 977f089d0d2883ca3ca1648b14423b738c9248ef [Refactor] Use arrays instead of unic vars in Chacha20 (warrows) d8abe323bd4f5cff3ca23fc00f0093ab7b143ead [Random] Add a missing include (warrows) 27663b8f16dfaac75bae1ab0e4a0c465b838c826 Do not permit copying FastRandomContexts (Pieter Wuille) 64e03e6de1a65c8d934d8df65af48e2b92bf887a Bugfix: randbytes should seed when needed (non reachable issue) (Pieter Wuille) e8f12aa3621e98588b82533edb56ed4865645862 Check if sys/random.h is required for getentropy on OSX. (James Hilliard) de85c7ae55681f8a9d322f4c48c6e9ba15ee0cf1 Add attribute [[noreturn]] (C++11) to functions that will not return (practicalswift) df46c7ff5a2e6a71ba636fa39d77f58e018816a6 Fix resource leak (Dag Robole) d426d856d23fe6c5d6ef3d57fe2c420b9a8e4e5e Clarify entropy source (Pieter Wuille) 30a320b3d12d3529dfca1daae0e477d76acd9ae7 Use cpuid intrinsics instead of asm code (Pieter Wuille) 0c21204e6dc714de6b917d9de12b1029d0bfb67b random: fix crash on some 64bit platforms (Cory Fields) b8bbb9c68d996b1bea579956b3c91872d6d94b8f Use rdrand as entropy source on supported platforms (Pieter Wuille) 8e19443e41f748ad54d0f8aa25907bdad0815c19 [Tests] Fix compilation (warrows) f53edec73901a91a6c4f595b248e4058e8be5468 [Rand/test] scripted-diff: Use new naming style for insecure_rand* functions (warrows) 272f3a5ac83ba0ab72a799ce524b6605b3555751 [Random / tests] scripted-diff: Use randbits/bool instead of randrange (warrows) 0173ee3332990ac1eea134cca5c21cc451b81d92 Replace rand() & ((1 << N) - 1) with randbits(N) (Pieter Wuille) 250de7426e8317f86fc25fe60e960c2e9e903b69 Replace more rand() % NUM by randranges (Pieter Wuille) d6904136030d94e844074fb75b9cafda46253ce0 [Random / tests] scripted-diff: use insecure_rand256/randrange more (warrows) 4a811ff9542439d43e3df3460114d9a24f849a36 Merge test_random.h into test_bitcoin.h (Pieter Wuille) f275e638bf0a752d9f231f2e4a781432deafbfe8 Add various insecure_rand wrappers for tests (Pieter Wuille) 602af4fb3ef45a2a821f1b76d0dbd418ec852683 Add FastRandomContext::rand256() and ::randbytes() (Pieter Wuille) 90549785824f4c13ee8cf36e77f737b5d89d0f02 Add perf counter data to GetStrongRandBytes state in scheduler (Matt Corallo) 17dd13e746b567f44015fbefaa634b278010fccf Add internal method to add new random data to our internal RNG state (Matt Corallo) c7a1602879aff2ae72bdefe959d4026ee8c0aefc Use sanity check timestamps as entropy (Pieter Wuille) f671fe99994cb220dede903459307c61475aeef8 Test that GetPerformanceCounter() increments (Pieter Wuille) dcb536fb2cef719abef9bb4104242410987bfd3d Use hardware timestamps in RNG seeding (Pieter Wuille) 7c3f290b50d32a3290a2c49542d72a3a7466da58 [Random] Fix compilation (warrows) 22b78957cc90252c958bc66439c098001d47b8fc random: only use getentropy on openbsd (Cory Fields) e5750e5c9831edf01026643deed249e0efb28f0b Add a FastRandomContext::randrange and use it (Pieter Wuille) 2a0f6cd1ca2224e2ba30aea4ab312a1919dedc0c Switch FastRandomContext to ChaCha20 (Pieter Wuille) 401ca7db468939184681b73f4b346aebcfc378c6 Introduce FastRandomContext::randbool() (Pieter Wuille) 3d056d6ed478c5bbd42d7dde4f1ab0a75ffd42b1 Add ChaCha20 (Pieter Wuille) 3c97f3f2cb1b3dc2f5d642877d9a2de0a85c06bf Kill insecure_random and associated global state (Wladimir J. van der Laan) 68ba16c78593bf221844dae95fc5edacbf69ba62 Maintain state across GetStrongRandBytes calls (Pieter Wuille) c3c399e199c2d25d10e782dca2ef17f55b6939d0 random: Add fallback if getrandom syscall not available (Wladimir J. van der Laan) 7a8111f915fc7ed23aaec86cd95904bf5d5972fd sanity: Move OS random to sanity check function (Wladimir J. van der Laan) 387c2e9574b56ae941b7723c561ed39488891782 squashme: comment that NUM_OS_RANDOM_BYTES should not be changed lightly (Wladimir J. van der Laan) 9e8c266b158c26b361d68a3cccdbcfd7bee0b376 util: Specific GetOSRandom for Linux/FreeBSD/OpenBSD (Wladimir J. van der Laan) f989b865f215c797e77483a04cc7fe9026904517 Don't use assert for catching randomness failures (Pieter Wuille) a15419e13204d96416b0cce9aad2ae4e35ea5ed5 Always require OS randomness when generating secret keys (Pieter Wuille) Pull request description: Since #576 hasn't changed in over a month, here is a reworked version of it. So in this PR: -We add the memory_cleanse function from upstream, to remove a number of OpenSSL calls. -We use OS randomness in addition to OpenSSL randomness (see #576 for why it's needed). ACKs for top commit: random-zebra: ACK https://github.com/PIVX-Project/PIVX/pull/643/commits/b7dda924cfc2405386968b0eaa32aa0e546f322c furszy: ACK [`b7dda92`](https://github.com/PIVX-Project/PIVX/commit/b7dda924cfc2405386968b0eaa32aa0e546f322c) Tree-SHA512: d92cbc14d844263ced753248e646f5cd4f03ec37546f50ff1b558fc3076b6d777c7efcb899c0400bc510e21311b5bd93d9aca26d811033fedb370f8457204035 * Revert "Merge #643: [Crypto] Use stronger rand for key generation" This reverts commit ae3e2562220577420e37a6367feeaa3cfedc67e9. * Revert "Add Chacha20 crypto files" This reverts commit ddd102fca86499062dac0ea3a40a8d8b26d039f5. * Revert "random files updates" This reverts commit f29e0844099487811d4f3c7af7f6d089adc2f6cf. * Revert "random: fix crash on some 64bit platforms" This reverts commit 5a5241452e1515fbd554ee99cc14e552fcb98051. * Revert "random: Add fallback if getrandom syscall not available" This reverts commit b46c1cb2b466f4bce60db364bf26f2cf791cdf38. * fix redeclaration * fix signing code * do not return in void func * remove unused funcs * remove on error * revert "Refactor miner" * fix repeat of masternode in fundamental node payment log * fix name collision * [Wallet] Look at last CoinsView block for corruption fix process * Fix crashes * Merge #941: [Refactor] Move ThreadStakeMinter out of net.cpp 42fe4040acff08450168416ccf35413abdc1747a [Refactor] Move ThreadStakeMinter out of net.cpp (Fuzzbawls) Pull request description: This moves the implementation function and thread creation to a more appropriate file (`miner.cpp`). Resolves #938 ACKs for top commit: CaveSpectre11: ACK https://github.com/PIVX-Project/PIVX/commit/42fe4040acff08450168416ccf35413abdc1747a random-zebra: ACK https://github.com/PIVX-Project/PIVX/pull/941/commits/42fe4040acff08450168416ccf35413abdc1747a furszy: utACK [42fe404](https://github.com/PIVX-Project/PIVX/pull/941/commits/42fe4040acff08450168416ccf35413abdc1747a) Tree-SHA512: d3417f03cab63aa41b1ff0fb4d391d42f4448b83efe70391d4014407008af34eb265a23b5807ebecb90cf9637fb74e5b330b02c92e7038e5400a5f16608800ee * Merge #865: [Main] Don't return an invalid state when shutting down the wallet 8126729 Don't return an invalid state when shutting down the wallet (Fuzzbawls) Tree-SHA512: b7042df42af620faca5d415382ad0a6ae973e9dbad7e527957c2ac3c7d0cb5470fcca555b3fe262f5f06e334504ded657db879c2471ecc290247e0f7e2bbf5f3 * Merge #958: [Staking] Modify miner and staking thread for efficiency b7a9a9cdb20cdee869906fb24e2bc984e8a93802 [Staking] Modify miner and staking thread for efficency (Cave Spectre) Pull request description: ### **Release notes** - [Mining] Unnecessary mining threads now exit after PoS has begun - [Staking] The staking thread dormancy is more efficient during PoW - [Performance] Some unnecessary processing in the mining thread removed - [RPC] setgenerate errors if attempted to turn on after end of PoW - [Build] Wallet only routines conditionalized in miner.h - [Refactoring] Log messages with bitcoin named routines changed for easier rename of routines in the future. In order to better explain the changes in this PR, a review of the existing code would be helpful. ### **Backstory** When fixing a PIVX forked coin's transition from PoW to PoS, it was observed that mining threads go into tight infinite loops after the switch to proof of stake. This can be seen with a simple `pivx-qt -testnet -gen`, and watching the debug log. This observation triggered the below code review (re-written for the current release; some of the findings in the code originally reviewed had already been addressed in PIVX). ### **Code Review** _BitcoinMiner()_ This routine is a worker routine for both staking and mining threads; fProofOfStake is set to true when running in the staking thread, false when running in the mining thread. fGeneratedBitcoins is set when mining is started; and cleared on shutdown or when mining is stopped. _In the initial fProofOfStake section:_ ``` if ((GetTime() - nMintableLastCheck > 5 * 60)) // 5 minute check time { nMintableLastCheck = GetTime(); fMintableCoins = pwallet->MintableCoins(); } if (chainActive.Tip()->nHeight < Params().LAST_POW_BLOCK()) { MilliSleep(5000); continue; } ``` Every 5 minutes this code will scan through the wallet and look to see if mintable coins exist. After it's done that, it checks if the network is still in the proof of work phase. It doesn't make sense to scan the wallet until after the proof of work phase is ending. If it is in the proof of work phase; the staking thread sleeps for 5 seconds (`MilliSleep(5000)`). This only makes sense if the block time is 5 seconds. Technically the thread could determine the number of blocks remaining in the proof of work phase, and use that to calculate a hibernation of sorts. Using the target spacing to wait for the block to pass is a 91% reduction in the number of passes through the loop (12 vs. 1 when the target spacing is one minute), and doesn't run into issues where adjustments to the actual spacing throw the timing of a hibernation off. ``` if (!fMintableCoins) { if (GetTime() - nMintableLastCheck > 1 * 60) // 1 minute check time { nMintableLastCheck = GetTime(); fMintableCoins = pwallet->MintableCoins(); } } MilliSleep(5000); ``` We come to this section of code in the waiting while loop; if we don't have mintable coins yet, or our wallet is locked, or we're not synched; or a host of other potential things that would prevent staking. However, we check again for mintable coins; and then we wait 5 seconds after that check before we come out. Since the first time we come into this loop; we have already checked mintable coins (within the last 5 minutes); we should sleep first, and then do the check closer to the time we're actually going to go around the loop again; so we're working with the most recent data. ``` if (!fGenerateBitcoins && !fProofOfStake) continue; ``` This check is buried in a `if (fProofOfStake)` conditional, where fProofOfStake is a parameter that is passed in by the caller to BitcoinMiner(). `!fProofOfStake` will never be true; so this condition will never be true. Removing it we are left with !fGenerateBitcoins; which is unrelated to this section of code anyway. If we're down to this point, we're not in PoW mode anymore, so there shouldn't be a mining thread (more on that later). But the most compelling part of this condition,whether it passes or not, is that it "continues" the while loop it's in; and since it's at the end of the actual while loop, it's going to iterate into another round of the while loop as soon as it finishes with this conditional anyway. Long story short; the code doesn't do anything. _Overall logic_ The mining thread will run until mining is turned off (`setgenerate false`, or the mining flags taken off the invocation). However, there is no need to continue to mine with proof of work after the proof of stake phase begins. In fact, there really is no consideration of that at all; PoW will continue to try to generate blocks well after PoS has begun. Yes, it's within the users control to stop mining; but it's within the power of the code to take care of that for the user, and stop the mining thread(s) after the transition to proof of stake. ### **This PR:** The logical issues above have been corrected. The dormancy for the proof of stake thread is held for just one block rather than any extreme hibernation. Combined with moving that code to be the first thing in the while loop; it also removes the processing done to search through the wallets looking for stakeable coins. Since a significant amount of code assumes there will not be a transition from PoS to PoW; code was added to BitcoinMiner() to exit the mining threads if it's transitioned into the PoS phase. The threads will continue for a little bit, in case there is any rewind on the chain; but after 6 PoS blocks are accepted, the miner threads will exit. To prevent the PoW thread from trying to generate blocks after the PoS phase has begun, logic was added in CreateNewBlockWithKey() to return quickly. Rather than create a situation where the mining thread ends up in a tight loop in the initial phases of PoS, the mining thread will be held for 1/2 of the target spacing before returning. Lastly; Some tweaks were made to the logging; removing hardcoded references to "BitcoinMiner()", in case refactoring in the future changes the name of the routine. Similarly, since there is a log message when the mining thread starts "PIVXMiner started"; rather than reporting thread exits or errors as "ThreadBitcoinMiner", they have been changed to PIVXMiner to match. Much of this can be observed with an errant `pivx-qt -gen`. With this PR, it simply tries to start the miner, and then exits. _With PR code - `pivx-qt -gen -testnet`_ ``` 2019-07-21 20:04:22 PIVXMiner started 2019-07-21 20:04:22 BitcoinMiner: Exiting Proof of Work Mining Thread at height: 1160732 2019-07-21 20:04:22 PIVXMiner exiting ``` _Current release:_ ``` 2019-07-21 19:37:06 CreateNewBlock(): total size 1000 2019-07-21 19:37:06 ERROR: CheckProofOfWork() : hash doesn't match nBits 2019-07-21 19:37:06 ERROR: CheckBlockHeader() : proof of work failed 2019-07-21 19:37:06 ERROR: CheckBlock() : CheckBlockHeader failed 2019-07-21 19:37:06 CreateNewBlock() : TestBlockValidity failed ``` is repeated in a tight loop. ACKs for top commit: furszy: ACK [b7a9a9c](https://github.com/PIVX-Project/PIVX/commit/b7a9a9cdb20cdee869906fb24e2bc984e8a93802) random-zebra: ACK https://github.com/PIVX-Project/PIVX/pull/958/commits/b7a9a9cdb20cdee869906fb24e2bc984e8a93802 and merging... Tree-SHA512: 064619ce1fde75f1baf35ddb202b4573f2a8c0effc5d2939a678612e3f1d56b08d5c5d7c7bb30ce919e5a648f3f406cca35093b9a3a04c799f746fff5c6a35b6 * remove duplicate stake thread code * add missing include * remove zvitae pos validator * Refine signblock code * revert zerocoin added check * Miner reference updates * revert miner changes * fix fundamentalnode sync checker * [Refactor] Move rpc files to its own folder * update includes * fix rpcserver include to rpc/server.h * [Refactor] Move wallet files to wallet dir * [Trivial] Fix db.h path * Merge #982: [Miner] Don't create new keys when generating PoS blocks 74e7fc7d432c7f6d586d54ff276d6babad0fe505 [Miner] Don't create new keys when generating PoS blocks (random-zebra) Pull request description: This fixes the calls to `CreateNewBlockWithKey` with PoS blocks, which are not needed. We simply call `CreateNewBlock` with an empy script for coinbase. It should prevent the problem described in https://github.com/PIVX-Project/PIVX/pull/956#issuecomment-520166966 ACKs for top commit: CaveSpectre11: utACK https://github.com/PIVX-Project/PIVX/pull/982/commits/74e7fc7d432c7f6d586d54ff276d6babad0fe505 akshaynexus: utACK https://github.com/PIVX-Project/PIVX/commit/74e7fc7d432c7f6d586d54ff276d6babad0fe505 furszy: utACK [74e7fc7](https://github.com/PIVX-Project/PIVX/pull/982/commits/74e7fc7d432c7f6d586d54ff276d6babad0fe505) Warrows: Pretty straightforward indeed, utACK 74e7fc7 Tree-SHA512: bcb9752a689a058b6c33317136d16afd694b5da66c69cdd32a8b89b71059c5b2c5dc20c88045c52429fae99d45f508ea223f1b7d1afa5db3c77eb59befd48d51 * Merge #549: [Crypto] Switch to libsecp256k1 signature verification and update the lib f10439c [Crypto] Add ctx initialisation for bip38 (warrows) 21234db [Crypto] Bring back function CKey.SetPrivKey for zPIV (warrows) 65e009a [Tests] Add new auto generated script tests (warrows) 6839f3b [Crypto] Switch from openssl to secp256k1 for consensus (warrows) 8a901f9 Squashed 'src/secp256k1/' content from commit 452d8e4d2 (warrows) d98a584 [Refactor] Delete secp256k1 folder for subtreefication (warrows) Tree-SHA512: f0f6777be57777ba86f83af1b891a6c0f384e6b059afc9249599269c71e5d3bf46a6498325488878af71b6685c6dac6cb672d0147c2ebf43b36f6d786fc38a10 * fix keystore refs * update libscepk265k1 * [RPC/REST] Migrate to libevent based httpd server * fix mempooltojson and misc erroros * Merge #769: [Main] Unify shutdown proceedure in init rather than per-app 518542f [Main] Unify shutdown proceedure in init rather than per-app (Fuzzbawls) Tree-SHA512: 6e734ce002e236c3c2c59c2e8e93d87f5fe577b3f5546804b117a71255266cc4012d847d6a1d9b019a7b763124b45987c2dc3f95294670489cac269beceb2e3a * Merge #542: [Network] Remove vfReachable and modify IsReachable to only use vfLimited. 38ed737 Remove vfReachable and modify IsReachable to only use vfLimited. (Patrick Strateman) Tree-SHA512: 8aa61f9abd8356522f3ba71b35497aac07e10333a39ff4b98bb635d1246e6b1be6b0ee0b236944fbfaa558af0e6c0cc219e540eacb2232bc05db393e4a72b088 * [Refactor] Refactor bignum header file into several files * [zerocoin] Free memory from ToString() Simple fix for a minor memory leak * Revert "[Refactor] Refactor bignum header file into several files" This reverts commit c9aa98fe451e27d2bb4564747ffb0768cce56d53. * fix delay in mining blocks * Vitaefy Debian Docs * Fix error when trying to stop mining process (#41) * Give error if pow phase is finished * Force Sync to allow for staking without mnsync * Fix error * Update rpcmisc.cpp * Update rpcmisc.cpp * more ignore * Update fundamentalnode-sync.cpp * Add gitian build python script * Update Univalue * add missing gencpp * Add missing includes * Fix Gitian build Update Univalue add missing gencpp Add missing includes * Revert "Update fundamentalnode-sync.cpp" This reverts commit 323b13fc599b03982b0ca7c3f32342537a6b992f * Fix gitian build and minor updates * remove forcesync * [Model] Invalid iterator position crash fix. * background loading crash fixed. * random: fix crash on some 64bit platforms rbx needs to be stashed in a 64bit register on 64bit platforms. With this crash in particular, it was holding a stack canary which was not properly restored after the cpuid. Split out the x86+PIC case so that x86_64 doesn't have to worry about it. * random: Add fallback if getrandom syscall not available If the code was compiled with newer (>=3.17) kernel headers but executed on a system without the system call, every use of random would crash the program. Add a fallback for that case. * Don't assert if we were beaten to the block A timing window exists where a wallet could be creating a new block from within the miner thread when a new block is received to the wallet. This window will create a situation where TestBlockValidity() fails because the chain tip has changed between the time it created the new block and the time it tested the validity of the block. This situation would result in the wallet being asserted; however this is a little overkill. rather than asserting if the tip has changed, it is better to throw the block away. This problem was revealed during a testnet test of an altcoin, and very prevalent when multiple wallet existed with the exact same number of staking coins received in the same transaction; or when multiple wallets were staking the same coins via import private key. The problem happens significantly less in more normal circumstances, but was still observed in a testing environment with fast blocks. It is likely that this scenario has been encountered but never determined to be root cause, as a crashed wallet could be restarted, re-indexed and never investigated further. * [Wallet] Add some LOCK to avoid crash Github-Pull: #625 Rebased-From: ed23d0d62054156e0c2cb632fecbd65588c50e8c * testnet mining fixes * fix placement of utxo signer * dont sign twice * prevent logging twice * random files updates * Add Chacha20 crypto files * Merge #643: [Crypto] Use stronger rand for key generation b7dda924cfc2405386968b0eaa32aa0e546f322c [Log] Replace a string by the function name in a log (warrows) 977f089d0d2883ca3ca1648b14423b738c9248ef [Refactor] Use arrays instead of unic vars in Chacha20 (warrows) d8abe323bd4f5cff3ca23fc00f0093ab7b143ead [Random] Add a missing include (warrows) 27663b8f16dfaac75bae1ab0e4a0c465b838c826 Do not permit copying FastRandomContexts (Pieter Wuille) 64e03e6de1a65c8d934d8df65af48e2b92bf887a Bugfix: randbytes should seed when needed (non reachable issue) (Pieter Wuille) e8f12aa3621e98588b82533edb56ed4865645862 Check if sys/random.h is required for getentropy on OSX. (James Hilliard) de85c7ae55681f8a9d322f4c48c6e9ba15ee0cf1 Add attribute [[noreturn]] (C++11) to functions that will not return (practicalswift) df46c7ff5a2e6a71ba636fa39d77f58e018816a6 Fix resource leak (Dag Robole) d426d856d23fe6c5d6ef3d57fe2c420b9a8e4e5e Clarify entropy source (Pieter Wuille) 30a320b3d12d3529dfca1daae0e477d76acd9ae7 Use cpuid intrinsics instead of asm code (Pieter Wuille) 0c21204e6dc714de6b917d9de12b1029d0bfb67b random: fix crash on some 64bit platforms (Cory Fields) b8bbb9c68d996b1bea579956b3c91872d6d94b8f Use rdrand as entropy source on supported platforms (Pieter Wuille) 8e19443e41f748ad54d0f8aa25907bdad0815c19 [Tests] Fix compilation (warrows) f53edec73901a91a6c4f595b248e4058e8be5468 [Rand/test] scripted-diff: Use new naming style for insecure_rand* functions (warrows) 272f3a5ac83ba0ab72a799ce524b6605b3555751 [Random / tests] scripted-diff: Use randbits/bool instead of randrange (warrows) 0173ee3332990ac1eea134cca5c21cc451b81d92 Replace rand() & ((1 << N) - 1) with randbits(N) (Pieter Wuille) 250de7426e8317f86fc25fe60e960c2e9e903b69 Replace more rand() % NUM by randranges (Pieter Wuille) d6904136030d94e844074fb75b9cafda46253ce0 [Random / tests] scripted-diff: use insecure_rand256/randrange more (warrows) 4a811ff9542439d43e3df3460114d9a24f849a36 Merge test_random.h into test_bitcoin.h (Pieter Wuille) f275e638bf0a752d9f231f2e4a781432deafbfe8 Add various insecure_rand wrappers for tests (Pieter Wuille) 602af4fb3ef45a2a821f1b76d0dbd418ec852683 Add FastRandomContext::rand256() and ::randbytes() (Pieter Wuille) 90549785824f4c13ee8cf36e77f737b5d89d0f02 Add perf counter data to GetStrongRandBytes state in scheduler (Matt Corallo) 17dd13e746b567f44015fbefaa634b278010fccf Add internal method to add new random data to our internal RNG state (Matt Corallo) c7a1602879aff2ae72bdefe959d4026ee8c0aefc Use sanity check timestamps as entropy (Pieter Wuille) f671fe99994cb220dede903459307c61475aeef8 Test that GetPerformanceCounter() increments (Pieter Wuille) dcb536fb2cef719abef9bb4104242410987bfd3d Use hardware timestamps in RNG seeding (Pieter Wuille) 7c3f290b50d32a3290a2c49542d72a3a7466da58 [Random] Fix compilation (warrows) 22b78957cc90252c958bc66439c098001d47b8fc random: only use getentropy on openbsd (Cory Fields) e5750e5c9831edf01026643deed249e0efb28f0b Add a FastRandomContext::randrange and use it (Pieter Wuille) 2a0f6cd1ca2224e2ba30aea4ab312a1919dedc0c Switch FastRandomContext to ChaCha20 (Pieter Wuille) 401ca7db468939184681b73f4b346aebcfc378c6 Introduce FastRandomContext::randbool() (Pieter Wuille) 3d056d6ed478c5bbd42d7dde4f1ab0a75ffd42b1 Add ChaCha20 (Pieter Wuille) 3c97f3f2cb1b3dc2f5d642877d9a2de0a85c06bf Kill insecure_random and associated global state (Wladimir J. van der Laan) 68ba16c78593bf221844dae95fc5edacbf69ba62 Maintain state across GetStrongRandBytes calls (Pieter Wuille) c3c399e199c2d25d10e782dca2ef17f55b6939d0 random: Add fallback if getrandom syscall not available (Wladimir J. van der Laan) 7a8111f915fc7ed23aaec86cd95904bf5d5972fd sanity: Move OS random to sanity check function (Wladimir J. van der Laan) 387c2e9574b56ae941b7723c561ed39488891782 squashme: comment that NUM_OS_RANDOM_BYTES should not be changed lightly (Wladimir J. van der Laan) 9e8c266b158c26b361d68a3cccdbcfd7bee0b376 util: Specific GetOSRandom for Linux/FreeBSD/OpenBSD (Wladimir J. van der Laan) f989b865f2…

* Updates for stability + Gitian fixes (#43) * Depencies, change to macOS and macOS version bump * Update MacOSX to macOS * Fix typo and unneeded var * Added Make Deploy for Portable Installs * Build System Fix(Gitain) * Give error if pow phase is finished * Force Sync to allow for staking without mnsync * Fix error * Update rpcmisc.cpp * Update rpcmisc.cpp * more ignore * Update fundamentalnode-sync.cpp * Add gitian build python script * Update Univalue * add missing gencpp * Add missing includes * Fix Gitian build Update Univalue add missing gencpp Add missing includes * Revert "Update fundamentalnode-sync.cpp" This reverts commit 323b13fc599b03982b0ca7c3f32342537a6b992f * Fix gitian build and minor updates * remove forcesync * Build System Fix(Gitain) (#31) * Give error if pow phase is finished * Force Sync to allow for staking without mnsync * Fix error * Update rpcmisc.cpp * Update rpcmisc.cpp * more ignore * Update fundamentalnode-sync.cpp * Add gitian build python script * Update Univalue * add missing gencpp * Add missing includes * Fix Gitian build Update Univalue add missing gencpp Add missing includes * Revert "Update fundamentalnode-sync.cpp" This reverts commit 323b13fc599b03982b0ca7c3f32342537a6b992f * Fix gitian build and minor updates * remove forcesync * Updates + crash fixes (#34) * Give error if pow phase is finished * Force Sync to allow for staking without mnsync * Fix error * Update rpcmisc.cpp * Update rpcmisc.cpp * more ignore * Update fundamentalnode-sync.cpp * Add gitian build python script * Update Univalue * add missing gencpp * Add missing includes * Fix Gitian build Update Univalue add missing gencpp Add missing includes * Revert "Update fundamentalnode-sync.cpp" This reverts commit 323b13fc599b03982b0ca7c3f32342537a6b992f * Fix gitian build and minor updates * remove forcesync * [Model] Invalid iterator position crash fix. * background loading crash fixed. * random: fix crash on some 64bit platforms rbx needs to be stashed in a 64bit register on 64bit platforms. With this crash in particular, it was holding a stack canary which was not properly restored after the cpuid. Split out the x86+PIC case so that x86_64 doesn't have to worry about it. * random: Add fallback if getrandom syscall not available If the code was compiled with newer (>=3.17) kernel headers but executed on a system without the system call, every use of random would crash the program. Add a fallback for that case. * Don't assert if we were beaten to the block A timing window exists where a wallet could be creating a new block from within the miner thread when a new block is received to the wallet. This window will create a situation where TestBlockValidity() fails because the chain tip has changed between the time it created the new block and the time it tested the validity of the block. This situation would result in the wallet being asserted; however this is a little overkill. rather than asserting if the tip has changed, it is better to throw the block away. This problem was revealed during a testnet test of an altcoin, and very prevalent when multiple wallet existed with the exact same number of staking coins received in the same transaction; or when multiple wallets were staking the same coins via import private key. The problem happens significantly less in more normal circumstances, but was still observed in a testing environment with fast blocks. It is likely that this scenario has been encountered but never determined to be root cause, as a crashed wallet could be restarted, re-indexed and never investigated further. * [Wallet] Add some LOCK to avoid crash Github-Pull: #625 Rebased-From: ed23d0d62054156e0c2cb632fecbd65588c50e8c * Sync develop with master (#35) * Build System Fix(Gitain) * Give error if pow phase is finished * Force Sync to allow for staking without mnsync * Fix error * Update rpcmisc.cpp * Update rpcmisc.cpp * more ignore * Update fundamentalnode-sync.cpp * Add gitian build python script * Update Univalue * add missing gencpp * Add missing includes * Fix Gitian build Update Univalue add missing gencpp Add missing includes * Revert "Update fundamentalnode-sync.cpp" This reverts commit 323b13fc599b03982b0ca7c3f32342537a6b992f * Fix gitian build and minor updates * remove forcesync * Fix for supply --dirty dirty fix for moneysupply * Revert "Fix for supply --dirty" This reverts commit 8060c6c1f840f477cb7803000e91be8a632aa03f. * Another attempt at syncing master and develop (#36) * Build System Fix(Gitain) * Give error if pow phase is finished * Force Sync to allow for staking without mnsync * Fix error * Update rpcmisc.cpp * Update rpcmisc.cpp * more ignore * Update fundamentalnode-sync.cpp * Add gitian build python script * Update Univalue * add missing gencpp * Add missing includes * Fix Gitian build Update Univalue add missing gencpp Add missing includes * Revert "Update fundamentalnode-sync.cpp" This reverts commit 323b13fc599b03982b0ca7c3f32342537a6b992f * Fix gitian build and minor updates * remove forcesync * Fix for supply --dirty dirty fix for moneysupply * Revert "Fix for supply --dirty" This reverts commit 8060c6c1f840f477cb7803000e91be8a632aa03f. * Mining Fixes (#37) * Give error if pow phase is finished * Force Sync to allow for staking without mnsync * Fix error * Update rpcmisc.cpp * Update rpcmisc.cpp * more ignore * Update fundamentalnode-sync.cpp * Add gitian build python script * Update Univalue * add missing gencpp * Add missing includes * Fix Gitian build Update Univalue add missing gencpp Add missing includes * Revert "Update fundamentalnode-sync.cpp" This reverts commit 323b13fc599b03982b0ca7c3f32342537a6b992f * Fix gitian build and minor updates * remove forcesync * [Model] Invalid iterator position crash fix. * background loading crash fixed. * random: fix crash on some 64bit platforms rbx needs to be stashed in a 64bit register on 64bit platforms. With this crash in particular, it was holding a stack canary which was not properly restored after the cpuid. Split out the x86+PIC case so that x86_64 doesn't have to worry about it. * random: Add fallback if getrandom syscall not available If the code was compiled with newer (>=3.17) kernel headers but executed on a system without the system call, every use of random would crash the program. Add a fallback for that case. * Don't assert if we were beaten to the block A timing window exists where a wallet could be creating a new block from within the miner thread when a new block is received to the wallet. This window will create a situation where TestBlockValidity() fails because the chain tip has changed between the time it created the new block and the time it tested the validity of the block. This situation would result in the wallet being asserted; however this is a little overkill. rather than asserting if the tip has changed, it is better to throw the block away. This problem was revealed during a testnet test of an altcoin, and very prevalent when multiple wallet existed with the exact same number of staking coins received in the same transaction; or when multiple wallets were staking the same coins via import private key. The problem happens significantly less in more normal circumstances, but was still observed in a testing environment with fast blocks. It is likely that this scenario has been encountered but never determined to be root cause, as a crashed wallet could be restarted, re-indexed and never investigated further. * [Wallet] Add some LOCK to avoid crash Github-Pull: #625 Rebased-From: ed23d0d62054156e0c2cb632fecbd65588c50e8c * testnet mining fixes * Update developer-notes.md * Update Copyright * Fix compile error (#38) * Give error if pow phase is finished * Force Sync to allow for staking without mnsync * Fix error * Update rpcmisc.cpp * Update rpcmisc.cpp * more ignore * Update fundamentalnode-sync.cpp * Add gitian build python script * Update Univalue * add missing gencpp * Add missing includes * Fix Gitian build Update Univalue add missing gencpp Add missing includes * Revert "Update fundamentalnode-sync.cpp" This reverts commit 323b13fc599b03982b0ca7c3f32342537a6b992f * Fix gitian build and minor updates * remove forcesync * [Model] Invalid iterator position crash fix. * background loading crash fixed. * random: fix crash on some 64bit platforms rbx needs to be stashed in a 64bit register on 64bit platforms. With this crash in particular, it was holding a stack canary which was not properly restored after the cpuid. Split out the x86+PIC case so that x86_64 doesn't have to worry about it. * random: Add fallback if getrandom syscall not available If the code was compiled with newer (>=3.17) kernel headers but executed on a system without the system call, every use of random would crash the program. Add a fallback for that case. * Don't assert if we were beaten to the block A timing window exists where a wallet could be creating a new block from within the miner thread when a new block is received to the wallet. This window will create a situation where TestBlockValidity() fails because the chain tip has changed between the time it created the new block and the time it tested the validity of the block. This situation would result in the wallet being asserted; however this is a little overkill. rather than asserting if the tip has changed, it is better to throw the block away. This problem was revealed during a testnet test of an altcoin, and very prevalent when multiple wallet existed with the exact same number of staking coins received in the same transaction; or when multiple wallets were staking the same coins via import private key. The problem happens significantly less in more normal circumstances, but was still observed in a testing environment with fast blocks. It is likely that this scenario has been encountered but never determined to be root cause, as a crashed wallet could be restarted, re-indexed and never investigated further. * [Wallet] Add some LOCK to avoid crash Github-Pull: #625 Rebased-From: ed23d0d62054156e0c2cb632fecbd65588c50e8c * testnet mining fixes * fix placement of utxo signer * dont sign twice * prevent logging twice * random files updates * Add Chacha20 crypto files * Merge #643: [Crypto] Use stronger rand for key generation b7dda924cfc2405386968b0eaa32aa0e546f322c [Log] Replace a string by the function name in a log (warrows) 977f089d0d2883ca3ca1648b14423b738c9248ef [Refactor] Use arrays instead of unic vars in Chacha20 (warrows) d8abe323bd4f5cff3ca23fc00f0093ab7b143ead [Random] Add a missing include (warrows) 27663b8f16dfaac75bae1ab0e4a0c465b838c826 Do not permit copying FastRandomContexts (Pieter Wuille) 64e03e6de1a65c8d934d8df65af48e2b92bf887a Bugfix: randbytes should seed when needed (non reachable issue) (Pieter Wuille) e8f12aa3621e98588b82533edb56ed4865645862 Check if sys/random.h is required for getentropy on OSX. (James Hilliard) de85c7ae55681f8a9d322f4c48c6e9ba15ee0cf1 Add attribute [[noreturn]] (C++11) to functions that will not return (practicalswift) df46c7ff5a2e6a71ba636fa39d77f58e018816a6 Fix resource leak (Dag Robole) d426d856d23fe6c5d6ef3d57fe2c420b9a8e4e5e Clarify entropy source (Pieter Wuille) 30a320b3d12d3529dfca1daae0e477d76acd9ae7 Use cpuid intrinsics instead of asm code (Pieter Wuille) 0c21204e6dc714de6b917d9de12b1029d0bfb67b random: fix crash on some 64bit platforms (Cory Fields) b8bbb9c68d996b1bea579956b3c91872d6d94b8f Use rdrand as entropy source on supported platforms (Pieter Wuille) 8e19443e41f748ad54d0f8aa25907bdad0815c19 [Tests] Fix compilation (warrows) f53edec73901a91a6c4f595b248e4058e8be5468 [Rand/test] scripted-diff: Use new naming style for insecure_rand* functions (warrows) 272f3a5ac83ba0ab72a799ce524b6605b3555751 [Random / tests] scripted-diff: Use randbits/bool instead of randrange (warrows) 0173ee3332990ac1eea134cca5c21cc451b81d92 Replace rand() & ((1 << N) - 1) with randbits(N) (Pieter Wuille) 250de7426e8317f86fc25fe60e960c2e9e903b69 Replace more rand() % NUM by randranges (Pieter Wuille) d6904136030d94e844074fb75b9cafda46253ce0 [Random / tests] scripted-diff: use insecure_rand256/randrange more (warrows) 4a811ff9542439d43e3df3460114d9a24f849a36 Merge test_random.h into test_bitcoin.h (Pieter Wuille) f275e638bf0a752d9f231f2e4a781432deafbfe8 Add various insecure_rand wrappers for tests (Pieter Wuille) 602af4fb3ef45a2a821f1b76d0dbd418ec852683 Add FastRandomContext::rand256() and ::randbytes() (Pieter Wuille) 90549785824f4c13ee8cf36e77f737b5d89d0f02 Add perf counter data to GetStrongRandBytes state in scheduler (Matt Corallo) 17dd13e746b567f44015fbefaa634b278010fccf Add internal method to add new random data to our internal RNG state (Matt Corallo) c7a1602879aff2ae72bdefe959d4026ee8c0aefc Use sanity check timestamps as entropy (Pieter Wuille) f671fe99994cb220dede903459307c61475aeef8 Test that GetPerformanceCounter() increments (Pieter Wuille) dcb536fb2cef719abef9bb4104242410987bfd3d Use hardware timestamps in RNG seeding (Pieter Wuille) 7c3f290b50d32a3290a2c49542d72a3a7466da58 [Random] Fix compilation (warrows) 22b78957cc90252c958bc66439c098001d47b8fc random: only use getentropy on openbsd (Cory Fields) e5750e5c9831edf01026643deed249e0efb28f0b Add a FastRandomContext::randrange and use it (Pieter Wuille) 2a0f6cd1ca2224e2ba30aea4ab312a1919dedc0c Switch FastRandomContext to ChaCha20 (Pieter Wuille) 401ca7db468939184681b73f4b346aebcfc378c6 Introduce FastRandomContext::randbool() (Pieter Wuille) 3d056d6ed478c5bbd42d7dde4f1ab0a75ffd42b1 Add ChaCha20 (Pieter Wuille) 3c97f3f2cb1b3dc2f5d642877d9a2de0a85c06bf Kill insecure_random and associated global state (Wladimir J. van der Laan) 68ba16c78593bf221844dae95fc5edacbf69ba62 Maintain state across GetStrongRandBytes calls (Pieter Wuille) c3c399e199c2d25d10e782dca2ef17f55b6939d0 random: Add fallback if getrandom syscall not available (Wladimir J. van der Laan) 7a8111f915fc7ed23aaec86cd95904bf5d5972fd sanity: Move OS random to sanity check function (Wladimir J. van der Laan) 387c2e9574b56ae941b7723c561ed39488891782 squashme: comment that NUM_OS_RANDOM_BYTES should not be changed lightly (Wladimir J. van der Laan) 9e8c266b158c26b361d68a3cccdbcfd7bee0b376 util: Specific GetOSRandom for Linux/FreeBSD/OpenBSD (Wladimir J. van der Laan) f989b865f215c797e77483a04cc7fe9026904517 Don't use assert for catching randomness failures (Pieter Wuille) a15419e13204d96416b0cce9aad2ae4e35ea5ed5 Always require OS randomness when generating secret keys (Pieter Wuille) Pull request description: Since #576 hasn't changed in over a month, here is a reworked version of it. So in this PR: -We add the memory_cleanse function from upstream, to remove a number of OpenSSL calls. -We use OS randomness in addition to OpenSSL randomness (see #576 for why it's needed). ACKs for top commit: random-zebra: ACK https://github.com/PIVX-Project/PIVX/pull/643/commits/b7dda924cfc2405386968b0eaa32aa0e546f322c furszy: ACK [`b7dda92`](https://github.com/PIVX-Project/PIVX/commit/b7dda924cfc2405386968b0eaa32aa0e546f322c) Tree-SHA512: d92cbc14d844263ced753248e646f5cd4f03ec37546f50ff1b558fc3076b6d777c7efcb899c0400bc510e21311b5bd93d9aca26d811033fedb370f8457204035 * Revert "Merge #643: [Crypto] Use stronger rand for key generation" This reverts commit ae3e2562220577420e37a6367feeaa3cfedc67e9. * Revert "Add Chacha20 crypto files" This reverts commit ddd102fca86499062dac0ea3a40a8d8b26d039f5. * Revert "random files updates" This reverts commit f29e0844099487811d4f3c7af7f6d089adc2f6cf. * Revert "random: fix crash on some 64bit platforms" This reverts commit 5a5241452e1515fbd554ee99cc14e552fcb98051. * Revert "random: Add fallback if getrandom syscall not available" This reverts commit b46c1cb2b466f4bce60db364bf26f2cf791cdf38. * fix redeclaration * fix signing code * do not return in void func * remove unused funcs * remove on error * revert "Refactor miner" * Fix name collision + mac crashes (#39) * Give error if pow phase is finished * Force Sync to allow for staking without mnsync * Fix error * Update rpcmisc.cpp * Update rpcmisc.cpp * more ignore * Update fundamentalnode-sync.cpp * Add gitian build python script * Update Univalue * add missing gencpp * Add missing includes * Fix Gitian build Update Univalue add missing gencpp Add missing includes * Revert "Update fundamentalnode-sync.cpp" This reverts commit 323b13fc599b03982b0ca7c3f32342537a6b992f * Fix gitian build and minor updates * remove forcesync * [Model] Invalid iterator position crash fix. * background loading crash fixed. * random: fix crash on some 64bit platforms rbx needs to be stashed in a 64bit register on 64bit platforms. With this crash in particular, it was holding a stack canary which was not properly restored after the cpuid. Split out the x86+PIC case so that x86_64 doesn't have to worry about it. * random: Add fallback if getrandom syscall not available If the code was compiled with newer (>=3.17) kernel headers but executed on a system without the system call, every use of random would crash the program. Add a fallback for that case. * Don't assert if we were beaten to the block A timing window exists where a wallet could be creating a new block from within the miner thread when a new block is received to the wallet. This window will create a situation where TestBlockValidity() fails because the chain tip has changed between the time it created the new block and the time it tested the validity of the block. This situation would result in the wallet being asserted; however this is a little overkill. rather than asserting if the tip has changed, it is better to throw the block away. This problem was revealed during a testnet test of an altcoin, and very prevalent when multiple wallet existed with the exact same number of staking coins received in the same transaction; or when multiple wallets were staking the same coins via import private key. The problem happens significantly less in more normal circumstances, but was still observed in a testing environment with fast blocks. It is likely that this scenario has been encountered but never determined to be root cause, as a crashed wallet could be restarted, re-indexed and never investigated further. * [Wallet] Add some LOCK to avoid crash Github-Pull: #625 Rebased-From: ed23d0d62054156e0c2cb632fecbd65588c50e8c * testnet mining fixes * fix placement of utxo signer * dont sign twice * prevent logging twice * random files updates * Add Chacha20 crypto files * Merge #643: [Crypto] Use stronger rand for key generation b7dda924cfc2405386968b0eaa32aa0e546f322c [Log] Replace a string by the function name in a log (warrows) 977f089d0d2883ca3ca1648b14423b738c9248ef [Refactor] Use arrays instead of unic vars in Chacha20 (warrows) d8abe323bd4f5cff3ca23fc00f0093ab7b143ead [Random] Add a missing include (warrows) 27663b8f16dfaac75bae1ab0e4a0c465b838c826 Do not permit copying FastRandomContexts (Pieter Wuille) 64e03e6de1a65c8d934d8df65af48e2b92bf887a Bugfix: randbytes should seed when needed (non reachable issue) (Pieter Wuille) e8f12aa3621e98588b82533edb56ed4865645862 Check if sys/random.h is required for getentropy on OSX. (James Hilliard) de85c7ae55681f8a9d322f4c48c6e9ba15ee0cf1 Add attribute [[noreturn]] (C++11) to functions that will not return (practicalswift) df46c7ff5a2e6a71ba636fa39d77f58e018816a6 Fix resource leak (Dag Robole) d426d856d23fe6c5d6ef3d57fe2c420b9a8e4e5e Clarify entropy source (Pieter Wuille) 30a320b3d12d3529dfca1daae0e477d76acd9ae7 Use cpuid intrinsics instead of asm code (Pieter Wuille) 0c21204e6dc714de6b917d9de12b1029d0bfb67b random: fix crash on some 64bit platforms (Cory Fields) b8bbb9c68d996b1bea579956b3c91872d6d94b8f Use rdrand as entropy source on supported platforms (Pieter Wuille) 8e19443e41f748ad54d0f8aa25907bdad0815c19 [Tests] Fix compilation (warrows) f53edec73901a91a6c4f595b248e4058e8be5468 [Rand/test] scripted-diff: Use new naming style for insecure_rand* functions (warrows) 272f3a5ac83ba0ab72a799ce524b6605b3555751 [Random / tests] scripted-diff: Use randbits/bool instead of randrange (warrows) 0173ee3332990ac1eea134cca5c21cc451b81d92 Replace rand() & ((1 << N) - 1) with randbits(N) (Pieter Wuille) 250de7426e8317f86fc25fe60e960c2e9e903b69 Replace more rand() % NUM by randranges (Pieter Wuille) d6904136030d94e844074fb75b9cafda46253ce0 [Random / tests] scripted-diff: use insecure_rand256/randrange more (warrows) 4a811ff9542439d43e3df3460114d9a24f849a36 Merge test_random.h into test_bitcoin.h (Pieter Wuille) f275e638bf0a752d9f231f2e4a781432deafbfe8 Add various insecure_rand wrappers for tests (Pieter Wuille) 602af4fb3ef45a2a821f1b76d0dbd418ec852683 Add FastRandomContext::rand256() and ::randbytes() (Pieter Wuille) 90549785824f4c13ee8cf36e77f737b5d89d0f02 Add perf counter data to GetStrongRandBytes state in scheduler (Matt Corallo) 17dd13e746b567f44015fbefaa634b278010fccf Add internal method to add new random data to our internal RNG state (Matt Corallo) c7a1602879aff2ae72bdefe959d4026ee8c0aefc Use sanity check timestamps as entropy (Pieter Wuille) f671fe99994cb220dede903459307c61475aeef8 Test that GetPerformanceCounter() increments (Pieter Wuille) dcb536fb2cef719abef9bb4104242410987bfd3d Use hardware timestamps in RNG seeding (Pieter Wuille) 7c3f290b50d32a3290a2c49542d72a3a7466da58 [Random] Fix compilation (warrows) 22b78957cc90252c958bc66439c098001d47b8fc random: only use getentropy on openbsd (Cory Fields) e5750e5c9831edf01026643deed249e0efb28f0b Add a FastRandomContext::randrange and use it (Pieter Wuille) 2a0f6cd1ca2224e2ba30aea4ab312a1919dedc0c Switch FastRandomContext to ChaCha20 (Pieter Wuille) 401ca7db468939184681b73f4b346aebcfc378c6 Introduce FastRandomContext::randbool() (Pieter Wuille) 3d056d6ed478c5bbd42d7dde4f1ab0a75ffd42b1 Add ChaCha20 (Pieter Wuille) 3c97f3f2cb1b3dc2f5d642877d9a2de0a85c06bf Kill insecure_random and associated global state (Wladimir J. van der Laan) 68ba16c78593bf221844dae95fc5edacbf69ba62 Maintain state across GetStrongRandBytes calls (Pieter Wuille) c3c399e199c2d25d10e782dca2ef17f55b6939d0 random: Add fallback if getrandom syscall not available (Wladimir J. van der Laan) 7a8111f915fc7ed23aaec86cd95904bf5d5972fd sanity: Move OS random to sanity check function (Wladimir J. van der Laan) 387c2e9574b56ae941b7723c561ed39488891782 squashme: comment that NUM_OS_RANDOM_BYTES should not be changed lightly (Wladimir J. van der Laan) 9e8c266b158c26b361d68a3cccdbcfd7bee0b376 util: Specific GetOSRandom for Linux/FreeBSD/OpenBSD (Wladimir J. van der Laan) f989b865f215c797e77483a04cc7fe9026904517 Don't use assert for catching randomness failures (Pieter Wuille) a15419e13204d96416b0cce9aad2ae4e35ea5ed5 Always require OS randomness when generating secret keys (Pieter Wuille) Pull request description: Since #576 hasn't changed in over a month, here is a reworked version of it. So in this PR: -We add the memory_cleanse function from upstream, to remove a number of OpenSSL calls. -We use OS randomness in addition to OpenSSL randomness (see #576 for why it's needed). ACKs for top commit: random-zebra: ACK https://github.com/PIVX-Project/PIVX/pull/643/commits/b7dda924cfc2405386968b0eaa32aa0e546f322c furszy: ACK [`b7dda92`](https://github.com/PIVX-Project/PIVX/commit/b7dda924cfc2405386968b0eaa32aa0e546f322c) Tree-SHA512: d92cbc14d844263ced753248e646f5cd4f03ec37546f50ff1b558fc3076b6d777c7efcb899c0400bc510e21311b5bd93d9aca26d811033fedb370f8457204035 * Revert "Merge #643: [Crypto] Use stronger rand for key generation" This reverts commit ae3e2562220577420e37a6367feeaa3cfedc67e9. * Revert "Add Chacha20 crypto files" This reverts commit ddd102fca86499062dac0ea3a40a8d8b26d039f5. * Revert "random files updates" This reverts commit f29e0844099487811d4f3c7af7f6d089adc2f6cf. * Revert "random: fix crash on some 64bit platforms" This reverts commit 5a5241452e1515fbd554ee99cc14e552fcb98051. * Revert "random: Add fallback if getrandom syscall not available" This reverts commit b46c1cb2b466f4bce60db364bf26f2cf791cdf38. * fix redeclaration * fix signing code * do not return in void func * remove unused funcs * remove on error * revert "Refactor miner" * fix repeat of masternode in fundamental node payment log * fix name collision * [Wallet] Look at last CoinsView block for corruption fix process * Fix crashes * Merge #941: [Refactor] Move ThreadStakeMinter out of net.cpp 42fe4040acff08450168416ccf35413abdc1747a [Refactor] Move ThreadStakeMinter out of net.cpp (Fuzzbawls) Pull request description: This moves the implementation function and thread creation to a more appropriate file (`miner.cpp`). Resolves #938 ACKs for top commit: CaveSpectre11: ACK https://github.com/PIVX-Project/PIVX/commit/42fe4040acff08450168416ccf35413abdc1747a random-zebra: ACK https://github.com/PIVX-Project/PIVX/pull/941/commits/42fe4040acff08450168416ccf35413abdc1747a furszy: utACK [42fe404](https://github.com/PIVX-Project/PIVX/pull/941/commits/42fe4040acff08450168416ccf35413abdc1747a) Tree-SHA512: d3417f03cab63aa41b1ff0fb4d391d42f4448b83efe70391d4014407008af34eb265a23b5807ebecb90cf9637fb74e5b330b02c92e7038e5400a5f16608800ee * Merge #865: [Main] Don't return an invalid state when shutting down the wallet 8126729 Don't return an invalid state when shutting down the wallet (Fuzzbawls) Tree-SHA512: b7042df42af620faca5d415382ad0a6ae973e9dbad7e527957c2ac3c7d0cb5470fcca555b3fe262f5f06e334504ded657db879c2471ecc290247e0f7e2bbf5f3 * Merge #958: [Staking] Modify miner and staking thread for efficiency b7a9a9cdb20cdee869906fb24e2bc984e8a93802 [Staking] Modify miner and staking thread for efficency (Cave Spectre) Pull request description: ### **Release notes** - [Mining] Unnecessary mining threads now exit after PoS has begun - [Staking] The staking thread dormancy is more efficient during PoW - [Performance] Some unnecessary processing in the mining thread removed - [RPC] setgenerate errors if attempted to turn on after end of PoW - [Build] Wallet only routines conditionalized in miner.h - [Refactoring] Log messages with bitcoin named routines changed for easier rename of routines in the future. In order to better explain the changes in this PR, a review of the existing code would be helpful. ### **Backstory** When fixing a PIVX forked coin's transition from PoW to PoS, it was observed that mining threads go into tight infinite loops after the switch to proof of stake. This can be seen with a simple `pivx-qt -testnet -gen`, and watching the debug log. This observation triggered the below code review (re-written for the current release; some of the findings in the code originally reviewed had already been addressed in PIVX). ### **Code Review** _BitcoinMiner()_ This routine is a worker routine for both staking and mining threads; fProofOfStake is set to true when running in the staking thread, false when running in the mining thread. fGeneratedBitcoins is set when mining is started; and cleared on shutdown or when mining is stopped. _In the initial fProofOfStake section:_ ``` if ((GetTime() - nMintableLastCheck > 5 * 60)) // 5 minute check time { nMintableLastCheck = GetTime(); fMintableCoins = pwallet->MintableCoins(); } if (chainActive.Tip()->nHeight < Params().LAST_POW_BLOCK()) { MilliSleep(5000); continue; } ``` Every 5 minutes this code will scan through the wallet and look to see if mintable coins exist. After it's done that, it checks if the network is still in the proof of work phase. It doesn't make sense to scan the wallet until after the proof of work phase is ending. If it is in the proof of work phase; the staking thread sleeps for 5 seconds (`MilliSleep(5000)`). This only makes sense if the block time is 5 seconds. Technically the thread could determine the number of blocks remaining in the proof of work phase, and use that to calculate a hibernation of sorts. Using the target spacing to wait for the block to pass is a 91% reduction in the number of passes through the loop (12 vs. 1 when the target spacing is one minute), and doesn't run into issues where adjustments to the actual spacing throw the timing of a hibernation off. ``` if (!fMintableCoins) { if (GetTime() - nMintableLastCheck > 1 * 60) // 1 minute check time { nMintableLastCheck = GetTime(); fMintableCoins = pwallet->MintableCoins(); } } MilliSleep(5000); ``` We come to this section of code in the waiting while loop; if we don't have mintable coins yet, or our wallet is locked, or we're not synched; or a host of other potential things that would prevent staking. However, we check again for mintable coins; and then we wait 5 seconds after that check before we come out. Since the first time we come into this loop; we have already checked mintable coins (within the last 5 minutes); we should sleep first, and then do the check closer to the time we're actually going to go around the loop again; so we're working with the most recent data. ``` if (!fGenerateBitcoins && !fProofOfStake) continue; ``` This check is buried in a `if (fProofOfStake)` conditional, where fProofOfStake is a parameter that is passed in by the caller to BitcoinMiner(). `!fProofOfStake` will never be true; so this condition will never be true. Removing it we are left with !fGenerateBitcoins; which is unrelated to this section of code anyway. If we're down to this point, we're not in PoW mode anymore, so there shouldn't be a mining thread (more on that later). But the most compelling part of this condition,whether it passes or not, is that it "continues" the while loop it's in; and since it's at the end of the actual while loop, it's going to iterate into another round of the while loop as soon as it finishes with this conditional anyway. Long story short; the code doesn't do anything. _Overall logic_ The mining thread will run until mining is turned off (`setgenerate false`, or the mining flags taken off the invocation). However, there is no need to continue to mine with proof of work after the proof of stake phase begins. In fact, there really is no consideration of that at all; PoW will continue to try to generate blocks well after PoS has begun. Yes, it's within the users control to stop mining; but it's within the power of the code to take care of that for the user, and stop the mining thread(s) after the transition to proof of stake. ### **This PR:** The logical issues above have been corrected. The dormancy for the proof of stake thread is held for just one block rather than any extreme hibernation. Combined with moving that code to be the first thing in the while loop; it also removes the processing done to search through the wallets looking for stakeable coins. Since a significant amount of code assumes there will not be a transition from PoS to PoW; code was added to BitcoinMiner() to exit the mining threads if it's transitioned into the PoS phase. The threads will continue for a little bit, in case there is any rewind on the chain; but after 6 PoS blocks are accepted, the miner threads will exit. To prevent the PoW thread from trying to generate blocks after the PoS phase has begun, logic was added in CreateNewBlockWithKey() to return quickly. Rather than create a situation where the mining thread ends up in a tight loop in the initial phases of PoS, the mining thread will be held for 1/2 of the target spacing before returning. Lastly; Some tweaks were made to the logging; removing hardcoded references to "BitcoinMiner()", in case refactoring in the future changes the name of the routine. Similarly, since there is a log message when the mining thread starts "PIVXMiner started"; rather than reporting thread exits or errors as "ThreadBitcoinMiner", they have been changed to PIVXMiner to match. Much of this can be observed with an errant `pivx-qt -gen`. With this PR, it simply tries to start the miner, and then exits. _With PR code - `pivx-qt -gen -testnet`_ ``` 2019-07-21 20:04:22 PIVXMiner started 2019-07-21 20:04:22 BitcoinMiner: Exiting Proof of Work Mining Thread at height: 1160732 2019-07-21 20:04:22 PIVXMiner exiting ``` _Current release:_ ``` 2019-07-21 19:37:06 CreateNewBlock(): total size 1000 2019-07-21 19:37:06 ERROR: CheckProofOfWork() : hash doesn't match nBits 2019-07-21 19:37:06 ERROR: CheckBlockHeader() : proof of work failed 2019-07-21 19:37:06 ERROR: CheckBlock() : CheckBlockHeader failed 2019-07-21 19:37:06 CreateNewBlock() : TestBlockValidity failed ``` is repeated in a tight loop. ACKs for top commit: furszy: ACK [b7a9a9c](https://github.com/PIVX-Project/PIVX/commit/b7a9a9cdb20cdee869906fb24e2bc984e8a93802) random-zebra: ACK https://github.com/PIVX-Project/PIVX/pull/958/commits/b7a9a9cdb20cdee869906fb24e2bc984e8a93802 and merging... Tree-SHA512: 064619ce1fde75f1baf35ddb202b4573f2a8c0effc5d2939a678612e3f1d56b08d5c5d7c7bb30ce919e5a648f3f406cca35093b9a3a04c799f746fff5c6a35b6 * remove duplicate stake thread code * add missing include * remove zvitae pos validator * Refine signblock code * revert zerocoin added check * Miner reference updates * revert miner changes * fix fundamentalnode sync checker * [Refactor] Move rpc files to its own folder * update includes * fix rpcserver include to rpc/server.h * [Refactor] Move wallet files to wallet dir * [Trivial] Fix db.h path * Merge #982: [Miner] Don't create new keys when generating PoS blocks 74e7fc7d432c7f6d586d54ff276d6babad0fe505 [Miner] Don't create new keys when generating PoS blocks (random-zebra) Pull request description: This fixes the calls to `CreateNewBlockWithKey` with PoS blocks, which are not needed. We simply call `CreateNewBlock` with an empy script for coinbase. It should prevent the problem described in https://github.com/PIVX-Project/PIVX/pull/956#issuecomment-520166966 ACKs for top commit: CaveSpectre11: utACK https://github.com/PIVX-Project/PIVX/pull/982/commits/74e7fc7d432c7f6d586d54ff276d6babad0fe505 akshaynexus: utACK https://github.com/PIVX-Project/PIVX/commit/74e7fc7d432c7f6d586d54ff276d6babad0fe505 furszy: utACK [74e7fc7](https://github.com/PIVX-Project/PIVX/pull/982/commits/74e7fc7d432c7f6d586d54ff276d6babad0fe505) Warrows: Pretty straightforward indeed, utACK 74e7fc7 Tree-SHA512: bcb9752a689a058b6c33317136d16afd694b5da66c69cdd32a8b89b71059c5b2c5dc20c88045c52429fae99d45f508ea223f1b7d1afa5db3c77eb59befd48d51 * Merge #549: [Crypto] Switch to libsecp256k1 signature verification and update the lib f10439c [Crypto] Add ctx initialisation for bip38 (warrows) 21234db [Crypto] Bring back function CKey.SetPrivKey for zPIV (warrows) 65e009a [Tests] Add new auto generated script tests (warrows) 6839f3b [Crypto] Switch from openssl to secp256k1 for consensus (warrows) 8a901f9 Squashed 'src/secp256k1/' content from commit 452d8e4d2 (warrows) d98a584 [Refactor] Delete secp256k1 folder for subtreefication (warrows) Tree-SHA512: f0f6777be57777ba86f83af1b891a6c0f384e6b059afc9249599269c71e5d3bf46a6498325488878af71b6685c6dac6cb672d0147c2ebf43b36f6d786fc38a10 * fix keystore refs * update libscepk265k1 * [RPC/REST] Migrate to libevent based httpd server * fix mempooltojson and misc erroros * Merge #769: [Main] Unify shutdown proceedure in init rather than per-app 518542f [Main] Unify shutdown proceedure in init rather than per-app (Fuzzbawls) Tree-SHA512: 6e734ce002e236c3c2c59c2e8e93d87f5fe577b3f5546804b117a71255266cc4012d847d6a1d9b019a7b763124b45987c2dc3f95294670489cac269beceb2e3a * Merge #542: [Network] Remove vfReachable and modify IsReachable to only use vfLimited. 38ed737 Remove vfReachable and modify IsReachable to only use vfLimited. (Patrick Strateman) Tree-SHA512: 8aa61f9abd8356522f3ba71b35497aac07e10333a39ff4b98bb635d1246e6b1be6b0ee0b236944fbfaa558af0e6c0cc219e540eacb2232bc05db393e4a72b088 * [Refactor] Refactor bignum header file into several files * [zerocoin] Free memory from ToString() Simple fix for a minor memory leak * Revert "[Refactor] Refactor bignum header file into several files" This reverts commit c9aa98fe451e27d2bb4564747ffb0768cce56d53. * fix delays in mining code in testnet (#40) * Give error if pow phase is finished * Force Sync to allow for staking without mnsync * Fix error * Update rpcmisc.cpp * Update rpcmisc.cpp * more ignore * Update fundamentalnode-sync.cpp * Add gitian build python script * Update Univalue * add missing gencpp * Add missing includes * Fix Gitian build Update Univalue add missing gencpp Add missing includes * Revert "Update fundamentalnode-sync.cpp" This reverts commit 323b13fc599b03982b0ca7c3f32342537a6b992f * Fix gitian build and minor updates * remove forcesync * [Model] Invalid iterator position crash fix. * background loading crash fixed. * random: fix crash on some 64bit platforms rbx needs to be stashed in a 64bit register on 64bit platforms. With this crash in particular, it was holding a stack canary which was not properly restored after the cpuid. Split out the x86+PIC case so that x86_64 doesn't have to worry about it. * random: Add fallback if getrandom syscall not available If the code was compiled with newer (>=3.17) kernel headers but executed on a system without the system call, every use of random would crash the program. Add a fallback for that case. * Don't assert if we were beaten to the block A timing window exists where a wallet could be creating a new block from within the miner thread when a new block is received to the wallet. This window will create a situation where TestBlockValidity() fails because the chain tip has changed between the time it created the new block and the time it tested the validity of the block. This situation would result in the wallet being asserted; however this is a little overkill. rather than asserting if the tip has changed, it is better to throw the block away. This problem was revealed during a testnet test of an altcoin, and very prevalent when multiple wallet existed with the exact same number of staking coins received in the same transaction; or when multiple wallets were staking the same coins via import private key. The problem happens significantly less in more normal circumstances, but was still observed in a testing environment with fast blocks. It is likely that this scenario has been encountered but never determined to be root cause, as a crashed wallet could be restarted, re-indexed and never investigated further. * [Wallet] Add some LOCK to avoid crash Github-Pull: #625 Rebased-From: ed23d0d62054156e0c2cb632fecbd65588c50e8c * testnet mining fixes * fix placement of utxo signer * dont sign twice * prevent logging twice * random files updates * Add Chacha20 crypto files * Merge #643: [Crypto] Use stronger rand for key generation b7dda924cfc2405386968b0eaa32aa0e546f322c [Log] Replace a string by the function name in a log (warrows) 977f089d0d2883ca3ca1648b14423b738c9248ef [Refactor] Use arrays instead of unic vars in Chacha20 (warrows) d8abe323bd4f5cff3ca23fc00f0093ab7b143ead [Random] Add a missing include (warrows) 27663b8f16dfaac75bae1ab0e4a0c465b838c826 Do not permit copying FastRandomContexts (Pieter Wuille) 64e03e6de1a65c8d934d8df65af48e2b92bf887a Bugfix: randbytes should seed when needed (non reachable issue) (Pieter Wuille) e8f12aa3621e98588b82533edb56ed4865645862 Check if sys/random.h is required for getentropy on OSX. (James Hilliard) de85c7ae55681f8a9d322f4c48c6e9ba15ee0cf1 Add attribute [[noreturn]] (C++11) to functions that will not return (practicalswift) df46c7ff5a2e6a71ba636fa39d77f58e018816a6 Fix resource leak (Dag Robole) d426d856d23fe6c5d6ef3d57fe2c420b9a8e4e5e Clarify entropy source (Pieter Wuille) 30a320b3d12d3529dfca1daae0e477d76acd9ae7 Use cpuid intrinsics instead of asm code (Pieter Wuille) 0c21204e6dc714de6b917d9de12b1029d0bfb67b random: fix crash on some 64bit platforms (Cory Fields) b8bbb9c68d996b1bea579956b3c91872d6d94b8f Use rdrand as entropy source on supported platforms (Pieter Wuille) 8e19443e41f748ad54d0f8aa25907bdad0815c19 [Tests] Fix compilation (warrows) f53edec73901a91a6c4f595b248e4058e8be5468 [Rand/test] scripted-diff: Use new naming style for insecure_rand* functions (warrows) 272f3a5ac83ba0ab72a799ce524b6605b3555751 [Random / tests] scripted-diff: Use randbits/bool instead of randrange (warrows) 0173ee3332990ac1eea134cca5c21cc451b81d92 Replace rand() & ((1 << N) - 1) with randbits(N) (Pieter Wuille) 250de7426e8317f86fc25fe60e960c2e9e903b69 Replace more rand() % NUM by randranges (Pieter Wuille) d6904136030d94e844074fb75b9cafda46253ce0 [Random / tests] scripted-diff: use insecure_rand256/randrange more (warrows) 4a811ff9542439d43e3df3460114d9a24f849a36 Merge test_random.h into test_bitcoin.h (Pieter Wuille) f275e638bf0a752d9f231f2e4a781432deafbfe8 Add various insecure_rand wrappers for tests (Pieter Wuille) 602af4fb3ef45a2a821f1b76d0dbd418ec852683 Add FastRandomContext::rand256() and ::randbytes() (Pieter Wuille) 90549785824f4c13ee8cf36e77f737b5d89d0f02 Add perf counter data to GetStrongRandBytes state in scheduler (Matt Corallo) 17dd13e746b567f44015fbefaa634b278010fccf Add internal method to add new random data to our internal RNG state (Matt Corallo) c7a1602879aff2ae72bdefe959d4026ee8c0aefc Use sanity check timestamps as entropy (Pieter Wuille) f671fe99994cb220dede903459307c61475aeef8 Test that GetPerformanceCounter() increments (Pieter Wuille) dcb536fb2cef719abef9bb4104242410987bfd3d Use hardware timestamps in RNG seeding (Pieter Wuille) 7c3f290b50d32a3290a2c49542d72a3a7466da58 [Random] Fix compilation (warrows) 22b78957cc90252c958bc66439c098001d47b8fc random: only use getentropy on openbsd (Cory Fields) e5750e5c9831edf01026643deed249e0efb28f0b Add a FastRandomContext::randrange and use it (Pieter Wuille) 2a0f6cd1ca2224e2ba30aea4ab312a1919dedc0c Switch FastRandomContext to ChaCha20 (Pieter Wuille) 401ca7db468939184681b73f4b346aebcfc378c6 Introduce FastRandomContext::randbool() (Pieter Wuille) 3d056d6ed478c5bbd42d7dde4f1ab0a75ffd42b1 Add ChaCha20 (Pieter Wuille) 3c97f3f2cb1b3dc2f5d642877d9a2de0a85c06bf Kill insecure_random and associated global state (Wladimir J. van der Laan) 68ba16c78593bf221844dae95fc5edacbf69ba62 Maintain state across GetStrongRandBytes calls (Pieter Wuille) c3c399e199c2d25d10e782dca2ef17f55b6939d0 random: Add fallback if getrandom syscall not available (Wladimir J. van der Laan) 7a8111f915fc7ed23aaec86cd95904bf5d5972fd sanity: Move OS random to sanity check function (Wladimir J. van der Laan) 387c2e9574b56ae941b7723c561ed39488891782 squashme: comment that NUM_OS_RANDOM_BYTES should not be changed lightly (Wladimir J. van der Laan) 9e8c266b158c26b361d68a3cccdbcfd7bee0b376 util: Specific GetOSRandom for Linux/FreeBSD/OpenBSD (Wladimir J. van der Laan) f989b865f215c797e77483a04cc7fe9026904517 Don't use assert for catching randomness failures (Pieter Wuille) a15419e13204d96416b0cce9aad2ae4e35ea5ed5 Always require OS randomness when generating secret keys (Pieter Wuille) Pull request description: Since #576 hasn't changed in over a month, here is a reworked version of it. So in this PR: -We add the memory_cleanse function from upstream, to remove a number of OpenSSL calls. -We use OS randomness in addition to OpenSSL randomness (see #576 for why it's needed). ACKs for top commit: random-zebra: ACK https://github.com/PIVX-Project/PIVX/pull/643/commits/b7dda924cfc2405386968b0eaa32aa0e546f322c furszy: ACK [`b7dda92`](https://github.com/PIVX-Project/PIVX/commit/b7dda924cfc2405386968b0eaa32aa0e546f322c) Tree-SHA512: d92cbc14d844263ced753248e646f5cd4f03ec37546f50ff1b558fc3076b6d777c7efcb899c0400bc510e21311b5bd93d9aca26d811033fedb370f8457204035 * Revert "Merge #643: [Crypto] Use stronger rand for key generation" This reverts commit ae3e2562220577420e37a6367feeaa3cfedc67e9. * Revert "Add Chacha20 crypto files" This reverts commit ddd102fca86499062dac0ea3a40a8d8b26d039f5. * Revert "random files updates" This reverts commit f29e0844099487811d4f3c7af7f6d089adc2f6cf. * Revert "random: fix crash on some 64bit platforms" This reverts commit 5a5241452e1515fbd554ee99cc14e552fcb98051. * Revert "random: Add fallback if getrandom syscall not available" This reverts commit b46c1cb2b466f4bce60db364bf26f2cf791cdf38. * fix redeclaration * fix signing code * do not return in void func * remove unused funcs * remove on error * revert "Refactor miner" * fix repeat of masternode in fundamental node payment log * fix name collision * [Wallet] Look at last CoinsView block for corruption fix process * Fix crashes * Merge #941: [Refactor] Move ThreadStakeMinter out of net.cpp 42fe4040acff08450168416ccf35413abdc1747a [Refactor] Move ThreadStakeMinter out of net.cpp (Fuzzbawls) Pull request description: This moves the implementation function and thread creation to a more appropriate file (`miner.cpp`). Resolves #938 ACKs for top commit: CaveSpectre11: ACK https://github.com/PIVX-Project/PIVX/commit/42fe4040acff08450168416ccf35413abdc1747a random-zebra: ACK https://github.com/PIVX-Project/PIVX/pull/941/commits/42fe4040acff08450168416ccf35413abdc1747a furszy: utACK [42fe404](https://github.com/PIVX-Project/PIVX/pull/941/commits/42fe4040acff08450168416ccf35413abdc1747a) Tree-SHA512: d3417f03cab63aa41b1ff0fb4d391d42f4448b83efe70391d4014407008af34eb265a23b5807ebecb90cf9637fb74e5b330b02c92e7038e5400a5f16608800ee * Merge #865: [Main] Don't return an invalid state when shutting down the wallet 8126729 Don't return an invalid state when shutting down the wallet (Fuzzbawls) Tree-SHA512: b7042df42af620faca5d415382ad0a6ae973e9dbad7e527957c2ac3c7d0cb5470fcca555b3fe262f5f06e334504ded657db879c2471ecc290247e0f7e2bbf5f3 * Merge #958: [Staking] Modify miner and staking thread for efficiency b7a9a9cdb20cdee869906fb24e2bc984e8a93802 [Staking] Modify miner and staking thread for efficency (Cave Spectre) Pull request description: ### **Release notes** - [Mining] Unnecessary mining threads now exit after PoS has begun - [Staking] The staking thread dormancy is more efficient during PoW - [Performance] Some unnecessary processing in the mining thread removed - [RPC] setgenerate errors if attempted to turn on after end of PoW - [Build] Wallet only routines conditionalized in miner.h - [Refactoring] Log messages with bitcoin named routines changed for easier rename of routines in the future. In order to better explain the changes in this PR, a review of the existing code would be helpful. ### **Backstory** When fixing a PIVX forked coin's transition from PoW to PoS, it was observed that mining threads go into tight infinite loops after the switch to proof of stake. This can be seen with a simple `pivx-qt -testnet -gen`, and watching the debug log. This observation triggered the below code review (re-written for the current release; some of the findings in the code originally reviewed had already been addressed in PIVX). ### **Code Review** _BitcoinMiner()_ This routine is a worker routine for both staking and mining threads; fProofOfStake is set to true when running in the staking thread, false when running in the mining thread. fGeneratedBitcoins is set when mining is started; and cleared on shutdown or when mining is stopped. _In the initial fProofOfStake section:_ ``` if ((GetTime() - nMintableLastCheck > 5 * 60)) // 5 minute check time { nMintableLastCheck = GetTime(); fMintableCoins = pwallet->MintableCoins(); } if (chainActive.Tip()->nHeight < Params().LAST_POW_BLOCK()) { MilliSleep(5000); continue; } ``` Every 5 minutes this code will scan through the wallet and look to see if mintable coins exist. After it's done that, it checks if the network is still in the proof of work phase. It doesn't make sense to scan the wallet until after the proof of work phase is ending. If it is in the proof of work phase; the staking thread sleeps for 5 seconds (`MilliSleep(5000)`). This only makes sense if the block time is 5 seconds. Technically the thread could determine the number of blocks remaining in the proof of work phase, and use that to calculate a hibernation of sorts. Using the target spacing to wait for the block to pass is a 91% reduction in the number of passes through the loop (12 vs. 1 when the target spacing is one minute), and doesn't run into issues where adjustments to the actual spacing throw the timing of a hibernation off. ``` if (!fMintableCoins) { if (GetTime() - nMintableLastCheck > 1 * 60) // 1 minute check time { nMintableLastCheck = GetTime(); fMintableCoins = pwallet->MintableCoins(); } } MilliSleep(5000); ``` We come to this section of code in the waiting while loop; if we don't have mintable coins yet, or our wallet is locked, or we're not synched; or a host of other potential things that would prevent staking. However, we check again for mintable coins; and then we wait 5 seconds after that check before we come out. Since the first time we come into this loop; we have already checked mintable coins (within the last 5 minutes); we should sleep first, and then do the check closer to the time we're actually going to go around the loop again; so we're working with the most recent data. ``` if (!fGenerateBitcoins && !fProofOfStake) continue; ``` This check is buried in a `if (fProofOfStake)` conditional, where fProofOfStake is a parameter that is passed in by the caller to BitcoinMiner(). `!fProofOfStake` will never be true; so this condition will never be true. Removing it we are left with !fGenerateBitcoins; which is unrelated to this section of code anyway. If we're down to this point, we're not in PoW mode anymore, so there shouldn't be a mining thread (more on that later). But the most compelling part of this condition,whether it passes or not, is that it "continues" the while loop it's in; and since it's at the end of the actual while loop, it's going to iterate into another round of the while loop as soon as it finishes with this conditional anyway. Long story short; the code doesn't do anything. _Overall logic_ The mining thread will run until mining is turned off (`setgenerate false`, or the mining flags taken off the invocation). However, there is no need to continue to mine with proof of work after the proof of stake phase begins. In fact, there really is no consideration of that at all; PoW will continue to try to generate blocks well after PoS has begun. Yes, it's within the users control to stop mining; but it's within the power of the code to take care of that for the user, and stop the mining thread(s) after the transition to proof of stake. ### **This PR:** The logical issues above have been corrected. The dormancy for the proof of stake thread is held for just one block rather than any extreme hibernation. Combined with moving that code to be the first thing in the while loop; it also removes the processing done to search through the wallets looking for stakeable coins. Since a significant amount of code assumes there will not be a transition from PoS to PoW; code was added to BitcoinMiner() to exit the mining threads if it's transitioned into the PoS phase. The threads will continue for a little bit, in case there is any rewind on the chain; but after 6 PoS blocks are accepted, the miner threads will exit. To prevent the PoW thread from trying to generate blocks after the PoS phase has begun, logic was added in CreateNewBlockWithKey() to return quickly. Rather than create a situation where the mining thread ends up in a tight loop in the initial phases of PoS, the mining thread will be held for 1/2 of the target spacing before returning. Lastly; Some tweaks were made to the logging; removing hardcoded references to "BitcoinMiner()", in case refactoring in the future changes the name of the routine. Similarly, since there is a log message when the mining thread starts "PIVXMiner started"; rather than reporting thread exits or errors as "ThreadBitcoinMiner", they have been changed to PIVXMiner to match. Much of this can be observed with an errant `pivx-qt -gen`. With this PR, it simply tries to start the miner, and then exits. _With PR code - `pivx-qt -gen -testnet`_ ``` 2019-07-21 20:04:22 PIVXMiner started 2019-07-21 20:04:22 BitcoinMiner: Exiting Proof of Work Mining Thread at height: 1160732 2019-07-21 20:04:22 PIVXMiner exiting ``` _Current release:_ ``` 2019-07-21 19:37:06 CreateNewBlock(): total size 1000 2019-07-21 19:37:06 ERROR: CheckProofOfWork() : hash doesn't match nBits 2019-07-21 19:37:06 ERROR: CheckBlockHeader() : proof of work failed 2019-07-21 19:37:06 ERROR: CheckBlock() : CheckBlockHeader failed 2019-07-21 19:37:06 CreateNewBlock() : TestBlockValidity failed ``` is repeated in a tight loop. ACKs for top commit: furszy: ACK [b7a9a9c](https://github.com/PIVX-Project/PIVX/commit/b7a9a9cdb20cdee869906fb24e2bc984e8a93802) random-zebra: ACK https://github.com/PIVX-Project/PIVX/pull/958/commits/b7a9a9cdb20cdee869906fb24e2bc984e8a93802 and merging... Tree-SHA512: 064619ce1fde75f1baf35ddb202b4573f2a8c0effc5d2939a678612e3f1d56b08d5c5d7c7bb30ce919e5a648f3f406cca35093b9a3a04c799f746fff5c6a35b6 * remove duplicate stake thread code * add missing include * remove zvitae pos validator * Refine signblock code * revert zerocoin added check * Miner reference updates * revert miner changes * fix fundamentalnode sync checker * [Refactor] Move rpc files to its own folder * update includes * fix rpcserver include to rpc/server.h * [Refactor] Move wallet files to wallet dir * [Trivial] Fix db.h path * Merge #982: [Miner] Don't create new keys when generating PoS blocks 74e7fc7d432c7f6d586d54ff276d6babad0fe505 [Miner] Don't create new keys when generating PoS blocks (random-zebra) Pull request description: This fixes the calls to `CreateNewBlockWithKey` with PoS blocks, which are not needed. We simply call `CreateNewBlock` with an empy script for coinbase. It should prevent the problem described in https://github.com/PIVX-Project/PIVX/pull/956#issuecomment-520166966 ACKs for top commit: CaveSpectre11: utACK https://github.com/PIVX-Project/PIVX/pull/982/commits/74e7fc7d432c7f6d586d54ff276d6babad0fe505 akshaynexus: utACK https://github.com/PIVX-Project/PIVX/commit/74e7fc7d432c7f6d586d54ff276d6babad0fe505 furszy: utACK [74e7fc7](https://github.com/PIVX-Project/PIVX/pull/982/commits/74e7fc7d432c7f6d586d54ff276d6babad0fe505) Warrows: Pretty straightforward indeed, utACK 74e7fc7 Tree-SHA512: bcb9752a689a058b6c33317136d16afd694b5da66c69cdd32a8b89b71059c5b2c5dc20c88045c52429fae99d45f508ea223f1b7d1afa5db3c77eb59befd48d51 * Merge #549: [Crypto] Switch to libsecp256k1 signature verification and update the lib f10439c [Crypto] Add ctx initialisation for bip38 (warrows) 21234db [Crypto] Bring back function CKey.SetPrivKey for zPIV (warrows) 65e009a [Tests] Add new auto generated script tests (warrows) 6839f3b [Crypto] Switch from openssl to secp256k1 for consensus (warrows) 8a901f9 Squashed 'src/secp256k1/' content from commit 452d8e4d2 (warrows) d98a584 [Refactor] Delete secp256k1 folder for subtreefication (warrows) Tree-SHA512: f0f6777be57777ba86f83af1b891a6c0f384e6b059afc9249599269c71e5d3bf46a6498325488878af71b6685c6dac6cb672d0147c2ebf43b36f6d786fc38a10 * fix keystore refs * update libscepk265k1 * [RPC/REST] Migrate to libevent based httpd server * fix mempooltojson and misc erroros * Merge #769: [Main] Unify shutdown proceedure in init rather than per-app 518542f [Main] Unify shutdown proceedure in init rather than per-app (Fuzzbawls) Tree-SHA512: 6e734ce002e236c3c2c59c2e8e93d87f5fe577b3f5546804b117a71255266cc4012d847d6a1d9b019a7b763124b45987c2dc3f95294670489cac269beceb2e3a * Merge #542: [Network] Remove vfReachable and modify IsReachable to only use vfLimited. 38ed737 Remove vfReachable and modify IsReachable to only use vfLimited. (Patrick Strateman) Tree-SHA512: 8aa61f9abd8356522f3ba71b35497aac07e10333a39ff4b98bb635d1246e6b1be6b0ee0b236944fbfaa558af0e6c0cc219e540eacb2232bc05db393e4a72b088 * [Refactor] Refactor bignum header file into several files * [zerocoin] Free memory from ToString() Simple fix for a minor memory leak * Revert "[Refactor] Refactor bignum header file into several files" This reverts commit c9aa98fe451e27d2bb4564747ffb0768cce56d53. * fix delay in mining blocks * Vitaefy Debian Docs * Fix error when trying to stop mining process (#41) * Give error if pow phase is finished * Force Sync to allow for staking without mnsync * Fix error * Update rpcmisc.cpp * Update rpcmisc.cpp * more ignore * Update fundamentalnode-sync.cpp * Add gitian build python script * Update Univalue * add missing gencpp * Add missing includes * Fix Gitian build Update Univalue add missing gencpp Add missing includes * Revert "Update fundamentalnode-sync.cpp" This reverts commit 323b13fc599b03982b0ca7c3f32342537a6b992f * Fix gitian build and minor updates * remove forcesync * [Model] Invalid iterator position crash fix. * background loading crash fixed. * random: fix crash on some 64bit platforms rbx needs to be stashed in a 64bit register on 64bit platforms. With this crash in particular, it was holding a stack canary which was not properly restored after the cpuid. Split out the x86+PIC case so that x86_64 doesn't have to worry about it. * random: Add fallback if getrandom syscall not available If the code was compiled with newer (>=3.17) kernel headers but executed on a system without the system call, every use of random would crash the program. Add a fallback for that case. * Don't assert if we were beaten to the block A timing window exists where a wallet could be creating a new block from within the miner thread when a new block is received to the wallet. This window will create a situation where TestBlockValidity() fails because the chain tip has changed between the time it created the new block and the time it tested the validity of the block. This situation would result in the wallet being asserted; however this is a little overkill. rather than asserting if the tip has changed, it is better to throw the block away. This problem was revealed during a testnet test of an altcoin, and very prevalent when multiple wallet existed with the exact same number of staking coins received in the same transaction; or when multiple wallets were staking the same coins via import private key. The problem happens significantly less in more normal circumstances, but was still observed in a testing environment with fast blocks. It is likely that this scenario has been encountered but never determined to be root cause, as a crashed wallet could be restarted, re-indexed and never investigated further. * [Wallet] Add some LOCK to avoid crash Github-Pull: #625 Rebased-From: ed23d0d62054156e0c2cb632fecbd65588c50e8c * testnet mining fixes * fix placement of utxo signer * dont sign twice * prevent logging twice * random files updates * Add Chacha20 crypto files * Merge #643: [Crypto] Use stronger rand for key generation b7dda924cfc2405386968b0eaa32aa0e546f322c [Log] Replace a string by the function name in a log (warrows) 977f089d0d2883ca3ca1648b14423b738c9248ef [Refactor] Use arrays instead of unic vars in Chacha20 (warrows) d8abe323bd4f5cff3ca23fc00f0093ab7b143ead [Random] Add a missing include (warrows) 27663b8f16dfaac75bae1ab0e4a0c465b838c826 Do not permit copying FastRandomContexts (Pieter Wuille) 64e03e6de1a65c8d934d8df65af48e2b92bf887a Bugfix: randbytes should seed when needed (non reachable issue) (Pieter Wuille) e8f12aa3621e98588b82533edb56ed4865645862 Check if sys/random.h is required for getentropy on OSX. (James Hilliard) de85c7ae55681f8a9d322f4c48c6e9ba15ee0cf1 Add attribute [[noreturn]] (C++11) to functions that will not return (practicalswift) df46c7ff5a2e6a71ba636fa39d77f58e018816a6 Fix resource leak (Dag Robole) d426d856d23fe6c5d6ef3d57fe2c420b9a8e4e5e Clarify entropy source (Pieter Wuille) 30a320b3d12d3529dfca1daae0e477d76acd9ae7 Use cpuid intrinsics instead of asm code (Pieter Wuille) 0c21204e6dc714de6b917d9de12b1029d0bfb67b random: fix crash on some 64bit platforms (Cory Fields) b8bbb9c68d996b1bea579956b3c91872d6d94b8f Use rdrand as entropy source on supported platforms (Pieter Wuille) 8e19443e41f748ad54d0f8aa25907bdad0815c19 [Tests] Fix compilation (warrows) f53edec73901a91a6c4f595b248e4058e8be5468 [Rand/test] scripted-diff: Use new naming style for insecure_rand* functions (warrows) 272f3a5ac83ba0ab72a799ce524b6605b3555751 [Random / tests] scripted-diff: Use randbits/bool instead of randrange (warrows) 0173ee3332990ac1eea134cca5c21cc451b81d92 Replace rand() & ((1 << N) - 1) with randbits(N) (Pieter Wuille) 250de7426e8317f86fc25fe60e960c2e9e903b69 Replace more rand() % NUM by randranges (Pieter Wuille) d6904136030d94e844074fb75b9cafda46253ce0 [Random / tests] scripted-diff: use insecure_rand256/randrange more (warrows) 4a811ff9542439d43e3df3460114d9a24f849a36 Merge test_random.h into test_bitcoin.h (Pieter Wuille) f275e638bf0a752d9f231f2e4a781432deafbfe8 Add various insecure_rand wrappers for tests (Pieter Wuille) 602af4fb3ef45a2a821f1b76d0dbd418ec852683 Add FastRandomContext::rand256() and ::randbytes() (Pieter Wuille) 90549785824f4c13ee8cf36e77f737b5d89d0f02 Add perf counter data to GetStrongRandBytes state in scheduler (Matt Corallo) 17dd13e746b567f44015fbefaa634b278010fccf Add internal method to add new random data to our internal RNG state (Matt Corallo) c7a1602879aff2ae72bdefe959d4026ee8c0aefc Use sanity check timestamps as entropy (Pieter Wuille) f671fe99994cb220dede903459307c61475aeef8 Test that GetPerformanceCounter() increments (Pieter Wuille) dcb536fb2cef719abef9bb4104242410987bfd3d Use hardware timestamps in RNG seeding (Pieter Wuille) 7c3f290b50d32a3290a2c49542d72a3a7466da58 [Random] Fix compilation (warrows) 22b78957cc90252c958bc66439c098001d47b8fc random: only use getentropy on openbsd (Cory Fields) e5750e5c9831edf01026643deed249e0efb28f0b Add a FastRandomContext::randrange and use it (Pieter Wuille) 2a0f6cd1ca2224e2ba30aea4ab312a1919dedc0c Switch FastRandomContext to ChaCha20 (Pieter Wuille) 401ca7db468939184681b73f4b346aebcfc378c6 Introduce FastRandomContext::randbool() (Pieter Wuille) 3d056d6ed478c5bbd42d7dde4f1ab0a75ffd42b1 Add ChaCha20 (Pieter Wuille) 3c97f3f2cb1b3dc2f5d642877d9a2de0a85c06bf Kill insecure_random and associated global state (Wladimir J. van der Laan) 68ba16c78593bf221844dae95fc5edacbf69ba62 Maintain state across GetStrongRandBytes calls (Pieter Wuille) c3c399e199c2d25d10e782dca2ef17f55b6939d0 random: Add fallback if getrandom syscall not available (Wladimir J. van der Laan) 7a8111f915fc7ed23aaec86cd95904bf5d5972fd sanity: Move OS random to sanity check function (Wladimir J. van der Laan) 387c2e9574b56ae941b7723c561ed39488891782 squashme: comment that NUM_OS_RANDOM_BYTES should not be changed lightly (Wladimir J. van der Laan) 9e8c266b158c26b361d68a3cccdbcfd7bee0b376 util: Specific GetOSRandom for Linux/FreeBSD/OpenBSD (W…

furszy

Thanks for this work man.

Just did a first review, i'm not so sure about some of the code here (maybe because it's 2017 dash back ported code and requires further cleanup and code improvements), but let's continue talking and try to determine the best path for this. In one way or another, we need bip32/44 implemented.

Going to pull the code and think more about it.
Will check how upstream has it too, this has been already standardized. We need to prevent any bad (or not well thought) code here, it's one of the most important areas in the wallet.

furszy · 2020-01-28T14:59:37Z

This files was deprecated (the whole trezor repository was) and there is an open discussion about PBKDF2 weakness.

Thoughts?

Unless im wrong,i dont see the whole repo being depricated,more so they moved to a new repo.Since i didnt see deprication mentioned anywhere on their repo.

Checked their new repo,it uses mostly the same code in bip39 if im not mistaken,but in c.

If the repository was moved to other place, not updated anymore, and the code was ported to another language plus some changes were done. Then the code is deprecated. The "mostly the same code" in this key creation code is not enough.

If we are going to back port code from other place, we need to ensure that it's the latest one to not end up having problems that were already solved in other place.

Sorry if I sound hard but this piece of code is in charge of the master key creation, we must go secure in this area.

These were the changes,What they (trezor) have done is move those files to a new repo and refactored bip39 code,other than that i dont see any changes.

What is important is what it's inside of those commits and where they moved from cpp to c code (what changes they did there and how that changes this code -- if it changes it or not.)

It seems that there is no previous history. Instead of refactor the file name, they created this .c as a new one.

furszy · 2020-01-28T15:08:53Z

Why this method is inside the wallet class and not in the hd chain class (or an intermediary HD chain family class)?

furszy · 2020-01-28T15:11:27Z

Instead of doing this nasty nasty thing, bump the version and add an if here.

bump what version exactly?

furszy · 2020-01-28T15:14:58Z

I think that this is not decoupled properly.

Would be better suited inside the hd chain object or an intermediary hd family object, for example:

Wallet --> HD chain ("family") --> Set of simple chains (the current HDChain object).

furszy · 2020-01-28T15:15:57Z

Same here, would be better suited inside the hd chain object or an intermediary hd family object, for example:

Wallet --> HD chain ("family") --> Set of simple chains (the current HDChain object).

akshaynexus · 2020-01-28T15:32:03Z

Thanks for the review comments,ill first get the pr building again by rebasing it and look ad possible cleanup.Most of the code was from dash's 2017 pr as you mentioned so probably got cleaned up later on on their commit,anyways thats to be decided on what all needs to be cleaned up

fix missing header delcaration

akshaynexus · 2020-01-28T15:44:36Z

Rebased on master,should be ready for testing/ functional.

furszy · 2020-01-28T19:03:16Z

k thanks 👍 , going to continue reviewing it in the upcoming days.

akshaynexus · 2020-02-21T14:59:51Z

closing this in favour of furszy's hdwallet v2 pr.

74e7fc7d432c7f6d586d54ff276d6babad0fe505 [Miner] Don't create new keys when generating PoS blocks (random-zebra) Pull request description: This fixes the calls to `CreateNewBlockWithKey` with PoS blocks, which are not needed. We simply call `CreateNewBlock` with an empy script for coinbase. It should prevent the problem described in PIVX-Project/PIVX#956 (comment) ACKs for top commit: CaveSpectre11: utACK PIVX-Project/PIVX@74e7fc7 akshaynexus: utACK PIVX-Project/PIVX@74e7fc7 furszy: utACK [74e7fc7](PIVX-Project/PIVX@74e7fc7) Warrows: Pretty straightforward indeed, utACK 74e7fc7 Tree-SHA512: bcb9752a689a058b6c33317136d16afd694b5da66c69cdd32a8b89b71059c5b2c5dc20c88045c52429fae99d45f508ea223f1b7d1afa5db3c77eb59befd48d51

akshaynexus changed the title ~~[Core] Add HD Wallet support~~ [Core/Qt] Add HD Wallet support Jul 19, 2019

akshaynexus changed the title ~~[Core/Qt] Add HD Wallet support~~ [Core][Qt] Add HD Wallet support Jul 19, 2019

Warrows reviewed Jul 19, 2019

View reviewed changes

Warrows added Backport Needs Release Notes Placeholder tag for anything needing mention in the "Notable Changes" section of release notes review Usability Wallet labels Jul 19, 2019

Warrows added this to the 4.0.0 milestone Jul 19, 2019

Warrows mentioned this pull request Jul 19, 2019

Add BIP39/44 functionality #378

Closed

CaveSpectre11 reviewed Jul 19, 2019

View reviewed changes

CaveSpectre11 reviewed Jul 21, 2019

View reviewed changes

Comment thread src/wallet/wallet.h Outdated

Warrows self-requested a review July 21, 2019 21:21

akshaynexus mentioned this pull request Jul 25, 2019

[Tests] Tests fail when a warning is given #960

Closed

akshaynexus changed the title ~~[Core][Qt] Add HD Wallet support~~ [Core] Add HD Wallet support Jul 25, 2019

JSKitty reviewed Jul 25, 2019

View reviewed changes

Comment thread src/hdchain.h Outdated

random-zebra reviewed Jul 25, 2019

View reviewed changes

random-zebra assigned akshaynexus Jul 25, 2019

random-zebra self-requested a review July 25, 2019 20:51

akshaynexus changed the title ~~[Core] Add HD Wallet support~~ [Core][Tests] Add HD Wallet support Aug 9, 2019

random-zebra mentioned this pull request Aug 11, 2019

[Miner] Don't create new keys when generating PoS blocks #982

Merged

Fuzzbawls modified the milestones: 4.0.0, 4.1.0 Nov 27, 2019

furszy reviewed Jan 28, 2020

View reviewed changes

HD Wallet

7173390

fix missing header delcaration

akshaynexus changed the title ~~[Core][Tests] Add HD Wallet support~~ [Core][RPC][Tests] Add HD Wallet support Jan 28, 2020

akshaynexus added 5 commits January 28, 2020 22:24

Fix compile and some tests

e9f065e

Fix wallet_hd Tests

7aa46fc

Use GetStrongRandBytes in CMnemonic::Generate

d497b3c

Cleanup HDChain and CHDChain set code

5ca6af2

Fix sethdchain error

d09f053

akshaynexus closed this Feb 21, 2020

Fuzzbawls removed the Needs Release Notes Placeholder tag for anything needing mention in the "Notable Changes" section of release notes label Apr 22, 2020

Conversation

akshaynexus commented Jul 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Release Notes

TODO

Uh oh!

random-zebra commented Jul 19, 2019

Uh oh!

Warrows left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

random-zebra left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Fuzzbawls commented Jul 27, 2019

Uh oh!

akshaynexus commented Jul 27, 2019

Uh oh!

random-zebra commented Jul 27, 2019

Uh oh!

akshaynexus commented Jul 29, 2019

Uh oh!

yurykk commented Aug 10, 2019

Uh oh!

akshaynexus commented Aug 10, 2019

Uh oh!

akshaynexus commented Aug 10, 2019

Uh oh!

yurykk commented Aug 10, 2019

Uh oh!

yurykk commented Aug 10, 2019

Uh oh!

yurykk commented Aug 10, 2019

Uh oh!

random-zebra commented Aug 11, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

furszy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

furszy Jan 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

akshaynexus commented Jul 19, 2019 •

edited

Loading

random-zebra commented Aug 11, 2019 •

edited

Loading

furszy Jan 28, 2020 •

edited

Loading