aboutsummaryrefslogtreecommitdiff
path: root/test
AgeCommit message (Collapse)Author
2023-07-11Merge bitcoin/bitcoin#28025: test: refactor: deduplicate legacy ECDSA ↵Andrew Chow
signing for tx inputs 5cf44275c8ca8c32d238f37f717d78e9823f44c2 test: refactor: deduplicate legacy ECDSA signing for tx inputs (Sebastian Falbesoner) Pull request description: There are several instances in functional tests and the framework (MiniWallet, feature_block.py, p2p_segwit.py) where we create a legacy ECDSA signature for a certain transaction's input by doing the following steps: 1. calculate the `LegacySignatureHash` with the desired sighash type 2. create the actual digital signature by calling `ECKey.sign_ecdsa` on the signature message hash calculated above 3. put the DER-encoded result as CScript data push into tx input's scriptSig Create a new helper `sign_input_legacy` which hides those details and takes only the necessary parameters (tx, input index, relevant scriptPubKey, private key, sighash type [SIGHASH_ALL by default]). For further convenience, the signature is prepended to already existing data-pushes in scriptSig, in order to avoid rehashing the transaction after calling the new signing function. ACKs for top commit: dimitaracev: ACK `5cf4427` achow101: ACK 5cf44275c8ca8c32d238f37f717d78e9823f44c2 pinheadmz: ACK 5cf44275c8ca8c32d238f37f717d78e9823f44c2 Tree-SHA512: 8f0e4fb2c3e0f84fac5dbc4dda87973276242b0f628034272a7f3e45434c1e17dd1b26a37edfb302dcaf380dbfe98b0417391ace5e0ac9720155d8fba702031e
2023-07-11Merge bitcoin/bitcoin#28028: test: Check expected_stderr after stopfanquake
faf902858d38150caa8991b0ab9d7cfee2905684 test: Check expected_stderr after stop (MarcoFalke) Pull request description: This fixes a bug where stderr wasn't checked for the shutdown sequence. Fix that by waiting for the shutdown to finish and then check stderr. ACKs for top commit: theStack: ACK faf902858d38150caa8991b0ab9d7cfee2905684 Tree-SHA512: a70cd1e6cda84d542782e41e8b59741dbcd472c0d0575bcef5cbfd1418473ce94efe921481d557bae3fbbdd78f1c49c09c48872883c052d87c5c9a9a51492692
2023-07-10init: don't start indexes sync thread prematurelyfurszy
By moving the 'StartIndexes()' call into the 'initload' thread, we can remove the threads active wait. Optimizing the available resources. The only difference with the current state is that now the indexes threads will only be started when they can process work and not before it.
2023-07-10test: Check expected_stderr after stopMarcoFalke
2023-07-07scripted-diff: rename 'loadblk' thread name to 'initload'furszy
The thread does not only load blocks, it loads the mempool and, in a future commit, will start the indexes as well. Also, renamed the 'ThreadImport' function to 'ImportBlocks' And the 'm_load_block' class member to 'm_thread_load'. -BEGIN VERIFY SCRIPT- sed -i "s/ThreadImport/ImportBlocks/g" $(git grep -l ThreadImport -- ':!/doc/') sed -i "s/loadblk/initload/g" $(git grep -l loadblk -- ':!/doc/release-notes/') sed -i "s/m_load_block/m_thread_load/g" $(git grep -l m_load_block) -END VERIFY SCRIPT-
2023-07-07Merge bitcoin/bitcoin#28038: wallet: address book migration bug fixesfanquake
7ecc29a0b7a23d8f5d3c1e6a0dad29b3ad839eb9 test: wallet, add coverage for addressbook migration (furszy) a277f8357ad8b0eb26f33fc36f919d868c06847b wallet: migration bugfix, persist empty labels (furszy) 1b64f6498c394a143df196172a14204fe3b8a744 wallet: migration bugfix, clone 'send' record label to all wallets (furszy) Pull request description: Addressing two specific bugs encountered during the wallet migration process, related to the address book, and improves the test coverage for it. Bug 1: Non-Cloning of External 'Send' Records The external 'send' records were not being correctly cloned to all wallets. Bug 2: Persistence of Empty Labels As address book entries without associated db label records can be treated as change (the `label` field inside the `CAddressBookData` class is optional, `nullopt` labels make `CAddressBookData ::IsChange()` return true), we must persist empty labels during the migration process. The user might have called `setlabel` with an "" string for an external address and that must be retained during migration. ACKs for top commit: achow101: ACK 7ecc29a0b7a23d8f5d3c1e6a0dad29b3ad839eb9 Tree-SHA512: b8a8483a4178a37c49af11eb7ba8a82ca95e54a6cd799e155e33f9fbe7f37b259e28372c77d6944d46b6765f9eaca6b8ca8d1cdd9d223120a3653e4e41d0b6b7
2023-07-07Merge bitcoin/bitcoin#28015: fuzz: Generate rpc fuzz targets individuallyfanquake
fa1e27fe8ec42764d0250c82a83d774c15798c4a fuzz: Generate rpc fuzz targets individually (MarcoFalke) Pull request description: The `rpc` fuzz target was added more than two years ago in e45863166f5e44cc2c380f4667812fcd3cddc73b. However, the bug https://github.com/bitcoin/bitcoin/issues/27913 was only found recently. Thus, it is pretty clear that fuzz engines can't deal with a search space that is too broad and can be extended in too many directions. Fix that by limiting the search space to each RPC method name and then iterate over all names, instead of letting the fuzz engine do the iteration. With this, the bug can be found in seconds, as opposed to years of CPU time (or never). ACKs for top commit: brunoerg: ACK fa1e27fe8ec42764d0250c82a83d774c15798c4a dergoegge: ACK fa1e27fe8ec42764d0250c82a83d774c15798c4a Tree-SHA512: 45ccba842367650d010320603153276b1b303deda9ba8c6bb31a4d2473b00aa5bca866db95f541485d65efd8276e2575026968c037872ef344fa33cf45bcdcd7
2023-07-06Merge bitcoin/bitcoin#27861: kernel: Rm ShutdownRequested and AbortNode from ↵Ryan Ofsky
validation code. 6eb33bd0c21b3e075fbab596351cacafdc947472 kernel: Add fatalError method to notifications (TheCharlatan) 7320db96f8d2aeff0bc5dc67d8b7b37f5f808990 kernel: Add flushError method to notifications (TheCharlatan) 3fa9094b92c5d37f486b0f8265062d3456796a50 scripted-diff: Rename FatalError to FatalErrorf (TheCharlatan) edb55e2777063dfeba0a52bbd0b92af8b4688501 kernel: Pass interrupt reference to chainman (TheCharlatan) e2d680a32d757de0ef8eb836047a0daa1d82e3c4 util: Add SignalInterrupt class and use in shutdown.cpp (TheCharlatan) Pull request description: Get rid of all `ShutdownRequested` calls in validation code by introducing an interrupt object that applications can use to cancel long-running kernel operations. Replace all `AbortNode` calls in validation code with new fatal error and flush error notifications so kernel applications can be notified about failures and choose how to handle them. --- This pull request is part of the `libbitcoinkernel` project https://github.com/bitcoin/bitcoin/issues/27587 https://github.com/orgs/bitcoin/projects/3 and more specifically its "Step 2: Decouple most non-consensus code from libbitcoinkernel". The pull request mostly allows dropping the kernel dependency on shutdown.cpp. The only dependency left after this is a `StartShutdown` call which will be removed in followup PR https://github.com/bitcoin/bitcoin/pull/27711. This PR also drops the last reference to the `uiInterface` global in kernel code. The process of moving the `uiInterface` out of the kernel was started in https://github.com/bitcoin/bitcoin/pull/27636. This pull request contains a subset of patches originally proposed in #27711. It will be part of a series of changes required to make handling of interrupts (or in other words the current shutdown procedure) in the kernel library more transparent and less reliable on global mutable state. The set of patches contained here was originally proposed by @ryanofsky [here](https://github.com/bitcoin/bitcoin/pull/27711#issuecomment-1580779869). ACKs for top commit: achow101: light ACK 6eb33bd0c21b3e075fbab596351cacafdc947472 hebasto: ACK 6eb33bd0c21b3e075fbab596351cacafdc947472, I have reviewed the code and it looks OK. ryanofsky: Code review ACK 6eb33bd0c21b3e075fbab596351cacafdc947472. No changes since last review other than rebase. Tree-SHA512: 7d2d05fa4805428a09466d43c11ae32946cbb25aa5e741b1eec9cd142e4de4bb311e13ebf1bb125ae490c9d08274f2d56c93314e10f3d69e7fec7445e504987c
2023-07-06test: wallet, add coverage for addressbook migrationfurszy
2023-07-06Merge bitcoin/bitcoin#27869: wallet: Give deprecation warning when loading a ↵glozow
legacy wallet 8fbb6e99bfc85a1b9003cae402a7335843a86abd wallet: Give deprecation warning when loading a legacy wallet (Andrew Chow) Pull request description: Next step in legacy wallet deprecation. ACKs for top commit: S3RK: reACK 8fbb6e99bfc85a1b9003cae402a7335843a86abd jonatack: re-ACK 8fbb6e99bfc85a1b9003cae402a7335843a86abd Tree-SHA512: 902984b09452926cf199f06e5fb56e4985325cdd5e0dcc829992158488f42d5fbc33e9a30a29303feac24c8315193e8d31712022e2a0503abd6b67169a0027f4
2023-07-03test: refactor: deduplicate legacy ECDSA signing for tx inputsSebastian Falbesoner
There are several instances in functional tests and the framework (MiniWallet, feature_block.py, p2p_segwit.py) where we create a legacy ECDSA signature for a certain transaction's input by doing the following steps: 1) calculate the `LegacySignatureHash` with the desired sighash type 2) create the actual digital signature by calling `ECKey.sign_ecdsa` on the signature message hash calculated above 3) put the DER-encoded result as CScript data push into tx input's scriptSig Create a new helper `sign_input_legacy` which hides those details and takes only the necessary parameters (tx, input index, relevant scriptPubKey, private key, sighash type [SIGHASH_ALL by default]). For further convenience, the signature is prepended to already existing data-pushes in scriptSig, in order to avoid rehashing the transaction after calling the new signing function.
2023-06-30Merge bitcoin/bitcoin#24005: test: add python implementation of Elligator swiftfanquake
4f4d039a98370a91e3cd5977352a9a4b260aa06b test: add ellswift test vectors from BIP324 (stratospher) a31287718aebad847b232eff59adc16c166c99e8 test: Add ellswift unit tests (stratospher) 714fb2c02ab4bfdd8a5a4f420036ece217c8b474 test: Add python ellswift implementation to test framework (stratospher) Pull request description: Built on top of https://github.com/bitcoin/bitcoin/pull/26222. This PR introduces Elligator swift encoding and decoding in the functional test framework. It's used in #24748 for writing p2p encryption tests. ACKs for top commit: sipa: ACK 4f4d039a98370a91e3cd5977352a9a4b260aa06b theStack: ACK 4f4d039a98370a91e3cd5977352a9a4b260aa06b :crocodile: Tree-SHA512: 32bc8e88f715f2cd67dc04cd38db92680872072cb3775478e2c30da89aa2da2742992779ea14da2f1faca09228942cfbd86d6957402b24bf560244b389e03540
2023-06-30Merge bitcoin/bitcoin#28009: script, test: python typing and linter updatesfanquake
6c97757a480b6e71a0750330d69ff18ac7cc6127 script: appease spelling linter (Jon Atack) 1316119ce7ba3de4135bbf1e5ac28c9ea26f62e1 script: update ignored-words.txt (Jon Atack) 146c861da2e4236997bee3eed6110a5016a8b86b script: update linter dependencies (Jon Atack) 92408224a4cb2f454465d5ff8445c247f2c4318a test: fix PEP484 no implicit optional argument types errors (Jon Atack) f86a3014338de6a2204bbdda10795b75ef6654c0 script, test: add missing python type annotations (Jon Atack) Pull request description: With these updates, `./test/lint/lint-python.py` and `./test/lint/lint-spelling.py` should be green again for developers using relatively recent Python dependencies, in particular mypy 0.991 (released 11/2022) and later. Please see the commit messages for details. ACKs for top commit: fanquake: ACK 6c97757a480b6e71a0750330d69ff18ac7cc6127 Tree-SHA512: 8a46a4d36d5978affdcecf4f2ace20ca1b52d483e098304911a2169afe60ccb9b042fa90c04b762d94f3ce53d2cafe6f24476ae839867a770c7f31e7e7242d99
2023-06-29script: update ignored-words.txtJon Atack
2023-06-29test: fix PEP484 no implicit optional argument types errorsJon Atack
Fix warnings for these files when ./test/lint/lint-python.py is run using mypy 0.991 (released 11/2022) and later: $ test/lint/lint-python.py test/functional/test_framework/coverage.py:23: error: Incompatible default for argument "coverage_logfile" (default has type "None", argument has type "str") [assignment] test/functional/test_framework/coverage.py:23: note: PEP 484 prohibits implicit Optional. Accordingly, mypy has changed its default to no_implicit_optional=True test/functional/test_framework/util.py:318: error: Incompatible default for argument "timeout" (default has type "None", argument has type "int") [assignment] test/functional/test_framework/util.py:318: note: PEP 484 prohibits implicit Optional. Accordingly, mypy has changed its default to no_implicit_optional=True test/functional/test_framework/util.py:318: error: Incompatible default for argument "coveragedir" (default has type "None", argument has type "str") [assignment] test/functional/interface_rest.py:67: error: Incompatible default for argument "query_params" (default has type "None", argument has type "dict[str, Any]") [assignment] test/functional/interface_rest.py:67: note: PEP 484 prohibits implicit Optional. Accordingly, mypy has changed its default to no_implicit_optional=True Verified using https://github.com/hauntsaninja/no_implicit_optional For details, see: https://mypy-lang.blogspot.com/2022/11/mypy-0990-released.html
2023-06-29script, test: add missing python type annotationsJon Atack
Fix warnings for these files when ./test/lint/lint-python.py is run using mypy 0.991 (released 11/2022) and later: "By default the bodies of untyped functions are not checked, consider using --check-untyped-defs [annotation-unchecked]" For details, see: https://mypy-lang.blogspot.com/2022/11/mypy-0990-released.html
2023-06-29test: add ellswift test vectors from BIP324stratospher
The test vector input file is taken from: 1. https://github.com/bitcoin/bips/blob/master/bip-0324/xswiftec_inv_test_vectors.csv 2. https://github.com/bitcoin/bips/blob/master/bip-0324/ellswift_decode_test_vectors.csv Co-authored-by: theStack <sebastian.falbesoner@gmail.com>
2023-06-29test: Add ellswift unit testsstratospher
remove util also since unit tests there were removed in #27538 Co-authored-by: theStack <sebastian.falbesoner@gmail.com>
2023-06-29test: Add python ellswift implementation to test frameworkstratospher
Co-authored-by: Pieter Wuille <pieter.wuille@gmail.com>
2023-06-29test: Rename EncodeDecimal to serialization_fallbackMarcoFalke
The new name better explains that the function handles fallbacks, without listing all in the function name.
2023-06-29Merge bitcoin/bitcoin#27932: test: Fuzz on macOSfanquake
fae7c50d201726f605938c3511dd9119efeea5ec test: Run fuzz tests on macOS (MarcoFalke) Pull request description: Any reason not to? ACKs for top commit: jamesob: Github ACK https://github.com/bitcoin/bitcoin/pull/27932/commits/fae7c50d201726f605938c3511dd9119efeea5ec dergoegge: utACK fae7c50d201726f605938c3511dd9119efeea5ec Tree-SHA512: e45122d73fafb17cea312258314b826cb0745e08daadd28465f687ec02d4c127d2f8cbe20179a9fff5712038850c02c968abb4838fa088b7555e28709317d3a3
2023-06-29Merge bitcoin/bitcoin#27884: test: Use TestNode datadir_path or chain_path ↵fanquake
where possible aaaa3aefbdfca1c9243057eeefdc19940e60bf18 test: Use TestNode *_path properties where possible (MarcoFalke) dddd89962b26b5593860d016586ee8feb5aeea24 test: Allow pathlib.Path as RPC argument via authproxy (MarcoFalke) fa41614a0abc05cbfbf76d6af3a186ab8d79c3f2 scripted-diff: Use wallets_path and chain_path where possible (MarcoFalke) fa493fadfb0ac73b7c0ee308f6623213702ae6f4 test: Use wallet_dir lambda in wallet_multiwallet test where possible (MarcoFalke) Pull request description: It seems inconsistent, fragile and verbose to: * Call `get_datadir_path` to recreate the path that already exists as field in TestNode * Call `os.path.join` with the hardcoded chain name or `self.chain` to recreate the TestNode `chain_path` property * Sometimes even use the hardcoded node dir name (`"node0"`) Fix all issues by using the TestNode properties. ACKs for top commit: willcl-ark: re-ACK aaaa3aefbdfca1c9243057eeefdc19940e60bf18 theStack: Code-review ACK aaaa3aefbdfca1c9243057eeefdc19940e60bf18 🌊 Tree-SHA512: e4720278085beb8164e1fe6c1aa18f601558a9263494ce69a83764c1487007de63ebb51d1b1151862dc4d5b49ded6162a5c1553cd30ea1c28627d447db4d8e72
2023-06-28Merge bitcoin/bitcoin#26222: Introduce secp256k1 module with field and group ↵Andrew Chow
classes to test framework d4fb58ae8ae9772d025ead184ef8f2c0ea50df3e test: EC: optimize scalar multiplication of G by using lookup table (Sebastian Falbesoner) 1830dd8820fb90bac9aea32000e47d7eb1a99e1b test: add secp256k1 module with FE (field element) and GE (group element) classes (Pieter Wuille) Pull request description: This PR rewrites a portion of `test_framework/key.py`, in a compatible way, by introducing classes that encapsulate field element and group element logic, in an attempt to be more readable and reusable. To maximize readability, the group element logic does not use Jacobian coordinates. Instead, group elements just store (affine) X and Y coordinates directly. To compensate for the performance loss this causes, field elements are represented as fractions. This undoes most, but not all, of the performance loss, and there is a few % slowdown (as measured in `feature_taproot.py`, which heavily uses this). The upside is that the implementation for group laws (point doubling, addition, subtraction, ...) is very close to the mathematical description of elliptic curves, and this extends to potential future extensions (e.g. ElligatorSwift as needed by #27479). ACKs for top commit: achow101: ACK d4fb58ae8ae9772d025ead184ef8f2c0ea50df3e theStack: re-ACK d4fb58ae8ae9772d025ead184ef8f2c0ea50df3e stratospher: tested ACK d4fb58a. really liked how this PR makes the secp256k1 code in the tests more intuitive and easier to follow! Tree-SHA512: 9e0d65d7de0d4fb35ad19a1c19da7f41e5e1db33631df898c6d18ea227258a8ba80c893dab862b0fa9b0fb2efd0406ad4a72229ee26d7d8d733dee1d56947f18
2023-06-28fuzz: Generate rpc fuzz targets individuallyMarcoFalke
2023-06-28kernel: Add fatalError method to notificationsTheCharlatan
FatalError replaces what previously was the AbortNode function in shutdown.cpp. This commit is part of the libbitcoinkernel project and further removes the shutdown's and, more generally, the kernel library's dependency on interface_ui with a kernel notification method. By removing interface_ui from the kernel library, its dependency on boost is reduced to just boost::multi_index. At the same time it also takes a step towards de-globalising the interrupt infrastructure. Co-authored-by: Russell Yanofsky <russ@yanofsky.org> Co-authored-by: TheCharlatan <seb.kung@gmail.com>
2023-06-28scripted-diff: Rename FatalError to FatalErrorfTheCharlatan
This is done in preparation for the next commit where a new FatalError function is introduced. FatalErrorf follows common convention to append 'f' for functions accepting format arguments. -BEGIN VERIFY SCRIPT- sed -i 's/FatalError/FatalErrorf/g' $( git grep -l 'FatalError') -END VERIFY SCRIPT-
2023-06-27Merge bitcoin/bitcoin#27896: Remove the syscall sandboxAndrew Chow
32e2ffc39374f61bb2435da507f285459985df9e Remove the syscall sandbox (fanquake) Pull request description: After initially being merged in #20487, it's no-longer clear that an internal syscall sandboxing mechanism is something that Bitcoin Core should have/maintain, especially when compared to better maintained/supported alterantives, i.e [firejail](https://github.com/netblue30/firejail). There is more related discussion in #24771. Note that given where it's used, the sandbox also gets dragged into the kernel. If it's removed, this should not require any sort of deprecation, as this was only ever an opt-in, experimental feature. Closes #24771. ACKs for top commit: davidgumberg: crACK https://github.com/bitcoin/bitcoin/pull/27896/commits/32e2ffc39374f61bb2435da507f285459985df9e achow101: ACK 32e2ffc39374f61bb2435da507f285459985df9e dergoegge: ACK 32e2ffc39374f61bb2435da507f285459985df9e Tree-SHA512: 8cf71c5623bb642cb515531d4a2545d806e503b9d57bfc15a996597632b06103d60d985fd7f843a3c1da6528bc38d0298d6b8bcf0be6f851795a8040d71faf16
2023-06-27Merge bitcoin/bitcoin#27940: test: Add ↵fanquake
implicit-signed-integer-truncation:*/include/c++/ suppression fae55f989e2654582271af3ca635fd6c4948e3be test: Add implicit-signed-integer-truncation:*/include/c++/ suppression (MarcoFalke) Pull request description: Needed for aarch64. Steps to test on aarch64: ``` lscpu | grep Arch FILE_ENV="./ci/test/00_setup_env_native_fuzz.sh" ./ci/test_run_all.sh ``` ACKs for top commit: fanquake: ACK fae55f989e2654582271af3ca635fd6c4948e3be - reproduced the failure: Tree-SHA512: b5058873118d285cc5d678a572cf4b890f8d68a24e1ac0987490f1b4123469a2b4456b08474f372e6aa49bb0d69e16f2c8277208b1cde3222a317f000beb5056
2023-06-27test: EC: optimize scalar multiplication of G by using lookup tableSebastian Falbesoner
On my machine, this speeds up the functional test feature_taproot.py by a factor of >1.66x (runtime decrease from 1m16.587s to 45.334s). Co-authored-by: Pieter Wuille <pieter@wuille.net>
2023-06-27test: add secp256k1 module with FE (field element) and GE (group element) ↵Pieter Wuille
classes These are primarily designed for ease of understanding, not performance.
2023-06-23Merge bitcoin/bitcoin#27631: test: avoid sporadic MINIMALDATA failure in ↵Andrew Chow
feature_taproot.py (fixes #27595) 54877253c807dac7a3720b2c3d1d989c410259a7 test: avoid sporadic MINIMALDATA failure in feature_taproot.py (fixes #27595) (Sebastian Falbesoner) Pull request description: The functional test feature_taproot.py fails in some rare cases on the execution of the following `"branched_codesep"` spending script (can be reproduced via `$ ./test/functional/feature_taproot.py --randomseed 9048710178866422833` on master / 137a98c5a22e058ed7a7997a0a4dbd75301de51e): https://github.com/bitcoin/bitcoin/blob/9d85c03620bf660cfa7d13080f5c0b191579cbc3/test/functional/feature_taproot.py#L741 The problem occurs if the first data-push (having random content with a random length in the range [0, 510]) has a length of 1 and the single byte has value of [1...16] or [-1]; in this case, the data-push is not minimally encoded by test framework's CScript class (i.e. doesn't use the special op-codes OP_1...OP_16 or OP_1NEGATE) and the script interpreter throws an SCRIPT_ERR_MINIMALDATA error: ``` test_framework.authproxy.JSONRPCException: non-mandatory-script-verify-flag (Data push larger than necessary) (-26) ``` Background: the functional test framework's CScript class translates passed bytes/bytearrays always to data pushes using OP_PUSHx/OP_PUSHDATA{1,2,4} op-codes (see `CScript.__coerce_instance(...)`). E.g. the expression `CScript(bytes([1]))` yields `bytes([OP_PUSH1, 1])` instead of the minimal-encoded `bytes([OP_1])`. Fix this by adapting the random-size range to [2,...], i.e. never pass byte-arrays below length two to be pushed. Closes #27595. ACKs for top commit: instagibbs: ACK https://github.com/bitcoin/bitcoin/pull/27631/commits/54877253c807dac7a3720b2c3d1d989c410259a7 sipa: utACK 54877253c807dac7a3720b2c3d1d989c410259a7 achow101: ACK 54877253c807dac7a3720b2c3d1d989c410259a7 Tree-SHA512: 3ffad89b2c3985c20702242192e744c9b10188bff880efaf3c38424a00fa07bd4608d8c948678ff9cdbb4e1e5b06696c7f55407ee10bb05edbb3ee03aa599cdc
2023-06-23Merge bitcoin/bitcoin#27577: p2p: give seednodes time before falling back to ↵Andrew Chow
fixed seeds 30778124b82791abdc6e930373460ef1dd587cb2 net: Give seednodes time before falling back to fixed seeds (Martin Zumsande) Pull request description: `-seednode` is an alternative bootstrap mechanism - when choosing it, we make a `AddrFetch` connection to the specified peer, gather addresses from them, and then disconnect. Presumably, if users specify a seednode they prefer addresses from that node over fixed seeds. However, when disabling dns seeds and specifiying `-seednode`, `CConnman::ProcessAddrFetch()` immediately removes the entry from `m_addr_fetches` (before the seednode could give us addresses) - and once `m_addr_fetches` is empty, `ThreadOpenConnections` will add fixed seeds, resulting in a "race" between the fixed seeds and seednodes filling up AddrMan. This PR suggests to check for any provided `-seednode` arg instead of using the size of `m_addr_fetches`, thus delaying the querying of fixed seeds for 1 minute when specifying any seednode (as we already do for `addnode` peers). That way, we actually give the seednodes a chance for to provide us with addresses before falling back to fixed seeds. This can be tested with `bitcoind -debug=net -dnsseed=0 -seednode=(...)` on a node without `peers.dat` and observing the debug log. ACKs for top commit: ajtowns: utACK 30778124b82791abdc6e930373460ef1dd587cb2 achow101: ACK 30778124b82791abdc6e930373460ef1dd587cb2 dergoegge: Code review ACK 30778124b82791abdc6e930373460ef1dd587cb2 sr-gi: ACK [3077812](https://github.com/bitcoin/bitcoin/pull/27577/commits/30778124b82791abdc6e930373460ef1dd587cb2) with a tiny nit, feel free to ignore it Tree-SHA512: 96446eb34c0805f10ee158a00a3001a07029e795ac40ad5638228d426e30e9bb836c64ac05d145f2f9ab23ec5a528f3a416e3d52ecfdfb0b813bd4b1ebab3c01
2023-06-23wallet: Give deprecation warning when loading a legacy walletAndrew Chow
2023-06-23test: Add implicit-signed-integer-truncation:*/include/c++/ suppressionMarcoFalke
2023-06-22Merge bitcoin/bitcoin#27831: test: handle failed `assert_equal()` assertions ↵fanquake
in bcc callback functions 61f4b9b7ad6e992a9dbbbb091e9b7ba9abe529ac Manage exceptions in bcc callback functions (virtu) Pull request description: Address #27380 (and similar future issues) by handling failed `assert_equal()` assertions in bcc callback functions ### Problem Exceptions are not propagated in ctype callback functions used by bcc. This means an AssertionError exception raised by `assert_equal()` to signal a failed assertion is not getting caught and properly logged. Instead, the error is logged to stdout and execution of the callback stops. The current workaround to check whether all `assert_equal()` assertions in a callback succeeded is to increment a success counter after the assertions (which only gets incremented if none exception is raised and stops execution). Then, outside the callback, the success counter can be used to check whether a callback executed successfully. One issue with the described workaround is that when an exception occurs, there is no way of telling which of the `assert_equal()` statements caused the exception; moreover, there is no way of inspecting how the pieces of data that got compared in `assert_equal()` differed (often a crucial clue when debugging what went wrong). This problem is happening in #27380: Sporadically, in the `mempool:rejected` test, execution does not reach the end of the callback function and the success counter is not incremented. Thus, the test fails when comparing the counter to its expected value of one. Without knowing which of the asserts failed any why it failed, this issue is hard to debug. ### Solution Two fixes come to mind. The first involves having the callback function make event data accessible outside the callback and inspecting the event using `assert_equal()` outside the callback. This solution still requires a counter in the callback in order to tell whether a callback was actually executed or if instead the call to perf_buffer_poll() timed out. The second fix entails wrapping all relevant `assert_equal()` statements inside callback functions into try-catch blocks and manually logging AssertionErrors. While not as elegant in terms of design, this approach can be more pragmatic for more complex tests (e.g., ones involving multiple events, events of different types, or the order of events). The solution proposed here is to select the most pragmatic fix on a case-by-case basis: Tests in `interface_usdt_net.py`, `interface_usdt_mempool.py` and `interface_usdt_validation.py` have been refactored to use the first approach, while the second approach was chosen for `interface_usdt_utxocache.py` (partly to provide a reference for the second approach, but mainly because the utxocache tests are the most intricate tests, and refactoring them to use the first approach would negatively impact their readability). Lastly, `interface_usdt_coinselection.py` was kept unchanged because it does not use `assert_equal()` statements inside callback functions. ACKs for top commit: 0xB10C: Reviewed the changes since my last review. ACK 61f4b9b7ad6e992a9dbbbb091e9b7ba9abe529ac. I've tested that the combined log contains both exceptions by modifying `interface_usdt_utxocache.py`. willcl-ark: utACK 61f4b9b stickies-v: utACK 61f4b9b7a Tree-SHA512: 85cdaabf370d4f09a9eab6af9ce7c796cd9d08cb91f38f021f71adda34c5f643331022dd09cadb95be2185dad6016c95cbb8942e41e4fbd566a49bf431c5141a
2023-06-22test: Run fuzz tests on macOSMarcoFalke
Also, fix a few bugs: * Error: RPC command "enumeratesigners" not found in RPC_COMMANDS_SAFE_FOR_FUZZING or RPC_COMMANDS_NOT_SAFE_FOR_FUZZING. Please update test/fuzz/rpc.cpp. * in run_once: ...format(" ".join(result.args), ... TypeError: sequence item 2: expected str instance, PosixPath found
2023-06-22Merge bitcoin/bitcoin#27889: test: Kill `BOOST_ASSERT` and update the linterfanquake
28fff06afe98177c14a932abf95b380bb51c6653 test: Make linter to look for `BOOST_ASSERT` macros (Hennadii Stepanov) 47fe551e52d8b3f607d55ad20073c0436590e081 test: Kill `BOOST_ASSERT` (Hennadii Stepanov) Pull request description: One of the goals of https://github.com/bitcoin/bitcoin/pull/27783 was to get rid of the `BOOST_ASSERT` macros instead of including the `boost/assert.hpp` headers. See https://github.com/bitcoin/bitcoin/pull/27783#discussion_r1210612717. It turns out that a couple of those macros sneaked into the codebase in https://github.com/bitcoin/bitcoin/pull/27790. This PR makes the linter guard against new instances of the `BOOST_ASSERT` macros and replaces the current ones. ACKs for top commit: kevkevinpal: ACK [28fff06](https://github.com/bitcoin/bitcoin/pull/27889/commits/28fff06afe98177c14a932abf95b380bb51c6653) stickies-v: ACK 28fff06af TheCharlatan: ACK 28fff06afe98177c14a932abf95b380bb51c6653 Tree-SHA512: 371f613592cf677afe0196d18c83943c6c8f1e998f57b4ff3ee58bfeff8636e4dac1357840d8611b4f7b197def94df10fe1a8ca3282b00b7b4eff4624552dda8
2023-06-21net: Give seednodes time before falling back to fixed seedsMartin Zumsande
Before, we'd remove a seednode from the list right after connecting to it, leading to a race with loading the fixed seed and connecting to them.
2023-06-21Merge bitcoin/bitcoin#27733: test: refactor: introduce `generate_keypair` ↵fanquake
helper with WIF support 1a572ce7d6e2b8282c6ad457cf8ecd2cf5ab7fd6 test: refactor: introduce `generate_keypair` helper with WIF support (Sebastian Falbesoner) Pull request description: In functional tests it is a quite common scenario to generate fresh elliptic curve keypairs, which is currently a bit cumbersome as it involves multiple steps, e.g.: privkey = ECKey() privkey.generate() privkey_wif = bytes_to_wif(privkey.get_bytes()) pubkey = privkey.get_pubkey().get_bytes() Simplify this by providing a new `generate_keypair` helper function that returns the private key either as `ECKey` object or as WIF-string (depending on the boolean `wif` parameter) and the public key as byte-string; these formats are what we mostly need (currently we don't use `ECPubKey` objects from generated keypairs anywhere). With this, most of the affected code blocks following the pattern above can be replaced by one-liners, e.g.: privkey, pubkey = generate_keypair(wif=True) Note that after this commit, the only direct uses of `ECKey` remain in situations where we want to set the private key explicitly, e.g. in MiniWallet (test/functional/test_framework/wallet.py) or the test for the signet miner script (test/functional/tool_signet_miner.py). ACKs for top commit: instagibbs: ACK https://github.com/bitcoin/bitcoin/pull/27733/commits/1a572ce7d6e2b8282c6ad457cf8ecd2cf5ab7fd6 kevkevinpal: reACK [1a572ce](https://github.com/bitcoin/bitcoin/pull/27733/commits/1a572ce7d6e2b8282c6ad457cf8ecd2cf5ab7fd6) stratospher: ACK 1a572ce7. neat to have this since keypair generation is done in lots of places. Tree-SHA512: ceb695ba7b34dc9f65357b55be03e67609e7e13a178083d405284eff4d8d3c5cea4fb0b6632658604a533f38ebfefc33e0c375995cc21ebc7843442ad764287b
2023-06-21Merge bitcoin/bitcoin#27919: ci: Run fuzz target even if input folder is emptyfanquake
0000f552937ee787d25c8fd0af3278ea94889216 ci: Run fuzz target even if input folder is empty (MarcoFalke) Pull request description: This should catch trivial integer sanitizer bugs if the author and all reviewers forget to look for them. ACKs for top commit: brunoerg: reACK 0000f552937ee787d25c8fd0af3278ea94889216 dergoegge: reACK 0000f552937ee787d25c8fd0af3278ea94889216 Tree-SHA512: f139b9d56f0cf1aae339c2890721c77c88d1fea77b73d492c1386ec99b4f393c5b664029919ff4a22e4e8a2929f085699a148c6acc2cc3e40df8a72fd39ff474
2023-06-21test: Use TestNode *_path properties where possibleMarcoFalke
Seems odd to place the burden on test writers to hardcode the chain or datadir path for the nodes under test.
2023-06-21test: Allow pathlib.Path as RPC argument via authproxyMarcoFalke
Also, add datadir_path property to TestNode
2023-06-21scripted-diff: Use wallets_path and chain_path where possibleMarcoFalke
Instead of passing the datadir and chain name to os.path.join, just use the existing properties, which are the same. -BEGIN VERIFY SCRIPT- sed -i --regexp-extended 's|\.datadir, self\.chain, .wallets.|.wallets_path|g' $(git grep -l '\.datadir, self\.chain,') sed -i --regexp-extended 's|\.datadir, self\.chain,|.chain_path,|g' $(git grep -l '\.datadir, self\.chain,') -END VERIFY SCRIPT-
2023-06-21test: Use wallet_dir lambda in wallet_multiwallet test where possibleMarcoFalke
Seems odd to hardcode all parent directory names in the path for no good reason. Also, add wallet_path property to TestNode. Also, rework wallet_backup.py test for scripted-diff in the next commit.
2023-06-20Merge bitcoin/bitcoin#27890: refactor: Make m_count_with_* in ↵glozow
CTxMemPoolEntry int64_t, drop UBSAN supp fa76f0d0efccd1ea272a46060022eea3e998268e refactor: Make m_count_with_* in CTxMemPoolEntry int64_t, drop UBSAN supp (MarcoFalke) Pull request description: This is a refactor as long as no signed integer overflow appears. In normal operation and absent bugs, signed integer overflow should never happen in the touched code paths. The main benefit of this refactor is to drop the file-wide ubsan suppression `unsigned-integer-overflow:txmempool.cpp`. For now, this only changes the internal private representation and the publicly returned type remains `uint64_t`. ACKs for top commit: glozow: ACK fa76f0d0ef ryanofsky: Code review ACK fa76f0d0efccd1ea272a46060022eea3e998268e Tree-SHA512: a09e33a915d60c65d369d44ba1a45ce4a6a76e6dc2bea43216ba02b5eab0b74e214b2c7cc44360493f2c483d18d96e4636b7a75b23050976efc80e38de852c39
2023-06-20Merge bitcoin/bitcoin#26740: wallet: Migrate wallets that are not in a ↵Ryan Ofsky
wallet dir a1e653828bc59351b2a0dd5a70f519e6b61199bc test: Add test for migrating default wallet and plain file wallet (Andrew Chow) bdbe3fd76b4b9186503dc1926a2fa3f8178d00a5 wallet: Generated migrated wallet's path from walletdir and name (Andrew Chow) Pull request description: This PR fixes an assertion error that is hit during the setup of the new database during migration of a wallet that was not contained in a wallet dir. Also added a test for this case as well as one for migrating the default wallet. ACKs for top commit: ryanofsky: Code review ACK a1e653828bc59351b2a0dd5a70f519e6b61199bc furszy: ACK a1e65382 Tree-SHA512: 96b218c0de8567d8650ec96e1bf58b0f8ca4c4726f5efc6362453979b56b9d569baea0bb09befb3a5aed8d16d29bf75ed5cd8ffc432bbd4cbcad3ac5574bc479
2023-06-20Merge bitcoin/bitcoin#27632: Raise on invalid -debug and -loglevel config ↵Andrew Chow
options daa5a658c0e79172e4dea0758246f11281790d29 refactor: rename BCLog::BLOCKSTORE to BLOCKSTORAGE (Jon Atack) cf622b214bfe0a97e403f1e9dc54bf5bbfc59fc3 doc: release note re raising on invalid -debug/debugexclude/loglevel (Jon Atack) 6cb1c66041ee14dbedad3aeeb90190ea5dddf917 init: remove config option names from translated -loglevel strings (Jon Atack) 25478292726dd7208b22a8924c8f1fdeac5c33f5 test: -loglevel raises on invalid values (Jon Atack) a9c295888b82c86ef4629aa2d9061ea152b48f20 init: raise on invalid loglevel config option (Jon Atack) b0c3995393c592fa96306e077ed64e65d5400882 test: -debug and -debugexclude raise on invalid values (Jon Atack) 4c3c19d943a0a4cf191495f6ebe9b964835607a4 init: raise on invalid debug/debugexclude config options (Jon Atack) Pull request description: and rename BCLog::BLOCKSTORE to BLOCKSTORAGE so the enum is the same as its value like the other BCLog enums. Per discussion in bitcoin-core-dev IRC today from https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2023-05-11#921458. ACKs for top commit: achow101: ACK daa5a658c0e79172e4dea0758246f11281790d29 ryanofsky: Code review ACK daa5a658c0e79172e4dea0758246f11281790d29. Just translated string template cleanup since last review pinheadmz: re-ACK daa5a658c0e79172e4dea0758246f11281790d29 Tree-SHA512: 4c107a93d8e8ce4e2ee81d44aec672526ca354ec390b241221067f68204beac8b4ba7a65748bcfa124ff2245c4307fa9243ec4fe0b464d0fa69c787fb322c3cc
2023-06-20ci: Run fuzz target even if input folder is emptyMarcoFalke
2023-06-20Merge bitcoin/bitcoin#27622: Fee estimation: avoid serving stale fee estimateglozow
d2b39e09bc6a5982fc5cf4b538b7fdb0e3cae576 test: ensure old fee_estimate.dat not read on restart and flushed (ismaelsadeeq) cf219f29f3c5b41070eaab9a549a476f01990f3a tx fees, policy: read stale fee estimates with a regtest-only option (ismaelsadeeq) 3eb241a141defa564c94cb95c5bbaf4c5bd9682e tx fees, policy: do not read estimates of old fee_estimates.dat (ismaelsadeeq) 5b886f2b436eaa8c2b7de58dc4644dc6223040da tx fees, policy: periodically flush fee estimates to fee_estimates.dat (ismaelsadeeq) Pull request description: Fixes #27555 The issue arises when an old `fee_estimates.dat` file is sometimes read during initialization. Or after an unclean shutdown, the latest fee estimates are not flushed to `fee_estimates.dat`. If the fee estimates in the old file are old, they can cause transactions to become stuck in the mempool. This PR ensures that nodes do not use stale estimates from the old file during initialization. If `fee_estimates.dat` has not been updated for 60 hours or more, it is considered stale and will not be read during initialization. To avoid having old estimates, the `fee_estimates.dat` file will be flushed periodically every hour. As mentioned #27555 > "The immediate improvement would be to store fee estimates to disk once an hour or so to reduce the chance of having an old file. From there, this case could probably be detected, and refuse to serve estimates until we sync." In addition, I will follow-up PR to persist the `mempoolminfee` across restarts. ACKs for top commit: willcl-ark: ACK d2b39e09bc instagibbs: reACK https://github.com/bitcoin/bitcoin/pull/27622/commits/d2b39e09bc6a5982fc5cf4b538b7fdb0e3cae576 glozow: ACK d2b39e09bc6a5982fc5cf4b538b7fdb0e3cae576. One nit if you follow up. Tree-SHA512: 4f6e0c296995d0eea5cf80c6aefdd79b7295a6a0ba446f2166f32afc105fe4f831cfda1ad3abd13c5c752b4fbea982cf4b97eaeda2af1fd7184670d41edcfeec
2023-06-19test: refactor: introduce `generate_keypair` helper with WIF supportSebastian Falbesoner
In functional tests it is a quite common scenario to generate fresh elliptic curve keypairs, which is currently a bit cumbersome as it involves multiple steps, e.g.: privkey = ECKey() privkey.generate() privkey_wif = bytes_to_wif(privkey.get_bytes()) pubkey = privkey.get_pubkey().get_bytes() Simplify this by providing a new `generate_keypair` helper function that returns the private key either as `ECKey` object or as WIF-string (depending on the boolean `wif` parameter) and the public key as byte-string; these formats are what we mostly need (currently we don't use `ECPubKey` objects from generated keypairs anywhere). With this, most of the affected code blocks following the pattern above can be replaced by one-liners, e.g.: privkey, pubkey = generate_keypair(wif=True) Note that after this commit, the only direct uses of `ECKey` remain in situations where we want to set the private key explicitly, e.g. in MiniWallet (test/functional/test_framework/wallet.py) or the test for the signet miner script (test/functional/tool_signet_miner.py).
generated by cgit v1.2.3 (git 2.26.2) at 2024-11-12 21:20:53 +0000