Commit graph

21392 commits

Author SHA1 Message Date
MarcoFalke
f4e5d704f2
Merge bitcoin/bitcoin#24118: Add 'sendall' RPC née sweep
bb84b7145b add tests for no recipient and using send_max while inputs are specified (ishaanam)
49090ec402 Add sendall RPC née sweep (Murch)
902793c777 Extract FinishTransaction from send() (Murch)
6d2208a3f6 Extract interpretation of fee estimation arguments (Murch)
a31d75e5fb Elaborate error messages for outdated options (Murch)
35ed094e4b Extract prevention of outdated option names (Murch)

Pull request description:

  Add sendall RPC née sweep

  _Motivation_
  Currently, the wallet uses a fSubtractFeeAmount (SFFO) flag on the
  recipients objects for all forms of sending calls. According to the
  commit discussion, this flag was chiefly introduced to permit sweeping
  without manually calculating the fees of transactions. However, the flag
  leads to unintuitive behavior and makes it more complicated to test
  many wallet RPCs exhaustively. We proposed to introduce a dedicated
  `sendall` RPC with the intention to cover this functionality.

  Since the proposal, it was discovered in further discussion that our
  proposed `sendall` rpc and SFFO have subtly different scopes of
  operation.
  • sendall:
    Use _given UTXOs_ to pay a destination the remainder after fees.
  • SFFO:
    Use a _given budget_ to pay an address the remainder after fees.

  While `sendall` will simplify cases of spending a given set of
  UTXOs such as paying the value from one or more specific UTXOs, emptying
  a wallet, or burning dust, we realized that there are some cases in
  which SFFO is used to pay other parties from a limited budget,
  which can often lead to the creation of change outputs. This cannot be
  easily replicated using `sendall` as it would require manual
  computation of the appropriate change amount.

  As such, sendall cannot replace all uses of SFFO, but it still has a
  different use case and will aid in simplifying some wallet calls and
  numerous wallet tests.

  _Sendall call details_
  The proposed sendall call builds a transaction from a specific
  subset of the wallet's UTXO pool (by default all of them) and assigns
  the funds to one or more receivers. Receivers can either be specified
  with a given amount or receive an equal share of the remaining
  unassigned funds. At least one recipient must be provided without
  assigned amount to collect the remainder. The `sendall` call will
  never create change. The call has a `send_max` option that changes the
  default behavior of spending all UTXOs ("no UTXO left behind"), to
  maximizing the output amount of the transaction by skipping uneconomic
  UTXOs. The `send_max` option is incompatible with providing a specific
  set of inputs.

  ---
  Edit: Replaced OP with latest commit message to reflect my updated motivation of the proposal.

ACKs for top commit:
  achow101:
    re-ACK bb84b7145b

Tree-SHA512: 20aaf75d268cb4b144f5d6437d33ec7b5f989256b3daeeb768ae1e7f39dc6b962af8223c5cb42ecc72dc38cecd921c53c077bc0ec300b994e902412213dd2cc3
2022-03-30 15:02:49 +02:00
fanquake
f9aedbc300
Merge bitcoin/bitcoin#24690: util: Add inotify_rm_watch to syscall sandbox (AllowFileSystem)
f05a4cdf5a util: Add inotify_rm_watch to syscall sandbox (AllowFileSystem) (Hennadii Stepanov)

Pull request description:

  This PR fixes the current master (3297f5c11c) when running `bitcoin-qt` on Ubuntu 22.04 and quitting:
  ```
  $ ./src/qt/bitcoin-qt -signet -sandbox=log-and-abort
  Warning: Ignoring XDG_SESSION_TYPE=wayland on Gnome. Use QT_QPA_PLATFORM=wayland to run on Wayland anyway.
  ERROR: The syscall "inotify_rm_watch" (syscall number 255) is not allowed by the syscall sandbox in thread "main". Please report.
  terminate called without an active exception
  Aborted (core dumped)
  ```

  Also see https://github.com/bitcoin/bitcoin/pull/24659#discussion_r835747166

ACKs for top commit:
  fanquake:
    ACK f05a4cdf5a - checked that qt is using this in it's filesystem watcher code.

Tree-SHA512: 9c7920a25422cd3a040bc1cbc487c12c3dc2b91358c3757f1030d6a1ff12c18c688a8e5b7466f683da88a5e4f5f15d442975660022d706e47021253c24c58f4a
2022-03-30 10:19:25 +01:00
MarcoFalke
f4fba57829
Merge bitcoin/bitcoin#24704: compat: remove strnlen back-compat code
d4ba2b2cbc compat: remove strnlen back-compat code (fanquake)

Pull request description:

  This was needed for mingw (not mingw-w64), and some older versions of
  macOS, which we no-longer support.

