-
Notifications
You must be signed in to change notification settings - Fork 38.6k
Remove -bip16 and -paytoscripthashtime command-line arguments #970
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Conversation
This file contains hidden or 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
Member
|
Code-visual ACK. |
Member
|
ACK, patch looks OK |
Contributor
|
ACK |
2 similar comments
Member
|
ACK |
Contributor
|
ACK |
suprnurd
pushed a commit
to chaincoin-legacy/chaincoin
that referenced
this pull request
Dec 5, 2017
* Improve JSON error reporting in CGovernanceObject::LoadData * Changed JSON parsing to match current version of sentinel which now sends correct JSON integers instead of quoting them as strings
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
91b48c7 [Build] Add new merkle files to CMake lists (warrows) 48a3aff [Wallet] Ignore coinbase and zc tx "conflicts" (warrows) 3572354 [Wallet] Fix an error in tx depth computation (warrows) 6928369 [Tests] Enable abandonconflict functional test (warrows) 34cd496 Fix that CWallet::AbandonTransaction would only traverse one level (Ben Woosley) aba5b75 Fix calculation of balances and available coins. (Alex Morcos) 48d705f Remove stale wallet transactions on initial load. (presstab) 12985ae Flush wallet after abandontransaction (Alex Morcos) 8f87956 [Wallet] sort pending wallet transactions before reaccepting (dexX7) 9c2f445 [Wallet] Call notification signal when a transaction is abandoned (Jonas Schnelli) 778ebf3 Add new rpc call: abandontransaction (Alex Morcos) 0e86c3e Make wallet descendant searching more efficient (Alex Morcos) d0083a8 Make sure conflicted wallet tx's update balances (Alex Morcos) 6a50e03 [Wallet] Keep track of explicit wallet conflicts instead of using mempool (warrows) 7ccb2b5 [Wallet] Do not flush the wallet in AddToWalletIfInvolvingMe(..) (warrows) 47345be [Refactor] Move wallet functions out of header (warrows) ab9efb8 [Wallet] Switch to a constant-space Merkle root/branch algorithm (warrows) 5447622 [Wallet] Do not store Merkle branches in the wallet (warrows) Pull request description: This pull request is a happy melting pot of improvements regarding transactions handling. Most of them are backports from bitcoin. I advise reviewers to check the code of the different commits independently to understand them more easily. However, testing is probably better done all at once. I am making a single pull request because these changes are all entangled and introducing some of them without others would probably introduce temporary bugs. ## Commits details ## - 6c3e2ac backport of bitcoin#6550 - 5304fdf backport of bitcoin#6508 - c3eeeac simple code move from the header to the cpp file. It contains no functional change. - 6cc4d37 backport of bitcoin#4805 - 10be1db backport of bitcoin#7105 - 8a34c32 backport of bitcoin#7306 - 3caf123, 9e17178 and 240f5b4 are the backport for bitcoin#7312 - ad6d0b1 backport of bitcoin#5511 - fcc07c3562 backport of bitcoin#9311 - 5ed5e266794 is an update of bitcoin#825 - 392d504 backport of bitcoin#7715 - 7199f3a backport of bitcoin#13652 - f09d999 enables and fixes the test from bitcoin#7312 - 4fd43c5 fixes an oversight in bitcoin#7105 backport ACKs for top commit: random-zebra: ACK 91b48c7 Fuzzbawls: ACK 91b48c7 Tree-SHA512: 2628cebe98805b8048b920b51ee26fd4f0c53643d78da9b8cb265aede52dfe1d40c8c19d34293c232c5c35be7f1ab89ff5b4a07073a4b27c371ea70eb8708669
lateminer
pushed a commit
to lateminer/bitcoin
that referenced
this pull request
Oct 30, 2019
…e's inputs to the wallet 3a7ec7c [Tests] Add wallet_reorg-stake functional test to test_runner.py (random-zebra) a0285e4 [Wallet] Fix bug with coinstake inputs wrongly marked as spent (random-zebra) bb683c7 [Tests] Add wallet_reorg-stake functional test (random-zebra) 3eca8da [Core][Tests] REGTEST: fix nStakeModifier=0 (random-zebra) Pull request description: Additional bug introduced with the changes of bitcoin#970 (and not caught in bitcoin#1040 even though the culprit is the same: `GetDepthInMainChain` returning a value `0` when `-1` was expected). After a block reorganization, the coins used as coinstake inputs in the orphan chain were still marked as spent in the wallet. Thus there were inconsistencies in the balance (either displayed in the GUI or returned by `getbalance` via CLI) and missing utxos in the wallet (either accessed through coincontrol in the GUI or returned by `listunspent` via CLI). PIVX-Project@a0285e4 fixes it by marking as "spent" the inputs of not-in-mempool txes only for non-coinstakes (coinstakes don't hit the mempool so, when not in chain, their inputs should be considered unspent). PIVX-Project@bb683c7 Introduces the functional test `wallet_reorg-stake` to reproduce the issue. The test fails without PIVX-Project@a0285e4 and passes with it. ACKs for top commit: Warrows: ACK 3a7ec7c Tree-SHA512: 8f97e3d48720b776c84820e0ab8257665ac4c4c9d394db0e4b9f3a05b0904bf9f70cf54865338c4e29066e6b3670e9e90f77780d3ddd198e8e2b6e416c4cb49c
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.
Now that the BIP16 switchover is locked-in, I think it is best to remove the command-line arguments that were only needed during the transition period.