Page MenuHomePhabricator
Differential D9037

scripted-diff: [net processing] Rename PeerLogicValidation to PeerManager
ClosedPublic
Actions

Authored by Fabien on Jan 22 2021, 11:50.

Details

Reviewers
PiRK
Group Reviewers
Restricted Project
Commits
rABC09281847a858: scripted-diff: [net processing] Rename PeerLogicValidation to PeerManager
Summary
-BEGIN VERIFY SCRIPT-
sed -i 's/PeerLogicValidation/PeerManager/g' $(git grep -l
PeerLogicValidation ./src ./test)
sed -i 's/peer_logic/peerman/g' $(git grep -l peer_logic ./src ./test)
-END VERIFY SCRIPT-

PeerLogicValidation was originally net_processing's implementation to
the validation interface. It has since grown to contain much of
net_processing's logic. Therefore rename it to reflect its
responsibilities.

Partial backport (3/9) of core PR19791:
https://github.com/bitcoin/bitcoin/pull/19791/commits/58bd369b0ddd3383f7bdf7840912d18b96545f91

Since the name conflicts with avalanche's PeerManager in tests, the full
namespaced class name is used.

Test Plan
ninja all check-all

Diff Detail

Event Timeline

Fabien created this revision.Jan 22 2021, 11:50
Herald added a reviewer: Restricted Project. · View Herald TranscriptJan 22 2021, 11:50
Fabien requested review of this revision.Jan 22 2021, 11:50
teamcity edited the summary of this revision. (Show Details)Jan 22 2021, 11:51
Harbormaster completed remote builds in B14950: Diff 27259.Jan 22 2021, 12:10
teamcity added a comment.Jan 22 2021, 12:10
Build Bitcoin ABC Diffs / Diff Testing (build-fuzzer) failed.

Tail of the build log:

[543/625] Linking CXX executable src/test/fuzz/addrdb
[544/625] Linking CXX executable src/test/fuzz/block_header
[545/625] Linking CXX executable src/test/fuzz/partial_merkle_tree_deserialize
[546/625] Linking CXX executable src/test/fuzz/addition_overflow
[547/625] Linking CXX executable src/test/fuzz/bloom_filter
[548/625] Linking CXX executable src/test/fuzz/netaddress
[549/625] Linking CXX executable src/test/fuzz/cashaddr
[550/625] Linking CXX executable src/test/fuzz/blockheader_deserialize
[551/625] Linking CXX executable src/test/fuzz/parse_iso8601
[552/625] Linking CXX executable src/test/fuzz/chain
[553/625] Linking CXX executable src/test/fuzz/strprintf
[554/625] Linking CXX executable src/test/fuzz/cuckoocache
[555/625] Linking CXX executable src/test/fuzz/tx_in
[556/625] Linking CXX executable src/test/fuzz/locale
[557/625] Linking CXX executable src/test/fuzz/parse_numbers
[558/625] Linking CXX executable src/test/fuzz/parse_script
[559/625] Linking CXX executable src/test/fuzz/blockfilter
[560/625] Linking CXX executable src/test/fuzz/tx_out
[561/625] Linking CXX executable src/test/fuzz/pub_key_deserialize
[562/625] Linking CXX executable src/test/fuzz/parse_univalue
[563/625] Linking CXX executable src/test/fuzz/prevector
[564/625] Linking CXX executable src/test/fuzz/block_file_info_deserialize
[565/625] Linking CXX executable src/test/fuzz/block_filter_deserialize
[566/625] Linking CXX executable src/test/fuzz/blocktransactionsrequest_deserialize
[567/625] Linking CXX executable src/test/fuzz/pow
[568/625] Linking CXX executable src/test/fuzz/base_encode_decode
[569/625] Linking CXX executable src/test/fuzz/addrman_deserialize
[570/625] Linking CXX executable src/test/fuzz/primitives_transaction
[571/625] Linking CXX executable src/test/fuzz/fees
[572/625] Linking CXX executable src/test/fuzz/psbt
[573/625] Linking CXX executable src/test/fuzz/process_messages
[574/625] Linking CXX executable src/test/fuzz/protocol
[575/625] Linking CXX executable src/test/fuzz/flat_file_pos_deserialize
[576/625] Linking CXX executable src/test/fuzz/script_ops
[577/625] Linking CXX executable src/test/fuzz/random
[578/625] Linking CXX executable src/test/fuzz/rolling_bloom_filter
[579/625] Linking CXX executable src/test/fuzz/script_flags
[580/625] Linking CXX executable src/test/fuzz/block_header_and_short_txids_deserialize
[581/625] Linking CXX executable src/test/fuzz/checkqueue
[582/625] Linking CXX executable src/test/fuzz/descriptor_parse
[583/625] Linking CXX executable src/test/fuzz/signature_checker
[584/625] Linking CXX executable src/test/fuzz/blocktransactions_deserialize
[585/625] Linking CXX executable src/test/fuzz/bloomfilter_deserialize
[586/625] Linking CXX executable src/test/fuzz/spanparsing
[587/625] Linking CXX executable src/test/fuzz/txoutcompressor_deserialize
[588/625] Linking CXX executable src/test/fuzz/span
[589/625] Linking CXX executable src/test/fuzz/blockmerkleroot
[590/625] Linking CXX executable src/test/fuzz/timedata
[591/625] Linking CXX executable src/test/fuzz/transaction
[592/625] Linking CXX executable src/test/fuzz/addr_info_deserialize
[593/625] Linking CXX executable src/test/fuzz/netaddr_deserialize
[594/625] Linking CXX executable src/test/fuzz/key_io
[595/625] Linking CXX executable src/test/fuzz/blocklocator_deserialize
[596/625] Linking CXX executable src/test/fuzz/blockundo_deserialize
[597/625] Linking CXX executable src/test/fuzz/coins_deserialize
[598/625] Linking CXX executable src/test/fuzz/banentry_deserialize
[599/625] Linking CXX executable src/test/fuzz/block_deserialize
[600/625] Linking CXX executable src/test/fuzz/diskblockindex_deserialize
ninja: build stopped: cannot make progress due to previous errors.
Build build-fuzzer failed with exit code 1
teamcity added a comment.Jan 22 2021, 12:16
Build Bitcoin ABC Diffs / Diff Testing (build-diff) failed.