ACKs for top commit:
  hebasto:
    ACK d4ba2b2cbc

Tree-SHA512: d1beb9df58464feea3076091361d7d46e4a8901e347644a5fa6f24e052ca24ee0c7c0dd3f2a3d682b0204bf50430fa89eac62121691ea08af6dcf6b907bdec87
2022-03-30 08:19:09 +02:00
fanquake
f089a0802c
Merge bitcoin/bitcoin#24692: refactoring: [Net Processing] Follow-ups to #21160
a40978dcbd [fuzz] Assert that Peer.m_tx_relay.m_relay_txs has been set correctly (John Newbery)
0bca5f2b46 [net processing] PushNodeVersion() takes a const Peer& (John Newbery)
21154ff927 net_processing: move CNode data access out of lock (John Newbery)

Pull request description:

  #21160 ([net/net processing]: Move tx inventory into net_processing) had some unaddressed review comments when it was merged. This branch addresses those comments.

ACKs for top commit:
  MarcoFalke:
    review ACK a40978dcbd
  dergoegge:
    ACK a40978dcbd
  ajtowns:
    ACK a40978dcbd

Tree-SHA512: 46624e275f918c5f32d0adab0766e9b3ef8ebdbc74a3c8886d8a2e2ff1079029dcc371b40ef0d787609e9c05219b7456f3e2dfe4fb0cb7bf23ef966769aef1a1
2022-03-30 07:13:52 +01:00
Murch
49090ec402
Add sendall RPC née sweep
_Motivation_
Currently, the wallet uses a fSubtractFeeAmount (SFFO) flag on the
recipients objects for all forms of sending calls. According to the
commit discussion, this flag was chiefly introduced to permit sweeping
without manually calculating the fees of transactions. However, the flag
leads to unintuitive behavior and makes it more complicated to test
many wallet RPCs exhaustively. We proposed to introduce a dedicated
`sendall` RPC with the intention to cover this functionality.

Since the proposal, it was discovered in further discussion that our
proposed `sendall` rpc and SFFO have subtly different scopes of
operation.
• sendall:
  Use _specific UTXOs_ to pay a destination the remainder after fees.
• SFFO:
  Use a _specific budget_ to pay an address the remainder after fees.

While `sendall` will simplify cases of spending from specific UTXOs,
emptying a wallet, or burning dust, we realized that there are some
cases in which SFFO is used to pay other parties from a limited budget,
which can often lead to the creation of change outputs. This cannot be
easily replicated using `sendall` as it would require manual computation
of the appropriate change amount.

As such, sendall cannot replace all uses of SFFO, but it still has a
different use case and will aid in simplifying some wallet calls and
numerous wallet tests.

_Sendall call details_
The proposed sendall call builds a transaction from a specific subset of
the wallet's UTXO pool (by default all of them) and assigns the funds to
one or more receivers. Receivers can either be specified with a specific
amount or receive an equal share of the remaining unassigned funds. At
least one recipient must be provided without assigned amount to collect
the remainder. The `sendall` call will never create change. The call has
a `send_max` option that changes the default behavior of spending all
UTXOs ("no UTXO left behind"), to maximizing the output amount of the
transaction by skipping uneconomic UTXOs. The `send_max` option is
incompatible with providing a specific set of inputs.
2022-03-29 16:37:47 -04:00
John Newbery
a40978dcbd [fuzz] Assert that Peer.m_tx_relay.m_relay_txs has been set correctly 2022-03-29 15:54:22 +01:00
John Newbery
0bca5f2b46 [net processing] PushNodeVersion() takes a const Peer&
The peer object is not mutated by PushNodeVersion, so pass a const reference
2022-03-29 15:54:16 +01:00
laanwj
9e32adbb5c
Merge bitcoin/bitcoin#24523: build: Fix Boost.Process test for Boost 1.78
532c64a726 build: Fix Boost.Process test for Boost 1.78 (Hennadii Stepanov)

Pull request description:

  Rebased #24415 with Luke's suggestion.

  Fixes #24413.

ACKs for top commit:
  hebasto:
    ACK 532c64a726, tested on Mac mini (M1, 2020) + macOS Monterey 12.3 (21E230).

Tree-SHA512: 74f779695f6bbc45a2b7341a1402f747cc0d433d74825c7196cb9f156db0c0299895365f01665bd0bff12a8ebb5ea33a29b9a52f5eac0007ec35d1dca6544705
2022-03-29 13:36:45 +02:00
fanquake
d4ba2b2cbc
compat: remove strnlen back-compat code
This was needed for mingw (not mingw-w64), and some older versions of
macOS, which we no-longer support.
2022-03-29 10:15:33 +01:00
MarcoFalke
9d00406dc9
Merge bitcoin/bitcoin#24677: refactor: fix wallet and related named args
21db4eb3ff test: fix incorrect named args in wallet tests (fanquake)
8b0e776718 test: fix incorrect named args in coin_selection tests (fanquake)
6fc00f7331 bench: fix incorrect named args in coin_selection bench (fanquake)

