af3b0dfc5463c42fb9bff39f020fc1728ed44bc7 net: fix output of peer address in version message (Vasil Dimov)
Pull request description:
If `-logips -debug=net` is specified then we print the contents of the
version message we send to the peer, including his address. Because the
addresses in the version message use pre-BIP155 encoding they cannot
represent a Tor v3 address and we would actually send 16 `0`s instead (a
dummy IPv6 address). However we would print the full address in the log
message. Before this fix:
```
2020-10-21T12:24:17Z send version message: version 70016, blocks=653500, us=[::]:0, them=xwjtp3mj427zdp4tljiiivg2l5ijfvmt5lcsfaygtpp6cw254kykvpyd.onion:8333, peer=0
```
This is confusing because we pretend to send one thing while we actually
send another. Adjust the printout to reflect what we are sending. After
this fix:
```
2020-10-21T12:26:54Z send version message: version 70016, blocks=653500, us=[::]:0, them=[::]:0, peer=0
```
ACKs for top commit:
MarcoFalke:
review ACK af3b0dfc5463c42fb9bff39f020fc1728ed44bc7
jnewbery:
utACK af3b0dfc5463c42fb9bff39f020fc1728ed44bc7
Tree-SHA512: f169d7b4f07c219e541f7c37ea23b82c77e50085fc72ec62f1dd46970389916e177268d07d45c7be94dd209d1903f8f23eaff62b7fa782f6057dd36bb96bba82
fa4fc8856b239059421a8e507b3e3e7f4b379a72 validation: Add and use HaveTxsDownloaded where appropriate (MarcoFalke)
Pull request description:
`nChainTx` is an implementation detail that shouldn't be exposed without a wrapper that comes with appropriate documentation.
Tree-SHA512: 56ab7378c2ce97794498724c271f861de982de69099e90ec09632a26230ae6fded3c59668adb378bd64dcb8ef714769b970210977b88a53fc7550774ddba3d59
054d01d0a87a5adc43428588ecc29f1339a69dd2 Do not relay banned IP addresses (Pieter Wuille)
Pull request description:
Tree-SHA512: 538c43781c789949e1ae566533e76835d478e40e8ba6427b22234ee611cb4a311b2940a214e37c1e9c9afe28a6814a00d490a39e3580bb5ebd85b03e95040246
b191c7dfb7 doc: add comment explaining recentRejects-DoS behavior (James O'Beirne)
Pull request description:
When we receive invalid txs for the first time, we mark the sender as
misbehaving. If we receive the same tx before a new block is seen, we *don't*
punish the second sender (in the same way we do the original sender). It wasn't
initially clear to me that this is intentional, so add a clarifying comment.
Tree-SHA512: d12c674db137ed3ad83e0b941bffe6ddcd2982238048742afa574a4235881f0e58cfc0a4a576a0503e74c5c5240c270b9520fa30221e8b43a371fb3e0b37066b
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>
fa8ced32a60dea37ac169241cf9a1f708ef46c4b doc: Mention blocksonly in reduce-traffic.md, unhide option (MarcoFalke)
fa320de79faaca2b088fcbe7f76701faa9bff236 test: Add test for p2p_blocksonly (MarcoFalke)
fa3872e7b4540857261aed948b94b6b2bfdbc3d1 test: Format predicate source as multiline on error (MarcoFalke)
fa1dce7329d3e74d46ab98b93772b1832a3f1819 net: Rename ::fRelayTxes to ::g_relay_txes (MarcoFalke)
Pull request description:
This is de-facto no longer hidden
ACKs for commit fa8ced:
jamesob:
utACK fa8ced32a6
Tree-SHA512: 474fbdee6cbd035ed9068a066b6056c1f909ec7520be0417820fcd1672ab3069b53f55c5147968978d9258fd3a3933fe1a9ef8e4f6e14fb6ebbd79701a0a1245
Backporting 15654 in 4213 broke devnet connections because of SanitizeString for cleanSubVer. The real issue is using unsafe character in devnet uacomments actually, so to fix this we should replace unsafe `=` with something safe e.g. `.`.
e414486d56b9f06af7aeb07ce13e3c3780c2b69b Do not permit copying FastRandomContexts (Pieter Wuille)
022cf47dd7ef8f46e32a184e84f94d1e9f3a495c Simplify testing RNG code (Pieter Wuille)
fd3e7973ffaaa15ed32e5aeadcb02956849b8fc7 Make unit tests use the insecure_rand_ctx exclusively (Pieter Wuille)
8d98d426116f0178612f14d1874d331042c4c4b7 Bugfix: randbytes should seed when needed (non reachable issue) (Pieter Wuille)
273d02580aa736b7ccea8fce51d90541665fdbd1 Use a FastRandomContext in LimitOrphanTxSize (Pieter Wuille)
3db746beb407f7cdd9cd6a605a195bef1254b4c0 Introduce a Shuffle for FastRandomContext and use it in wallet and coinselection (Pieter Wuille)
8098379be5465f598220e1d6174fc57c56f9da42 Use a local FastRandomContext in a few more places in net (Pieter Wuille)
9695f31d7544778853aa373f0aeed629fa68d85e Make addrman use its local RNG exclusively (Pieter Wuille)
Pull request description:
This improves a few minor issues with the RNG code:
* Avoid calling `GetRand*()` functions (which currently invoke OpenSSL, later may switch to using our own RNG pool) inside loops in addrman, networking code, `KnapsackSolver`, and `LimitOrphanSize`
* Fix a currently unreachable bug in `FastRandomContext::randbytes`.
* Make a number of simplifications to the unit tests' randomness code (some tests unnecessarily used their own RNG or the OpenSSL one, instead of using the unit test specific `insecure_rand_ctx`).
* As a precaution, make it illegal to copy a `FastRandomContext`.
Tree-SHA512: 084c70b533ea68ca7adc0186c39f0b3e0a5c0ae43a12c37286e5d42086e056a8cd026dde61b12c0a296dc80f87fdc87fe303b9e8e6161b460ac2086cf7615f9d
c5b404e8f1973afe071a07c63ba1038eefe13f0f Add functional tests for flexible whitebind/list (nicolas.dorier)
d541fa391844f658bd7035659b5b16695733dd56 Replace the use of fWhitelisted by permission checks (nicolas.dorier)
ecd5cf7ea4c3644a30092100ffc399e30e193275 Do not disconnect peer for asking mempool if it has NO_BAN permission (nicolas.dorier)
e5b26deaaa6842f7dd7c4537ede000f965ea0189 Make whitebind/whitelist permissions more flexible (nicolas.dorier)
Pull request description:
# Motivation
In 0.19, bloom filter will be disabled by default. I tried to make [a PR](https://github.com/bitcoin/bitcoin/pull/16176) to enable bloom filter for whitelisted peers regardless of `-peerbloomfilters`.
Bloom filter have non existent privacy and server can omit filter's matches. However, both problems are completely irrelevant when you connect to your own node. If you connect to your own node, bloom filters are the most bandwidth efficient way to synchronize your light client without the need of some middleware like Electrum.
It is also a superior alternative to BIP157 as it does not require to maintain an additional index and it would work well on pruned nodes.
When I attempted to allow bloom filters for whitelisted peer, my proposal has been NACKed in favor of [a more flexible approach](https://github.com/bitcoin/bitcoin/pull/16176#issuecomment-500762907) which should allow node operator to set fine grained permissions instead of a global `whitelisted` attribute.
Doing so will also make follow up idea very easy to implement in a backward compatible way.
# Implementation details
The PR propose a new format for `--white{list,bind}`. I added a way to specify permissions granted to inbound connection matching `white{list,bind}`.
The following permissions exists:
* ForceRelay
* Relay
* NoBan
* BloomFilter
* Mempool
Example:
* `-whitelist=bloomfilter@127.0.0.1/32`.
* `-whitebind=bloomfilter,relay,noban@127.0.0.1:10020`.
If no permissions are specified, `NoBan | Mempool` is assumed. (making this PR backward compatible)
When we receive an inbound connection, we calculate the effective permissions for this peer by fetching the permissions granted from `whitelist` and add to it the permissions granted from `whitebind`.
To keep backward compatibility, if no permissions are specified in `white{list,bind}` (e.g. `--whitelist=127.0.0.1`) then parameters `-whitelistforcerelay` and `-whiterelay` will add the permissions `ForceRelay` and `Relay` to the inbound node.
`-whitelistforcerelay` and `-whiterelay` are ignored if the permissions flags are explicitly set in `white{bind,list}`.
# Follow up idea
Based on this PR, other changes become quite easy to code in a trivially review-able, backward compatible way:
* Changing `connect` at rpc and config file level to understand the permissions flags.
* Changing the permissions of a peer at RPC level.
ACKs for top commit:
laanwj:
re-ACK c5b404e8f1973afe071a07c63ba1038eefe13f0f
Tree-SHA512: adfefb373d09e68cae401247c8fc64034e305694cdef104bdcdacb9f1704277bd53b18f52a2427a5cffdbc77bda410d221aed252bc2ece698ffbb9cf1b830577
7257353b93 Select orphan transaction uniformly for eviction (Pieter Wuille)
Pull request description:
The previous code was biased towards evicting transactions whose txid has a larger gap (lexicographically) with the previous txid in the orphan pool.
Tree-SHA512: e35f700aea5ed79d1bc57f64bffcb623424b40156fd0a12f05f74f981a8aa4175d5c18d042989243f7559242bdf1d6d720bcf588d28f43d74a798a4843f09c70
Signed-off-by: pasta <pasta@dashboost.org>
f34c8c466a0e514edac2e8683127b4176ad5d321 Make objects in range declarations immutable by default. Avoid unnecessary copying of objects in range declarations. (practicalswift)
Pull request description:
Make objects in range declarations immutable by default.
Rationale:
* Immutable objects are easier to reason about.
* Prevents accidental or hard-to-notice change of value.
Tree-SHA512: cad69d35f0cf8a938b848e65dd537c621d96fe3369be306b65ef0cd1baf6cc0a9f28bc230e1e383d810c555a6743d08cb6b2b0bd51856d4611f537a12e5abb8b
18185b57c32d0a43afeca4c125b9352c692923e9 scripted-diff: batch-recase BanMan variables (Carl Dong)
c2e04d37f3841d109c1fe60693f9622e2836cc29 banman: Add, use CBanEntry ctor that takes ban reason (Carl Dong)
1ffa4ce27d4ea6c1067d8984455df97994c7713e banman: reformulate nBanUtil calculation (Carl Dong)
daae598feb034f2f56e0b00ecfb4854d693d3641 banman: add thread annotations and mark members const where possible (Cory Fields)
84fc3fbd0304a7d6e660bf783c84bed2dd415141 scripted-diff: batch-rename BanMan members (Cory Fields)
af3503d903b1a608cd212e2d74b274103199078c net: move BanMan to its own files (Cory Fields)
d0469b2e9386a7a4b268cb9725347e7517acace6 banman: pass in default ban time as a parameter (Cory Fields)
2e56702ecedd83c4b7cb8de9de5c437c8c08e645 banman: pass the banfile path in (Cory Fields)
4c0d961eb0d7825a1e6f8389d7f5545114ee18c6 banman: create and split out banman (Cory Fields)
83c1ea2e5e66b8a83072e3d5ad6a4ced406eb1ba net: split up addresses/ban dumps in preparation for moving them (Cory Fields)
136bd7926c72659dd277a7b795ea17f72e523338 tests: remove member connman/peerLogic in TestingSetup (Cory Fields)
7cc2b9f6786f9bc33853220551eed33ca6b7b7b2 net: Break disconnecting out of Ban() (Cory Fields)
Pull request description:
**Old English à la Beowulf**
```
Banman wæs bréme --blaéd wíde sprang--
Connmanes eafera Coreum in.
aéglaéca léodum forstandan
Swá bealdode bearn Connmanes
guma gúðum cúð gódum daédum·
dréah æfter dóme· nealles druncne slóg
```
**Modern English Translation**
```
Banman was famed --his renown spread wide--
Conman's hier, in Core-land.
against the evil creature defend the people
Thus he was bold, the son of Connman
man famed in war, for good deeds;
he led his life for glory, never, having drunk, slew
```
--
With @theuni's blessing, here is Banman, rebased. Original PR: https://github.com/bitcoin/bitcoin/pull/11457
--
Followup PRs:
1. Give `CNode` a `Disconnect` method ([source](https://github.com/bitcoin/bitcoin/pull/14605#discussion_r248065847))
2. Add a comment to `std::atomic_bool fDisconnect` in `net.h` that setting this to true will cause the node to be disconnected the next time `DisconnectNodes()` runs ([source](https://github.com/bitcoin/bitcoin/pull/14605#discussion_r248384309))
Tree-SHA512: 9c207edbf577415c22c9811113e393322d936a843d4ff265186728152a67c057779ac4d4f27b895de9729f7a53e870f828b9ebc8bcdab757520c2aebe1e9be35
86d4cf42d97abf4c436d1eabf29e2ed150f69c1e Increase the ip address relay branching factor for unreachable networks (Pieter Wuille)
Pull request description:
Onion addresses propagate very badly among the IPv4/IPv6 network, resulting
in difficulty for those to find each other.
The branching factor 1 is probably so low that propagations die out before
they reach another onion peer. Increase it to 1.5 on average.
ACKs for top commit:
practicalswift:
ACK 86d4cf42d97abf4c436d1eabf29e2ed150f69c1e -- patch looks correct
naumenkogs:
ACK 86d4cf4
jonatack:
ACK 86d4cf42d97abf4c436d1eabf29e2ed150f69c1e. Code review, built and running with some sanity check logging. `RelayAddress()` is called by `ProcessMessage() ADDR` msg handling, from within the loop while processing each new address to relay it to a limited number of other nodes. According to git blame, the line setting `nRelayNodes` hasn't been touched since 2016 in e736772c56 *Move network-msg-processing code out of main to its own file*, which moved the line but otherwise did not change it. Running a mixed clearnet/onion node with this patch and the logging below, I'm only seeing values of `fReachable 1, nRelayNodes 2`. IIUC, I need to use the settings in `init.cpp` that call `SetReachable(*, false)`. *Edit:* with `onlynet=onion` am now seeing entries of `fReachable 0` with `nRelayNodes` values of 1 and 2.
vasild:
ACK 86d4cf42d
Tree-SHA512: 22391e16d60bcfdec9a9336728da39d68a24a183b3d1b0e8fbc038d265ca6ddf71d16db018f3678745fd9f3e9281049e42197fa0a29124833c50a9170ed6f793
dddd9270f85bd2e71fd281a0c6b4053e02fce93c net: Document what happens to getdata of unknonw type (MarcoFalke)
Pull request description:
Any getdata of unknown type will never be processed and blocks all future messages from a peer. This isn't obviously clear from reading the code, so document it.
Top commit has no ACKs.
Tree-SHA512: 4f8e43bbe6534242facfcfffae28b7a6aa2d228841fa2146a87d494e69f614b0da23cf7a5f3d4367358a7c1981fe2ec196a21c437ae1653f1c7e0351be22598a
9e0a514112 Add compile time checking for all cs_main runtime locking assertions (practicalswift)
Pull request description:
Add compile time checking for `cs_main` runtime locking assertions.
This PR is a subset of #12665. The PR was broken up to make reviewing easier.
The intention is that literally all `EXCLUSIVE_LOCKS_REQUIRED`/`LOCKS_EXCLUDED`:s added in this PR should follow either directly or indirectly from `AssertLockHeld(…)`/`AssertLockNotHeld(…)`:s already existing in the repo.
Consider the case where function `A(…)` contains `AssertLockHeld(cs_foo)` (without
first locking `cs_foo` in `A`), and that `B(…)` calls `A(…)` (without first locking `cs_main`):
* It _directly_ follows that: `A(…)` should have an `EXCLUSIVE_LOCKS_REQUIRED(cs_foo)` annotation.
* It _indirectly_ follows that: `B(…)` should have an `EXCLUSIVE_LOCKS_REQUIRED(cs_foo)` annotation.
Tree-SHA512: 120e7410c4c223dbc7d42030b1a19e328d01a55f041bb6fb5eaac10ac35cb0c5d469b9b3bda6444731164c73b88ac6495a00890672b107d9305e891571f64dd6
# Conflicts:
# src/validation.cpp
# src/validation.h
# src/wallet/feebumper.cpp
# src/wallet/rpcwallet.cpp
# src/wallet/wallet.h
# Conflicts:
# src/wallet/wallet.h
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
fa8548c5d1 net: Remove unused unsanitized user agent string CNode::strSubVer (MarcoFalke)
Pull request description:
I fail to see a use case for this unsanitized byte array. In fact this can easily be confused with `cleanSubVer` and be displayed to the user (or logged) by a simple typo that is hard to find in review.
Further reading: https://btcinformation.org/en/developer-reference#version
ACKs for commit fa8548:
promag:
utACK fa8548c, good catch.
practicalswift:
utACK fa8548c5d13957f57f9b1e20e03002600962f7f0
sipa:
utACK fa8548c5d13957f57f9b1e20e03002600962f7f0
Tree-SHA512: 3c3ff1504d1583ad099df9a6aa761458a82ec48a58ef7aaa9b5679a5281dd1b59036ba2932ed708488951a565b669a3083ef70be5a58472ff8677b971162ae2f
ef0019e054734a14214dfbce56611ce4db1688a5 Generate log entry when blocks messages are received unexpectedly. (Patrick Strateman)
Pull request description:
Currently these are incorrectly logged as an unknown command.
Tree-SHA512: dd272388a90b79897f8c1ea6d4c949323fcf75493f3a5b2ec9a26a2cf6a8ee743b497941702f21df8fae0f5b9481444363643379832dbd5053b0cc0b0363de04
c3f34d06be Make it clear which functions that are intended to be translation unit local (practicalswift)
Pull request description:
Make it clear which functions that are intended to be translation unit local.
Do not share functions that are meant to be translation unit local with other translation units. Use internal linkage for those consistently.
Tree-SHA512: 05eebd233d5cfbf6116724eec3a99b465bf534ca220f2b6f5e56341a7da41387454d3cb6ceadd8ab6714a5df94069e4ad0dcab8801ccc7e8949be7199a19fb53
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
Comment from 6f7b52ac63a71d2706022ca58d69a1a622e0fa37: "The fix for CPubKey is a part of `#13557: BIP 174 PSBT Serializations and RPCs` which wasn't backported yet"
* 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>
* 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>
* llmq: Return bool in EnsureQuorumConnections
* llmq: Call RemoveMasternodeQuorumNodes directly
Instead of removing the good nodes from the vector in first loop and
call it with the bad ones in the second loop.
* refactor: Introduce and use CLLMQUtils::IsWatchQuorumsEnabled
* llmq: Move IsWatchQuorumsEnabled into EnsureQuorumConnections
* Fix extra line break in CommitTransaction log message
Introduced in #3680ebe7e80a49 (diff-b2bb174788c7409b671c46ccc86034bdR4113)
* doc: Fix `quorum sign` help
* doc: Add `getdata` to quorum commands list help
* doc: Drop "P2WSH" from signrawtransactionwithkey help
* trivial: Replace "push_back(Pair(..))" with "pushKV"
* trivial: Reorder wallet cmd-line options
* git: Add macos debug simbols to .gitignore
* trivial: Fix typos and whitespaces, drop unused stuff
* partial backport 18544: net: limit BIP37 filter lifespan (active between 'filterload'..'filterclear')
Previously, a default match-everything bloom filter was set for every peer,
i.e. even before receiving a 'filterload' message and after receiving a
'filterclear' message code branches checking for the existence of the filter
by testing the pointer "pfilter" were _always_ executed.
* net: Match the backport PR a bit more
Co-authored-by: xdustinface <xdustinfacex@gmail.com>
* version: Bump PROTOCOL_VERSION and MIN_MASTERNODE_PROTO_VERSION
* version: Introduce LLMQ_DATA_MESSAGES_VERSION for QGETDATA/QDATA support
* test: Bump MY_VERSION to 70219 (LLMQ_DATA_MESSAGES_VERSION)
* llmq: Introduce CQuorumDataRequest as wrapper for QGETDATA requests
* llmq: Implement CQuorum::{SetVerificationVector, SetSecretKeyShare}
* llmq|net|protocol: Implement QGETDATA/QDATA P2P messages
* llmq: Restrict processing QGETDATA/QDATA to masternodes only
* llmq: Implement request limiting for QGETDATA/QDATA
* llmq: Implement CQuorumManger::RequestQuorumData
* rpc: Implement "quorum getdata" as wrapper around QGETDATA
Allows to trigger sending QGETDATA messages to connected peers by RPC.
* test: Handle QGETDATA/QDATA messages in mininode
* test: Add data structures to support QGETDATA/QDATA
* test: Add some helper in test_framework.py
* test: Implement tests for QGETDATA/QDATA in p2p_quorum_data.py
* test: Add p2p_quorum_data.py to BASE_SCRIPTS
* llmq|test: Add QWATCH support for QGETDATA/QDATA
* llmq: Store CQuorumPtr in cache, not CQuorumCPtr
* llmq: Fix cache usage after recent changes
* Use uacomment to create/find specific p2ps
* No need to use network adjusted time here, GetTime should be enough
* rpc: check proTxHash
* minor tweaks
* test: Adjustments after 4e27d6513e
* llmq: Rename and improve error lambda in CQuorumManager::ProcessMessage
* llmq: Process QDATA if -watchquorums is enabled
* test: Handle qwatch messages in mininode
* test: Add test for -watchquorums support
* test: Just some empty lines
* test: Properly stop the p2p network thread at the end of the test
* rpc: Adjust "quorum getdata" parameter descriptions
Co-authored-by: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com>
* rpc: Fix optionality of proTxHash in "quorum getdata" command
* test: Test optionality of proTxHash for "quorum getdata" command
* test: Be more specific about imports in p2p_quorum_data.py
* llmq|rpc: Add some comments about the request.GetDataMask checks
* test: Some more empty lines
* rpc: One more parameter description
Co-authored-by: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com>
* test: Unify assert statements / drop parentheses for all of them
* fix typo
Signed-off-by: pasta <pasta@dashboost.org>
* adjust some line wrapping to 80 chars
Signed-off-by: pasta <pasta@dashboost.org>
* tests: Seperate out into dif atomic methods, add logging
Signed-off-by: pasta <pasta@dashboost.org>
* test: Avoid restarting masternodes, just let available requests expire
Just takes a lot time and isn't required imo.
* test: Drop redundant code/tests after separation
This was introduced in 9e224ec2f2
* test: Merge three tests
"test_mnauth_restriction", "test_invalid_messages" and "test_invalid_unexpected_qdata" with the resulting name "test_basics" because i don't feel like DKG recovery thing should be part of a test called "test_invalid_messages" and giving it an own test probably wouldn't make a lot sense because it would still depend on "test_invalid_messages". I also think there is no need for a separated "test_invalid_unexpected_qdata".
* test: Rename test_ratelimiting_banscore -> test_request_limit
* test: Apply python style
* test: Wrap all at 120 characters
Thats the default "draw annoying warnings" setting for PyCharm (and IMO a reasonable line length).
* test: Move some variables
* test: Optimize for speed
* tests: use wait_until in get_mininode_id
* test: Don't use `!=` to check for `None`
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
Co-authored-by: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com>
Co-authored-by: pasta <pasta@dashboost.org>
* llmq: Drop hash parameter in PreVerifyMessage methods
* llmq: Drop some unused variables
* rpc: Drop unused variable
* llmq|net: Drop some unused CConnman parameter
* llmq: Drop some unused quorum parameter
* llmq: Drop some unused nodeId parameter
* Drop unused variables
* llmq: Drop more
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
* Backport Statoshi
This backports some of https://github.com/jlopp/statoshi.
Missing stuff: README.md and client name changes, segwit and fee estimation stats.
Fix RejectCodeToString
Fix copy-paste mistake s/InvalidBlockFound/InvalidChainFound/
* Merge #16728: move-only: move coins statistics utils out of RPC
8a3b2eb17572ca2131778d52cc25ec359470a90f move-only: move coins statistics utils out of RPC (James O'Beirne)
Pull request description:
This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11):
Parent PR: #15606
Issue: #15605
Specification: https://github.com/jamesob/assumeutxo-docs/tree/master/proposal
---
In the short-term, this move-only commit will help with fuzzing (https://github.com/bitcoin/bitcoin/pull/15606#issuecomment-524482297). Later, these procedures will be used to compute statistics (particularly a content hash) for UTXO sets coming in from snapshots.
Most easily reviewed with `git ... --color-moved=dimmed_zebra`. A nice follow-up would be adding unittests, which I'll do if nobody else gets around to it.
ACKs for top commit:
MarcoFalke:
ACK 8a3b2eb17572ca2131778d52cc25ec359470a90f, checked --color-moved=dimmed-zebra
Tree-SHA512: a187d2f7590ad2450b8e8fa3d038c80a04fc3d903618c24222d7e3172250ce51badea35860c86101f2ba266eb4354e6efb8d7d508b353f29276e4665a1efdf74
* Fix 16728
* Modernize StatsdClient
- Reuse some functionality from netbase
- Switch from GetRand to FastRandomContext
- Drop `using namespace std` and add `// namespace statsd`
* Introduce PeriodicStats and make StatsdClient configurable via -stats<smth> (enabled/host/port/ns/period)
* Move/rename tip stats from CheckBlock to ConnectBlock
* Add new false positives to lint-format-strings.py
* Add snprintf in statsd_client to the list of known violations in lint-locale-dependence.sh
* Fix incorrect include guard
* Use bracket syntax includes
* Replace magic numbers with defaults
* Move connection stats calculation into its own function
And bail out early if stats are disabled
* assert in PeriodicStats
Co-authored-by: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com>
Co-authored-by: MarcoFalke <falke.marco@gmail.com>
Co-authored-by: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com>
e4d0b44 Consistently log CValidationState on failure (Ben Woosley)
Pull request description:
This replaces potential silent failures and partial logging with full logging. Seems providing at least minimal visibility to the failure is a good practice. E.g. `FlushStateToDisk` can return a rare but meaningful out of disk space error that would be better to note than leave out.
Note many of these are related to `ActivateBestChain` or `FlushStateToDisk`. Only a few cases of ignored state remain, e.g. LoadExternalBlockFile and RelayWalletTransaction, where I expect logging would likely be spammy.
Tree-SHA512: fb0e521039e5a5250cd9c82e7a8676423b5e3899d495649c0e71752059d1984e5175f556386ade048f51a7d59f5c8e467df7fe91d746076f97d24c000ccf7891
13020 continued
Signed-off-by: pasta <pasta@dashboost.org>
* Merge #13622: Remove mapRequest tracking that just effects Qt display.
beef7ec4be725beea870a2da510d2817487601ec Remove useless mapRequest tracking that just effects Qt display. (Matt Corallo)
Pull request description:
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.
Tree-SHA512: c9d0808fb60146919bb78d0860ec2193601966c944887eaae7837408422f7e85dfdb306407a613200cdd4726aec66da18df618ebc6a8cfe8650bf08d4a8dc155
Signed-off-by: pasta <pasta@dashboost.org>
# Conflicts:
# src/interfaces/wallet.cpp
# src/interfaces/wallet.h
# src/net_processing.cpp
# src/qt/guiconstants.h
# src/qt/transactiondesc.cpp
# src/qt/transactionrecord.cpp
# src/qt/transactionrecord.h
# src/qt/transactiontablemodel.cpp
# src/validationinterface.cpp
# src/validationinterface.h
# src/wallet/wallet.cpp
* drop TX_STATUS_OFFLINE
Signed-off-by: pasta <pasta@dashboost.org>
Co-authored-by: Wladimir J. van der Laan <laanwj@gmail.com>
4b75dcf devtools: Make linter check LogPrint calls (MarcoFalke)
ff2ad2d Add missing newlines to LogPrint debug logging (Wladimir J. van der Laan)
Pull request description:
~~Don't we have a linter that should catch these?~~
Tree-SHA512: 1a58eca01ded9c1719e943c09447deeb59bb06dba00528cf460eefe857fdf95b42671fbdebc87cdd2f51e931e86942d06587ffd097cbb0d8dd9eb7a0ba17a8f0
4b6ab02122d225818f6c6a4b4ef343d2c90e6d55 Remove unused argument to ProcessGetBlockData(...) (practicalswift)
c469ecf22e8ab55acf2cf8fc6fcf34415b541f76 net: Remove unused interrupt from SendMessages (fanquake)
Pull request description:
Discussed very briefly with cfields.
Includes 65b4400 from #13554 as it's a similar refactor.
Tree-SHA512: 45cd64208a5c8164242db74e6687e9344ea592bab5e7f9ba8e1bb449057fc908ec9d8b8523748a68426e4a4304e3388a138cd834698b39837b2149b72beefdc9
e56771365b Do not use uppercase characters in source code filenames (practicalswift)
419a1983ca docs: Add a note about the source code filename naming convention (practicalswift)
Pull request description:
Add a note about the source code filename naming convention.
Tree-SHA512: 8d329bd9e19bcd26e74b0862fb0bc2369b46095dbd3e69d34859908632763abd7c3d00ccc44ee059772ad4bae4460c2bcc1c0e22fd9d8876d57e5fcd346cea4b
* Revert "Only call SendMessages when ProcessMessages did some work or when some time passed"
This reverts commit 24ead62905.
* Only skip SendMessages() for fMasternode connections
b7cd08b71 Add documentation to PeerLogicValidation interface and related functions (James O'Beirne)
Pull request description:
Adds docs for PeerLogicValidation's public interface and two related functions.
Tree-SHA512: b4c2f47e9baa9396d2b6faf3792e46b371c50cd91b9ac890f263f4d14eb24a71e7b40ceb4cbb41e254f5008eff357f417b842618e7ebece9039802ab2a5dd728
4d9b4256d8 Fix typos (Dimitris Apostolou)
Pull request description:
Unfortunately I messed up my repo while trying to squash #12593 so I created a PR with just the correct fixes.
Tree-SHA512: 295d77b51bd2a9381f1802c263de7ffb2edd670d9647391e32f9a414705b3c8b483bb0e469a9b85ab6a70919ea13397fa8dfda2aea7a398b64b187f178fe6a06
Signed-off-by: pasta <pasta@dashboost.org>
9ad6746ccd Use static_cast instead of C-style casts for non-fundamental types (practicalswift)
Pull request description:
A C-style cast is equivalent to try casting in the following order:
1. `const_cast(...)`
2. `static_cast(...)`
3. `const_cast(static_cast(...))`
4. `reinterpret_cast(...)`
5. `const_cast(reinterpret_cast(...))`
By using `static_cast<T>(...)` explicitly we avoid the possibility of an unintentional and dangerous `reinterpret_cast`. Furthermore `static_cast<T>(...)` allows for easier grepping of casts.
For a more thorough discussion, see ["ES.49: If you must use a cast, use a named cast"](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es49-if-you-must-use-a-cast-use-a-named-cast) in the C++ Core Guidelines (Stroustrup & Sutter).
Tree-SHA512: bd6349b7ea157da93a47b8cf238932af5dff84731374ccfd69b9f732fabdad1f9b1cdfca67497040f14eaa85346391404f4c0495e22c467f26ca883cd2de4d3c
66b3fc5437 Skip stale tip checking if outbound connections are off or if reindexing. (Gregory Maxwell)
Pull request description:
I got tired of the pointless stale tip notices in reindex and on nodes with connections disabled.
Tree-SHA512: eb07d9c5c787ae6dea02cdd1d67a48a36a30adc5ccc74d6f1c0c7364d404dc8848b35d2b8daf5283f7c8f36f1a3c463aacb190d70a22d1fe796a301bb1f03228
fa74d3d720 qa: Remove unused deserialization code in msg_version (MarcoFalke)
fa5099ceb7 p2p: Remove dead code for nVersion=10300 (MarcoFalke)
Pull request description:
This code is undocumented and confusing as well as dead, since peers with a version that old are disconnected immediately.
Tree-SHA512: 58c131a2730b630ffdc191cd65fe736ed1bd57e184902e2af1b1399443c4654617e68774432016df023434055e85d2e8cd32fb03b40c508c3bb8db6d19427434
e254ff5d53b79bee29203b965fca572f218bff54 Introduce a maximum size for locators. (Gregory Maxwell)
Pull request description:
The largest sensible size for a locator is log in the number of blocks.
But, as noted by Coinr8d on BCT a maximum size message could encode a
hundred thousand locators. If height were used to limit the messages
that could open new attacks where peers on long low diff forks would
get disconnected and end up stuck.
Ideally, nodes first first learn to limit the size of locators they
send before limiting what would be processed, but common implementations
back off with an exponent of 2 and have an implicit limit of 2^32
blocks, so they already cannot produce locators over some size.
Locators are cheap to process so allowing a few more is harmless,
so this sets the maximum to 64-- which is enough for blockchains
with 2^64 blocks before the get overhead starts increasing.
Tree-SHA512: da28df9c46c988980da861046c62e6e7f93d0eaab3083d32e408d1062f45c00316d5e1754127e808c1feb424fa8e00e5a91aea2cc3b80326b71c148696f7cdb3
1e3bcd251768baeb95e555d51d2dc787a6b2acee [net_processing] Add thread safety annotations (Jesse Cohen)
f393a533bebc088985f94c725b9af881500ba998 Annotate AssertLockHeld() with ASSERT_CAPABILITY() for thread safety analysis (Jesse Cohen)
Pull request description:
(note that this depends on #13417)
This commit fully annotates all globals in net_processing with clang thread safety annotations. Subsequent commits will begin transitioning some of this data away from cs_main into locks that are local to net_processing. Static thread safety analysis should it easier to verify correctness of that process.
Tree-SHA512: b47aa410cb9ada21072370176aea9a74c575643fa1ee8cf1d43c8e28675eef17f33e5242ac422f840e8178e132ecb58412034c6334b68f1b57c686df80d4e8e2
3339ba28e9 Make g_enable_bip61 a member variable of PeerLogicValidation (Jesse Cohen)
6690a28606 Restrict as much as possible in net_processing to translation unit (Jesse Cohen)
1d4df02b7e [move-only] Move things only referenced in net_processing out of header file (Jesse Cohen)
02bbc05310 Rescope g_enable_bip61 to net_processing (Jesse Cohen)
Pull request description:
As part of a larger effort to decouple net_processing and validation a bit, these are a bunch of simple scope cleanups. I've moved things out of the header file that are only referenced in net_processing and added static (or anonymous namespace) modifiers to everything possible in net_processing.
There are a handful of functions which could be static except that they are exposed for the sake of unit testing - these are explicitly commented. There has been some discussion of a compile time annotation, but no conclusion has been reached on that yet.
This is somewhat related to other prs #12934#13413#13407 and will be followed by prs that reduce reliance on cs_main to synchronize data structures which are translation unit local to net_processing
Tree-SHA512: 46c9660ee4e06653feb42ba92189565b0aea17aac2375c20747c0d091054c63829cbf66d2daddf65682b58ce1d6922e23aefea051a7f2c8abbb6db253a609082
Signed-off-by: Pasta <pasta@dashboost.org>
# Conflicts:
# src/init.cpp
# src/net_processing.cpp
# src/net_processing.h
# src/test/test_dash.cpp
77a733a99 [tests] Add additional unit tests for -nofoo edge cases (Anthony Towns)
af173c2be [tests] Check GetChainName works with config entries (Anthony Towns)
fa27f1c23 [tests] Add unit tests for ReadConfigStream (Anthony Towns)
087c5d204 ReadConfigStream: assume the stream is good (Anthony Towns)
6d5815aad Separate out ReadConfigStream from ReadConfigFile (Anthony Towns)
834d30341 [tests] Add unit tests for GetChainName (Anthony Towns)
11b6b5b86 Move ChainNameFromCommandLine into ArgsManager and rename to GetChainName (Anthony Towns)
Pull request description:
This does a bit of refactoring of the configuration handling code in order to add additional tests to make adding support for [test]/[regtest] sections in the config file in #11862 easier. Should not cause any behaviour changes.
Tree-SHA512: 8d2ce1449fc180de03414e7e569d1a21ba1e9f6564e13d3faf3961f710adc725fa0d4ab49b89ebd2baa11ea36ac5018377f693a84037d386a8b8697c9d6db3e9
* Merge #10387: Eventually connect to NODE_NETWORK_LIMITED peers
eb91835 Add setter for g_initial_block_download_completed (Jonas Schnelli)
3f56df5 [QA] add NODE_NETWORK_LIMITED address relay and sync test (Jonas Schnelli)
158e1a6 [QA] fix mininode CAddress ser/deser (Jonas Schnelli)
fa999af [QA] Allow addrman loopback tests (add debug option -addrmantest) (Jonas Schnelli)
6fe57bd Connect to peers signaling NODE_NETWORK_LIMITED when out-of-IBD (Jonas Schnelli)
31c45a9 Accept addresses with NODE_NETWORK_LIMITED flag (Jonas Schnelli)
Pull request description:
Eventually connect to peers signalling NODE_NETWORK_LIMITED if we are out of IBD.
Accept and relay NODE_NETWORK_LIMITED peers in addrman.
Tree-SHA512: 8a238fc97f767f81cae1866d6cc061390f23a72af4a711d2f7158c77f876017986abb371d213d1c84019eef7be4ca951e8e6f83fda36769c4e1a1d763f787037
Signed-off-by: Pasta <pasta@dashboost.org>
# Conflicts:
# src/init.cpp
# src/protocol.h
# test/functional/node_network_limited.py
* remove witness
Signed-off-by: Pasta <pasta@dashboost.org>
* fix test expecting witness flag
Signed-off-by: Pasta <pasta@dashboost.org>
Co-authored-by: Wladimir J. van der Laan <laanwj@gmail.com>
* Merge #13134: net: Add option `-enablebip61` to configure sending of BIP61 notifications
87fe292d897e09e176ac7e254144466c319cc9ac doc: Mention disabling BIP61 in bips.md (Wladimir J. van der Laan)
fe16dd8226d924f44432c5b5014aa49ff45c82ff net: Add option `-enablebip61` to configure sending of BIP61 notifications (Wladimir J. van der Laan)
Pull request description:
This commit adds a boolean option `-peersendreject`, defaulting to `1`, that can be used to disable the sending of [BIP61](https://github.com/bitcoin/bips/blob/master/bip-0061.mediawiki) `reject` messages. This functionality has been requested for various reasons:
- security (DoS): reject messages can reveal internal state that can be used to target certain resources such as the mempool more easily.
- bandwidth: a typical node sends lots of reject messages; this counts against upstream bandwidth. Also the reject messages tend to be larger than the message that was rejected.
On the other hand, reject messages can be useful while developing client software (I found them indispensable while creating bitcoin-submittx), as well as for our own test cases, so whatever the default becomes on the long run, IMO the functionality should be retained as option. But that's a discussion for later, for now it's simply a node operator decision.
Also adds a RPC test that checks the functionality.
Tree-SHA512: 9488cc53e13cd8e5c6f8eb472a44309572673405c1d1438c3488f627fae622c95e2198bde5ed7d29e56b948e2918bf1920239e9f865889f4c37c097c37a4d7a9
* 0.17 -> 0.16
Signed-off-by: Pasta <pasta@dashboost.org>
* tx1 -> base_ tx fixing 13134
Signed-off-by: Pasta <pasta@dashboost.org>
* move added bip61 message checking up
Signed-off-by: Pasta <pasta@dashboost.org>
* Dash specific code, only send reject messages if bip61 is enabled
Signed-off-by: Pasta <pasta@dashboost.org>
* Fix invalidtxrequest.py
Co-authored-by: Wladimir J. van der Laan <laanwj@gmail.com>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
* Merge #13151: net: Serve blocks directly from disk when possible
0bf431870e45d8e20c4671e51a782ebf97b75fac net: Serve blocks directly from disk when possible (Wladimir J. van der Laan)
Pull request description:
In `ProcessGetBlockData`, send the block data directly from disk if type MSG_WITNESS_BLOCK is requested. This is a valid shortcut as the on-disk format matches the network format.
This is expected to increase performance because a deserialization and subsequent serialization roundtrip is avoided.
Tree-SHA512: 9a9500b4c1354eaae1a6f1c6ef2416c1c1985029852589266f3a70e808f6c7482c135e9ab251a527566935378ab7c32dba4ed43ba5451e802d8e72b77d1ba472
Signed-off-by: Pasta <pasta@dashboost.org>
# Conflicts:
# src/net_processing.cpp
# src/validation.cpp
* fix extra bracket
Signed-off-by: Pasta <pasta@dashboost.org>
* remove read raw block from disk functions
Signed-off-by: Pasta <pasta@dashboost.org>
* correct `if (pblock)` scope
Signed-off-by: Pasta <pasta@dashboost.org>
Co-authored-by: Wladimir J. van der Laan <laanwj@gmail.com>
This was only used in only one remaining place and only to ensure that
reject messages are sent before closing sockets. This is solved by the
previous commit now.
1a8f0d5a74d5cc0000456932babf35301f5c1686 [tools] update nNextInvSend to use mockable time (Amiti Uttarwar)
4de630354fc6808b9b13b9e82da1a82f2f50f26a [tools] add PoissonNextSend method that returns mockable time (Amiti Uttarwar)
Pull request description:
Introduce a Poisson helper method that wraps the existing method to return `std::chrono::duration` type, which is mockable.
Needed for https://github.com/bitcoin/bitcoin/pull/16698.
ACKs for top commit:
ajtowns:
ACK 1a8f0d5a74d5cc0000456932babf35301f5c1686
MarcoFalke:
re-ACK 1a8f0d5a74d5cc0000456932babf35301f5c1686
naumenkogs:
ACK 1a8f0d5, and let's merge it and come back to it later.
Tree-SHA512: 7e2325d7c55fc0b4357cb86b83e0c218ba269f678c1786342d8bc380bfd9696373bc24ff124b9ff17a6e761c62b2b44ff5247c3911e2afdc7cc5c20417e8290b
d45b344ffd Bucket for inbound when scheduling invs to hide tx time (Gleb)
Pull request description:
It has been brought up to my attention that current random delays mechanism (originally intended to obfuscate transaction metadata) allows to easily estimate the time a transaction was received by a node.
It may be done by connecting multiple observer nodes to the same node. Each of those nodes will generate its own schedule of delays. Combined metadata regarding those events from different sources allows an observer to estimate transaction time.
After this patch a spy won't gain additional information by just creating multiple connections to a target.
Tree-SHA512: c71dae5ff350b614cb40a8e201fd0562d3e03e3e72a5099718cd451f0d84c66d5e52bbaf0d5b4b75137514c8efdedcc6ef4df90142b360153f04ad0721545ab1
308b76732f Fix bug around transaction requests (Suhas Daftuar)
f635a3ba11 Expire old entries from the in-flight tx map (Suhas Daftuar)
e32e08407e Remove NOTFOUND transactions from in-flight data structures (Suhas Daftuar)
23163b7593 Add an explicit memory bound to m_tx_process_time (Suhas Daftuar)
218697b645 Improve NOTFOUND comment (Suhas Daftuar)
Pull request description:
#14897 introduced several bugs that could lead to a node no longer requesting transactions from one or more of its peers. Credit to ajtowns for originally reporting many of these bugs along with an originally proposed fix in #15776.
This PR does a few things:
- Fix a bug in NOTFOUND processing, where the in-flight map for a peer was keeping transactions it shouldn't
- Eliminate the possibility of a memory attack on the CNodeState `m_tx_process_time` data structure by explicitly bounding its size
- Remove entries from a peer's in-flight map after 10 minutes, so that we should always eventually resume transaction requests even if there are other bugs like the NOTFOUND one
- Fix a bug relating to the coordination of request times when multiple peers announce the same transaction
The expiry mechanism added here is something we'll likely want to remove in the future, but is belt-and-suspenders for now to try to ensure we don't have other bugs that could lead to transaction relay failing due to some unforeseen conditions.
ACKs for commit 308b76:
ajtowns:
utACK 308b76732f97020c86977e29c854e8e27262cf7c
morcos:
light ACK 308b767
laanwj:
Code review ACK 308b76732f97020c86977e29c854e8e27262cf7c
jonatack:
Light ACK 308b76732f97020c86977e29c854e8e27262cf7c.
jamesob:
ACK 308b76732f
MarcoFalke:
ACK 308b76732f97020c86977e29c854e8e27262cf7c (Tested two of the three bugs this pull fixes, see comment above)
jamesob:
Concept ACK 308b76732f
MarcoFalke:
ACK 308b76732f
Tree-SHA512: 8865dca5294447859d95655e8699085643db60c22f0719e76e961651a1398251bc932494b68932e33f68d4f6084579ab3bed7d0e7dd4ac6c362590eaf9414eda
1cff3d6cb0 Change in transaction pull scheduling to prevent InvBlock-related attacks (Gleb Naumenko)
Pull request description:
This code makes executing two particular (and potentially other) attacks harder.
### InvBlock
This behavior was described well [here](https://www.cs.umd.edu/projects/coinscope/coinscope.pdf) (page 11).
Per current implementation, if node A receives _INV_ (tx) from node B, node A sends _GETDATA_ to B and waits for _TX_ message back.
Node A is likely to receive more _INVs_ (regarding the same tx) from other peers. But node A would not send another _GETDATA_ unless it does not hear _TX_ back from node B for next 2 minutes (to save bandwidth)
Thus, if B is a malicious node, it can prevent node A from getting the transaction (even if all A’s peers have it) for 2 minutes.
This behavior seems to be an inherent limitation of the current P2P relay protocol, and I don’t see how it can be fundamentally changed (I can see workarounds which involve rewriting a lot of P2P code though).
### What does this PR fix?
The attacks I’m looking at involve preventing A from learning the transaction for 2*N minutes. To do that, an attacker has to spin up N nodes and send N _INVs_ simultaneously to node A (then InvBlocks will be queued with an interval of 2 minutes according to current implementation)
More precisely, 2 scenarios I’m looking at are:
1. An attacker censors a particular transaction. By performing InvBlock from different nodes, an attacker can execute a network-wide censorship of a particular transaction (or all transactions). The earlier an attacker founds the transaction he wants to censor, the easier it is to perform an attack. As it was pointed out by @gwillen, this is even more dangerous in the case of lightning, where transactions are known in advance.
2. Topology inference described in papers [1](https://www.cs.umd.edu/projects/coinscope/coinscope.pdf), [2](https://arxiv.org/pdf/1812.00942.pdf) involve network-wide InvBlock. This fix would not mitigate this type of inference, but I believe it will make it more expensive to perform (an attacker would have to create more transactions and perform more rounds to learn the topology, the second paper itself notes that InvBlock isolation is important for the attack).
### How does it work
This PR introduces bias toward outbound connections (they have higher priority when a node chooses from whom it should request a transaction) and randomizes the order.
As per @gmaxwell suggestion, GETDATA requests queue is created after processing all incoming messages from all nodes.
After this fix, if the incoming messages were [I1, I2, I3, O1, O2, O3, O4], the queue for _GETDATA_ may look like [O2, O1, O3, O4, I1, I3, I2, ….].
If {I1, I2, I3} were significantly earlier (but the difference is less than TX_TIMEOUT=60 s) than others, the queue for _GETDATA_ may look like [I2, O2, O1, O3, O4, I1, I3, ….].
### Other comments:
1. This mitigation works better if the connectivity is higher (especially outbound, because it would be less likely that 2 _GETDATAs_ for inbound malicious nodes queued together)
Tree-SHA512: 2ad1e80c3c7e16ff0f2d1160aa7d9a5eaae88baa88467f156b987fe2a387f767a41e11507d7f99ea02ab75e89ab93b6a278d138cb1054f1aaa2df336e9b2ca6a
a720b92 Remove includes in .cpp files for things the corresponding .h file already included (practicalswift)
Pull request description:
Remove includes in .cpp files for things the corresponding .h file already included.
Example case:
* `addrdb.cpp` includes `addrdb.h` and `fs.h`
* `addrdb.h` includes `fs.h`
Then remove the direct inclusion of `fs.h` in `addrman.cpp` and rely on the indirect inclusion of `fs.h` via the included `addrdb.h`.
In line with the header include guideline (see #10575).
Tree-SHA512: 8704b9de3011a4c234db336a39f7d2c139e741cf0f7aef08a5d3e05197e1e18286b863fdab25ae9638af4ff86b3d52e5cab9eed66bfa2476063aa5c79f9b0346
* scripted-diff: Replace #include "" with #include <> (ryanofsky)
-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-
Signed-off-by: Pasta <pasta@dashboost.org>
* scripted-diff: Replace #include "" with #include <> (Dash Specific)
-BEGIN VERIFY SCRIPT-
for f in \
src/bls/*.cpp \
src/bls/*.h \
src/evo/*.cpp \
src/evo/*.h \
src/governance/*.cpp \
src/governance/*.h \
src/llmq/*.cpp \
src/llmq/*.h \
src/masternode/*.cpp \
src/masternode/*.h \
src/privatesend/*.cpp \
src/privatesend/*.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-
Signed-off-by: Pasta <pasta@dashboost.org>
* build: Remove -I for everything but project root
Remove -I from build system for everything but the project root,
and built-in dependencies.
Signed-off-by: Pasta <pasta@dashboost.org>
# Conflicts:
# src/Makefile.test.include
* qt: refactor: Use absolute include paths in .ui files
* qt: refactor: Changes to make include paths absolute
This makes all include paths in the GUI absolute.
Many changes are involved as every single source file in
src/qt/ assumes to be able to use relative includes.
Signed-off-by: Pasta <pasta@dashboost.org>
# Conflicts:
# src/qt/dash.cpp
# src/qt/optionsmodel.cpp
# src/qt/test/rpcnestedtests.cpp
* test: refactor: Use absolute include paths for test data files
* Recommend #include<> syntax in developer notes
* refactor: Include obj/build.h instead of build.h
* END BACKPORT #11651 Remove trailing whitespace causing travis failure
* fix backport 11651
Signed-off-by: Pasta <pasta@dashboost.org>
* More of 11651
* fix blockchain.cpp
Signed-off-by: pasta <pasta@dashboost.org>
* Add missing "qt/" in includes
* Add missing "test/" in includes
* Fix trailing whitespaces
Co-authored-by: Wladimir J. van der Laan <laanwj@gmail.com>
Co-authored-by: Russell Yanofsky <russ@yanofsky.org>
Co-authored-by: MeshCollider <dobsonsa68@gmail.com>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
bf64c3cb3 Ignore transactions added to mempool during a reorg for fee estimation purposes. (Alex Morcos)
04f78ab5b Do not reject based on mempool min fee when bypass_limits is set. (Alex Morcos)
fd849e1b0 Change AcceptToMemoryPool function signature (Alex Morcos)
Pull request description:
First commit just removes default arguments from `AcceptToMemoryPool` and consolidates two arguments, it does not change behavior.
Second commit finally fixes the fact that we're not meant to reject based on mempool min fee when adding a transaction from a disconnected block during a reorg as mentioned [here](https://github.com/bitcoin/bitcoin/pull/9602#issue-202197849)
Third commit makes fee estimation ignore transactions added from a disconnected block during a reorg. I think this was another source of fee estimates returning estimates below 1000 sat/kB as in #11303.
Tree-SHA512: 30925ca8b341915bb214f1d2590b36b7931f2e125b7660150e38ae70338f00db5aa7f1608546dddb181446924177eb7cf62ea8bd2583068acc074d6c3f86bc0c
fix 11309
Signed-off-by: Pasta <pasta@dashboost.org>
fix &
Signed-off-by: Pasta <pasta@dashboost.org>
a357293 Use MakeUnique<Db>(...) (practicalswift)
3e09b39 Use MakeUnique<T>(...) instead of std::unique_ptr<T>(new T(...)) (practicalswift)
8617989 Add MakeUnique (substitute for C++14 std::make_unique) (practicalswift)
d223bc9 Use unique_ptr for pcoinscatcher/pcoinsdbview/pcoinsTip/pblocktree (practicalswift)
b45c597 Use unique_ptr for pdbCopy (Db) and fix potential memory leak (practicalswift)
29ab96d Use unique_ptr for dbenv (DbEnv) (practicalswift)
f72cbf9 Use unique_ptr for pfilter (CBloomFilter) (practicalswift)
8ccf1bb Use unique_ptr for sem{Addnode,Outbound} (CSemaphore) (practicalswift)
73db063 Use unique_ptr for upnp_thread (boost::thread) (practicalswift)
0024531 Use unique_ptr for dbw (CDBWrapper) (practicalswift)
fa6d122 Use unique_ptr:s for {fee,short,long}Stats (TxConfirmStats) (practicalswift)
5a6f768 Use unique_ptr for httpRPCTimerInterface (HTTPRPCTimerInterface) (practicalswift)
860e912 Use unique_ptr for pwalletMain (CWallet) (practicalswift)
Pull request description:
Use `std::unique_ptr` (C++11) where possible.
Rationale:
1. Avoid resource leaks (specifically: forgetting to `delete` an object created using `new`)
2. Avoid undefined behaviour (specifically: double `delete`:s)
**Note to reviewers:** Please let me know if I've missed any obvious `std::unique_ptr` candidates. Hopefully this PR should cover all the trivial cases.
Tree-SHA512: 9fbeb47b800ab8ff4e0be9f2a22ab63c23d5c613a0c6716d9183db8d22ddbbce592fb8384a8b7874bf7375c8161efb13ca2197ad6f24b75967148037f0f7b20c
fbf327b Minimal code changes to allow msvc compilation. (Aaron Clauson)
Pull request description:
These changes are required to allow the Bitcoin source to build with Microsoft's C++ compiler (#11562 is also required).
I looked around for a better place for the typedef of ssize_t which is in random.h. The best candidate looks like src/compat.h but I figured including that header in random.h is a bigger change than the typedef. Note that the same typedef is in at least two other places including the OpenSSL and Berkeley DB headers so some of the Bitcoin code already picks it up.
Tree-SHA512: aa6cc6283015e08ab074641f9abdc116c4dc58574dc90f75e7a5af4cc82946d3052370e5cbe855fb6180c00f8dc66997d3724ff0412e4b7417e51b6602154825
de74c62 [Doc] Update bip.md, add support for BIP 159 (Jonas Schnelli)
e054d0e [QA] Add node_network_limited test (Jonas Schnelli)
bd09416 Avoid leaking the prune height through getdata (fingerprinting countermeasure) (Jonas Schnelli)
27df193 Always set NODE_NETWORK_LIMITED bit (Jonas Schnelli)
7caba38 Add NODE_NETWORK_LIMITED flags and min block amount constants (Jonas Schnelli)
Pull request description:
Extracted from #10387.
Does implement BIP159, but only the signalling part. No connections are made to NODE_NETWORK_LIMITED in this PR.
The address relay and connection work (the more complicated part) can then be separated (probably in #10387).
Tree-SHA512: e3218eb4789a9320b0f42dc10f62d30c13c49bdef00443fbe653bee22933477adcfc1cf8f6a95269324560b5721203ed41f3c5e2dd8a98ec2791f6a9d8346b1a
We could be reading multiple messages from a socket buffer at once _without actually processing them yet_ which means that `fSuccessfullyConnected` might not be switched to `true` at the time we already parsed `VERACK` message and started to parse the next one. This is basically a false positive and we drop a legit node as a result even though the order of messages sent by this node was completely fine. To fix this I partially reverted #2790 (where the issue was initially introduced) and moved the logic for tracking the first message into ProcessMessage instead.
63c2d83 Explicitly state assumption that state.m_chain_sync.m_work_header != nullptr in ConsiderEviction (practicalswift)
Pull request description:
Explicitly state assumption that `state.m_chain_sync.m_work_header != nullptr` in `ConsiderEviction(…)`.
Static analyzer (and humans!) will see the null-check in ...
```
else if (state.m_chain_sync.m_timeout == 0 || (state.m_chain_sync.m_work_header != nullptr && ...
```
... and infer that `state.m_chain_sync.m_work_header` might be set to `nullptr` when reaching `else if (state.m_chain_sync.m_timeout > 0 && time_in_seconds > state.m_chain_sync.m_timeout)` and thus flag `state.m_chain_sync.m_work_header->GetBlockHash().ToString()` as a potential null pointer dereference.
This commit makes the tacit assumption of `state.m_chain_sync.m_work_header != nullptr` explicit.
Code introduced in 5a6d00c6de ("Permit disconnection of outbound peers on bad/slow chains") which was merged into master four days ago.
Friendly ping @sdaftuar :-)
Tree-SHA512: 32e5631025b7ba7556a02c89d040fbe339c482a03f28d0dbc9871c699e1f8ac867619b89c5fd41fdcfcf0dc4d7c859295b26ccd988572145cc244261aec18ce9
fad63eb [logging] Don't incorrectly log that REJECT messages are unknown. (John Newbery)
Pull request description:
Reject messages are logged to debug.log if NET debug logging is enabled.
Because of the way the `ProcessMessages()` function is structured,
processing for REJECT messages will also drop through to the default
branch and incorrectly log `Unknown command "reject" from peer-?`. Fix
that by exiting from `ProcessMessages()` early.
without this PR:
```
2018-05-03T17:37:00.930600Z received: reject (21 bytes) peer=0
2018-05-03T17:37:00.930620Z Reject message code 16: spammy spam
2018-05-03T17:37:00.930656Z Unknown command "reject" from peer=0
```
with this PR:
```
2018-05-03T17:35:04.751246Z received: reject (21 bytes) peer=0
2018-05-03T17:35:04.751274Z Reject message code 16: spammy spam
```
Tree-SHA512: 5c84c98433ab99e0db2dd481f9c2db6f87ff0d39022ff317a791737e918714bbcb4a23e81118212ed8e594ebcf098ab7f52f7fd5e21ebc3f07b1efb279b9b30b
d3a185a net: Move misbehaving logging to net logging category (Wladimir J. van der Laan)
Pull request description:
This moves the error messages for misbehavior (when available) into the line that reports the misbehavior, as well as moves the logging to the `net` category.
This is a continuation of #11583 and avoids serious-looking errors due to misbehaving peers. As it is impossible to correlate the `peer=X` numbers to specific incoming connections now without enabling the `net` category, it doesn't really help to see these messages by default.
To do this, Misbehaving() gains an optional `message` argument.
E.g. change:
2018-01-18 16:02:27 Misbehaving: x.x.x.x:62174 peer=164603 (80 -> 100) BAN THRESHOLD EXCEEDED
2018-01-18 16:02:27 ERROR: non-continuous headers sequence
to
2018-01-18 16:02:27 Misbehaving: x.x.x.x:62174 peer=164603 (80 -> 100) BAN THRESHOLD EXCEEDED: non-continuous headers sequence
When there is a category for "important" net messages (see #12219 ), we should move it there.
Tree-SHA512: 51c97e9a649bf5409f2fd4625fa1243a036e9c9de6037bb064244207408c2e0eb025e3af80866df673cdc006b8f35dc4078d074033f0d4c6a73bbb03949a269f
be9f38c Do not make it trivial for inbound peers to generate log entries (Matt Corallo)
Pull request description:
Based on #11580 because I'm lazy.
We should generally avoid writing to debug.log unconditionally for
inbound peers which misbehave (the peer being about to be banned
being an exception, since they cannot do this twice).
Tree-SHA512: 8e59c8d08d00b1527951b30f4842d010a4c2fc440503ade112baa2c1b9afd0e0d1c5c2df83dde25183a242af45089cf9b9f873b71796771232ffb6c5fc6cc0cc
fa6c3dea420b6c50c164ccc34f4e9e8a7d9a8022 p2p: Clarify control flow in ProcessMessage() (MarcoFalke)
Pull request description:
`ProcessMessage` is effectively a massive switch case construct. In the past there were attempts to clarify the control flow in `ProcessMessage()` by moving each case into a separate static function (see #9608). It was closed because it wasn't clear if moving each case into a function was the right approach.
Though, we can quasi treat each case as a function by adding a return statement to each case. (Can be seen as a continuation of bugfix #13162)
This patch does exactly that.
Also note that this patch is a subset of previous approaches such as #9608 and #10145.
Review suggestion: `git diff HEAD~ --function-context`
Tree-SHA512: 91f6106840de2f29bb4f10d27bae0616b03a91126e6c6013479e1dd79bee53f22a78902b631fe85517dd5dc0fa7239939b4fefc231851a13c819458559f6c201
* Check MNs up to 24 blocks deep when verifying `dstx`
* Handle DSTX-es more like regular txes and not like "other" invs
* Try asking for a DSTX too when trying to find missing tx parents
* Check DSTX-es when chainlock arrives
`HasChainLock` was always `false` in `IsExpired` because tip is updated before the corresponding chainlock is received
* Apply `Handle DSTX-es more like regular txes` idea to `AlreadyHave()`
* Alternative handling of DSTX+recentRejects
Co-authored-by: Alexander Block <ablock84@gmail.com>
* [tests] Change feature_block.py to use BitcoinTestFramework
* [tests] Fix flake8 warnings in feature_block.py
* [tests] Tidy up feature_block.py
- move all helper methods to the end
- remove block, create_tx and create_and_sign_tx shortcuts
- remove --runbarelyexpensive option, since it defaults to True and it's
unlikely that anyone ever runs the test with this option set to false.
* [tests] Add logging to feature_block.py
* [tests] Improve assert message when wait_until() fails
* Merge #13048: [tests] Fix feature_block flakiness
c1d742025c [tests] Fix feature_block flakiness (John Newbery)
Pull request description:
feature_block.py occasionally fails on Travis. I believe this is due to
a a race condition when reconnecting to bitcoind after a subtest that
expects disconnection. If the test runs ahead and sends the INV for the
subsequent test before we've received the initial sync getheaders, then
we may end up sending two headers messages - one as a response to the
initial sync getheaders and one in response to the INV getheaders. If
both of those headers fail validation with a DoS score of 50 or higher,
then we'll unexpectedly be disconnected.
There is only one validation failure that has a DoS score bewteen 50 and
100, which is high-hash. That's why the test is failing immediately
after the "Reject a block with invalid work" subtest.
Fix is to wait for the initial getheaders from the peer before we
start populating our blockstore. That way we won't have any invalid
headers to respond to it with.
Tree-SHA512: dc17d795fcfaf0f8c0bf1e9732b5e11fbc8febbfafba4c231b7c13a5404a2c297dcd703a7a75bc7f353c893e12efc87f424f2201abd47ba5268af32d4d2e841f
* Temporarely rename MAX_BLOCK_SIZE -> MAX_BLOCK_BASE_SIZE
We'll undo this after the next commit. This avoids merge many conflicts and
makes reviewing easier.
* Rename MAX_BLOCK_BASE_SIZE back to MAX_BLOCK_SIZE
* Use DoS score of 100 for bad-blk-sigops
This was accidently changed to 10 while backporting bitcoin#7287 and causes
test failures in p2p-fullblocktest.py
* Use allowOptimisticSend=true when sending reject messages
This fixes test failures in p2p-fullblocktest.py which expects reject
messages to be sent/received before connections get closed.
* Fix p2p-fullblocktest.py
- CBlock and friends are still in test_framework.mininode
- "-whitelist" causes connections to not be dropped, which in turn causes
sync_blocks with reconnect=True to fail
- "bad-cb-amount" does not cause a ban in Dash, so reconnect must be False
- Dash already bans when a header is received which is a child of an invalid
header, causing block requests to never happen
* Backport missing changes from bitcoin#13003
bitcoin#13003 was backported out of order which causes missed changes.
* Bump p2p-fullblocktest timeouts
* Increase RPC timeout in p2p-fullblocktest.py
Co-authored-by: John Newbery <jonnynewbs@gmail.com>
Co-authored-by: MarcoFalke <falke.marco@gmail.com>
c00199244 Fix potential null dereferences (MeshCollider)
Pull request description:
Picked up by the static analyzer [Facebook Infer](http://fbinfer.com/) which I was playing around with for another research project. Just adding some asserts before dereferencing potentially null pointers.
Tree-SHA512: 9c01dab2d21bce75c7c7ef867236654ab538318a1fb39f96f09cdd2382a05be1a6b2db0a1169a94168864e82ffeae0686a383db6eba799742bdd89c37ac74397
64fb0ac Declare single-argument (non-converting) constructors "explicit" (practicalswift)
Pull request description:
Declare single-argument (non-converting) constructors `explicit`.
In order to avoid unintended implicit conversions.
For a more thorough discussion, see ["C.46: By default, declare single-argument constructors explicit"](http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c46-by-default-declare-single-argument-constructors-explicit) in the C++ Core Guidelines (Stroustrup & Sutter).
Tree-SHA512: e0c6922e56b11fa402621a38656d8b1122d16dd8f160e78626385373cf184ac7f26cb4c1851eca47e9b0dbd5e924e39a85c3cbdcb627a05ee3a655ecf5f7a0f1
c4af738 Fix ignoring tx data requests when fPauseSend is set on a peer (Matt Corallo)
Pull request description:
This resolves a bug introduced in
66aa1d58a1 where, if when responding
to a series of transaction requests in a getdata we hit the send
buffer limit and set fPauseSend, we will skip one transaction per
call to ProcessGetData.
Bug found by Cory Fields (@theuni).
Probably worth slipping into 0.16 :/.
Tree-SHA512: a9313cef8ac6da31eb099c9925c8401a638220cf7bc9b7b7b83151ecae4b02630f2db45ef6668302b9bb0f38571afbd764993427f1ec9e4d74d9a3be6647d299
This avoids sorting before looping through it to figure out what to
request. The assumption that sorting would be cheap when vecAskFor is
already mostly sorted (only unsorted at the end) turned out to be false.
In reality, ~50% of CPU time was consumed by the sort when a lot of traffic
(thousands of TXs) happen.
This makes orphan processing work like handling getdata messages:
After every actual transaction validation attempt, interrupt
processing to deal with messages arriving from other peers.
* Ignore recent rejects filter for locked txes
If we had a conflicting tx in the mempool before the locked tx arrived and the locked one arrived before the corresponding islock (i.e. we don't really know it's the one that should be included yet), the locked one is going to be rejected due to a mempool conflict. The old tx is going to be removed from the mempool by an incoming islock a bit later, however, we won't be able to re-request the locked tx until the tip changes because of the recentRejects filter. This patch fixes it.
* Add some explanation
6262915 Add unit test for stale tip checking (Suhas Daftuar)
83df257 Add CConnmanTest to mutate g_connman in tests (João Barbosa)
ac7b37c Connect to an extra outbound peer if our tip is stale (Suhas Daftuar)
db32a65 Track tip update time and last new block announcement from each peer (Suhas Daftuar)
2d4327d net: Allow connecting to extra outbound peers (Suhas Daftuar)
Pull request description:
This is an alternative approach to #11534. Rather than disconnect an outbound peer when our tip looks stale, instead try to connect to an additional outbound peer.
Periodically, check to see if we have more outbound peers than we target (ie if any extra peers are in use), and if so, disconnect the one that least recently announced a new block (breaking ties by choosing the newest peer that we connected to).
Tree-SHA512: 8f19e910e0bb36867f81783e020af225f356451899adfc7ade1895d6d3bd5afe51c83759610dfd10c62090c4fe404efa0283b2f63fde0bd7da898a1aaa7fb281
f3d4adf Make p2p-acceptablock not an extended test (Matt Corallo)
00dcda6 [qa] test that invalid blocks on an invalid chain get a disconnect (Matt Corallo)
015a525 Reject headers building on invalid chains by tracking invalidity (Matt Corallo)
932f118 Accept unrequested blocks with work equal to our tip (Matt Corallo)
3d9c70c Stop always storing blocks from whitelisted peers (Matt Corallo)
3b4ac43 Rewrite p2p-acceptblock in preparation for slight behavior changes (Matt Corallo)
Pull request description:
@sdaftuar pointed out that the version in #11487 was somewhat DoS-able as someone could feed you a valid chain that forked off the the last checkpoint block and force you to do lots of work just walking backwards across blocks for each new block they gave you. We came up with a few proposals but settled on the one implemented here as likely the simplest without obvious DoS issues. It uses our existing on-load mapBlockIndex walk to make sure everything that descends from an invalid block is marked as such, and then simply caches blocks which we attempted to connect but which were found to be invalid. To avoid DoS issues during IBD, this will need to depend on #11458.
Includes tests from #11487.
Tree-SHA512: 46aff8332908e122dae72ceb5fe8cd241902c2281a87f58a5fb486bf69d46458d84a096fdcb5f3e8e07fbcf7466232b10c429f4d67855425f11b38ac0bf612e1
2530bf2 net: Add missing lock in ProcessHeadersMessage(...) (practicalswift)
Pull request description:
Add missing lock in `ProcessHeadersMessage(...)`.
Reading the variable `mapBlockIndex` requires holding the mutex `cs_main`.
The new "Disconnect outbound peers relaying invalid headers" code added in commit 37886d5e2f and merged as part of #11568 two days ago did not lock `cs_main` prior to accessing `mapBlockIndex`.
Tree-SHA512: b799c234be8043d036183a00bc7867bbf3bd7ffe3baa94c88529da3b3cd0571c31ed11dadfaf29c5b8498341d6d0a3c928029a43b69f3267ef263682c91563a3
37886d5e2 Disconnect outbound peers relaying invalid headers (Suhas Daftuar)
4637f1852 moveonly: factor out headers processing into separate function (Suhas Daftuar)
Pull request description:
Alternate to #11446.
Disconnect outbound (non-manual) peers that serve us block headers that are already known to be invalid, but exempt compact block announcements from such disconnects.
We restrict disconnection to outbound peers that are using up an outbound connection slot, because we rely on those peers to give us connectivity to the honest network (our inbound peers are not chosen by us and hence could all be from an attacker/sybil). Maintaining connectivity to peers that serve us invalid headers is sometimes desirable, eg after a soft-fork, to protect unupgraded software from being partitioned off the honest network, so we prefer to only disconnect when necessary.
Compact block announcements are exempted from this logic to comply with BIP 152, which explicitly permits nodes to relay compact blocks before fully validating them.
Tree-SHA512: 3ea88e4ccc1184f292a85b17f800d401d2c3806fefc7ad5429d05d6872c53acfa5751e3df83ce6b9c0060ab289511ed70ae1323d140ccc5b12e3c8da6de49936
e065249 Add unit test for outbound peer eviction (Suhas Daftuar)
5a6d00c Permit disconnection of outbound peers on bad/slow chains (Suhas Daftuar)
c60fd71 Disconnecting from bad outbound peers in IBD (Suhas Daftuar)
Pull request description:
The first commit will disconnect an outbound peer that serves us a headers chain with insufficient work while we're in IBD.
The second commit introduces a way to disconnect outbound peers whose chains fall out of sync with ours:
For a given outbound peer, we check whether their best known block (which is known from the blocks they announce to us) has at least as much work as our tip. If it doesn't, we set a 20 minute timeout, and if we still haven't heard about a block with as much work as our tip had when we set the timeout, then we send a single getheaders message, and wait 2 more minutes. If after two minutes their best known block has insufficient work, we disconnect that peer.
We protect 4 of our outbound peers (who provide some "good" headers chains, ie a chain with at least as much work as our tip at some point) from being subject to this logic, to prevent excessive network topology changes as a result of this algorithm, while still ensuring that we have a reasonable number of nodes not known to be on bogus chains.
We also don't require our peers to be on the same chain as us, to prevent accidental partitioning of the network in the event of a chain split. Note that if our peers are ever on a more work chain than our tip, then we will download and validate it, and then either reorg to it, or learn of a consensus incompatibility with that peer and disconnect. This PR is designed to protect against peers that are on a less work chain which we may never try to download and validate.
Tree-SHA512: 2e0169a1dd8a7fb95980573ac4a201924bffdd724c19afcab5efcef076fdbe1f2cec7dc5f5d7e0a6327216f56d3828884f73642e00c8534b56ec2bb4c854a656
2525b972a net: stop both net/net_processing before destroying them (Cory Fields)
80e2e9d0c net: drop unused connman param (Cory Fields)
8ad663c1f net: use an interface class rather than signals for message processing (Cory Fields)
28f11e940 net: pass CConnman via pointer rather than reference (Cory Fields)
Pull request description:
See individual commits.
Benefits:
- Allows us to begin moving stuff out of CNode and into CNodeState (after #10652 and follow-ups)
- Drops boost dependency and overhead
- Drops global signal registration
- Friendlier backtraces
Tree-SHA512: af2038c959dbec25f0c90c74c88dc6a630e6b9e984adf52aceadd6954aa463b6aadfccf979c2459a9f3354326b5077ee02048128eda2a649236fadb595b66ee3
15f5d3b17 Switch DNSSeed-needed metric to any-automatic-nodes, not services (Matt Corallo)
5ee88b4bd Clarify docs for requirements/handling of addnode/connect nodes (Matt Corallo)
57edc0b0c Rename fAddnode to a more-descriptive "manual_connection" (Matt Corallo)
44407100f Replace relevant services logic with a function suite. (Matt Corallo)
Pull request description:
This was mostly written as a way to clean things up so that the NETWORK_LIMITED PR (#10387) can be simplified a ton, but its also a nice standalone cleanup that will also require a bit of review because it tweaks a lot of stuff across net. The new functions are fine in protocol.h right now since they're straight-forward, but after NETWORK_LIMITED will really want to move elsewhere after @theuni moves the nServices-based selection to addrman from connman.
Adds HasAllRelevantServices and GetRelevantServices, which check
for NETWORK|WITNESS.
This changes the following:
* Removes nRelevantServices from CConnman, disconnecting it a bit
more from protocol-level logic.
* Replaces our sometimes-connect-to-!WITNESS-nodes logic with
simply always requiring WITNESS|NETWORK for outbound non-feeler
connections (feelers still only require NETWORK).
* This has the added benefit of removing nServicesExpected from
CNode - instead letting net_processing's VERSION message
handling simply check HasAllRelevantServices.
* This implies we believe WITNESS nodes to continue to be a
significant majority of nodes on the network, but also because
we cannot sync properly from !WITNESS nodes, it is strange to
continue using our valuable outbound slots on them.
* In order to prevent this change from preventing connection to
-connect= nodes which have !WITNESS, -connect nodes are now
given the "addnode" flag. This also allows outbound connections
to !NODE_NETWORK nodes for -connect nodes (which was already true
of addnodes).
* Has the (somewhat unintended) consequence of changing one of the
eviction metrics from the same
sometimes-connect-to-!WITNESS-nodes metric to requiring
HasRelevantServices.
This should make NODE_NETWORK_LIMITED much simpler to implement.
Tree-SHA512: 90606896c86cc5da14c77843b16674a6a012065e7b583d76d1c47a18215358abefcbab44ff4fab3fadcd39aa9a42d4740c6dc8874a58033bdfc8ad3fb5c649fc
01b52ce Add comment explaining forced processing of compact blocks (Suhas Daftuar)
08fd822 qa: add test for minchainwork use in acceptblock (Suhas Daftuar)
ce8cd7a Don't process unrequested, low-work blocks (Suhas Daftuar)
Pull request description:
A peer could try to waste our resources by sending us unrequested blocks with
low work (eg to fill up our disk). Since e265200 we no longer request blocks until we
know we're on a chain with more than nMinimumChainWork (our anti-DoS
threshold), but we would still process unrequested blocks that had more work
than our tip (which generally has low-work during IBD), even though we may not
yet have found a headers chain with sufficient work.
Fix this and add a test.
Tree-SHA512: 1a4fb0bbd78054b84683f995c8c3194dd44fa914dc351ae4379c7c1a6f83224f609f8b9c2d9dde28741426c6af008ffffea836d21aa31a5ebaa00f8e0f81229e
eac64bb7a [qa] Test nMinimumChainWork (Suhas Daftuar)
0311836f6 Allow setting nMinimumChainWork on command line (Suhas Daftuar)
Pull request description:
As discussed briefly here: https://botbot.me/freenode/bitcoin-core-dev/2017-02-28/?msg=81712308&page=4
This adds a hidden command line option for setting `nMinimumChainWork`, which allows us to test this parameter in our functional tests, as well as allowing for niche use cases like syncing nodes that are otherwise disconnected from the network.
See also #10345, which proposes a new use of `nMinimumChainWork`.
Tree-SHA512: fe4d8f4f289697615c98d8760f1cc74c076110310ea0b5b875fcab78c127da9195b4eb84148aebacc7606c246e5773d3f13bd5d9559d0a8bffac20a3a28c62df
3f8fa7f Make sure to clean up mapBlockSource if we've already seen the block (Cory Fields)
Pull request description:
Otherwise we may leave them dangling.
Credit TheBlueMatt.
Tree-SHA512: 8be77e08ebfc4f5b206d5ee7cfbe87f92c1eb5bc2b412471993658fe210306789aaf0f3d1454c635508a7d8effede2cf5ac144d622b0157b872733d9661d65c3
176c021 [qa] Test non-atomic chainstate writes (Suhas Daftuar)
d6af06d Dont create pcoinsTip until after ReplayBlocks. (Matt Corallo)
eaca1b7 Random db flush crash simulator (Pieter Wuille)
0580ee0 Adapt memory usage estimation for flushing (Pieter Wuille)
013a56a Non-atomic flushing using the blockchain as replay journal (Pieter Wuille)
b3a279c [MOVEONLY] Move LastCommonAncestor to chain (Pieter Wuille)
Tree-SHA512: 47ccc62303f9075c44d2a914be75bd6969ff881a857a2ff1227f05ec7def6f4c71c46680c5a28cb150c814999526797dc05cf2701fde1369c06169f46eccddee
5a9b508 [trivial] Add end of namespace comments (practicalswift)
Tree-SHA512: 92b0fcae4d1d3f4da9e97569ae84ef2d6e09625a5815cd0e5f0eb6dd2ecba9852fa85c184c5ae9de5117050330ce995e9867b451fa8cd5512169025990541a2b
* Remove ppszTypeName from protocol.cpp and reimplement GetCommand
This removes the need to carefully maintain ppszTypeName, which required
correct order and also did not allow to permanently remove old message
types.
To get the command name for an INV type, GetCommandInternal uses a switch
which needs to be maintained from now on.
The way this is implemented also resembles the way it is implemented in
Bitcoin today, but it's not identical. The original PR that introduced the
switch case in Bitcoin was part of the Segwit changes and thus never got
backported. I decided to implement it in a slightly different way that
avoids throwing exceptions when an unknown INV type is encountered.
IsKnownType will now also leverage GetCommandInternal() to figure out if
the INV type is known locally. This has the side effect of old/legacy
message types to return false from now on. We will depend on this side
effect in later commits when we remove legacy InstantSend code.
* Stop handling/relaying legacy IX messages
When we receive an IX message, we simply treat it as a regular TX and relay
it as such.
We'll however still request IX messages when they are announced to us. We
can't simply revert to requesting TX messages in this case as it might
result in the other peer not answering due to the TX not being in mapRelay
yet. We should at some point in the future completely drop handling of IX
messages instead.
* Remove IsNewInstantSendEnabled() and only use IsInstantSendEnabled()
* Remove legacy InstantSend from GUI
* Remove InstantSend from Bitcoin/Dash URIs
* Remove legacy InstantSend from RPC commands
* Remove legacy InstantSend from wallet
* Remove legacy instantsend.h include
* Remove legacy InstantSend from validation code
* Completely remove remaining legacy InstantSend code
* Remove now unused spork
* Fix InstantSend related test failures
* Remove now obsolete auto IS tests
* Make spork2 and spork3 disabled by default
This should have no influence on mainnet as these sporks are actually set
there. This will however affect regtest, which shouldn't have LLMQ based
InstantSend enabled by default.
* Remove instantsend tests from dip3-deterministicmns.py
These were only testing legacy InstantSend
* Fix .QCheckBox#checkUsePrivateSend styling a bit
* s/TXLEGACYLOCKREQUEST/LEGACYTXLOCKREQUEST/
* Revert "verified via InstantSend" back to "verified via LLMQ based InstantSend"
* Use cmd == nullptr instead of !cmd
* Remove last parameter from AvailableCoins call
This was for fUseInstantSend which is not present anymore since rebase
211adc0 Use range-based for loops (C++11) when looping over vector elements (practicalswift)
Tree-SHA512: 0e007f20dcef99d3c7a1036265e00f689d69f42e02fd82dd8389f45b52d31947e5f9388de2610d3d9bd9f554915ce0d35ebce561e5ae3a9013956d0ee4937145
0f3471f net: make CNode's id private (Cory Fields)
9ff0a51 scripted-diff: net: Use accessor rather than node's id directly (Cory Fields)
e50c33e devtools: add script to verify scriptable changes (Cory Fields)
skipped travis changes
Tree-SHA512: a0ff50f4e1d38a2b63109b4996546c91b3e02e00d92c0bf04f48792948f78b1f6d9227a15d25c823fd4723a0277fc6a32c2c1287c7abbb7e50fd82ffb0f8d994
pnode->id to pnode->GetId()
Signed-off-by: Pasta <pasta@dashboost.org>
* Generalize CBLSLazyWrapper so that it can be used of pubkeys and secret keys
* Implement == and != operators for CBLSLazyWrapper
* Implement cached hash for CBLSLazyWrapper
* Use CBLSLazyPublicKey for CDeterministicMNState::pubKeyOperator
* Speed up GetProjectedMNPayees by sorting the MN list by last paid
Instead of updating a temporary list for each projected height and calling
GetMNPayee() on it.
* Cache intermediate lists in GetListForBlock
This avoids re-loading and applying diffs again and again.
* Only update masternode list UI max once every 3 seconds
This avoids updating the UI on every block, which turned out to be very
expensive.
* Fix compilation
* Drop time restrictions for mn list update in ClientModel
They are fully handled by MasternodeList now.
b1a6d4c Take a CTransactionRef in AddToWalletIfInvolvingMe to avoid a copy (Matt Corallo)
1c95e2f Use std::shared_ptr instead of boost::shared_ptr in ScriptForMining (Matt Corallo)
91f1e6c Remove dead-code tracking of requests for blocks we generated (Matt Corallo)
acad82f Add override to functions using CValidationInterface methods (Matt Corallo)
e6d5e6c Hold cs_wallet for whole block [dis]connection processing (Matt Corallo)
461e49f SyncTransaction->TxAddedToMempool/BlockConnected/Disconnected (Matt Corallo)
f404334 Handle SyncTransaction in ActivateBestChain instead of ConnectTrace (Matt Corallo)
a147687 Keep conflictedTxs in ConnectTrace per-block (Matt Corallo)
d3167ba Handle conflicted transactions directly in ConnectTrace (Matt Corallo)
29e6e23 Make ConnectTrace::blocksConnected private, hide behind accessors (Matt Corallo)
822000c Add pblock to connectTrace at the end of ConnectTip, not start (Matt Corallo)
f5e9a01 Include missing #include in zmqnotificationinterface.h (Matt Corallo)
Tree-SHA512: 8893d47559da3b28d2ef7359768547cba8a4b43b6f891d80f5848f995a84b1517bfb0f706fdc8cd43f09a1350349eb440d9724a59363ab517dfcc4fcb31b2018
* Tighten rules for dstx
* Tighten rules for dsvin
* NULL -> nullptr
* Make `ConsumeCollateral()` a private function instead of a lamda and reuse it in `Charge*Fees()`
* Make sure inputs and outputs are of the same size
Introduces new response ERR_SIZE_MISMATCH, old clients will simply bail out.
* Drop now redundant vecTxOut.size() check
* Check max inputs size
* Fix log category
* Contains dashification. disables `-debug dash`
Merge #9424: Change LogAcceptCategory to use uint32_t rather than sets of strings.
6b3bb3d Change LogAcceptCategory to use uint32_t rather than sets of strings. (Gregory Maxwell)
Tree-SHA512: ebb5bcf9a7d00a32dd1390b727ff4d29330a038423611da01268d8e1d2c0229e52a1098e751d4e6db73ef4ae862e1e96d38249883fcaf12b68f55ebb01035b34
Signed-off-by: Pasta <Pasta@dash.org>
31 -> 32
Signed-off-by: Pasta <Pasta@dash.org>
* Merge #10123: Allow debug logs to be excluded from specified component
3bde556 Add -debugexclude option to switch off logging for specified components (John Newbery)
Tree-SHA512: 30202e3f2085fc2fc5dd4bedb92988f4cb162c612a42cf8f6395a7da326f34975ddc347f82bc4ddca6c84c438dc0cc6e87869f90c7ff88105dbeaa52a947fa43
* bump to uint64_t due to added Dash codes
Signed-off-by: Pasta <Pasta@dash.org>
* bump to uint64_t due to added Dash codes cont.
Signed-off-by: Pasta <Pasta@dash.org>
* string -> BCLog format
Signed-off-by: Pasta <Pasta@dash.org>
* uint32_t -> uint64_t
Signed-off-by: Pasta <Pasta@dash.org>
* Fix CBatchedLogger
* Fix most fDebug-s
* Fix `debug` rpc
* Fix BENCH and RAND conflicts
* Add ALERT and use it
* Update LogPrint-s in dash-specific code
* Tweak few log categories
Specifically:
- use PRIVATESEND in `CPrivateSendClientManager::GetRandomNotUsedMasternode()`
- use ZMQ in `CZMQPublishRawGovernanceVoteNotifier::NotifyGovernanceVote()` and `CZMQPublishRawGovernanceObjectNotifier::NotifyGovernanceObject()`
* Drop no longer used MASTERNODE category
* Merge #10153: logging: Fix off-by-one for shrinkdebugfile default
faab624 logging: Fix off-by-one for shrinkdebugfile (MarcoFalke)
Tree-SHA512: d6153e06067906172ff0611af9e585a3ecf0a7d56925b6ad7c12e75aa802441047059b9b6f6c78e79916c3f2abc8f1998bfd2d5b84201ec6421f727c08da3c21
* Shift dash-specific log categories to start from `1ul << 32` to avoid potential future conflicts with bitcoin ones
* Fix `dash` category
* remove debugCategories
Signed-off-by: Pasta <Pasta@dash.org>
* Prepend "std::" to find call
* Check for BCLog::PRIVATESEND instead of logCategories != BCLog::NONE
* Use BCLog::MNPAYMENTS category instead of checking for logCategories != BCLog::NONE
* Move "End Dash" comment below "ALERT"
When adding new entries here, we'll otherwise get confused with ordering
and might end up forgetting that adding something Dash specific must
continue with the bit after 43.
* rename instantx.* file to instantsend.*
* Reorganize Dash Specific code into folders
add to privatesend folder the header files
Signed-off-by: Pasta <Pasta@dash.org>
add "masternode" to imports
Signed-off-by: Pasta <Pasta@dash.org>
add "masternode" to imports
Signed-off-by: Pasta <Pasta@dash.org>
add "masternode" to imports pt 2
Signed-off-by: Pasta <Pasta@dash.org>
* Remove code for QDEBUGSTATUS propagation
This turned out to be too expensive and could easily take the network
down by bringing all nodes to 100% CPU usage. Better to fully remove this
functionality.
* Apply suggestions from code review
Co-Authored-By: codablock <ablock84@gmail.com>
* Update src/rpc/rpcquorums.cpp
Co-Authored-By: codablock <ablock84@gmail.com>
Observed on testnet that MNs tend to re-request the same objects multiple
times when load becomes high, which results in the same objects being
received multiple times.
* Merge #13176: Improve CRollingBloomFilter performance: replace modulus with FastMod
9aac9f90d5e56752cc6cbfac48063ad29a01143c replace modulus with FastMod (Martin Ankerl)
Pull request description:
Not sure if this is optimization is necessary, but anyway I have some spare time so here it is. This replaces the slow modulo operation with a much faster 64bit multiplication & shift. This works when the hash is uniformly distributed between 0 and 2^32-1. This speeds up the benchmark by a factor of about 1.3:
```
RollingBloom, 5, 1500000, 3.73733, 4.97569e-07, 4.99002e-07, 4.98372e-07 # before
RollingBloom, 5, 1500000, 2.86842, 3.81630e-07, 3.83730e-07, 3.82473e-07 # FastMod
```
Be aware that this changes the internal data of the filter, so this should probably
not be used for CBloomFilter because of interoperability problems.
Tree-SHA512: 04104f3fb09f56c9d14458a6aad919aeb0a5af944e8ee6a31f00e93c753e22004648c1cd65bf36752b6addec528d19fb665c27b955ce1666a85a928e17afa47a
* Use unordered_map in CSporkManager
In one of my profiling sessions with many InstantSend transactions
happening, calls into CSporkManager added up to about 1% of total CPU time.
This is easily avoidable by using unordered maps.
* Use std::unordered_map instead of std::map in limitedmap
* Use unordered_set for CNode::setAskFor
* Add serialization support for unordered maps and sets
* Use unordered_map for mapArgs and mapMultiArgs
* Let limitedmap prune in batches and use unordered_multimap
Due to the batched pruning, there is no need to maintain an ordered map
of values anymore. Only when nPruneAfterSize, there is a need to create
a temporary ordered vector of values to figure out what can be removed.
* Instead of using a multimap for mapAskFor, use a vector which we sort on demand
CNode::AskFor will now push entries into an initially unordered vector
instead of an ordered multimap. Only when we later want to use vecAskFor in
SendMessages, we sort the vector.
The vector will actually be mostly sorted in most cases as insertion order
usually mimics the desired ordering. Only the last few entries might need
some shuffling around. Doing the sort on-demand should be less wasteful
then trying to maintain correct order all the time.
* Fix compilation of tests
* Fix limitedmap tests
* Rename limitedmap to unordered_limitedmap to ensure backports conflict
This ensures that future backports that depends on limitedmap's ordering
conflict so that we are made aware of needed action.
* Fix compilation error on Travis
* Sort evo/* source files in Makefile.am
* Keep track of proRegTxHash in CConnman::masternodeQuorumNodes map
We will later need the proRegTxHash
* Fix serialization of std::tuple with const rvalue elements
Having serialization and deserialization in the same specialized template
results in compilation failures due to the "if(for_read)" branch.
* Implement MNAUTH message
This allows masternodes to authenticate themself.
* Protect fresh incoming connections for a second from eviction
Give fresh connections some time to do the VERSION/VERACK handshake and
an optional MNAUTH when it's a masternode. When an MNAUTH happened, the
incoming connection is then forever protected against eviction.
If a timeout of 1 second occurs or the first message after VERACK is not
MNAUTH, the node is not protected anymore and becomes eligable for
eviction.
* Avoid connecting to masternodes if an incoming connection is from the same one
Now that incoming connections from MNs authenticate them self, we can avoid
connecting to the same MNs through intra-quorum connections.
* Apply review suggestions
* Introduce "qsendrecsigs" to indicate that plain recovered sigs should be sent
Full nodes, including masternodes, will send this message automatically.
Other node implementations (e.g. SPV) are usually not interested and would
not send this message.
* Use std::atomic<bool> instead of std::atomic_bool
Not related to this PR, but a small enough change to include it here as
well.
* Don't rely on UTXO set in CheckCanLock
The UTXO set only works for TXs in the mempool and won't work when we try
to retroactively lock unlocked TXs from blocks.
This is safe as ProcessTx is only called when a TX was accepted into the
mempool or connected in a block, which means that all input checks were
good.
* Rename RetryLockMempoolTxs to RetryLockTxs and let it retry connected TXs
* Instead of manually calling ProcessTx, let SyncTransaction handle all cases
SyncTransaction is called from AcceptToMemoryPool and when transactions got
connected in a block. So this is the time we want to run TXs through
ProcessTx. This also enables retroactive signing of TXs that were unknown
before a new block appeared.
* Test retroactive signing and safe TXs in LLMQ ChainLocks tests
* Also test for retroactive signing of chained TXs
* Honor lockedParentTx when looking for TXs to retry signing
* Stop scanning for TXs to retry after a depth of 6
* Generate 6 block to avoid retroactive signing overloading Travis
* Avoid retroactive signing
* Don't rely on NewPoWValidBlock and use SyncTransaction to build blockTxs
NewPoWValidBlock is not guaranteed to be called when blocks come in fast.
When a block is accepted in AcceptBlock, NewPoWValidBlock is only called
when the new block is a successor of the currently active tip. This is not
the case when after the first block a second block is accepted immediately
as the first block is not connected yet.
This might be a bug actually in the handling of NewPoWValidBlock, so we
might need to check/fix this later, but currently I prefer to not touch
that part.
Instead, we now use SyncTransaction to gather TXs for blockTxs. This works
because SyncTransaction is called for all transactions in a freshly
connected block in one go. The call also happens before UpdatedBlockTip is
called, so it's fine with the existing logic.
* Use tx.IsCoinBase() instead of checking index 0
Also check for empty vin.
* Remove unused parameters from CInstantSendManager::ProcessTx
* Pass txHash in CheckCanLock by reference instead of pointer
* Dont' allow locking of TXs without inputs
* Remove unused local variable nInstantSendConfirmationsRequired
* Don't subtract 1 from nInstantSendConfirmationsRequired
This was necessary in the old system but is not necessary in the new system.
It also prevented proper retroactive signing of chained TXs in regtest as
it resulted in child TXs to return true immediately for CheckCanLock when
it should actually have waited for the parent TX to become locked first.
* Access chainActive.Height() while cs_main is locked
* Properly read and write lastChainLockBlock
"pindex" is NOT the chainlocked block after the while loop finishes. We
must use the pindex (renamed to pindexChainLock now) given on method entry.
Also, the GetLastChainLockBlock() result was not assigned to,
lastChainLockBlock which resulted in the while loop to run unnecessarily
long.
* Generalize filtering in NewPoWValidBlock and SyncTransaction
We're actually interested in all TXs that have inputs, so no need to
explicitly check for tx types.
* Use tx.IsCoinBase() instead of checking for index 0
* Handle cases where a TX is not received yet in wait_for_instantlock
* Wait on all nodes for the locks
Otherwise we end up with the sender having it locked but other nodes
not yet, failing the test.
* Fix LogPrintf call in CChainLocksHandler::DoInvalidateBlock
725b79a [test] Verify node doesn't send headers that haven't been fully validated (Russell Yanofsky)
3788a84 Do not send (potentially) invalid headers in response to getheaders (Matt Corallo)
Pull request description:
Nowhere else in the protocol do we send headers which are for
blocks we have not fully validated except in response to getheaders
messages with a null locator. On my public node I have not seen any
such request (whether for an invalid block or not) in at least two
years of debug.log output, indicating that this should have minimal
impact.
Tree-SHA512: c1f6e0cdcdfb78ea577d555f9b3ceb1b4b60eff4f6cf313bfd8b576c9562d797bea73abc23f7011f249ae36dd539c715f3d20487ac03ace60e84e1b77c0c1e1a
eff4bd8 [test] P2P functional test for certain fingerprinting protections (Jim Posen)
a2be3b6 [net] Ignore getheaders requests for very old side blocks (Jim Posen)
Pull request description:
Sending a getheaders message with an empty locator and a stop hash is a request for a single header by hash. The node will respond with headers for blocks not in the main chain as well as those in the main chain. To avoid fingerprinting, the node should, however, ignore requests for headers on side branches that are too old. This replicates the logic that currently exists for `getdata` requests for blocks.
Tree-SHA512: e04ef61e2b73945be6ec5977b3c5680b6dc3667246f8bfb67afae1ecaba900c0b49b18bbbb74869f7a37ef70b6ed99e78ebe0ea0a1569369fad9e447d720ffc4
b49ad44 Add comment about cs_most_recent_block coverage (Matt Corallo)
c47f5b7 Cache witness-enabled state with recent-compact-block-cache (Matt Corallo)
efc135f Use cached [compact] blocks to respond to getdata messages (Matt Corallo)
Tree-SHA512: ffc478bddbf14b8ed304a3041f47746520ce545bdeffa9652eff2ccb25c8b0d5194abe72568c10f9c1b246ee361176ba217767af834752a2ca7263d292005e87
This seems to be backported wrongly. In the Bitcoin code, there is a
condition on requested witness data and we took the other branch which
recreates the compact block. We should have taken the other branch because
we always send with witness data (there is no Segwit in Dash).
* Add libbacktrace to depends
This is currently only useful to extract symbols. It fails to gather
stacktraces when compiled with MinGW, so we can only use it to get symbol
information from a stack trace which we gathered outside of libbacktrace.
* Add -mbig-obj to CXXFLAGS for MinGW builds
* Implement stacktraces for C++ exceptions
This is a hack and should only be used for debugging. It works by wrapping
the C++ ABI __wrap___cxa_allocate_exception. The wrapper records a backtrace
and stores it in a global map. Later the stacktrace can be retrieved with
GetExceptionStacktraceStr.
This commit also adds handlers to pretty print uncaught exceptions and
signals.
* Use GetPrettyExceptionStr for all unhandled exceptions
* Use --enable-stacktraces in CI for linux32/linux64
* Register exception translators to pretty print exceptions in unit tests
* Catch and print python exceptions when stopping nodes
Otherwise the code at the bottom is never executed when nodes crash,
leading to no output of debug.log files on Travis.
* Remove now unneeded/unused TestCrash methods
* Drop registry deletion of the old key
* Drop no longer used CGovernanceObjectVoteFile::RemoveOldVotes()
* Drop temporary disconnect code and bump min protos to 70213
* drop comment
* fix
* Split up remaining logic from CMasternodeMan into CMasternodeMetaMan and CMasternodeUtils
Also get rid of CMastermode and store remaining meta info
in CMasternodeMetaInfo
* Also allow non-const T in Serialize/Unserialize for shared_ptr
* Rename CActiveDeterministicMasternodeManager to CActiveMasternodeManager
* Fix nowallet compile in masternode-utils.cpp
* Remove CActiveLegacyMasternodeManager
* Remove sentinelping RPC
* Remove unused P2P messages and inv types
There are still places where these are used in the code. The next commits
will clean these up.
* Remove MNB/MNP/MNVERIFY related code from masternode(man).h/cpp
* Remove all legacy code regarding block MN payee voting
* Remove MASTERNODE_SYNC_LIST and MASTERNODE_SYNC_MNW states
Also replace all uses of IsMasternodeListSynced and IsWinnersListSynced
with IsBlockchainSynced.
* Remove unsupported masternode RPCs
* Remove UpdateLastPaid methods
* Remove duplicate deterministicmns.h include
* Remove masternode.conf support
* Remove legacy MN lists support from masternode list GUI
* Remove unnecessary AskForMN call
* Remove compatibility code in CPrivateSendQueue::GetSignatureHash
* Don't add locally calculated MN payee in case GetBlockTxOuts failed
This is not valid in DIP3 mode
* Remove check for IsDeterministicMNsSporkActive in "masternode status"
* Move CMasternode::IsValidNetAddr to CActiveDeterministicMasternodeManager
* Remove use of CMasternode::CheckCollateral in governance code
* Remove uses of MASTERNODE_SENTINEL_PING_MAX_SECONDS/MASTERNODE_SENTINEL_PING_MAX_SECONDS
* Remove support for "-masternodeprivkey"
* Remove pre-DIP3 vote cleanup
* Remove compatibility code for quorumModifierHash/masternodeProTxHash
* Remove check for invalid nBlockHeight in CMasternodePayments::GetBlockTxOuts
...and let it crash instead. We expect this method to be called with the
correct height now (after DIP3 was fully deployed).
* Remove ECDSA based Sign/CheckSignature from CGovernanceObject
Only masternodes sign governance objects, so there is no need for ECDSA
support here anymore.
* Always add superblock and MN reward payments into new block
* Always check block payees (except if fLiteMode==true)
* Always allow superblock and MN payees in same block
* Remove/Fix a few references to masternode.conf and related stuff
Also delete guide-startmany.md and masternode_conf.md
* Implement NotifyMasternodeListChanged signal and call governance maintenance
* Remove non-DIP3 code path from CMasternodeMan::Find
* Remove remaining unused code from CMasternode/CMasternodeMan
* Always load governance.dat on startup
* Mine an empty block instead of incrementing nHeight from chain tip in miner tests
This test is crashing otherwise in GetBlockTxOuts as it tries to access a
previous block that is not existing.
* Skip MN payments verification on historical blocks (pre-DIP3 blocks)
Even though DIP3 was active on BIP9 level, the spork was not active yet at
that point meaning that payments were not enforced at that time.
* Remove unused state and CollateralStatus enums
* Unconditionally return false from IsBlockPayeeValid when IsTransactionValid returns false
IsTransactionValid already handles the case where IsDIP3Active() returns
false, making it return true.
* Add override keyword to CDSNotificationInterface::NotifyMasternodeListChanged
* Fix help for masternodelist status (POSE_BANNED and no OUTPOINT_SPENT)
* Replace IsDeterministicMNsSporkActive with IsDIP3Active
IsDIP3Active will now use a fixed parameter from consensus params.
Values for DIP0003Height/DIP0003Hash need to be updated when spork15
activates on mainnet.
Also enforce correct block hash on testnet/mainnet for DIP3 activation
block.
* Remove SPORK_15_DETERMINISTIC_MNS_ENABLED
* Replace all uses of IsDeterministicMNsSporkActive with IsDIP3Active
* Remove DIP3 upgrade-path tests and directly start with DIP3 enabled tests
* Make -masternodeprivkey non-mandatory
This code will vanish later.
* Bump PROTOCOL_VERSION and DMN_PROTO_VERSION to 70213
This will disconnect all nodes from the old testnet chain when DIP3 gets
activated through BIP9.
Also update comments where 70212/70213 was referenced.
* Also bump MIN_PEER_PROTO_VERSION_DIP3 and MIN_PRIVATESEND_PEER_PROTO_VERSION
* Implement creation and propagation of dummy contributions
These act as a ping which is broadcast a few blocks before the dummy
commitments are created. They are meant to determine online/offline members.
* Use information about received dummy contributions to determine validMembers
* Fix PoSe tests
* Fix dummy DKG phase progress in PoSe tests and give tests more time
Mine one block at a time until we reach the mining phase.
* Fix spork syncing issue in sporks tests
https://github.com/dashpay/dash/pull/2522 caused an issue with sporks syncing
in tests. The introduced time check in CMasternodeSync::ProcessTick causes
masternode sync to never start when mocktime is enabled, so this commit
disables mocktime for sporks.py.
Disabling mocktime however leads to fInitialDownload never becoming false
in CMasternodeSync::UpdatedBlockTip, so mnsync is never started. To fix
this, the tests now create a block before connecting the last node.
This however doesn't work because node1 will ignore the "getheaders" request
from node2 as it has not finished mnsync yet...so we also have to force
finish mnsync for node1.
* Also respond with getdata for announced sporks while in IBD
There was never a good reason to ignore spork announcements while in IBD.
At the same time, this poses the risk of missing out on sporks while in IBD.
This also fixes an issue in sporks testing, as nodes did not request for
announced sporks.
* Use wait_to_sync instead of custom loop
This is moving up the RemoveAskFor call above the ProcessTxLockRequest
call. If ProcessTxLockRequest fails, we should not re-request the same
TX/IX from other nodes as it will continue to fail.
* Move deserialization of REJECT message fields out of if(fDebug)
* Ban nodes which keep requesting and then rejecting blocks
* Don't read/deserialize reject hash twice
* Only use ban score of 1
* Add SPORK_17_QUORUM_DKG_ENABLED spork
* Implement CDummyDKG and CDummyCommitment until we have the real DKG merged
This is only used on testnet/devnet/regtest and will NEVER be used on
mainnet. It is NOT SECURE AT ALL!
See comment in quorums_dummydkg.h for more details.
* Test simple PoSe in DIP3 tests
* Generate 2 instead of 4 blocks per iteration in PoSe tests
4 was based on old chainparams where I used larger phases.
* Only sleep when necessary in PoSe tests
* Fix typo in comment
* Give PoSe tests more time and sync after fast-forward
* Add LLMQ parameters to consensus params
* Add DIP6 quorum commitment special TX
* Implement CQuorumBlockProcessor which validates and handles commitments
* Add quorum commitments to new blocks
* Propagate QFCOMMITMENT messages to all nodes
* Allow special transactions in blocks which have no inputs/outputs
But only for TRANSACTION_QUORUM_COMMITMENT for now.
* Add quorum commitments to self-crafted blocks in DIP3 tests
* Add simple fork logic for current testnet
This should avoid a fork on the current testnet. It only applies to the
current chain which activated DIP3 at height 264000 and block
00000048e6e71d4bd90e7c456dcb94683ae832fcad13e1760d8283f7e89f332f.
When we revert the chain to retest the DIP3 deployment, this fork logic
can be removed again.
* Use quorumVvecHash instead of quorumHash to make null commitments unique
Implementation of https://github.com/dashpay/dips/pull/31
* Re-add quorum commitments after pruning mempool selected blocks
* Refactor CQuorumBlockProcessor::ProcessBlock to have less nested if/else statements
Also add BEGIN/END markers for temporary code.
* Add comments/documentation to LLMQParams
* Move code which determines if a commitment is required into IsCommitmentRequired
This should make the code easier to read and also removes some duplication.
The also changes the error types that are possible from 3 to 2 now. Instead
of having "bad-qc-already-mined" and "bad-qc-not-mining-phase", there is
only "bad-qc-not-allowed" now.
* Use new parameter from consensus parames for the temporary fork
* Implement RemoveAskFor to indicate that we're not interested in an item anymore
When an INV item is received from the first node, the item is requested
immediately. If the same item is received from another node, an entry is
added to mapAskFor which marks the item for re-requesting in case the first
node did not respond. When the item is received from the first node,
the item was previously never removed from mapAskFor. Only the later getdata
loop in SendMessages would then gradually remove items from the map. This
is quite delayed however as the entries in mapAskFor have a timeout value.
RemoveAskFor allows to remove all entries from mapAskFor and setAskFor
when we are not interested in the item anymore (e.g. because we received
it already).
* Call RemoveAskFor whenever we receive a message
* Only pass hash instead of CInv object to RemoveAskFor
* Use BLS keys for operator keys
* Add "bls generate" RPC to generate BLS keys
* Use unique_ptr to store blsKeyOperator and blsPubKeyOperator
Needed because the Chia BLS library crashes when keys are created before
the library is initialized, even if keys are not used. This is the case here
as we have static instances here.
* Remove unnecessary CheckSignature calls
This seems to be some garbage I left in by mistake.
* Fixed review comments
* Fix rpc help for operator keys
All keys that are used as examples are random. None of the secret keys
belongs to any of the public keys.
* Use .GetHash() instead of ::SerializeHash() for BLS pubkeys in txmempool.cpp
* Rename mapProTxBlsPubKeys to mapProTxBlsPubKeyHashes
* add locktransaction rpc call
* Remove special instantsend fee for simple transactions
* Function to check if trx is simple enough to be autolocked
* Automatic lock for all received from peers simple trxes
If we get a new transaction with CInv message and it is "simple" and
is accepted in mempool, we initiate its lock. We don't lock orphan trxes
that accepted in mempool after this trx because they are locked by other
peers.
* Automatically lock simple trxes in wallet
* protocol bump for InstantSend without special fee
* Add function to detect used mempool share
* Mempool threshold for auto IX locks
* Add SPORK_16_INSTANTSEND_AUTOLOCKS spork
* Make autolocks active only when spork SPORK_16_INSTANTSEND_AUTOLOCKS is active
* BIP9 autolocks activation
* revert increasing min peer protocol version for mn rank
* move IsTrxSimple check to CTxLockRequest class
* make MAX_INPUTS_FOR_AUTO_IX private member of CTxLockRequest class
* make AUTO_IX_MEMPOOL_THRESHOLD private member of CInstantSend class
* remove locktransaction RPC call
* tests for automatic IS locks
* fix mempool threshod calculation
* bump mocktime in activate_autoix_bip9
* set node times
* no need to spam the node with gettransaction rpc requests that often
* use `spork active` instead of leaking spork logic into tests
* codestyle fixes
* add test description in comments
* fix typo
* sync test nodes more often during BIP9 activation
* Use 4th bit in BIP9 activation
* Fix comments according codestyle guide
* Call AcceptLockRequest and Vote at the first node creating autoix lock
* fix mempool used memory calculation
* rallback not necessary change in CWallet::CreateTransaction
* test for stopping autolocks for full mempool
* Inject "simple autolockable" txes into txlockrequest logic
* Remove leftover RBF code from BTC
* remove rbf #include
* remove rbf in rpc-tests
* removes replace-by-fee.py
* remove help text related to rbf
* remove comment text relating to rbf
* remove "-mempoolreplacement" cli option
* Remove (effectively dead) RBF code which would never have been called anyway and some assosiated variables
* since `setConflicts` is always empty, this is dead code
* Since we don't have RBF, don't have to do this check. Also, since `setConflicts` is always empty this is dead code
* removes unneccesary if as it will always be true
* remove unused `set<uint256> setConflicts`
* Removes replacement of conflicting txs, as conflicting txs are never accepted
* removes RBF from `validForFeeEstimation`
* removes (probably) unnecessary lock
* remove replacing part of the AcceptToMemoryPool and AcceptToMemoryPoolWIthTime
* fixes err in ps.cpp, didn't remove arg
* RBF in net_processing.cpp
* remove arg in ps-server.cpp
* removes another arg in PS code
* removes rawtx.c AcceptToMemoryPool arg
* removes arg in txvalidationcache_tests.cpp
* remove extra args
* forgot an arg
* fix typo in 82898b0
* remove unused fEnableReplacement in validation.h
* remove the removal reason REPLACED in txmempool.h
* removed unused variable
* comment typo
* Split keyIDMasternode into keyIDOwner/keyIDOperator/keyIDVoting
keyIDOwner is the key used for things which should stay in control of the
collateral owner, like proposal voting.
keyIDOperator is the key used for operational things, like signing network
messages, signing trigger/watchdog objects and trigger votes.
keyIDVoting is the key used for proposal voting
Legacy masternodes will always have the same key for all 3 to keep
compatibility.
Using different keys is only allowed after spork15 activation.
* Forbid reusing collateral keys for operator/owner keys and vice versa
* Bump SERIALIZATION_VERSION_STRING in CMasternodeMan
* CSporkManager class serialization
* Read/write sporks.dat file
* Move mapSporks into CSporkManager and serialize it
* fix GetSporkByHash
* spork tests
* add missed cs lock
* clear mapSporksByHash in CSporkManager::Clear
* use spork active rpc call to hide spork inner logic
* set small pause between rpc calls in cycles
* Split CActiveMasternode into CActiveMasternodeInfo and CLegacyActiveMasternodeManager
* Use CKeyID instead of CPubKey whenever possible in masternode code
* Rename activeMasternode to activeMasternodeInfo and make it a struct
* Rename pubKeyIDXXX to keyIDXXX
* Bump SERIALIZATION_VERSION_STRING
* Fix build error after rebase
* Fix compilation warning/error with clang
Messages should be serialized according the protocol of the peer who asked us or otherwise peers running on other protocols won't be able to deserialize the message correctly.
* Drop CInstantSend::IsEnoughOrphanVotesForTxAndOutPoint
* small cleanup in rpc tests
* move some pieces from .h to .cpp
* fix few log outputs
* fix some comments
* some trivial fixes + readability
* Drop custom logic for delaying GETHEADERS
Reverts "Fix duplicate headers download in initial sync (#1589)" and all following fixes
This reverts commit 169afafd50.
* Fix duplicate initial headers sync
* Merge #8068: Compact Blocks
48efec8 Fix some minor compact block issues that came up in review (Matt Corallo)
ccd06b9 Elaborate bucket size math (Pieter Wuille)
0d4cb48 Use vTxHashes to optimize InitData significantly (Matt Corallo)
8119026 Provide a flat list of txid/terators to txn in CTxMemPool (Matt Corallo)
678ee97 Add BIP 152 to implemented BIPs list (Matt Corallo)
56ba516 Add reconstruction debug logging (Matt Corallo)
2f34a2e Get our "best three" peers to announce blocks using cmpctblocks (Matt Corallo)
927f8ee Add ability to fetch CNode by NodeId (Matt Corallo)
d25cd3e Add receiver-side protocol implementation for CMPCTBLOCK stuff (Matt Corallo)
9c837d5 Add sender-side protocol implementation for CMPCTBLOCK stuff (Matt Corallo)
00c4078 Add protocol messages for short-ids blocks (Matt Corallo)
e3b2222 Add some blockencodings tests (Matt Corallo)
f4f8f14 Add TestMemPoolEntryHelper::FromTx version for CTransaction (Matt Corallo)
85ad31e Add partial-block block encodings API (Matt Corallo)
5249dac Add COMPACTSIZE wrapper similar to VARINT for serialization (Matt Corallo)
cbda71c Move context-required checks from CheckBlockHeader to Contextual... (Matt Corallo)
7c29ec9 If AcceptBlockHeader returns true, pindex will be set. (Matt Corallo)
96806c3 Stop trimming when mapTx is empty (Pieter Wuille)
* Merge #8408: Prevent fingerprinting, disk-DoS with compact blocks
1d06e49 Ignore CMPCTBLOCK messages for pruned blocks (Suhas Daftuar)
1de2a46 Ignore GETBLOCKTXN requests for unknown blocks (Suhas Daftuar)
* Merge #8418: Add tests for compact blocks
45c7ddd Add p2p test for BIP 152 (compact blocks) (Suhas Daftuar)
9a22a6c Add support for compactblocks to mininode (Suhas Daftuar)
a8689fd Tests: refactor compact size serialization in mininode (Suhas Daftuar)
9c8593d Implement SipHash in Python (Pieter Wuille)
56c87e9 Allow changing BIP9 parameters on regtest (Suhas Daftuar)
* Merge #8505: Trivial: Fix typos in various files
1aacfc2 various typos (leijurv)
* Merge #8449: [Trivial] Do not shadow local variable, cleanup
a159f25 Remove redundand (and shadowing) declaration (Pavel Janík)
cce3024 Do not shadow local variable, cleanup (Pavel Janík)
* Merge #8739: [qa] Fix broken sendcmpct test in p2p-compactblocks.py
157254a Fix broken sendcmpct test in p2p-compactblocks.py (Suhas Daftuar)
* Merge #8854: [qa] Fix race condition in p2p-compactblocks test
b5fd666 [qa] Fix race condition in p2p-compactblocks test (Suhas Daftuar)
* Merge #8393: Support for compact blocks together with segwit
27acfc1 [qa] Update p2p-compactblocks.py for compactblocks v2 (Suhas Daftuar)
422fac6 [qa] Add support for compactblocks v2 to mininode (Suhas Daftuar)
f5b9b8f [qa] Fix bug in mininode witness deserialization (Suhas Daftuar)
6aa28ab Use cmpctblock type 2 for segwit-enabled transfer (Pieter Wuille)
be7555f Fix overly-prescriptive p2p-segwit test for new fetch logic (Matt Corallo)
06128da Make GetFetchFlags always request witness objects from witness peers (Matt Corallo)
* Merge #8882: [qa] Fix race conditions in p2p-compactblocks.py and sendheaders.py
b55d941 [qa] Fix race condition in sendheaders.py (Suhas Daftuar)
6976db2 [qa] Another attempt to fix race condition in p2p-compactblocks.py (Suhas Daftuar)
* Merge #8904: [qa] Fix compact block shortids for a test case
4cdece4 [qa] Fix compact block shortids for a test case (Dagur Valberg Johannsson)
* Merge #8637: Compact Block Tweaks (rebase of #8235)
3ac6de0 Align constant names for maximum compact block / blocktxn depth (Pieter Wuille)
b2e93a3 Add cmpctblock to debug help list (instagibbs)
fe998e9 More agressively filter compact block requests (Matt Corallo)
02a337d Dont remove a "preferred" cmpctblock peer if they provide a block (Matt Corallo)
* Merge #8975: Chainparams: Trivial: In AppInit2(), s/Params()/chainparams/
6f2f639 Chainparams: Trivial: In AppInit2(), s/Params()/chainparams/ (Jorge Timón)
* Merge #8968: Don't hold cs_main when calling ProcessNewBlock from a cmpctblock
72ca7d9 Don't hold cs_main when calling ProcessNewBlock from a cmpctblock (Matt Corallo)
* Merge #8995: Add missing cs_main lock to ::GETBLOCKTXN processing
dfe7906 Add missing cs_main lock to ::GETBLOCKTXN processing (Matt Corallo)
* Merge #8515: A few mempool removal optimizations
0334430 Add some missing includes (Pieter Wuille)
4100499 Return shared_ptr<CTransaction> from mempool removes (Pieter Wuille)
51f2783 Make removed and conflicted arguments optional to remove (Pieter Wuille)
f48211b Bypass removeRecursive in removeForReorg (Pieter Wuille)
* Merge #9026: Fix handling of invalid compact blocks
d4833ff Bump the protocol version to distinguish new banning behavior. (Suhas Daftuar)
88c3549 Fix compact block handling to not ban if block is invalid (Suhas Daftuar)
c93beac [qa] Test that invalid compactblocks don't result in ban (Suhas Daftuar)
* Merge #9039: Various serialization simplifcations and optimizations
d59a518 Use fixed preallocation instead of costly GetSerializeSize (Pieter Wuille)
25a211a Add optimized CSizeComputer serializers (Pieter Wuille)
a2929a2 Make CSerAction's ForRead() constexpr (Pieter Wuille)
a603925 Avoid -Wshadow errors (Pieter Wuille)
5284721 Get rid of nType and nVersion (Pieter Wuille)
657e05a Make GetSerializeSize a wrapper on top of CSizeComputer (Pieter Wuille)
fad9b66 Make nType and nVersion private and sometimes const (Pieter Wuille)
c2c5d42 Make streams' read and write return void (Pieter Wuille)
50e8a9c Remove unused ReadVersion and WriteVersion (Pieter Wuille)
* Merge #9058: Fixes for p2p-compactblocks.py test timeouts on travis (#8842)
dac53b5 Modify getblocktxn handler not to drop requests for old blocks (Russell Yanofsky)
55bfddc [qa] Fix stale data bug in test_compactblocks_not_at_tip (Russell Yanofsky)
47e9659 [qa] Fix bug in compactblocks v2 merge (Russell Yanofsky)
* Merge #9160: [trivial] Fix hungarian variable name
ec34648 [trivial] Fix hungarian variable name (Russell Yanofsky)
* Merge #9159: [qa] Wait for specific block announcement in p2p-compactblocks
dfa44d1 [qa] Wait for specific block announcement in p2p-compactblocks (Russell Yanofsky)
* Merge #9125: Make CBlock a vector of shared_ptr of CTransactions
b4e4ba4 Introduce convenience type CTransactionRef (Pieter Wuille)
1662b43 Make CBlock::vtx a vector of shared_ptr<CTransaction> (Pieter Wuille)
da60506 Add deserializing constructors to CTransaction and CMutableTransaction (Pieter Wuille)
0e85204 Add serialization for unique_ptr and shared_ptr (Pieter Wuille)
* Merge #8872: Remove block-request logic from INV message processing
037159c Remove block-request logic from INV message processing (Matt Corallo)
3451203 [qa] Respond to getheaders and do not assume a getdata on inv (Matt Corallo)
d768f15 [qa] Make comptool push blocks instead of relying on inv-fetch (mrbandrews)
* Merge #9199: Always drop the least preferred HB peer when adding a new one.
ca8549d Always drop the least preferred HB peer when adding a new one. (Gregory Maxwell)
* Merge #9233: Fix some typos
15fa95d Fix some typos (fsb4000)
* Merge #9260: Mrs Peacock in The Library with The Candlestick (killed main.{h,cpp})
76faa3c Rename the remaining main.{h,cpp} to validation.{h,cpp} (Matt Corallo)
e736772 Move network-msg-processing code out of main to its own file (Matt Corallo)
87c35f5 Remove orphan state wipe from UnloadBlockIndex. (Matt Corallo)
* Merge #9014: Fix block-connection performance regression
dd0df81 Document ConnectBlock connectTrace postconditions (Matt Corallo)
2d6e561 Switch pblock in ProcessNewBlock to a shared_ptr (Matt Corallo)
2736c44 Make the optional pblock in ActivateBestChain a shared_ptr (Matt Corallo)
ae4db44 Create a shared_ptr for the block we're connecting in ActivateBCS (Matt Corallo)
fd9d890 Keep blocks as shared_ptrs, instead of copying txn in ConnectTip (Matt Corallo)
6fdd43b Add struct to track block-connect-time-generated info for callbacks (Matt Corallo)
* Merge #9240: Remove txConflicted
a874ab5 remove internal tracking of mempool conflicts for reporting to wallet (Alex Morcos)
bf663f8 remove external usage of mempool conflict tracking (Alex Morcos)
* Merge #9344: Do not run functions with necessary side-effects in assert()
da9cdd2 Do not run functions with necessary side-effects in assert() (Gregory Maxwell)
* Merge #9273: Remove unused CDiskBlockPos* argument from ProcessNewBlock
a13fa4c Remove unused CDiskBlockPos* argument from ProcessNewBlock (Matt Corallo)
* Merge #9352: Attempt reconstruction from all compact block announcements
813ede9 [qa] Update compactblocks test for multi-peer reconstruction (Suhas Daftuar)
7017298 Allow compactblock reconstruction when block is in flight (Suhas Daftuar)
* Merge #9252: Release cs_main before calling ProcessNewBlock, or processing headers (cmpctblock handling)
bd02bdd Release cs_main before processing cmpctblock as header (Suhas Daftuar)
680b0c0 Release cs_main before calling ProcessNewBlock (cmpctblock handling) (Suhas Daftuar)
* Merge #9283: A few more CTransactionRef optimizations
91335ba Remove unused MakeTransactionRef overloads (Pieter Wuille)
6713f0f Make FillBlock consume txn_available to avoid shared_ptr copies (Pieter Wuille)
62607d7 Convert COrphanTx to keep a CTransactionRef (Pieter Wuille)
c44e4c4 Make AcceptToMemoryPool take CTransactionRef (Pieter Wuille)
* Merge #9375: Relay compact block messages prior to full block connection
02ee4eb Make most_recent_compact_block a pointer to a const (Matt Corallo)
73666ad Add comment to describe callers to ActivateBestChain (Matt Corallo)
962f7f0 Call ActivateBestChain without cs_main/with most_recent_block (Matt Corallo)
0df777d Use a temp pindex to avoid a const_cast in ProcessNewBlockHeaders (Matt Corallo)
c1ae4fc Avoid holding cs_most_recent_block while calling ReadBlockFromDisk (Matt Corallo)
9eb67f5 Ensure we meet the BIP 152 old-relay-types response requirements (Matt Corallo)
5749a85 Cache most-recently-connected compact block (Matt Corallo)
9eaec08 Cache most-recently-announced block's shared_ptr (Matt Corallo)
c802092 Relay compact block messages prior to full block connection (Matt Corallo)
6987219 Add a CValidationInterface::NewPoWValidBlock callback (Matt Corallo)
180586f Call AcceptBlock with the block's shared_ptr instead of CBlock& (Matt Corallo)
8baaba6 [qa] Avoid race in preciousblock test. (Matt Corallo)
9a0b2f4 [qa] Make compact blocks test construction using fetch methods (Matt Corallo)
8017547 Make CBlockIndex*es in net_processing const (Matt Corallo)
* Merge #9486: Make peer=%d log prints consistent
e6111b2 Make peer id logging consistent ("peer=%d" instead of "peer %d") (Matt Corallo)
* Merge #9400: Set peers as HB peers upon full block validation
d4781ac Set peers as HB peers upon full block validation (Gregory Sanders)
* Merge #9499: Use recent-rejects, orphans, and recently-replaced txn for compact-block-reconstruction
c594580 Add braces around AddToCompactExtraTransactions (Matt Corallo)
1ccfe9b Clarify comment about mempool/extra conflicts (Matt Corallo)
fac4c78 Make PartiallyDownloadedBlock::InitData's second param const (Matt Corallo)
b55b416 Add extra_count lower bound to compact reconstruction debug print (Matt Corallo)
863edb4 Consider all (<100k memusage) txn for compact-block-extra-txn cache (Matt Corallo)
7f8c8ca Consider all orphan txn for compact-block-extra-txn cache (Matt Corallo)
93380c5 Use replaced transactions in compact block reconstruction (Matt Corallo)
1531652 Keep shared_ptrs to recently-replaced txn for compact blocks (Matt Corallo)
edded80 Make ATMP optionally return the CTransactionRefs it replaced (Matt Corallo)
c735540 Move ORPHAN constants from validation.h to net_processing.h (Matt Corallo)
* Merge #9587: Do not shadow local variable named `tx`.
44f2baa Do not shadow local variable named `tx`. (Pavel Janík)
* Merge #9510: [trivial] Fix typos in comments
cc16d99 [trivial] Fix typos in comments (practicalswift)
* Merge #9604: [Trivial] add comment about setting peer as HB peer.
dd5b011 [Trivial] add comment about setting peer as HB peer. (John Newbery)
* Fix using of AcceptToMemoryPool in PrivateSend code
* add `override`
* fSupportsDesiredCmpctVersion
* bring back tx ressurection in DisconnectTip
* Fix delayed headers
* Remove unused CConnman::FindNode overload
* Fix typos and comments
* Fix minor code differences
* Don't use rejection cache for corrupted transactions
Partly based on https://github.com/bitcoin/bitcoin/pull/8525
* Backport missed cs_main locking changes
Missed from 58a215ce8c
* Backport missed comments and mapBlockSource.emplace call
Missed from two commits:
88c35491ab7c98ce584e
* Add CheckPeerHeaders() helper and check in (nCount == 0) too
* Remove remains of workaround that was needed while backporting
* Add missing closing round bracket to help string
* Remove now unnecessary .encode() calls in wallet.py
Now that we only support python3, we can fix this TODO.
* Use Dash block for DeserializeAndCheckBlockTest
Replaced Bitcoin block with the largest Dash block I could find on mainnet.
* Store hashDevnetGenesisBlock in Consensus::Params
Remove the need for chainparams to be available when the devnetGenesis hash
is needed. Fixes a crash in CheckBlockHeader() when called from benchmarking
code, which does not initialize the Params() function.
d943491 qa: add a test to detect leaky p2p messages (Cory Fields)
8650bbb qa: Expose on-connection to mininode listeners (Matt Corallo)
5b5e4f8 qa: mininode learns when a socket connects, not its first action (Matt Corallo)
cbfc5a6 net: require a verack before responding to anything else (Cory Fields)
8502e7a net: parse reject earlier (Cory Fields)
c45b9fb net: correctly ban before the handshake is complete (Cory Fields)
0729102 Net: pass interruptMsgProc as const where possible (Jorge Timón)
fc7f2ff Net: Make CNetMsgMaker more const (Jorge Timón)
d45955f Net: CConnman: Make some methods const (Jorge Timón)
032ba3f RPC help documentation for addnode peerinfo. (Gregory Maxwell)
90f13e1 Add release notes for addnode changes. (Gregory Maxwell)
50bd12c Break addnode out from the outbound connection limits. (Gregory Maxwell)
dd0df81 Document ConnectBlock connectTrace postconditions (Matt Corallo)
2d6e561 Switch pblock in ProcessNewBlock to a shared_ptr (Matt Corallo)
2736c44 Make the optional pblock in ActivateBestChain a shared_ptr (Matt Corallo)
ae4db44 Create a shared_ptr for the block we're connecting in ActivateBCS (Matt Corallo)
fd9d890 Keep blocks as shared_ptrs, instead of copying txn in ConnectTip (Matt Corallo)
6fdd43b Add struct to track block-connect-time-generated info for callbacks (Matt Corallo)
81e3228 Make CTransaction actually immutable (Pieter Wuille)
42fd8de Make DecodeHexTx return a CMutableTransaction (Pieter Wuille)
c3f5673 Make CWalletTx store a CTransactionRef instead of inheriting (Pieter Wuille)
a188353 Switch GetTransaction to returning a CTransactionRef (Pieter Wuille)
Instead of deriving from CTransaction, we now have a CTransactionRef member
in CTxLockCandidate. This is needed for the next backported PR #8580,
which will make CTransaction immutable.
Also use CTransactionRef in CDarkSendEntry, CDarksendBroadcastTx and
CPrivateSendServer
c7be56d net: push only raw data into CConnman (Cory Fields)
2ec935d net: add CVectorWriter and CNetMsgMaker (Cory Fields)
b7695c2 net: No need to check individually for disconnection anymore (Cory Fields)
fedea8a net: don't send any messages before handshake or after requested disconnect (Cory Fields)
d74e352 net: Set feelers to disconnect at the end of the version message (Cory Fields)
d2b88f9 Move orphan-conflict removal from main logic into a callback (Matt Corallo)
97e2802 Erase orphans per-transaction instead of per-block (Matt Corallo)
ec4525c Move orphan processing to ActivateBestChain (Matt Corallo)
037159c Remove block-request logic from INV message processing (Matt Corallo)
3451203 [qa] Respond to getheaders and do not assume a getdata on inv (Matt Corallo)
d768f15 [qa] Make comptool push blocks instead of relying on inv-fetch (mrbandrews)
b4e4ba4 Introduce convenience type CTransactionRef (Pieter Wuille)
1662b43 Make CBlock::vtx a vector of shared_ptr<CTransaction> (Pieter Wuille)
da60506 Add deserializing constructors to CTransaction and CMutableTransaction (Pieter Wuille)
0e85204 Add serialization for unique_ptr and shared_ptr (Pieter Wuille)
1df3111 protocol.h: Make enums in GetDataMsg concrete values (Wladimir J. van der Laan)
2c09a52 protocol.h: Move MESSAGE_START_SIZE into CMessageHeader (Wladimir J. van der Laan)
f9bd92d version.h: s/shord/short/ in comment (Wladimir J. van der Laan)
54326a6 Increase maximum orphan size to 100,000 bytes. (Gregory Maxwell)
8c99d1b Treat orphans as implicit inv for parents, discard when parents rejected. (Gregory Maxwell)
11cc143 Adds an expiration time for orphan tx. (Gregory Maxwell)
db0ffe8 This eliminates the primary leak that causes the orphan map to always grow to its maximum size. (Gregory Maxwell)
1b0bcc5 Track orphan by prev COutPoint rather than prev hash (Pieter Wuille)
ff2dcf2 Tests: Edit bloated varint test and add option for 'barely expensive' tests (mrbandrews)
12c5a16 Catch exceptions from non-canonical encoding and print only to log (mrbandrews)
291f8aa Continuing port of java comptool (mrbandrews)
8c9e681 Tests: Rework blockstore to avoid re-serialization. (mrbandrews)
eebc232 test: Add more test vectors for siphash (Wladimir J. van der Laan)
8884830 Use C++11 thread-safe static initializers (Pieter Wuille)
c31b24f Use 64-bit SipHash of netgroups in eviction (Pieter Wuille)
9bf156b Support SipHash with arbitrary byte writes (Pieter Wuille)
053930f Avoid recalculating vchKeyedNetGroup in eviction logic. (Patrick Strateman)
288d85d Get rid of CTxMempool::lookup() entirely (Pieter Wuille)
c2a4724 Optimization: use usec in expiration and reuse nNow (Pieter Wuille)
e9b4780 Optimization: don't check the mempool at all if no mempool req ever (Pieter Wuille)
dbfb426 Optimize the relay map to use shared_ptr's (Pieter Wuille)
8d39d7a Switch CTransaction storage in mempool to std::shared_ptr (Pieter Wuille)
1b9e6d3 Add support for unique_ptr and shared_ptr to memusage (Pieter Wuille)
3d3602f Add RPC test for the p2p mempool command in conjunction with disabled bloomfilters (Jonas Schnelli)
beceac9 Disable the mempool P2P command when bloom filters disabled (Peter Todd)
Bitcoin #7877 changed mapRelay to be indexed by hash instead of inv. This
means that we may end up with a false-positive match here and send out an
instant transaction as normal transaction.
Fix was introdeced in:
https://github.com/dashpay/dash/pull/1169
A fix was later applied to fix a race condition:
https://github.com/dashpay/dash/pull/1178
This has to be reverted to be able to backport multiple changes from Bitcoin.
Later backported PRs from Bitcoin will remove cs_mapRelay and thus also
remove the deadlock that was initially fixed with the above PRs.
a68ec21 Use SipHash-2-4 for address relay selection (Pieter Wuille)
8cc9cfe Switch CTxMempool::mapTx to use a hash index for txids (Pieter Wuille)
382c871 Use SipHash-2-4 for CCoinsCache index (Pieter Wuille)
0b1295b Add SipHash-2-4 primitives to hash (Pieter Wuille)
Bitcoin #7840 has split the INVs to send into block and TX and completely
ignores non-tx/non-block items in PushInventory. This is fine for Bitcoin,
as they only use it for blocks and TXs, but we also have a lot of MN related
messages which also need to be relayed.
b559914 Move bloom and feerate filtering to just prior to tx sending. (Gregory Maxwell)
4578215 Return mempool queries in dependency order (Pieter Wuille)
ed70683 Handle mempool requests in send loop, subject to trickle (Pieter Wuille)
dc13dcd Split up and optimize transaction and block inv queues (Pieter Wuille)
f2d3ba7 Eliminate TX trickle bypass, sort TX invs for privacy and priority. (Gregory Maxwell)
* Initial devnet
* Move genesis block adding into its own method
* Introduce -allowprivatenet to lift limitation on RFC1918 addresses
Normally, RFC1918 (192.168.x.x/10.x.x.x/...) addresses are not allowed
to be relayed. Also, masternodes won't start when the address is considered
invalid.
This is needed to test local devnet or regtest based networks.
* Lift the requirement of minimum MN age for regtest/devnet
* Implement named devnets
This allows the creation of multiple independent devnets. Each one is
identified by a name which is hardened into a "devnet genesis" block,
which is automatically positioned at height 1. Validation rules will
ensure that a node from devnet=test1 never be able to accept blocks
from devnet=test2. This is done by checking the expected devnet genesis
block.
The genesis block of the devnet is the same as the one from regtest. This
starts the devnet with a very low difficulty, allowing us to fill up
needed balances for masternodes very fast.
Also, the devnet name is put into the sub-version of the VERSION message.
If a node connects to the wrong network, it will immediately be disconnected.
* Allow to select multiple addresses from the same group in devnet/regtest
The selection code normally only allows to select addresses from the same
group (e.g. 192.168.x.x) once. This results in connecting to only a single
node in devnet/regtest.
* Show the devnet name in the title bar and on the loading screen
* Add AllowMultipleAddressesFromGroup to chainparams and use it in net.cpp
* Remove unused/unneeded scripts from devnet geneses creation
1. OP_RETURN not needed in input script of devnet genesis
2. genesisOutputScript was unused
* Fix copy/paste error in -allowprivatenet description
* Improve -devnet parameter error handling
- Only allow one of -devnet, -regtest or -testnet
- Only allow -devnet=name to be specified once
* Use different datadir for each devnet
* Fix `devnet-devnet` issue
* Fix devnet splashscreen (should use testnet img)
* Avoid passing devNetName around (most of the time)
* Remove nMaxTipAge from CDevNetParams
Not present anymore after rebase on develop
