fae7c50d201726f605938c3511dd9119efeea5ec test: Run fuzz tests on macOS (MarcoFalke)
Pull request description:
Any reason not to?
ACKs for top commit:
jamesob:
Github ACK fae7c50d20
dergoegge:
utACK fae7c50d201726f605938c3511dd9119efeea5ec
Tree-SHA512: e45122d73fafb17cea312258314b826cb0745e08daadd28465f687ec02d4c127d2f8cbe20179a9fff5712038850c02c968abb4838fa088b7555e28709317d3a3
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
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
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>
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-
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 32e2ffc393
achow101:
ACK 32e2ffc39374f61bb2435da507f285459985df9e
dergoegge:
ACK 32e2ffc39374f61bb2435da507f285459985df9e
Tree-SHA512: 8cf71c5623bb642cb515531d4a2545d806e503b9d57bfc15a996597632b06103d60d985fd7f843a3c1da6528bc38d0298d6b8bcf0be6f851795a8040d71faf16
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>
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):
9d85c03620/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 54877253c8
sipa:
utACK 54877253c807dac7a3720b2c3d1d989c410259a7
achow101:
ACK 54877253c807dac7a3720b2c3d1d989c410259a7
Tree-SHA512: 3ffad89b2c3985c20702242192e744c9b10188bff880efaf3c38424a00fa07bd4608d8c948678ff9cdbb4e1e5b06696c7f55407ee10bb05edbb3ee03aa599cdc
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](30778124b8) with a tiny nit, feel free to ignore it
Tree-SHA512: 96446eb34c0805f10ee158a00a3001a07029e795ac40ad5638228d426e30e9bb836c64ac05d145f2f9ab23ec5a528f3a416e3d52ecfdfb0b813bd4b1ebab3c01
Even though we expect these functions to only produce one event,
we still keep a counter to check if that's true. By simply storing
all the events, we can remove the counters and make debugging
easier, by allowing pdb to access the events.
By storing the events instead of doing the comparison inside the
handle_utxocache_* functions, we simplify the overall logic and
potentially making debugging easier, by allowing pdb to access the
events.
Mostly a refactor, but changes logging behaviour slightly by not
raising and not calling self.log.exception("Assertion failed")
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
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
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](28fff06afe)
stickies-v:
ACK 28fff06af
TheCharlatan:
ACK 28fff06afe98177c14a932abf95b380bb51c6653
Tree-SHA512: 371f613592cf677afe0196d18c83943c6c8f1e998f57b4ff3ee58bfeff8636e4dac1357840d8611b4f7b197def94df10fe1a8ca3282b00b7b4eff4624552dda8
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 1a572ce7d6
kevkevinpal:
reACK [1a572ce](1a572ce7d6)
stratospher:
ACK 1a572ce7. neat to have this since keypair generation is done in lots of places.
Tree-SHA512: ceb695ba7b34dc9f65357b55be03e67609e7e13a178083d405284eff4d8d3c5cea4fb0b6632658604a533f38ebfefc33e0c375995cc21ebc7843442ad764287b
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
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-
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.
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
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
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
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 d2b39e09bc
glozow:
ACK d2b39e09bc6a5982fc5cf4b538b7fdb0e3cae576. One nit if you follow up.
Tree-SHA512: 4f6e0c296995d0eea5cf80c6aefdd79b7295a6a0ba446f2166f32afc105fe4f831cfda1ad3abd13c5c752b4fbea982cf4b97eaeda2af1fd7184670d41edcfeec
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).
Exceptions are not propagated in ctype callback functions used by bcc.
This means an AssertionError exception raised by check_equal() to signal
a failed assertion is not getting caught and properly logged. Instead,
the error is logged to stdout and execution of the handler stops.
The current workaround to check whether all check_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 check_equal() statements
caused the exception; moreover, there is no way of inspecting how the
pieces of data that got compared in check_equal() differed (often
a crucial clue when debugging what went wrong).
Two fixes to this problem come to mind. The first involves having the
callback function make event data accessible outside the callback and
inspecting the event using check_equal() outside the callback. This
solution still requires a counter in the callback 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 check_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 check_equal() statements inside callback functions.
14405e8d4d259c18a21fc006d0a27550be3171f8 doc: test: update TestShell instructions (ismaelsadeeq)
Pull request description:
Fixes #27904
From #27904 and IRC.
Update [Testshell instructions ](https://github.com/bitcoin/bitcoin/blob/master/test/functional/test-shell.md#2-importing-testshell-from-the-bitcoin-core-repository)
E.g `TestShell.setup()` throws
```
AttributeError: type object 'TestShell' has no attribute 'setup'
```
Parentheses are missing, it should be `TestShell().setup()`
ACKs for top commit:
Sjors:
utACK 14405e8d4d259c18a21fc006d0a27550be3171f8
brunoerg:
crACK 14405e8d4d259c18a21fc006d0a27550be3171f8
hernanmarino:
utACK 14405e8d4d259c18a21fc006d0a27550be3171f8
Tree-SHA512: ffe5fa1103a3b00ef0ee99879adae967b0da07cb8f8451c4c261b0a70b3b666af7aeaacd6f46f85a84ee5e9c7c7ed49700209b5b1f124d7a76efc420ad5c9cd9
5524fa00faebfe040f126a4152640f9e9ed572b1 doc: add release note about removal of `deprecatedrpc=walletwarningfield` flag (Sebastian Falbesoner)
5c77db73542fe4c76fd53526ae560d56dde5f830 Restorewallet/createwallet help documentation fixups/improvements (Jon Atack)
a00ae31fccba63d5fd409ffb39c1622df2ea3e8c rpc: remove deprecated "warning" field from {create,load,restore,unload}wallet (Sebastian Falbesoner)
Pull request description:
The "warning" string field for wallet creating/loading RPCs (`createwallet`, `loadwallet`, `unloadwallet` and `restorewallet`) has been deprecated with the configuration option `-deprecatedrpc=walletwarningfield` in PR #27279 (released in v25.0). For the next release v26.0, the field and the configuration option can be removed.
ACKs for top commit:
achow101:
ACK 5524fa00faebfe040f126a4152640f9e9ed572b1
jonatack:
ACK 5524fa00faebfe040f126a4152640f9e9ed572b1
Tree-SHA512: 8212f72067d08095304018b8a95d2ebef630004b65123483fbbfb078cc5709c2d825bbc35b16ea5f6b28ae7377347382d7e9afaf7bdbf0575d2c229d970784de
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.
Note that given where it's used, the sandbox also gets dragged into the
kernel.
There is some related discussion in #24771.
This should not require any sort of deprecation, as this was only ever
an opt-in, experimental feature.
Closes#24771.
a97c59f12d50d11d8859f4bbfb9fcf66de667ca0 test: p2p: check misbehavior for non-continuous headers messages (Sebastian Falbesoner)
Pull request description:
This PR adds missing test coverage for a peer sending a `headers` message where the headers don't connect to each other, which should be treated as misbehaving (not disconnecting though, as the score increase is only 20). The relevant code path is `PeerManagerImpl::ProcessHeadersMessage` -> `PeerManagerImpl::CheckHeadersPoW` -> `PeerManagerImpl::CheckHeadersAreContinuous`:
17acb2782a/src/net_processing.cpp (L2415-L2419)17acb2782a/src/net_processing.cpp (L2474-L2484)
ACKs for top commit:
sr-gi:
ACK a97c59f12d
achow101:
ACK a97c59f12d50d11d8859f4bbfb9fcf66de667ca0
instagibbs:
ACK a97c59f12d50d11d8859f4bbfb9fcf66de667ca0
Tree-SHA512: 3f8d6a2492e5c8b63c7b11be2e4ec455f83581b2c58f2d4e705baadfe8d7c6377296d6cd0eda679d291a13d8930b09443f8e3d183795df34b780c703d5d3aeb3
