30729 Commits

Author SHA1 Message Date
merge-script
8e9801bfc4
Merge bitcoin/bitcoin#22818: test: Activate all regtest softforks at height 1, unless overridden
fa4db8671bb604e11b43a837f91de8866226f166 test: Activate all regtest softforks at height 1, unless overridden (MarcoFalke)
faad1e5ffda255aecf1b0ea2152cd4f6805e678f Introduce -testactivationheight=name@height setting (MarcoFalke)
fadb2ef2fa8561882db463f35df9b8a0e9609658 test: Add extra_args argument to TestChain100Setup constructor (MarcoFalke)
faa46986aaec69e4cf016101ae517ce8778e2ac5 test: Remove version argument from build_next_block in p2p_segwit test (MarcoFalke)
fa086ef5398b5ffded86e4f0d6633c523cb774e9 test: Remove unused ~TestChain100Setup (MarcoFalke)

Pull request description:

  All softforks that are active at the tip of mainnet, should also be active from genesis in regtest. Otherwise their rules might not be enforced in user testing, thus making their testing less useful.

  To still allow tests to check pre-softfork rules, a runtime argument can change the activation height.

ACKs for top commit:
  laanwj:
    Code review ACK fa4db8671bb604e11b43a837f91de8866226f166
  theStack:
    re-ACK fa4db8671bb604e11b43a837f91de8866226f166

