| Age | Commit message (Collapse) | Author |
|---|
|
436ce0233c276e263dcb441255dc0b881cb39cfb sync.h: strengthen AssertLockNotHeld assertion (Anthony Towns)
7d73f58e9cea8f4b0bc16512983898fddde3d764 Increase threadsafety annotation coverage (Anthony Towns)
Pull request description:
This changes `AssertLockNotHeld` so that it is annotated with the negative capability for the mutex it refers to. clang applies negative capabilities recursively, so this helps avoid forgetting to annotate functions.
Note that this can't reasonably be used for globals, because clang would require every function to be annotated with `EXCLUSIVE_LOCKS_REQUIRED(!g_mutex)` for each global mutex. At present, the only global mutexes that use `AssertLockNotHeld` are `RecursiveMutex` so we treat that as an exception in order to avoid having to add an excessive number of negative annotations.
ACKs for top commit:
vasild:
ACK 436ce0233c276e263dcb441255dc0b881cb39cfb
MarcoFalke:
review ACK 436ce0233c276e263dcb441255dc0b881cb39cfb 🌺
Tree-SHA512: 5f16d098790a36b5277324d5ee89cdc87033c19b11c7943c2f630a41c2e3998eb39d356a763e857f4d8fefb6c0c02291f720bb6769bcbdf5e2cd765bf266ab8c
|
|
|
|
```
-BEGIN VERIFY SCRIPT-
s() { git grep -l "$1" src test doc | xargs sed -i "s/$1/$2/g"; }
s 'MainSignalsInstance' 'MainSignalsImpl'
-END VERIFY SCRIPT-
|
|
and use Doxygen documentation for it, per our developer notes.
Context:
MainSignalsInstance was created in 3a19fed9db5 and originally was a struct
collection of boost::signals methods moved to validationinterface.cpp, in order
to no longer need to include boost/signals in validationinterface.h.
MainSignalsInstance then evolved in d6815a23131 to remove boost/signals2 and became class-like.
[C.8: Use class rather than struct if any member is
non-public](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rc-class)
[C.2: Use class if the class has an invariant; use struct if the data members can vary
independently](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c2-use-class-if-the-class-has-an-invariant-use-struct-if-the-data-members-can-vary-independently)
A class also has the advantage of default private access, as opposed to public for a struct.
|
|
|
|
Using the zmq notifications to avoid excessive mempool polling can be difficult
given the current notifications available. It announces all transactions
being added to mempool or included in blocks, but announces no evictions
and gives no indication if the transaction is in the mempool or a block.
Block notifications for zmq are also substandard, in that it only announces
block tips, while all block transactions are still announced.
This commit adds a unified stream which can be used to closely track mempool:
1) getrawmempool to fill out mempool knowledge
2) if txhash is announced, add or remove from set
based on add/remove flag
3) if blockhash is announced, get block txn list,
remove from those transactions local view of mempool
4) if we drop a sequence number, go to (1)
The mempool sequence number starts at the value 1, and
increments each time a transaction enters the mempool,
or is evicted from the mempool for any reason, including
block inclusion. The mempool sequence number is published
via ZMQ for any transaction-related notification.
These features allow for ZMQ/RPC consumer to track mempool
state in a more exacting way, without unnecesarily polling
getrawmempool. See interface_zmq.py::test_mempool_sync for
example usage.
|
|
https://github.com/bitcoin/bitcoin/pull/18982#pullrequestreview-416974841
|
|
This fix is a based on the fix by Antoine Riard <ariard@student.42.fr> in
https://github.com/bitcoin/bitcoin/pull/18600.
Unlike that PR, which implements some new behavior, this just restores previous
wallet notification and status behavior for transactions removed from the
mempool because they conflict with transactions in a block. The behavior was
accidentally changed in two `CWallet::BlockConnected` updates:
a31be09bfd77eed497a8e251d31358e16e2f2eb1 and
7e89994133725125dddbfa8d45484e3b9ed51c6e from
https://github.com/bitcoin/bitcoin/pull/16624, causing issue
https://github.com/bitcoin/bitcoin/issues/18325.
The change here could be improved and replaced with a more comprehensive
cleanup, so it includes a detailed comment explaining future considerations.
Fixes #18325
Co-authored-by: Antoine Riard <ariard@student.42.fr>
|
|
|
|
The previous code for MainSignalsInstance::Clear would decrement the reference
count of every interface, including ones that were already Unregister()ed but
still being executed.
|
|
Stop using boost::signals2 internally in validationinterface. Replace with
std::list and Add/Remove/Clear/Iterate helper functions.
Motivation for change is to reduce dependencies and avoid issues happening with
boost versions before 1.59:
https://github.com/bitcoin/bitcoin/issues/18517
https://github.com/bitcoin/bitcoin/pull/18471
|
|
Currently it's possible for ReleaseWallet to delete the CWallet pointer while
it is processing BlockConnected, etc chain notifications.
To fix this, unregister from notifications earlier in UnloadWallet instead of
ReleaseWallet, and use a new RegisterSharedValidationInterface function to
prevent the CValidationInterface shared_ptr from being deleted until the last
notification is actually finished.
|
|
signals
e57980b4738c10344baf136de3e050a3cb958ca5 [mempool] Remove NotifyEntryAdded and NotifyEntryRemoved callbacks (John Newbery)
2dd561f36124972d2364f941de9c3417c65f05b6 [validation] Remove pool member from ConnectTrace (John Newbery)
969b65f3f527631ede1a31c7855151e5c5d91f8f [validation] Remove NotifyEntryRemoved callback from ConnectTrace (John Newbery)
5613f9842b4000fed088b8cf7b99674c328d15e1 [validation] Remove conflictedTxs from PerBlockConnectTrace (John Newbery)
cdb893443cc16edf974f099b8485e04b3db1b1d7 [validation interface] Remove vtxConflicted from BlockConnected (John Newbery)
1168394d759b13af68acec6d5bfa04aaa24561f8 [wallet] Notify conflicted transactions in TransactionRemovedFromMempool (John Newbery)
Pull request description:
These boost signals were added in #9371, before we had a `TransactionRemovedFromMempool` method in the validation interface. The `NotifyEntryAdded` callback was used by validation to build a vector of conflicted transactions when connecting a block, which the wallet was notified of in the `BlockConnected` CValidationInterface callback.
Now that we have a `TransactionRemovedFromMempool` callback, we can fire that signal directly from the mempool for conflicted transactions.
Note that #9371 was implemented to ensure `-walletnotify` events were fired for these conflicted transaction. We inadvertently stopped sending these notifications in #16624 (Sep 2019 commit 7e89994). We should probably fix that, but in a different PR.
ACKs for top commit:
jonatack:
Re-ACK e57980b
ryanofsky:
Code review ACK e57980b4738c10344baf136de3e050a3cb958ca5, no code changes since previous review, but helpful new code comments have been added and the PR description is now more clear about where the old code came from
Tree-SHA512: 3bdbaf1ef2731e788462d4756e69c42a1efdcf168691ce1bbfdaa4b7b55ac3c5b1fd4ab7b90bcdec653703600501b4224d252cfc086aef28f9ce0da3b0563a69
|
|
The wallet now uses TransactionRemovedFromMempool to be notified about
conflicted wallet, and no other clients use vtxConflicted.
|
|
|
|
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./
-END VERIFY SCRIPT-
|
|
f9abf4ab6d3d3e4d4b7e90723020b5422a141a6f Add logging for CValidationInterface events (Jeffrey Czyz)
6edebacb2191373e76d79a4972d6192300976096 Refactor FormatStateMessage for clarity (Jeffrey Czyz)
72f3227c83810936e7a334304e5fd7c6dab8e91b Format CValidationState properly in all cases (Jeffrey Czyz)
428ac70095253225f64462ee15c595644747f376 Add VALIDATION to BCLog::LogFlags (Jeffrey Czyz)
Pull request description:
Add logging of `CValidationInterface` callbacks using a new `VALIDATIONINTERFACE` log flag (see #12994). A separate flag is desirable as the logging can be noisy and thus may need to be disabled without affecting other logging.
This could help debug issues where there may be race conditions at play, such as #12978.
ACKs for top commit:
jnewbery:
ACK f9abf4ab6d3d3e4d4b7e90723020b5422a141a6f
hebasto:
ACK f9abf4ab6d3d3e4d4b7e90723020b5422a141a6f
ariard:
ACK f9abf4a, only changes since 0cadb12 are replacing log indication `VALIDATIONINTERFACE` by `VALIDATION` and avoiding a forward declaration with a new include
ryanofsky:
Code review ACK f9abf4ab6d3d3e4d4b7e90723020b5422a141a6f. Just suggested changes since last review (thanks!)
Tree-SHA512: 3e0f6e2c8951cf46fbad3ff440971d95d526df2a52a2e4d6452a82785c63d53accfdabae66b0b30e2fe0b00737f8d5cb717edbad1460b63acb11a72c8f5d4236
|
|
This could help debug issues where there may be race conditions at play,
such as #12978.
Fixes #12994.
|
|
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./
-END VERIFY SCRIPT-
|
|
This commit fires TransactionRemovedFromMempool callbacks from the
mempool and cleans up a bunch of code.
|
|
To do so we update CValidationInterface::BlockDisconnect to take a
CBlockIndex pointing to the block being disconnected.
This new parameter will be use in the following commit to establish
wallet height.
|
|
Split CValidationState into TxValidationState and BlockValidationState
to store validation results for transactions and blocks respectively.
|
|
|
|
|
|
Removes the now-unused Broadcast/ResendWalletTransactions interface from
validationinterface.
The wallet_resendwallettransactions.py needs a sleep added at the start
to make sure that the rebroadcast scheduler is warmed up before the next
block is mined.
|
|
Move nTimeBestReceived (which is only used for wallet
rebroadcasts) into the wallet.
|
|
When a wallet is created it is registered in the validation interface (in
CWallet::CreateWalletFromFile) but it is not immediately added to the
wallets list. If a shutdown is requested before AddWallet (case more
evident when -rescan is set) then m_internals can be released (in
Shutdown -> UnregisterBackgroundSignalScheduler) before the wallet and
then ReleaseWallet would call UnregisterValidationInterface with
m_internals already released.
|
|
In CMainSignals::RegisterWithMempoolSignals running under Ubuntu 14.04
(QT 5.2), absent piecewise construction this fails to create the pair
because the argument is a connection, which is converted into a
non-copyable scoped_connection.
validationinterface.cpp:80:186: required from here
/usr/include/boost/signals2/connection.hpp:234:7: error: ‘boost::signals2::scoped_connection::scoped_connection(const boost::signals2::scoped_connection&)’ is private
scoped_connection(const scoped_connection &other);
^
In file included from /usr/include/c++/4.8/utility:70:0,
from /usr/include/c++/4.8/algorithm:60,
from ./prevector.h:13,
from ./script/script.h:10,
from ./primitives/transaction.h:11,
from ./validationinterface.h:9,
from validationinterface.cpp:6:
/usr/include/c++/4.8/bits/stl_pair.h:134:45: error: within this context
: first(std::forward<_U1>(__x)), second(__y) { }
https://travis-ci.org/bitcoin/bitcoin/jobs/473689141#L2172
|
|
cb53b825c2 scripted-diff: Replace boost::bind with std::bind (Chun Kuan Lee)
2196c51821 refactor: Use boost::scoped_connection in signal/slot, also prefer range-based loop instead of std::transform (Chun Kuan Lee)
Pull request description:
Replace boost::bind with std::bind
- In `src/rpc/server.cpp`, replace `std::transform` with simple loop.
- In `src/validation.cpp`, store the `boost::signals2::connection` object and use it to disconnect.
- In `src/validationinterface.cpp`, use 2 map to store the `boost::signals2::scoped_connection` object.
Tree-SHA512: 6653cbe00036fecfc495340618efcba6d7be0227c752b37b81a27184433330f817e8de9257774e9b35828026cb55f11ee7f17d6c388aebe22c4a3df13b5092f0
|
|
-BEGIN VERIFY SCRIPT-
mkdir -p src/util
git mv src/util.h src/util/system.h
git mv src/util.cpp src/util/system.cpp
git mv src/utilmemory.h src/util/memory.h
git mv src/utilmoneystr.h src/util/moneystr.h
git mv src/utilmoneystr.cpp src/util/moneystr.cpp
git mv src/utilstrencodings.h src/util/strencodings.h
git mv src/utilstrencodings.cpp src/util/strencodings.cpp
git mv src/utiltime.h src/util/time.h
git mv src/utiltime.cpp src/util/time.cpp
sed -i 's/<util\.h>/<util\/system\.h>/g' $(git ls-files 'src/*.h' 'src/*.cpp')
sed -i 's/<utilmemory\.h>/<util\/memory\.h>/g' $(git ls-files 'src/*.h' 'src/*.cpp')
sed -i 's/<utilmoneystr\.h>/<util\/moneystr\.h>/g' $(git ls-files 'src/*.h' 'src/*.cpp')
sed -i 's/<utilstrencodings\.h>/<util\/strencodings\.h>/g' $(git ls-files 'src/*.h' 'src/*.cpp')
sed -i 's/<utiltime\.h>/<util\/time\.h>/g' $(git ls-files 'src/*.h' 'src/*.cpp')
sed -i 's/BITCOIN_UTIL_H/BITCOIN_UTIL_SYSTEM_H/g' src/util/system.h
sed -i 's/BITCOIN_UTILMEMORY_H/BITCOIN_UTIL_MEMORY_H/g' src/util/memory.h
sed -i 's/BITCOIN_UTILMONEYSTR_H/BITCOIN_UTIL_MONEYSTR_H/g' src/util/moneystr.h
sed -i 's/BITCOIN_UTILSTRENCODINGS_H/BITCOIN_UTIL_STRENCODINGS_H/g' src/util/strencodings.h
sed -i 's/BITCOIN_UTILTIME_H/BITCOIN_UTIL_TIME_H/g' src/util/time.h
sed -i 's/ util\.\(h\|cpp\)/ util\/system\.\1/g' src/Makefile.am
sed -i 's/utilmemory\.\(h\|cpp\)/util\/memory\.\1/g' src/Makefile.am
sed -i 's/utilmoneystr\.\(h\|cpp\)/util\/moneystr\.\1/g' src/Makefile.am
sed -i 's/utilstrencodings\.\(h\|cpp\)/util\/strencodings\.\1/g' src/Makefile.am
sed -i 's/utiltime\.\(h\|cpp\)/util\/time\.\1/g' src/Makefile.am
sed -i 's/-> util ->/-> util\/system ->/' test/lint/lint-circular-dependencies.sh
sed -i 's/src\/util\.cpp/src\/util\/system\.cpp/g' test/lint/lint-format-strings.py test/lint/lint-locale-dependence.sh
sed -i 's/src\/utilmoneystr\.cpp/src\/util\/moneystr\.cpp/g' test/lint/lint-locale-dependence.sh
sed -i 's/src\/utilstrencodings\.\(h\|cpp\)/src\/util\/strencodings\.\1/g' test/lint/lint-locale-dependence.sh
sed -i 's/src\\utilstrencodings\.cpp/src\\util\\strencodings\.cpp/' build_msvc/libbitcoinconsensus/libbitcoinconsensus.vcxproj
-END VERIFY SCRIPT-
|
|
-BEGIN VERIFY SCRIPT-
for j in $(seq 1 5)
do
sed -i "s/ _${j}/ std::placeholders::_${j}/g" $(git grep --name-only " _${j}" -- '*.cpp' '*.h')
done
sed -i "s/boost::bind/std::bind/g" $(git grep --name-only boost::bind -- '*.cpp' '*.h')
sed -i "s/boost::ref/std::ref/g" $(git grep --name-only boost::ref -- '*.cpp' '*.h')
sed -i '/boost\/bind/d' $(git grep --name-only boost/bind)
-END VERIFY SCRIPT-
|
|
range-based loop instead of std::transform
|
|
|
|
|
|
I thought we had removed this a long time ago, TBH, its really
confusing feedback to users that we display whether a tx was
broadcast to immediate neighbor nodes, given that has little
indication of whether the tx propagated very far.
|
|
These were entirely unused, as based on successful compilation
and a grep for:
\bStartShutdown\(\)|\bShutdownRequested\(\)|\bInterrupt\(\)|\bShutdown\(\)|\bInitLogging\(\)|\bInitParameterInteraction\(\)|\bAppInitBasicSetup\(\)|\bAppInitParameterInteraction\(\)|\bAppInitSanityChecks\(\)|\bAppInitLockDataDirectory\(\)|\bAppInitMain\(\)|\bSetupServerArgs\(\)|\bLicenseInfo\(\)|g_wallet_init_interface|init.h
|
|
This much more accurately captures the meaning of the callback.
-BEGIN VERIFY SCRIPT-
sed -i 's/SetBestChain/ChainStateFlushed/g' src/validationinterface.h src/validationinterface.cpp src/wallet/wallet.h src/wallet/wallet.cpp src/validation.cpp src/index/txindex.h src/index/txindex.cpp
-END VERIFY SCRIPT-
|
|
Ensures that callbacks are invoked in the order in which the chain is updated
Resolves #12978
|
|
|
|
|
|
|
|
PR #10286 introduced a few steps which are not robust to early shutdown
in initialization.
Stumbled upon this with #11781, not sure if there are other scenarios
that can trigger it, but it's harden against this in any case.
|
|
-BEGIN VERIFY SCRIPT-
for f in \
src/*.cpp \
src/*.h \
src/bench/*.cpp \
src/bench/*.h \
src/compat/*.cpp \
src/compat/*.h \
src/consensus/*.cpp \
src/consensus/*.h \
src/crypto/*.cpp \
src/crypto/*.h \
src/crypto/ctaes/*.h \
src/policy/*.cpp \
src/policy/*.h \
src/primitives/*.cpp \
src/primitives/*.h \
src/qt/*.cpp \
src/qt/*.h \
src/qt/test/*.cpp \
src/qt/test/*.h \
src/rpc/*.cpp \
src/rpc/*.h \
src/script/*.cpp \
src/script/*.h \
src/support/*.cpp \
src/support/*.h \
src/support/allocators/*.h \
src/test/*.cpp \
src/test/*.h \
src/wallet/*.cpp \
src/wallet/*.h \
src/wallet/test/*.cpp \
src/wallet/test/*.h \
src/zmq/*.cpp \
src/zmq/*.h
do
base=${f%/*}/ relbase=${base#src/} sed -i "s:#include \"\(.*\)\"\(.*\):if test -e \$base'\\1'; then echo \"#include <\"\$relbase\"\\1>\\2\"; else echo \"#include <\\1>\\2\"; fi:e" $f
done
-END VERIFY SCRIPT-
|
|
Note that UpdatedBlockTip is also used in net_processing to
announce new blocks to peers. As this may need additional review,
this change is included in its own commit.
|
|
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 #9583, re-enabling some of the gains from
#7946. This does not, however, re-enable the gains achieved by
repeatedly releasing cs_main between each transaction processed.
|
|
|
|
This is both good practice (we want to move all such callbacks
into a background thread eventually) and prevents a lock inversion
when we go to use this in wallet (mempool.cs->cs_wallet and
cs_wallet->mempool.cs would otherwise both be used).
|
|
This is currently unused, but will by used by wallet to cache when
transactions are in the mempool, obviating the need for calls to
mempool from CWalletTx::InMempool()
|
|
This should fix all the non-dependancy issues for termux builds.
See Github issue #11388.
|
|
In order to avoid unintended implicit conversions.
|
