Age | Commit message (Collapse) | Author |
|
|
|
|
|
Prior to this commit, the peer was connected, and then the services and
connectivity fields in the CNode object were manually set. Instead, send
p2p `version` and `verack` messages, and have net_processing's internal
logic set the state of the node.
This ensures that the node's internal state is consistent with how it
would be set in the live code.
Prior to this commit, `dummyNode1.nServices` was set to `NODE_NONE`
which was not a problem since `CNode::fClient` and
`CNode::m_limited_node` are default initialised to false. Now that we
are doing the actual version handshake, the values of `fClient` and
`m_limited_node` are set during the handshake and cause the test to fail
if we do not set `dummyNode1.nServices` to a reasonable value
(NODE_NETWORK | NODE_WITNESS).
|
|
Track services offered by us and the peer in the Peer object.
|
|
`Misbehaving` has several coding related issues (ignoring the conceptual
issues here for now):
* It is public, but it is not supposed to be called from outside of
net_processing. Fix that by making it private and creating a public
`UnitTestMisbehaving` method for unit testing only.
* It doesn't do anything if a `nullptr` is passed. It would be less
confusing to just skip the call instead. Fix that by passing `Peer&`
to `Misbehaving()`.
* It calls `GetPeerRef`, causing `!m_peer_mutex` lock annotations to be
propagated. This is harmless, but verbose. Fix it by removing the no
longer needed call to `GetPeerRef` and the no longer needed lock
annotations.
|
|
|
|
|
|
PeerManagerImpl
778343a379026ef233dffea67f5226565f6d5720 scripted-diff: Rename PeerManagerImpl members (dergoegge)
91c339243e11ec42eeeaca8fe015fc1c3e6338e1 [net processing] Move nHighestFastAnnounce into PeerManagerImpl (dergoegge)
10b83e2aa3393ef2c942fde7ac86e8cf3ea224c1 [net processing] Move block cache state into PeerManagerImpl (dergoegge)
a4c55a93ef9277e1043c286120e2417652ee8bbb [net processing] Inline and simplify UpdatePreferredDownload (dergoegge)
490c08f96a34ed436c3d2cf7b9a3ed72694b6147 [net processing] Move nPreferredDownload into PeerManagerImpl (dergoegge)
a292df283a596efe7e1d40c33a6d614d70ed564d [net processing] Move mapNodeState into PeerManagerImpl (dergoegge)
37ecaf3e7a028486a0a1c9b717e8eb4214215805 [net processing] Move CNodeState declaration above PeerManagerImpl (dergoegge)
Pull request description:
This PR moves the remaining net processing globals into `PeerManagerImpl`. This will make testing the peer manager in isolation easier and also acts as a code clean up.
ACKs for top commit:
jnewbery:
Code review ACK 778343a379026ef233dffea67f5226565f6d5720
MarcoFalke:
ACK 778343a379026ef233dffea67f5226565f6d5720 🗒
Tree-SHA512: 4f22105d1de37b94c3ef349f38784a30cf8d450d394a6a7849e5bd78940a71e3edbffa3d25e8efb35d7f698fd255f199de7bd4c33e23af5621a6e4e67ed43cb5
|
|
|
|
36f814c0e84d009c0e0aa26981a20ac4cf338a85 [netgroupman] Remove NetGroupManager::GetAsmap() (John Newbery)
4709fc2019e27e74be02dc5fc123b9f6f46d7990 [netgroupman] Move asmap checksum calculation to NetGroupManager (John Newbery)
1b978a7e8c71dcc1501705022e66f6779c8c4528 [netgroupman] Move GetMappedAS() and GetGroup() logic to NetGroupManager (John Newbery)
ddb4101e6377a998b7c598bf52217b47698ddec9 [net] Only use public CNetAddr functions and data in GetMappedAS() and GetGroup() (John Newbery)
6b2268162e96bc4fe1a3ebad454996b1d3d4615c [netgroupman] Add GetMappedAS() and GetGroup() (John Newbery)
19431560e3e1124979c60f39eca9429c4a0df29f [net] Move asmap into NetGroupManager (John Newbery)
17c24d458042229e00dd4e0b75a32e593be29564 [init] Add netgroupman to node.context (John Newbery)
9b3836710b8160d212aacd56154938e5bb4b26b7 [build] Add netgroup.cpp|h (John Newbery)
Pull request description:
The asmap data is currently owned by addrman, but is used by both addrman and connman. #22791 made the data const and private (so that it can't be updated by other components), but it is still passed out of addrman as a reference to const, and used by `CNetAddress` to calculate the group and AS of the net address.
This RFC PR proposes to move all asmap data and logic into a new `NetGroupManager` component. This is initialized at startup, and the client components addrman and connman simply call `NetGroupManager::GetGroup(const CAddress&)` and `NetGroupManager::GetMappedAS(const CAddress&)` to get the net group and AS of an address.
ACKs for top commit:
mzumsande:
Code Review ACK 36f814c0e84d009c0e0aa26981a20ac4cf338a85
jnewbery:
CI failure seems spurious. I rebased onto latest master to trigger a new CI run, but whilst I was doing that, mzumsande ACKed https://github.com/bitcoin/bitcoin/commit/36f814c0e84d009c0e0aa26981a20ac4cf338a85, so I've reverted to that.
dergoegge:
Code review ACK 36f814c0e84d009c0e0aa26981a20ac4cf338a85
Tree-SHA512: 244a89cdfd720d8cce679eae5b7951e1b46b37835fccb6bdfa362856761bb110e79e263a6eeee8246140890f3bee2850e9baa7bc14a388a588e0e29b9d275175
|
|
|
|
|
|
|
|
This is needed to use ASSERT_DEBUG_LOG, which may include a fixed node
number
|
|
Change `CNode` to use a pointer to `Sock` instead of a bare `SOCKET`.
This will help mocking / testing / fuzzing more code.
|
|
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./
-END VERIFY SCRIPT-
Commits of previous years:
* 2020: fa0074e2d82928016a43ca408717154a1c70a4db
* 2019: aaaaad6ac95b402fe18d019d67897ced6b316ee0
|
|
-BEGIN VERIFY SCRIPT-
ren() { sed -i "s/\<$1\>/$2/g" $( git grep -l "$1" ./src/ ) ; }
ren nLastBlockTime m_last_block_time
ren nLastTXTime m_last_tx_time
ren nTimeConnected m_connected
-END VERIFY SCRIPT-
|
|
This allows to revert the temporary commit
0bfb9208df556f77cddfedfd73e5811a0e031d34 (test: fix test failures in
test/functional/p2p_timeouts.py).
|
|
|
|
up test
fadc0c80ae4e526fb2b503f7cc02f6122aaf1de5 p2p: Make timeout mockable and type safe, speed up test (MarcoFalke)
fa6d5a238d2c94440105ddd4f1554f85659d6c5b scripted-diff: Rename m_last_send and m_last_recv (MarcoFalke)
Pull request description:
Use type-safe time for better code readability/maintainability and mockable time for better testability. This speeds up the p2p_timeout test.
This is also a bugfix for intermittent test issues like: https://cirrus-ci.com/task/4769904156999680?command=ci#L2836
Fixes #20654
ACKs for top commit:
laanwj:
Code review ACK fadc0c80ae4e526fb2b503f7cc02f6122aaf1de5
naumenkogs:
ACK fadc0c80ae4e526fb2b503f7cc02f6122aaf1de5
Tree-SHA512: 28c6544c97f188c8a0fbc80411c74ab74ffd055885322c325aa3d1c404b29c3fd70a737e86083eecae58ef394db1cb56bc122d06cff63742aa89a8e868730c64
|
|
4740fe8212da21e86664355b4c6d0d7d838e4382 test: Add test for block relay only eviction (Martin Zumsande)
Pull request description:
Adds a unit test for block-relay-only eviction logic added in #19858, which was not covered by any tests before. The added test is very similar to the existing `stale_tip_peer_management` unit test, which tests the analogous logic for regular outbound peers.
ACKs for top commit:
glozow:
reACK 4740fe8212da21e86664355b4c6d0d7d838e4382
rajarshimaitra:
tACK https://github.com/bitcoin/bitcoin/pull/23614/commits/4740fe8212da21e86664355b4c6d0d7d838e4382
shaavan:
ACK 4740fe8212da21e86664355b4c6d0d7d838e4382. Great work @ mzumsande!
LarryRuane:
ACK 4740fe8212da21e86664355b4c6d0d7d838e4382
Tree-SHA512: 5985afd7d8f7ae311903dbbf6b7d526e16309c83c88ae6dd6551960c0b186156310a6be0cf6b684f82ac1378d0fc5aa3717f0139e078471013fceb6aebe81bf6
|
|
|
|
|
|
-BEGIN VERIFY SCRIPT-
perl -0777 -pi -e 's:((\(|\{|,)(\n| )*)\/\* ?([^=* ]+) ?\*\/ ?:\1/*\4=*/:g' $( git ls-files ./src/test ./src/wallet/test )
-END VERIFY SCRIPT-
|
|
|
|
|
|
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.
|
|
Save the banlist in `banlist.json` instead of `banlist.dat`.
This makes it possible to store Tor v3 entries in the banlist on disk
(and any other addresses that cannot be serialized in addrv1 format).
Only read `banlist.dat` if it exists and `banlist.json` does not
exist (first start after an upgrade).
Supersedes https://github.com/bitcoin/bitcoin/pull/20904
Resolves https://github.com/bitcoin/bitcoin/issues/19748
|
|
Chainstate!
6f994882deafe62e97f0a889d8bdb8c96dcf913d validation: Farewell, global Chainstate! (Carl Dong)
972c5166ee685447a6d4bf5e501b07a0871fba85 qt/test: Reset chainman in ~ChainstateManager instead (Carl Dong)
6c3b5dc0c13c3ac8c6e86298f924abe99d8d6bd1 scripted-diff: tree-wide: Remove all review-only assertions (Carl Dong)
3e82abb8dd7e21ec918966105648be7ae077fd8c tree-wide: Remove stray review-only assertion (Carl Dong)
f323248aba5088c9630e5cdfe5ce980f21633fe8 qt/test: Use existing chainman in ::TestGUI (can be scripted-diff) (Carl Dong)
6c15de129cd645bf0547cb184003fae131b95b83 scripted-diff: wallet/test: Use existing chainman (Carl Dong)
ee0ab1e959e0e75e04d87fabae8334ad4656f3e5 fuzz: Initialize a TestingSetup for test_one_input (Carl Dong)
0d61634c066a7102d539e85e2b1a4ca15be9660a scripted-diff: test: Use existing chainman in unit tests (Carl Dong)
e197076219e986ede6cf924e0ea36bd723503b2d test: Pass in CoinsTip to ValidateCheckInputsForAllFlags (Carl Dong)
4d99b61014ba26eb1f3713df5528d2804edff165 test/miner_tests: Pass in chain tip to CreateBlockIndex (Carl Dong)
f0dd5e6bb4b16e69d35b648b7ef973a732229873 test/util: Use existing chainman in ::PrepareBlock (Carl Dong)
464c313e304cef04a82e14f736e3c44ed5604a4e init: Use existing chainman (Carl Dong)
Pull request description:
Based on: #21767
à la Mr. Sandman
```
Mr. Chainman, bring me a tip (bung, bung, bung, bung)
Make it the most work that I've ever seen (bung, bung, bung, bung)
Rewind old tip till we're at the fork point (bung, bung, bung, bung)
Then tell it that it's time to call Con-nectTip
Chainman, I'm so alone (bung, bung, bung, bung)
No local objects to call my own (bung, bung, bung, bung)
Please make sure I have a ref
Mr. Chainman, bring me a tip!
```
This is the last bundle in the #20158 series. Thanks everyone for their diligent review.
I would like to call attention to https://github.com/bitcoin/bitcoin/issues/21766, where a few leftover improvements were collated.
- Remove globals:
- `ChainstateManager g_chainman`
- `CChainState& ChainstateActive()`
- `CChain& ChainActive()`
- Remove all review-only assertions.
ACKs for top commit:
jamesob:
reACK https://github.com/bitcoin/bitcoin/pull/21866/commits/6f994882deafe62e97f0a889d8bdb8c96dcf913d based on the contents of
ariard:
Code Review ACK 6f99488.
jnewbery:
utACK 6f994882deafe62e97f0a889d8bdb8c96dcf913d
achow101:
Code Review ACK 6f994882deafe62e97f0a889d8bdb8c96dcf913d
ryanofsky:
Code review ACK 6f994882deafe62e97f0a889d8bdb8c96dcf913d.
Tree-SHA512: 4052ea79360cf0efd81ad0ee3f982e1d93aab1837dcec75f875a56ceda085de078bb3099a2137935d7cc2222004ad88da94b605ef5efef35cb6bc733725debe6
|
|
-BEGIN VERIFY SCRIPT-
git ls-files -- src/test \
| grep -v '^src/test/fuzz' \
| xargs sed -i -E \
-e 's@g_chainman\.m_blockman@m_node.chainman->m_blockman@g' \
-e 's@([^:])(Chain(state|)Active)@\1::\2@g' \
-e 's@::Chain(state|)Active\(\)@m_node.chainman->ActiveChain\1()@g'
-END VERIFY SCRIPT-
|
|
|
|
`ArgsManager.GetDataDirBase()` in tests
-BEGIN VERIFY SCRIPT-
git ls-files src/test/*_tests.cpp src/test/util/setup_common.cpp | xargs sed -i 's/.GetDataDirPath()/.GetDataDirBase()/g';
-END VERIFY SCRIPT-
|
|
BasicTestingSetup class instead of implicitly relying on gArgs.
-BEGIN VERIFY SCRIPT-
git ls-files src/test/dbwrapper_tests.cpp src/test/denialofservice_tests.cpp src/test/flatfile_tests.cpp src/test/fs_tests.cpp src/test/settings_tests.cpp src/test/util_tests.cpp | xargs sed -i 's/GetDataDir()/m_args.GetDataDirPath()/g';
-END VERIFY SCRIPT-
|
|
Doing it there will reduce code bloat and also ensure no test can "forget" to reset it
|
|
|
|
Use `CConnmanTest` instead of `CConnman` and add the nodes to it
so that their `fDisconnect` flag is set during disconnection.
|
|
This is a non-functional change that replaces the `CNode` on-stack
variables with `CNode` pointers.
The reason for this is that it would allow us to add those `CNode`s
to `CConnman::vNodes[]` which in turn would allow us to check that they
are disconnected properly - a `CNode` object must be in
`CConnman::vNodes[]` in order for its `fDisconnect` flag to be set.
If we store pointers to the on-stack variables in `CConnman` then it
would crash at the end, trying to `delete` them.
|
|
PeerManager can just call directly into CAddrMan::Connected() now.
|
|
|
|
node.context owns the CAddrMan. CConnman holds a reference to
the CAddrMan.
|
|
-BEGIN VERIFY SCRIPT-
git rm src/util/memory.h
sed -i -e 's/MakeUnique/std::make_unique/g' $(git grep -l MakeUnique src)
sed -i -e '/#include <util\/memory.h>/d' $(git grep -l '#include <util/memory.h>' src)
sed -i -e '/util\/memory.h \\/d' src/Makefile.am
-END VERIFY SCRIPT-
|
|
-BEGIN VERIFY SCRIPT-
sed -i 's/mapOrphanTransactionsByPrev/m_outpoint_to_orphan_it/g' src/txorphanage.h src/txorphanage.cpp
sed -i 's/mapOrphanTransactions/m_orphans/g' src/txorphanage.h src/txorphanage.cpp src/net_processing.cpp src/test/denialofservice_tests.cpp
sed -i 's/g_orphan_list/m_orphan_list/g' src/txorphanage.h src/txorphanage.cpp
sed -i 's/g_orphans_by_wtxid/m_wtxid_to_orphan_it/g' src/txorphanage.h src/txorphanage.cpp
sed -i 's/nMaxOrphans/max_orphans/g' src/txorphanage.h src/txorphanage.cpp
sed -i 's/COrphanTx/OrphanTx/g' src/txorphanage.h src/txorphanage.cpp src/test/denialofservice_tests.cpp
-END VERIFY SCRIPT-
|
|
Collects all the orphan handling globals into a single member var in
net_processing, and ensures access is encapuslated into the interface
functions. Also adds doxygen comments for methods.
|
|
Rather than checking net_processing's internal implementation of
AddOrphanTx, test txorphanage's exported AddTx interface. Note that
this means AddToCompactExtraTransactions is no longer tested here.
|
|
This module captures orphan tracking code for tx relay.
Can be reviewed with --color-moved=dimmed-zebra
|
|
and to allow the compiler to warn if uninitialized in the ctor
or omitted in the caller.
|
|
|
|
|
|
|
|
Connected() updates the time we serve in addr messages, so avoid leaking
block-relay-only peer connections by avoiding these calls.
|