Age | Commit message (Collapse) | Author |
|
6aed8b7e9b206e728367fee9edfeb85b536bba69 [test] tx processing before and after ibd (glozow)
b9e105b6643bada4c80f8d521394be4d55e8e4fc [net_processing] ignore all transactions during ibd (glozow)
Pull request description:
This is basically a mini, IBD-only version of #21224
Incoming transactions aren't really relevant until we're caught up. That's why we send a giant feefilter and don't send tx getdatas, but we also shouldn't process them if peers send them anyway. Simply ignore them.
ACKs for top commit:
jnewbery:
reACK 6aed8b7e9b
laanwj:
Code review ACK 6aed8b7e9b206e728367fee9edfeb85b536bba69
Tree-SHA512: 8e1616bf355f9d0b180bdbc5461f24c757dc5d7bc7bf651470f3b0bffcca5d5e68287106255b5cede2d96b42bce448a0f8c0649de35a530c5e079f7c89c70a35
|
|
|
|
CTxMemPool::check() will carry out internal consistency checks 1/n times,
where n is set by the `-checkmempool` configuration option. By default,
mempool consistency checks are disabled entirely on mainnet.
Therefore, this change has no effect on mainnet nodes running with
default configuration. It simply removes the responsibility to trigger
mempool consistency checks from net_processing.
|
|
|
|
This just calls through to AcceptToMemoryPool() internally, and is currently unused.
Also add a new transaction validation failure reason TX_NO_MEMPOOL to
indicate that there is no mempool.
|
|
Co-authored-by: Amiti Uttarwar <amiti@uttarwar.org>
|
|
performance of check() and remove dependency on validation
082c5bf099c64e3d27abe9b68a71ce500b693e7e [refactor] pass coinsview and height to check() (glozow)
ed6115f1eae0eb4669601106a9aaff078a2f3a74 [mempool] simplify some check() logic (glozow)
9e8d7ad5d9cc4b013826daead9cee09aad539401 [validation/mempool] use Spend/AddCoin instead of UpdateCoins (glozow)
09d18916afb0ecae90700d4befd9d5dc52767970 MOVEONLY: remove single-use helper func CheckInputsAndUpdateCoins (glozow)
e8639ec26aaf4de3fae280963434bf1cf2017b6f [mempool] remove now-unnecessary code (glozow)
54c6f3c1da01090aee9691a2c2bee0984a054ce8 [mempool] speed up check() by using coins cache and iterating in topo order (glozow)
30e240f65e69c6dffcd033afc63895345bd51f53 [bench] Benchmark CTxMemPool::check() (glozow)
cb1407196fba648aa75504e3ab3d46aa0181563a [refactor/bench] make mempool_stress bench reusable and parameterizable (glozow)
Pull request description:
Remove the txmempool <-> validation circular dependency by removing txmempool's dependency on validation. There are two functions in txmempool that need validation right now: `check()` and `removeForReorg()`. This PR removes the dependencies in `check()`.
This PR also improves the performance of `CTxMemPool::check()` by walking through the entries exactly once, in ascending ancestorcount order, which guarantees that we see parents before children.
ACKs for top commit:
jnewbery:
reACK 082c5bf099c64e3d27abe9b68a71ce500b693e7e
GeneFerneau:
tACK [082c5bf](https://github.com/bitcoin/bitcoin/pull/23157/commits/082c5bf099c64e3d27abe9b68a71ce500b693e7e)
rajarshimaitra:
tACK https://github.com/bitcoin/bitcoin/pull/23157/commits/082c5bf099c64e3d27abe9b68a71ce500b693e7e
theStack:
Code-review ACK 082c5bf099c64e3d27abe9b68a71ce500b693e7e
Tree-SHA512: 40ac622af1627b5c3e6abb4f0f035d833265a8c5e8dc88faf5354875dfb5137f137825e54bbd2a2668ed37b145c5d02285f776402629f58596e51853a9a79d29
|
|
fa4ec1c0bdaef9f082a6661d7faf16149774e145 Make GenTxid boolean constructor private (MarcoFalke)
faeb9a575367119dbff60c35fa2c13547718e179 remove unused CTxMemPool::info(const uint256& txid) (MarcoFalke)
Pull request description:
This boolean argument is either verbose (when used with a named arg) or unintuitive and dangerous (when used as a plain bool).
Fix that by making the constructor private.
ACKs for top commit:
laanwj:
Code review ACK fa4ec1c0bdaef9f082a6661d7faf16149774e145
jnewbery:
Code review ACK fa4ec1c0bdaef9f082a6661d7faf16149774e145
glozow:
code review ACK fa4ec1c0bdaef9f082a6661d7faf16149774e145
Tree-SHA512: bf08ee09168885cfda71e5a01ec412b93964662a90dd9d91e75f7fdf2eaff7c21a95204d0e90b00438bfeab564d0aea66bdb9c0394ee7a05743e65a817159446
|
|
disconnecting misbehaving peer
fa2662c293ec0aaa93092b59b6632f74729c4283 net: Avoid logging AlreadyHaveTx when disconnecting misbehaving peer (MarcoFalke)
Pull request description:
There is no need to log `AlreadyHaveTx` for an inv when a peer is marked for disconnection due to sending that inv. In fact, I find it confusing that a `block-relay-only` connection calls `AlreadyHaveTx` at all. Also there is no need to call `AddKnownTx` when the peer is marked for disconnection.
ACKs for top commit:
naumenkogs:
ACK fa2662c293ec0aaa93092b59b6632f74729c4283
jnewbery:
Code review ACK fa2662c293ec0aaa93092b59b6632f74729c4283
dunxen:
Concept and code review ACK fa2662c293ec0aaa93092b59b6632f74729c4283
Tree-SHA512: 9996b807a824021f992b5281d82ff0cbbe6a442c2fedf7dfd6adda64ccc5e0ef4fb0ff91ab75086f975837bbbb7a5934ac7e671a80dcababa7203c92fc0c7f84
|
|
|
|
4307849256761fe2440d82bbec892d0e8e6b4dd4 [mempool] delete exists(uint256) function (glozow)
d50fbd4c5b4bc72415854d582cedf94541a46983 create explicit GenTxid::{Txid, Wtxid} ctors (glozow)
Pull request description:
We use the same type for txids and wtxids, `uint256`. In places where we want the ability to pass either one, we distinguish them using `GenTxid`.
The (overloaded) `CTxMemPool::exists()` function is defined as follows:
```c
bool exists(const uint256& txid) const { return exists(GenTxid{false, txid}); }
```
It always assumes that a uint256 is a txid, which is a footgunny interface.
Querying by wtxid returns a false negative if the transaction has a witness. :bug:
Another approach would be to try both:
```c
bool exists(const uint256& txid) const { return exists(GenTxid{false, txid}) || exists(GenTxid{false, txid}); }
```
But that's slower and wrongfully placing the burden on the callee; the caller always knows whether the hash is a txid or a wtxid.
ACKs for top commit:
laanwj:
Code review ACK 4307849256761fe2440d82bbec892d0e8e6b4dd4
jnewbery:
Tested and code review ACK 4307849256761fe2440d82bbec892d0e8e6b4dd4
MarcoFalke:
review ACK 4307849256761fe2440d82bbec892d0e8e6b4dd4 👘
Tree-SHA512: 8ed167a96f3124b6c14e41073c8358658114ce121a15a4cca2db7a5ac565903a6236e34e88ac03382b8bb8b68e3999abbfc5718bc8c22476554d6b49a5298eec
|
|
Allowing callers to pass in a uint256 (which could be txid or wtxid)
but then always assuming that it's a txid is a footgunny interface.
|
|
|
|
details
021f86953e8a1dff8ecc768186368d345c865cc2 [style] Run changed files through clang formatter. (Amiti Uttarwar)
375750387e35ed751d1f5ab48860bdec93977f64 scripted-diff: Rename CAddrInfo to AddrInfo (Amiti Uttarwar)
dd8f7f250095e58bbf4cd4effb481b52143bd1ed scripted-diff: Rename CAddrMan to AddrMan (Amiti Uttarwar)
3c263d3f63c3598954ee2b65a0e721e3c22e52f8 [includes] Fix up included files (Amiti Uttarwar)
29727c2aa1233f7c5b91a17884c405e0aef10c6e [doc] Update comments (Amiti Uttarwar)
14f9e000d05f82b364d5a142cafc70b10406b660 [refactor] Update GetAddr_() function signature (Amiti Uttarwar)
40acd6fc9a8098fed85abf4fb727a5f0dff8a2ff [move-only] Move constants to test-only header (Amiti Uttarwar)
7cf41bbb38db5008f9b69037b88138076d6a6cc5 [addrman] Change CAddrInfo access (Amiti Uttarwar)
e3f1ea659c9eb1e8be4579923d6acaaab148c2ef [move-only] Move CAddrInfo to test-only header file (Amiti Uttarwar)
7cba9d56185b9325ce41d79364e448462fff0f6a [net, addrman] Remove external dependencies on CAddrInfo objects (Amiti Uttarwar)
8af5b54f973e11c847345418d8631bc301b96130 [addrman] Introduce CAddrMan::Impl to encapsulate addrman implementation. (Amiti Uttarwar)
f2e5f38f09ee40933f752680fe7d75ee8e529fae [move-only] Match ordering of CAddrMan declarations and definitions (Amiti Uttarwar)
5faa7dd6d871eac1a0ec5c4a93f2ad7577781a56 [move-only] Move CAddrMan function definitions to cpp (Amiti Uttarwar)
Pull request description:
Introduce the pimpl pattern for AddrMan to separate the implementation details from the externally used object representation. This reduces compile-time dependencies and conceptually clarifies AddrMan's interface from the implementation specifics.
Since the unit & fuzz tests currently rely on accessing AddrMan internals, this PR introduces addrman_impl.h, which is exclusively imported by addrman.cpp and test files.
ACKs for top commit:
jnewbery:
ACK 021f86953e8a1dff8ecc768186368d345c865cc2
GeneFerneau:
utACK [021f869](https://github.com/bitcoin/bitcoin/pull/22950/commits/021f86953e8a1dff8ecc768186368d345c865cc2)
mzumsande:
ACK 021f86953e8a1dff8ecc768186368d345c865cc2
rajarshimaitra:
Concept + Code Review ACK https://github.com/bitcoin/bitcoin/pull/22950/commits/021f86953e8a1dff8ecc768186368d345c865cc2
theuni:
ACK 021f86953e8a1dff8ecc768186368d345c865cc2
Tree-SHA512: aa70cb77927a35c85230163c0cf6d3872382d79048b0fb79341493caa46f8e91498cb787d8b06aba4da17b2f921f2230e73f3d66385519794fff86a831b3a71d
|
|
9d0379cea6c164610d05287ae6dd4e66f35b92b3 consensus: use <cstdint> over <stdint.h> in amount.h (fanquake)
863e52fe63a67fa020fb1ef527b9095a35ab77a5 consensus: make COIN & MAX_MONEY constexpr (fanquake)
d09071da5bc997f2de1f55ca7a9babc3d7619329 [MOVEONLY] consensus: move amount.h into consensus (fanquake)
Pull request description:
A first step (of a few) towards some source code reorganization, as well as making libbitcoinconsensus slightly more self contained.
Related to #15732.
ACKs for top commit:
MarcoFalke:
concept ACK 9d0379cea6c164610d05287ae6dd4e66f35b92b 🏝
Tree-SHA512: 97fc79262dcb8c00996852a288fee69ddf8398ae2c95700bba5b326f1f38ffcfaf8fa66e29d0cb446d9b3f4e608a96525fae0c2ad9cd531ad98ad2a4a687cd6a
|
|
Removes check's dependency on validation.h
|
|
Move amount.h to consensus/amount.h.
Renames, adds missing and removes uneeded includes.
|
|
-BEGIN VERIFY SCRIPT-
git grep -l CAddrMan src/ test/ | xargs sed -i 's/CAddrMan/AddrMan/g'
-END VERIFY SCRIPT-
|
|
|
|
Can be reviewed with --color-moved=dimmed-zebra
|
|
The new name describes better what the bool does and also limits the confusion of the three different concepts:
* fBlocksOnly (This bool to skip tx invs)
* -blocksonly (A setting to ignore incoming txs)
* block-relay-only (A connection type in the block-relay-only P2P graph)
|
|
Improve readability of code, simplify future scripted diff cleanup PRs, and be
more consistent with naming for GetBoolArg.
This will also be useful for replacing runtime settings type checking
with compile time checking.
-BEGIN VERIFY SCRIPT-
git grep -l GetArg | xargs sed -i 's/GetArg(\([^)]*\( [0-9]\+\|-1\|port\|BaseParams().RPCPort()\|Params().GetDefaultPort()\|_TIMEOUT\|Height\|_WORKQUEUE\|_THREADS\|_CONNECTIONS\|LIMIT\|SigOp\|Bytes\|_VERSION\|_AGE\|_CHECKS\|Checks() ? 1 : 0\|_BANTIME\|Cache\|BLOCKS\|LEVEL\|Weight\|Version\|BUFFER\|TARGET\|WEIGHT\|TXN\|TRANSACTIONS\|ADJUSTMENT\|i64\|Size\|nDefault\|_EXPIRY\|HEIGHT\|SIZE\|SNDHWM\|_TIME_MS\)\))/GetIntArg(\1)/g'
-END VERIFY SCRIPT-
Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
|
|
https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines.html#c131-avoid-trivial-getters-and-setters
|
|
The us=... message in the debug log when sending a version message is
always [::]:0, because we no longer send our own address there.
Therefore, this information is entirely redundant. Remove it.
|
|
Historically, the VERSION message contains an "addrMe" and an "addrYou". As
these are sent before version negotiation is complete, the protocol version is
INIT_PROTO_VERSION (209), and in that protocol, CAddress is serialized without
nTime.
This is in fact the only situation left where a CAddress is (de)serialized
without nTime. As it's such a simple structure (CService for ip/port + uint64_t
for nServices), just inline that structure in the few places where it occurs,
and remove the logic for dealing with missing nTime from CAddress.
|
|
d930c7f5b091687eb4208a5ffe8a2abe311d8054 p2p, rpc, test: address rate-limiting follow-ups (Jon Atack)
Pull request description:
Incorporates review feedback in #22387.
Edit, could be considered separately: should a release note (or two) be added for 22.0? e.g. the new getpeerinfo fields in `Updated RPCs` and the rate-limiting itself in `P2P and network changes`.
ACKs for top commit:
MarcoFalke:
review ACK d930c7f5b091687eb4208a5ffe8a2abe311d8054
theStack:
re-ACK d930c7f5b091687eb4208a5ffe8a2abe311d8054 🌮
Zero-1729:
crACK d930c7f
Tree-SHA512: b2101cad87f59c238603f38bd8e8df7a4d48929794e4de9e0e0ff2afa935a68475c2d369aa669d124a0bec2f50280fb47e8b980bde6ad812db08cf67b71c066a
|
|
9778b0fec13c047a4c7f3ae425d044da26eadc81 [net_processing] Provide debug error if code assumptions change. (Amiti Uttarwar)
aa79c912608fdc77c69dd43653aa87f1f34e01dd [docs] Add release notes for #21528 (Amiti Uttarwar)
Pull request description:
Adds a release note & addresses [this](https://github.com/bitcoin/bitcoin/pull/21528#discussion_r680963101) review comment to make expectations more explicit.
ACKs for top commit:
Zero-1729:
re-ACK 9778b0fec13c047a4c7f3ae425d044da26eadc81
jonatack:
ACK 9778b0fec13c047a4c7f3ae425d044da26eadc81
Tree-SHA512: 9507df5f2746d05c6df8c86b7a19364610ebfafc81af7650be7e68d7536a0685cce9fd2e5f287ef92b6245c584f8875b24a958109ba5bd8acf3c8fc9fd19eef2
|
|
Currently, this call to SetupAddressRelay will never return false because of
the previous guard that returns early if the peer is not an inbound connection.
Rather than implicitly relying on this guarantee, throw an error in the debug
build if it ever changes.
|
|
|
|
5e33f762d44557a1e3f0ff3c280d8a3ab98e3867 p2p, rpc: address relay fixups (Jon Atack)
Pull request description:
Following review of new changes merged today, move a use of `statestats` in getpeerinfo to within the section guarded by `if (fStateStats)`, e.g. `PeerManagerImpl::GetNodeStateStats` true, and pass an in-param by reference to const.
ACKs for top commit:
amitiuttarwar:
ACK 5e33f762d4
jnewbery:
ACK 5e33f762d44557a1e3f0ff3c280d8a3ab98e3867
Tree-SHA512: b42f33c615b14079e2c4e6060209de8707d71b351dd1e11e04a2a6fc12d15747d0c5d9b24850217080fd1ef92e63f96d6925c4badf280b781edd696c349be7d6
|
|
scheduler threads
703b1e612a4bd4521e20ae21eb8fb7c19f4ef942 Close minor startup race between main and scheduler threads (Larry Ruane)
Pull request description:
This is a low-priority bug fix. The scheduler thread runs `CheckForStaleTipAndEvictPeers()` every 45 seconds (EXTRA_PEER_CHECK_INTERVAL). If its first run happens before the active chain is set up (`CChain::SetTip()`), `bitcoind` will assert:
```
(...)
2021-07-28T22:16:49Z init message: Loading block index…
bitcoind: validation.cpp:4968: CChainState& ChainstateManager::ActiveChainstate() const: Assertion `m_active_chainstate' failed.
Aborted (core dumped)
```
I ran into this while using the debugger to investigate an unrelated problem. Single-stepping through threads with a debugger can cause the relative thread execution timing to be very different than usual. I don't think any automated tests are needed for this PR. I'll give reproduction steps in the next PR comment.
ACKs for top commit:
MarcoFalke:
cr ACK 703b1e612a4bd4521e20ae21eb8fb7c19f4ef942
tryphe:
tested ACK 703b1e612a4bd4521e20ae21eb8fb7c19f4ef942
0xB10C:
ACK 703b1e612a4bd4521e20ae21eb8fb7c19f4ef942
glozow:
code review ACK 703b1e612a4bd4521e20ae21eb8fb7c19f4ef942 - it makes sense to me to start peerman's background tasks here, after `chainstate->LoadChainTip()` and `node.connman->Start()` have been called.
Tree-SHA512: 9316ad768cba3b171f62e2eb400e3790af66c47d1886d7965edb38d9710fc8c8f8e4fb38232811c9346732ce311d39f740c5c2aaf5f6ca390ddc48c51a8d633b
|
|
|
|
3f7250b328b8b2f5d63f323702445ac5c989b73d [test] Use the new endpoint to improve tests (Amiti Uttarwar)
3893da06db1eb622f540605700f8663f8d87b2df [RPC] Add field to getpeerinfo to indicate if addr relay is enabled (Amiti Uttarwar)
0980ca78cd930a00c9985d7f00083a3b8e8be89e [test] Test that we intentionally select addr relay peers. (Amiti Uttarwar)
c061599e40dc3d379c10b914765061a7a8449dd7 [net_processing] Remove RelayAddrsWithPeer function (Amiti Uttarwar)
201e4964816f8896cfe7b4f6d8ddbfffe7102f87 [net_processing] Introduce new field to indicate if addr relay is enabled (Amiti Uttarwar)
1d1ef2db7ea0d93c7dab4a9800ec74afa7a019eb [net_processing] Defer initializing m_addr_known (Amiti Uttarwar)
6653fa3328b5608fcceda1c6ea8e68c5d58739ec [test] Update p2p_addr_relay test to prepare (Amiti Uttarwar)
2fcaec7bbb96d6fe72a7e3a5744b0c35c79733e8 [net_processing] Introduce SetupAddressRelay (Amiti Uttarwar)
Pull request description:
This PR builds on the test refactors extracted into #22306 (first 5 commits).
This PR aims to reduce addr blackholes. When we receive an `addr` message that contains 10 or less addresses, we forward them to 1-2 peers. This is the main technique we use for self advertisements, so sending to peers that wouldn't relay would effectively "blackhole" the trickle. Although we cannot prevent this in a malicious case, we can improve it for the normal, honest cases, and reduce the overall likelihood of occurrence. Two known cases where peers would not participate in addr relay are if they have connected to you as a block-relay-only connection, or if they are a light client.
This implementation defers initialization of `m_addr_known` until it is needed, then uses its presence to decide if the peer is participating in addr relay. For outbound (not block-relay-only) peers, we initialize the filter before sending the initial self announcement when processing their `version` message. For inbound peers, we initialize the filter if/when we get an addr related message (`ADDR`, `ADDRV2`, `GETADDR`). We do NOT initialize the filter based on a `SENDADDRV2` message.
To communicate about these changes beyond bitcoin core & to (try to) ensure that no other software would be disrupted, I have:
- Posted to the [mailing list](https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2021-April/018784.html)
- Researched other open source clients to confirm compatibility, opened issues in all the projects & documented in https://github.com/bitcoin/bitcoin/pull/21528#issuecomment-809906430. Many have confirmed that this change would not be problematic.
- Raised as topic during [bitcoin-core-dev meeting](https://www.erisian.com.au/bitcoin-core-dev/log-2021-03-25.html#l-954)
- Raised as topic during [bitcoin p2p meeting](https://www.erisian.com.au/bitcoin-core-dev/log-2021-04-20.html#l-439)
ACKs for top commit:
jnewbery:
reACK 3f7250b328b8b2f5d63f323702445ac5c989b73d
glozow:
ACK 3f7250b328b8b2f5d63f323702445ac5c989b73d
ajtowns:
utACK 3f7250b328b8b2f5d63f323702445ac5c989b73d
Tree-SHA512: 29069282af684c1cd37d107c395fdd432dcccb11626f3c2dabfe92fdc4c85e74c7c4056fbdfa88017fec240506639b72ac6c311f8ce7c583112eb15f47e421af
|
|
Don't schedule class PeerManagerImpl's background tasks from its
constructor, but instead do that from a separate method,
StartScheduledTasks(), that can be called later at the end of startup,
after other things, such as the active chain, are initialzed.
|
|
|
|
Now that we have a simple boolean stored on the field, the wrapper function is
no longer necessary.
|
|
|
|
Use SetupAddressRelay to only initialize `m_addr_known` as needed. For outbound
peers, we initialize the filter before sending our self announcement (not
applicable for block-relay-only connections). For inbound peers, we initialize
the filter when we get an addr related message (ADDR, ADDRV2, GETADDR).
These changes intend to mitigate address blackholes. Since an inbound peer has
to send us an addr related message to become eligible as a candidate for addr
relay, this should reduce our likelihood of sending them self-announcements.
|
|
Idempotent function that initializes m_addr_known for connections that support
address relay (anything other than block-relay-only). Unused until the next
commit.
|
|
PeerManagerImpl ctor
fde1bf4f6136638e84cdf9806eedaae08e841bbf [net processing] Default initialize m_recent_confirmed_transactions (John Newbery)
37dcd12d539e4a875581fa049aa0f7fafeb932a4 scripted-diff: Rename recentRejects (John Newbery)
cd9902ac5054c01228d52616bf85f7196364d4ff [net processing] Default initialize recentRejects (John Newbery)
a28bfd1d4cfa523a6abf3832dbfd6183cd546944 [net processing] Default initialize m_stale_tip_check_time (John Newbery)
9190b01d8dcf03b74e9b9e1653688a97ac171b37 [net processing] Add Orphanage empty consistency check (John Newbery)
Pull request description:
- Use default initialization of PeerManagerImpl members where possible
- Remove unique_ptr indirection where it's not needed
ACKs for top commit:
MarcoFalke:
ACK fde1bf4f6136638e84cdf9806eedaae08e841bbf 👞
theStack:
re-ACK fde1bf4f6136638e84cdf9806eedaae08e841bbf
Tree-SHA512: 7ddedcc972df8e933e1fbe5c88b8ea17df89e1e58fc769518512c5540e49dc8eddb3f47e78d1329a6fc5644d2c1d11c981f681fd633f5218bfa4b3e6a86f3d7b
|
|
Can be used to monitor in- and outbound node traffic.
Based on ealier work by jb55.
Co-authored-by: William Casarin <jb55@jb55.com>
|
|
`PeerManagerImpl::Misbehaving(...)`
8858e88c840197cdcabea07dd1380ef2aa4ece02 p2p: refactor: tidy up `PeerManagerImpl::Misbehaving(...)` (Sebastian Falbesoner)
Pull request description:
This simple refactoring PR has the goal to improve the readability of the `Misbehaving` method by
- introducing constant variables `score_before` and `score_now` (to avoid repeatedly calculating the former)
- deduplicating calls to LogPrint(), eliminates else-branch
ACKs for top commit:
jnewbery:
utACK 8858e88c840197cdcabea07dd1380ef2aa4ece02
rajarshimaitra:
tACK https://github.com/bitcoin/bitcoin/pull/22495/commits/8858e88c840197cdcabea07dd1380ef2aa4ece02
Tree-SHA512: 1d4dd5ac1d16ee9595edf4fa46e4960915a203641d74e6c33cffaba62ea71328834309a4451256fb45daf759f0cf6f4f199c46815afff6c89c0746e2ad4d4092
|
|
- introduce constant variables `score_before` and
`score_after` in order to improve readability
- deduplicate calls to LogPrint(), eliminates else-branch
|
|
a806647d260132a00cd633160040625c7dd17803 [validation] Always include merkle root in coinbase commitment (Dhruv Mehta)
189128c220190a588500b8e74ee7ae47671b9558 [validation] Set witness script flag with p2sh for blocks (Dhruv Mehta)
ac82b99db77ec843af82dcdf040dfdbc98c8ff26 [p2p] remove redundant NODE_WITNESS checks (Dhruv Mehta)
6f8b198b8256a6703a6f5e592dfa77fa024a7035 [p2p] remove unused segwitheight=-1 option (Dhruv Mehta)
eba5b1cd6460c98e75d0422bd394e12af7f11e4c [test] remove or move tests using `-segwitheight=-1` (Dhruv Mehta)
Pull request description:
Builds on #21009 and makes progress on remaining items in #17862
Removing `RewindBlockIndex()` in #21009 allows the following:
- removal of tests using `segwitheight=-1` in `p2p_segwit.py`.
- move `test_upgrade_after_activation()` out of `p2p_segwit.py` reducing runtime
- in turn, that allows us to drop support for `-segwitheight=-1`, which is only supported for that test.
- that allows us to always set `NODE_WITNESS` in our local services. The only reason we don't do that is to support `-segwitheight=-1`.
- that in turn allows us to drop all of the `GetLocalServices() & NODE_WITNESS` checks inside `net_processing.cpp`, since our local services would always include `NODE_WITNESS`
ACKs for top commit:
mzumsande:
Code-Review ACK a806647d260132a00cd633160040625c7dd17803
laanwj:
Code review ACK a806647d260132a00cd633160040625c7dd17803, nice cleanup
jnewbery:
utACK a806647d260132a00cd633160040625c7dd17803
theStack:
ACK a806647d260132a00cd633160040625c7dd17803
Tree-SHA512: 73e1a69d1d7eca1f5c38558ec6672decd0b60b16c2ef6134df6f6af71bb159e6eea160f9bb5ab0eb6723c6632d29509811e29469d0d87abbe9b69a2890fbc73e
|
|
self-announcements
5730a43703f7e5a5ca26245ba3b55fbdd027d0b6 test: Add functional test for AddrFetch connections (Martin Zumsande)
c34ad3309f93979b274a37de013502b05d25fad8 net, rpc: Enable AddrFetch connections for functional testing (Martin Zumsande)
533500d9072b7d5a36a6491784bdeb9247e91fb0 p2p: Add timeout for AddrFetch peers (Martin Zumsande)
b6c5d1e450dde6a54bd785504c923adfb45c7060 p2p: AddrFetch - don't disconnect on self-announcements (Martin Zumsande)
Pull request description:
AddrFetch connections (old name: oneshots) are intended to be short-lived connections on which we ask a peer for addresses via `getaddr` and disconnect after receiving them.
This is done by disconnecting after receiving the first `addr`. However, it is no longer working as intended, because nowadays, the first `addr` a typical bitcoin core node sends is its self-announcement.
So we'll disconnect before the peer gets a chance to answer our `getaddr`.
I checked that this affects both `-seednode` peers specified manually, and DNS seeds when AddrFetch is used as a fallback if DNS doesn't work for us.
The current behavior of getting peers via AddrFetch when starting with an empty addrman would be to connect to the peer, receive its self-announcement and add it to addrman, disconnect, reconnect to the same peer again as a full outbound (no other addresses in addrman) and then receive more `addr`. This is silly and not in line with AddrFetch peer being intended to be short-lived peers.
Fix this by only disconnecting after receiving an `addr` message of size > 1.
[Edit] As per review discussion, this PR now also adds a timeout after which we disconnect if we haven't received any suitable `addr`, and a functional test.
ACKs for top commit:
amitiuttarwar:
reACK 5730a43703f7e5a5ca26245ba3b55fbdd027d0b6
naumenkogs:
ACK 5730a43703f7e5a5ca26245ba3b55fbdd027d0b6
jnewbery:
ACK 5730a43703
Tree-SHA512: 8a81234f37e827705138eb254223f7f3b3bf44a06cb02126fc7990b0d231b9bd8f07d38d185cc30d55bf35548a6fdc286b69602498d875b937e7c58332158bf9
|
|
Now that m_recent_confirmed_transactions is owned by PeerManagerImpl,
and PeerManagerImpl's lifetime is managed by the node context, we can
just default initialize m_recent_confirmed_transactions during object
initialization. We can also remove the unique_ptr indirection.
|
|
-BEGIN VERIFY SCRIPT-
ren() { sed -i "s:\<$1\>:$2:g" $(git grep -l "\<$1\>" ./src ./test); }
ren recentRejects m_recent_rejects
-END VERIFY SCRIPT-
|
|
Now that recentRejects is owned by PeerManagerImpl, and
PeerManagerImpl's lifetime is managed by the node context, we can just
default initialize recentRejects during object initialization. We can
also remove the unique_ptr indirection.
|
|
|
|
When removing the final peer, assert that m_tx_orphanage is empty.
|