aboutsummaryrefslogtreecommitdiff
path: root/src/addrdb.cpp
AgeCommit message (Collapse)Author
2024-05-01scripted-diff: Add IWYU pragma keep to bitcoin-config.h includesMarcoFalke
-BEGIN VERIFY SCRIPT- perl -0777 -pi -e 's/#if defined\(HAVE_CONFIG_H\)\n#include <config\/bitcoin-config.h>.*\n#endif.*\n/#include <config\/bitcoin-config.h> \/\/ IWYU pragma: keep\n/g' $( git grep -l '#include <config/bitcoin-config.h>' ) -END VERIFY SCRIPT-
2024-03-23addrman: drop /*deterministic=*/ comment0xb10c
Just having deterministic is enough. See https://github.com/bitcoin/bitcoin/pull/29007#discussion_r1488241966
2024-03-12Merge bitcoin/bitcoin#29236: log: Nuke error(...)fanquake
fa391513949b7a3b56321436e2015c7e9e6dac2b refactor: Remove unused error() (MarcoFalke) fad0335517096f435d76adce7833e213d3cc23d1 scripted-diff: Replace error() with LogError() (MarcoFalke) fa808fb74972637840675e310f6d4a0f06028d61 refactor: Make error() return type void (MarcoFalke) fa1d62434843866d242bff9f9c55cb838a4f0d83 scripted-diff: return error(...); ==> error(...); return false; (MarcoFalke) fa9a5e80ab86c997102a1c3d4ba017bbe86641d5 refactor: Add missing {} around error() calls (MarcoFalke) Pull request description: `error(...)` has many issues: * It is often used in the context of `return error(...)`, implying that it has a "fancy" type, creating confusion with `util::Result/Error` * `-logsourcelocations` does not work with it, because it will pretend the error happened inside of `logging.h` * The log line contains `ERROR: `, as opposed to `[error]`, like for other errors logged with `LogError`. Fix all issues by removing it. ACKs for top commit: fjahr: re-utACK fa391513949b7a3b56321436e2015c7e9e6dac2b stickies-v: re-ACK fa391513949b7a3b56321436e2015c7e9e6dac2b, no changes since 4a903741b0 ryanofsky: Code review ACK fa391513949b7a3b56321436e2015c7e9e6dac2b. Just rebase since last review Tree-SHA512: ec5bb502ab0d3733fdb14a8a00762638fce0417afd8dd6294ae0d485ce2b7ca5b1efeb50fc2cd7467f6c652e4ed3e99b0f283b08aeca04bbfb7ea4f2c95d283a
2024-03-11Merge bitcoin/bitcoin#29007: test: create deterministic addrman in the ↵Ava Chow
functional tests 2cc8ca19f4185490f30a49516c890b2289fbab71 [test] Use deterministic addrman in addrman info tests (stratospher) a8978661093500df8d8dcf2a9c90837afacd0aab [test] Restart a node with empty addrman (stratospher) 71c19915c0c716d6f8a539dd92b8ad41e8c447ee [test] Use deterministic addrman in addpeeraddress test (stratospher) 7b868e6b678502e86571976d696c0e3cb72c0884 Revert "test: avoid non-determinism in asmap-addrman test" (stratospher) 69e091f3e1f65b12cf1804e7d39651f55a01827a [init] Create deterministic addrman in tests using -test=addrman (stratospher) be25ac3092b7755e26e1ec6c33a27cd0e3dd9eac [init] Remove -addrmantest command line arg (stratospher) 802e6e128bba5ffa6d4ec53ff45acccb7cb28f21 [init] Add new command line arg for use only in functional tests (stratospher) Pull request description: An address is placed in a `[bucket,position]` in the addrman table (new table or tried table) using the `addpeeraddress` RPC. This `[bucket,position]` is calculated using `nKey`(and other metrics) for the addrman which is chosen randomly during every run. Supposing there are 2 addresses to be placed in an addrman table. During every test run, a different `[bucket,position]` would be calculated for each address.These calculated `[bucket,position]` could even be the same for the 2 addresses in some test runs and result in collisions in the addrman. We wouldn't be able to predict when the collisions are going to happen because we can't predict the `nKey` value which is chosen at random. This can cause flaky tests. Because of these non deterministic collisions, we are limited in what we can do to test addrman functionality. Currently in our tests don't add a second address to prevent these collisions from happening - we only keep 1 address in the new table and 1 address in the tried table. See https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1091145647, https://github.com/bitcoin/bitcoin/pull/23084, [#22831(comment)](https://github.com/bitcoin/bitcoin/pull/22831/files#r708302639). This PR lets us create a deterministic addrman with fixed `nKey` so that we can know the `[bucket,position]` collisions beforehand, safely add more addresses in an addrman table and write more extensive tests. ACKs for top commit: amitiuttarwar: ACK 2cc8ca19f4185490f30a49516c890b2289fbab71 achow101: ACK 2cc8ca19f4185490f30a49516c890b2289fbab71 0xB10C: ACK 2cc8ca19f4185490f30a49516c890b2289fbab71 mzumsande: Code Review ACK 2cc8ca19f4185490f30a49516c890b2289fbab71 Tree-SHA512: 8acd9bdfe7de1eb44d22373bf13533d8ecf602df966fdd5b8b78afcd8cc35a286c95d2712f67a89473a0d68dded7d38f5599f6e4bf95a6589475444545bfb189
2024-03-11scripted-diff: Replace error() with LogError()MarcoFalke
This fixes the log output when -logsourcelocations is used. Also, instead of 'ERROR:', the log will now say '[error]', like other errors logged with LogError. -BEGIN VERIFY SCRIPT- sed -i --regexp-extended 's! error\("([^"]+)"! LogError("\1\\n"!g' $( git grep -l ' error(' ./src/ ) -END VERIFY SCRIPT-
2024-03-11scripted-diff: return error(...); ==> error(...); return false;MarcoFalke
This is needed for the next commit. -BEGIN VERIFY SCRIPT- # Separate sed invocations to replace one-line, and two-line error(...) calls sed -i --regexp-extended 's!( +)return (error\(.*\);)!\1\2\n\1return false;!g' $( git grep -l 'return error(' ) sed -i --null-data --regexp-extended 's!( +)return (error\([^\n]*\n[^\n]*\);)!\1\2\n\1return false;!g' $( git grep -l 'return error(' ) -END VERIFY SCRIPT-
2024-02-13scripted-diff: Fix bitcoin_config_h includesTheCharlatan
-BEGIN VERIFY SCRIPT- regex_string='^(?!//).*(AC_APPLE_UNIVERSAL_BUILD|BOOST_PROCESS_USE_STD_FS|CHAR_EQUALS_INT8|CLIENT_VERSION_BUILD|CLIENT_VERSION_IS_RELEASE|CLIENT_VERSION_MAJOR|CLIENT_VERSION_MINOR|COPYRIGHT_HOLDERS|COPYRIGHT_HOLDERS_FINAL|COPYRIGHT_HOLDERS_SUBSTITUTION|COPYRIGHT_YEAR|ENABLE_ARM_SHANI|ENABLE_AVX2|ENABLE_EXTERNAL_SIGNER|ENABLE_SSE41|ENABLE_TRACING|ENABLE_WALLET|ENABLE_X86_SHANI|ENABLE_ZMQ|HAVE_BOOST|HAVE_BUILTIN_CLZL|HAVE_BUILTIN_CLZLL|HAVE_BYTESWAP_H|HAVE_CLMUL|HAVE_CONSENSUS_LIB|HAVE_CXX20|HAVE_DECL_BE16TOH|HAVE_DECL_BE32TOH|HAVE_DECL_BE64TOH|HAVE_DECL_BSWAP_16|HAVE_DECL_BSWAP_32|HAVE_DECL_BSWAP_64|HAVE_DECL_FORK|HAVE_DECL_FREEIFADDRS|HAVE_DECL_GETIFADDRS|HAVE_DECL_HTOBE16|HAVE_DECL_HTOBE32|HAVE_DECL_HTOBE64|HAVE_DECL_HTOLE16|HAVE_DECL_HTOLE32|HAVE_DECL_HTOLE64|HAVE_DECL_LE16TOH|HAVE_DECL_LE32TOH|HAVE_DECL_LE64TOH|HAVE_DECL_PIPE2|HAVE_DECL_SETSID|HAVE_DECL_STRERROR_R|HAVE_DEFAULT_VISIBILITY_ATTRIBUTE|HAVE_DLFCN_H|HAVE_DLLEXPORT_ATTRIBUTE|HAVE_ENDIAN_H|HAVE_EVHTTP_CONNECTION_GET_PEER_CONST_CHAR|HAVE_FDATASYNC|HAVE_GETENTROPY_RAND|HAVE_GETRANDOM|HAVE_GMTIME_R|HAVE_INTTYPES_H|HAVE_LIBADVAPI32|HAVE_LIBCOMCTL32|HAVE_LIBCOMDLG32|HAVE_LIBGDI32|HAVE_LIBIPHLPAPI|HAVE_LIBKERNEL32|HAVE_LIBOLE32|HAVE_LIBOLEAUT32|HAVE_LIBSHELL32|HAVE_LIBSHLWAPI|HAVE_LIBUSER32|HAVE_LIBUUID|HAVE_LIBWINMM|HAVE_LIBWS2_32|HAVE_MALLOC_INFO|HAVE_MALLOPT_ARENA_MAX|HAVE_MINIUPNPC_MINIUPNPC_H|HAVE_MINIUPNPC_UPNPCOMMANDS_H|HAVE_MINIUPNPC_UPNPERRORS_H|HAVE_NATPMP_H|HAVE_O_CLOEXEC|HAVE_POSIX_FALLOCATE|HAVE_PTHREAD|HAVE_PTHREAD_PRIO_INHERIT|HAVE_STDINT_H|HAVE_STDIO_H|HAVE_STDLIB_H|HAVE_STRERROR_R|HAVE_STRINGS_H|HAVE_STRING_H|HAVE_STRONG_GETAUXVAL|HAVE_SYSCTL|HAVE_SYSCTL_ARND|HAVE_SYSTEM|HAVE_SYS_ENDIAN_H|HAVE_SYS_PRCTL_H|HAVE_SYS_RESOURCES_H|HAVE_SYS_SELECT_H|HAVE_SYS_STAT_H|HAVE_SYS_SYSCTL_H|HAVE_SYS_TYPES_H|HAVE_SYS_VMMETER_H|HAVE_THREAD_LOCAL|HAVE_TIMINGSAFE_BCMP|HAVE_UNISTD_H|HAVE_VM_VM_PARAM_H|LT_OBJDIR|PACKAGE_BUGREPORT|PACKAGE_NAME|PACKAGE_STRING|PACKAGE_TARNAME|PACKAGE_URL|PACKAGE_VERSION|PTHREAD_CREATE_JOINABLE|QT_QPA_PLATFORM_ANDROID|QT_QPA_PLATFORM_COCOA|QT_QPA_PLATFORM_MINIMAL|QT_QPA_PLATFORM_WINDOWS|QT_QPA_PLATFORM_XCB|QT_STATICPLUGIN|STDC_HEADERS|STRERROR_R_CHAR_P|USE_ASM|USE_BDB|USE_DBUS|USE_NATPMP|USE_QRCODE|USE_SQLITE|USE_UPNP|_FILE_OFFSET_BITS|_LARGE_FILES)' exclusion_files=":(exclude)src/minisketch :(exclude)src/crc32c :(exclude)src/secp256k1 :(exclude)src/crypto/sha256_arm_shani.cpp :(exclude)src/crypto/sha256_avx2.cpp :(exclude)src/crypto/sha256_sse41.cpp :(exclude)src/crypto/sha256_x86_shani.cpp" git grep --perl-regexp --files-with-matches "$regex_string" -- '*.cpp' $exclusion_files | xargs git grep -L "bitcoin-config.h" | while read -r file; do line_number=$(awk -v my_file="$file" '/\/\/ file COPYING or https?:\/\/www.opensource.org\/licenses\/mit-license.php\./ {line = NR} /^\/\// && NR == line + 1 {while(getline && /^\/\//) line = NR} END {print line+1}' "$file"); sed -i "${line_number}i\\\\n\#if defined(HAVE_CONFIG_H)\\n#include <config/bitcoin-config.h>\\n\#endif" "$file"; done; git grep --perl-regexp --files-with-matches "$regex_string" -- '*.h' $exclusion_files | xargs git grep -L "bitcoin-config.h" | while read -r file; do sed -i "/#define.*_H/a \\\\n\#if defined(HAVE_CONFIG_H)\\n#include <config/bitcoin-config.h>\\n\#endif" "$file"; done; for file in $(git grep --files-with-matches 'bitcoin-config.h' -- '*.cpp' '*.h' $exclusion_files); do if ! grep -q --perl-regexp "$regex_string" $file; then sed -i '/HAVE_CONFIG_H/{N;N;N;d;}' $file; fi; done; -END VERIFY SCRIPT- The first command creates a regular expression for matching all bitcoin-config.h symbols in the following form: ^(?!//).*(AC_APPLE_UNIVERSAL_BUILD|BOOST_PROCESS_USE_STD_FS|...|_LARGE_FILES). It was generated with: ./autogen.sh && printf '^(?!//).*(%s)' $(awk '/^#undef/ {print $2}' src/config/bitcoin-config.h.in | paste -sd "|" -) The second command holds a list of files and directories that should not be processed. These include subtree directories as well as some crypto files that already get their symbols through the makefile. The third command checks for missing bitcoin-config headers in .cpp files and adds the header if it is missing. The fourth command checks for missing bitcoin-config headers in .h files and adds the header if it is missing. The fifth command checks for unneeded bitcoin-config headers in sources files and removes the header if it is unneeded.
2024-01-08[init] Create deterministic addrman in tests using -test=addrmanstratospher
Supposing there are 2 different addresses to be placed in an addrman table. During every test run, a different [bucket,position] would be calculated for each address. These calculated [bucket,position] could end up being the same for the 2 different addresses in some test runs and result in collisions in the addrman. We wouldn't be able to predict when the collisions are going to happen because we can't predict the nKey value which is chosen at random. This can cause flaky tests. Improve this by allowing deterministic addrman creation in the functional tests. This creates an addrman with fixed `nKey` = 1 and we can know the [bucket,position] collisions beforehand, safely add more addresses in an addrman table and write more extensive tests.
2023-10-24build: Bump minimum supported Clang to clang-13MarcoFalke
2023-09-15Merge bitcoin/bitcoin#28473: refactor: Serialization parameter cleanupsfanquake
fb6a2ab63e310d8b600352ef41aab6dafccfbff0 scripted-diff: use SER_PARAMS_OPFUNC (Anthony Towns) 5e5c8f86b60a8018e8801fb44bbe56ce97d9deef serialize: add SER_PARAMS_OPFUNC (Anthony Towns) 33203f59b482bddfe0bbe7d497cb8731ce8334a4 serialize: specify type for ParamsWrapper not ref (Anthony Towns) bf147bfffa1afb11721f30e83eec1fa829f64d5f serialize: move ser_action functions out of global namespace (Anthony Towns) Pull request description: Cleanups after #25284: * ser_action namespacing - https://github.com/bitcoin/bitcoin/pull/25284#discussion_r1316189977 * make reference implicit - https://github.com/bitcoin/bitcoin/pull/25284#discussion_r1316277030 * function notation - https://github.com/bitcoin/bitcoin/pull/25284#issuecomment-1710714821 ACKs for top commit: MarcoFalke: lgtm ACK fb6a2ab63e310d8b600352ef41aab6dafccfbff0 💨 TheCharlatan: ACK fb6a2ab63e310d8b600352ef41aab6dafccfbff0 Tree-SHA512: aacca2ee9cfec360ade6b394606e13d1dfe05bc29c5fbdd48a4e6992bd420312d4ed0d32218d95c560646af326e9977728dc2e759990636298e326947f6f9526
2023-09-14scripted-diff: use SER_PARAMS_OPFUNCAnthony Towns
-BEGIN VERIFY SCRIPT- sed -i 's/WithParams(\(CAddress::V[12]_[A-Z]*\) *, */\1(/g' $(git grep -l 'WithParams' src/) sed -i 's/WithParams(\(CNetAddr::V[12]\) *, */\1(/g' $(git grep -l 'WithParams' src/) sed -i 's@\(CNetAddr::V1.CService{}.*\) //@\1 //@' src/test/util/net.cpp -END VERIFY SCRIPT-
2023-09-12kernel: Move MessageStartChars to its own fileTheCharlatan
The protocol.h file contains many non-consensus related definitions and should thus not be part of the libbitcoinkernel. This commit makes protocol.h no longer a required include for users of the libbitcoinkernel. This commit is part of the libbitcoinkernel project, namely its stage 1 step 3: Decouple most non-consensus headers from libbitcoinkernel. Co-Authored-By: Cory Fields <cory-nospam-@coryfields.com>
2023-09-12[refactor] Define MessageStartChars as std::arrayTheCharlatan
2023-09-05Use serialization parameters for CAddress serializationMarcoFalke
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>
2023-06-29script: appease spelling linterJon Atack
2023-05-30scripted-diff: move settings to common namespaceTheCharlatan
-BEGIN VERIFY SCRIPT- sed -i 's/namespace\ util/namespace\ common/g' src/common/settings.cpp src/common/settings.h sed -i 's/util\:\:GetSetting/common\:\:GetSetting/g' $( git grep -l 'util\:\:GetSetting') sed -i 's/util\:\:Setting/common\:\:Setting/g' $( git grep -l 'util\:\:Setting') sed -i 's/util\:\:FindKey/common\:\:FindKey/g' $( git grep -l 'util\:\:FindKey') sed -i 's/util\:\:ReadSettings/common\:\:ReadSettings/g' $( git grep -l 'util\:\:ReadSettings') sed -i 's/util\:\:WriteSettings/common\:\:WriteSettings/g' $( git grep -l 'util\:\:WriteSettings') -END VERIFY SCRIPT-
2023-05-30move-only: Move settings to the common libraryTheCharlatan
The background of this commit is an ongoing effort to decouple the libbitcoinkernel library from code that is not strictly required by it. The settings code belongs into the common library and namespace, since the kernel library should not depend on it. See doc/design/libraries.md for more information on this rationale. Changing the namespace of the moved functions is scripted in the following commit.
2023-05-29Add [[nodiscard]] where ignoring a Result return type is an errorMarcoFalke
2023-05-24refactor: Replace std::optional<bilingual_str> with util::ResultRyan Ofsky
2023-04-19move-only: Extract common/args and common/config.cpp from util/systemTheCharlatan
This is an extraction of ArgsManager related functions from util/system into their own common file. Config file related functions are moved to common/config.cpp. The background of this commit is an ongoing effort to decouple the libbitcoinkernel library from the ArgsManager. The ArgsManager belongs into the common library, since the kernel library should not depend on it. See doc/design/libraries.md for more information on this rationale.
2023-03-23refactor: Move fs.* to util/fs.*TheCharlatan
The fs.* files are already part of the libbitcoin_util library. With the introduction of the fs_helpers.* it makes sense to move fs.* into the util/ directory as well.
2023-03-23refactor: Extract util/fs_helpers from util/systemBen Woosley
This is an extraction of filesystem related functions from util/system into their own utility file. The background of this commit is an ongoing effort to decouple the libbitcoinkernel library from the ArgsManager defined in system.h. Moving these functions out of system.h allows including them from a separate source file without including the ArgsManager definitions from system.h.
2023-03-13refactor: Move error() from util/system.h to logging.hBen Woosley
error is a low-level function with a sole dependency on LogPrintf, which is defined in logging.h The background of this commit is an ongoing effort to decouple the libbitcoinkernel library from the ArgsManager defined in system.h. Moving the function out of system.h allows including it from a separate source file without including the ArgsManager definitions from system.h.
2023-01-31Merge bitcoin/bitcoin#26847: p2p: track AddrMan totals by network and table, ↵Andrew Chow
improve precision of adding fixed seeds 80f39c99ef2d30e3e2d8dbc068d25cf92aa32344 addrman, refactor: combine two size functions (Amiti Uttarwar) 4885d6f197736cb89fdfac250b280ec10829d903 addrman, refactor: move count increment into Create() (Martin Zumsande) c77c877a8e916878e09f64b2faa12eeca7528cc8 net: Load fixed seeds from reachable networks for which we don't have addresses (Martin Zumsande) d35595a78a4a6cae72d3204c1ec3f82f77a10d56 addrman: add function to return size by network and table (Martin Zumsande) Pull request description: AddrMan currently doesn't track the number of its entries by network, it only knows the total number of addresses. This PR makes AddrMan keep track of these numbers, which would be helpful for multiple things: 1. Allow to specifically add fixed seeds to AddrMan of networks where we don't have any addresses yet - even if AddrMan as a whole is not empty (partly fixing #26035). This is in particular helpful if the user abruptly changes `-onlynet` settings (such that addrs that used to be reachable are no longer and vice versa), in which case they currently could get stuck and not find any outbound peers. The second commit of this PR implements this. 1. (Future work): Add logic for automatic connection management with respect to networks - such as making attempts to have at least one connection to each reachable network as suggested [here](https://github.com/bitcoin/bitcoin/issues/26035#issuecomment-1249420209). This would involve requesting an address from a particular network from AddrMan, and expanding its corresponding function `AddrMan::Select()` to do this requires internal knowledge of the current number of addresses for each network and table to avoid getting stuck in endless loops. 1. (Future work): Perhaps display the totals to users. At least I would find this helpful to debug, the existing option (`./bitcoin-cli -addrinfo`) is rather indirect by doing the aggregation itself in each call, doesn't distinguish between new and tried, and being based on `AddrMan::GetAddr()` it's also subject to a quality filter which we probably don't want in this spot. ACKs for top commit: naumenkogs: utACK 80f39c9 stratospher: ACK 80f39c9 achow101: ACK 80f39c99ef2d30e3e2d8dbc068d25cf92aa32344 vasild: ACK 80f39c99ef2d30e3e2d8dbc068d25cf92aa32344 Tree-SHA512: 6359f2e3f4db7c120c0789d92d74cb7d87a2ceedb7d6a34b5eff20c7f55c5c81092d10ed94efe29afc1c66947820a0d9c14876ee0c8d1f8e068a6df4e1131927
2023-01-26addrman, refactor: combine two size functionsAmiti Uttarwar
The functionality of the old size() is covered by the new Size() when no arguments are specified, so this does not change behavior. Co-authored-by: Martin Zumsande <mzumsande@gmail.com>
2023-01-17addrdb: Only call Serialize() onceMartin Zumsande
The previous logic would call it once for serializing into the filestream, and then again for serializing into the hasher. If AddrMan was changed in between these calls by another thread, the resulting peers.dat would be corrupt with non-matching checksum and data. Fix this by using HashedSourceWriter, which writes the data to the underlying stream and keeps track of the hash in one go.
2022-12-24scripted-diff: Bump copyright headersHennadii Stepanov
-BEGIN VERIFY SCRIPT- ./contrib/devtools/copyright_header.py update ./ -END VERIFY SCRIPT- Commits of previous years: - 2021: f47dda2c58b5d8d623e0e7ff4e74bc352dfa83d7 - 2020: fa0074e2d82928016a43ca408717154a1c70a4db - 2019: aaaaad6ac95b402fe18d019d67897ced6b316ee0
2022-07-30Use steady clock for all millis bench loggingMacroFake
2022-05-12Merge bitcoin/bitcoin#24925: refactor: make GetRand a template, remove ↵MacroFake
GetRandInt ab1ea29ba1b8379a21fabd3dc859552c470a6421 refactor: make GetRand a template, remove GetRandInt (pasta) Pull request description: makes GetRand a template for which any integral type can be used, where the default behavior is to return a random integral up to the max of the integral unless a max is provided. This simplifies a lot of code from GetRand(std::numeric_limits<uint64_t>::max() -> GetRand<uint64_t>() ACKs for top commit: laanwj: Code review ACK ab1ea29ba1b8379a21fabd3dc859552c470a6421 Tree-SHA512: db5082a0e21783389f1be898ae73e097b31ab48cab1a2c0e29348a4adeb545d4098193aa72a547c6baa6e8205699aafec38d6a27b3d65522fb3246f91b4daae9
2022-05-03Merge bitcoin/bitcoin#24470: Disallow more unsafe string->path conversions ↵MacroFake
allowed by path append operators f64aa9c411ad78259756a28756ec1eb8069b5ab4 Disallow more unsafe string->path conversions allowed by path append operators (Ryan Ofsky) Pull request description: Add more `fs::path` `operator/` and `operator+` overloads to prevent unsafe string->path conversions on Windows that would cause strings to be decoded according to the current Windows locale & code page instead of the correct string encoding. Update application code to deal with loss of implicit string->path conversions by calling `fs::u8path` or `fs::PathFromString` explicitly, or by just changing variable types from `std::string` to `fs::path` to avoid conversions altogether, or make them happen earlier. In all cases, there's no change in behavior either (1) because strings only contained ASCII characters and would be decoded the same regardless of what encoding was used, or (2) because of the 1:1 mapping between paths and strings using the `PathToString` and `PathFromString` functions. Motivation for this PR was just that I was experimenting with #24469 and noticed that operations like `fs::path / std::string` were allowed, and I thought it would be better not to allow them. ACKs for top commit: hebasto: ACK f64aa9c411ad78259756a28756ec1eb8069b5ab4 Tree-SHA512: 944cce49ed51537ee7a35ea4ea7f5feaf0c8fff2fa67ee81ec5adebfd3dcbaf41b73eb35e49973d5f852620367f13506fd12a7a9b5ae3a7a0007414d5c9df50f
2022-04-22refactor: make GetRand a template, remove GetRandIntpasta
2022-04-22Merge bitcoin/bitcoin#22910: net: Encapsulate asmap in NetGroupManagerfanquake
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
2022-04-21Disallow more unsafe string->path conversions allowed by path append operatorsRyan Ofsky
Add more fs::path operator/ and operator+ overloads to prevent unsafe string->path conversions on Windows that would cause strings to be decoded according to the current Windows locale & code page instead of the correct string encoding. Update application code to deal with loss of implicit string->path conversions by calling fs::u8path or fs::PathFromString explicitly, or by just changing variable types from std::string to fs::path to avoid conversions altoghther, or make them happen earlier. In all cases, there's no change in behavior either (1) because strings only contained ASCII characters and would be decoded the same regardless of what encoding was used, or (2) because of the 1:1 mapping between paths and strings using the PathToString and PathFromString functions. Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
2022-04-21Merge bitcoin/bitcoin#24213: refactor: use Span in random.*laanwj
3ae7791bcaa88f5c68592673b8926ee807242ce7 refactor: use Span in random.* (pasta) Pull request description: ~This PR does two things~ 1. use a Span<unsigned char> for GetRandBytes and GetStrongRandBytes ~2. make GetRand a template for which any integral type can be used, where the default behavior is to return a random integral up to the max of the integral unless a max is provided. This simplifies a lot of code from `GetRand(std::numeric_limits<uint64_t>::max()` -> `GetRand<uint64_t>()`~ MarcoFalke this was inspired by your comment here: https://github.com/bitcoin/bitcoin/pull/24185#issuecomment-1025514263 about using Span, so hopefully I'll be able to get this PR done and merged 😂 ~Also, if requested I could revert the `GetRand(std::numeric_limits<uint64_t>::max()` -> `GetRand<uint64_t>()` related changes if it ends up causing too many conflicts~ ACKs for top commit: laanwj: Thank you! Code review re-ACK 3ae7791bcaa88f5c68592673b8926ee807242ce7 Tree-SHA512: 12375a83b68b288916ba0de81cfcab4aac14389a66a36811ae850427435eb67dd55e47df9ac3ec47db4e214f4330139e548bec815fff8a3f571484ea558dca79
2022-04-20[net] Move asmap into NetGroupManagerJohn Newbery
2022-04-04refactor: fix clang-tidy named args usagefanquake
2022-03-23refactor: use Span in random.*pasta
2022-02-25Merge bitcoin/bitcoin#24201: p2p: Avoid InitError when downgrading peers.datMarcoFalke
d41ed3215355582879c8eb6c99c2da33852f6cb1 p2p: Avoid InitError when downgrading peers.dat (junderw) Pull request description: fixes #24188 (also see https://github.com/bitcoin/bitcoin/pull/22762#issuecomment-951063826) When downgrading, a peers.dat with a future version that has a minimum required version larger than the downgraded Bitcoin Core version would cause an InitError. This commit changes this behavior to overwrite the existing peers.dat with a new empty one. ACKs for top commit: prayank23: reACK https://github.com/bitcoin/bitcoin/pull/24201/commits/d41ed3215355582879c8eb6c99c2da33852f6cb1 kallewoof: reACK d41ed3215355582879c8eb6c99c2da33852f6cb1 Tree-SHA512: c8e625fe36ce0b1aab6c8ef7241c8954038bb856f2de27bdc4814dc9a60e51be28815c7d77d0f96eace49687a0cea02deb713978bbd3a5add742f50a675f2a40
2022-02-25p2p: Avoid InitError when downgrading peers.datjunderw
fixes #24188 When downgrading, a peers.dat with a future version that has a minimum required version larger than the downgraded version would cause an InitError. This commit changes this behavior to overwrite the existing peers.dat with a new empty one, while creating a backup in peers.dat.bak.
2022-02-03refactor: replace boost::filesystem with std::filesystemKiminuo
Warning: Replacing fs::system_complete calls with fs::absolute calls in this commit may cause minor changes in behaviour because fs::absolute no longer strips trailing slashes; however these changes are believed to be safe. Co-authored-by: Russell Yanofsky <russ@yanofsky.org> Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
2021-12-30scripted-diff: Bump copyright headersHennadii Stepanov
-BEGIN VERIFY SCRIPT- ./contrib/devtools/copyright_header.py update ./ -END VERIFY SCRIPT- Commits of previous years: * 2020: fa0074e2d82928016a43ca408717154a1c70a4db * 2019: aaaaad6ac95b402fe18d019d67897ced6b316ee0
2021-10-05refactor: Block unsafe fs::path std::string conversion callsRussell Yanofsky
There is no change in behavior. This just helps prepare for the transition from boost::filesystem to std::filesystem by avoiding calls to methods which will be unsafe after the transaction to std::filesystem to due lack of a boost::filesystem::path::imbue equivalent and inability to set a predictable locale. Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Co-authored-by: Kiminuo <kiminuo@protonmail.com> Co-authored-by: MarcoFalke <falke.marco@gmail.com>
2021-09-28scripted-diff: Rename CAddrMan to AddrManAmiti Uttarwar
-BEGIN VERIFY SCRIPT- git grep -l CAddrMan src/ test/ | xargs sed -i 's/CAddrMan/AddrMan/g' -END VERIFY SCRIPT-
2021-09-27scripted-diff: Rename overloaded int GetArg to GetIntArgRussell Yanofsky
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>
2021-09-09Raise InitError when peers.dat is invalid or corruptedMarcoFalke
2021-09-09Inline ReadPeerAddressesMarcoFalke
No need to have a function that is only called in one place
2021-09-09Move LoadAddrman from init to addrdbMarcoFalke
Init should only concern itself with the initialization order, not the detailed initialization logic of every module. Also, inlining logic into a method that is ~800 lines of code, makes it impossible to unit test on its own.
2021-09-07Remove confusing CAddrDBMarcoFalke
The class only stores the file path, reading it from a global. Globals are confusing and make testing harder. The method reading from a stream does not even use any class members, so putting it in a class is also confusing.
2021-09-03MOVEONLY: Expose BanMapToJson / BanMapFromJsonRussell Yanofsky
CSubNet serialization code that was removed in fa4e6afdae7b82df638b60edf37ac36d57a8cb4f was needed by multiprocess code to share ban map between gui and node processes. Rather than adding it back, use suggestion from MarcoFalke <falke.marco@gmail.com> https://github.com/bitcoin/bitcoin/pull/10102#discussion_r690922929 to use JSON serialization. This requires making BanMapToJson / BanMapFromJson functions public.
2021-08-18[tests] Remove CAddrMan.Clear() call from CAddrDB::Read()John Newbery
`bool CAddrDB::Read(CAddrMan& addr, CDataStream& ssPeers)` is _only_ called from the tests, and the call to addr.Clear() only exists so that a test that Clear() is called passes. Remove that test and the call.
generated by cgit v1.2.3 (git 2.26.2) at 2024-10-12 13:10:41 +0000