| Age | Commit message (Collapse) | Author |
|---|
|
|
|
This also cleans up the addrman (de)serialization code paths to only
allow `Disk` serialization. Some unit tests previously forced a
`Network` serialization, which does not make sense, because Bitcoin Core
in production will always `Disk` serialize.
This cleanup idea was suggested by Pieter Wuille and implemented by Anthony
Towns.
Co-authored-by: Pieter Wuille <pieter@wuille.net>
Co-authored-by: Anthony Towns <aj@erisian.com.au>
|
|
-BEGIN VERIFY SCRIPT-
ren() { sed --regexp-extended -i "s|$1|$2|g" $(git grep -l --extended-regexp "$1"); }
# Replace FUZZ_TARGET_INIT
ren 'FUZZ_TARGET_INIT\((.+), (.+)\)' 'FUZZ_TARGET(\1, .init = \2)'
# Delete unused FUZZ_TARGET_INIT
sed -i -e '37,39d' src/test/fuzz/fuzz.h
-END VERIFY SCRIPT-
|
|
This is a follow-up to previous commits moving the chain constants out
of chainparamsbase.
The script removes the chainparamsbase header in all files where it is
included, but not used. This is done by filtering against all defined
symbols of the header as well as its respective .cpp file.
The kernel chainparams now no longer relies on chainparamsbase.
-BEGIN VERIFY SCRIPT-
sed -i '/#include <chainparamsbase.h>/d' $( git grep -l 'chainparamsbase.h' | xargs grep -L 'CBaseChainParams\|CreateBaseChainParams\|SetupChainParamsBaseOptions\|BaseParams\|SelectBaseParams\|chainparamsbase.cpp' )
-END VERIFY SCRIPT-
|
|
This commit effectively moves the definition of these constants
out of the chainparamsbase to their own file.
Using the ChainType enums provides better type safety compared to
passing around strings.
The commit is part of an ongoing effort to decouple the libbitcoinkernel
library from the ArgsManager and other functionality that should not be
part of the kernel library.
|
|
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./
-END VERIFY SCRIPT-
Commits of previous years:
- 2021: f47dda2c58b5d8d623e0e7ff4e74bc352dfa83d7
- 2020: fa0074e2d82928016a43ca408717154a1c70a4db
- 2019: aaaaad6ac95b402fe18d019d67897ced6b316ee0
|
|
|
|
|
|
|
|
Blindly chose a cap of 10000 iterations for every loop, except for
the two in script_ops.cpp and scriptnum_ops.cpp which appeared to
(sometimes) be deserializing individual bytes; capped those to one
million to ensure that sometimes we try working with massive scripts.
There was also one fuzzer-controlled loop in timedata.cpp which was
already capped, so I left that alone.
git grep 'while (fuzz' should now run clean except for timedata.cpp
|
|
This saves passing around a reference to the asmap std::vector<bool>.
|
|
SanityCheckASMap(asmap, bits) simply calls through to SanityCheckASMap(asmap)
in util/asmap. Update all callers to simply call that function.
|
|
724c4975622bc22cedc3f3814dfc8e66cf8371f7 [fuzz] Add ConsumeAsmap() function (John Newbery)
5840476714ffebb2599999c85a23b52ebcff6090 [addrman] Make m_asmap private (John Newbery)
f9002cb5dbd573cd9ca200de21319fa296e26055 [net] Rename the copyStats arg from m_asmap to asmap (John Newbery)
f572f2b2048994b3b50f4cfd5de19e40b1acfb22 [addrman] Set m_asmap in CAddrMan initializer list (John Newbery)
593247872decd6d483a76e96d79433247226ad14 [net] Remove CConnMan::SetAsmap() (John Newbery)
50fd77045e2f858a53486b5e02e1798c92ab946c [init] Read/decode asmap before constructing addrman (John Newbery)
Pull request description:
Commit 181a1207 introduced an initialization order bug: CAddrMan's m_asmap must be set before deserializing peers.dat.
The first commit restores the correct initialization order. The remaining commits make `CAddrMan::m_asmap` usage safer:
- don't reach into `CAddrMan`'s internal data from `CConnMan`
- set `m_asmap` in the initializer list and make it const
- make `m_asmap` private, and access it (as a reference to const) from a getter.
This ensures that peers.dat deserialization must happen after setting m_asmap, since m_asmap is set during CAddrMan construction.
ACKs for top commit:
mzumsande:
Tested ACK 724c4975622bc22cedc3f3814dfc8e66cf8371f7
amitiuttarwar:
code review but utACK 724c497562
naumenkogs:
utACK 724c4975622bc22cedc3f3814dfc8e66cf8371f7
vasild:
ACK 724c4975622bc22cedc3f3814dfc8e66cf8371f7
MarcoFalke:
review ACK 724c4975622bc22cedc3f3814dfc8e66cf8371f7 👫
Tree-SHA512: 684a4cf9e3d4496c9997fb2bc4ec874809987055c157ec3fad1d2143b8223df52b5a0af787d028930b27388c8efeba0aeb2446cb35c337a5552ae76112ade726
|
|
The m_ prefix indicates that a variable is a data member. Using it as
a parameter name is misleading.
Also update the name of the function from copyStats to CopyStats to
comply with our style guide.
|
|
https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines.html#c131-avoid-trivial-getters-and-setters
|
|
This logic is a no-op since it was introduced in commit
f9f5cfc50637f2cd1540923caf337e2651ec1625.
m_addr_name is never initialized to the empty string, because
ToStringIPPort never returns an empty string.
|
|
|
|
|
|
-BEGIN VERIFY SCRIPT-
git rm src/optional.h
sed -i -e 's/Optional</std::optional</g' $(git grep -l 'Optional<' src)
sed -i -e 's/{nullopt}/{std::nullopt}/g' $(git grep -l 'nullopt' src)
sed -i -e 's/ nullopt;/ std::nullopt;/g' $(git grep -l 'nullopt' src)
sed -i -e 's/ nullopt)/ std::nullopt)/g' $(git grep -l 'nullopt' src)
sed -i -e 's/(nullopt)/(std::nullopt)/g' $(git grep -l 'nullopt' src)
sed -i -e 's/ nullopt,/ std::nullopt,/g' $(git grep -l 'nullopt' src)
sed -i -e 's/? nullopt :/? std::nullopt :/g' $(git grep -l 'nullopt' src)
sed -i -e 's/: nullopt}/: std::nullopt}/g' $(git grep -l 'nullopt' src)
sed -i -e '/optional.h \\/d' src/Makefile.am
sed -i -e '/#include <optional.h>/d' src/test/fuzz/autofile.cpp src/test/fuzz/buffered_file.cpp src/test/fuzz/node_eviction.cpp
sed -i -e 's/#include <optional.h>/#include <optional>/g' $(git grep -l '#include <optional.h>' src)
-END VERIFY SCRIPT-
|
|
-BEGIN VERIFY SCRIPT-
# Rename
sed -i -e 's/MakeFuzzingContext/MakeNoLogFileContext/g' $(git grep -l MakeFuzzingContext)
# Bump the copyright of touched files in this scripted diff to avoid touching them again later
./contrib/devtools/copyright_header.py update ./src/test/fuzz/
-END VERIFY SCRIPT-
|
|
Previously, the {Basic,}TestingSetup for fuzzers were set up in many ways:
1. Calling InitializeFuzzingContext, which implicitly constructs a static
const BasicTestingSetup
2. Directly constructing a static const BasicTestingSetup in the initialize_*
function
3. Directly constructing a static TestingSetup and reproducing the
initialization arguments (I'm assuming because
InitializeFuzzingContext only initializes a BasicTestingSetup)
The new, relatively-simple MakeFuzzingContext function allows us to
consolidate these methods of initialization by being flexible enough to
be used in all situations. It:
1. Is templated so that we can choose to initialize any of
the *TestingSetup classes
2. Has sane defaults which are often used in fuzzers but are also
easily overridable
3. Returns a unique_ptr, explicitly transferring ownership to the caller
to deal with according to its situation
|
|
Can be reviewed with --ignore-all-space
|
|
|
|
|
|
|
|
fa13e1b0c52738492310b6b421d8e38cb04da5b1 build: Add option --enable-danger-fuzz-link-all (MarcoFalke)
44444ba759480237172d83f42374c5c29c76eda0 fuzz: Link all targets once (MarcoFalke)
Pull request description:
Currently the linker is invoked more than 150 times when compiling with `--enable-fuzz`. This is problematic for several reasons:
* It wastes disk space north of 20 GB, as all libraries and sanitizers are linked more than 150 times
* It wastes CPU time, as the link step can practically not be cached (similar to ccache for object files)
* It makes it a blocker to compile the fuzz tests by default for non-fuzz builds #19388, for the aforementioned reasons
* The build file is several thousand lines of code, without doing anything meaningful except listing each fuzz target in a highly verbose manner
* It makes writing new fuzz tests unnecessarily hard, as build system knowledge is required; Compare that to boost unit tests, which can be added by simply editing an existing cpp file
* It encourages fuzz tests that re-use the `buffer` or assume the `buffer` to be concatenations of seeds, which increases complexity of seeds and complexity for the fuzz engine to explore; Thus reducing the effectiveness of the affected fuzz targets
Fixes #20088
ACKs for top commit:
practicalswift:
Tested ACK fa13e1b0c52738492310b6b421d8e38cb04da5b1
sipa:
ACK fa13e1b0c52738492310b6b421d8e38cb04da5b1. Reviewed the code changes, and tested the 3 different test_runner.py modes (run once, merge, generate). I also tested building with the new --enable-danger-fuzz-link-all
Tree-SHA512: 962ab33269ebd51810924c51266ecc62edd6ddf2fcd9a8c359ed906766f58c3f73c223f8d3cc49f2c60f0053f65e8bdd86ce9c19e673f8c2b3cd676e913f2642
|
|
by using mocked GetTime()
8c09c0c1d18885ef94f79b3f2d073f43269bc95d fuzz: Avoid time-based "non-determinism" in fuzzing harnesses by using mocked GetTime() (practicalswift)
Pull request description:
Avoid time-based "non-determinism" in fuzzing harnesses by using mocked `GetTime()`.
Prior to this commit the fuzzing harnesses `banman`, `connman`, `net` and `rbf` had time-based "non-determinism". `addrman` is fixed in #20425. `process_message` and `process_messages` are left to fix: simply using mock time is not enough for them due to interaction with `IsInitialBlockDownload()`.
See [`doc/fuzzing.md`](https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md) for information on how to fuzz Bitcoin Core. Don't forget to contribute any coverage increasing inputs you find to the [Bitcoin Core fuzzing corpus repo](https://github.com/bitcoin-core/qa-assets).
Happy fuzzing :)
ACKs for top commit:
MarcoFalke:
review ACK 8c09c0c1d18885ef94f79b3f2d073f43269bc95d
practicalswift:
> review ACK [8c09c0c](https://github.com/bitcoin/bitcoin/commit/8c09c0c1d18885ef94f79b3f2d073f43269bc95d)
Tree-SHA512: 32dfbead3dfd18cf4ff56dc2ea341aa977441b4e19a54879cf54fa5820c7e2b14b92c7e238d32fd785654f3b28cc82826ae66c03e94c292633c63c41196ba9a8
|
|
|
|
|
|
mocked GetTime()
|
|
|
|
fa5ed3b4ca609426b2622cad235e107d33db7b30 net: Use Span in ReceiveMsgBytes (MarcoFalke)
Pull request description:
Pass a data pointer and a size as span in `ReceiveMsgBytes` to get the benefits of a span
ACKs for top commit:
jonatack:
ACK fa5ed3b4ca609426b2622cad235e107d33db7b30 code review, rebased to current master 12a1c3ad1a43634, debug build, unit tests, ran bitcoind/-netinfo/getpeerinfo
theStack:
ACK fa5ed3b4ca609426b2622cad235e107d33db7b30
Tree-SHA512: 89bf111323148d6e6e50185ad20ab39f73ab3a58a27e46319e3a08bcf5dcf9d6aa84faff0fd6afb90cb892ac2f557a237c144560986063bc736a69ace353ab9d
|
|
src/test/fuzz/net
|
|
|
|
|
|
There is no change in behavior on the P2P network.
|
|
We previously identified if we relay addresses to the connection by checking
for the existence of the m_addr_known data structure. With this commit, we
answer this question based on the connection type.
IsAddrRelayPeer() checked for the existence of the m_addr_known
|
|
-BEGIN VERIFY SCRIPT-
sed -i 's/OUTBOUND, /OUTBOUND_FULL_RELAY, /g' src/net.h
sed -i 's/ConnectionType::OUTBOUND/ConnectionType::OUTBOUND_FULL_RELAY/g' src/test/net_tests.cpp src/test/fuzz/process_message.cpp src/test/fuzz/process_messages.cpp src/net.cpp src/test/denialofservice_tests.cpp src/net.h src/test/fuzz/net.cpp
-END VERIFY SCRIPT-
|
|
|
