Commit Graph

40419 Commits

Author SHA1 Message Date
Ryan Ofsky ef174e9ed2 test: assumeutxo snapshot block CheckBlockIndex crash test
Add a test for a CheckBlockIndex crash that would happen before previous
"assumeutxo: Get rid of faked nTx and nChainTx values" commit.

The crash was an assert failure in the (pindex->nChainTx == pindex->nTx +
prev_chain_tx) check that would previously happen if the snapshot block was
submitted after loading the snapshot and downloading a few blocks after the
snapshot. In that case ReceivedBlockTransactions() previously would overwrite
the nChainTx value of the submitted snapshot block with a fake value based on
the previous block, so the (pindex->nChainTx == pindex->nTx + prev_chain_tx)
check would later fail on the first block after the snapshot. This test was
originally posted by Martin Zumsande <mzumsande@gmail.com> in
https://github.com/bitcoin/bitcoin/pull/29370#issuecomment-1974096225

Co-authored-by: Martin Zumsande <mzumsande@gmail.com>
2024-03-18 11:28:40 -05:00
Ryan Ofsky 0391458d76 test: assumeutxo stale block CheckBlockIndex crash test
Add a test for a CheckBlockIndex crash that would happen before previous
"assumeutxo: Get rid of faked nTx and nChainTx values" commit.

The crash was an assert failure in the (pindex->nChainTx == pindex->nTx +
prev_chain_tx) check that would previously happen if a snapshot was loaded, and
a block was submitted which forked from the chain before the snapshot block and
after the last downloaded background chain block. This block would not be
marked assumed-valid because it would not be an ancestor of the snapshot, and
it would have nTx set, nChainTx unset, and prev->nChainTx set with a fake
value, so the assert would fail. After the fix, prev->nChainTx is unset instead
of being set to a fake value, so the assert succeeds. This test was originally
posted by maflcko in
https://github.com/bitcoin/bitcoin/issues/29261#issuecomment-1918947945