Pull request description:

  Should be one of the last changes split from #24661.

  Motivation:
  > Incorrect named args are source of bugs, like https://github.com/bitcoin/bitcoin/pull/22979.

  > To allow them being checked by clang-tidy, use a format it can understand.

ACKs for top commit:
  MarcoFalke:
    Concept ACK 21db4eb3ff

Tree-SHA512: c29743a70f6118cf73dc37b56b30f45da55b7d7b3b8ed36859ad59f602c3e6692eb755e05d9a4dd17f05085bcd6cb5b8c4007090a76e4fbfb053f925322cf985
2022-03-28 14:56:46 +02:00
fanquake
a13946b822
Merge bitcoin/bitcoin#23083: rpc: Fail to return undocumented or misdocumented JSON
fc892c3a80 rpc: Fail to return undocumented or misdocumented JSON (MarcoFalke)
f4bc4a705a rpc: Add m_skip_type_check to RPCResult (MarcoFalke)

Pull request description:

  This avoids documentation shortcomings such as the ones fixed in commit e7b6272b30, 138d55e6a0, 577bd51a4b, f8c84e047c, 0ee9a00f90, 13f41855c5, or faecb2ee0a

ACKs for top commit:
  fanquake:
    ACK fc892c3a80 - tested that this catches issue, i.e #24691:

Tree-SHA512: 9d0d7e6291bfc6f67541a4ff746d374ad8751fefcff6d103d8621c0298b190ab1d209ce96cfc3a0d4a6a5460a9f9bb790eb96027b16e5ff91f2512e40c92ca84
2022-03-28 12:16:42 +01:00
MarcoFalke
faac877ffd
doc: Fix getpeerinfo doc 2022-03-28 10:00:45 +02:00
John Newbery
21154ff927 net_processing: move CNode data access out of lock
CNode::m_relays_tx and CNode::m_bloom_filter_loaded access don't require
the Peer::TxRelay::m_bloom_filter_mutex lock, so move them out of the
lock scope.

See https://github.com/bitcoin/bitcoin/pull/21160#discussion_r736785417
and https://github.com/bitcoin/bitcoin/pull/21160#discussion_r736785662.
2022-03-28 08:23:32 +01:00
Hennadii Stepanov
f05a4cdf5a
util: Add inotify_rm_watch to syscall sandbox (AllowFileSystem) 2022-03-28 09:14:29 +02:00
MarcoFalke
dc0ba8ce9f
Merge bitcoin/bitcoin#24659: util: add linkat to syscall sandbox (AllowFileSystem)
9809db3577 util: add linkat to syscall sandbox (AllowFileSystem) (fanquake)

Pull request description:

  Should fix #24536.

ACKs for top commit:
  MarcoFalke:
    cr ACK 9809db3577
  Rspigler:
    Tested ACK (commit 9809db3577) - this fixes https://github.com/bitcoin/bitcoin/issues/24536

Tree-SHA512: 2642f7dfa806e166fb32639a29b509b2edc8b919516c1f12430fc96f9887952395e157d71ef99fbaef8f7bcce1920530c24ecbce605b8a374b05d586f1f22a24
2022-03-28 09:04:38 +02:00
MarcoFalke
3d2f24bb38
Merge bitcoin/bitcoin#24656: refactor: Move mempool RPCs to rpc/mempool
fac5a51c47 Move mempool RPCs to rpc/mempool (MarcoFalke)
fa0f666dd7 style: Add static keyword where possible in rpc/mempool (MarcoFalke)

Pull request description:

  This moves the remaining mempool RPCs to `rpc/mempool`. Previously all mempool RPCs from the `blockchain` category have been moved. This patch moves the ones from the `rawtransactions` category.

  In the future, as a follow-up to this refactoring patch, it could be considered whether a new `mempool` category should be introduced.

  Beside a clearer code organization, this pull request should also reduce the compile time and space of the `rawtransactions.cpp` file.

ACKs for top commit:
  promag:
    Code review ACK fac5a51c47.

