Commit Graph

3970 Commits

Author SHA1 Message Date
W. J. van der Laan
2d8e0c0c3c
Merge bitcoin/bitcoin#20457: util: Make Parse{Int,UInt}{32,64} use locale independent std::from_chars(…) (C++17) instead of locale dependent strto{l,ll,ul,ull}
4747db8761 util: Introduce ToIntegral<T>(const std::string&) for locale independent parsing using std::from_chars(…) (C++17) (practicalswift)

Pull request description:

  Make `Parse{Int,UInt}{32,64}` use locale independent `std::from_chars(…)` (C++17) instead of locale dependent `strto{l,ll,ul,ull}`.

  [About `std::from_chars`](https://en.cppreference.com/w/cpp/utility/from_chars): _"Unlike other parsing functions in C++ and C libraries, `std::from_chars` is locale-independent, non-allocating, and non-throwing."_

ACKs for top commit:
  laanwj:
    Code review ACK 4747db8761

Tree-SHA512: 40f2cd582bc19ddcf2c498eca3379167619eff6aa047bbac2f73b8fd8ecaefe5947c66700a189f83848751f9f8c05645e83afd4a44a1679062aee5440dba880a
2021-09-30 15:14:58 +02:00
W. J. van der Laan
1cf7fb9fd6
Merge bitcoin/bitcoin#23104: log: Avoid breaking single log lines over multiple lines in the log file
2222c04e1b log: Adjust coin selection log string (MarcoFalke)
fa6c1e850f test: Fix typos in tests (MarcoFalke)
faeae2980f log: Avoid broken DEBUG_LOCKORDER log (MarcoFalke)
faffaa85cd log: Avoid broken SELECTCOINS log (MarcoFalke)

Pull request description:

  Follow up to commit d8b4b3077f

ACKs for top commit:
  laanwj:
    re-ACK 2222c04e1b
  practicalswift:
    cr ACK 2222c04e1b

Tree-SHA512: e0daf76815a1b7c4898ceffedeaf7ede093223abf709874f9a0d78c8e41551c14e8b56d055c8fdf06ec698df64e67dfc168bbd8716131b23648d1d1294fa6636
2021-09-30 14:42:11 +02:00
fanquake
69a66dcd0d
Merge bitcoin/bitcoin#23126: doc: update developer docs for subtree renaming
2b90eae33c doc: update developer docs for subtree renaming (fanquake)

Pull request description:

  Update the developer docs after the [recent subtree renaming](https://github.com/bitcoin/bitcoin/pull/22646#issuecomment-921154730).

ACKs for top commit:
  hebasto:
    ACK 2b90eae33c

Tree-SHA512: ed0eec8db888e60595c07f4fad0a506673e4b10345fb2dd6d1a98d785da22bddf1fe8896aa52fd67f5e1688e3c91c6b642739e08646f1b920f50f0d35037d961
2021-09-30 08:52:36 +08:00
MarcoFalke
fa6c1e850f
test: Fix typos in tests 2021-09-29 18:47:45 +02:00
MarcoFalke
d648bbb0a7
Merge bitcoin/bitcoin#23124: Fix feature_segwit.py failure due to witness
b207971465 Fix feature_segwit failure due to witness (Samuel Dobson)

Pull request description:

  Fixes #23116

  The failure is due to sometimes spending segwit outputs, which add an additional 1 sigop in the witness, added to the 2 (*4) in the outputs.

ACKs for top commit:
  brunoerg:
    tACK b207971465

Tree-SHA512: 7e3c09d162e6941a3028514e95ae3aab2a3f858e2f056229c5faff5e37ece897dc63e3f8c0e07fad9fc4621b74a6dee5ff777e910b2b009a41bb6e72a09217bd
2021-09-29 13:36:19 +02:00
MarcoFalke
33e31f8df9
Merge bitcoin/bitcoin#23079: test: use MiniWallet for p2p_filter.py
cfdb6baa22 test: use MiniWallet for p2p_filter.py (Sebastian Falbesoner)
6fc2cd3f09 test: introduce helper to create random P2WPKH scriptPubKeys (Sebastian Falbesoner)
aa26797f69 test: MiniWallet: add `send_to` method to create arbitrary txouts (Sebastian Falbesoner)

Pull request description:

  This PR enables one more of the non-wallet functional tests (p2p_filter.py) to be run even with the Bitcoin Core wallet disabled by using the MiniWallet instead, as proposed in #20078.
  For this purpose, a MiniWallet method `send_to` is introduced first, which allows to create arbitrary outputs (scriptPubKey/amount). Note that the implementation for this is already present in feature_rbf.py (recently added in PR #22998), i.e. it is simply moved to the MiniWallet interface.

ACKs for top commit:
  laanwj:
    Code review ACK cfdb6baa22

Tree-SHA512: 13b063631f0d7af065b7757cfe8b47c9be6cb9850ac5db2968a2bba4f5a18cdc9f89173a9b03971545356225082042f5fdbe49d3036027d18e8b7eb042d04f5e
2021-09-29 09:13:07 +02:00
fanquake
2b90eae33c
doc: update developer docs for subtree renaming 2021-09-29 15:12:12 +08:00
MarcoFalke
7d4ea7c7ef
Merge bitcoin/bitcoin#23120: test: Remove unused and confusing main parameter from script_util
fa54efda9b test: pep-8 touched test (MarcoFalke)
fa46768059 test: Remove unused and confusing main parameter from script_util (MarcoFalke)

Pull request description:

ACKs for top commit:
  fanquake:
    ACK fa54efda9b

Tree-SHA512: 124e888e16c92bb24ab4d6f68a768d295500a48f8c6c0b4f069493effcd761f80be78dd93b31edbb529ebe4c8aaca764aaff48d1e3b23659dde722981dc787a5
2021-09-29 08:08:25 +02:00
Samuel Dobson
b207971465 Fix feature_segwit failure due to witness 2021-09-29 17:41:35 +13:00
Samuel Dobson
6a5381a06b
Merge bitcoin/bitcoin#20591: wallet, bugfix: fix ComputeTimeSmart function during rescanning process.
240ea294d5 doc: update doxygen documention of ComputeTimeSmart() and AddToWalletIfInvolvingMe() regarding rescanning_old_block parameter (BitcoinTsunami)
d6eb39af21 test: add functional test to check transaction time determination during block rescanning (BitcoinTsunami)
07b44f16e7 wallet: fix ComputeTimeSmart algorithm to use blocktime during old block rescanning (BitcoinTsunami)

Pull request description:

  The function ComputeTimeSmart in wallet.cpp assume that transaction are discovered in the right order.
  Moreover the 'smarttime' determination algorithm is coded with realtime scenario in mind and not rescanning of old block.

  The functional test demonstrate that if the user import a wallet, then rescan only recent history, and then rescan the entire history, the older transaction discovered would have an incorrect time determination.
  In the context of rescanning old block, the only time value that as a meaning is the blocktime.

  That's why I've fixed the problem with a simple separation between rescanning of old block and realtime time determination. The fix is written to have no impact on every realtime scenario and only impact the behaviour during a rescanning process.
  This PR Fixes #20181.

  To be fair, I don't think that this bug could be triggered with the wallet GUI, because it always proceed with a proper rescan.
  But RPC API provide the possibility to trigger it. I've discovered it, because Specter desktop v0.10.0 was impacted. (https://github.com/cryptoadvance/specter-desktop/issues/680).

ACKs for top commit:
  jonatack:
    ACK 240ea294d5 per `git diff b92d552 240ea29`, re-verified rebase to latest master + debug build clean + new test passes on the branch and fails on master, only change since my review a few hours ago is incorporation of latest review suggestions
  meshcollider:
    re-utACK 240ea294d5

Tree-SHA512: 514b02e41d011ddfa325f5e8080b93800e1ea4ed5853fa420670a6ac700e8b463000dbea65f8ced8565cfb950c7f51b69154034dcb111e67aca3b964a0061494
2021-09-29 11:18:23 +13:00
Samuel Dobson
b55232a337
Merge bitcoin/bitcoin#22722: rpc: update estimatesmartfee to return max of CBlockPolicyEstimator::estimateSmartFee, mempoolMinFee and minRelayTxFee
ea31caf6b4 update estimatesmartfee rpc to return max of estimateSmartFee, mempoolMinFee and minRelayTxFee. (pranabp-bit)

Pull request description:

  This PR is in response to the issue [#19699](https://github.com/bitcoin/bitcoin/issues/19699).

  Based on the discussion in the comments of PR [#22673](https://github.com/bitcoin/bitcoin/pull/22673) changes have been made in the `estimatesmartfee` itself such that it takes into account `mempoolMinFee` and `relayMinFee` . Hence it provides a fee estimate that is most likely to be paid by the user in an actual transaction, preventing issues such as [#16072](https://github.com/bitcoin/bitcoin/issues/16072).

  The test file test/functional/feature_fee_estimation.py has also been updated to check this functionality.

ACKs for top commit:
  meshcollider:
    re-utACK ea31caf6b4

Tree-SHA512: 8f36153a07cbd552c5c13d11d9c6e987a7a555ea4cc83f2573c0c92dd97c706d90c30a7248671437c2f3a836d3272f8fad53d15a5fa6efaa0409ae8009b0a18d
2021-09-29 10:55:29 +13:00
Samuel Dobson
d6492d4ed0
Merge bitcoin/bitcoin#22650: Remove -deprecatedrpc=addresses flag and corresponding code/logic
43cd6b8af9 doc: add release notes for removal of the -deprecatedrpc=addresses flag (Michael Dietz)
2b1fdc2c6c refactor: minor styling, prefer snake case and same line if (Michael Dietz)
d64deac7b8 refactor: share logic between ScriptPubKeyToUniv and ScriptToUniv (Michael Dietz)
8721638daa rpc: remove deprecated addresses and reqSigs from rpc outputs (Michael Dietz)

Pull request description:

  Resolves #21797 now that we've branched-off to v23 ("addresses" and "reqSigs" deprecated) "ExtractDestinations" should be removed.

   `-deprecatedrpc=addresses` was initially added in this PR #20286 (which resolved the original issue #20102).

  Some chunks of code and logic are no longer used/necessary with the removal of this, and therefore some minor refactoring is done in this PR as well (separated commits)

ACKs for top commit:
  MarcoFalke:
    re-ACK 43cd6b8af9 🐉
  meshcollider:
    Code review ACK 43cd6b8af9
  jonatack:
    ACK 43cd6b8af9 per `git range-diff a9d0cec 92dc5e9 43cd6b8`, also rebased to latest master, debug built + quick re-review of each commit to bring back context, and ran tests locally at the final commit

Tree-SHA512: fba83495e396d3c06f0dcf49292f14f4aa6b68fa758f0503941fade1a6e7271cda8378e2734af1faea550d1b43c85a36c52ebcc9dec0732936f9233b4b97901c
2021-09-29 10:41:30 +13:00
BitcoinTsunami
d6eb39af21 test: add functional test to check transaction time determination during block rescanning 2021-09-28 21:49:22 +02:00
MarcoFalke
fa54efda9b
test: pep-8 touched test
Can be reviewed with "--word-diff-regex=.".
2021-09-28 15:48:55 +02:00
MarcoFalke
fa46768059
test: Remove unused and confusing main parameter from script_util
Bitcoin script opcodes are equal on all chains (main and test) anyway.

Can be reviewed with "--word-diff-regex=.".
2021-09-28 15:46:57 +02:00
W. J. van der Laan
efa227f5df
Merge bitcoin/bitcoin#23097: Run specified functional tests with all matching flags
b8909b0746 Run functional tests with all possible flags (Samuel Dobson)

Pull request description:

  Functional tests which use flags like `--descriptors` or `--legacy-wallet` won't run if only the base script is given to `test_runner.py` because it doesn't match any script in the list exactly. It would be easier if it would just run both options.

  For example, instead of:
  ```
  test_runner.py 'wallet_basic.py --legacy-wallet' 'wallet_basic.py --descriptors'
  ```

  We can now just run:
  ```
  test_runner.py wallet_basic
  ```

  Also useful for `--usecli`, the IPv4/IPv6/nonloopback `rpc_bind.py` variations, etc.

ACKs for top commit:
  laanwj:
    Code review ACK b8909b0746
  MarcoFalke:
    review ACK b8909b0746

Tree-SHA512: d367037cb170e705551726d47fe4569ebc3ceadece280dd3edbb3ecb41e19f3263d6d272b407316ed6011164e850df4321fb340b1b183b34497c9f7cc439f4d8
2021-09-28 15:32:23 +02:00
pranabp-bit
ea31caf6b4 update estimatesmartfee rpc to return max of estimateSmartFee, mempoolMinFee and minRelayTxFee.
This will provide better estimates which would be closer to fee paid in actual
transactions.
The test has also been changed such that when the node is restarted with a
high mempoolMinFee, the estimatesmartfee still returns a feeRate greater
than or equal to the mempoolMinFee, minRelayTxFee.(just like the feeRate of actual transactions)
2021-09-28 18:36:38 +05:30
MarcoFalke
fa8f3ba131
test: pep-8 2021-09-28 10:52:33 +02:00
MarcoFalke
fac5708afc
test: Use assert_equal over assert for easier debugging 2021-09-28 10:48:51 +02:00
MarcoFalke
a9d0cec499
Merge bitcoin/bitcoin#23106: Ensure wallet is unlocked before signing PSBT with walletprocesspsbt and GUI
7e3ee4cdd0 GUI: Ask user to unlock wallet before signing psbt (Samuel Dobson)
0f3acecf33 Add test that walletprocesspsbt requires unlocked wallet when signing (Samuel Dobson)
0e895212bb Ensure wallet is unlocked before signing in walletprocesspsbt (Samuel Dobson)

Pull request description:

  If signing a PSBT, we need to ensure the wallet is unlocked.

  Fixes #22874, fixes bitcoin-core/gui#312

ACKs for top commit:
  achow101:
    ACK 7e3ee4cdd0
  lsilva01:
    Code Review ACK 7e3ee4cdd0
  benthecarman:
    ACK 7e3ee4cdd0

Tree-SHA512: 6726a873582747900ab454ea21153a92be86808a4c1517dc2856b389876a2da9e8df1ffa9b567b6bd017038342c3544ecf5ca3c97744e7debe0a5ee65563687d
2021-09-28 09:49:44 +02:00
Samuel Dobson
0f3acecf33 Add test that walletprocesspsbt requires unlocked wallet when signing 2021-09-28 13:27:07 +13:00
Sebastian Falbesoner
cfdb6baa22 test: use MiniWallet for p2p_filter.py
This test can now be run even with the Bitcoin Core wallet disabled.
2021-09-27 13:55:25 +02:00
Sebastian Falbesoner
6fc2cd3f09 test: introduce helper to create random P2WPKH scriptPubKeys 2021-09-27 13:55:25 +02:00
Sebastian Falbesoner
aa26797f69 test: MiniWallet: add send_to method to create arbitrary txouts
With this new method, outputs to an arbitrary scriptPubKey/amount can
be created. Note that the implementation was already present in the
test feature_rbf.py and is just moved to the MiniWallet interface, in
order to enable other tests to also use it.
2021-09-27 13:55:25 +02:00
merge-script
632be5514c
Merge bitcoin/bitcoin#23061: Fix (inverse) meaning of -persistmempool
faa9c19a4b doc: Add 23061 release notes (MarcoFalke)
faff17bbde Fix (inverse) meaning of -persistmempool (MarcoFalke)

Pull request description:

  Passing `-persistmempool` is currently treated as `-nopersistmempool`

ACKs for top commit:
  jnewbery:
    reACK faa9c19a4b
  hebasto:
    ACK faa9c19a4b, I have reviewed the code and it looks OK, I agree it can be merged.

Tree-SHA512: f34a89a07745dabe340eb845b2a348b79c093e9056f7a21c17e1ba2e278177c9b4cf30e8095791fd645a7f90eb34850b2eee0c869b4f6ec02bf749c73b0e52ee
2021-09-27 10:12:14 +02:00
merge-script
58c25bdcea
Merge bitcoin/bitcoin#23092: test: Remove Windows workaround in authproxy
fad02274ba test: Remove Windows workaround in authproxy (MarcoFalke)

Pull request description:

  Might no longer be needed after https://github.com/bitcoin/bitcoin/pull/23089

ACKs for top commit:
  hebasto:
    ACK fad02274ba, passed 3 consequential runs on my [personal CI](https://cirrus-ci.com/task/4897456077930496):

Tree-SHA512: 3c408e068ad7590dc0e5922c0b4cd3bf927e22fe624b13b8ab38db848342d310c9c934f358638fd5234c7b5f10f95d3bddc676deb925dcbba54945e2e8229275
2021-09-27 09:26:21 +02:00
merge-script
ba3f0b2c48
Merge bitcoin/bitcoin#23084: test: avoid non-determinism in asmap-addrman test
5825b34783 test: avoid non-determinism in asmap-addrman test (Jon Atack)

Pull request description:

  This is the same approach as for the addpeeraddress test in `test/functional/rpc_net.py` in commit 869f1368.

  The probability of collision when adding an addrman entry is expected to be 1/2^16 = 1/65536 for an address from a different /16.  This change hopes to avoid these collisions by adding 1 tried entry before adding 1 new table one, instead of 2 tried entries followed by 2 new entries, which appears to have caused a collision in the CI.

  To verify the regression test still fails when expected:

  - `git checkout 181a120 && git cherry-pick ef242f5`
  - recompile bitcoind
  - git checkout this branch and run `test/functional/feature_asmap.py`. Expected output:

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

  Closes #23078.

  Co-authored-by: Martin Zumsande <mzumsande@gmail.com>

ACKs for top commit:
  mzumsande:
    Re-ACK 5825b34783

Tree-SHA512: eb6624a196fa5617c84aad860c8f91e8a8f60fc9fe2d7168facc298ee38db5e93b7e988e11c2ac1b399dc2c6b8fad360fb10bdbf10e598a1878300388475a200
2021-09-27 09:20:25 +02:00
W. J. van der Laan
09cb5ec6c8
Merge bitcoin/bitcoin#23065: Allow UTXO locks to be written to wallet DB
d96b000e94 Make GUI UTXO lock/unlock persistent (Samuel Dobson)
077154fe69 Add release note for lockunspent change (Samuel Dobson)
719ae927dc Update lockunspent tests for lock persistence (Samuel Dobson)
f13fc16295 Allow lockunspent to store the lock in the wallet DB (Samuel Dobson)
c52789365e Allow locked UTXOs to be store in the wallet database (Samuel Dobson)

Pull request description:

  Addresses and closes #22368

  As per that issue (and its predecessor #14907), there seems to be some interest in allowing unspent outputs to be locked persistently. This PR does so by adding a flag to lockunspent to store the change in the wallet database. Defaults to false, so there is no change in default behaviour.

  Edit: GUI commit changes default behaviour. UTXOs locked/unlocked via the GUI are now persistent.

ACKs for top commit:
  achow101:
    ACK d96b000e94
  kristapsk:
    ACK d96b000e94
  lsilva01:
    Tested ACK d96b000e94 on Ubuntu 20.04
  prayank23:
    ACK d96b000e94

Tree-SHA512: 957a5bbfe7f763036796906ccb1598feb6c14c5975838be1ba24a198840bf59e83233165cb112cebae909b6b25bf27275a4d7fa425923ef6c788ff671d7a89a8
2021-09-26 11:30:18 +02:00
Samuel Dobson
b8909b0746 Run functional tests with all possible flags 2021-09-26 13:58:19 +13:00
Samuel Dobson
719ae927dc Update lockunspent tests for lock persistence 2021-09-25 23:50:06 +12:00
merge-script
16ccb3a1cd
Merge bitcoin/bitcoin#23086: test: Add -testactivationheight tests to rpc_blockchain
fa4ca8d579 test: Add -testactivationheight tests to rpc_blockchain (MarcoFalke)

Pull request description:

  Suggested: https://github.com/bitcoin/bitcoin/pull/22818#discussion_r712513991

ACKs for top commit:
  laanwj:
    Code review ACK fa4ca8d579
  theStack:
    Concept and code-review ACK fa4ca8d579

Tree-SHA512: 41304db1a15c0c705a9cc2808c9f1d7831a321a8a7948a28ec5d3ee1ed3da6a0ce67cd50c99a33aaed86830c59608eb6ffadbeaba67d95245c490f9b6c277912
2021-09-25 09:39:42 +02:00
MarcoFalke
fad02274ba
test: Remove Windows workaround in authproxy
This reverts commit fab9899204.
2021-09-25 09:33:16 +02:00
merge-script
442e32e117
Merge bitcoin/bitcoin#22817: test: Avoid race after connect_nodes
fa04f26aa7 test: Avoid race after connect_nodes (MarcoFalke)

Pull request description:

  Wait until the connection is fully established on both sides (verack). Fixes #22714

ACKs for top commit:
  kiminuo:
    utACK fa04f26aa7

Tree-SHA512: bc2c44b44b688086ff84046924cf5251dd625584e93ce8fa17de27023855b32f3bb55109b846abbcec775e2836c7f3c5a81d6b4aff7c4ac065b9aefa044c1883
2021-09-25 08:50:52 +02:00
Jon Atack
5825b34783
test: avoid non-determinism in asmap-addrman test
This is the same approach as for the addpeeraddress test in
`test/functional/rpc_net.py` in commit 869f1368.

The probability of collision when adding an addrman entry is
expected to be 1/2^16 = 1/65536 for an address from a different /16.

This change hopes to avoid these collisions by adding 1 tried entry
before adding 1 new table one, instead of 2 tried entries followed
by 2 new entries, which appears to have caused a collision in the CI.

To verify the regression test stills fails when expected:

- git checkout 181a120 && git cherry-pick ef242f5
- recompile bitcoind
- git checkout this branch and run test/functional/feature_asmap.py. Expected output:

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

Co-authored-by: Martin Zumsande <mzumsande@gmail.com>
2021-09-24 22:00:24 +02:00
Michael Dietz
8721638daa
rpc: remove deprecated addresses and reqSigs from rpc outputs 2021-09-24 14:22:49 -05:00
W. J. van der Laan
01b5cfb951
Merge bitcoin/bitcoin#23047: test: Use MiniWallet in mempool_persist
faae0988d6 test: Check other fields are loaded correctly as well (MarcoFalke)
fa4db92617 test: Remove unused self.connect_nodes (MarcoFalke)
fafb7b7a89 test: pep8 (MarcoFalke)
fa32cb2467 test: Use MiniWallet in mempool_persist (MarcoFalke)
faca688a85 test: Add MiniWallet get_descriptor function (MarcoFalke)

Pull request description:

ACKs for top commit:
  laanwj:
    Code review ACK faae0988d6

Tree-SHA512: 6124f16ee1f3f416c50dc07aebe8846ff7e2b7c8e5dd84f9517cb5f1df021b9e57ed7c7e17bc099a37c663cd93f6d417c5e0622c0b359956403d53e705eb5549
2021-09-24 17:09:44 +02:00
MarcoFalke
fa4ca8d579
test: Add -testactivationheight tests to rpc_blockchain 2021-09-24 15:32:00 +02:00
merge-script
8e9801bfc4
Merge bitcoin/bitcoin#22818: test: Activate all regtest softforks at height 1, unless overridden
fa4db8671b test: Activate all regtest softforks at height 1, unless overridden (MarcoFalke)
faad1e5ffd Introduce -testactivationheight=name@height setting (MarcoFalke)
fadb2ef2fa test: Add extra_args argument to TestChain100Setup constructor (MarcoFalke)
faa46986aa test: Remove version argument from build_next_block in p2p_segwit test (MarcoFalke)
fa086ef539 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 fa4db8671b
  theStack:
    re-ACK fa4db8671b

Tree-SHA512: 6397d46ff56ebc48c007a4cda633904d6ac085bc76b4ecf83097c546c7eec93ac0c44b88083b2611b9091c8d1fb8ee1e314065de078ef15e922c015de7ade8bf
2021-09-24 14:04:51 +02:00
fanquake
86c3b84388
Merge bitcoin/bitcoin#23036: test: use test_framework.p2p P2P_SERVICES constant in functional tests
b69a106bcd 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 b69a106bcd
  klementtan:
    crACK b69a106bcd
  fanquake:
    ACK b69a106bcd - didn't look at the formatting changes.

Tree-SHA512: f83e593663a69182986325d9ba2b4b787b87896d6648973f4f802f191a2573201b9e7d7e10e69662ef1965fa63268845726ed1aa5742a2e38dcccf4aebc6a961
2021-09-23 17:13:02 +08:00
MarcoFalke
faff17bbde
Fix (inverse) meaning of -persistmempool 2021-09-22 11:29:44 +02:00
MarcoFalke
fa5e8c1044
Revert "test: Add missing suppression signed-integer-overflow:addrman.cpp"
This reverts commit facb534c37.
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
f6e4db27ce 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
fa3669f72f fuzz: Move all addrman fuzz targets to one file (MarcoFalke)
fa7a883f5a addrman: Replace assert with throw on corrupt data (MarcoFalke)
fa298971e6 Refactor: Turn the internal addrman check helper into a forced check (MarcoFalke)
fae5c633dc 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 803ef70fd9, 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 fa3669f72f
  jnewbery:
    Code review ACK fa3669f72f
  vasild:
    ACK fa3669f72f

Tree-SHA512: 687e4a4765bbc66495152fa7a49d28ee84b405dc5370ba87b4016b5593e45f54c4ce5cae579e4d433e0e082d20fc263969fa602679c911accef0adb2d6213bd6
2021-09-21 18:21:00 +02:00
merge-script
ae674a0198
Merge bitcoin/bitcoin#22998: test: use MiniWallet for make_utxo helper in feature_rbf.py
f680d27155 test: use MiniWallet for make_utxo helper in feature_rbf.py (Sebastian Falbesoner)
0f27524602 test: scale amounts in test_doublespend_tree down by factor 10 (Sebastian Falbesoner)
d1e2481274 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 f680d27155 🦐

Tree-SHA512: 46c8c245086a9e79855c4ede2f8f412333cf2658136805196b203b3567c89398d77fcb80715c0bb72fdc84331cc67544b2fdc259193a3adcb2fc36e147c26fce
2021-09-21 15:20:07 +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
merge-script
223ad2fd0d
Merge bitcoin/bitcoin#22831: test: add addpeeraddress "tried", test addrman checks on restart with asmap
cdaab90662 Add test for addrman consistency check on restart with asmap (Jon Atack)
869f136816 Add test for rpc addpeeraddress with "tried" argument (Jon Atack)
ef242f5213 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 cdaab90662
  mzumsande:
    re-ACK cdaab90662 (based on code review of diff to d586817)
  vasild:
    ACK cdaab90662

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

Pull request description:

  Add missing test coverage

ACKs for top commit:
  jonatack:
    Light code review ACK faa81f9486 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
fa7e3f1fc1 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 fa7e3f1fc1

Tree-SHA512: 6f47d2acac9f180b2b0f8f04797e74ecb1fc180f6b164c67813a3a1f97acea54baed74e5e0a3512e3babf76b105c09e1ba4cad818c83c7cb2beb7377b4c96954
2021-09-21 09:22:30 +02:00
fanquake
1260b7e483
Merge bitcoin/bitcoin#23001: doc: Enable TLS in links in documentation
9bdda50151 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 9bdda50151

Tree-SHA512: 9d04d8771a9daf3c3b9914ff324e2eabfdf3ff5ae7f7dc92b84a1f3527010ceb860e73873a8f24d6051763eb472d9ea324ccbd6129a40318a520ca88c05f0586
2021-09-21 14:47:05 +08:00
W. J. van der Laan
488e745560
Merge bitcoin/bitcoin#12677: RPC: Add ancestor{count,size,fees} to listunspent output
6cb60f3e6d doc/release-notes: Add new listunspent fields (Luke Dashjr)
0be2f17ef5 QA: Add tests for listunspent ancestor{count,size,fees} to mempool_packages (Luke Dashjr)
6966e80f45 RPC: Add ancestor{count,size,fees} to listunspent output (Luke Dashjr)
3f77dfdaf0 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 6cb60f3e6d
  kiminuo:
    ACK [6cb60f3](6cb60f3e6d)
  achow101:
    Code Review ACK 6cb60f3e6d
  naumenkogs:
    ACK 6cb60f3e6d
  darosior:
    utACK 6cb60f3e6d

Tree-SHA512: 5d16e5799558691e5853ab7ea2cc85514cb45da3ce69134d855c71845beef32ec6af5ab28d4462683e9800c8ea126f162773a9d3d5660edac08fd8edbfeda173
2021-09-20 19:25:43 +02:00