Co-authored-by: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
2024-03-18 11:28:40 -05:00
Ryan Ofsky ef29c8b662 assumeutxo: Get rid of faked nTx and nChainTx values
The `PopulateAndValidateSnapshot` function introduced in
f6e2da5fb7 from #19806 has been setting fake
`nTx` and `nChainTx` values that can show up in RPC results (see #29328) and
make `CBlockIndex` state hard to reason about, because it is difficult to know
whether the values are real or fake.

Revert to previous behavior of setting `nTx` and `nChainTx` to 0 when the
values are unknown, instead of faking them.

This commit fixes at least two assert failures in the (pindex->nChainTx ==
pindex->nTx + prev_chain_tx) check that would happen previously. Tests for
these failures are added separately in the next two commits.

Compatibility note: This change could result in -checkblockindex failures if a
snapshot was loaded by a previous version of Bitcoin Core and not fully
validated, because fake nTx values will have been saved to the block index. It
would be pretty easy to avoid these failures by adding some compatibility code
to `LoadBlockIndex` and changing `nTx` values from 1 to 0 when they are fake
(when `(pindex->nStatus & BLOCK_VALID_MASK) < BLOCK_VALID_TRANSACTIONS`), but a
little simpler not to worry about being compatible in this case.
2024-03-18 11:28:40 -05:00
Ryan Ofsky 9b97d5bbf9 doc: Improve comments describing setBlockIndexCandidates checks
The checks are changing slightly in the next commit, so try to explains the
ones that exist to avoid confusion
(https://github.com/bitcoin/bitcoin/pull/29370#discussion_r1499519079)
2024-03-18 11:28:40 -05:00
Ryan Ofsky 0fd915ee6b validation: Check GuessVerificationProgress is not called with disconnected block
Use Assume macro as suggested https://github.com/bitcoin/bitcoin/pull/29370#discussion_r1479427801
2024-03-18 11:28:40 -05:00
Ryan Ofsky 63e8fc912c ci: add getchaintxstats ubsan suppressions
Add ubsan suppressions for integer overflows in the getchaintxstats RPC.

getchainstatstx line "int nTxDiff = pindex->nChainTx - past_block.nChainTx" can
trigger ubsan integer overflows when assumeutxo snapshots are loaded, from
subtracting unsigned values and assigning the result to a signed int.

The overflow behavior probably exists in current code but is hard to trigger
because it would require calling getchainstatstx at the right time with
specific parameters as background blocks are being downloaded. But the overflow
behavior becomes easier to trigger in the upcoming commit removing fake
nChainTx values, so a suppression needs to be added before then for CI to pass.

getchainstatstx should probably be improved separately in another PR to not
need this suppression, and handle edge cases and missing nChainTx values more
carefully.
2024-03-18 11:28:40 -05:00
Ryan Ofsky f252e687ec assumeutxo test: Add RPC test for fake nTx and nChainTx values
The fake values will be removed in an upcoming commit, so it is useful to have
test coverage confirming the change in behavior.
2024-03-18 11:28:40 -05:00
fanquake 9a459e3ab9
Merge bitcoin/bitcoin#29669: ci: Drop `--enable-c++20` option
64722e4359 ci: Drop `--enable-c++20` option (Hennadii Stepanov)

Pull request description:

  This option has ceased to exist since https://github.com/bitcoin/bitcoin/pull/28349.

ACKs for top commit:
  maflcko:
    ACK 64722e4359

Tree-SHA512: bd392c331f775605615e1b236682269b83a1e6363a4d3f09c4d8d54495cf3d22973a921ebf6b8a9f813ba6c024d3324761f3291aaf7f473995f5eaa4c195bc43
2024-03-18 16:28:40 +00:00
fanquake aba9024c0c
Merge bitcoin/bitcoin#29659: ci: Bump `TIDY_LLVM_V`
636c9862cf ci: Bump `TIDY_LLVM_V` (Hennadii Stepanov)

Pull request description:

  This PR switches to the latest [IWYU 0.22](https://github.com/include-what-you-use/include-what-you-use/releases/tag/0.22), which is compatible with Clang 18.

ACKs for top commit:
  fanquake:
    ACK 636c9862cf

Tree-SHA512: 78ce89244c5e487dd1be8b4bd2ca6f06d19b04b78289ebc21985110574053545dcce5eb622edf2bede2cf7bb58360170e976d30a4484a127d34dd17b1c604e9c
2024-03-18 15:10:31 +00:00
fanquake 7af95afa8b
Merge bitcoin/bitcoin#29091: build: Bump g++ minimum supported version to 11
fa5844f06d Remove unused g++-10 workaround (MarcoFalke)
fa8409e760 build: Bump g++ minimum supported version to 11 (MarcoFalke)

Pull request description:

  This drops support for vanilla Ubuntu Focal 20.04 and Debian (Oldstable) Bullseye, compiling from source. Users on those operating systems would have to stick with a pre-compiled release, a previous release branch of Bitcoin Core, upgrade their system, compile their own compiler, or use a non-vanilla PPA or package manager.

  Otherwise, g++-11 is offered by common distributions:

  * https://packages.ubuntu.com/jammy/g++ (`g++-11`)
  * https://packages.debian.org/bookworm/g++ (`g++-12`)
  * FreeBSD 12/13 ships with g++ 12
  * CentOS-like 9 ships with g++ 11
  * OpenSuse Tumbleweed ships with g++ 13 https://software.opensuse.org/package/gcc13-c++ (No idea about OpenSuse Leap)

ACKs for top commit:
  TheCharlatan:
    ACK fa5844f06d
  fanquake:
    ACK fa5844f06d

Tree-SHA512: fc72d3a53956a0a4a6475ebf56b5fce76c3c4c793ed8e774327cad2b0f307d2d1c8aeafe2a414a7eb51f8de6d4bb78d30b8f60bf6e383234079851e72015c6e3
2024-03-18 14:56:39 +00:00
fanquake f1a19d79ff
Merge bitcoin/bitcoin#29537: lint: Misc improvements for lint runner
742d2b9347 lint: Add lint runner build dir and lint pycache to clean task (Fabian Jahr)
cfa057b86d lint: Add lint runner build dir to gitignore (Fabian Jahr)
fad7f42324 lint: Clarify lint runner rust dependency (Fabian Jahr)

Pull request description:

  1. Document the dependency to rust being installed locally
  2. Add the build output directory to gitignore
  3. Clean up the build output directory when running `make clean`

ACKs for top commit:
  maflcko:
    ACK 742d2b9347
  TheCharlatan:
    ACK 742d2b9347

Tree-SHA512: 36751d852e579830a9e6915b846886a6edaf4e42d508a4773ab502afda10b47c30c7c6bbd3e3158539ea5cf51592c2fe49c4221d271511006653a2d79119ed8c
2024-03-18 13:49:54 +00:00
Fabian Jahr 742d2b9347
lint: Add lint runner build dir and lint pycache to clean task 2024-03-17 21:24:04 +01:00
Fabian Jahr cfa057b86d
lint: Add lint runner build dir to gitignore 2024-03-17 21:24:03 +01:00
Fabian Jahr fad7f42324
lint: Clarify lint runner rust dependency 2024-03-17 21:24:02 +01:00
Hennadii Stepanov 64722e4359
ci: Drop `--enable-c++20` option
This option has ceased to exist since https://github.com/bitcoin/bitcoin/pull/28349.
2024-03-17 16:54:47 +00:00
Hennadii Stepanov 636c9862cf
ci: Bump `TIDY_LLVM_V`
This change switches to the latest IWYU 0.22, which is compatible with
Clang 18.
2024-03-15 13:34:05 +00:00
fanquake 015ac13dcc
Merge bitcoin/bitcoin#29487: lint: Fix lint-whitespace issues
5555395c15 lint: Use git --no-pager to print any output in one go (MarcoFalke)
fa5729436c lint: Fix lint-whitespace issues (MarcoFalke)

Pull request description:

  The lint check has many issues:

  * It uses `COMMIT_RANGE`, which is brittle code, apparently making it harder to run the CI locally, or self-hosted. See https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1457739319
  * The result depends on `COMMIT_RANGE`, or the number of commits passed to the script, which can cause false negatives or false positives.
  * It is based on the diff output, parsing it, and printing it again, which is brittle as well.
  * The output does not include line number, making it harder to act on a lint error.

  Fix all issues by removing the script and replacing it with a simple call to `git grep -I --line-number ...`.

ACKs for top commit:
  TheCharlatan:
    Re-ACK 5555395c15

Tree-SHA512: 71ea8b6382af064beb72fb17f21a0ae9e9238c97e7fa43c2ec353fd1dd73a7bbd696ba0f0a9f65d1eff7c86cbf6cc104a992cb5450a3d50f122955e835270065
2024-03-15 12:56:12 +00:00
fanquake 178b4d47cc
Merge bitcoin/bitcoin#29650: depends: drop 1 Qt determinism patch
76d6537698 depends: drop 1 qt determinism patch (fanquake)

Pull request description:

  No-longer required now that we are building with GCC 12.

  Guix Build (x86_64 && aarch64):
  ```bash
  e1c5b2c1c1a184e9d6985f26d26c61ca049e4541c699c6c9ce334beb832ed8da  guix-build-76d6537698e4/output/aarch64-linux-gnu/SHA256SUMS.part
  22f5d39fd9eac2d1fdbf2794fd6272ce05a1dfda2aeb2280a5dbafe76d0eec3d  guix-build-76d6537698e4/output/aarch64-linux-gnu/bitcoin-76d6537698e4-aarch64-linux-gnu-debug.tar.gz
  1cc798fb30b9e85e3b94049a671e2881b6b8694e52ae5e6468805c8ea6ea637f  guix-build-76d6537698e4/output/aarch64-linux-gnu/bitcoin-76d6537698e4-aarch64-linux-gnu.tar.gz
  a8c4ecc550aba01292885343ae9d53e31dfc0ef26f885fcf00493c754c5f9deb  guix-build-76d6537698e4/output/arm-linux-gnueabihf/SHA256SUMS.part
  d037b87949640d441b1ea000fd3fb27a508a699c5a6d69b6647611325cb9b7f5  guix-build-76d6537698e4/output/arm-linux-gnueabihf/bitcoin-76d6537698e4-arm-linux-gnueabihf-debug.tar.gz
  a858a0bce444a9eaf2b36d188198e54cdc14d85e344863d9e176dca94e458537  guix-build-76d6537698e4/output/arm-linux-gnueabihf/bitcoin-76d6537698e4-arm-linux-gnueabihf.tar.gz
  a802412eb9f2bbe2c573052581c21c44281e78c65d6ebc243105d5004768f0f9  guix-build-76d6537698e4/output/arm64-apple-darwin/SHA256SUMS.part
  20eeb2a28f096f53eeae61c64082c7eef0fb4d4e8dd7fb173a6cc19bf1960165  guix-build-76d6537698e4/output/arm64-apple-darwin/bitcoin-76d6537698e4-arm64-apple-darwin-unsigned.tar.gz
  54fae4652f21772d4d861b1a9dd3dcb913f088e6b7049a566c748ccdf2acede3  guix-build-76d6537698e4/output/arm64-apple-darwin/bitcoin-76d6537698e4-arm64-apple-darwin-unsigned.zip
  e986f3d8311df3ab26860c8747e9634687617609a317fdf242365d408235f417  guix-build-76d6537698e4/output/arm64-apple-darwin/bitcoin-76d6537698e4-arm64-apple-darwin.tar.gz
  3efced764d3b62362c906f1fbbdecf347be1aab877afb2d6ce8f39d24b3d7437  guix-build-76d6537698e4/output/dist-archive/bitcoin-76d6537698e4.tar.gz
  6bd047bd080ae0d0a08a15e83a79dfd17bf29227599517d0bccae17a0224a741  guix-build-76d6537698e4/output/powerpc64-linux-gnu/SHA256SUMS.part
  102909ef544962e08577464b3293c0013237391b7577d7abc26f1244d3d0157d  guix-build-76d6537698e4/output/powerpc64-linux-gnu/bitcoin-76d6537698e4-powerpc64-linux-gnu-debug.tar.gz
  01cf35c37093f768c953697c8d0102316e1e719befd2853a74266bcc2105c52c  guix-build-76d6537698e4/output/powerpc64-linux-gnu/bitcoin-76d6537698e4-powerpc64-linux-gnu.tar.gz
  467f858d1aba32ee290e6ba00feee632fcb56907f77e5b48f4de969d8ce78457  guix-build-76d6537698e4/output/riscv64-linux-gnu/SHA256SUMS.part
  893cb65a79709c58ebafb003ce43b1cd51434d9c0a9175b7dfede6aa99fec3d2  guix-build-76d6537698e4/output/riscv64-linux-gnu/bitcoin-76d6537698e4-riscv64-linux-gnu-debug.tar.gz
  be3bd03cdef59928eb8a18bd59f48ad27ae38a6382bf94651774845adbd28308  guix-build-76d6537698e4/output/riscv64-linux-gnu/bitcoin-76d6537698e4-riscv64-linux-gnu.tar.gz
  1ff2b04cccd44c4c6526387307fb381f52fbc36b31a51730435d6b99e0fa4610  guix-build-76d6537698e4/output/x86_64-apple-darwin/SHA256SUMS.part
  9c84639c4b7e1e39c06da4c9430efe94993ce97fbc279b38502c1d4fc25ac801  guix-build-76d6537698e4/output/x86_64-apple-darwin/bitcoin-76d6537698e4-x86_64-apple-darwin-unsigned.tar.gz
  e65c009c728aa42f24b6970018336058adc78fba09b85aa76310de98fdabb8ad  guix-build-76d6537698e4/output/x86_64-apple-darwin/bitcoin-76d6537698e4-x86_64-apple-darwin-unsigned.zip
  1a85307eec81cc13e5d599db1bb7cddd3d4f6847f2bad7685e096c439b44489a  guix-build-76d6537698e4/output/x86_64-apple-darwin/bitcoin-76d6537698e4-x86_64-apple-darwin.tar.gz
  10189926b6ccef3ab1feee3edce34a80a30e60ee67c00519e344fefd6c9880d0  guix-build-76d6537698e4/output/x86_64-linux-gnu/SHA256SUMS.part
  0094570197c0a91b7a903c1250bf899ea50d7452608da03f5dd825febd5e216b  guix-build-76d6537698e4/output/x86_64-linux-gnu/bitcoin-76d6537698e4-x86_64-linux-gnu-debug.tar.gz
  8375afd9ea4376b354548270323fa0f5f3244579b59dcdf9c26330337b5719ab  guix-build-76d6537698e4/output/x86_64-linux-gnu/bitcoin-76d6537698e4-x86_64-linux-gnu.tar.gz
  5a30053ee8db9eb2d083e8569a1a69b24acc84de1028f3f40d5e902a1174e49e  guix-build-76d6537698e4/output/x86_64-w64-mingw32/SHA256SUMS.part
  1d624077e027dd6f213c85d75fdbac12d61c45235946817c406132fbd222c939  guix-build-76d6537698e4/output/x86_64-w64-mingw32/bitcoin-76d6537698e4-win64-debug.zip
  e75107ce5608d83708b4e9b5a64d50e0282560ee2d8d915a20993fd383d2d456  guix-build-76d6537698e4/output/x86_64-w64-mingw32/bitcoin-76d6537698e4-win64-setup-unsigned.exe
  7fb1f412fd71e0e8302add6bcc5679ad6990d87c16688a396769844f72ae7c82  guix-build-76d6537698e4/output/x86_64-w64-mingw32/bitcoin-76d6537698e4-win64-unsigned.tar.gz
  be24df85e0834823f0ad9611667100330972d3a18460099d7df5b4386fbd6403  guix-build-76d6537698e4/output/x86_64-w64-mingw32/bitcoin-76d6537698e4-win64.zip
  ```

ACKs for top commit:
  TheCharlatan:
    ACK 76d6537698

Tree-SHA512: 69e698e9b0036ecb1f89db82427c25d0368d2178c3dc2bc751181c19a1139929bf0da160af6f3e021ca3da59ea66f7b7330aa6295f5e65c6ef0bbcf369fcbc94
2024-03-15 11:31:23 +00:00
MarcoFalke fa5844f06d
Remove unused g++-10 workaround
This reverts d4999d40b9
2024-03-14 15:53:43 +01:00
glozow 3d255dfb67
Merge bitcoin/bitcoin#29459: test: check_mempool_result negative feerate
bf264e0598 test: check_mempool_result negative feerate (kevkevin)

Pull request description:

  Adds test coverage in `mempool_accept.py` to check if a negative `maxfeerate` is input into `check_mempool_result`
  Asserts "Amount out of range" error message and `-3` error code

  Motivated by this [comment](https://github.com/bitcoin/bitcoin/pull/29434/files#r1491112250)

ACKs for top commit:
  maflcko:
    lgtm ACK bf264e0598
  brunoerg:
    nice, utACK bf264e0598
  davidgumberg:
    Looks great, ACK bf264e0598

Tree-SHA512: 58931b774cc887c616f2fd91af3ee65cc5db55acd8e2875c76de448c80bd4e020b057c5f4f85556431377f0d0e7553771fb285d1ec20cf64f64ec92a47776b78
2024-03-14 11:16:50 +00:00
MarcoFalke fa8409e760
build: Bump g++ minimum supported version to 11 2024-03-14 12:15:22 +01:00
fanquake 76d6537698
depends: drop 1 qt determinism patch
No-longer required now that we are building with GCC 12.
2024-03-14 10:40:17 +00:00
fanquake e1ce5b8ae9
Merge bitcoin/bitcoin#27897: guix: use GCC 12.3.0 to build releases
10d56530e0 guix: temporarily disable powerpcle taget (fanquake)
001412a4d2 guix: use GCC 12.3.0 (fanquake)
ce54330cf6 ci: use Debian Bookworm (GCC 12) for ARM ci job (fanquake)
0da6451c58 ci: use Debian Bookworm (GCC 12) for win64 job (fanquake)

Pull request description:

  Switch to using [GCC `12.3.0`](https://gcc.gnu.org/gcc-12/) to build release binaries.

  Temporarily disables the `powerpc64le-linux-gnu` target due to non-determinism issues when building across `aarch64` and `x86_64`. Trying to fix the non-determinism was going to require trying to selectively disable optimization flags, which is already not ideal (and didn't fix all issues), and the migration to GCC 12 as our release compiler is now the blocker for multiple other (c++20 and similar) changes, so leaving this blocked on the `powerpc64le` binaries does not seem like a good tradeoff.

ACKs for top commit:
  TheCharlatan:
    ACK 10d56530e0

Tree-SHA512: 401bbaaf2b72c795a06a24875ffd666151b41bae8f45bda10526ff4f6b59782704246afc6585f6b849021cbff8a7b861961d139bffe45536aaaeb3952b72ae57
2024-03-14 10:17:47 +00:00
fanquake 6850d72174
Merge bitcoin/bitcoin#29497: test: simplify test_runner.py
0831b54dfc test: simplify test_runner.py (tdb3)

Pull request description:

  Implements the simplifications to test_runner.py proposed by sipa in PR #23995.

  Remove the num_running variable as it can be implied by the length of the jobs list.

  Remove the i variable as it can be implied by the length of the test_results list.

  Instead of counting results to determine if finished, make the queue object itself
  responsible (by looking at running jobs and jobs left).

ACKs for top commit:
  mzumsande:
    re-ACK 0831b54
  davidgumberg:
    reACK 0831b54dfc
  marcofleon:
    re-ACK 0831b54dfc

Tree-SHA512: e5473e68d49cd779b29d97635329283ae7195412cb1e92461675715ca7eedb6519a1a93ba28d40ca6f015d270f7bcd3e77cef279d9cd655155ab7805b49638f1
2024-03-14 10:09:00 +00:00
fanquake 55c6323434
Merge bitcoin/bitcoin#29649: netbase: remove unnecessary log message
c70e4fc9a3 netbase: remove unnecessary log message (Matthew Zipkin)

Pull request description:

  This is a follow-up to #27375 that removes a spammy non-debug-level log message we don't need.
  See https://github.com/bitcoin/bitcoin/pull/27375#issuecomment-1994498888

ACKs for top commit:
  fanquake:
    ACK c70e4fc9a3 - thanks. Merging this now because it's swamping non-debug logs. i.e:

Tree-SHA512: 837682860abdf740fce5dc88c8599e066660cf16b4cab1473391eb51ad538ae52d236ecd3543df866e2a2165870397a8bf21ad9f5125ed8212a3fe207d615553
2024-03-14 09:48:58 +00:00
Matthew Zipkin c70e4fc9a3
netbase: remove unnecessary log message
see https://github.com/bitcoin/bitcoin/pull/27375#issuecomment-1994498888
2024-03-13 14:09:50 -04:00
MarcoFalke 5555395c15
lint: Use git --no-pager to print any output in one go 2024-03-13 17:07:15 +01:00
Ava Chow a85e5a7c9a
Merge bitcoin/bitcoin#29478: test: Test new header sync behavior in loadtxoutset
1ec6684b08 test: Add test for loadtxoutset when headers are not synced (Fabian Jahr)
2bc1ecfaa9 test: Remove unnecessary sync_blocks in assumeutxo tests (Fabian Jahr)

Pull request description:

  It adds a test for the change to `loadtxoutset` made in #29345.  Before that change the test doesn't fail right away but times out after 10 minutes.

  Also removes a `sync_blocks` call that didn't seem to do anything valuable.

ACKs for top commit:
  achow101:
    ACK 1ec6684b08
  pablomartin4btc:
    tACK 1ec6684b08
  BrandonOdiwuor:
    ACK 1ec6684b08
  theStack:
    ACK 1ec6684b08

Tree-SHA512: 1337decdf91e4a4f7213fcf8ace1d705e5c1422e0ac3872a59b5be9c33e743850cb8f5f7474750a534952eefd5cfe43fe85a54efb9bc0e47515136a2903676e5
2024-03-13 12:03:55 -04:00
Ava Chow ef6329f052
Merge bitcoin/bitcoin#28193: test: add script compression coverage for not-on-curve P2PK outputs
28287cfbe1 test: add script compression coverage for not-on-curve P2PK outputs (Sebastian Falbesoner)

Pull request description:

  This PR adds unit test coverage for the script compression functions `{Compress,Decompress}Script` in the special case of uncompressed P2PK outputs (scriptPubKey: OP_PUSH65 <0x04 ....> OP_CHECKSIG) with [pubkeys that are not fully valid](44b05bf3fe/src/pubkey.cpp (L297-L302)), i.e. where the encoded point is not on the secp256k1 curve. For those outputs, script compression is not possible, as the y coordinate of the pubkey can't be recovered (see also call-site of `IsToPubKey`):

  44b05bf3fe/src/compressor.cpp (L49-L50)

  Likewise, for a compressed script of an uncompressed P2PK script (i.e. compression ids 4 and 5) where the x coordinate is not on the curve, decompression fails:

  44b05bf3fe/src/compressor.cpp (L122-L129)

  Note that the term "compression" is used here in two different meanings (though they are related), which might be a little confusing. The encoding of a pubkey can either be compressed (33-bytes with 0x02/0x03 prefixes) or uncompressed (65-bytes with 0x04 prefix). On the other hand there is also compression for whole output scripts, which is used for storing scriptPubKeys in the UTXO set in a compact way (and also for the `dumptxoutset` result, accordingly). P2PK output scripts with uncompressed pubkeys get compressed by storing only the x-coordinate and the sign as a prefix (0x04 = even, 0x05 = odd). Was diving deeper into the subject while working on https://github.com/bitcoin/bitcoin/pull/27432, where the script decompression of uncompressed P2PK needed special handling (see also https://github.com/bitcoin/bitcoin/issues/24628#issuecomment-1108798536).

  Trivia: as of now (block 801066), there are 13 uncompressed P2PK outputs in the UTXO set with a pubkey not on the curve (which obviously means they are unspendable).

ACKs for top commit:
  achow101:
    ACK 28287cfbe1
  tdb3:
    ACK for 28287cfbe1.
  cbergqvist:
    ACK 28287cf!
  marcofleon:
    Nicely done, ACK 28287cfbe1. Built the PR branch, ran the unit and functional tests, everything passed.

Tree-SHA512: 777b6c3065654fbfa1ce94926f4cadb91a9ca9dc4dd4af6008ad77bd1da5416f156ad0dfa880d26faab2e168bf9b27e0a068abc9a2be2534d82bee61ee055c65
2024-03-13 11:02:23 -04:00
Ava Chow c38157b9b9
Merge bitcoin/bitcoin#29606: refactor: Reserve memory for ToLower/ToUpper conversions
6f2f4a4d09 Reserve memory for ToLower/ToUpper conversions (Lőrinc)

Pull request description:

  Similarly to https://github.com/bitcoin/bitcoin/pull/29458, we're preallocating the result string based on the input string's length.
  The methods were already [covered by tests](https://github.com/bitcoin/bitcoin/blob/master/src/test/util_tests.cpp#L1250-L1276).

ACKs for top commit:
  tdb3:
    ACK for 6f2f4a4d09
  maflcko:
    lgtm ACK 6f2f4a4d09
  achow101:
    ACK 6f2f4a4d09
  Empact:
    Code Review ACK 6f2f4a4d09
  stickies-v:
    ACK 6f2f4a4d09

Tree-SHA512: e3ba7af77decdc73272d804c94fef0b11028a85f3c0ea1ed6386672611b1c35fce151f02e64f5bb5acb5ba506aaa54577719b07925b9cc745143cf5c7e5eb262
2024-03-13 08:18:06 -04:00
Ava Chow 264ca9db24
Merge bitcoin/bitcoin#29619: refactor: consolidate MempoolAcceptResult processing
07cd510ffe [refactor] consolidate invalid MempoolAcceptResult processing (glozow)
9353aa4066 [refactor] consolidate valid MempoolAcceptResult processing (glozow)

Pull request description:

  Every time we try to `ProcessTransaction` (i.e. submit a tx to mempool), we use the result to update a few net processing data structures. For example, after a failure, the {wtxid, txid, both, neither} (depending on reason) should be cached in `m_recent_rejects` so we don't try to download/validate it again.

  There are 2 current places and at least 1 future place where we need to process `MempoolAcceptResult`:
  - In the `ProcessMessage` logic after receiving and validating a tx
  - In `ProcessOrphanTx` where we retry orphans
  - With #28970, after processing a package of transactions, we should do these updates for each tx in the package.

  Consolidate this code so it isn't repeated in 2 places and so we can reuse it in a future PR.

ACKs for top commit:
  instagibbs:
    ACK 07cd510ffe
  achow101:
    ACK 07cd510ffe
  dergoegge:
    Code review ACK 07cd510ffe
  TheCharlatan:
    ACK 07cd510ffe

Tree-SHA512: c4e74cb65e4f52882fca52e6682efa5dcf1562d98418454e09be256ffd026caae642a90aa5b9cccaae214be240d6f4be9d87b516953b2ee69a655f16ea569ed9
2024-03-13 07:26:34 -04:00
Ava Chow 0ed2c130e7
Merge bitcoin/bitcoin#27375: net: support unix domain sockets for -proxy and -onion
567cec9a05 doc: add release notes and help text for unix sockets (Matthew Zipkin)
bfe5192891 test: cover UNIX sockets in feature_proxy.py (Matthew Zipkin)
c65c0d0163 init: allow UNIX socket path for -proxy and -onion (Matthew Zipkin)
c3bd43142e gui: accomodate unix socket Proxy in updateDefaultProxyNets() (Matthew Zipkin)
a88bf9dedd i2p: construct Session with Proxy instead of CService (Matthew Zipkin)
d9318a37ec net: split ConnectToSocket() from ConnectDirectly() for unix sockets (Matthew Zipkin)
ac2ecf3182 proxy: rename randomize_credentials to m_randomize_credentials (Matthew Zipkin)
a89c3f59dc netbase: extend Proxy class to wrap UNIX socket as well as TCP (Matthew Zipkin)
3a7d6548ef net: move CreateSock() calls from ConnectNode() to netbase methods (Matthew Zipkin)
74f568cb6f netbase: allow CreateSock() to create UNIX sockets if supported (Matthew Zipkin)
bae86c8d31 netbase: refactor CreateSock() to accept sa_family_t (Matthew Zipkin)
adb3a3e51d configure: test for unix domain sockets (Matthew Zipkin)

Pull request description:

  Closes https://github.com/bitcoin/bitcoin/issues/27252

  UNIX domain sockets are a mechanism for inter-process communication that are faster than local TCP ports (because there is no need for TCP overhead) and potentially more secure because access is managed by the filesystem instead of serving an open port on the system.

  There has been work on [unix domain sockets before](https://github.com/bitcoin/bitcoin/pull/9979) but for now I just wanted to start on this single use-case which is enabling unix sockets from the client side, specifically connecting to a local Tor proxy (Tor can listen on unix sockets and even enforces strict curent-user-only access permission before binding) configured by `-onion=` or `-proxy=`

  I copied the prefix `unix:` usage from Tor. With this patch built locally you can test with your own filesystem path (example):

  `tor --SocksPort unix:/Users/matthewzipkin/torsocket/x`

  `bitcoind -proxy=unix:/Users/matthewzipkin/torsocket/x`

  Prep work for this feature includes:
  - Moving where and how we create `sockaddr` and `Sock` to accommodate `AF_UNIX` without disturbing `CService`
  - Expanding `Proxy` class to represent either a `CService` or a UNIX socket (by its file path)

  Future work:
  - Enable UNIX sockets for ZMQ (https://github.com/bitcoin/bitcoin/pull/27679)
  - Enable UNIX sockets for I2P SAM proxy (some code is included in this PR but not tested or exposed to user options yet)
  - Enable UNIX sockets on windows where supported
  - Update Network Proxies dialog in GUI to support UNIX sockets

ACKs for top commit:
  Sjors:
    re-ACK 567cec9a05
  tdb3:
    re ACK for 567cec9a05.
  achow101:
    ACK 567cec9a05
  vasild:
    ACK 567cec9a05

Tree-SHA512: de81860e56d5de83217a18df4c35297732b4ad491e293a0153d2d02a0bde1d022700a1131279b187ef219651487537354b9d06d10fde56225500c7e257df92c1
2024-03-13 06:53:07 -04:00
tdb3 0831b54dfc
test: simplify test_runner.py
Remove the num_running variable as it can be implied by the
length of the jobs list.

Remove the i variable as it can be implied by the length of the
test_results list.

Instead of counting results to determine if finished, make the
queue object itself responsible (by looking at running jobs and
jobs left).

Originally proposed by @sipa in PR #23995.

Co-authored-by: Pieter Wuille <pieter@wuille.net>
2024-03-12 18:00:04 -04:00
fanquake 1105aa46dd
Merge bitcoin/bitcoin#29633: log: Remove error() reference
d0e6564240 log: Remove error() reference (Fabian Jahr)

Pull request description:

  Mini-followup to #29236 that was just merged. Removes a reference to `error()` that was missed in a comment.

ACKs for top commit:
  ryanofsky:
    Code review ACK d0e6564240. Just dropped LogPrintf reference since last review
  stickies-v:
    ACK d0e6564240
  Empact:
    ACK d0e6564240

Tree-SHA512: 8abe4895951013c2ceca9a57743aacabaf8af831d07eee9ae8372c121c16e88b7226f0e537200c3464792e19ac7e03b57ba0be31f43add8802753972b0aefc48
2024-03-12 18:44:39 +00:00
Ava Chow bde3db40f6
Merge bitcoin/bitcoin#26415: rpc,rest,zmq: faster getblock, NotifyBlock and rest_block by reading raw block
e710cefd57 rest: read raw block in rest_block and deserialize for json (Andrew Toth)
95ce0783a6 rpc: read raw block in getblock and deserialize for verbosity > 0 (Andrew Toth)
0865ab8712 test: check more details on zmq raw block response (Andrew Toth)
38265cc14e zmq: read raw block with ReadRawBlockFromDisk (Andrew Toth)
da338aada7 blockstorage: check nPos in ReadRawBlockFromDisk before seeking back (Andrew Toth)

Pull request description:

  For the `getblock` endpoint with `verbosity=0`, the  `rest_block` REST endpoint for `bin` and `hex`, and zmq `NotifyBlock` we don't have to deserialize the block since we're just sending the raw data. This PR uses `ReadRawBlockFromDisk` instead of `ReadBlockFromDisk` to serve these requests, and only deserializes for `verbosity > 0` and `json` REST requests. See benchmarks in https://github.com/bitcoin/bitcoin/pull/26684.

  Benchmarked using ApacheBench. Requesting block 750,000 in binary 10k times on a single core (set `-rest=1` in config):
  `ab -n 10000 -c 1 "http://127.0.0.1:8332/rest/block/0000000000000000000592a974b1b9f087cb77628bb4a097d5c2c11b3476a58e.bin"`

  On master, mean time 15ms.
  On this branch, mean time 1ms.

  For RPC
  ```
  echo '{"jsonrpc": "1.0", "id": "curltest", "method": "getblock", "params": ["0000000000000000000592a974b1b9f087cb77628bb4a097d5c2c11b3476a58e", 0]}' > /tmp/data.json
  ab -p /tmp/data.json -n 1000 -c 1 -A user:password "http://127.0.0.1:8332/"
  ```
  On master, mean time 32ms
  On this branch, mean time 13ms

ACKs for top commit:
  achow101:
    re-ACK e710cefd57

Tree-SHA512: 4cea13c7b563b2139d041b1fdcfdb793c8cc688654ae08db07e7ee6b875c5e582b8185db3ae603abbfb06d2164724f29205774620b48c493726b991999af289e
2024-03-12 13:17:57 -04:00
Ava Chow bef99176e6
Merge bitcoin/bitcoin#27114: p2p: Allow whitelisting manual connections
0a533613fb docs: add release notes for #27114 (brunoerg)
e6b8f19de9 test: add coverage for whitelisting manual connections (brunoerg)
c985eb854c test: add option to speed up tx relay/mempool sync (brunoerg)
66bc6e2d17 Accept "in" and "out" flags to -whitelist to allow whitelisting manual connections (Luke Dashjr)
8e06be347c net_processing: Move extra service flag into InitializeNode (Luke Dashjr)
9133fd69a5 net: Move `NetPermissionFlags::Implicit` verification to `AddWhitelistPermissionFlags` (Luke Dashjr)
2863d7dddb net: store `-whitelist{force}relay` values in `CConnman` (brunoerg)

Pull request description:

  Revives #17167. It allows whitelisting manual connections. Fixes #9923

  Since there are some PRs/issues around this topic, I'll list some motivations/comments for whitelisting outbound connections from them:
  - Speed-up tx relay/mempool sync for testing purposes (my personal motivation for this) - In #26970, theStack pointed out that we whitelist peers to speed up tx relay for fast mempool synchronization, however, since it applies only for inbound connections and considering the topology `node0 <--- node1 <---- node2 <--- ... <-- nodeN`,  if a tx is submitted from any node other than node0, the mempool synchronization can take quite long.
  - https://github.com/bitcoin/bitcoin/pull/29058#issuecomment-1865155764 - "Before enabling -v2transport by default (which I'd image may happen after https://github.com/bitcoin/bitcoin/pull/24748) we could consider a way to force manual connections to be only-v1 or even only-v2 (disabling reconnect-with-v1). A possibility could be through a net permission flag, if https://github.com/bitcoin/bitcoin/pull/27114 makes it in."
  - https://github.com/bitcoin/bitcoin/pull/17167#issuecomment-1168606032 - "This would allow us to use https://github.com/bitcoin/bitcoin/pull/25355 when making outgoing connections to all nodes, except to whitelisted ones for which we would use our persistent I2P address."
  - Force-relay/mempool permissions for a node you intentionally connected to.

ACKs for top commit:
  achow101:
    ACK 0a533613fb
  sr-gi:
    re-ACK [0a53361](0a533613fb)
  pinheadmz:
    ACK 0a533613fb

Tree-SHA512: 97a79bb854110da04540897d2619eda409d829016aafdf1825ab5515334b0b42ef82f33cd41587af235b3af6ddcec3f2905ca038b5ab22e4c8a03d34f27aebe1
2024-03-12 12:59:02 -04:00
Andrew Toth e710cefd57
rest: read raw block in rest_block and deserialize for json
Note that for speed this commit also removes the proof of work and
signet signature checks before returning the block in getblock.
It is assumed if a block is stored it will be valid.
2024-03-12 12:48:04 -04:00
Andrew Toth 95ce0783a6
rpc: read raw block in getblock and deserialize for verbosity > 0
Note that for speed this commit also removes the proof of work and
signet signature checks before returning the block in getblock.
It is assumed if a block is stored it will be valid.
2024-03-12 12:47:17 -04:00
Andrew Toth 0865ab8712
test: check more details on zmq raw block response 2024-03-12 12:47:01 -04:00
Andrew Toth 38265cc14e
zmq: read raw block with ReadRawBlockFromDisk 2024-03-12 12:46:46 -04:00
Andrew Toth da338aada7
blockstorage: check nPos in ReadRawBlockFromDisk before seeking back
ReadRawBlockFromDisk assumes a non-null pos that has an nPos >= 8.
This simple check makes the function safer to call in the future,
so callers don't need to worry about causing UB if the pos is null.
2024-03-12 12:46:07 -04:00
fanquake 10d56530e0
guix: temporarily disable powerpcle taget
There non-determinism issues when compiling for this target across
x86_64 and aarch64.
2024-03-12 16:26:27 +00:00
fanquake 001412a4d2
guix: use GCC 12.3.0
Retain native GCC 10 toolchain for macOS, to prevent compile failures in
native tools (this will be removed entirely when we tansition to LLD).
Update the vmov-alignment patch, for changes in GCC 12.
2024-03-12 16:26:27 +00:00
fanquake ce54330cf6
ci: use Debian Bookworm (GCC 12) for ARM ci job
Co-authored-by: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
2024-03-12 16:26:27 +00:00
fanquake 0da6451c58
ci: use Debian Bookworm (GCC 12) for win64 job
Co-authored-by: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
2024-03-12 16:26:27 +00:00
Ava Chow 12dae637a4
Merge bitcoin/bitcoin#29306: policy: enable sibling eviction for v3 transactions
1342a31f3a [functional test] sibling eviction (glozow)
5fbab37859 [unit test] sibling not returned from SingleV3Checks if 1p2c or 3gen (glozow)
170306728a [policy] sibling eviction for v3 transactions (glozow)
b5d15f764f [refactor] return pair from SingleV3Checks (glozow)

Pull request description:

  When we receive a v3 transaction that would bust a mempool transaction's descendant limit, instead of rejecting the new tx, consider replacing the other descendant if it is much higher feerate (using existing RBF criteria to assess that it's more incentive compatible and to avoid DoS).

  Delving post with more background and motivation: https://delvingbitcoin.org/t/sibling-eviction-for-v3-transactions/472

ACKs for top commit:
  sdaftuar:
    ACK 1342a31f3a
  achow101:
    ACK 1342a31f3a
  instagibbs:
    ACK 1342a31f3a

Tree-SHA512: dd957d49e51db78758f566c49bddc579b72478e371275c592d3d5ba097d20de47a6c81952045021b99d82a787f5b799baf16dd0ee0e6de90ba12e21e275352be
2024-03-12 12:19:48 -04:00
Fabian Jahr d0e6564240
log: Remove error() reference 2024-03-12 16:26:15 +01:00
MarcoFalke fa5729436c
lint: Fix lint-whitespace issues 2024-03-12 13:38:39 +01:00
fanquake d14c7286b6
Merge bitcoin/bitcoin#29620: ci: add print of powershell version to win64 job
115c283516 ci: add print of powershell version to win64 job (Max Edwards)

Pull request description:

  Extraction of just printing powershell version from closed PR: https://github.com/bitcoin/bitcoin/pull/29581

  See https://github.com/bitcoin/bitcoin/pull/29581#issuecomment-1984212990 for the cause of a CI failure which was a powershell update.

  This PR will make it easier to notice in the future that PS has changed.

ACKs for top commit:
  hebasto:
    ACK 115c283516. We still use PowerShell in some steps of the "Win64 native" CI job.

Tree-SHA512: 4c7ba9df4f0a98491120326f05e877a995f43a387fe9bbd193549b32f5a4488f85f83e472c9277db457110a7deda04f08832fe6e8129aff4b0b7278be23d4e35
2024-03-12 11:53:26 +00:00
fanquake bd55b7a528
Merge bitcoin/bitcoin#29610: ci: Fix "macOS native" job
acc06bc91f ci, macos: Use `--break-system-packages` with Homebrew's python (Hennadii Stepanov)
ae5f72027f ci: Add workaround for Homebrew's python link error (Hennadii Stepanov)

Pull request description:

  Homebrew [promoted](https://github.com/Homebrew/homebrew-core/pull/150390) `python@3.12` to the default `python3`. Now, our "macOS native" CI job is facing the following issues:

  1. Installing `qt@5` [requires](https://github.com/bitcoin/bitcoin/actions/runs/8216848118/job/22471875454#step:4:51) re-installing `python@3.12`:
  ```
  ==> Fetching dependencies for qt@5: readline, python@3.12 and gettext
  ```
  2. Re-installing `python@3.12` [fails](https://github.com/bitcoin/bitcoin/actions/runs/8216848118/job/22471875454#step:4:127) due to symbolic link conflicts on macOS `x86_64`:
  ```
  ==> Pouring python@3.12--3.12.2_1.ventura.bottle.tar.gz
  Error: The `brew link` step did not complete successfully
  ```
  3. Homebrew's `python@3.12` is marked as externally managed (according to PEP 668), necessitating different approaches for installing Python packages.

  This pull request resolves all the issues mentioned above.

ACKs for top commit:
  m3dwards:
    reACK acc06bc91f to get the CI passing again.

Tree-SHA512: 82cf72bff328f1e0725342431ac14ad4bae7a758186d97db6c7a558e4b661dcbf3fabe536978e26e709c5f6f7f5c11aac46642634c6685f1291592d8d825ad87
2024-03-12 11:43:33 +00:00