* feat(llmq): Introduce useRotation in LLMQParams
* fix(llmq): Fix IsQuorumRotationEnabled to recognize all dip0024 quorums
* fix(llmq): Do not allow rotation llmqs for `-llmqinstantsend` and non-rotation ones for `-llmqinstantsenddip0024`
* fix(llmq): Unify and fix IsMiningPhase
NOTE: no need for 1 extra block in mining phase for rotation quorums
* chore(llmq): Reduce the number of IsQuorumRotationEnabled calls
* chore(llmq): Improve logging
* feat(llmq): Make `llmq-` threads for rotation quorums distinguishable by quorum index
* fix(llmq): Fix another endless loop in GetQuorumRelayMembers
* throw an error when a llmq type with an incompatible rotation flag is picked for `-llmq...` params
* Add a note about loop conditions
* llmq: Make TransactionRemovedFromMempool the last action for invalid txes, just like we do for orphans with rejected parents
Write to log, send reject msg and (maybe) punish first and only then notify IS about the tx removal. Makes it easier to reason about it when reading logs.
* Remove unused variable
* [refactor] Move tx relay state to separate structure
* [refactor] Change tx_relay structure to be unique_ptr
* Check that tx_relay is initialized before access
* Add comment explaining intended use of m_tx_relay
* Add 2 outbound block-relay-only connections
Transaction relay is primarily optimized for balancing redundancy/robustness
with bandwidth minimization -- as a result transaction relay leaks information
that adversaries can use to infer the network topology.
Network topology is better kept private for (at least) two reasons:
(a) Knowledge of the network graph can make it easier to find the source IP of
a given transaction.
(b) Knowledge of the network graph could be used to split a target node or
nodes from the honest network (eg by knowing which peers to attack in order to
achieve a network split).
We can eliminate the risks of (b) by separating block relay from transaction
relay; inferring network connectivity from the relay of blocks/block headers is
much more expensive for an adversary.
After this commit, bitcoind will make 2 additional outbound connections that
are only used for block relay. (In the future, we might consider rotating our
transaction-relay peers to help limit the effects of (a).)
* Don't relay addr messages to block-relay-only peers
We don't want relay of addr messages to leak information about
these network links.
* doc: improve comments relating to block-relay-only peers
* Disconnect peers violating blocks-only mode
If we set fRelay=false in our VERSION message, and a peer sends an INV or TX
message anyway, disconnect. Since we use fRelay=false to minimize bandwidth,
we should not tolerate remaining connected to a peer violating the protocol.
* net_processing. Removed comment + fixed formatting
* Refactoring net_processing, removed duplicated code
* Refactor some bool in a many-arguments function to enum
It's made to avoid possible typos with arguments, because some of them have default values and it's very high probability to make a mistake here.
* Added UI debug option for Outbound
* Fixed data race related to `setInventoryTxToSend`, introduced in `[refactor] Move tx relay state to separate structure`
Co-authored-by: Suhas Daftuar <sdaftuar@gmail.com>
llmq/snapshot.cpp:31:22: error: loop variable 'h' binds to a temporary value produced by a range of type 'const std::vector<bool>' [-Werror,-Wrange-loop-bind-reference]
for (const auto& h : activeQuorumMembers) {
^
llmq/snapshot.cpp:31:10: note: use non-reference type 'std::__bit_const_reference<std::vector<bool>>'
for (const auto& h : activeQuorumMembers) {
^~~~~~~~~~~~~~~
1 error generated.
* Edge case fix
* Simpler syntax
* add a bit of documentation, and adjust scopes
* use a switch statment
* adjust how returning happens
Co-authored-by: pasta <pasta@dashboost.org>
* refactor(instantsend): make cs_db a Mutex
This introduces GetInstantSendLockByHashInternal and GetInstantSendLockHashByTxidInternal as they are called in Locked and Unlocked contexts. The Internal functions do not lock cs_db, the non-internal functions simply lock cs_db then call the internal function. This ensures saftety (all public functions lock cs_db immediately & all private functions have cs_db already locked) enforced via clang thread safety, while allowing us to use a Mutex here
* refactor(instantsend): remove CInstantSendManager::cs, replace it with cs_inputReqests, cs_creating, cs_pendingLocks, cs_nonLocked, cs_pendingRetry
LOCKS_EXCLUDED are used everywhere where the associated mutex is locked ensuring that deadlocks are impossible
* clang-tidy: avoid implicit conversion to bool
* clang-tidy: pointer / reference adjustments
* clang-tidy: avoid implicit conversion to bool
* clang-tidy: use braces for if
* clang-tidy: don't use else after return
* clang-tidy: mark auto stuff as pointers / const pointers
* clang-tidy: make static
* clang-tidy: avoid implicit conversion to bool
* clang-tidy: use emplace back
* clang-tidy: avoid implicit conversion to bool, and use WITH_LOCK
* use WITH_LOCK
* clang-tidy: avoid implicit conversion to bool
* clang-tidy: use more const / * with auto
* refactor: adjust formatting, and move IsNull to .h
* refactor: remove unused functions
* Update src/llmq/clsig.h
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
* fix(instantsend): avoid an iterator invalidation in pendingInstantSendLocks handling
this also removes the following, as it doesn't take into account deterministic / not deterministic
```
if (pendingInstantSendLocks.size() <= maxCount) {
pend = std::move(pendingInstantSendLocks);
}
```
Also uses structured bindings
* suggestions
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
-BEGIN VERIFY SCRIPT-
sed -i 's/\<strCommand\>/msg_type/g' src/coinjoin/client.cpp
sed -i 's/\<strCommand\>/msg_type/g' src/coinjoin/client.h
sed -i 's/\<strCommand\>/msg_type/g' src/coinjoin/server.cpp
sed -i 's/\<strCommand\>/msg_type/g' src/coinjoin/server.h
sed -i 's/\<strCommand\>/msg_type/g' src/evo/mnauth.cpp
sed -i 's/\<strCommand\>/msg_type/g' src/evo/mnauth.h
sed -i 's/\<strCommand\>/msg_type/g' src/governance/governance.cpp
sed -i 's/\<strCommand\>/msg_type/g' src/governance/governance.h
sed -i 's/\<strCommand\>/msg_type/g' src/llmq/blockprocessor.cpp
sed -i 's/\<strCommand\>/msg_type/g' src/llmq/blockprocessor.h
sed -i 's/\<strCommand\>/msg_type/g' src/llmq/chainlocks.cpp
sed -i 's/\<strCommand\>/msg_type/g' src/llmq/chainlocks.h
sed -i 's/\<strCommand\>/msg_type/g' src/llmq/dkgsessionhandler.cpp
sed -i 's/\<strCommand\>/msg_type/g' src/llmq/dkgsessionhandler.h
sed -i 's/\<strCommand\>/msg_type/g' src/llmq/dkgsessionmgr.cpp
sed -i 's/\<strCommand\>/msg_type/g' src/llmq/dkgsessionmgr.h
sed -i 's/\<strCommand\>/msg_type/g' src/llmq/instantsend.cpp
sed -i 's/\<strCommand\>/msg_type/g' src/llmq/instantsend.h
sed -i 's/\<strCommand\>/msg_type/g' src/llmq/quorums.cpp
sed -i 's/\<strCommand\>/msg_type/g' src/llmq/quorums.h
sed -i 's/\<strCommand\>/msg_type/g' src/llmq/signing.cpp
sed -i 's/\<strCommand\>/msg_type/g' src/llmq/signing.h
sed -i 's/\<strCommand\>/msg_type/g' src/llmq/signing_shares.cpp
sed -i 's/\<strCommand\>/msg_type/g' src/llmq/signing_shares.h
sed -i 's/\<strCommand\>/msg_type/g' src/masternode/sync.cpp
sed -i 's/\<strCommand\>/msg_type/g' src/masternode/sync.h
sed -i 's/\<strCommand\>/msg_type/g' src/net_processing.cpp
sed -i 's/\<strCommand\>/msg_type/g' src/spork.cpp
sed -i 's/\<strCommand\>/msg_type/g' src/spork.h
-END VERIFY SCRIPT-
* Added GET_SNAPSHOT_INFO message handling
* Quorum members by rotation
* Quorum utils functions
* Handle GET_QUORUM_ROTATION_INFO with baseBlockHash from client
* Storing QuorumSnaphots in evoDB when requesting them
* Added DIP Enforcement param
* quorumIndex cache
* Quorum Rotation deployment control
* Usage of Bitsets for storing CQuorumSnapshots
* Correct handling of early quorum quarters
* More asserts
* Corrections
* Handling of quorumIndex
* Refactoring of truncate mechanism
* Various fixes
* Interface correction
* Added template type for indexed cache
* Added quorumIndex into commitmenHash
* Various changes
* Needs to update maqQuorumsCache along with indexedQuorumsCache
* Added CFinalCommitment version 2
* Renamed variables
* Fixes
* Refactoring & correct caching of quorumMembers by rotation
* Added assertions
* Refactoring
* Interface change
* Handling of previous DKG session failure
* Applied refactoring
* Build quarter members improvments
* Merge Quorum Rotation and Decreased fee into one deployment (DIP24)
* Added new LLMQ Type
* Added functional tests + refactoring
* Refactoring
* Spreaded Quorum creation and Quorum Index adaptation
* quorumIndex adaptations
* Added quorumIndex in CFinalCommitment
* Latest work
* Final refactoring
* Batch of refactoring
* Fixes for tests
* Fix for CFinalCommitment
* Fix for Quorums
* Fix
* Small changes
* Thread sync fic
* Safety changes
* Reuse mns when needed
* Refactoring
* More refactoring
* Fixes for rotationinfo handling
* Fix for rotation of members
* Correct order of MNs lists in Quorum Snapshots
* Adding extra logs
* Sync rotation quorums + qrinfo changes
* Fix + extra logs
* Removed redundant field
* Fix for null final commitment + refactoring
* Added timers in tests
* Fix for qrinfo message: quorumdiff and merkleRootQuorums
* Small changes for rotation test
* Remove reading from scanQuorumCache
* Added quorum list output
* Crash fix
* Experimental commit
* apply changes to specialtxman.cpp from specialtx.cpp
* all the changes
* substancially speed up feature_llmq_rotation.py
* reenable asserts, add check for reorgs
* Refactoring
* Added extra logs
* format
* trivial
* drop extra boost includes
* drop ContainsMN
* fix ScanQuorums
* check quorum hash and index in CFinalCommitment::Verify
* fix/tweak tests
* IsQuorumRotationEnabled should be aware of the context
* Calculating members based on earlier block.
* Fix for Quorum Members Cache
* Removed duplicate size of baseBlockHashes
* Adaptations of qrinfo to -8 mn lists
* Introduction of llmqTypeDIP24InstantSend
* Adaptation for llmqTypeDIP24InstantSend
* Adaptations for IS
* bump protocol version
* Added feature_llmq_is_migration test
* Various cleanups
* use unordered_lru_cache for quorumSnapshotCache
* trivial refactor ComputeQuorumMembersByQuarterRotation
* Reduced CFinalCommitment::quorumIndex from 32 to 16 bits
* Keep verified LLMQ relay connections
* Experimental Relay connection fix
* Fix for EnsureQuorumConnections rotation
* Using only valid Mns for checking
* Override of nPowTargetSpacing (devnet only)
* Show penalty score in masternode rpc
* fixups
* Rotation refactoring
* Update src/chainparams.cpp
* Replaced LogPrintf with LogPrint
* IS locking fix once DIP24 activation
* Various cleanup
* Updated MIN_MASTERNODE_PROTO_VERSION
* Introduce LLMQ_TEST_INSTANTSEND reg-test only quorum and actually test switching to dip0024 quorums
* Renamed field lastQuorumHashPerIndex
* Renamed to DIP0024
* chore: update nStartTime and nTimeout for mainnet / testnet for DEPLOYMENT_DIP0024
Co-authored-by: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com>
Co-authored-by: pasta <pasta@dashboost.org>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
* chore: bump MIN_PEER_PROTO_VERSION to 70215
simplifies logic, removes branches
This protocol version is from v14 in May of 2019, should be more than safe to bump this
* drop MIN_COINJOIN_PEER_PROTO_VERSION
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
* refactor: optimizations for processing specialtransaction, specifcally IDB / reindex
GetValidMNsCount -> GetAllMNsCount for reservation as GetValidMNsCount is a slow operation, while GetAllMNsCount is simply .size
Reduce some locking
Pass const Consensus::Params& to CleanupCache. I saw in benchmarks that a substancial
all CheckProRegTx ish methods should take check_sigs
introduce new CDeterministicMNManager thread for handling cleanup
* use main scheduler
* Update src/evo/deterministicmns.cpp
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
* evo: resolve suggestions given in dash#4696
* evo: add known-good mainnet vectors for IsTriviallyValid()
* evo: add artificially malformed vectors for IsTriviallyValid()
* evo: add IsTriviallyValid() tests
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
* Make sure we deserialize the right type of a special tx in GetTxPayload, and debug assert
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
* style: use clang-tidy style named parameters
* refactor: make IsTimeOutOfBounds testable by having current time be a parameter
* style: use x-> not (*x).
* refactor: make SelectCoinsGroupedByAddresses return a vector, remove out param
previous semantics was return false if the vecTally vector was empty. Now we just let the caller check if it is empty or not
* refactor: fix some sign-compare warnings
* refactor: consistently pre-declare stuff as struct / class inline with underlying type
* refactor: don't return const bool
* refactor: use ref to string
* refactor: use = default for CompactTallyItem
* refactor: adjust "initialization" ordering
* refactor: adjust how we handle negatives in GetProjectedMNPayees, use std::min
* refactor: don't bind a reference to a temporary value
* refactor: use a ref
* refactor: ensure attempt in SelectMemberForRecovery is non-negative.
* refactor: remove unused this capture
* refactor: fix numerous sign-compare warnings
* refactor: more consistently use size_t, use empty()
Changes the logic to only request from up to 4 peers, and to prefer masternodes for these peers. In practice, even if these four nodes do not forward the transaction, it doesn't matter, and we will receive the tx soon via inv
* refactor: break circular dependencies(-13, +2)
introduces specialtxman, which handles validation of special transactions, specialtx is now simply the primitive underlying type. This breaks a lot of the circular depends
Also removes an unneeded `#include <masternode/payments.h>` in net_processing.cpp, which resolves a circular dependency. (we know it's okay to remove b/c masternode/payments.h isn't included in any header files, and removing it doesn't break compilation)
* format: make clang-format happy
* remove unrelated change
* remove some unneeded includes to `evo/deterministicmns.h`, explicitly include some previously implicitly included includes.
Resolves two circular dependencies
* refactor: remove circular depend, unused include
* refactor: use a vector instead of map for llmqs
this is a valuable refactor for a number of reasons.
it forces the removal of more verbose `Params().GetConsensus().llmqs.count` and instead call to `Params().HasLLMQ()`
`llmqs` is now stored in contiguous memory (which hopefully means better lookup time / iteration time)
std::vector is much more constexpr friendly, and normally is better optimized
Signed-off-by: Pasta <pasta@dashboost.org>
* use copy_if
Signed-off-by: Pasta <pasta@dashboost.org>
* fixes
Signed-off-by: Pasta <pasta@dashboost.org>
* refactor: numerous changes to avoid passing around a const ref to shared_ptr of CDeterministicMNC when not needed.
Introduces ForEachMNShared, a version of ForEachMN that uses a shared_ptr, and may extend the lifetime of the underlying shared_ptr. This is not preferred, should prefer ForEachMN. See docs.
Adjusts ForEachMN to pass a reference. This is preferred for use over ForEachMNShared. See docs. A reference should be used since in usage we assume it's non-null anyway. Additionally, it allows us to know that the lifespan of the dmn is not being being extended (if lifespan needs to be extended, should use ForEachMNShared.
IsMNValid, IsMNPoSeBanned, UpdateMN, UpdateMN, AddUniqueProperty, DeleteUniqueProperty, UpdateUniqueProperty now take a const reference to CDeterministicMN instead of a const reference to shared_ptr<CDeterministicMN>. All of these functions previously assumed (or would've crashed) a non-null ptr, and non extended lifetime, as such converting to ref is appropriate.
CompareByLastPaid ptr overload now takes raw ptr instead of a const ref to shared. Since we simply dereference them, a raw ptr makes the most sense. This also avoids a potential expensive and implicit raw ptr -> shared ptr conversion if the function was called with raw ptrs.
rpcevo BuildDMNListEntry now takes a const ref for reasons as stated above
Signed-off-by: Pasta <pasta@dashboost.org>
* make stuff const
Signed-off-by: Pasta <pasta@dashboost.org>
* refactor/llmq: use ranges count_if
Signed-off-by: Pasta <pasta@dashboost.org>
* src/evo/evodb.cpp:57:29: warning: Assert statement calls a function which may have desired side effects: 'IsClean'. [assertWithSideEffect]
* src/llmq/quorums.cpp:635:37: note: Null pointer dereference
src/llmq/quorums.cpp:635:37: warning: Either the condition 'pFrom==nullptr' is redundant or there is possible null pointer dereference: pFrom. [nullPointerRedundantCheck]
src/llmq/quorums.cpp:636:81: note: Assuming that condition 'pFrom==nullptr' is not redundant
* fix a bunch of cppcheck warnings
* cppcheck: run on many more files. Enable all checks except a few ignored ones.
ignored
```
"Consider using std::transform algorithm instead of a raw loop."
"Consider using std::accumulate algorithm instead of a raw loop."
```
* ci: build specific version of cppcheck instead of install from apt
* ci: use cppcheck 2.4, remove commented out line, fix symlink
cppcheck 2.6 is latest, however causes issues
```
src/spork.cpp:135:51: warning: Analysis failed. If the code is valid then please report this failure. [cppcheckError]
```
cppcheck 2.5 appears to get into an infinite loop
* no need to check presence before insertion
* use if-init, remove redundant check
* remove redundant check
* don't remove cmake? fix macOs depends build?
* cppcheck: one per line, alphabetize
* remove duplicate cmake install
* refactor: introduce ranges.h for prettier std algo. Also use it in dash core
* fix formatting, use ranges instead of std
* remove commented out code
* introduce ranges find_if_opt, count_if, find_if. Use them all, and more
* use std::accumulate
* capture everything so that threadsaftey analysis is happy
* fix linter
* fix linter
* remove pessimizing move
* instantsend: Avoid writing IS locks for unknown txes
* instantsend: Allow a competing tx into mempool if there is an islock waiting for it
* use try_emplace
* Hold cs_main while calling ResetBlockFailureFlags
* fix: replace seemingly buggy loop with std::adjacent_find
* Remove redundant variable declaration
* use std::fill instead of a loop
* remove a few raw for loops
* use unique_ptr instead of shared
Signed-off-by: pasta <pasta@dashboost.org>
* unique_ptr over shared_ptr
Signed-off-by: pasta <pasta@dashboost.org>
* remove unneeded ptr
Signed-off-by: pasta <pasta@dashboost.org>
* Adjust IsTxSafeForMining checks
Signed-off-by: pasta <pasta@dashboost.org>
* use const ref
Signed-off-by: pasta <pasta@dashboost.org>
* add a todo
Signed-off-by: pasta <pasta@dashboost.org>
* use optional instead of magic max value
fixes a hypothetical bug where myIdx is not "initialized" (ie max), and we sleep forever
Signed-off-by: pasta <pasta@dashboost.org>
* simplify relay check
Signed-off-by: pasta <pasta@dashboost.org>
* use count_if instead of a loop
Signed-off-by: pasta <pasta@dashboost.org>
* add a few vector reserves
Signed-off-by: pasta <pasta@dashboost.org>
* Handle attempts to read non-existent records from isdb properly
* Do not reject blocks that conflict with islocks while still syncing
Otherwise you can stuck with no new blocks/headers which means you won't be able to verify new chainlocks that might override stored islocks
* Handle duplicates/conflicting islocks better
* More constness
519b0bc5dc5155b6f7e2362c2105552bb7618ad0 Make last disconnected block BLOCK_FAILED_VALID, even when aborted (Pieter Wuille)
8d220417cd7bc34464e28a4861a885193ec091c2 Optimization: don't add txn back to mempool after 10 invalidates (Pieter Wuille)
9ce9c37004440d6a329874dbf66b51666d497dcb Prevent callback overruns in InvalidateBlock and RewindBlockIndex (Pieter Wuille)
9bb32eb571a846b66ed3bac493f55cee11a3a1b9 Release cs_main during InvalidateBlock iterations (Pieter Wuille)
9b1ff5c742dec0a6e0d6aab29b0bb771ad6d8135 Call InvalidateBlock without cs_main held (Pieter Wuille)
241b2c74ac8c4c3000e778554da1271e3f293e5d Make RewindBlockIndex interruptible (Pieter Wuille)
880ce7d46b51835c00d77a366ec28f54a05239df Call RewindBlockIndex without cs_main held (Pieter Wuille)
436f7d735f1c37e77d42ff59d4cbb1bd76d5fcfb Release cs_main during RewindBlockIndex operation (Pieter Wuille)
1d342875c21b5d0a17cf4d176063bb14b35b657e Merge the disconnection and erasing loops in RewindBlockIndex (Pieter Wuille)
32b2696ab4b079db736074b57bbc24deaee0b3d9 Move erasure of non-active blocks to a separate loop in RewindBlockIndex (Pieter Wuille)
9d6dcc52c6cb0cdcda220fddccaabb0ffd40068d Abstract EraseBlockData out of RewindBlockIndex (Pieter Wuille)
Pull request description:
This PR makes a number of improvements to the InvalidateBlock (`invalidateblock` RPC) and RewindBlockIndex functions, primarily around breaking up their long-term cs_main holding. In addition:
* They're made safely interruptible (`bitcoind` can be shutdown, and no progress in either will be lost, though if incomplete, `invalidateblock` won't continue after restart and will need to be called again)
* The validation queue is prevented from overflowing (meaning `invalidateblock` on a very old block will not drive bitcoind OOM) (see #14289).
* `invalidateblock` won't bother to move transactions back into the mempool after 10 blocks (optimization).
This is not an optimal solution, as we're relying on the scheduler call sites to make sure the scheduler doesn't overflow. Ideally, the scheduler would guarantee this directly, but that needs a few further changes (moving the signal emissions out of cs_main) to prevent deadlocks.
I have manually tested the `invalidateblock` changes (including interrupting, and running with -checkblockindex and -checkmempool), but haven't tried the rewinding (which is probably becoming increasingly unnecessary, as very few pre-0.13.1 nodes remain that would care to upgrade).
Tree-SHA512: 692e42758bd3d3efc2eb701984a8cb5db25fbeee32e7575df0183a00d0c2c30fdf72ce64c7625c32ad8c8bdc56313da72a7471658faeb0d39eefe39c4b8b8474
* replace raw owning ptr with unique ptr
Signed-off-by: pasta <pasta@dashboost.org>
* Add GUARDED_BY annotation to llmq_versionbitscache
Signed-off-by: pasta <pasta@dashboost.org>
* limit scope of locking cs_llmq_vbc
Signed-off-by: pasta <pasta@dashboost.org>
* use llmq_versionbitscache instead of versionbitscache in UpdatedBlockTip to avoid cs_main locking
Signed-off-by: pasta <pasta@dashboost.org>
* drop unneeded cs_main ::mempool.cs
* lock cs_main and mempool.cs in Db::Upgrade
* include adjustments
* fix macOs build failure
Signed-off-by: pasta <pasta@dashboost.org>
* expcitly include array in spork.h
* sort includes in most files
* merge bitcoin#15855: Add missing LockAnnotation for cs_main
* mutex: update cs_main locks, assertions and annotations
This commit is a squash between 8c98823 and 90d0535
bf2e01097 uint256: Remove unnecessary crypto/common.h use (Karl-Johan Alm)
Pull request description:
This is an alternative to #13242 which keeps the `ReadLE64` part, but moves the `crypto/common.h` dependency into `crypto/common.h` as a function outside of `uint256`.
**Reason:** this change will remove dependencies for `uint256` to `crypto/common.h`, `compat/endian.h`, and `compat/byteswap.h`.
This PR removes the need to update tests to be endian-aware/-independent, but keeps the (arguably dubious) `ReadLE64` part (which was only introduced to fix the tests, not for any functionality).
Tree-SHA512: 78b35123cdb185b3b3ec59aba5ca8a5db72624d147f2d6a5484ffa5ce626a72f782a01dc6893fc8f5619b03e2eae7b5a03b0df5d43460f3bda428e719e188aec
* refactor: introduce PendingSignatureData
* refactor: explicitly capture in lambdas
* refactor: use structured bindings
* refactor: use more const
* refactor: remove unused include header
* refactor: use if-init
* refactor: add nodiscard
* refactor: initialize llmqType
* refactor: add override
* refactor: remove redundant specifiers
* refactor: prevent shadowing
* refactor: use try_emplace where possible
* refactor: use more accurate name
string_view is needed for compile time interpretation (constexpr) of the LLMQParams
constexpr is good to add such that we aren't initializing in static scope, but instead at compile time
Sadly using string_view will require that we construct a std::string when passing into univalue, however, this isn't in a performance important section (rpc) and as such another allocation is an okay trade off for the benefits of constexpr
Also, now we aren't having extern variables, which should make the linker mildly happier
Signed-off-by: pasta <pasta@dashboost.org>
When receiving an islock, propagate it as islock.
When creating/receiving and isdlock, propagate it as isdlock to peers which support it and as islock to peers which don't.
Functional tests to cover both islock and isdlock scenarios.
* bls: use constexpr int instead of #define
* lint: bump c++ version to 17
* test: use BOOST_CHECK_EQUAL instead of BOOST_ASSERT, call boost assert in another location
* coinjoin: fix typo
* drop redundant LLMQType cast
* use numeric_limits instead of magic value
* net_processing.cpp whitespace fixes
* Add some const
* use std::all_of instead of raw for loop
* Introduce UNINITIALIZED_SESSION_ID and use it instead of a magic number
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
* Don't lock instantsend cs where not needed (db interactions)
* Introduce cs_db to CInstantSendDb
All private members are GUARDED_BY or EXCLUSIVE_LOCKS_REQUIRED w/ cs_db
All public functions lock cs_db immediately
* adjust cs usage a bit more in CInstantSendManager
* Use WITH_LOCK in a few spots
* Make AddNonLockedTx lock internally instead of externally
* instantsend adjust formatting
* instantsend move IsInstantSendEnabled enabled check up
This backport does not include changes that depend on bitcoin pr 18037
70a6b529f306ff72ea1badf25e970a92b2b17ab3 lint-cppcheck: Remove -DHAVE_WORKING_BOOST_SLEEP_FOR (Anthony Towns)
294937b39de5924e772f8ed90d35c53290c8acab scheduler_tests: re-enable mockforward test (Anthony Towns)
cea19f685915be8affb2203184a549576194413f Drop unused reverselock.h (Anthony Towns)
d0ebd93270758ea97ea956b8821e17a2d001ea94 scheduler: switch from boost to std (Anthony Towns)
b9c426012770d166e6ebfab27689be44e6e89aa5 sync.h: add REVERSE_LOCK (Anthony Towns)
306f71b4eb4a0fd8e64f47dc008bc235b80b13d9 scheduler: don't rely on boost interrupt on shutdown (Anthony Towns)
Pull request description:
Replacing boost functionality with C++11 stuff.
Motivated by #18227, but should stand alone. Changing from `boost::condition_var` to `std::condition_var` means `threadGroup.interrupt_all` isn't enough to interrupt `serviceQueue` anymore, so that means calling `stop()` before `join_all()` is needed. And the existing reverselock.h code doesn't work with sync.h's DebugLock code (because the reversed lock won't be removed from `g_lockstack` which then leads to incorrect potential deadlock warnings), so I've replaced that with a dedicated class and macro that's aware of our debug lock behaviour.
Fixes#16027, Fixes#14200, Fixes#18227
ACKs for top commit:
laanwj:
ACK 70a6b529f306ff72ea1badf25e970a92b2b17ab3
Tree-SHA512: d1da13adeabcf9186d114e2dad9a4fdbe2e440f7afbccde0c13dfbaf464efcd850b69d3371c5bf8b179d7ceb9d81f4af3cc22960b90834e41eaaf6d52ef7d331
# Conflicts:
# src/reverselock.h
# src/rpc/misc.cpp
# src/scheduler.cpp
# src/scheduler.h
# src/sync.cpp
# src/sync.h
# src/test/reverselock_tests.cpp
# src/test/scheduler_tests.cpp
# src/test/test_dash.cpp
# test/lint/extended-lint-cppcheck.sh
* Avoid locking cs_main before logging, instead log, then lock where possible
* use WITH_LOCK in a number of locations as a replacement of simple scopes
* add EXCLUSIVE_LOCKS_REQUIRED
* minimize locked scope in rpc
* fix macOS build error
Signed-off-by: pasta <pasta@dashboost.org>
* add missing EXCLUSIVE_LOCKS_REQUIRED(cs_main)
Signed-off-by: pasta <pasta@dashboost.org>
* revert an rpc scope reduction
* revert an rpc scope reduction
77777c5624 log: Construct global logger on first use (MarcoFalke)
Pull request description:
The (de)initialization order is not well defined in C++, so generally it is not safe to use globals as the (de/con)structor of one global could use the (de/con)structor of another global before/after it has been (con/de)structed.
Specifically this fixes:
* `g_logger` might not be initialized on the first use, so do that. (Fixes#15111)
Tree-SHA512: eb9c22f4baf31ebc5b0b9ee6a51d1354bae1f0df186cc0ce818b4483c7b5a7f90268d2b549ee96b4c57f8ef36ab239dc6497f74f3e2ef166038f7437c368297d
* refactor: Add `const` qualifier to various llmq related functions
* refactor: Add thread safety annotations to various llmq related functions
And around them where it's required
Fixes
```
POTENTIAL DEADLOCK DETECTED
Previous lock order was:
(1) cs llmq/quorums_instantsend.cpp:1071 (in thread )
(2) cs_main llmq/quorums_instantsend.cpp:1405 (in thread )
Current lock order is:
(2) cs_main net_processing.cpp:2535 (in thread )
(1) cs llmq/quorums_instantsend.cpp:1559 (in thread )
```
* Merge #13743: refactor: Replace boost::bind with std::bind
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
* Replace boost::bind with std::bind and remove Boost.Bind includes
Co-authored-by: MarcoFalke <falke.marco@gmail.com>
* llmq: Split "isdb" out of "llmq" db
We write and erase islocks a lot. It probably makes sense to have a standalone db with a larger cache for this.
* llmq: Split "recsigdb" out of "llmq" db
* llmq: Migrate recovered sigs
* llmq: Split "dkgdb" out of "llmq" db
* llmq: Migrate DKG db
* llmq: Wipe llmq
* Make constructors explicit
Signed-off-by: pasta <pasta@dashboost.org>
* static analysis fixes
Signed-off-by: pasta <pasta@dashboost.org>
* Make pFrom nullptr check it's own
Signed-off-by: pasta <pasta@dashboost.org>
* revert std thread changes and hasOperatorKey
Signed-off-by: pasta <pasta@dashboost.org>
* refactor/bls: misc refactoring and spelling/grammar fixes in bls code
* refactor/evo: misc refactoring and spelling/grammar fixes in evo code
* refactor: some include changes
* refactor: remove redundant `public`
* fix linter
Signed-off-by: pasta <pasta@dashboost.org>
* Sort includes
* Move `class CTxDSIn;`
* Drop unused functions in CBLSWorker
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
Function CWallet::KeepKey requires locking as it has concurrent access to database and member nKeysLeftSinceAutoBackup.
Avoid data race when reading setInventoryTxToSend size by locking the read. If locking happens after the read, the size may change.
Lock cs_mnauth when reading verifiedProRegTxHash.
Make fRPCRunning atomic as it can be read/written from different threads simultaneously.
Make m_masternode_iqr_connection atomic as it can be read/written from different threads simultaneously.
Use a recursive mutex to synchronize concurrent access to quorumVvec.
Make m_masternode_connection atomic as it can be read/written from different threads simultaneously.
Make m_masternode_probe_connection atomic as it can be read/written from different threads simultaneously.
Use a recursive mutex in order to lock access to activeMasterNode.
Use a recursive mutex to synchronize concurrent access to skShare.
Guarded all mnauth fields of a CNode.
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
Removes all (besides one) usages of "Params().GetConsensus().llmqs.at" and instead uses the wrapper in quorum_utils.cpp
Rename all params to llmq_params for consistency and not conflict with non-llmq params
make some llmq_params const where possible
remove unneeded llmq_params variables where it's only used once
Signed-off-by: pasta <pasta@dashboost.org>
Change ctpl implementation to use STL queue & mutex.
Use ctpl synchronized queue instead of boost lockfree queue in bls worker aggregator.
Use smart pointers for memory management of Aggregator and VectorAggregator. With 'delete this;' the objects are prone to data race on the delete operator.
Use smart pointers for memory management of ContributionVerifier.
Pass shared_ptr by value to other threads via worker pool.
1fabd59e7 Break circular dependency: init -> * -> init by extracting shutdown.h (Ben Woosley)
e62fdfeea Drop unused init.h includes (Ben Woosley)
Pull request description:
Most includers just wanted to react to pending shutdown.
This isolates access to `fRequestShutdown` and limits access to the shutdown api functions, including the new `CancelShutdown` for setting it to `false`.
Tree-SHA512: df42f75dfbba163576710e9a67cf1228531fd99d70a2f187bfba0bcc476d6749cf88180a97e66a81bb5b6c3c7f0917de7402d26039ba7b644cb7509b02f7e267
* instantsend: make stuff const where possible
Signed-off-by: pasta <pasta@dashboost.org>
* instantsend: remove unused `params`
Signed-off-by: pasta <pasta@dashboost.org>
* instantsend: combine two nested if's into one
Signed-off-by: pasta <pasta@dashboost.org>
* instantsend: use auto in spots where possible and clear
Signed-off-by: pasta <pasta@dashboost.org>
* coinjoin: make IsValidStructure const
Signed-off-by: pasta <pasta@dashboost.org>
* coinjoin: divide by an integer to avoid double implicit conversions
Signed-off-by: pasta <pasta@dashboost.org>
* coinjoin: make unused parameter unnamed
Signed-off-by: pasta <pasta@dashboost.org>
* quorums.* use const and use references where possible
Signed-off-by: pasta <pasta@dashboost.org>
* quorums.h don't return const when returning by value
Signed-off-by: pasta <pasta@dashboost.org>
* quorums_blockprocessor.cpp remove redundant casts and combine two if statements
Signed-off-by: pasta <pasta@dashboost.org>
* quorums_blockprocessor.cpp make values const
Signed-off-by: pasta <pasta@dashboost.org>
* quorums_chainlocks.cpp access static function statically
Signed-off-by: pasta <pasta@dashboost.org>
* quorums_chainlocks.h remove commented out include
Signed-off-by: pasta <pasta@dashboost.org>
* quorums_commitment.cpp remove redundant casts
Signed-off-by: pasta <pasta@dashboost.org>
* quorums_debug.cpp remove redundant casts and add const
Signed-off-by: pasta <pasta@dashboost.org>
* quorums_dkgsession.cpp use const where possible
Signed-off-by: pasta <pasta@dashboost.org>
* quorums_dkgsessionhandler.cpp use const where possible
Signed-off-by: pasta <pasta@dashboost.org>
* quorums_dkgsessionhandler.cpp don't use std::move which apparently prevents "copy elision" in this instance
Signed-off-by: pasta <pasta@dashboost.org>
* quorums_dkgsessionhandler.cpp use const
Signed-off-by: pasta <pasta@dashboost.org>
* quorums_dkgsessionmgr.cpp misc refactoring
Signed-off-by: pasta <pasta@dashboost.org>
* quorums_signing.* misc refactoring
Signed-off-by: pasta <pasta@dashboost.org>
* quorums_signing_shares.* misc refactoring
Signed-off-by: pasta <pasta@dashboost.org>
* quorums_utils.cpp misc refactoring
Signed-off-by: pasta <pasta@dashboost.org>
232f96f5c8a3920c09db92f4dbac2ad7d10ce8cf doc: Add release notes for -avoidpartialspends (Karl-Johan Alm)
e00b4699cc6d2ee5697d38dd6607eb2631c9b77a clean-up: Remove no longer used ivars from CInputCoin (Karl-Johan Alm)
43e04d13b1ffc02b1082176e87f420198b40c7b1 wallet: Remove deprecated OutputEligibleForSpending (Karl-Johan Alm)
0128121101fb3ee82f3abd3973a967a4226ffe0e test: Add basic testing for wallet groups (Karl-Johan Alm)
59d6f7b4e2f847ec1f2ff46c84e6157655984f85 wallet: Switch to using output groups instead of coins in coin selection (Karl-Johan Alm)
87ebce25d66952f5ce565bb5130dcf5e24049872 wallet: Add output grouping (Karl-Johan Alm)
bb629cb9dc567cc819724d9f4852652926e60cbf Add -avoidpartialspends and m_avoid_partial_spends (Karl-Johan Alm)
65b3eda458221644616d0fdd6ba0fe01bdbce893 wallet: Add input bytes to CInputCoin (Karl-Johan Alm)
a443d7a0ca333b0bae63e04b5d476f9ad9c7aeac moveonly: CoinElegibilityFilter into coinselection.h (Karl-Johan Alm)
173e18a289088c6087ba6fac708e322aa63b7a94 utils: Add insert() convenience templates (Karl-Johan Alm)
Pull request description:
This PR adds an optional (off by default) `-avoidpartialspends` flag, which changes coin select to use output groups rather than outputs, where each output group corresponds to all outputs with the same destination.
It is a privacy improvement, as each time you spend some output, any other output that is publicly associated with the destination (address) will also be spent at the same time, at the cost of fee increase for cases where coin select without group restriction would find a more optimal set of coins (see example below).
For regular use without address reuse, this PR should have no effect on the user experience whatsoever; it only affects users who, for some reason, have multiple outputs with the same destination (i.e. address reuse).
Nodes with this turned off will still try to avoid partial spending, if the fee of the resulting transaction is not greater than the fee of the original transaction.
Example: a node has four outputs linked to two addresses `A` and `B`:
* 1.0 btc to `A`
* 0.5 btc to `A`
* 1.0 btc to `B`
* 0.5 btc to `B`
The node sends 0.2 btc to `C`. Without `-avoidpartialspends`, the following coin selection will occur:
* 0.5 btc to `A` or `B` is picked
* 0.2 btc is output to `C`
* 0.3 - fee is output to (unique change address)
With `-avoidpartialspends`, the following will instead happen:
* Both of (0.5, 1.0) btc to `A` or `B` is picked (one or the other pair)
* 0.2 btc is output to `C`
* 1.3 - fee is output to (unique change address)
As noted, the pro here is that, assuming nobody sends to the address after you spend from it, you will only ever use one address once. The con is that the transaction becomes slightly larger in this case, because it is overpicking outputs to adhere to the no partial spending rule.
This complements #10386, in particular it addresses @luke-jr and @gmaxwell's concerns in https://github.com/bitcoin/bitcoin/pull/10386#issuecomment-300667926 and https://github.com/bitcoin/bitcoin/pull/10386#issuecomment-302361381.
Together with `-avoidreuse`, this fully addresses the concerns in #10065 I believe.
Tree-SHA512: 24687a4490ba59cf4198ed90052944ff4996653a4257833bb52ed24d058b3e924800c9b3790aeb6be6385b653b49e304453e5d7ff960e64c682fc23bfc447621
# Conflicts:
# src/Makefile.am
# src/bench/coin_selection.cpp
# src/wallet/coincontrol.h
# src/wallet/coinselection.cpp
# src/wallet/coinselection.h
# src/wallet/init.cpp
# src/wallet/test/coinselector_tests.cpp
# src/wallet/wallet.cpp
# src/wallet/wallet.h
# test/functional/test_runner.py
* instantsend: remove unused variable sum of input value
Signed-off-by: pasta <pasta@dashboost.org>
* instantsend: Access static member statically
Signed-off-by: pasta <pasta@dashboost.org>
* refac: make functions private as possible
8c2d695c4a util: Store debug log file path in BCLog::Logger member. (Jim Posen)
8e7b961388 scripted-diff: Rename BCLog::Logger member variables. (Jim Posen)
1eac317f25 util: Refactor GetLogCategory. (Jim Posen)
3316a9ebb6 util: Encapsulate logCategories within BCLog::Logger. (Jim Posen)
6a6d764ca5 util: Move debug file management functions into Logger. (Jim Posen)
f55f4fcf05 util: Establish global logger object. (Jim Posen)
Pull request description:
This is purely a refactor with no behavior changes.
This creates a new class `BCLog::Logger` to encapsulate all global logging configuration and state.
Tree-SHA512: b34811f54a53b7375d7b6f84925453c6f2419d21179379ee28b3843d0f4ff8e22020de84a5e783453ea927e9074e32de8ecd05a6fa50d7bb05502001aaed8e53
9b2704777c [doc] Include txindex changes in the release notes. (Jim Posen)
ed77dd6b30 [test] Simple unit test for TxIndex. (Jim Posen)
6d772a3d44 [rpc] Public interfaces to GetTransaction block until synced. (Jim Posen)
a03f804f2a [index] Move disk IO logic from GetTransaction to TxIndex::FindTx. (Jim Posen)
e0a3b80033 [validation] Replace tx index code in validation code with TxIndex. (Jim Posen)
8181db88f6 [init] Initialize and start TxIndex in init code. (Jim Posen)
f90c3a62f5 [index] TxIndex method to wait until caught up. (Jim Posen)
70d510d93c [index] Allow TxIndex sync thread to be interrupted. (Jim Posen)
94b4f8bbb9 [index] TxIndex initial sync thread. (Jim Posen)
34d68bf3a3 [index] Create new TxIndex class. (Jim Posen)
c88bcec93f [db] Migration for txindex data to new, separate database. (Jim Posen)
0cb8303241 [db] Create separate database for txindex. (Jim Posen)
Pull request description:
I'm re-opening #11857 as a new pull request because the last one stopped loading for people
-------------------------------
This refactors the tx index code to be in it's own class and get built concurrently with validation code. The main benefit is decoupling and moving the txindex into a separate DB. The primary motivation is to lay the groundwork for other indexers that might be desired (such as the [compact filters](https://github.com/bitcoin/bips/pull/636)). The basic idea is that the TxIndex spins up its own thread, which first syncs the txindex to the current block index, then once in sync the BlockConnected ValidationInterface hook writes new blocks.
### DB changes
At the suggestion of some other developers, the txindex has been split out into a separate database. A data migration runs at startup on any nodes with a legacy txindex. Currently the migration blocks node initialization until complete.
### Open questions
- Should the migration of txindex data from the old DB to the new DB block in init or should it happen in a background thread? The downside to backgrounding it is that `getrawtransaction` would return an error message saying the txindex is syncing while the migration is running.
### Impact
In a sample size n=1 test where I synced nodes from scratch, the average time [Index writing](https://github.com/bitcoin/bitcoin/blob/master/src/validation.cpp#L1903) was 3.36ms in master and 1.72ms in this branch. The average time between `UpdateTip` log lines for sequential blocks between 400,000 and IBD end on mainnet was 0.297204s in master and 0.286134s in this branch. Most likely this is just variance in IBD times, but I can try with some more trials if people want.
Tree-SHA512: 451fd7d95df89dfafceaa723cdf0f7b137615b531cf5c5035cfb54e9ccc2026cec5ac85edbcf71b7f4e2f102e36e9202b8b3a667e1504a9e1a9976ab1f0079c4
* instantsend: Upgrade IS db on dip0020 activation
* instantsend: Do not remove islocks for txes removed from mempool until dip0020 is activated
* refactor: introduce fUpgradedDB to avoid excessive locking and checking the state
* llmq: Decouple `fUpgradedDB` from `cs`
* Update src/llmq/quorums_instantsend.cpp
Co-authored-by: dustinface <35775977+xdustinface@users.noreply.github.com>
* Fix deadlock
Upgrade locks cs_main via GetTransaction
Co-authored-by: xdustinface <xdustinfacex@gmail.com>
Co-authored-by: dustinface <35775977+xdustinface@users.noreply.github.com>
* instantsend: Resolve block conflicts first and take care of mempool ones later
* refactor: Rename RemoveChainLockConflictingLock -> RemoveConflictingLock
* instantsend: Handle transaction removal from mempool (for all reasons besides inclusion in blocks)
* instantsend: Remove old islocks with no known txes from db (once)
* refactor: Replace magic number with CURRENT_VERSION
* fix: Do not remove islocks for (yet) valid orphans
* Apply suggestions from code review
Co-authored-by: dustinface <35775977+xdustinface@users.noreply.github.com>
Co-authored-by: dustinface <35775977+xdustinface@users.noreply.github.com>
* tests: Check that CLs override ISes which invalidated non-CLed blocks earlier
* partial revert 3987: Do not mark blocks which conflict with ISes as "conflicting"
1527015 Avoid std::locale/imbue in DateTimeStrFormat (Pieter Wuille)
Pull request description:
And replace them with just hardcoded ISO8601 strings and `gmtime_r`.
Pointed out by @laanwj here: https://github.com/bitcoin/bitcoin/pull/12970#issuecomment-380962488
Tree-SHA512: a459758b42ca56f8462115aefe8e6377c1319fce509ea64dbb767f3f087c9b848335954cb684e5896c38008847684045505a3e1559fb3e83b8e80e10b003d1e7
* Simple changes, mostly just includes
* Continued include changes combined with using pointers to avoid including more than necessary in headers
* remove dup include
* masternode-utils.cpp include net.h in all builds
* resolve linter -7 +11
* drop quorums.h from dkgsessionhandler.cpp
* Add `<utilstrencodings.h>`
* Initialize lastMNListForVotingKeys
* Refactor GetMinedCommitment to return sharedptr
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
