Move wallet callbacks into cs_main (this effectively reverts #7946) #9583
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
3127935 to
f73bd2c
Compare
sipa
reviewed
Jan 19, 2017
| @@ -2473,20 +2473,20 @@ bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams, | |||
| pindexNewTip = chainActive.Tip(); | |||
| pindexFork = chainActive.FindFork(pindexOldTip); | |||
| fInitialDownload = IsInitialBlockDownload(); | |||
|
|
|||
| // throw all transactions though the signal-interface | |||
| // while _not_ holding the cs_main lock | |||
sipa
reviewed
Jan 19, 2017
| const CBlock& block = *(pair.second); | ||
| for (unsigned int i = 0; i < block.vtx.size(); i++) | ||
| GetMainSignals().SyncTransaction(*block.vtx[i], pair.first, i); | ||
| } | ||
| } |
There was a problem hiding this comment.
I think you need to clear connectTrace.blocksConnected here.
There was a problem hiding this comment.
It will be automatically when it continues the loop.
There was a problem hiding this comment.
Oh, loop-scoped variable, never mind.
f73bd2c to
9899893
Compare
Merged
|
utACK I know we think this is on the top of our minds for further improvement, but I think it would make sense to document at least in the comments and maybe also the commit message why this matter. |
|
utACK 9899893. I think it make sense to apply this revert and go for a better solution in 0.15. |
|
Concept ACK |
2 similar comments
|
Concept ACK |
|
Concept ACK
|
|
Concept ACK. |
sipa
reviewed
Jan 20, 2017
| const CBlock& block = *(pair.second); | ||
| for (unsigned int i = 0; i < block.vtx.size(); i++) | ||
| GetMainSignals().SyncTransaction(*block.vtx[i], pair.first, i); | ||
| } | ||
| } |
There was a problem hiding this comment.
Oh, loop-scoped variable, never mind.
TheBlueMatt
added a commit
to TheBlueMatt/bitcoin
that referenced
this pull request
Jul 11, 2017
This runs BlockConnectedDisconnected, TransactionAddedToMempool, SetBestChain, and Inventory on the background scheduler thread. This partially reverts bitcoin#9583, re-enabling some of the gains from bitcoin#7946. This does not, however, re-enable the gains achieved by repeatedly releasing cs_main between each transaction processed.
TheBlueMatt
added a commit
to TheBlueMatt/bitcoin
that referenced
this pull request
Aug 14, 2017
This runs BlockConnectedDisconnected, TransactionAddedToMempool, SetBestChain, and Inventory on the background scheduler thread. This partially reverts bitcoin#9583, re-enabling some of the gains from bitcoin#7946. This does not, however, re-enable the gains achieved by repeatedly releasing cs_main between each transaction processed.
TheBlueMatt
added a commit
to TheBlueMatt/bitcoin
that referenced
this pull request
Aug 17, 2017
This runs BlockConnectedDisconnected, TransactionAddedToMempool, SetBestChain, and Inventory on the background scheduler thread. This partially reverts bitcoin#9583, re-enabling some of the gains from bitcoin#7946. This does not, however, re-enable the gains achieved by repeatedly releasing cs_main between each transaction processed.
TheBlueMatt
added a commit
to TheBlueMatt/bitcoin
that referenced
this pull request
Sep 12, 2017
This runs BlockConnectedDisconnected, TransactionAddedToMempool, SetBestChain, and Inventory on the background scheduler thread. This partially reverts bitcoin#9583, re-enabling some of the gains from bitcoin#7946. This does not, however, re-enable the gains achieved by repeatedly releasing cs_main between each transaction processed.
jnewbery
pushed a commit
to jnewbery/bitcoin
that referenced
this pull request
Sep 13, 2017
This runs BlockConnectedDisconnected, TransactionAddedToMempool, SetBestChain, and Inventory on the background scheduler thread. This partially reverts bitcoin#9583, re-enabling some of the gains from bitcoin#7946. This does not, however, re-enable the gains achieved by repeatedly releasing cs_main between each transaction processed.
TheBlueMatt
added a commit
to TheBlueMatt/bitcoin
that referenced
this pull request
Oct 1, 2017
This runs Block{Connected,Disconnected}, SetBestChain, Inventory,
and TransactionAddedToMempool on the background scheduler thread.
Of those, only BlockConnected is used outside of Wallet/ZMQ, and
is used only for orphan transaction removal in net_processing,
something which does not need to be synchronous with anything
else.
This partially reverts bitcoin#9583, re-enabling some of the gains from
bitcoin#7946. This does not, however, re-enable the gains achieved by
repeatedly releasing cs_main between each transaction processed.
TheBlueMatt
added a commit
to TheBlueMatt/bitcoin
that referenced
this pull request
Oct 13, 2017
This runs Block{Connected,Disconnected}, SetBestChain, Inventory,
and TransactionAddedToMempool on the background scheduler thread.
Of those, only BlockConnected is used outside of Wallet/ZMQ, and
is used only for orphan transaction removal in net_processing,
something which does not need to be synchronous with anything
else.
This partially reverts bitcoin#9583, re-enabling some of the gains from
bitcoin#7946. This does not, however, re-enable the gains achieved by
repeatedly releasing cs_main between each transaction processed.
laanwj
added a commit
that referenced
this pull request
Nov 15, 2017
…ut cs_main) 89f0312 Remove redundant pwallet nullptr check (Matt Corallo) c4784b5 Add a dev notes document describing the new wallet RPC blocking (Matt Corallo) 3ea8b75 Give ZMQ consistent order with UpdatedBlockTip on scheduler thread (Matt Corallo) cb06edf Fix wallet RPC race by waiting for callbacks in sendrawtransaction (Matt Corallo) e545ded Also call other wallet notify callbacks in scheduler thread (Matt Corallo) 17220d6 Use callbacks to cache whether wallet transactions are in mempool (Matt Corallo) 5d67a78 Add calls to CWallet::BlockUntilSyncedToCurrentChain() in RPCs (Matt Corallo) 5ee3172 Add CWallet::BlockUntilSyncedToCurrentChain() (Matt Corallo) 0b2f42d Add CallFunctionInQueue to wait on validation interface queue drain (Matt Corallo) 2b4b345 Add ability to assert a lock is not held in DEBUG_LOCKORDER (Matt Corallo) 0343676 Call TransactionRemovedFromMempool in the CScheduler thread (Matt Corallo) a7d3936 Add a CValidationInterface::TransactionRemovedFromMempool (Matt Corallo) Pull request description: Based on #10179, this effectively reverts #9583, regaining most of the original speedups of #7946. This concludes the work of #9725, #10178, and #10179. See individual commit messages for more information. Tree-SHA512: eead4809b0a75d1fb33b0765174ff52c972e45040635e38cf3686cef310859c1e6b3c00e7186cbd17374c6ae547bfbd6c1718fe36f26c76ba8a8b052d6ed7bc9
codablock
pushed a commit
to codablock/dash
that referenced
this pull request
Jan 21, 2018
…ely reverts bitcoin#7946) 9899893 Move wallet callbacks into cs_main (this effectively reverts bitcoin#7946) (Matt Corallo)
HashUnlimited
pushed a commit
to chaincoin/chaincoin
that referenced
this pull request
Mar 13, 2018
This runs Block{Connected,Disconnected}, SetBestChain, Inventory,
and TransactionAddedToMempool on the background scheduler thread.
Of those, only BlockConnected is used outside of Wallet/ZMQ, and
is used only for orphan transaction removal in net_processing,
something which does not need to be synchronous with anything
else.
This partially reverts bitcoin#9583, re-enabling some of the gains from
bitcoin#7946. This does not, however, re-enable the gains achieved by
repeatedly releasing cs_main between each transaction processed.
andvgal
pushed a commit
to energicryptocurrency/gen2-energi
that referenced
this pull request
Jan 6, 2019
…ely reverts bitcoin#7946) 9899893 Move wallet callbacks into cs_main (this effectively reverts bitcoin#7946) (Matt Corallo)
CryptoCentric
pushed a commit
to absolute-community/absolute
that referenced
this pull request
Feb 27, 2019
…ely reverts bitcoin#7946) 9899893 Move wallet callbacks into cs_main (this effectively reverts bitcoin#7946) (Matt Corallo)
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Feb 13, 2020
… (without cs_main) 89f0312 Remove redundant pwallet nullptr check (Matt Corallo) c4784b5 Add a dev notes document describing the new wallet RPC blocking (Matt Corallo) 3ea8b75 Give ZMQ consistent order with UpdatedBlockTip on scheduler thread (Matt Corallo) cb06edf Fix wallet RPC race by waiting for callbacks in sendrawtransaction (Matt Corallo) e545ded Also call other wallet notify callbacks in scheduler thread (Matt Corallo) 17220d6 Use callbacks to cache whether wallet transactions are in mempool (Matt Corallo) 5d67a78 Add calls to CWallet::BlockUntilSyncedToCurrentChain() in RPCs (Matt Corallo) 5ee3172 Add CWallet::BlockUntilSyncedToCurrentChain() (Matt Corallo) 0b2f42d Add CallFunctionInQueue to wait on validation interface queue drain (Matt Corallo) 2b4b345 Add ability to assert a lock is not held in DEBUG_LOCKORDER (Matt Corallo) 0343676 Call TransactionRemovedFromMempool in the CScheduler thread (Matt Corallo) a7d3936 Add a CValidationInterface::TransactionRemovedFromMempool (Matt Corallo) Pull request description: Based on bitcoin#10179, this effectively reverts bitcoin#9583, regaining most of the original speedups of bitcoin#7946. This concludes the work of bitcoin#9725, bitcoin#10178, and bitcoin#10179. See individual commit messages for more information. Tree-SHA512: eead4809b0a75d1fb33b0765174ff52c972e45040635e38cf3686cef310859c1e6b3c00e7186cbd17374c6ae547bfbd6c1718fe36f26c76ba8a8b052d6ed7bc9
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Feb 13, 2020
… (without cs_main) 89f0312 Remove redundant pwallet nullptr check (Matt Corallo) c4784b5 Add a dev notes document describing the new wallet RPC blocking (Matt Corallo) 3ea8b75 Give ZMQ consistent order with UpdatedBlockTip on scheduler thread (Matt Corallo) cb06edf Fix wallet RPC race by waiting for callbacks in sendrawtransaction (Matt Corallo) e545ded Also call other wallet notify callbacks in scheduler thread (Matt Corallo) 17220d6 Use callbacks to cache whether wallet transactions are in mempool (Matt Corallo) 5d67a78 Add calls to CWallet::BlockUntilSyncedToCurrentChain() in RPCs (Matt Corallo) 5ee3172 Add CWallet::BlockUntilSyncedToCurrentChain() (Matt Corallo) 0b2f42d Add CallFunctionInQueue to wait on validation interface queue drain (Matt Corallo) 2b4b345 Add ability to assert a lock is not held in DEBUG_LOCKORDER (Matt Corallo) 0343676 Call TransactionRemovedFromMempool in the CScheduler thread (Matt Corallo) a7d3936 Add a CValidationInterface::TransactionRemovedFromMempool (Matt Corallo) Pull request description: Based on bitcoin#10179, this effectively reverts bitcoin#9583, regaining most of the original speedups of bitcoin#7946. This concludes the work of bitcoin#9725, bitcoin#10178, and bitcoin#10179. See individual commit messages for more information. Tree-SHA512: eead4809b0a75d1fb33b0765174ff52c972e45040635e38cf3686cef310859c1e6b3c00e7186cbd17374c6ae547bfbd6c1718fe36f26c76ba8a8b052d6ed7bc9
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Feb 29, 2020
… (without cs_main) 89f0312 Remove redundant pwallet nullptr check (Matt Corallo) c4784b5 Add a dev notes document describing the new wallet RPC blocking (Matt Corallo) 3ea8b75 Give ZMQ consistent order with UpdatedBlockTip on scheduler thread (Matt Corallo) cb06edf Fix wallet RPC race by waiting for callbacks in sendrawtransaction (Matt Corallo) e545ded Also call other wallet notify callbacks in scheduler thread (Matt Corallo) 17220d6 Use callbacks to cache whether wallet transactions are in mempool (Matt Corallo) 5d67a78 Add calls to CWallet::BlockUntilSyncedToCurrentChain() in RPCs (Matt Corallo) 5ee3172 Add CWallet::BlockUntilSyncedToCurrentChain() (Matt Corallo) 0b2f42d Add CallFunctionInQueue to wait on validation interface queue drain (Matt Corallo) 2b4b345 Add ability to assert a lock is not held in DEBUG_LOCKORDER (Matt Corallo) 0343676 Call TransactionRemovedFromMempool in the CScheduler thread (Matt Corallo) a7d3936 Add a CValidationInterface::TransactionRemovedFromMempool (Matt Corallo) Pull request description: Based on bitcoin#10179, this effectively reverts bitcoin#9583, regaining most of the original speedups of bitcoin#7946. This concludes the work of bitcoin#9725, bitcoin#10178, and bitcoin#10179. See individual commit messages for more information. Tree-SHA512: eead4809b0a75d1fb33b0765174ff52c972e45040635e38cf3686cef310859c1e6b3c00e7186cbd17374c6ae547bfbd6c1718fe36f26c76ba8a8b052d6ed7bc9
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Feb 29, 2020
… (without cs_main) 89f0312 Remove redundant pwallet nullptr check (Matt Corallo) c4784b5 Add a dev notes document describing the new wallet RPC blocking (Matt Corallo) 3ea8b75 Give ZMQ consistent order with UpdatedBlockTip on scheduler thread (Matt Corallo) cb06edf Fix wallet RPC race by waiting for callbacks in sendrawtransaction (Matt Corallo) e545ded Also call other wallet notify callbacks in scheduler thread (Matt Corallo) 17220d6 Use callbacks to cache whether wallet transactions are in mempool (Matt Corallo) 5d67a78 Add calls to CWallet::BlockUntilSyncedToCurrentChain() in RPCs (Matt Corallo) 5ee3172 Add CWallet::BlockUntilSyncedToCurrentChain() (Matt Corallo) 0b2f42d Add CallFunctionInQueue to wait on validation interface queue drain (Matt Corallo) 2b4b345 Add ability to assert a lock is not held in DEBUG_LOCKORDER (Matt Corallo) 0343676 Call TransactionRemovedFromMempool in the CScheduler thread (Matt Corallo) a7d3936 Add a CValidationInterface::TransactionRemovedFromMempool (Matt Corallo) Pull request description: Based on bitcoin#10179, this effectively reverts bitcoin#9583, regaining most of the original speedups of bitcoin#7946. This concludes the work of bitcoin#9725, bitcoin#10178, and bitcoin#10179. See individual commit messages for more information. Tree-SHA512: eead4809b0a75d1fb33b0765174ff52c972e45040635e38cf3686cef310859c1e6b3c00e7186cbd17374c6ae547bfbd6c1718fe36f26c76ba8a8b052d6ed7bc9
furszy
pushed a commit
to furszy/bitcoin-core
that referenced
this pull request
Feb 10, 2021
This runs Block{Connected,Disconnected}, SetBestChain, Inventory,
and TransactionAddedToMempool on the background scheduler thread.
Of those, only BlockConnected is used outside of Wallet/ZMQ, and
is used only for orphan transaction removal in net_processing,
something which does not need to be synchronous with anything
else.
This partially reverts bitcoin#9583, re-enabling some of the gains from
bitcoin#7946. This does not, however, re-enable the gains achieved by
repeatedly releasing cs_main between each transaction processed.
furszy
pushed a commit
to furszy/bitcoin-core
that referenced
this pull request
Feb 18, 2021
This runs Block{Connected,Disconnected}, SetBestChain, Inventory,
and TransactionAddedToMempool on the background scheduler thread.
Of those, only BlockConnected is used outside of Wallet/ZMQ, and
is used only for orphan transaction removal in net_processing,
something which does not need to be synchronous with anything
else.
This partially reverts bitcoin#9583, re-enabling some of the gains from
bitcoin#7946. This does not, however, re-enable the gains achieved by
repeatedly releasing cs_main between each transaction processed.
furszy
pushed a commit
to furszy/bitcoin-core
that referenced
this pull request
Feb 19, 2021
This runs Block{Connected,Disconnected}, SetBestChain, Inventory,
and TransactionAddedToMempool on the background scheduler thread.
Of those, only BlockConnected is used outside of Wallet/ZMQ, and
is used only for orphan transaction removal in net_processing,
something which does not need to be synchronous with anything
else.
This partially reverts bitcoin#9583, re-enabling some of the gains from
bitcoin#7946. This does not, however, re-enable the gains achieved by
repeatedly releasing cs_main between each transaction processed.
furszy
pushed a commit
to furszy/bitcoin-core
that referenced
this pull request
Feb 21, 2021
This runs Block{Connected,Disconnected}, SetBestChain, Inventory,
and TransactionAddedToMempool on the background scheduler thread.
Of those, only BlockConnected is used outside of Wallet/ZMQ, and
is used only for orphan transaction removal in net_processing,
something which does not need to be synchronous with anything
else.
This partially reverts bitcoin#9583, re-enabling some of the gains from
bitcoin#7946. This does not, however, re-enable the gains achieved by
repeatedly releasing cs_main between each transaction processed.
gades
pushed a commit
to cosanta/cosanta-core
that referenced
this pull request
Jun 30, 2021
… (without cs_main) 89f0312 Remove redundant pwallet nullptr check (Matt Corallo) c4784b5 Add a dev notes document describing the new wallet RPC blocking (Matt Corallo) 3ea8b75 Give ZMQ consistent order with UpdatedBlockTip on scheduler thread (Matt Corallo) cb06edf Fix wallet RPC race by waiting for callbacks in sendrawtransaction (Matt Corallo) e545ded Also call other wallet notify callbacks in scheduler thread (Matt Corallo) 17220d6 Use callbacks to cache whether wallet transactions are in mempool (Matt Corallo) 5d67a78 Add calls to CWallet::BlockUntilSyncedToCurrentChain() in RPCs (Matt Corallo) 5ee3172 Add CWallet::BlockUntilSyncedToCurrentChain() (Matt Corallo) 0b2f42d Add CallFunctionInQueue to wait on validation interface queue drain (Matt Corallo) 2b4b345 Add ability to assert a lock is not held in DEBUG_LOCKORDER (Matt Corallo) 0343676 Call TransactionRemovedFromMempool in the CScheduler thread (Matt Corallo) a7d3936 Add a CValidationInterface::TransactionRemovedFromMempool (Matt Corallo) Pull request description: Based on bitcoin#10179, this effectively reverts bitcoin#9583, regaining most of the original speedups of bitcoin#7946. This concludes the work of bitcoin#9725, bitcoin#10178, and bitcoin#10179. See individual commit messages for more information. Tree-SHA512: eead4809b0a75d1fb33b0765174ff52c972e45040635e38cf3686cef310859c1e6b3c00e7186cbd17374c6ae547bfbd6c1718fe36f26c76ba8a8b052d6ed7bc9
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.
This is an alternative to #9570 as well as at least two other fixes which would be required to wallet consistency issues generated by #7946, none of which I believe are reasonable to make 0.14.