Tree-SHA512: 5578b894b68d0595869a9b03ed8dceebe3366f73dec5f090ccc36ff4002b1bc4d58af77546c2d71537c1be03694d9a28c4b1bfbb3569560997879293c5c0301e
2022-03-28 08:53:48 +02:00
fanquake
21db4eb3ff
test: fix incorrect named args in wallet tests 2022-03-25 21:27:57 +00:00
fanquake
8b0e776718
test: fix incorrect named args in coin_selection tests 2022-03-25 21:27:40 +00:00
fanquake
6fc00f7331
bench: fix incorrect named args in coin_selection bench 2022-03-25 21:27:39 +00:00
fanquake
2f0f056e08
Merge bitcoin/bitcoin#24665: doc: document clang tidy named args
7e22d80af3 addrman: fix incorrect named args (fanquake)
67f654ef61 doc: Document clang-tidy in dev notes (MarcoFalke)

Pull request description:

  The documentation, and a single commit extracted from #24661.

  Motivation:
  > Incorrect named args are source of bugs, like https://github.com/bitcoin/bitcoin/pull/22979.

  > To allow them being checked by clang-tidy, use a format it can understand.

ACKs for top commit:
  glozow:
    ACK 7e22d80af3

Tree-SHA512: 4037fcea59fdf583b171bce7ad350299fe5f9feb3c398413432168f3b9a185e51884d5b30e4b4ab9c6c5bb896c178cfaee1d78d5b4f0034cd70121c9ea4184b7
2022-03-25 21:04:51 +00:00
fanquake
6d5771ba07
Merge bitcoin/bitcoin#24494: wallet: generate random change target for each tx for better privacy
9053f64fcb [doc] release notes for random change target (glozow)
46f2fed6c5 [wallet] remove MIN_CHANGE (glozow)
a44236addd [wallet] randomly generate change targets (glozow)
1e52e6bd0a refactor coin selection for parameterizable change target (glozow)

Pull request description:

  Closes #24458 - the wallet always chooses 1 million sats as its change target, making it easier to fingerprint transactions created by the Core wallet. Instead of using a fixed value, choose one randomly each time (within a range). Using 50ksat (around $20) as the lower bound and `min(1 million sat, 2 * average payment value)` as the upper bound.
  RFC: If the payment is <25ksat, this doesn't work, so we're using the range (payment amount, 50ksat) instead.

ACKs for top commit:
  achow101:
    ACK 9053f64fcb
  Xekyo:
    reACK 9053f64fcb

Tree-SHA512: 45ce5d064697065549473347648e29935733f3deffc71a6ab995449431f60302d1f9911a0994dfdb960b48c48b5d8859f168b396ff2a62db67d535a7db041d35
2022-03-25 21:03:32 +00:00
MarcoFalke
f66c827c2d
Merge bitcoin/bitcoin#24502: wallet: don't create long chains by default
da2bc865d6 [wallet] don't create long chains by default (glozow)

Pull request description:

  Default mempool policy doesn't let you have chains longer than 25 transactions. This is locally configurable of course, but it's not really safe to assume that a chain longer than 25 transactions will propagate. Thus, the wallet should probably avoid creating such transactions by default; set `DEFAULT_WALLET_REJECT_LONG_CHAINS` to true.

  Closes #9752
  Closes #10004

ACKs for top commit:
  MarcoFalke:
    re-ACK da2bc865d6 only change is fixing typos in tests 🎏

Tree-SHA512: 65d8e4ec437fe928adf554aa7e819a52e0599b403d5310895f4e371e99bbc838219b3097c4d2f775bc870ac617ef6b4227b94291f2b376f824f14e8f2b152f31
2022-03-25 17:16:13 +01:00
glozow
da2bc865d6 [wallet] don't create long chains by default 2022-03-25 16:02:37 +00:00
MarcoFalke
7878c8655c
Merge bitcoin/bitcoin#24674: refactor: remove unused boost include in bitcoin-util.cpp
3bb9627463 refactor: remove unused boost header include in bitcoin-util.cpp (Sebastian Falbesoner)

Pull request description:

  This header was included since the introduction of bitcoin-util in
  commit 13762bcc96, but boost was
  actually never used (see `git log -S boost ./src/bitcoin-util.cpp`).

  Cherry-picked out of #22953, which currently needs rebase. This commit could just be merged on its own.

ACKs for top commit:
  MarcoFalke:
    review ACK 3bb9627463

Tree-SHA512: 201ee1aa4d49074056654203db73a473479c2b92c49df8dbf8e35979f85178013c66540a665f0f6dc0a2efef88eb091e2b088bebff85d840033dffd8ae719349
2022-03-25 16:44:28 +01:00
fanquake
6b1f93700c
Merge bitcoin/bitcoin#24666: refactor: Fix coinselection.h include, Make COutput a struct
fab287cedd Clarify that COutput is a struct, not a class (MarcoFalke)
fa61cdf464 wallet: Fix coinselection include (MarcoFalke)

