aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2024-01-29Merge bitcoin/bitcoin#24748: test/BIP324: functional tests for v2 P2P encryptionAva Chow
bc9283c4415a932ec1eeb70ca2aa4399c80437b3 [test] Add functional test to test early key response behaviour in BIP 324 (stratospher) ffe6a56d75c0b47d0729e4e0b7225a827b43ad89 [test] Check whether v2 TestNode performs downgrading (stratospher) ba737358a37438c18f0fba723eab10ccfd9aae9b [test] Add functional tests to test v2 P2P behaviour (stratospher) 4115cf995647d1a513caecb54a4ff3f51927aa8e [test] Ignore BIP324 decoy messages (stratospher) 8c054aa04d33b247744b3747cd5bf3005a013e90 [test] Allow inbound and outbound connections supporting v2 P2P protocol (stratospher) 382894c3acd2dbf3e4198814f547c75b6fb17706 [test] Reconnect using v1 P2P when v2 P2P terminates due to magic byte mismatch (stratospher) a94e350ac0e5b65ef23a84b05fb10d1204c98c97 [test] Build v2 P2P messages (stratospher) bb7bffed799dc5ad8b606768164fce46d4cbf9d0 [test] Use lock for sending P2P messages in test framework (stratospher) 5b91fb14aba7d7fe45c9ac364526815bec742356 [test] Read v2 P2P messages (stratospher) 05bddb20f5cc9036fd680500bde8ece70dbf0646 [test] Perform initial v2 handshake (stratospher) a049d1bd08c8cdb3b693520f24f8a82572dcaab1 [test] Introduce EncryptedP2PState object in P2PConnection (stratospher) b89fa59e715a185d9fa7fce089dad4273d3b1532 [test] Construct class to handle v2 P2P protocol functions (stratospher) 8d6c848a48530893ca40be5c1285541b3e7a94f3 [test] Move MAGIC_BYTES to messages.py (stratospher) 595ad4b16880ae1f23463ca9985381c8eae945d8 [test/crypto] Add ECDH (stratospher) 4487b8051797173c7ab432e75efa370afb03b529 [rpc/net] Allow v2 p2p support in addconnection (stratospher) Pull request description: This PR introduces support for v2 P2P encryption(BIP 324) in the existing functional test framework and adds functional tests for the same. ### commits overview 1. introduces a new class `EncryptedP2PState` to store the keys, functions for performing the initial v2 handshake and encryption/decryption. 3. this class is used by `P2PConnection` in inbound/outbound connections to perform the initial v2 handshake before the v1 version handshake. Only after the initial v2 handshake is performed do application layer P2P messages(version, verack etc..) get exchanged. (in a v2 connection) - `v2_state` is the object of class `EncryptedP2PState` in `P2PConnection` used to store its keys, session-id etc. - a node [advertising](https://github.com/stratospher/blogosphere/blob/main/integration_test_bip324.md#advertising-to-support-v2-p2p) support for v2 P2P is different from a node actually [supporting v2 P2P](https://github.com/stratospher/blogosphere/blob/main/integration_test_bip324.md#supporting-v2-p2p) (differ when false advertisement of services occur) - introduce a boolean variable `supports_v2_p2p` in `P2PConnection` to denote if it supports v2 P2P. - introduce a boolean variable `advertises_v2_p2p` to denote whether `P2PConnection` which mimics peer behaviour advertises V2 P2P support. Default option is `False`. - In the test framework, you can create Inbound and Outbound connections to `TestNode` 1. During **Inbound Connections**, `P2PConnection` is the initiator [`TestNode` <--------- `P2PConnection`] - Case 1: - if the `TestNode` advertises/signals v2 P2P support (means `self.nodes[i]` set up with `"-v2transport=1"`), different behaviour will be exhibited based on whether: 1. `P2PConnection` supports v2 P2P 2. `P2PConnection` does not support v2 P2P - In a real world scenario, the initiator node would intrinsically know if they support v2 P2P based on whatever code they choose to run. However, in the test scenario where we mimic peer behaviour, we have no way of knowing if `P2PConnection` should support v2 P2P or not. So `supports_v2_p2p` boolean variable is used as an option to enable support for v2 P2P in `P2PConnection`. - Since the `TestNode` advertises v2 P2P support (using "-v2transport=1"), our initiator `P2PConnection` would send: 1. (if the `P2PConnection` supports v2 P2P) ellswift + garbage bytes to initiate the connection 2. (if the `P2PConnection` does not support v2 P2P) version message to initiate the connection - Case 2: - if the `TestNode` doesn't signal v2 P2P support; `P2PConnection` being the initiator would send version message to initiate a connection. 2. During **Outbound Connections** [TestNode --------> P2PConnection] - initiator `TestNode` would send: - (if the `P2PConnection` advertises v2 P2P) ellswift + garbage bytes to initiate the connection - (if the `P2PConnection` advertises v2 P2P) version message to initiate the connection - Suppose `P2PConnection` advertises v2 P2P support when it actually doesn't support v2 P2P (false advertisement scenario) - `TestNode` sends ellswift + garbage bytes - `P2PConnection` receives but can't process it and disconnects. - `TestNode` then tries using v1 P2P and sends version message - `P2PConnection` receives/processes this successfully and they communicate on v1 P2P 4. the encrypted P2P messages follow a different format - 3 byte length + 1-13 byte message_type + payload + 16 byte MAC 5. includes support for testing decoy messages and v2 connection downgrade(using false advertisement - when a v2 node makes an outbound connection to a node which doesn't support v2 but is advertised as v2 by some malicious intermediary) ### run the tests * functional test - `test/functional/p2p_v2_encrypted.py` `test/functional/p2p_v2_earlykeyresponse.py` I'm also super grateful to @ dhruv for his really valuable feedback on this branch. Also written a more elaborate explanation here - https://github.com/stratospher/blogosphere/blob/main/integration_test_bip324.md ACKs for top commit: naumenkogs: ACK bc9283c4415a932ec1eeb70ca2aa4399c80437b3 mzumsande: Code Review ACK bc9283c4415a932ec1eeb70ca2aa4399c80437b3 theStack: Code-review ACK bc9283c4415a932ec1eeb70ca2aa4399c80437b3 glozow: ACK bc9283c4415a932ec1eeb70ca2aa4399c80437b3 Tree-SHA512: 9b54ed27e925e1775e0e0d35e959cdbf2a9a1aab7bcf5d027e66f8b59780bdd0458a7a4311ddc7dd67657a4a2a2cd5034ead75524420d58a83f642a8304c9811
2024-01-29Merge bitcoin/bitcoin#27495: ci: Use LLVM 17.0.6 & DEBUG=1 in depends for ↵fanquake
MSAN jobs 8531e1e7312efe66204dc8ce56f21e44de99e956 ci: Use DEBUG=1 in depends for MSAN jobs (fanquake) 800ddef6b994b8b05a11f2c1c6d265daaec5751a ci: use LLVM 17.0.6 in MSAN jobs (fanquake) Pull request description: Switch to using LLVM 17.0.6 and `DEBUG=1` in MSAN CI jobs. ACKs for top commit: maflcko: lgtm ACK 8531e1e7312efe66204dc8ce56f21e44de99e956 Tree-SHA512: 819889762aeb78f95c4f955978890c6d98884bed0c7ff97ec072f4c7c1119ee3e3268ccab795bb1c801d36a206e16c6c1195e7a2bc7af94b580d17e49c632161
2024-01-29Merge bitcoin/bitcoin#29329: fuzz: Print coverage summary after run_oncefanquake
fab97d81ce3740509dbbe9270ca67a1b65b00c72 fuzz: Print coverage summary after run_once (MarcoFalke) Pull request description: This can be used to quickly check the coverage effects of a code change or qa-assets change. ACKs for top commit: dergoegge: ACK fab97d81ce3740509dbbe9270ca67a1b65b00c72 Tree-SHA512: 0ac913c14698f39e76e0e7bf124f182220031796d6443edb34c6e4615e128157cf746da661b216c4640a41964e977249712445ca9c005b1b4a3737adabdb4a7d
2024-01-29fuzz: Print coverage summary after run_onceMarcoFalke
2024-01-29Merge bitcoin/bitcoin#29298: depends: patch libool out of libnatpmp/miniupnpcfanquake
5b9d5bf866506b22270598aa2dc1269bc02e38e2 depends: remove (darwin) libtool now that it's no longer used (Cory Fields) 3ef6563495428d605409089b2267e18f2bf491f9 depends: use ar rather than libtool for miniupnpc/libnatpmp (Cory Fields) Pull request description: An alternative to https://github.com/bitcoin/bitcoin/pull/29232 Rather than switching to the CMake builds which [proved problematic](https://github.com/bitcoin/bitcoin/pull/29232#issuecomment-1898513919), do the quick and dirty thing of just patching out libtool. Doesn't seem to introduce any new issues. This should buy us time to upstream the necessary CMake fixes. ACKs for top commit: TheCharlatan: ACK 5b9d5bf866506b22270598aa2dc1269bc02e38e2 fanquake: ACK 5b9d5bf866506b22270598aa2dc1269bc02e38e2 Tree-SHA512: c75c4bcc9332d8c1fc3395e2b5fc7265849186afc7005700f662ab291e6ea1f111025fad733d0b0b39d35029d1b757d3f1937d63aad3c0c3b88d0f8ac902ee18
2024-01-29doc: update `BroadcastTransaction` commentismaelsadeeq
BroadcastTransaction is also called by submitpackage RPC. It's not maintainable to list all the callers of a function.
2024-01-29test: fix wallet_import_rescan unrounded minimum amountstickies-v
Fixes a `JSONRPCException: Invalid amount (-3)` exception by ensuring the amount sent to `sendtoaddress` is rounded to 8 decimals. See https://cirrus-ci.com/task/5562947183837184?logs=ci#L2559
2024-01-29test: Remove struct import from messages.pyMarcoFalke
2024-01-29scripted-diff: test: Use int from_bytes and to_bytes over struct packingMarcoFalke
-BEGIN VERIFY SCRIPT- sed -i --regexp-extended 's!struct.unpack\("(|<|>)B", (.*)\)\[0\]!int.from_bytes(\2, "little")!g' ./test/functional/test_framework/messages.py sed -i --regexp-extended 's!struct.unpack\("<(H|I|Q)", (.*)\)\[0\]!int.from_bytes(\2, "little")!g' ./test/functional/test_framework/messages.py sed -i --regexp-extended 's!struct.unpack\("<(h|i|q)", (.*)\)\[0\]!int.from_bytes(\2, "little", signed=True)!g' ./test/functional/test_framework/messages.py sed -i --regexp-extended 's!struct.unpack\(">(H|I|Q)", (.*)\)\[0\]!int.from_bytes(\2, "big")!g' ./test/functional/test_framework/messages.py sed -i --regexp-extended 's!struct.pack\("<?B", (.*)\)!\1.to_bytes(1, "little")!g' ./test/functional/test_framework/messages.py sed -i --regexp-extended 's!struct.pack\("<I", (.*)\)!\1.to_bytes(4, "little")!g' ./test/functional/test_framework/messages.py sed -i --regexp-extended 's!struct.pack\("<i", (.*)\)!\1.to_bytes(4, "little", signed=True)!g' ./test/functional/test_framework/messages.py sed -i --regexp-extended 's!struct.pack\("<Q", (.*)\)!\1.to_bytes(8, "little")!g' ./test/functional/test_framework/messages.py sed -i --regexp-extended 's!struct.pack\("<q", (.*)\)!\1.to_bytes(8, "little", signed=True)!g' ./test/functional/test_framework/messages.py sed -i --regexp-extended 's!struct.pack\(">H", (.*)\)!\1.to_bytes(2, "big")!g' ./test/functional/test_framework/messages.py -END VERIFY SCRIPT-
2024-01-29test: Use int from_bytes and to_bytes over struct packingMarcoFalke
This is done in prepration for the scripted diff, which can not deal with the 0 literal int.
2024-01-29test: Treat msg_version.relay as unsignedMarcoFalke
The C++ code treats bool as uint8_t, so the python tests should as well. This also allows to simplify the code, because converting an empty byte array to int gives int(0). >>> int.from_bytes(b'') 0
2024-01-26Merge bitcoin/bitcoin#29180: crypto: remove use of BUILD_BITCOIN_INTERNAL ↵Ava Chow
macro in sha256 bbf218d06164b7247f5e9df5ba143383022fbf74 crypto: remove sha256_sse4 from the base crypto helper lib (Cory Fields) 4dbd0475d8c16ed10c309bf6badc17f2d2eaaa69 crypto: remove use of BUILD_BITCOIN_INTERNAL macro in sha256 (Cory Fields) Pull request description: Replace it with a more explicit `DISABLE_OPTIMIZED_SHA256` and clean up some. The macro was originally used by libbitcoinconsensus which opts out of optimized sha256 for the sake of simplicity. Also remove the `BUILD_BITCOIN_INTERNAL` define from libbitcoinkernel for now as it does not export an api. When it does we can pick a less confusing define to control its exports. Removing the define should have the effect of enabling sha256 optimizations for the kernel. ACKs for top commit: TheCharlatan: Re-ACK bbf218d06164b7247f5e9df5ba143383022fbf74 hebasto: re-ACK bbf218d06164b7247f5e9df5ba143383022fbf74 Tree-SHA512: 7c17592bb2d3e671779f96903cb36887c5785408213bffbda1ae37b66e6bcfaffaefd0c1bf2d1a407060cd377e3d4881cde3a73c429a1aacb677f370314a066a
2024-01-26Merge bitcoin/bitcoin#29283: test: ensure output is large enough to pay for ↵Ava Chow
its fees 3bfc5bd36e38ca07306a41a3f56ea102ffe02b67 test: ensure output is large enough to pay for its fees (stickies-v) Pull request description: Fixes a (rare) intermittency issue in wallet_import_rescan.py Since we [use](https://github.com/bitcoin/bitcoin/blob/03752444cd54df05a731557968d5a9f33a55c55c/test/functional/wallet_import_rescan.py#L296) `subtract_fee_from_outputs=[0]` in the `send` command, the output amount must at least be as large as the fee we're paying. Example in CI: https://api.cirrus-ci.com/v1/task/6107972259020800/logs/ci.log ``` 2024-01-18T22:16:12.383000Z TestFramework (INFO): Test that the mempool is rescanned as well if the rescan parameter is set to true 2024-01-18T22:16:20.187000Z TestFramework (ERROR): JSONRPC error Traceback (most recent call last): File "/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 131, in main self.run_test() File "/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/wallet_import_rescan.py", line 292, in run_test child = self.nodes[1].send( File "/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/coverage.py", line 50, in __call__ return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs) File "/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/authproxy.py", line 129, in __call__ raise JSONRPCException(response['error'], status) test_framework.authproxy.JSONRPCException: The transaction amount is too small to pay the fee (-4) ``` Can be reproduced locally by forcing usage of the lowest possible value produced by `get_rand_amount()` ([thanks furszy](https://github.com/bitcoin/bitcoin/pull/29283#pullrequestreview-1832956095)): <details> <summary>git diff on 5f3a0574c4</summary> ```diff diff --git a/test/functional/wallet_import_rescan.py b/test/functional/wallet_import_rescan.py index 7f01d23941..925849d5c0 100755 --- a/test/functional/wallet_import_rescan.py +++ b/test/functional/wallet_import_rescan.py @@ -270,7 +270,7 @@ class ImportRescanTest(BitcoinTestFramework): address_type=variant.address_type.value, )) variant.key = self.nodes[1].dumpprivkey(variant.address["address"]) - variant.initial_amount = get_rand_amount() * 2 + variant.initial_amount = Decimal(str(round(AMOUNT_DUST, 8))) * 2 variant.initial_txid = self.nodes[0].sendtoaddress(variant.address["address"], variant.initial_amount) variant.confirmation_height = 0 variant.timestamp = timestamp ``` </details> ACKs for top commit: achow101: ACK 3bfc5bd36e38ca07306a41a3f56ea102ffe02b67 glozow: utACK 3bfc5bd36e38ca07306a41a3f56ea102ffe02b67, didn't experience this issue but in theory a minimum of `AMOUNT_DUST` could be too low to pay the fees furszy: utACK 3bfc5bd36 Tree-SHA512: 821ab94a510772e90528b2cef368bbf70309d8fd1dcda53dce75dd1bf91622358e80fea4d9fc68249b9d598892306c66f6c843b4a6855a9f9a9175f7b41109c6
2024-01-26Merge bitcoin-core/gui#789: Avoid non-self-contained Windows headerHennadii Stepanov
8023640a71a10ec54a6a8e6b95a29d07f7be218d qt: Avoid non-self-contained Windows header (Hennadii Stepanov) Pull request description: Using the `windows.h` header guarantees correctness regardless of the content of other headers. For more details, please refer to https://stackoverflow.com/questions/4845198/fatal-error-no-target-architecture-in-visual-studio Fixes the MSVC build when using the upcoming CMake-based build system and Qt packages installed via the vcpkg package manager. Related to https://github.com/hebasto/bitcoin/pull/77. ACKs for top commit: theuni: ACK 8023640a71a10ec54a6a8e6b95a29d07f7be218d. It's not completely clear to me why this currently works, but I don't think it's worth wasting more time on. `windows.h` seems more correct regardless. Tree-SHA512: 1c03f909943111fb2663f86d33ec9a947bc5903819e5bd94f436f6b0782d9f5c5d80d9cd3490674ecd8921b2981c509e97e41580bccc436f8b5c7db84b4e493c
2024-01-26depends: remove (darwin) libtool now that it's no longer usedCory Fields
Note that this is completely unrelated to gnu usage of libtool.
2024-01-26depends: use ar rather than libtool for miniupnpc/libnatpmpCory Fields
2024-01-26Merge bitcoin/bitcoin#29327: fuzz: also set MSAN_SYMBOLIZER_PATHfanquake
cf937b2068dba167b6c7ea6f8ee9ba366803c3bb fuzz: also set MSAN_SYMBOLIZER_PATH (fanquake) Pull request description: Should resolve: https://github.com/bitcoin-core/qa-assets/issues/167. ACKs for top commit: dergoegge: utACK cf937b2068dba167b6c7ea6f8ee9ba366803c3bb Tree-SHA512: a7670b5054c2c9ec830db2a4dd4d78d8a0ee7d793a80d32942d78b5e459015344040fa9ce9d73f4f23cd920d5ca2e65c110e201723e4935de8f57fda0b6d5ce7
2024-01-26fuzz: also set MSAN_SYMBOLIZER_PATHfanquake
2024-01-26ci: Use DEBUG=1 in depends for MSAN jobsfanquake
Followup to #27448, which was deffered, as it produces #27448 and another similar issue in sqlite, see comment here: https://github.com/bitcoin/bitcoin/pull/27448#issuecomment-1514902450.
2024-01-26ci: use LLVM 17.0.6 in MSAN jobsfanquake
2024-01-26Merge bitcoin/bitcoin#28875: build: Pass sanitize flags to instrument ↵fanquake
`libsecp256k1` code cbea49c0d32badb975fbf22d44f8e25cc7972af7 build: Pass sanitize flags to instrument `libsecp256k1` code (Hennadii Stepanov) Pull request description: This PR is a revived https://github.com/bitcoin/bitcoin/pull/27991 with an addressed [comment](https://github.com/bitcoin/bitcoin/pull/27991#discussion_r1252148488). Fixes https://github.com/bitcoin/bitcoin/issues/27990. Might be tested as follows: ``` $ ./autogen.sh && ./configure --enable-fuzz --with-sanitizers=fuzzer CC=clang-13 CXX=clang++-13 $ make clean > /dev/null && make $ objdump --disassemble=secp256k1_xonly_pubkey_serialize src/test/fuzz/fuzz | grep __sanitizer_cov 1953bd0:e8 bb c6 05 ff call 9b0290 <__sanitizer_cov_trace_const_cmp8> 1953d32:e8 69 c4 05 ff call 9b01a0 <__sanitizer_cov_trace_pc_indir> 1953d58:e8 43 c4 05 ff call 9b01a0 <__sanitizer_cov_trace_pc_indir> 1953d82:e8 19 c4 05 ff call 9b01a0 <__sanitizer_cov_trace_pc_indir> ``` ACKs for top commit: fanquake: ACK cbea49c0d32badb975fbf22d44f8e25cc7972af7 dergoegge: reACK cbea49c0d32badb975fbf22d44f8e25cc7972af7 Tree-SHA512: 801994e75b711d20eaf0d675f378da07d693f4a7de026efd93860f5f1deabed855a83eca3561725263e4fe605fcc5f91eb73c021ec91c831864e6deb575e3885
2024-01-25Merge bitcoin/bitcoin#29315: refactor: Compile unreachable walletdb codeAva Chow
fa3373d3adbace7e4665cf391363319a55a09a96 refactor: Compile unreachable code (MarcoFalke) Pull request description: When unreachable code isn't compiled, compile failures are not detected. Fix this by leaving it unreachable, but compiling it. Fixes https://github.com/bitcoin/bitcoin/pull/28999#discussion_r1465010916 ACKs for top commit: achow101: ACK fa3373d3adbace7e4665cf391363319a55a09a96 ryanofsky: Code review ACK fa3373d3adbace7e4665cf391363319a55a09a96. This looks good, and should prevent code in the else blocks from accidentally breaking. Tree-SHA512: 3a3764915dfc935bf5d7a48f1ca151dcbac340c1cbdce8236b24ae9b4f04d6ee9771ed058ca60bcbca6e19d13671de3517f828a8f7ab6444c7cc4e3538d1ba4e
2024-01-25Merge bitcoin/bitcoin#20827: During IBD, prune as much as possible until we ↵Ava Chow
get close to where we will eventually keep blocks d298ff8b62b2624ed390c8a2f905c888ffc956ff During IBD, prune as much as possible until we get close to where we will eventually keep blocks (Luke Dashjr) Pull request description: This should reduce pruning flushes even more, speeding up IBD with pruning on systems that have a sufficient dbcache. Assumes 1 MB per block between tip and best header chain. Simply adds this to the buffer pruning is trying to leave available, which results in pruning almost everything up until we get close to where we need to be keeping blocks. ACKs for top commit: andrewtoth: ACK d298ff8b62b2624ed390c8a2f905c888ffc956ff fjahr: utACK d298ff8b62b2624ed390c8a2f905c888ffc956ff achow101: ACK d298ff8b62b2624ed390c8a2f905c888ffc956ff Tree-SHA512: 2a482376bfb177e2ba7c2f0bb0b58b02efdb38b34755a18d1fc3e869df5959c85b6f1009e1386fa8b89c4f90d520383e36bd3e21dec221042315134efb1a455b
2024-01-25Merge bitcoin/bitcoin#29287: depends: Do not override `CFLAGS` when building ↵fanquake
SQLite with `DEBUG=1` 5fb8f0f80fc41cc636da56864195244d8fd9116e depends: Do not override CFLAGS when building SQLite with DEBUG=1 (Hennadii Stepanov) 2b0dd88f1ce9084324dc54db578fade9c926fd71 depends: Ensure definitions are passed when building SQLite with DEBUG=1 (Hennadii Stepanov) Pull request description: The `--enable-debug` configure option for the SQLite package does two things: ```autoconf #----------------------------------------------------------------------- # --enable-debug # AC_ARG_ENABLE(debug, [AS_HELP_STRING( [--enable-debug], [build with debugging features enabled [default=no]])], [], []) AC_MSG_CHECKING([Build type]) if test x"$enable_debug" = "xyes"; then BUILD_CFLAGS="$BUILD_CFLAGS -DSQLITE_DEBUG -DSQLITE_ENABLE_SELECTTRACE -DSQLITE_ENABLE_WHERETRACE" CFLAGS="-g -O0" AC_MSG_RESULT([debug]) else AC_MSG_RESULT([release]) fi #----------------------------------------------------------------------- ``` It adds three preprocessor definitions and overrides `CFLAGS` with `"-g -O0"`. The latter breaks the user's ability to provide sanitizer and LTO flags. This PR might be especially useful for OSS-Fuzz where `DEBUG=1` has been used since https://github.com/google/oss-fuzz/pull/10503. Also it makes a workaround for building SQLite for 32-bit unneeded. For details, please refer to https://github.com/hebasto/oss-fuzz/tree/240120-sqlite. Changes in https://github.com/bitcoin/bitcoin/pull/29282 might not be strictly required now. However, I consider them an improvement. ACKs for top commit: fanquake: ACK 5fb8f0f80fc41cc636da56864195244d8fd9116e - downstream is also green, so i'll fixup the PR there. Tree-SHA512: 8593d8a0237ebb270d5da763fb65ed642ab8ed0d44e57704a34154621f49e3d5c58b462cc0070251fa1ba556c58a3c7d3620530d6839dc6dc9e0887010330eca
2024-01-25refactor: Compile unreachable codeMarcoFalke
When unreachable code isn't compiled, compile failures are not detected. Fix this by leaving it unreachable, but compiling it. Fixes https://github.com/bitcoin/bitcoin/pull/28999#discussion_r1465010916 Can be reviewed with --ignore-all-space
2024-01-25Merge bitcoin/bitcoin#29313: ci: Update cache actionfanquake
ec25e745420fce5fd3e14b0c39e6f475d918d5ad ci: Update cache action (Hennadii Stepanov) Pull request description: This PR fixes deprecation [warnings](https://github.com/bitcoin/bitcoin/actions/runs/7652979339) for Node.js 16 actions in the GHA CI: ![image](https://github.com/bitcoin/bitcoin/assets/32963518/ea7b0708-8b2f-446f-a16d-ecc2c8a1da45) See: - https://github.com/marketplace/actions/cache - https://github.com/actions/cache/releases/tag/v4.0.0 Top commit has no ACKs. Tree-SHA512: 48503abab5d188d6fac2a1ead62512c217a831f611c4dce0e05666d72fac4db26f947cbe9a42fda0307cbdcb9aa0bd4b4d7a15ac2c14c757f92ba2916da0020b
2024-01-25depends: Do not override CFLAGS when building SQLite with DEBUG=1Hennadii Stepanov
The `--enable-debug` configure option for the SQLite package does two things. It adds three preprocessor definitions and overrides CFLAGS with "-g -O0". The latter breaks the user's ability to provide sanitizer and LTO flags.
2024-01-25depends: Ensure definitions are passed when building SQLite with DEBUG=1Hennadii Stepanov
The SQLite build system overrides the `CFLAGS` when is configured with the `--enable-debug` option.
2024-01-25ci: Update cache actionHennadii Stepanov
This change fixes deprecation warnings for Node.js 16 actions in the GHA CI. See: - https://github.com/marketplace/actions/cache - https://github.com/actions/cache/releases/tag/v4.0.0
2024-01-25qt: Avoid non-self-contained Windows headerHennadii Stepanov
Using the `windows.h` header guarantees correctness regardless of the content of other headers. For more details, please refer to https://stackoverflow.com/questions/4845198/fatal-error-no-target-architecture-in-visual-studio Fixes the MSVC build when using the upcoming CMake-based build system and Qt packages installed via the vcpkg package manager.
2024-01-25Merge bitcoin/bitcoin#29205: build: always set `-g -O2` in `CORE_CXXFLAGS`fanquake
00c1e2aa4496b5f038ae5199dbd16d8313766533 build: fix optimisation flags used for --coverage (fanquake) 1dc2c9b385f8345c588449848149b8e470653afc ci: cleanup C*FLAG usage in Valgrind jobs (fanquake) 6cc2a38c1388b696e9c28a08c6bd9c93da4fa6b8 build: add sanitizer flags to configure output (fanquake) 08cd5aca18f0774258c7c459773b9e8b386d48ef build: always set -g -O2 in CORE_CXXFLAGS (fanquake) Pull request description: Rather than trying to sporadically rely on / override Autoconf default behaviour. Just always override (if unset), and always set the flags we want (which are the same as the Autoconf defaults). Removes the need for duplicate code to clear (if not overridden) `CXXFLAGS`. Fixes cases of "missing" `-O2`. i.e this PR when running a Valgrind CI job with changes here: ```bash CXXFLAGS = -g -O2 -fdebug-prefix-map=$(abs_top_srcdir)=. -Wstack-protector -fstack-protector-all -mbranch-protection=bti -Werror -fsanitize=fuzzer -gdwarf-4 ``` Fixes configure output to reflect actual compilation flag ordering, so it's useful. Note that if we do still end up with a duplicate "-g -O2" when compiling, that has no effect, and I don't really thinks it's something worth trying to optimize. ACKs for top commit: TheCharlatan: lgtm ACK 00c1e2aa4496b5f038ae5199dbd16d8313766533 hebasto: ACK 00c1e2aa4496b5f038ae5199dbd16d8313766533, I have reviewed the code and it looks OK. Also tested `ci/test/00_setup_env_native_valgrind.sh`. theuni: ACK 00c1e2aa4496b5f038ae5199dbd16d8313766533 Tree-SHA512: cf6c7acf813ba10b198561e83eb72e9b2532a39cb1767c452d031e82921dcd42a47b129735b24c4e36131fd0c8fe7457f7cae870c1e011cdfdd430bdc4d4912b
2024-01-25[test] Add functional test to test early key response behaviour in BIP 324stratospher
- A node initiates a v2 connection by sending 64 bytes ellswift - In BIP 324 "The responder waits until one byte is received which does not match the V1_PREFIX (16 bytes consisting of the network magic followed by "version\x00\x00\x00\x00\x00".)" - It's possible that the 64 bytes ellswift sent by an initiator starts with a prefix of V1_PREFIX - Example form of 64 bytes ellswift could be: 4 bytes network magic + 60 bytes which aren't prefixed with remaining V1_PREFIX - We test this behaviour: - when responder receives 4 byte network magic -> no response received by initiator - when first mismatch happens -> response received by initiator
2024-01-25[test] Check whether v2 TestNode performs downgradingstratospher
2024-01-25[test] Add functional tests to test v2 P2P behaviourstratospher
2024-01-25[test] Ignore BIP324 decoy messagesstratospher
Also allow P2PConnection::send_message() to send decoy messages for writing tests.
2024-01-25[test] Allow inbound and outbound connections supporting v2 P2P protocolstratospher
- Add an optional `supports_v2_p2p` parameter to specify if the inbound and outbound connections support v2 P2P protocol. - In the `addconnection_callback` which gets called when creating outbound connections, call the `addconnection` RPC with v2 P2P protocol support enabled.
2024-01-25 [test] Reconnect using v1 P2P when v2 P2P terminates due to magic byte mismatchstratospher
- When a v2 TestNode makes an outbound connection to a P2PInterface node which doesn't support v2 but is advertised as v2 by some malicious intermediary, the TestNode sends 64 bytes ellswift. The v1 node doesn't understand this and disconnects. Then the v2 TestNode reconnects by sending a v1/version message.
2024-01-25[test] Build v2 P2P messagesstratospher
2024-01-25[test] Use lock for sending P2P messages in test frameworkstratospher
Messages are built, encrypted and sent over the socket in v2 connections. If a race condition happens between python's main thread and p2p thread with both of them trying to send a message, it's possible that the messages get encrypted with wrong keystream. Messages are built and sent over the socket in v1 connections. So there's no problem if messages are sent in the wrong order. Co-authored-by: Martin Zumsande <mzumsande@gmail.com> Co-authored-by: theStack <sebastian.falbesoner@gmail.com>
2024-01-25[test] Read v2 P2P messagesstratospher
2024-01-25[test] Perform initial v2 handshakestratospher
2024-01-25[test] Introduce EncryptedP2PState object in P2PConnectionstratospher
Instantiate this object when the connection supports v2 P2P transport protocol. - When a P2PConnection is opened, perform initiate_v2_handshake() if the connection is an initiator. application layer messages are only sent after the initial v2 handshake is over (for both initiator and responder).
2024-01-24Merge bitcoin/bitcoin#29302: wallet: clarify replaced_by_txid and ↵Ava Chow
replaces_txid in help output ff54314d4abed3bf9a78daf785a01c63af15c69d wallet: clarify replaced_by_txid and replaces_txid in help output (marco) Pull request description: Resolves issue #27781 ACKs for top commit: achow101: ACK ff54314d4abed3bf9a78daf785a01c63af15c69d ryanofsky: Code review ACK ff54314d4abed3bf9a78daf785a01c63af15c69d. Seems like a helpful clarification Tree-SHA512: b13a0e24505dfaee083467ac6f357b96460b5d1841dc29c4df4a503c290d379cef3d50fcc76f33bbc95741f484dd9d2461b0c2e8bdebf57a8a72edfbeece2a79
2024-01-24Merge bitcoin/bitcoin#29304: fuzz: Exit and log stderr for parse_test_list ↵fanquake
errors 9d09c873a50d344e2a9cb35fe246a52688b9fa34 fuzz: Exit and log stderr for parse_test_list errors (dergoegge) Pull request description: We should log all errors that occur when attempting to print the harness list in the fuzz test runner. ACKs for top commit: maflcko: lgtm ACK 9d09c873a50d344e2a9cb35fe246a52688b9fa34 Tree-SHA512: 50471b732c8cbe287dacba14487e7c8a5826f146432d93aa3bb55d063a8ba158d01641d6cb1360241dd4cd54ef5e045b0412f9cc34d06c181134921d1f1ceced
2024-01-24fuzz: Exit and log stderr for parse_test_list errorsdergoegge
2024-01-24[test] Construct class to handle v2 P2P protocol functionsstratospher
The class `EncryptedP2PState` stores the 4 32-byte keys, session id, garbage terminators, whether it's an initiator/responder, whether the initial handshake has been completed etc.. It also contains functions to perform the v2 handshake and to encrypt/decrypt p2p v2 messages. - In an inbound connection to TestNode, P2PConnection is the initiator and `initiate_v2_handshake()`, `complete_handshake()`, `authenticate_handshake()` are called on it. [ TestNode <----------------- P2PConnection ] - In an outbound connection from TestNode, P2PConnection is the responder and `respond_v2_handshake()`, `complete_handshake()`, `authenticate_handshake()` are called on it. [ TestNode -----------------> P2PConnection ]
2024-01-23wallet: clarify replaced_by_txid and replaces_txid in help outputmarco
2024-01-23init: settings, do not load auto-generated warning msgfurszy
The settings warning message is meant to be used only to discourage users from modifying the file manually. Therefore, there is no need to keep it in memory.
2024-01-23validation: move nChainTx assert down in CheckBlockIndexMartin Zumsande
There is a designated section meant for the actual consistency checks, marked by a comment.
2024-01-23doc: fix checkblockindex commentsMartin Zumsande
These exceptions are not related to situations specific to tests, but are required in general: Without the first check CheckBlockindex could fail for blocks where we only know the header. Without the second, it could fail when blocks are received out of order.