| Age | Commit message (Collapse) | Author |
|---|
|
|
|
|
|
|
|
|
|
* Replace wait_until with assert_equal where possible
* Use send_and_ping helper where possible
|
|
Can be reviewed with --word-diff-regex=.
|
|
-BEGIN VERIFY SCRIPT-
ren() { sed -i "s/\<$1\>/$2/g" test/functional/p2p_blockfilters.py ; }
# Rename from "node" to "peer" to avoid confusion with self.nodes
ren node0 peer_0
ren node1 peer_1
# Remove the confusing "C" prefix
ren CFiltersClient FiltersClient
-END VERIFY SCRIPT-
|
|
fa2d21fec8e532aa0781211e198ee6098f89350e add missing atomic include (MarcoFalke)
Pull request description:
`std::atomic` is used in the file, so to avoid compile issues, add the missing include.
ACKs for top commit:
practicalswift:
cr ACK fa2d21fec8e532aa0781211e198ee6098f89350e
jamesob:
crACK https://github.com/bitcoin/bitcoin/pull/22309/commits/fa2d21fec8e532aa0781211e198ee6098f89350e
hebasto:
ACK fa2d21fec8e532aa0781211e198ee6098f89350e, I have reviewed the code and it looks OK, I agree it can be merged.
prayank23:
crACK https://github.com/bitcoin/bitcoin/pull/22309/commits/fa2d21fec8e532aa0781211e198ee6098f89350e
Tree-SHA512: 307b15abd62006be4457b2437fb65de517c296bf0417e8acd181904eb6056dba5655dd5bc43b834bf68a087d06637f5e99ba5a6bc8be3e12388cea470dc155d0
|
|
|
|
This reverts commit faa94961d6e38392ba068381726ed4e033367b03.
|
|
|
|
Notebook
168b6c317ca054c1287c36be532964e861f44266 add dummy file param to fix jupyter (Josiah Baker)
Pull request description:
this fixes argparse to use `parse_known_args`. previously, if an unknown argument was passed, argparse would fail with an `unrecognized arguments: %s` error.
## why
the documentation mentions being able to run `TestShell` in a REPL interpreter or a jupyter notebook. when i tried to run inside a jupyter notebook, i got the following error:
this was due to the notebook passing the filename of the notebook as an argument. this is a known problem with notebooks and argparse, documented here: https://stackoverflow.com/questions/48796169/how-to-fix-ipykernel-launcher-py-error-unrecognized-arguments-in-jupyter
## testing
to test, make sure you have jupyter notebooks installed. you can do this by running:
```
pip install notebook
```
or following instructions from [here](https://jupyterlab.readthedocs.io/en/stable/getting_started/installation.html).
once installed, start a notebook (`jupyter notebook`), launch a python3 kernel and run the following snippet:
```python
import sys
# make sure this is the path for your system
sys.path.insert(0, "/path/to/bitcoin/test/functional")
from test_framework.test_shell import TestShell
test = TestShell().setup(num_nodes=2, setup_clean_chain=True)
```
you should see the following output, without errors:
if you are unfamiliar with notebooks, here is a short guide on using them: https://jupyter.readthedocs.io/en/latest/running.html
ACKs for top commit:
MarcoFalke:
review ACK 168b6c317ca054c1287c36be532964e861f44266
jamesob:
crACK https://github.com/bitcoin/bitcoin/pull/22201/commits/168b6c317ca054c1287c36be532964e861f44266
practicalswift:
cr ACK 168b6c317ca054c1287c36be532964e861f44266
Tree-SHA512: 4fee1563bf64a1cf9009934182412446cde03badf2f19553b78ad2cb3ceb0e5e085a5db41ed440473494ac047f04641311ecbba3948761c6553d0ca4b54937b4
|
|
Since m_next_addr_send is on a Poisson distribution, increase the mocktime bump
to ensure we don't experience flakiness in the tests. Closes #22243.
|
|
|
|
|
|
Add two simple helper functions to `AddrReceiver` to support callers currently
using `GetAddrStore` [used in next commit].
|
|
The `on_addr` functionality of `AddrReceiver` tests logic specific to how the
addr messages are set up in the test bodies. To allow other callers to also use
`AddrReceiver`, only apply the assertion logic if the caller indicates
desirability by setting `test_addr_contents` to true when initializing the
class.
|
|
to wiki
fa09fd1a0986aba918c5740f4c041ff2c7a7bae7 doc: Final merge of release notes snippets (MarcoFalke)
Pull request description:
None of the remaining pulls tagged for 22.0 have snippets, so merge them and move them to the wiki.
Trivial to review with `--color-moved=dimmed-zebra`.
ACKs for top commit:
laanwj:
ACK fa09fd1a0986aba918c5740f4c041ff2c7a7bae7
jonatack:
ACK fa09fd1a0986aba918c5740f4c041ff2c7a7bae7
Tree-SHA512: d5d272a9fc2217a87988efa189000cae5330da22f0459dd5e4340a18aba3d67d8fe8661016cb777e2e15f5d137c1c2b3de7d576c942eaf4201dabdc4e9c783f4
|
|
testshell in jupyter was failing due to an extra arg.
this adds a dummy -f param, which allows TestShell to
be used in a command line or jupyter environment
|
|
|
|
and check tx vsize
d6d2ab984547be4a9f7ba859a2a4c9ac9bfbf206 test: MiniWallet: fix fee calculation for P2PK and check tx vsize (Sebastian Falbesoner)
ce024b1c0ef2dcd307023aaaab40373c8bf17db1 test: MiniWallet: force P2PK signature to have fixed size (71 bytes) (Sebastian Falbesoner)
Pull request description:
This PR is a follow-up to #21945. It aims to both fix the fee calculation for P2PK mode transactions and enable its vsize check. Currently, the latter assumes a fixed tx length, which is fine for anyone-can-spend txs but doesn't apply to P2PK output spends due to varying DER signature size; the vsize check is therefore disabled for P2PK mode on master branch.
Creating one million DER signatures with MiniWallet shows the following distribution of sizes (smart people with better math skills probably could deduce the ratios without trying, but hey):
| DER signature size [bytes] | #occurences (ratio) |
| ------------- | ------------- |
| 71 | 498893 (49.89%) |
| 70 | 497244 (49.72%) |
| 69 | 3837 (0.38%) |
| 68 | 22 (0.0022%) |
Note that even smaller signatures are possible (for smaller R and S values with leading zero bytes), it's just that the probability decreases exponentially. Instead of choosing a large vsize check range and hoping that smaller signatures are never created (potentially leading to flaky tests), the proposed solution is ~~to limit the signature size to the two most common sizes 71 and 70 (>99.6% probability) and then accordingly only check for two vsize values; the value to be used for fee calculation is a decimal right between the two possible sizes (167.5 vbytes) and for the vsize check it's rounded down/up integer values are used.~~ to simply grind the signature to a fixed size of 71 bytes (49.89% probability, i.e. on average each call to `sign_tx()`, on average two ECC signing operations are needed).
~~The idea of grinding signatures to a fixed size (similar to https://github.com/bitcoin/bitcoin/pull/13666 which grinds to low-R values) would be counter-productive, as the signature creation in the test suite is quite expensive and this would significantly slow down tests that calculate hundreds of signatures (like e.g. feature_csv_activation.py).~~
For more about transaction sizes on different input/output types, see the following interesting article: https://medium.com/coinmonks/on-bitcoin-transaction-sizes-97e31bc9d816
ACKs for top commit:
MarcoFalke:
Concept ACK d6d2ab984547be4a9f7ba859a2a4c9ac9bfbf206
Tree-SHA512: 011c70ee0e4adf9ba12902e4b6c411db9ae96bdd8bc810bf1d67713367998e28ea328394503371fc1f5087a819547ddaea56c073b28db893ae1c0031d7927f32
|
|
time spent waiting
b9e76f1bf08c52fcd402b2314e00db4ad247ebc8 rpc: Add test for -rpcwaittimeout (Christian Decker)
f76cb10d7dc9a7b0c55d28011161606399417664 rpc: Prefix rpcwaittimeout error with details on its nature (Christian Decker)
c490e17ef698a1695050f82ef6567b3b87a21861 doc: Add release notes for the `-rpcwaittimeout` cli parameter (Christian Decker)
a7fcc8eb59fe51473571661316214156fbdbdcae rpc: Add a `-rpcwaittimeout` parameter to limit time spent waiting (Christian Decker)
Pull request description:
Adds a new numeric `-rpcwaittimeout` that can be used to limit the
time we spend waiting on the RPC server to appear. This is used by
downstream projects to provide a bit of slack when `bitcoind`s RPC
interface is not available right away.
This makes the `-rpcwait` argument more useful, since we can now limit
how long we'll ultimately wait, before potentially giving up and reporting
an error to the caller. It was discussed in the context of the BTCPayServer
wanting to have c-lightning wait for the RPC interface to become available
but still have the option of giving up eventually ([4355]).
I checked with laanwj whether this is already possible ([comment]), and
whether this would be a welcome change. Initially I intended to repurpose
the (optional) argument to `-rpcwait`, however I decided against it since it
would potentially break existing configurations, using things like `rpcwait=1`,
or `rpcwait=true` (the former would have an unintended short timeout, when
old behavior was to wait indefinitely).
~Due to its simplicity I didn't implement a test for it yet, but if that's desired I
can provide one.~ Test was added during reviews.
[4355]: https://github.com/ElementsProject/lightning/issues/4355
[comment]: https://github.com/ElementsProject/lightning/issues/4355#issuecomment-768288261
ACKs for top commit:
laanwj:
Code review ACK b9e76f1bf08c52fcd402b2314e00db4ad247ebc8
promag:
ACK b9e76f1bf08c52fcd402b2314e00db4ad247ebc8.
Tree-SHA512: 3cd6728038ec7ca7c35c2e7ccb213bfbe963f99a49bb48bbc1e511c4dd23d9957c04f9af1f8ec57120e47b26eaf580b46817b099d5fc5083c98da7aa92db8638
|
|
|
|
|
|
With `banlist.dat` (being written in addrv1 format) if we would try to
write a Tor v3 subnet, it would serialize as a dummy-all-0s IPv6
address and subsequently, when deserialized will not result in the same
subnet.
This problem does not exist with `banlist.json` where the data is saved
in textual, human-readable form.
|
|
Save the banlist in `banlist.json` instead of `banlist.dat`.
This makes it possible to store Tor v3 entries in the banlist on disk
(and any other addresses that cannot be serialized in addrv1 format).
Only read `banlist.dat` if it exists and `banlist.json` does not
exist (first start after an upgrade).
Supersedes https://github.com/bitcoin/bitcoin/pull/20904
Resolves https://github.com/bitcoin/bitcoin/issues/19748
|
|
|
|
-BEGIN VERIFY SCRIPT-
sed -i 's/\<FromHex\>/from_hex/g' $(git grep -l FromHex)
-END VERIFY SCRIPT-
Co-authored-by: MarcoFalke <falke.marco@gmail.com>
|
|
|
|
`FromHex` is mostly used for transactions, so we introduce a
shortcut `tx_from_hex` for `FromHex(CTransaction, hex_str)`.
|
|
4e353cb618745cdb5d98e58e7dcd400ded01299a http: Release work queue after event base finish (João Barbosa)
Pull request description:
This fixes a race between `http_request_cb` and `StopHTTPServer` where
the work queue is used after release.
Fixes #18856.
ACKs for top commit:
fjahr:
Code review ACK 4e353cb618745cdb5d98e58e7dcd400ded01299a
achow101:
ACK 4e353cb618745cdb5d98e58e7dcd400ded01299a
LarryRuane:
ACK 4e353cb618745cdb5d98e58e7dcd400ded01299a
hebasto:
ACK 4e353cb618745cdb5d98e58e7dcd400ded01299a, tested (rebased on top of master 9313c4e6aa4b707c06a86b33d5d2753cd8383340) on Linux Mint 20.1 (x86_64) using MarcoFalke's [patch](https://github.com/bitcoin/bitcoin/pull/19033#issuecomment-640106647), including different `-rpcthreads`/`-rpcworkqueue` cases. The bug is fixed. The code is correct.
Tree-SHA512: 185d2a9744d0d5134d782bf321ac9958ba17b11a5b3d70b4897c8243e6b146dfd3f23c57aef8e10ae9484374120b64389c1949a9cf0a21dccc47ffc934c20930
|
|
30aee2dfe671b347438c1c327c6f79edfacff1ce tests: Add test for compact block HB selection (Pieter Wuille)
6efbcec4ded6116a42d2783c96c60ef0f255a1b2 Protect last outbound HB compact block peer (Suhas Daftuar)
Pull request description:
If all our high-bandwidth compact block serving peers (BIP 152) stall block
download, then we can be denied a block for (potentially) a long time. As
inbound connections are much more likely to be adversarial than outbound
connections, mitigate this risk by never removing our last outbound HB peer if
it would be replaced by an inbound.
ACKs for top commit:
achow101:
ACK 30aee2dfe671b347438c1c327c6f79edfacff1ce
ariard:
Code ACK 30aee2dfe
jonatack:
ACK 30aee2dfe671b347438c1c327c6f79edfacff1ce
Tree-SHA512: 5c6c9326e3667b97e0864c371ae2174d2be9054dad479f4366127b9cd3ac60ffa01ec9707b16ef29cac122db6916cf56fd9985733390017134ace483278921d5
|
|
symbol-check
e8cd3700eeb27437f5ea435869c9d61214285fdd devtools: Integrate ARCH_MIN_GLIBC_VER table into MAX_VERSIONS in symbol-check.py (W. J. van der Laan)
a33381acf5ae2b43616fffaf26b1c8962e8ef0bb devtools: Add xkb version to symbol-check (W. J. van der Laan)
19e598bab0a1cb5ad93321eb9fa25d1a58d5e276 devtools: Fix verneed section parsing in pixie (W. J. van der Laan)
Pull request description:
I misunderstood the ELF specification for version symbols (verneed): The `vn_aux` pointer is relative to the main verneed record, not the start of the section.
This caused many symbols to not be versioned properly in the return value of `elf.dyn_symbols`. This was discovered in #21454.
Fix it by correcting the offset computation.
- xkb versions symbols (using the prefix `V`), as this library is used by bitcoin-qt, add it to the valid versions in `symbol-check.py`
This unfortunately brings to light some symbols that have been introduced since and weren't caught (from a gitian compile of master):
```
bitcoin-cli: symbol getrandom from unsupported version GLIBC_2.25
bitcoin-cli: failed IMPORTED_SYMBOLS
bitcoind: symbol getrandom from unsupported version GLIBC_2.25
bitcoind: symbol log from unsupported version GLIBC_2.29
bitcoind: symbol fcntl64 from unsupported version GLIBC_2.28
bitcoind: symbol pow from unsupported version GLIBC_2.29
bitcoind: symbol exp from unsupported version GLIBC_2.29
bitcoind: failed IMPORTED_SYMBOLS
bitcoin-qt: symbol exp from unsupported version GLIBC_2.29
bitcoin-qt: symbol fcntl64 from unsupported version GLIBC_2.28
bitcoin-qt: symbol log from unsupported version GLIBC_2.29
bitcoin-qt: symbol pow from unsupported version GLIBC_2.29
bitcoin-qt: symbol statx from unsupported version GLIBC_2.28
bitcoin-qt: symbol getrandom from unsupported version GLIBC_2.25
bitcoin-qt: symbol renameat2 from unsupported version GLIBC_2.28
bitcoin-qt: symbol getentropy from unsupported version GLIBC_2.25
bitcoin-qt: failed IMPORTED_SYMBOLS
bitcoin-wallet: symbol exp from unsupported version GLIBC_2.29
bitcoin-wallet: symbol log from unsupported version GLIBC_2.29
bitcoin-wallet: symbol fcntl64 from unsupported version GLIBC_2.28
bitcoin-wallet: failed IMPORTED_SYMBOLS
test_bitcoin: symbol getrandom from unsupported version GLIBC_2.25
test_bitcoin: symbol log from unsupported version GLIBC_2.29
test_bitcoin: symbol fcntl64 from unsupported version GLIBC_2.28
test_bitcoin: symbol pow from unsupported version GLIBC_2.29
test_bitcoin: symbol exp from unsupported version GLIBC_2.29
test_bitcoin: failed IMPORTED_SYMBOLS
```
ACKs for top commit:
hebasto:
ACK e8cd3700eeb27437f5ea435869c9d61214285fdd
Tree-SHA512: 8c15e3478eb642f01a1ddaadef03f80583f088f9fa8e3bf171ce16b0ec05ffb4675ec147d7ffc6a4360637ed47fca517c6ca2bac7bb30d794c03783cfb964b79
|
|
base_encode_decode
906d7913117c8f10934b37afa27ae8ac565da042 fuzz: add missing ECCVerifyHandle to base_encode_decode (Andrew Poelstra)
Pull request description:
It is possible to trigger a fuzztest failure in the `base_encode_decode` by asking it to decode any PSBT that has HD keypaths in it. For example, this one
```
cHNidP8BAFUCAAAAASeaIyOl37UfxF8iD6WLD8E+HjNCeSqF1+Ns1jM7XLw5AAAAAAD/////AaBa6gsAAAAAGXapFP/pwAYQl8w7Y28ssEYPpPxCfStFiKwAAAAAAAEBIJVe6gsAAAAAF6kUY0UgD2jRieGtwN8cTRbqjxTA2+uHIgIDsTQcy6doO2r08SOM1ul+cWfVafrEfx5I1HVBhENVvUZGMEMCIAQktY7/qqaU4VWepck7v9SokGQiQFXN8HC2dxRpRC0HAh9cjrD+plFtYLisszrWTt5g6Hhb+zqpS5m9+GFR25qaAQEEIgAgdx/RitRZZm3Unz1WTj28QvTIR3TjYK2haBao7UiNVoEBBUdSIQOxNBzLp2g7avTxI4zW6X5xZ9Vp+sR/HkjUdUGEQ1W9RiED3lXR4drIBeP4pYwfv5uUwC89uq/hJ/78pJlfJvggg71SriIGA7E0HMunaDtq9PEjjNbpfnFn1Wn6xH8eSNR1QYRDVb1GELSmumcAAACAAAAAgAQAAIAiBgPeVdHh2sgF4/iljB+/m5TALz26r+En/vykmV8m+CCDvRC0prpnAAAAgAAAAIAFAACAAAA=
```
which I took straight from the PSBT test vectors. The reason is that in src/psbt.h we call `DeserializeHDKeypaths`, which in turn calls `CPubKey::IsFullyValid`, which in turn asserts that a secp context has been created.
The error appears to be masked on many systems by the definition of `instance_of_eccryptoclosure` in src/script/bitcoinconsensus.cpp, which defines a static object which contains an `ECCVerifyHandle`. If you just comment out that line you can reliably trigger the fuzz test failure, e.g. by creating a file `crash` with the above PSBT, and runnnig
```
ASAN_OPTIONS=symbolize=0:detect_stack_use_after_return=1:check_initialization_order=1:strict_init_order=1 UBSAN_OPTIONS=suppressions=./test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1 FUZZ=base_encode_decode ./src/test/fuzz/fuzz -seed_inputs=crash
```
ACKs for top commit:
practicalswift:
cr ACK 906d7913117c8f10934b37afa27ae8ac565da042
Tree-SHA512: b98b60573c21efe28503fe351883c6f0d9ac99d0dd6f100537b16ac53476617b8a3f899faf0c23d893d34a01b3bbe4a784499ec6f9c7000292e850bed449bd85
|
|
test_no_inherited_signaling RBF test
fa7d71f270b89c9d06230d4ff262646f9ea29f4a test: Run pep-8 on touched test (MarcoFalke)
fab7e99c2a4b02a41b7448b45f0e6cdfdbb53ac3 test: Use MiniWallet in test_no_inherited_signaling RBF test (MarcoFalke)
fab871f649e3da4a5a5f6cffac3fc748bb1ca900 test: Remove unused generate() from test (MarcoFalke)
faff3f35b778d9af3d649b303d7edab49bfe40b4 test: Add txin.sequence option to MiniWallet (MarcoFalke)
Pull request description:
This comes with nice benefits:
* Less code and complexity
* Test can be run without wallet compiled in
Also add some additional checks for `getmempoolentry` (#22209) and other cleanups :art:
ACKs for top commit:
mjdietzx:
Tested ACK fa7d71f270b89c9d06230d4ff262646f9ea29f4a thanks for the explanations, nicely done
theStack:
ACK fa7d71f270b89c9d06230d4ff262646f9ea29f4a 🍷
Tree-SHA512: 0e9b8fe985779d8d7034d256deed627125bb374b6ae2972c461b3a220739a51061c6147ad69339bee16282f82716c7f3f8a7a89c693ceb1e47ea50709272332a
|
|
|
|
|
|
|
|
|
|
|
|
No need for this to be a part of the header anymore.
Includes a small reference type style change.
|
|
|
|
Also fix incorrect {}
|
|
|
|
|
|
|
|
|
|
b3c712cb2871e0034898b37454fb6a4e1fc78f84 contrib/signet/miner: remove debug code (Anthony Towns)
297e35159f99d4caa995de29d419aefa4b87630c bitcoin-util: use AddCommand / GetCommand (Anthony Towns)
b6d493fd4d602a67d16d2566466e18091e272fa0 contrib/signet/README.md: Update miner description (Anthony Towns)
e66543827cd4ae9b76ff4c5c6e414c1942c7d727 contrib/signet/miner: Automatic timestamp for first block (Anthony Towns)
a383ce5b4add6cdf4e9974527b609a1147a0d972 contrib/signet/miner: --grind-cmd is required for calibrate (Anthony Towns)
1a45cd2e51214f81442a5f9bf16612100adaf5b8 contrib/signet: Fix typos (Anthony Towns)
Pull request description:
Followups from #19937
ACKs for top commit:
laanwj:
Code review ACK b3c712cb2871e0034898b37454fb6a4e1fc78f84
Tree-SHA512: a1003f9ee3697438114b60872b50f4300c8b52f0d58551566eb61c421d787525807ae75be205dcab2c24358cd568f53260120880109a9d728773405ff987596f
|
|
55311197c483477b79883da5da09f2bc71acc7cf Added new test for future blocks reacceptance (sanket1729)
511a5af4622915c236cfb11df5234232c2983e45 Fixed inconsistencies between code and comments (sanket1729)
Pull request description:
This Commit does 3 things:
1) Adds a test case for checking reacceptance a previously rejected block which
was too far in the future.
~~2) clean up uses of rehash or calc_sha256 where it was not needed~~
3) While constructing block 44, this commit makes the code consistent with the expected figure in
the comment just above it by adding a transaction to the block.
4) Fix comment describing `sign_tx()` function
ACKs for top commit:
duncandean:
reACK 5531119
brunoerg:
reACK 55311197c483477b79883da5da09f2bc71acc7cf
Tree-SHA512: d40c72fcdbb0b2a0715adc58441eeea08147ee2ec5e371a4ccc824ebfdc6450698bd40aaeecb7ea7bfdb3cd1b264dd821b890276fff8b8d89b7225cdd9d6b546
|
|
processes when using `--failfast`
451b96f7d2796d00eabaec56d831f9e9b1a569cc test: kill process group to avoid dangling processes (S3RK)
Pull request description:
This is an alternative to #19281
This PR fixes a problem when after test failure with `--failfast` option there could be dangling nodes. The nodes will continue to occupy rpc/p2p ports on the machine and will cause further test failures.
If there are any dangling nodes left at the end of the test run we kill the whole process group.
Pros: the operations is immediate and won't lead to CI timeout
Cons: the test_runner process is also killed and exit code is 137
Example output:
```
...
Early exiting after test failure
TEST | STATUS | DURATION
rpc_decodescript.py | ✓ Passed | 2 s
rpc_deprecated.py | ✓ Passed | 2 s
rpc_deriveaddresses.py | ✓ Passed | 2 s
rpc_dumptxoutset.py | ✖ Failed | 2 s
ALL | ✖ Failed | 8 s (accumulated)
Runtime: 4 s
Killed: 9
> echo $?
137
```
ACKs for top commit:
MarcoFalke:
review ACK 451b96f7d2796d00eabaec56d831f9e9b1a569cc
aitorjs:
ACK 451b96f7d2796d00eabaec56d831f9e9b1a569cc. Manual testing with and without **--failfast**.
Tree-SHA512: 87e510a1411b9e7571e63cf7ffc8b9a8935daf9112ffc0f069d6c406ba87743ec439808181f7e13cb97bb200fad528589786c47f0b43cf3a2ef0d06a23cb86dd
|
