Yet another attempt of a "minimize to tray" that works on all OSes #941
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
ACK : and I agree with removing it (either completely or for Linux/Mac) if it still doesn't work. |
laanwj
added a commit
that referenced
this pull request
Mar 16, 2012
Yet another attempt of a "minimize to tray" that works on all OSes
coblee
referenced
this pull request
in litecoin-project/litecoin
Jul 17, 2012
Yet another attempt of a "minimize to tray" that works on all OSes
suprnurd
pushed a commit
to chaincoin-legacy/chaincoin
that referenced
this pull request
Dec 5, 2017
10c249f Simplify CountByIP()
ptschip
added a commit
to ptschip/bitcoin
that referenced
this pull request
Feb 26, 2018
…nding messages We do this only during IBD and this has the effect of distributing the load more evenly between the peers. Previously, because we are using PV, the very first peer to connect would always be favored and we would end up downloading a disproportionate amount of blocks from that peer during IBD. However, we still will download more from some peers based on their download response time performance.
ptschip
added a commit
to ptschip/bitcoin
that referenced
this pull request
Mar 5, 2018
* Update the block availaibility for all nodes during IBD
One of the problems with IBD is that we end up downloading
blocks from the same peer. This is because after downloading
all the headers at startup the block availability is only
being updated for that peer. We need to assume that all connected
or newly connected peers have the blocks that we need but we
only do this during IBD or when IsInitialBlockDownload() is true.
* Increase the block download window to 1024
This helps to keep IBD moving forward when we have a peer
that may be serving us blocks more slowly. And now that
blockvailability is being updated correctly we are getting blocks
from all connected peers so the chance of having a slow peer
is now much higher.
* Reset the single peer request mode age to 24 hours
One week isn't needed anymore because the new DAA is working
well and we're not falling behind in terms of blocks mined
per day.
* Remove the check for node ping in the request manager
This feature is causing occasional hangups during IBD and
is not really effective in selecting peers especially during
IBD. Since it is not used when the chain is sync'd then there
is no need for this feature to remain which only adds another
level of complexity to IBD. KISS can be applied here for a better
IBD experience.
* Only update xthin stats when IsChainNearlySyncd()
There is no need to lock vNodes and check all peers for
thinblocks in flight unless we are IsChainNearlySyncd() because
we would not have asked for any xthins if !IsChainNearlySyncd().
This is an expensive operation and which makes a performance hit
during IBD.
* Disconnect non NETWORK nodes during initial sync
We only want to have nodes we can download the full
blockchain from, connected to us while we do our initial sync.
* Increase the default max outbound connections to 16
Having a few more outbound connections can help especially
during the process of IBD.
* During IBD, batch the block reqeusts if possible.
By batching the blocks together we save a little bandwidth
and time in requesting blocks.
This method can then be applied to batching transactions which
should make a very good improvement during periods where transaction
rates are high.
* Replace missing cs_main lock
* fix formatting
* Fix locking order issue with cs_objectDownloader and cs_vNodes
cs_objectDownloader must be locked first to prevent a possible
deaklock.
* Fix formatting
* Use Logging namespace to access LogAcceptCategory
LogAcceptCategory and the enum used to define available logging
category is defined inside the Logging namespace. If used directly
outside of util.{h,cpp} reference to such namespace is needed.
This commit fix issue bitcoin#950
* Don't favor XTHIN nodes when sending messages
This has a negative effect on IBD
* Use more descriptive variable names for nodes we are disconnecting
ptemp1 and ptemp2 are getting hard to follow. We'll say what they are:
pNonXthinNode and pNonNodeNetwork
* Take the cs_objDownloader lock earlier when requesting multiple objects
This prevents anyone else from asking for these same objects before
we've notified the request manager of their existence. This could happen
for in stance during IBD where we look for the next blocks to download
but before we've notified the request manager of them all we then
go back and possibly request them again.
Also keeping the lock here allows the request manager to prepare
a better batch request of blocks and thereby keep a better order
of block requests.
* Call MarkBlocksAsInFlight() before asking for blocks
This ensures you don't receive any blocks back before you mark the block
in flight as could happen on regtest.
* Add/release node refs when making batch requests
We need to track the node refs correctly so we don't get disconnected
before we're done our work.
* Don't ask for the same blocks twice when doing IBD
If we've already asked for a block, we don't have to ask for it
again in FindNextBlocksToDownload(), instead we can rely o the request
manager handle potential re-requests.
* Calculate MaxBlocksInTransitPerPeer, but on an individual node basis
Before we were using the overall resonse times regardless of
which peer they came from. This made the selection of how many
blocks to donwload at one time not responsive to faster or
slower nodes. By tracking response times on a per node basis
we can deprioritize slower downloaders and get more blocks
from the faster nodes.
In HEADERS net processing, remove MAX_BLOCKS_IN_TRANSIT_PER_PEER
This has been replaced with pnode->nMaxBlocksInTransitPerPeer
use atomic store, load, fetch_add
* Fixes bitcoin#941: Rotate vNodes by one peer every 60 seconds when sending messages
We do this only during IBD and this has the effect of distributing
the load more evenly between the peers. Previously, because we are
using PV, the very first peer to connect would always be favored and
we would end up downloading a disproportionate amount of blocks from
that peer during IBD. However, we still will download more from
some peers based on their download response time performance.
* Small net optimization when sending messages
Just copy the entire vector rather than bothering to allocate
memory and then push_back for each entry as we iteration through
vNodes.
Make sure to LOCK vNodes then add the ref
* Fix getchaintips.py spurious failures
The hang results from a PV bug which is caused by a condition where
threads that are waiting for validation are forced to quit during chain
tip rollback. We don't in fact have to force any threads to quit since
any validation threads on the current chain will fail automatically
once we disconnect the tip. And then the waiting block validation
thread for the new chain can still continue without being inadvertanly
forced to quit.
* Do not to use an incorrect or invalid state
Use the node id from the iterator rather than looking up state
by the node we think we just passed in. Just in case they don't match
for some reason.
Also DbgAssert if the state is NULL and return false if we assert.
* Small edit to logprint
Add category BLK to logprint
remove printf
* During IBD only request blocks from peers that have block availability
Request from peers that are NODE_NETWORK and have demonstrated that
they have the blocks that we need.
Also move ProcessBlockAvailablity and UpdateBlockAvailabilty to
RequestManager.cpp
* Move FindNextBlocksToDownload() to the requestManager.cpp
Also change all NULL's to nullptr.
* Move MarkBlockAsInFlight() and MarkBlockAsReceived() to requestManager.cpp
This ia a Move only. Alhtough there are edits in order to
bring these into the requestManager class structure there
are no logic changes.
C++ nullptr replaces NULL's
* Update block availablity during the process of downloading headers
During initial sync we have to download a set of all headers but
we also need to update the block availabiity for each connected
peer so that when the request manager starts downloading blocks
it can have the correct list of peers that have available blocks
to download from, rather than just assuming every peer has all
blocks.
* EXIT cs_objDownloader earlier.
We don't have to repeatedly take and release the locks.
Just do it once for all items in the entire batch request.
* Only ask for headers to updateblockavailability if chain work is behind
When doing the initial request for headers and we're in the process
of also updating the block availability for each connected peer we
only need to ask for a header if any peer is actually behind in
terms of chain work. This prevents first of all requesting
headers we don't need but also any possible attack where we're being
fed an invalid group of headers which then causes us to request
large number of additional headers.
ptschip
added a commit
to ptschip/bitcoin
that referenced
this pull request
Mar 7, 2018
* Update the block availaibility for all nodes during IBD
One of the problems with IBD is that we end up downloading
blocks from the same peer. This is because after downloading
all the headers at startup the block availability is only
being updated for that peer. We need to assume that all connected
or newly connected peers have the blocks that we need but we
only do this during IBD or when IsInitialBlockDownload() is true.
* Increase the block download window to 1024
This helps to keep IBD moving forward when we have a peer
that may be serving us blocks more slowly. And now that
blockvailability is being updated correctly we are getting blocks
from all connected peers so the chance of having a slow peer
is now much higher.
* Reset the single peer request mode age to 24 hours
One week isn't needed anymore because the new DAA is working
well and we're not falling behind in terms of blocks mined
per day.
* Remove the check for node ping in the request manager
This feature is causing occasional hangups during IBD and
is not really effective in selecting peers especially during
IBD. Since it is not used when the chain is sync'd then there
is no need for this feature to remain which only adds another
level of complexity to IBD. KISS can be applied here for a better
IBD experience.
* Only update xthin stats when IsChainNearlySyncd()
There is no need to lock vNodes and check all peers for
thinblocks in flight unless we are IsChainNearlySyncd() because
we would not have asked for any xthins if !IsChainNearlySyncd().
This is an expensive operation and which makes a performance hit
during IBD.
* Disconnect non NETWORK nodes during initial sync
We only want to have nodes we can download the full
blockchain from, connected to us while we do our initial sync.
* Increase the default max outbound connections to 16
Having a few more outbound connections can help especially
during the process of IBD.
* During IBD, batch the block reqeusts if possible.
By batching the blocks together we save a little bandwidth
and time in requesting blocks.
This method can then be applied to batching transactions which
should make a very good improvement during periods where transaction
rates are high.
* Replace missing cs_main lock
* fix formatting
* Fix locking order issue with cs_objectDownloader and cs_vNodes
cs_objectDownloader must be locked first to prevent a possible
deaklock.
* Fix formatting
* Use Logging namespace to access LogAcceptCategory
LogAcceptCategory and the enum used to define available logging
category is defined inside the Logging namespace. If used directly
outside of util.{h,cpp} reference to such namespace is needed.
This commit fix issue bitcoin#950
* Don't favor XTHIN nodes when sending messages
This has a negative effect on IBD
* Use more descriptive variable names for nodes we are disconnecting
ptemp1 and ptemp2 are getting hard to follow. We'll say what they are:
pNonXthinNode and pNonNodeNetwork
* Take the cs_objDownloader lock earlier when requesting multiple objects
This prevents anyone else from asking for these same objects before
we've notified the request manager of their existence. This could happen
for in stance during IBD where we look for the next blocks to download
but before we've notified the request manager of them all we then
go back and possibly request them again.
Also keeping the lock here allows the request manager to prepare
a better batch request of blocks and thereby keep a better order
of block requests.
* Call MarkBlocksAsInFlight() before asking for blocks
This ensures you don't receive any blocks back before you mark the block
in flight as could happen on regtest.
* Add/release node refs when making batch requests
We need to track the node refs correctly so we don't get disconnected
before we're done our work.
* Don't ask for the same blocks twice when doing IBD
If we've already asked for a block, we don't have to ask for it
again in FindNextBlocksToDownload(), instead we can rely o the request
manager handle potential re-requests.
* Calculate MaxBlocksInTransitPerPeer, but on an individual node basis
Before we were using the overall resonse times regardless of
which peer they came from. This made the selection of how many
blocks to donwload at one time not responsive to faster or
slower nodes. By tracking response times on a per node basis
we can deprioritize slower downloaders and get more blocks
from the faster nodes.
In HEADERS net processing, remove MAX_BLOCKS_IN_TRANSIT_PER_PEER
This has been replaced with pnode->nMaxBlocksInTransitPerPeer
use atomic store, load, fetch_add
* Fixes bitcoin#941: Rotate vNodes by one peer every 60 seconds when sending messages
We do this only during IBD and this has the effect of distributing
the load more evenly between the peers. Previously, because we are
using PV, the very first peer to connect would always be favored and
we would end up downloading a disproportionate amount of blocks from
that peer during IBD. However, we still will download more from
some peers based on their download response time performance.
* Small net optimization when sending messages
Just copy the entire vector rather than bothering to allocate
memory and then push_back for each entry as we iteration through
vNodes.
Make sure to LOCK vNodes then add the ref
* Fix getchaintips.py spurious failures
The hang results from a PV bug which is caused by a condition where
threads that are waiting for validation are forced to quit during chain
tip rollback. We don't in fact have to force any threads to quit since
any validation threads on the current chain will fail automatically
once we disconnect the tip. And then the waiting block validation
thread for the new chain can still continue without being inadvertanly
forced to quit.
* Do not to use an incorrect or invalid state
Use the node id from the iterator rather than looking up state
by the node we think we just passed in. Just in case they don't match
for some reason.
Also DbgAssert if the state is NULL and return false if we assert.
* Small edit to logprint
Add category BLK to logprint
remove printf
* During IBD only request blocks from peers that have block availability
Request from peers that are NODE_NETWORK and have demonstrated that
they have the blocks that we need.
Also move ProcessBlockAvailablity and UpdateBlockAvailabilty to
RequestManager.cpp
* Move FindNextBlocksToDownload() to the requestManager.cpp
Also change all NULL's to nullptr.
* Move MarkBlockAsInFlight() and MarkBlockAsReceived() to requestManager.cpp
This ia a Move only. Alhtough there are edits in order to
bring these into the requestManager class structure there
are no logic changes.
C++ nullptr replaces NULL's
* Update block availablity during the process of downloading headers
During initial sync we have to download a set of all headers but
we also need to update the block availabiity for each connected
peer so that when the request manager starts downloading blocks
it can have the correct list of peers that have available blocks
to download from, rather than just assuming every peer has all
blocks.
* EXIT cs_objDownloader earlier.
We don't have to repeatedly take and release the locks.
Just do it once for all items in the entire batch request.
* Only ask for headers to updateblockavailability if chain work is behind
When doing the initial request for headers and we're in the process
of also updating the block availability for each connected peer we
only need to ask for a header if any peer is actually behind in
terms of chain work. This prevents first of all requesting
headers we don't need but also any possible attack where we're being
fed an invalid group of headers which then causes us to request
large number of additional headers.
lateminer
pushed a commit
to lateminer/bitcoin
that referenced
this pull request
Oct 30, 2019
42fe404 [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 bitcoin#938 ACKs for top commit: CaveSpectre11: ACK PIVX-Project@42fe404 random-zebra: ACK PIVX-Project@42fe404 furszy: utACK [42fe404](PIVX-Project@42fe404) Tree-SHA512: d3417f03cab63aa41b1ff0fb4d391d42f4448b83efe70391d4014407008af34eb265a23b5807ebecb90cf9637fb74e5b330b02c92e7038e5400a5f16608800ee
KolbyML
pushed a commit
to KolbyML/bitcoin
that referenced
this pull request
Sep 4, 2020
* 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…
sipa
added a commit
to sipa/bitcoin
that referenced
this pull request
Jul 14, 2021
be8d9c2 Merge bitcoin-core/secp256k1#965: gen_context: Don't use any ASM aeece44 gen_context: Don't use any ASM 7688a4f Merge bitcoin-core/secp256k1#963: "Schnorrsig API overhaul" fixups 90e8344 ci: Add C++ test f698caa Use unsigned char consistently for byte arrays b5b8e7b Don't declare constants twice 769528f Don't use string literals for char arrays without NUL termination 2cc3cfa Fix -Wmissing-braces warning in clang 0440945 Merge bitcoin#844: schnorrsig API overhaul ec3aaa5 Merge bitcoin#960: tests_exhaustive: check the result of secp256k1_ecdsa_sign a1ee83c tests_exhaustive: check the result of secp256k1_ecdsa_sign 253f90c Merge bitcoin-core/secp256k1#951: configure: replace AC_PATH_PROG to AC_CHECK_PROG 446d28d Merge bitcoin-core/secp256k1#944: Various improvements related to CFLAGS 0302138 ci: Make compiler warning into errors on CI b924e1e build: Ensure that configure's compile checks default to -O2 7939cd5 build: List *CPPFLAGS before *CFLAGS like on the compiler command line 595e8a3 build: Enable -Wcast-align=strict warning 0725626 build: Use own variable SECP_CFLAGS instead of touching user CFLAGS 4866178 Merge bitcoin-core/secp256k1#955: Add random field multiply/square tests 75ce488 Merge bitcoin-core/secp256k1#959: tests: really test the non-var scalar inverse 41ed139 tests: really test the non-var scalar inverse 5f6ceaf schnorrsig: allow setting MSGLEN != 32 in benchmark fdd06b7 schnorrsig: add tests for sign_custom and varlen msg verification d8d806a schnorrsig: add extra parameter struct for sign_custom a0c3fc1 schnorrsig: allow signing and verification of variable length msgs 5a8e499 Add secp256k1_tagged_sha256 as defined in BIP-340 b6c0b72 schnorrsig: remove noncefp args from sign; add sign_custom function bdf19f1 Add random field multiply/square tests 8ae56e3 Merge bitcoin#879: Avoid passing out-of-bound pointers to 0-size memcpy a4642fa configure: replace AC_PATH_PROG to AC_CHECK_PROG 1758a92 Merge bitcoin#950: ci: Add ppc64le build c58c4ea ci: Add ppc64le build 7973576 Merge bitcoin#662: Add ecmult_gen, ecmult_const and ecmult to benchmark 8f879c2 Fix array size in bench_ecmult 2fe1b50 Add ecmult_gen, ecmult_const and ecmult to benchmark 593e6ba Clean up ecmult_bench to make space for more benchmarks 50f3367 Merge bitcoin#947: ci: Run PRs on merge result even for i686 a35fdd3 ci: Run PRs on merge result even for i686 442cee5 schnorrsig: add algolen argument to nonce_function_hardened df3bfa1 schnorrsig: clarify result of calling nonce_function_bip340 without data 99e8614 README: mention schnorrsig module 3dc8c07 Merge bitcoin#846: ci: Run ASan/LSan and reorganize sanitizer and Valgrind jobs 02dcea1 ci: Make test iterations configurable and tweak for sanitizer builds 489ff5c tests: Treat empty SECP2561_TEST_ITERS as if it was unset fcfcb97 ci: Simplify to use generic wrapper for QEMU, Valgrind, etc de4157f ci: Run ASan/LSan and reorganize sanitizer and Valgrind jobs 399722a Merge bitcoin#941: Clean up git tree 09b3bb8 Clean up git tree bf0ac46 Merge bitcoin#930: Add ARM32/ARM64 CI 202a030 Merge bitcoin#850: add `secp256k1_ec_pubkey_cmp` method 1e78c18 Merge bitcoin-core/secp256k1#940: contrib: Explain explicit header guards 6939487 Merge bitcoin#926: secp256k1.h: clarify that by default arguments must be != NULL 6eceec6 add `secp256k1_xonly_pubkey_cmp` method 0d9561a add `secp256k1_ec_pubkey_cmp` method 22a9ea1 contrib: Explain explicit header guards 6c52ae8 Merge bitcoin#937: Have ge_set_gej_var, gej_double_var and ge_set_all_gej_var initialize all fields of their outputs. 185a6af Merge bitcoin#925: changed include statements without prefix 'include/' 14c9739 tests: Improve secp256k1_ge_set_all_gej_var for some infinity inputs 4a19668 tests: Test secp256k1_ge_set_all_gej_var for all infinity inputs 3c90bdd change local lib headers to be relative for those pointing at "include/" dir 45b6468 Have secp256k1_ge_set_all_gej_var initialize all fields. Previous behaviour would not initialize r->y values in the case where infinity is passed in. Furthermore, the previous behaviour wouldn't initialize anything in the case where all inputs were infinity. 31c0f6d Have secp256k1_gej_double_var initialize all fields. Previous behaviour would not initialize r->x and r->y values in the case where infinity is passed in. dd6c3de Have secp256k1_ge_set_gej_var initialize all fields. Previous behaviour would not initialize r->x and r->y values in the case where infinity is passed in. d0bd269 Merge bitcoin-core/secp256k1#936: Fix gen_context/ASM build on ARM 8bbad7a Add asm build to ARM32 CI 7d65ed5 Add ARM32/ARM64 CI c848352 Makefile.am: Don't pass a variable twice 2161f31 Makefile.am: Honor config when building gen_context 99f47c2 gen_context: Don't use external ASM because it complicates the build 98e0358 Merge bitcoin#933: Avoids a missing brace warning in schnorrsig/tests_impl.h on old compilers 99e2d5b Avoids a missing brace warning in schnorrsig/tests_impl.h on old compilers. 34388af Merge bitcoin#922: Add mingw32-w64/wine CI build 7012a18 Merge bitcoin#928: Define SECP256K1_BUILD in secp256k1.c directly. ed5a199 tests: fopen /dev/urandom in binary mode ae9e648 Define SECP256K1_BUILD in secp256k1.c directly. 4dc37bf Add mingw32-w64/wine CI build 0881633 secp256k1.h: clarify that by default arguments must be != NULL 9570f67 Avoid passing out-of-bound pointers to 0-size memcpy git-subtree-dir: src/secp256k1 git-subtree-split: be8d9c2
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The current "minimize to tray" implementation has problems on Windows. It converts the window to a tool window, but when it is brought back it is not converted back to a normal window, causing strange behavior (such as an ugly title bar), as tool windows don't get a ChangeEvent on unminimization.
This implements the eventual suggestion from the Qt forums http://qt-project.org/forums/viewthread/4423/P15 .
I've tested it on Ubuntu and Windows (XP) and it works there. Can someone test it on KDE please? @sje397?
If we don't manage to get it right this time I'm going to remove the "minimize to tray" functionality completely (but not "Minimize on close" of course, because that works fine...). Or maybe keep it only on Windows.