Pull request description:

  * Fix include (see commit message)
  * `{}`-init, see https://github.com/bitcoin/bitcoin/pull/24091#discussion_r831193284
  * `struct`, see https://github.com/bitcoin/bitcoin/pull/24091#discussion_r831192702

ACKs for top commit:
  theStack:
    Code-review ACK fab287cedd

Tree-SHA512: dd2cfb9c06a92295dbd8fbb6d56afcf00ebda2a0440e301d392cd183d1b9cd87626311d539e302a9e6c6521d69d6183c74a51934e3fc16e64a5dcaba60c7e3ce
2022-03-25 15:36:14 +00:00
Sebastian Falbesoner
3bb9627463
refactor: remove unused boost header include in bitcoin-util.cpp
This header was included since the introduction of bitcoin-util in
commit 13762bcc96, but boost was
actually never used (see `git log -S boost ./src/bitcoin-util.cpp`).
2022-03-25 15:30:20 +00:00
MarcoFalke
f10b24ad29
Merge bitcoin/bitcoin#24672: init: add missing cs_main lock
0346c26fca init: add missing cs_main lock (Anthony Towns)

Pull request description:

  `BlockManager::m_block_tree_db` is protected by `cs_main`, so take the
  `cs_main` lock while accessing it.

ACKs for top commit:
  jonatack:
    Code review ACK 0346c26fca

Tree-SHA512: d6dff0b2d58871c7fbb281558b59fa9ad26fa75b3ceca9232277fc49ab795325e5ac3d266db49e7bda33da6de0b014b1bdebdf2c2c4347d43e50c0433a2cf06c
2022-03-25 16:20:53 +01:00
Murch
902793c777
Extract FinishTransaction from send()
The final step of send either produces a PSBT or the final transaction.
We extract these steps to a new helper function `FinishTransaction()` to
reuse them in `sendall`.
2022-03-25 11:16:46 -04:00
Murch
6d2208a3f6
Extract interpretation of fee estimation arguments
This will be reused in `sendall`, so we extract a method to prevent
duplication.
2022-03-25 11:16:44 -04:00
Murch
a31d75e5fb
Elaborate error messages for outdated options 2022-03-25 11:16:42 -04:00
Murch
35ed094e4b
Extract prevention of outdated option names
This will be reused in `sendall` so we extract it to avoid
duplication.
2022-03-25 11:16:38 -04:00
fanquake
9344697e57
Merge bitcoin/bitcoin#21160: net/net processing: Move tx inventory into net_processing
1066d10f71 scripted-diff: rename TxRelay members (John Newbery)
575bbd0dea [net processing] Move tx relay data to Peer (John Newbery)
785f55f7ee [net processing] Move m_wtxid_relay to Peer (John Newbery)
36346703f8 [net] Add CNode.m_relays_txs and CNode.m_bloom_filter_loaded (John Newbery)

Pull request description:

  This continues the work of moving application layer data into net_processing, by moving all tx data into the new Peer object added in #19607.

  For motivation, see #19398.

ACKs for top commit:
  dergoegge:
    ACK 1066d10f71 - This is a good layer separation improvement with no behavior changes.
  glozow:
    utACK 1066d10f71

Tree-SHA512: 0c9d6b8a0a05e2d816b6d6588b7df133842ec960ae67667813422aa7bd8eb5308599c714f3822a98ddbdf364ffab9050b055079277ba4aff24092557ff99ebcc
2022-03-25 15:16:00 +00:00
Anthony Towns
0346c26fca init: add missing cs_main lock
BlockManager::m_block_tree_db is protected by cs_main, so take the
cs_main lock while accessing it.
2022-03-26 00:04:07 +10:00
laanwj
7c08d81e11
Merge bitcoin/bitcoin#23536: Enforce Taproot script flags whenever WITNESS is set
cccc1e70b8 Enforce Taproot script flags whenever WITNESS is set (MarcoFalke)
fa42299411 Remove nullptr check in GetBlockScriptFlags (MarcoFalke)
faadc606c7 refactor: Pass const reference instead of pointer to GetBlockScriptFlags (MarcoFalke)