Tree-SHA512: 6397d46ff56ebc48c007a4cda633904d6ac085bc76b4ecf83097c546c7eec93ac0c44b88083b2611b9091c8d1fb8ee1e314065de078ef15e922c015de7ade8bf
2021-09-24 14:04:51 +02:00
W. J. van der Laan
b7e3600815
Merge bitcoin/bitcoin#21526: validation: UpdateTip/CheckBlockIndex assumeutxo support
673a5bd3377929a0a6a62eda8b560e47bc2cca0c test: validation: add unittest for UpdateTip behavior (James O'Beirne)
2705570109a2a90ecfd3f4180944498626fc2707 test: refactor: separate CreateBlock in TestChain100Setup (James O'Beirne)
298bf5d563cc740c6ae71750d86942e0278b22d6 test: refactor: declare NoMalleation const auto (James O'Beirne)
071200993f3a9412821ce5387851d659baf85327 move-only: unittest: add test/util/chainstate.h (James O'Beirne)
8f5710fd0ac5173b577e5d00708485170b321bcc validation: fix CheckBlockIndex for multiple chainstates (James O'Beirne)
5a807736dacfc3e6fa57231219336acf08be38fb validation: insert assumed-valid block index entries into candidates (James O'Beirne)
01a9b8fe719efab2c268dc738bc93cfbdf92edb7 validation: set BLOCK_ASSUMED_VALID during snapshot load (James O'Beirne)
42b2520db93fd9feb3df4101654391fa7d3e2140 chain: add BLOCK_ASSUMED_VALID for use with assumeutxo (James O'Beirne)
b217020df78bc981d221fe04497c831120ef969f validation: change UpdateTip for multiple chainstates (James O'Beirne)
665072a36df2e4c88705fedd4ac7c955d7f6a488 doc: add comment for g_best_block (James O'Beirne)
ac4051d891e2d5c8ac130da16b85b9d880b44720 refactor: remove unused assumeutxo methods (James O'Beirne)
9f6bb539359b98d5b39482ab8a28a68608f0c645 validation: add chainman ref to CChainState (James O'Beirne)

Pull request description:

  This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11) (parent PR: #15606)

  ---

  Modify UpdateTip and CheckBlockIndex for use with multiple chainstates. Includes a new unittest verifying `g_best_block` behavior (previously untested at the unit level) and various changes necessary for running and testing `ProcessNewBlock()`-like behavior on the background validation chainstate.

  This changeset introduces a new block index `nStatus` flag called `BLOCK_ASSUMED_VALID`, and it is applied to block index entries that are beneath the UTXO snapshot base block upon snapshot load. Once each block is validated (during async background validation), the flag is removed. This allows us to avoid (ab)using `BLOCK_VALID_*` flags for snapshot chain block entries, and preserves the original meaning of those flags.

  Note: this PR previously incorporated changes to `LoadBlockIndex()` and `RewindBlockIndex()` as noted in Russ' comments below, but once I generated the changes necessary to test the UpdateTip change, I decided to split this changes out into another PR due to the size of this one.

ACKs for top commit:
  achow101:
    ACK 673a5bd3377929a0a6a62eda8b560e47bc2cca0c
  jonatack:
    Code-review re-ACK 673a5bd3377929a0a6a62eda8b560e47bc2cca0c reviewed diff, rebased to master/debug build/ran unit+functional tests
  naumenkogs:
    ACK 673a5bd3377929a0a6a62eda8b560e47bc2cca0c
  fjahr:
    Code review ACK 673a5bd3377929a0a6a62eda8b560e47bc2cca0c
  ariard:
    utACK 673a5bd3
  ryanofsky:
    Code review ACK 673a5bd3377929a0a6a62eda8b560e47bc2cca0c. Just linker fix and split commit changes mentioned https://github.com/bitcoin/bitcoin/pull/21526#issuecomment-921064563 since last review
  benthecarman:
    ACK 673a5bd3377929a0a6a62eda8b560e47bc2cca0c

Tree-SHA512: 0a6dc23d041b27ed9fd0ee1f3e5971b92fb1d2df2fc9b655d5dc48594235321ab1798d06de2ec55482ac3966a9ed56de8d56e9e29cae75bbe8690bafc2dda383
2021-09-23 22:22:07 +02:00
W. J. van der Laan
d8b4b3077f
Merge bitcoin/bitcoin#23057: log: Consolidate timedata logging
64e1ddd255771e57a88a20f07dbde04a83bf0c75 log: call LogPrint only once with time data samples (Martin Zumsande)

Pull request description:

  When timedata samples are logged, `LogPrint()` is currently invoked multiple times on the same log entry.
  This can lead to chaos in the log when other threads log concurrently, as in this example which motivated this PR:
  ```
  2021-09-20T00:28:57Z -48  -26  -11  -8  -6  Addrman checks started: new 37053, tried 83, total 37136
  2021-09-20T00:28:57Z -3  -1  -1  -1  -1  +0  |  nTimeOffset = -3  (+0 minutes)
  ```
  Fix this by building the log message in a string and logging it one `LogPrint()` call. I also changed the wording slightly so that it becomes understandable what is being logged, example:

  ```
  2021-09-21T21:03:24Z time data samples: -43  -18  -12  -4  -1  -1  +0  +0  +268  |  median offset = -1  (+0 minutes)
  ```

ACKs for top commit:
  jnewbery:
    Tested ACK 64e1ddd255
  laanwj:
    Tested ACK 64e1ddd255771e57a88a20f07dbde04a83bf0c75, new message lgtm

Tree-SHA512: ffb7a93166cc8fd6a39200b9e03a9d1e8e975b7ded822ccddd015f553258b991162a5cb867501f426d3ebcfef4f32f0e06e17b18e6b01486b967595d102f8379
2021-09-23 16:55:47 +02:00
merge-script
95b16e70a8
Merge bitcoin/bitcoin#23072: log: Remove unnecessary timing of Callbacks bench
ab278007991b912299eaf794d87a636423521d27 log: Remove unnecessary timing logs for Callbacks bench (Douglas Chimento)

Pull request description:

  Logging of Callbacks are no longer needed and records times that are not relevant for performance analysis.
  resolves #23071

ACKs for top commit:
  laanwj:
    Thanks. re-ACK ab278007991b912299eaf794d87a636423521d27
  jonatack:
    Code review ACK ab278007991b912299eaf794d87a636423521d27

Tree-SHA512: be1ea780c4db9407a8799065a8824b9d3610abac72af5907809ed62d493d5a54e65735de45ec5fdd0edb85ef21ec6036105abe8ca00093942980f6f92e7fec50
2021-09-23 15:07:16 +02:00
W. J. van der Laan
cdce149050
Merge bitcoin/bitcoin#23045: build: Restrict check for CRC32C intrinsic to aarch64
f2747d1602ec4e1128356b861b2167daf66a845b build: Restrict check for CRC32C intrinsic to aarch64 (W. J. van der Laan)

Pull request description:

  `crc32c`'s hardware accelerated code doesn't handle ARM 32-bit at all, but only 64-bit. Make the check in `configure.ac` check for this architecture explicitly. This change does not affect non-ARM architectures.

  For the release binaries, the current `configure.ac` check happens to work: it enables it on aarch64 but disables it for armhf. However some combination of compiler version and settings can cause this check to succeed on armhf (as reported on IRC). So make the 64-bit platform requirement explicit.

  (details: while the check already explicitly checks the `__crc32d` intrinsic, which strictly doesn't exist on 32-bit ARM, this is not enough! gcc happens to helpfully emulate it:
  > These built-in intrinsics for the ARMv8-A CRC32 extension are available when the -march=armv8-a+crc switch is used "uint32_t __crc32d (uint32_t, uint64_t)" Form of expected instruction(s): Two crc32w r0, r0, r0 instructions for AArch32

  )

ACKs for top commit:
  luke-jr:
    re-tACK f2747d1602ec4e1128356b861b2167daf66a845b
  jarolrod:
    ACK f2747d1602ec4e1128356b861b2167daf66a845b

Tree-SHA512: e5f05f05eeec310ac42685621d86862735586be66dc378db404ec9414ac5aaea7c53d76d76d875b15b11924eee6714076120c07b077183fd7af898704fd81823
2021-09-23 13:39:06 +02:00
Douglas Chimento
ab27800799 log: Remove unnecessary timing logs for Callbacks bench
Logging of Callbacks are no longer needed and records events that are not relevant for performance analysis.
2021-09-23 14:36:16 +03:00
fanquake
86c3b84388
Merge bitcoin/bitcoin#23036: test: use test_framework.p2p P2P_SERVICES constant in functional tests
b69a106bcd8ddefdb810df6ebb3625c430197e04 test: use test_framework.p2p P2P_SERVICES in functional tests (Jon Atack)

Pull request description:

  `P2P_SERVICES` is defined in `test/functional/test_framework/p2p.py`, so we can use it as a single definition for our functional tests. It may also be a tiny bit more efficient to use the constant rather than calculating `NODE_NETWORK | NODE_WITNESS` every time we need it in the tests.

ACKs for top commit:
  laanwj:
    Code review ACK b69a106bcd8ddefdb810df6ebb3625c430197e04
  klementtan:
    crACK b69a106bcd8ddefdb810df6ebb3625c430197e04
  fanquake:
    ACK b69a106bcd8ddefdb810df6ebb3625c430197e04 - didn't look at the formatting changes.

Tree-SHA512: f83e593663a69182986325d9ba2b4b787b87896d6648973f4f802f191a2573201b9e7d7e10e69662ef1965fa63268845726ed1aa5742a2e38dcccf4aebc6a961
2021-09-23 17:13:02 +08:00
fanquake
8bda5e0988
Merge bitcoin/bitcoin#22855: RBF move 3/3: move followups + improve RBF documentation
0ef08f8bed537435f3f9db1e38b7d6f3551fe830 add missing includes in policy/rbf (glozow)
c6abeb76fbb877f3f16d699c73a1828c7da2e6d1 make MAX_BIP125_RBF_SEQUENCE constexpr (glozow)
3cf46f6055f7cd2e5da81e0d29cafc51ad4aafba [doc] improve RBF documentation (glozow)
c78eb8651b0949fefcafb22940512f4ef98d3358 [policy/refactor] pass in relay fee instead of using global (glozow)

Pull request description:

  Followups to #22675 and documentation-only changes intended to clarify the code/logic concerning mempool Replace-by-Fee.

ACKs for top commit:
  jnewbery:
    utACK 0ef08f8bed537435f3f9db1e38b7d6f3551fe830
  fanquake:
    ACK 0ef08f8bed537435f3f9db1e38b7d6f3551fe830

Tree-SHA512: 6797ae758beca0c9673cb00ce85da48e9a4ac5cb5100074ca93e004cdb31d24d91a1a7721b57fc2f619addfeb4950d8caf45fee0f5b7528defbbd121eb4d271f
2021-09-23 16:40:41 +08:00
fanquake
b8cca9cac7
Merge bitcoin/bitcoin#23059: test: Revert "Add missing suppression signed-integer-overflow:addrman.cpp"
fa5e8c1044cb03a46807d6da6fe8d5f45f7e2e12 Revert "test: Add missing suppression signed-integer-overflow:addrman.cpp" (MarcoFalke)

Pull request description:

  Forgot to do this in #22734

ACKs for top commit:
  fanquake:
    ACK fa5e8c1044cb03a46807d6da6fe8d5f45f7e2e12

Tree-SHA512: cabd741b73c828ae3c40df37d80022039e41e571fa196eebcb41e51c582064a1463a3677eb3b0e997ed141b32003802019474d2b739f0b606b4a16da4f585faa
2021-09-23 16:20:54 +08:00
fanquake
3120bceeaf
Merge bitcoin/bitcoin#23054: Use C++11 member initializer in CTxMemPoolEntry
fa08d4cfb1e3447a76093d46edc1c9de5ceee454 Use C++11 member initializer in CTxMemPoolEntry (MarcoFalke)

Pull request description:

  This removes a bunch of boilerplate, makes the code easier to read.
  Also, C++11 member initialization avoids accidental uninitialized
  members.

  Can be reviewed with the git option "--word-diff-regex=." or with "git
  difftool --tool=meld".

ACKs for top commit:
  jnewbery:
    Code review ACK fa08d4cfb1e3447a76093d46edc1c9de5ceee454
  shaavan:
    Code Review ACK fa08d4cfb1e3447a76093d46edc1c9de5ceee454

Tree-SHA512: 2424861002fbcef2a3f01845662c115b973a7a5103f359305b5d9237c055eb003aa7646fc1cb30e6eaf90810d662f94cedc6f90795e30b56680f9c81f631d64b
2021-09-23 16:11:54 +08:00
fanquake
faecb2ee0a
Merge bitcoin/bitcoin#22798: doc: Fix RPC result documentation
fa10fbc6658fb8d87118cd550e4de406dca45f23 doc: Fix RPC result documentation (MarcoFalke)

Pull request description:

  Fix:
  * Incorrectly named fields
  * Add missing ones
  * Add missing optional flag

ACKs for top commit:
  fanquake:
    ACK fa10fbc6658fb8d87118cd550e4de406dca45f23

Tree-SHA512: 2b302e6f7ac8253a55882bc032ddda1932b728abddd12b0adb5fba814b12fb998a67b91e6dd124ebbe0ac16dccdace01332ade01c1dc00a3768647118dd3a2d2
2021-09-23 15:52:22 +08:00
Martin Zumsande
64e1ddd255 log: call LogPrint only once with time data samples
This prevents malformed log entries caused by other threads
logging concurrently.
2021-09-22 22:44:42 +02:00
merge-script
971cad475f
Merge bitcoin/bitcoin#23044: refactor: Remove unused validation includes
fa45a1338adb127d69aee982920e29519bc1fed6 refactor: Remove unused validation includes (MarcoFalke)

Pull request description:

  Unused includes will cause needless recompilation when headers are changed. Also, they pretend there are dependencies that don't exist.

  Fix both by removing them.

ACKs for top commit:
  laanwj:
    Code review ACK fa45a1338adb127d69aee982920e29519bc1fed6
  theStack:
    ACK fa45a1338adb127d69aee982920e29519bc1fed6 ♻️

Tree-SHA512: 69190fd09184b75bce34ce3f315a1817e09ea32779f9ddc2d4790c89b0887b6cebd88aba66fa054c43c9183fc66202a556d982dd7034fc389a75802d8aaac83a
2021-09-22 12:38:13 +02:00
MarcoFalke
fa5e8c1044
Revert "test: Add missing suppression signed-integer-overflow:addrman.cpp"
This reverts commit facb534c37725ca446fd56d781b70ba26508bd2a.
2021-09-22 09:01:06 +02:00
merge-script
51c7d88e67
Merge bitcoin/bitcoin#22790: test: add aarch64-apple-darwin platform entry to get_previous_releases
f6e4db27ceb67157dc13d13f34351cb87fec2be5 test: add aarch64-apple-darwin platform entry to get_previous_releases (Zero-1729)

Pull request description:

  Over the course of reviewing a PR, I had to edit `test/get_previous_releases.py` (after I ran `git clean -xdff`) to run the backwards compatibility tests (e.g. `wallet_upgradewallet`, `feature_backwards_compatibility`, etc.), as currently on master, running the script as indicated in [`test/README.md`](https://github.com/bitcoin/bitcoin/blob/master/test/README.md), for example, on an M1 machine results in the following error, as the `aarch64-apple-darwin*` platform entry is presently not recognised:

  > Output from an M1 machine running macOS v11.5.2

  ```sh
  $ test/get_previous_releases.py -b v0.20.1 v0.19.1 v0.18.1 v0.17.2 v0.16.3 v0.15.2
  Releases directory: releases
  Not sure which binary to download for aarch64-apple-darwin20.6.0
  ```

  As a quick fix, this PR adds the missing `aarch64-apple-darwin*` platform entry. Running the script now results in fetching the old binaries, as expected:

  ```sh
  $ test/get_previous_releases.py -b v0.20.1 v0.19.1 v0.18.1 v0.17.2 v0.16.3 v0.15.2

  Releases directory: releases
  Fetching: https://bitcoincore.org/bin/bitcoin-core-0.20.1/bitcoin-0.20.1-osx64.tar.gz
    % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                   Dload  Upload   Total   Spent    Left  Speed
    0 20.9M    0     0    0     0      0      0 --:--:--  0:00:02 --:--:--     0
    % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                   Dload  Upload   Total   Spent    Left  Speed
  100 20.9M  100 20.9M    0     0   136k      0  0:02:37  0:02:37 --:--:-- 95607
  Checksum matched

  …

  Checksum matched
  ```

  After this patch, the backwards compatibility tests also run successfully, as expected.

  **Note**: I am open to other possible solutions.

  ---

  Steps to reproduce:

  > Ensure you take out the binaries in `releases` if they already exist.

  Try running `test/get_previous_releases.py -b v0.15.2` or similar to fetch the old release binaries.

Top commit has no ACKs.

Tree-SHA512: a238d909b70a61be622234bc49b05d2e91a8acfc5ea348d29f2c8a927fb793cb97365e558571e3f46d6a5650c4f3c6e28fa126c6e56b38e1eb98f7c3e3594d0f
2021-09-22 08:13:06 +02:00
merge-script
a8a272ac32
Merge bitcoin/bitcoin#22734: addrman: Avoid crash on corrupt data, Force Check after deserialize
fa3669f72f69662049b55ad1a482b4a0f9f7ae40 fuzz: Move all addrman fuzz targets to one file (MarcoFalke)
fa7a883f5a219d5f3c2f992b090db4e6c279db12 addrman: Replace assert with throw on corrupt data (MarcoFalke)
fa298971e6890715e2b0b93f2a7f445d32d6622f Refactor: Turn the internal addrman check helper into a forced check (MarcoFalke)
fae5c633dc05a045aaac370b383e4799cb0e0590 move-only: Move CAddrMan::Check to cpp file (MarcoFalke)

Pull request description:

  Assert should only be used for program internal logic errors, not to sanitize external user input.

  The assert was introduced via the debug-only runtime option `-checkaddrman` in commit 803ef70fd9f65ef800567ff9456fac525bc3e3c2, thus won't need a backport.

  Also, it doesn't really make sense to continue when the deserialized addrman doesn't pass the sanity check.

  For example, if `nLastSuccess` is negative, it would  later result in integer overflows. Thus, this patch fixes #22931.

  Also,
  Fixes #22503
  Fixes #22504
  Fixes #22519

  Closes #22498

  Steps to test:

  ```
  mkdir -p /tmp/test_235/regtest/
  echo 'H4sIAAAAAAAAA/u1f+stZmUGYgELgwPRakfBKBgFo2AUjIJRMApGwSgYBaNgFIyCUTBswdyGpFnLjUKjP9e0bvjYusl6b+L2e7Vs2dd6N//Pua0/xQUALJAn93IQAAA=' | base64 --decode | zcat > /tmp/test_235/regtest/peers.dat
  ./src/qt/bitcoin-qt -regtest -datadir=/tmp/test_235/ -checkaddrman=1 -printtoconsole | grep -A2 'Loading P2P addresses'
  ```

  Output before:
  ```
  2021-09-10T11:28:37Z init message: Loading P2P addresses…
  2021-09-10T11:28:37Z ADDRMAN CONSISTENCY CHECK FAILED!!! err=-16
  bitcoin-qt: addrman.cpp:765: void CAddrMan::Check() const: Assertion `false' failed.

  (program crashes)
  ```

  Output after:
  ```
  2021-09-10T11:26:00Z init message: Loading P2P addresses…
  2021-09-10T11:26:00Z Error: Invalid or corrupt peers.dat (Corrupt data. Consistency check failed with code -16: iostream error). If you believe this is a bug, please report it to https://github.com/bitcoin/bitcoin/issues. As a workaround, you can move the file ("/tmp/test_235/regtest/peers.dat") out of the way (rename, move, or delete) to have a new one created on the next start.

  (program exits)
  ```

ACKs for top commit:
  naumenkogs:
    ACK fa3669f72f69662049b55ad1a482b4a0f9f7ae40
  jnewbery:
    Code review ACK fa3669f72f69662049b55ad1a482b4a0f9f7ae40
  vasild:
    ACK fa3669f72f69662049b55ad1a482b4a0f9f7ae40

Tree-SHA512: 687e4a4765bbc66495152fa7a49d28ee84b405dc5370ba87b4016b5593e45f54c4ce5cae579e4d433e0e082d20fc263969fa602679c911accef0adb2d6213bd6
2021-09-21 18:21:00 +02:00
MarcoFalke
fa08d4cfb1
Use C++11 member initializer in CTxMemPoolEntry
This removes a bunch of boilerplate, makes the code easier to read.
Also, C++11 member initialization avoids accidental uninitialized
members.

Can be reviewed with the git option "--word-diff-regex=." or with "git
difftool --tool=meld".
2021-09-21 16:04:27 +02:00
merge-script
ae674a0198
Merge bitcoin/bitcoin#22998: test: use MiniWallet for make_utxo helper in feature_rbf.py
f680d27155374de658d40db0ba40460919aa1ba2 test: use MiniWallet for make_utxo helper in feature_rbf.py (Sebastian Falbesoner)
0f275246027266fa256d0a09251bb2c88d9bd72f test: scale amounts in test_doublespend_tree down by factor 10 (Sebastian Falbesoner)
d1e2481274edf2ac14747be633d86ecd46814ef4 test: scale amounts in test_doublespend_chain down by factor 10 (Sebastian Falbesoner)

Pull request description:

  This PR aims to further increase MiniWallet usage in the functional test feature_rbf.py by using it in the `make_utxo(...)` helper, which is the only part that needs a wallet for most sub-tests. In order to do that, the amounts for the utxos have to be scaled down in two sub-tests first (`test_doublespend_chain` and `test_doublespend_tree`, see first two commits), since we need amounts passed to `make_utxo` than can be funded by only one input. For creating UTXOs with a value of 50 BTC, we'd need to implement a method for consolidating multiple utxos into one first, which seems to be overkill.

  Note that after this PR's change, there is only one sub-test left (`test_rpc`) that needs the wallet compiled into bitcoind.

ACKs for top commit:
  MarcoFalke:
    review ACK f680d27155374de658d40db0ba40460919aa1ba2 🦐

Tree-SHA512: 46c8c245086a9e79855c4ede2f8f412333cf2658136805196b203b3567c89398d77fcb80715c0bb72fdc84331cc67544b2fdc259193a3adcb2fc36e147c26fce
2021-09-21 15:20:07 +02:00
W. J. van der Laan
f2747d1602 build: Restrict check for CRC32C intrinsic to aarch64
`crc32c`'s hardware accelerated code doesn't handle ARM 32-bit at all.
Make the check in `configure.ac` check for this architecture explicitly.

For the release binaries, the current `configure.ac` check happens
to work: it enables it on aarch64 but disables it for armhf. However
some combination of compiler version and settings might ostensibly cause
this check to succeed on armhf (as reported on IRC). So make the 64-bit
platform requirement explicit.
2021-09-21 12:36:46 +02:00
MarcoFalke
fa10fbc665
doc: Fix RPC result documentation 2021-09-21 11:13:29 +02:00
MarcoFalke
fa3669f72f
fuzz: Move all addrman fuzz targets to one file
Can be reviewed with --color-moved=dimmed-zebra
2021-09-21 10:09:56 +02:00
MarcoFalke
fa7a883f5a
addrman: Replace assert with throw on corrupt data
Assert should only be used for program internal logic errors, not to
sanitize external user input.
2021-09-21 10:09:45 +02:00
MarcoFalke
fa298971e6
Refactor: Turn the internal addrman check helper into a forced check 2021-09-21 10:07:48 +02:00
MarcoFalke
fae5c633dc
move-only: Move CAddrMan::Check to cpp file
This speeds up compilation of the whole program because the included
header file is smaller.

Can be reviewed with --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space
2021-09-21 10:07:38 +02:00
fanquake
8f022a59b8
Merge bitcoin/bitcoin#22993: build: set OSX_MIN_VERSION to 10.15
a43b8e955558483d8893996cc3a67bc74cbaf358 build: set OSX_MIN_VERSION to 10.15 (fanquake)

Pull request description:

  Taken out of #20744, as splitting up some of the build changes was mentioned [here](https://github.com/bitcoin/bitcoin/pull/22937#discussion_r707303172).

  This is required to use `std::filesystem` on macOS, as support for it only landed in the libc++.dylib shipped with 10.15. So if we want to move to using `std::filesystem` for `23.0`, this bump is required.

  See also: https://developer.apple.com/documentation/xcode-release-notes/xcode-11-release-notes

  > Clang now supports the C++17 \<filesystem\> library for iOS 13, macOS 10.15, watchOS 6, and tvOS 13.

  macOS 10.15 was released in October 2019. macOS OS's seem to have a life of about 3 years, so it's possible that 10.14 will become officially unsupported by the end of 2021 and prior to the release of 23.0.

  Guix builds:
  ```bash
  bash-5.1# find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
  abc8b749be65f1339dcdf44bd1ed6ade2533b8e3b5030ad1dde0ae0cede78136  guix-build-a43b8e955558/output/dist-archive/bitcoin-a43b8e955558.tar.gz
  1edcc301eb4c02f3baa379beb8d4c78e661abc24a293813bc9d900cf7255b790  guix-build-a43b8e955558/output/x86_64-apple-darwin19/SHA256SUMS.part
  e9dbb5594a664519da778dde9ed861c3f0f631525672e17a67eeda599f16ff44  guix-build-a43b8e955558/output/x86_64-apple-darwin19/bitcoin-a43b8e955558-osx-unsigned.dmg
  11b23a17c630dddc7594c25625eea3de42db50f355733b9ce9ade2d8eba3a8f3  guix-build-a43b8e955558/output/x86_64-apple-darwin19/bitcoin-a43b8e955558-osx-unsigned.tar.gz
  257ba64a327927f94d9aa0a68da3a2695cf880b3ed1a0113c5a966dcc426eb5e  guix-build-a43b8e955558/output/x86_64-apple-darwin19/bitcoin-a43b8e955558-osx64.tar.gz
  ```

ACKs for top commit:
  hebasto:
    ACK a43b8e955558483d8893996cc3a67bc74cbaf358
  jarolrod:
    ACK a43b8e9

Tree-SHA512: 9ac77be7cb56c068578860a3b2b8b7487c9e18b71b14aedd77a9c663f5d4bb19756d551770c02ddd12f1797beea5757b261588e7b67fb53509bb998ee8022369
2021-09-21 15:37:12 +08:00
merge-script
223ad2fd0d
Merge bitcoin/bitcoin#22831: test: add addpeeraddress "tried", test addrman checks on restart with asmap
cdaab90662a54e331de0e49a89596bbb94a8ac45 Add test for addrman consistency check on restart with asmap (Jon Atack)
869f136816c6900ce84bc4b5a9c93c0deab85193 Add test for rpc addpeeraddress with "tried" argument (Jon Atack)
ef242f52137f2a79a739447251d7759bd4705be0 Allow passing "tried" to rpc addpeeraddress to call CAddrMan::Good() (Jon Atack)

Pull request description:

  This pull adds a `tried` argument to RPC addpeeraddress and a regression test for the recent addrman/asmap changes and issue.

  PR #22697 introduced a reproducible bug in commit 181a1207 that fails addrman consistency checks and causes it to significantly lose peer entries when the `-asmap` configuration option is used.

  The issue occurs upon bitcoind restart due to an initialization order change in `src/init.cpp` in that commit, whereby CAddrman asmap is set after deserializing `peers.dat`, rather than before.

  Issue reported on the `#bitcoin-core-dev` IRC channel starting at https://www.erisian.com.au/bitcoin-core-dev/log-2021-08-23.html#l-263.

  ```
  addrman lost 22813 new and 2 tried addresses due to collisions or invalid addresses
  ADDRMAN CONSISTENCY CHECK FAILED!!! err=-17 bitcoind: ./addrman.h:707: void CAddrMan::Check() const: Assertion `false' failed. Aborted
  ```

  How to reproduce:

  - `git checkout 181a1207`, build, and launch bitcoind with the `-asmap` and `-checkaddrman=1` configuration options enabled
  - restart bitcoind
  - bitcoind aborts on the second call to the addrman consistency checks in `CAddrMan::Check()`

  How to test this pull:

  - `git checkout 181a1207`, cherry pick the first commit of this branch, build, git checkout this branch, run `test/functional/rpc_net.py`, which should pass, and then run `test/functional/feature_asmap.py`, which should fail with the following output:

  ```
  AssertionError: Unexpected stderr bitcoind: ./addrman.h:739: void CAddrMan::Check() const: Assertion `false' failed.
  ```

ACKs for top commit:
  jnewbery:
    utACK cdaab90662a54e331de0e49a89596bbb94a8ac45
  mzumsande:
    re-ACK cdaab90662a54e331de0e49a89596bbb94a8ac45 (based on code review of diff to d586817)
  vasild:
    ACK cdaab90662a54e331de0e49a89596bbb94a8ac45

Tree-SHA512: 0251a18fea629b62486fc907d7ab0e96c6df6fadb9e4d62cff018bc681afb6ac31e0e7258809c0a88f91e4a36c4fb0b16ed294ce47ef30585217de89c3342399
2021-09-21 09:34:28 +02:00
merge-script
0c1a39390f
Merge bitcoin/bitcoin#23041: test: Add addrman deserialization error tests
faa81f948663a7155335986b028fb2ce19049a79 test: Add addrman deserialization error tests (MarcoFalke)

Pull request description:

  Add missing test coverage

ACKs for top commit:
  jonatack:
    Light code review ACK faa81f948663a7155335986b028fb2ce19049a79 and ran the test

Tree-SHA512: 8b254ba912c83473125faaf7df02a33a99840b40460bdce1486991a01de9ba6371c053354318f09b69fdc18c823bca3f2f7d341db0f8950e22d8435acbaa9cf5
2021-09-21 09:28:45 +02:00
merge-script
89447a63b9
Merge bitcoin/bitcoin#23017: test: Replace MiniWallet scan_blocks with rescan_utxos
fa7e3f1fc114056967963a4ad4863a56e406c57e test: Replace MiniWallet scan_blocks with rescan_utxos (MarcoFalke)

Pull request description:

  This avoids having to fiddle with the `start` and `num` parameters and instead use the `scantxoutset` RPC functionality via `rescan_utxos`.

ACKs for top commit:
  Shubhankar-Gambhir:
    ACK fa7e3f1, all tests were succesfull
  theStack:
    re-ACK fa7e3f1fc114056967963a4ad4863a56e406c57e

Tree-SHA512: 6f47d2acac9f180b2b0f8f04797e74ecb1fc180f6b164c67813a3a1f97acea54baed74e5e0a3512e3babf76b105c09e1ba4cad818c83c7cb2beb7377b4c96954
2021-09-21 09:22:30 +02:00
merge-script
eb180d807a
Merge bitcoin/bitcoin#23050: log: change an incorrect fee to fee rate, and vice-versa
c17f554fcc63e9e1f6ba64750df475d8a8d11f2e Fix BlockAssembler::AddToBlock, CTxMemPool::PrioritiseTransaction logging (Jon Atack)

Pull request description:

  This is a tale of two fees, er, fee rates... indeed, one is misdescribed as a fee, and the other is incorrectly called a fee rate.

  From this review discussion: https://github.com/bitcoin/bitcoin/pull/22689#discussion_r695866211 (thanks to John Newbery).

ACKs for top commit:
  laanwj:
    Code review ACK c17f554fcc63e9e1f6ba64750df475d8a8d11f2e

Tree-SHA512: 3d9df3209a72562c5f9bbf815923d5b089d04491b8d19caa2c04158c501b47ef01e47f1c32d89adcbaf3c6357329507f65b4bb2963214c3451bbfa61ac812530
2021-09-21 09:18:01 +02:00
fanquake
1260b7e483
Merge bitcoin/bitcoin#23001: doc: Enable TLS in links in documentation
9bdda50151dd808cbad094d457bf0ed7939a7c87 Enable TLS in links in documentation (Jeremy Rand)

Pull request description:

  This PR enables TLS in several documentation links, which improves security.

ACKs for top commit:
  fanquake:
    ACK 9bdda50151dd808cbad094d457bf0ed7939a7c87

Tree-SHA512: 9d04d8771a9daf3c3b9914ff324e2eabfdf3ff5ae7f7dc92b84a1f3527010ceb860e73873a8f24d6051763eb472d9ea324ccbd6129a40318a520ca88c05f0586
2021-09-21 14:47:05 +08:00
merge-script
e4ae0008d8
Merge bitcoin/bitcoin#23014: ci: Bump distro version, disable feature_bind_extra for two configurations
fa660de2ac28628e22ee0c18e04e520a41b3584e ci: Update valgrind config (MarcoFalke)
fad5dbc13c26901928e5b71bff047fe273f72a45 ci: Update s390x config (MarcoFalke)

Pull request description:

  (See commit messages)

ACKs for top commit:
  fanquake:
    ACK fa660de2ac28628e22ee0c18e04e520a41b3584e

Tree-SHA512: b5bf7c89042d5d8f49a92c59b934185447ae42bd055ccb1c6edb98fe104a35eb2d6c26d810bec9f52d545d09599f3d33fc8606fbfb793862610d276b2a4bac6c
2021-09-21 08:42:08 +02:00
fanquake
133446f3dc
Merge bitcoin/bitcoin#23043: ci: Set --nocleanup for Windows functional tests
fac67b393ba4ff2f6698955b5f35433dec64ae51 ci: Set --nocleanup for Windows functional tests (MarcoFalke)

Pull request description:

  Example: https://cirrus-ci.com/task/6292402450202624

ACKs for top commit:
  hebasto:
    ACK fac67b393ba4ff2f6698955b5f35433dec64ae51

Tree-SHA512: b8da82d1492f66b43e4afc525805a42709a7b13e64320681801e3e5b546da3d82933f5248464900620aa89eac4629f0b21cde623caf45950f1341979b47fa621
2021-09-21 11:45:46 +08:00
Jon Atack
c17f554fcc
Fix BlockAssembler::AddToBlock, CTxMemPool::PrioritiseTransaction logging 2021-09-20 22:40:15 +02:00
W. J. van der Laan
7f7bd3111c
Merge bitcoin/bitcoin#22974: addrman: Improve performance of Good
57ce20307e604530f78ef4f0f8d9fb94f80ca81b fuzz: allow lower number of sources (Martin Zumsande)
acf656d540a82e6fc30421590305cfe295eabbb5 fuzz: Use public interface to fill addrman tried tables (Martin Zumsande)
eb2e113df13c7b1ede279878f5cbad877af49f8e addrman: Improve performance of Good (Martin Zumsande)

Pull request description:

  Currently, `CAddrman::Good()` is rather slow because the process of moving an addr from new to tried involves looping over the new tables twice:
  1) In `Good_()`, there is a loop searching for a new bucket the addr is currently in, but this information is never used except for aborting if it is not found anywhere (since [this commit](e6b343d880 (diff-49d1faa58beca1ee1509a247e0331bb91f8604e30a483a7b2dea813e6cea02e2R263)) it is no longer passed to `MakeTried`)
  This is unnecessary because in a non-corrupted addrman, an address that is not in New must be either in Tried or not at all in addrman, both cases in which we'd return early in `Good_()` and never get to this point.
  I removed this loop (and left a check for `nRefCount` as a belt-and-suspenders check).

  2) In `MakeTried()`, which is called from `Good_()`, another loop removes all instances of this address from new. This can be spedup by stopping the search at  `nRefCount==0`. Further reductions in `nRefCount` would only lead to an assert anyway.
  Moreover, the search can be started at the bucket determined by the source of the addr for which `Good` was called, so that if it is present just once in New, no further buckets need to be checked.

  While calls to `Good()` are not that frequent normally, the performance gain is clearly seen in the fuzz target `addman_serdeser`, where, because of the slowness in creating a decently filled addrman, a shortcut was created that would directly populate the tried tables by reaching into addrman's internals, bypassing `Good()` (#21129).
  I removed this workaround in the second commit: Using `Good()` is still slower by a factor of 2 (down from a factor of ~60 before), but I think that this compensated by the advantages of not having to reach into the internal structures of addrman  (see https://github.com/jnewbery/bitcoin/pull/18#issuecomment-775218676).

  [Edit]: For benchmark results see https://github.com/bitcoin/bitcoin/pull/22974#issuecomment-919435266 and https://github.com/bitcoin/bitcoin/pull/22974#issuecomment-920445700 - the benchmark `AddrManGood` shows a significant speedup by a factor >100.

ACKs for top commit:
  naumenkogs:
    ACK 57ce20307e604530f78ef4f0f8d9fb94f80ca81b
  jnewbery:
    ACK 57ce20307e
  laanwj:
    Code review ACK 57ce20307e604530f78ef4f0f8d9fb94f80ca81b
  theStack:
    ACK 57ce20307e604530f78ef4f0f8d9fb94f80ca81b
  vasild:
    ACK 57ce20307e604530f78ef4f0f8d9fb94f80ca81b

Tree-SHA512: fb6dfc198f2e28bdbb41cef9709828f22d83b4be0e640a3155ca42e771b6f58466de1468f54d773e794f780a79113f9f7d522032e87fdd75bdc4d99330445198
2021-09-20 19:47:55 +02:00
W. J. van der Laan
488e745560
Merge bitcoin/bitcoin#12677: RPC: Add ancestor{count,size,fees} to listunspent output
6cb60f3e6d652ffa4cf570426a7cf1f690d15c45 doc/release-notes: Add new listunspent fields (Luke Dashjr)
0be2f17ef5649c2d77efbbbdd9222332b2ebf0d2 QA: Add tests for listunspent ancestor{count,size,fees} to mempool_packages (Luke Dashjr)
6966e80f453c46d5d0a923118205f19ac2f4e336 RPC: Add ancestor{count,size,fees} to listunspent output (Luke Dashjr)
3f77dfdaf0f0bfe0c4662a616d6943f31bdd5bf4 Expose ancestorsize and ancestorfees via getTransactionAncestry (Luke Dashjr)

Pull request description:

  Requested by a user

ACKs for top commit:
  prayank23:
    reACK 6cb60f3e6d
  fjahr:
    Code review re-ACK 6cb60f3e6d652ffa4cf570426a7cf1f690d15c45
  kiminuo:
    ACK [6cb60f3](6cb60f3e6d)
  achow101:
    Code Review ACK 6cb60f3e6d652ffa4cf570426a7cf1f690d15c45
  naumenkogs:
    ACK 6cb60f3e6d652ffa4cf570426a7cf1f690d15c45
  darosior:
    utACK 6cb60f3e6d652ffa4cf570426a7cf1f690d15c45

Tree-SHA512: 5d16e5799558691e5853ab7ea2cc85514cb45da3ce69134d855c71845beef32ec6af5ab28d4462683e9800c8ea126f162773a9d3d5660edac08fd8edbfeda173
2021-09-20 19:25:43 +02:00
W. J. van der Laan
d809d8bf12
Merge bitcoin/bitcoin#22959: cli: Display all proxies in -getinfo
7c3712fa32b79c1c3abc73ef89990c452e22ce2b cli: Display all proxies in -getinfo (klementtan)

Pull request description:

  **Changes**: Display all proxies in `-getinfo`

  **Motivation**:

  * Currently `-getinfo` only return the proxy of the first network in `getnetworkinfo`.
  * This PR will display all unique proxies in `getnetworkinfo` as suggested in https://github.com/bitcoin/bitcoin/issues/17314#issue-514543978
       >List all proxies, at least if they're different from the IPv4 one

  ![image](https://user-images.githubusercontent.com/49265907/133991832-a1f38b36-2975-4ce2-a427-e4ffab23383e.png)

  **Testing**:

  You can verify this change by starting bitcoind with
  ```shell
  ./src/bitcoind -signet --proxy=127.0.0.1:9050 --i2psam=127.0.0.1:7656
  ```

  Execute `-getinfo`
  ```shell
  ./src/bitcoin-cli -signet -getinfo
  ```

ACKs for top commit:
  laanwj:
    Tested ACK 7c3712fa32b79c1c3abc73ef89990c452e22ce2b
  prayank23:
    utACK 7c3712fa32

Tree-SHA512: 9eae97866220227f30ca4585f52799fa66fc1135047d869c4aabe598aee1a9414cb9e1c4a8d19165e52d65005f3c6d4bcc37463ace0ddb44389dfbcd4ca74095
2021-09-20 17:48:31 +02:00
glozow
0ef08f8bed add missing includes in policy/rbf 2021-09-20 13:34:48 +01:00
glozow
c6abeb76fb make MAX_BIP125_RBF_SEQUENCE constexpr 2021-09-20 13:32:28 +01:00
klementtan
7c3712fa32
cli: Display all proxies in -getinfo 2021-09-20 18:52:04 +08:00
MarcoFalke
fa45a1338a
refactor: Remove unused validation includes 2021-09-20 12:16:20 +02:00
MarcoFalke
fac67b393b
ci: Set --nocleanup for Windows functional tests
This avoids intermittent issues in the CI Task when a test passes
successfully. For example:

PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: 'C:\\Users\\ContainerAdministrator\\AppData\\Local\\Temp\\test_runner_₿_🏃_20210920_072037\\feature_versionbits_warning_89\\alert.txt'
2021-09-20 09:41:41 +02:00
MarcoFalke
faa81f9486
test: Add addrman deserialization error tests 2021-09-20 09:07:44 +02:00
MarcoFalke
fa7e3f1fc1
test: Replace MiniWallet scan_blocks with rescan_utxos 2021-09-20 08:31:04 +02:00
merge-script
226731ac11
Merge bitcoin/bitcoin#23037: test: fix confusing off-by-one nValue in feature_coinstatsindex.py
ebe49b5b7cda0f9e8dc4bb4795da0ca44a97aa1d test: fix confusing off-by-one nValue in feature_coinstatsindex.py (Sebastian Falbesoner)

Pull request description:

  Due to evil floating-point arithmetic, the creation of one of the transaction outputs in feature_coinstatsindex.py leads to it's nValue being off by one satoshi: the Python expression `int(21.99 * COIN)` doesn't yield 2199000000 as expected, but 2198999999.

  This makes the test more confusing than necessary (w.r.t. the expected `gettxoutsetinfo` values), and could also cause problems if the value is ever changed. Fix by using a `Decimal` type for specifying the value in BTC, rather than using a bare floating-point.

ACKs for top commit:
  MarcoFalke:
    cr ACK ebe49b5b7cda0f9e8dc4bb4795da0ca44a97aa1d

Tree-SHA512: c74c51dbf99818f3d1c881873e0c053a649e4fed9b36767ff971dd3a48bff7122afea4e07cc9925236570368b45579f63e443701f2aaef838a0fafdbe986dfd4
2021-09-20 07:59:49 +02:00
fanquake
2560b6862b
Merge bitcoin/bitcoin#23007: doc: remove WSL install instructions and point to upstream
f58f697c98ebe0660cb7c5c2048ae5b2acb35a15 doc: remove WSL install instructions and point to upstream (fanquake)

Pull request description:

  There's not really any need for us to have to replicate (ever-changing) instructions for installing an operating system in our build documentation.

ACKs for top commit:
  laanwj:
    ACK f58f697c98ebe0660cb7c5c2048ae5b2acb35a15
  hebasto:
    ACK f58f697c98ebe0660cb7c5c2048ae5b2acb35a15

Tree-SHA512: 3ca90cf7696909503ca500756c718f019c410b1e24c139e7e9fdc8726d1c37a7b6c869fdac1cb7ab8c8065bc49c808778ca5985f54d4a698c162866d00397d48
2021-09-20 11:43:28 +08:00
Sebastian Falbesoner
ebe49b5b7c test: fix confusing off-by-one nValue in feature_coinstatsindex.py
Due to evil floating-point arithmetic, the creation of one of the
transaction outputs in feature_coinstatsindex.py leads to it's nValue
being off by one satoshi: the Python expression `int(21.99 * COIN)`
doesn't yield 2199000000 as expected, but 2198999999.

This makes the test more confusing than necessary (w.r.t. the expected
`gettxoutsetinfo` values), and could also cause problems if the value
is ever changed. Fix by using a `Decimal` type for specifying the
value in BTC, rather than using a bare floating-point.
2021-09-19 21:23:24 +02:00
Jon Atack
b69a106bcd
test: use test_framework.p2p P2P_SERVICES in functional tests 2021-09-19 14:20:48 +02:00
fanquake
de2af19dc8
Merge bitcoin/bitcoin#22987: qa: Fix "RuntimeError: Event loop is closed" on Windows
357f0c723308b296c6250946c26cf476d9ecfda2 ci: Enable more functional tests on Windows MSVC task (Hennadii Stepanov)
f55932678facea2c36bc0708994dbef327a7df09 qa: Fix "RuntimeError: Event loop is closed" on Windows (Hennadii Stepanov)

Pull request description:

  On master (2161a058552ac938f2079b311a2d12f5d1772d01), running functional tests that use the P2P interface ends with an error:
  ```
  RuntimeError: Event loop is closed
  ```

  This PR fixes this bug, and enables more functional tests on Windows MSVC CI task.

  More details about bugfix:
  - [What’s New In Python 3.7](https://docs.python.org/3/whatsnew/3.7.html#asyncio)
  - https://bugs.python.org/issue33792
  - actual [change](https://docs.python.org/3.8/library/asyncio-policy.html#asyncio.WindowsSelectorEventLoopPolicy) done in Python 3.8

  Excluded tests, that are listed in the `EXCLUDE_TESTS` environment variable, need more thorough investigation to be enabled.

ACKs for top commit:
  MarcoFalke:
    review ACK 357f0c723308b296c6250946c26cf476d9ecfda2 🌆

Tree-SHA512: d0ba85be81d55c934959ce7402a9c726598125e9751a1de179d16759d0e8b8a915de879c3a62c12d3564c5e0d9649ebd86963744449626efaa42d9eaa99ad3d0
2021-09-18 16:49:19 +08:00
merge-script
e69cbac628
Merge bitcoin/bitcoin#22896: refactor: net: avoid duplicate map lookups to mapLocalHost
330d3aa1a2c740dfa31bed3a6ed6b5f88e5426ad refactor: net: avoid duplicate map lookups to `mapLocalHost` (Sebastian Falbesoner)

Pull request description:

  This simple refactoring PR aims to avoid duplicate lookups to `mapLocalHost`: instead of calling `count()` (to first find out whether a key is in the map) and then `operator[]` (to get the value to the passed key, or default-construct one if not found), use either
  * `find()` and dereference the returned iterator (for simple lookups), see https://www.cplusplus.com/reference/map/map/find/
  * `emplace()` and use the returned <iterator, inserted> pair (for lookups where a new element should be inserted if the key isn't found), see https://www.cplusplus.com/reference/map/map/emplace/

ACKs for top commit:
  naumenkogs:
    ACK 330d3aa1a2c740dfa31bed3a6ed6b5f88e5426ad
  jonatack:
    Code review ACK 330d3aa1a2c740dfa31bed3a6ed6b5f88e5426ad plus rebase to master + debug build

Tree-SHA512: d13da6a927ff561eee8ac6b093bf3586dfe31d6c94173a5a6d8f3698e0ee224fb394d3635155d5141c165da59d2c2c37260122eb4f2e8bcda3e8a29b901d213e
2021-09-17 14:25:10 +02:00
MarcoFalke
fa660de2ac
ci: Update valgrind config
* Set missing DOCKER_NAME_TAG
* Update TEST_RUNNER_EXTRA
2021-09-17 12:54:48 +02:00