Failed tests logs:

====== Bitcoin ABC functional tests with the next upgrade activated: p2p_leak.py ======

------- Stdout: -------
2021-01-22T12:13:46.584000Z TestFramework (INFO): Initializing test directory /work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20210122_121145/p2p_leak_567
2021-01-22T12:13:46.899000Z TestFramework.mininode (WARNING): Connection lost to 127.0.0.1:21817 due to [Errno 104] Connection reset by peer
2021-01-22T12:13:52.002000Z TestFramework (INFO): Check that the version message does not leak the local address of the node
2021-01-22T12:13:52.103000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/work/test/functional/test_framework/test_framework.py", line 122, in main
    self.run_test()
  File "/work/test/functional/p2p_leak.py", line 189, in run_test
    assert_greater_than_or_equal(ver.nTime, time_begin)
  File "/work/test/functional/test_framework/util.py", line 70, in assert_greater_than_or_equal
    raise AssertionError("{} < {}".format(str(thing1), str(thing2)))
AssertionError: 1611317631 < 1611317632
2021-01-22T12:13:52.156000Z TestFramework (INFO): Stopping nodes
2021-01-22T12:13:52.612000Z TestFramework (WARNING): Not cleaning up dir /work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20210122_121145/p2p_leak_567
2021-01-22T12:13:52.612000Z TestFramework (ERROR): Test failed. Test logging available at /work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20210122_121145/p2p_leak_567/test_framework.log
2021-01-22T12:13:52.613000Z TestFramework (ERROR): Hint: Call /work/test/functional/combine_logs.py '/work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20210122_121145/p2p_leak_567' to consolidate all logs

Each failure log is accessible here:
Bitcoin ABC functional tests with the next upgrade activated: p2p_leak.py

Fabien added a child revision: D9038: [net processing] Move ProcessHeadersMessage to PeerManager.Jan 22 2021, 13:05
PiRK requested changes to this revision.EditedJan 22 2021, 16:12
PiRK added a subscriber: PiRK.

<s>The code looks good. I don't understand the failures, maybe you should rebase.</s>
I didn't look closely enough at the fuzzer failure. See inline comment.

This revision now requires changes to proceed.Jan 22 2021, 16:12
PiRK added inline comments.Jan 22 2021, 16:15
src/test/fuzz/process_message.cpp
89

config should probably not havec been removed here

Fabien updated this revision to Diff 27276.Jan 22 2021, 19:36

Fix the fuzzer

Harbormaster completed remote builds in B14962: Diff 27276.Jan 22 2021, 19:49
PiRK accepted this revision.Jan 23 2021, 06:08
This revision is now accepted and ready to land.Jan 23 2021, 06:08
Closed by commit rABC09281847a858: scripted-diff: [net processing] Rename PeerLogicValidation to PeerManager (authored by John Newbery <john@johnnewbery.com>, committed by Fabien). · Explain WhyJan 23 2021, 15:04
This revision was automatically updated to reflect the committed changes.
Fabien added a commit: rABC09281847a858: scripted-diff: [net processing] Rename PeerLogicValidation to PeerManager.

Revision Contents
Changeset List

PathSize
src/
avalanche/
test/
10 lines
14 lines
16 lines
10 lines
66 lines
node/
4 lines
test/
10 lines
fuzz/
6 lines
2 lines
util/
4 lines
test/
sanitizer_suppressions/
2 lines

Diff 27259

View Options

src/avalanche/test/peermanager_tests.cpp

Loading...
View Options

src/avalanche/test/processor_tests.cpp

Loading...
View Options

src/init.cpp

Loading...
View Options

src/net_processing.h

Loading...
View Options

src/net_processing.cpp

Loading...
View Options

src/node/context.h

Loading...
View Options

src/test/denialofservice_tests.cpp

Loading...
View Options

src/test/fuzz/process_message.cpp

Loading...
View Options

src/test/fuzz/process_messages.cpp

Loading...
View Options

src/test/util/setup_common.cpp

Loading...
View Options

test/sanitizer_suppressions/tsan

Loading...