Pull request description:

  Now that Taproot is active, it makes sense to enforce its rules on all blocks, even historic ones, regardless of the deployment status.

  ### Benefits:

  (With "script flags" I mean "taproot script verification flags".)

  * Script flags are known ahead for all blocks (even blocks not yet created) and do not change. This may benefit static analysis, code review, and development of new script features that build on Taproot.
  * Any future bugs introduced in the deployment code won't have any effect on the script flags, as they are independent of deployment.
  * Enforcing the taproot rules regardless of the deployment status makes testing easier because invalid blocks after activation are also invalid before activation. So there is no need to differentiate the two cases.
  * It gives belt-and-suspenders protection against a practically expensive and theoretically impossible IBD reorg attack where the node is eclipsed. While `nMinimumChainWork` already protects against this, the cost for a few months worth of POW might be lowered until a major version release of Bitcoin Core reaches EOL. The needed work for the attack is the difference between `nMinimumChainWork` and the work at block 709632.

  For reference, previously the same was done for P2SH and WITNESS in commit 0a8b7b4b33.

  ### Implementation:

  I found one block which fails verification with the flags applied, so I added a `TaprootException`, similar to the `BIP16Exception`.

  For reference, the debug log:

  ```
  ERROR: ConnectBlock(): CheckInputScripts on b10c007c60e14f9d087e0291d4d0c7869697c6681d979c6639dbd960792b4d41 failed with non-mandatory-script-verify-flag (Witness program was passed an empty witness)
  BlockChecked: block hash=0000000000000000000f14c35b2d841e986ab5441de8c585d5ffe55ea1e395ad state=non-mandatory-script-verify-flag (Witness program was passed an empty witness)
  InvalidChainFound: invalid block=0000000000000000000f14c35b2d841e986ab5441de8c585d5ffe55ea1e395ad  height=692261  log2_work=92.988459  date=2021-07-23T08:24:20Z
  InvalidChainFound:  current best=0000000000000000000067b17a4c0ffd77c29941b15ad356ca8f980af137a25d  height=692260  log2_work=92.988450  date=2021-07-23T07:47:31Z
  ERROR: ConnectTip: ConnectBlock 0000000000000000000f14c35b2d841e986ab5441de8c585d5ffe55ea1e395ad failed, non-mandatory-script-verify-flag (Witness program was passed an empty witness)
  ```

  Hint for testing, make sure to set `-noassumevalid`.

  ### Considerations

  Obviously this change can lead to consensus splits on the network in light of massive reorgs. Currently the last block before Taproot activation, that is the last block without the Taproot script flags set, is only buried by a few days of POW. However, when and if this patch is included in the next major release, it will be buried by a few months of POW. BIP90 considerations apply when looking at reorgs this large.

ACKs for top commit:
  Sjors:
    tACK cccc1e70b8
  achow101:
    ACK cccc1e70b8
  laanwj:
    Code review ACK cccc1e70b8
  ajtowns:
    ACK cccc1e70b8 ; code review; wrote a "getblockscriptflags" rpc to quickly check that blocks just had bit 17 (taproot) added; review of earlier revisions had established non-exception blocks do validate with taproot rules enabled.
  jamesob:
    ACK cccc1e70b8 ([`jamesob/ackr/23536.1.MarcoFalke.enforce_taproot_script_f`](https://github.com/jamesob/bitcoin/tree/ackr/23536.1.MarcoFalke.enforce_taproot_script_f))

Tree-SHA512: 00044de68939caef6420ffd588c1291c041a8b397c80a3df1e3e3487fbeae1821d23975c51c95e44e774558db76f943b00b4e27cbd0213f64a9253116dc6edde
2022-03-25 14:11:18 +01:00
MarcoFalke
d2615312c1
Merge bitcoin/bitcoin#19385: test: Change default test logging directory
f8cba0d911 test: Change default test logging directory (Yancy Ribbens)

Pull request description:

  This PR changes the default test log location request here: https://github.com/bitcoin/bitcoin/issues/17224.  Instead of using the location of the makefile [automatic variable](https://www.gnu.org/software/make/manual/make.html#Automatic-Variables) `$<` I extract just the basename and then prepend a new location `./test`.  This is done because `$<` represents the variable name AND location of the prerequisite here.

Top commit has no ACKs.

Tree-SHA512: f0fbc530cf0e14c284b4bbf6671c145b1d7a2e1f5561f5c5d09f0cbe88b98e620e763bbbf2dfa9aeeec3dcc9b0127939e105e14c7e4f6660c7c19663622a393d
2022-03-25 13:30:26 +01:00
glozow
46f2fed6c5 [wallet] remove MIN_CHANGE 2022-03-25 11:57:51 +00:00
glozow
a44236addd [wallet] randomly generate change targets
If the wallet always chooses 1 million sats as its change target, it is
easier to fingerprint transactions created by the Core wallet.
2022-03-25 11:56:46 +00:00
glozow
1e52e6bd0a refactor coin selection for parameterizable change target
no behavior changes, since the target is always MIN_CHANGE
2022-03-25 11:56:46 +00:00
MarcoFalke
fab287cedd
Clarify that COutput is a struct, not a class
Also, use {}-initialization
2022-03-25 09:58:36 +01:00
MarcoFalke
fa61cdf464
wallet: Fix coinselection include
coinselection.h is not used by wallet.h but by qt/coincontroldialog.cpp
2022-03-25 09:57:42 +01:00
fanquake
7e22d80af3
addrman: fix incorrect named args 2022-03-25 08:22:24 +00:00
MarcoFalke
56c8658700
Merge bitcoin/bitcoin#24663: doc, init: add links to doc/cjdns.md
daae28885f doc, init: add links to doc/cjdns.md (Jon Atack)

Pull request description:

  Follow-up to #24555.

ACKs for top commit:
  jessebarton:
    ACK [daae288](daae28885f)

Tree-SHA512: eb8f4324f182f7917ddafa9b88ad753fe8f890c1c883c1342768ed9eac998c422ecd9e998fc977e72e26bc87f6aed295940b522187190481889255c8b2e05311
2022-03-25 07:27:29 +01:00
fanquake
3740cdd125
Merge bitcoin/bitcoin#24091: wallet: Consolidate CInputCoin and COutput
049003fe68 coinselection: Remove COutput operators == and != (Andrew Chow)
f6c39c6adb coinselection: Remove CInputCoin (Andrew Chow)
70f31f1a81 coinselection: Use COutput instead of CInputCoin (Andrew Chow)
14fbb57b79 coinselection: Add effective value and fees to COutput (Andrew Chow)
f0821230b8 moveonly: move COutput to coinselection.h (Andrew Chow)
42e974e15c wallet: Remove CWallet and CWalletTx from COutput's constructor (Andrew Chow)
14d04d5ad1 wallet: Replace CWalletTx in COutput with COutPoint and CTxOut (Andrew Chow)
0ba4d1916e wallet: Provide input bytes to COutput (Andrew Chow)
d51f27d3bb wallet: Store whether a COutput is from the wallet (Andrew Chow)
b799814bbd wallet: Store tx time in COutput (Andrew Chow)
46022953ee wallet: Remove use_max_sig default value (Andrew Chow)
10379f007f scripted-diff: Rename COutput member variables (Andrew Chow)
c7c64db41e wallet: cleanup COutput constructor (Andrew Chow)

Pull request description:

  While working on coin selection code, it occurred to me that `CInputCoin` is really a subset of `COutput` and the conversion of a `COutput` to a `CInputCoin` does not appear to be all that useful. So this PR adds fields that are present in `CInputCoin` to `COutput` and replaces the usage of `CInputCoin` with `COutput`.

  `COutput` is also moved to coinselection.h. As part of this move, the usage of `CWalletTx` is removed from `COutput`. It is instead replaced by storing a `COutPoint` and the `CTxOut` rather than the entire `CWalletTx` as coin selection does not really need the full `CWalletTx`. The `CWalletTx` was only used for figuring out whether the transaction containing the output was from the current wallet, and for the transaction's time. These are now parameters to `COutput`'s constructor.

ACKs for top commit:
  ryanofsky:
    Code review ACK 049003fe68, just adding comments and removing == operators since last review
  w0xlt:
    reACK 049003f
  Xekyo:
    reACK 049003fe68

Tree-SHA512: 048b4cd620a0415e1d9fe8597257ee4bc64656566e1d28a9bdd147d6d72dc87c3f34a3339fa9ab6acf42c388df7901fc4ee900ccaabc3de790ffad162b544c15
2022-03-24 20:46:43 +00:00
MarcoFalke
f0c9ba2b48
Merge bitcoin/bitcoin#24205: init, test: improve network reachability test coverage and safety
58a14795b8 test: passing -onlynet=onion with -onion=0/-noonion raises expected init error (Jon Atack)
7000f66d36 test: passing -onlynet=onion without -proxy/-onion raises expected init error (Jon Atack)
8332e6e4cf test: passing invalid -onion raises expected init error (Jon Atack)
d5edb08708 test: passing invalid -proxy raises expected init error (Jon Atack)
bd57dcbaf2 test: hoist proxy out of 2 network loops in feature_proxy.py (Jon Atack)
afdf2de282 test: add CJDNS to LimitedAndReachable_Network unit tests (Jon Atack)
2b7a8180a9 net, init: assert each network reachability is true by default (Jon Atack)

Pull request description:

  Adds missing network reachability test coverage and an assertion during init, noticed while reviewing #22834:

  - assert during init that each network reachability is  true by default
  - add CJDNS to the `LimitedAndReachable_Network` unit tests
  - hoist proxy out of two network loops in feature_proxy.py
  - test that passing invalid `-proxy` raises expected init error
  - test that passing invalid `-onion` raises expected init error
  - test that passing `-onlynet=onion` without `-proxy` and `-onion` raises expected init error
  - test that passing `-onlynet=onion` with `-onion=0` and with `-noonion` raises expected init error

ACKs for top commit:
  vasild:
    ACK 58a14795b8
  brunoerg:
    ACK 58a14795b8
  dongcarl:
    Code Review ACK 58a14795b8

Tree-SHA512: bdee6dd0c12bb63591ce7c9321fe77b509ab1265123054e774adc38a187746dddafe1627cbe89e990bcc78b45e194bfef8dc782710d5b217e2e2106ab0158827
2022-03-24 21:17:46 +01:00
Jon Atack
daae28885f
doc, init: add links to doc/cjdns.md 2022-03-24 20:12:32 +01:00
fanquake
0a14a16efe
Merge bitcoin/bitcoin#24625: Replace struct update_fee_delta with lambda
fa84a49526 Use CAmount for fee delta and modified fee (MarcoFalke)
fa8857c3f7 Replace struct update_fee_delta with lambda (MarcoFalke)

Pull request description:

  The same was done for another struct in e177fcab38.

  Also, change type of feeDelta from int64_t to CAmount.

ACKs for top commit:
  hebasto:
    re-ACK fa84a49526
  promag:
    Code review ACK fa84a49526.

Tree-SHA512: 2b9ee449d348b0f685793a35c6dd3c57ed97fdf707a89495a0518bb332f407303b48723e667351e96f2b698e0a2ade27095517a3accd926d4ec85e58d6fd441f
2022-03-24 16:58:06 +00:00
MarcoFalke
fc892c3a80 rpc: Fail to return undocumented or misdocumented JSON 2022-03-24 14:30:13 +01:00
MarcoFalke
f4bc4a705a rpc: Add m_skip_type_check to RPCResult
Used in the next commit.
2022-03-24 14:30:13 +01:00
MarcoFalke
4a0ab355b3
Merge bitcoin/bitcoin#24626: init: disallow reindex-chainstate when pruning
b2813980b8 init: disallow reindex-chainstate when pruning (Martin Zumsande)

Pull request description:

  The combination of `-reindex-chainstate` and `-prune` currently makes the node stuck in an endless loop:

  - `LoadChainstate()` will wipe the existing chainstate (so we have no genesis block anymore). It won't clean up unusable block files by calling `CleanupBlockRevFiles()` as for full `-reindex`.
  - `ThreadImport()` has [logic](91d12344b1/src/node/blockstorage.cpp (L855)) of reloading Genesis after reindexing. This is what makes full `-reindex` work with `-prune` but it's not executed for `-reindex-chainstate`.
  - Since we still don't have a genesis block, init will wait for it forever in an endless loop ([code](91d12344b1/src/init.cpp (L1630-L1640))).

  Fix this by disallowing `-reindex-chainstate` together with `-prune`. This is discouraged in the help for `-reindex-chainstate` anyway ("When in pruning mode or if blocks on disk might be corrupted, use full -reindex instead.") but wasn't enforced.

  Fixes #24242

ACKs for top commit:
  MarcoFalke:
    cr ACK b2813980b8

Tree-SHA512: 7220842daaf9a4f972d82b13b81fdeac2833bf5e665c5b0f8eaf6a4bcd0725c8e97d19ec956ca4b730065a983475bb3a2732713d338f4caf8666ccbf63d4d988
2022-03-24 14:27:13 +01:00
fanquake
213e98ca82
Merge bitcoin/bitcoin#24169: build: Add --enable-c++20 option
999982b06c build: Add --enable-c++20 option (MarcoFalke)
fae679065e Add CSerializedNetMsg::Copy() helper (MarcoFalke)
fabb7c4ba6 Make fs.h C++20 compliant (MarcoFalke)
fae2220f4e scheduler: Capture ‘this’ explicitly in lambda (MarcoFalke)

Pull request description:

  This is for CI and devs only and doesn't change that C++17 is the standard we are currently using. The option `--enable-c++20` allows CI to check that the C++17 code in the repo is  also valid C++20. (There are some cases where valid C++17 doesn't compile under C++20).

  Also, it allows developers to easily play with C++20 in the codebase.

ACKs for top commit:
  ryanofsky:
    Code review ACK 999982b06c. Since last review was rebased, and enum-conversion change was dropped, and CSerializedNetMsg copy workaround was added
  fanquake:
    utACK 999982b06c

Tree-SHA512: afc95ba03ea2b937017fc8e2b1449379cd2b6f7093c430d2e344c665a00c51e402d6651cbcbd0be8118ea1e54c3a86e67d2021d19ba1d4da67168e9fcb6b6f83
2022-03-24 13:01:01 +00:00