fab3658356 [qa] Test that getdata requests work as expected (Suhas Daftuar)
fa883ab35a net: Use mockable time for tx download (MarcoFalke)
Pull request description:
Two commits:
* First commit changes to mockable time for tx download (refactoring, should only have an effect on regtest)
* Second commit adds a test that uses mocktime to test tx download
ACKs for top commit:
laanwj:
code review ACK 16197/commits/fab365835639a3da03f8ad9a58a0db6c6c4c2314
jamesob:
ACK fab3658356
Tree-SHA512: 3a64a3e283ec4bab1f6e506404b11f0a564a5b61d2a7508ae738a61f035e57220484c66e0ae47d847fe9f7e3ff5cc834909d7b34a9bbcea6abe01f8742806908
e6f649cb2c test: Make tests arg type specific (Hennadii Stepanov)
b70cc5d733 Revamp option negating policy (Hennadii Stepanov)
db08edb303 Replace IsArgKnown() with FlagsOfKnownArg() (Hennadii Stepanov)
dde80c272a Use ArgsManager::NETWORK_ONLY flag (Hennadii Stepanov)
9a12733508 Remove unused m_debug_only member from Arg struct (Hennadii Stepanov)
fb4b9f9e3b scripted-diff: Use ArgsManager::DEBUG_ONLY flag (Hennadii Stepanov)
1b4b9422ca scripted-diff: Use Flags enum in AddArg() (Hennadii Stepanov)
265c1b58d8 Add Flags enum to ArgsManager (Hennadii Stepanov)
e0d187dfeb Refactor InterpretNegatedOption() function (Hennadii Stepanov)
e0e18a1017 refactoring: Check IsArgKnown() early (Hennadii Stepanov)
Pull request description:
This PR adds the `Flags` enum to the `ArgsManager` class. Also the `m_flags` member is added to the `Arg` struct. Flags denote an allowed type of an arg value and special hints.
This PR is only a refactoring and does not change behavior.
ACKs for top commit:
jamesob:
ACK e6f649cb2c
MarcoFalke:
ACK e6f649cb2c thanks for adding types to the command line options
Tree-SHA512: b867f8a9cbce2d2473c293d534af662d8cd5be15060ff0682e97af678974bdaac35e8bc6328ccba32f105034bcd38f169b92a6fb67798667891ce14d5d2a2dea
fb62f128bb Tidy up BroadcastTransaction() (John Newbery)
b8eecf8e79 Remove unused submitToMemoryPool and relayTransactions Chain interfaces (Antoine Riard)
8753f5652b Remove duplicate checks in SubmitMemoryPoolAndRelay (Antoine Riard)
611291c198 Introduce CWalletTx::SubmitMemoryPoolAndRelay (Antoine Riard)
8c8aa19b4b Add BroadcastTransaction utility usage in Chain interface (Antoine Riard)
Pull request description:
Remove CWalletTx::AcceptToMemoryPool
Replace CWalletTx::RelayWalletTransaction by SubmitMemoryPoolAndRelay
Add a relay flag to broadcastTransaction because wasn't sure of ReacceptWalletTransactions semantic.
Obviously, working on implementing https://github.com/bitcoin/bitcoin/pull/14978#issuecomment-459373984 to add the new higher-method in Node interface, will add a commit, just need more thought to do it cleanly
ACKs for top commit:
MarcoFalke:
re-ACK fb62f128bb
Sjors:
re-ACK fb62f128bb
Tree-SHA512: a7ee48b0545f537fa65cac8ed4cb24e777ab90b877d4eefb87971fa93c6a59bd555b62ad8940c6ffb40592a0bd50787d27587af99f20b56af72b415b6394251f
d6b3640ac7 [test] walletcreatefundedpsbt: check RBF is disabled when -walletrbf=0 (Sjors Provoost)
9ed062b568 [doc] rpc: remove "fallback to" from RBF default help (Sjors Provoost)
4fcb698bc2 [rpc] walletcreatefundedpsbt: use wallet default RBF (Sjors Provoost)
Pull request description:
The `walletcreatefundedpsbt` RPC call currently ignores `-walletrbf` and defaults to not use RBF. This PR fixes that.
This PR also replaces UniValue in `ConstructTransaction` with a `bool` in preparation of moving this helper method out of the RPC codebase entirely. This may be a bit overkill, but does slightly simplify it.
Fixes#15878
ACKs for top commit:
achow101:
Code Review ACK d6b3640ac7
l2a5b1:
re-ACK d6b3640
MarcoFalke:
ACK d6b3640ac7
Tree-SHA512: 55b9bccd1ef36b54f6b34793017dc0721103099ad3761b3b04862291ee13d6915915d4dbb1a8567924fa56e5e95dfe10eec070e06701610e70c87f8ea92b2a00
IsCoinBase check is already performed early by
AcceptToMemoryPoolWorker
GetDepthInMainChain check is already perfomed by
BroadcastTransaction
To avoid deadlock we MUST keep lock order in
ResendWalletTransactions and CommitTransaction,
even if we lock cs_main again further.
in BroadcastTransaction. Lock order will need
to be clean at once in a future refactoring
Access through a broadcastTransaction method.
Add a wait_callback flag to turn off race protection when wallet
already track its tx being in mempool
Standardise highfee, absurdfee variable name to max_tx_fee
We drop the P2P check in BroadcastTransaction as g_connman is only
called by RPCs and the wallet scheduler, both of which are initialized
after g_connman is assigned and stopped before g_connman is reset.
7a0c224289 Suppress output in test_bitcoin for expected errors (Gert-Jaap Glasbergen)
Pull request description:
Closes#15944
This adds two methods to noui, that allows temporarily suppressing (and then resuming) the output from `noui`. For situations where errors are expected, it's confusing for the test binary to output an error and then conclude with `No errors detected`.
It also uses this supress/reconnect in the tests that currently produce verbose errors when running `test_bitcoin`.
Output of `test_bitcoin` on current master:
```
gertjaap@gjdesktop:~/src/bitcoin$ src/test/test_bitcoin
Running 351 test cases...
Error: Specified -walletdir "/tmp/test_common_Bitcoin Core/1561389554_943311758/tempdir/path_does_not_exist" does not exist
Error: Specified -walletdir "/tmp/test_common_Bitcoin Core/1561389554_643733972/tempdir/not_a_directory.dat" is not a directory
Error: Specified -walletdir "wallets" is a relative path
*** No errors detected
```
Output after this code is merged:
```
gertjaap@gjdesktop:~/src/bitcoin$ src/test/test_bitcoin
Running 351 test cases...
*** No errors detected
```
ACKs for top commit:
l2a5b1:
ACK 7a0c224 - tested and reviewed.
laanwj:
ACK 7a0c224289
Tree-SHA512: c7881f7a431a065329360ffa9937ce4742694c646c90c019d3aff95dfd7fccbdcda9116c5762feb6dfd1108d14f9fb386e203b173c4bde9093afb2b8c977d13d
b078067b9c gui: Remove unused RPCConsole::tabFocus (João Barbosa)
Pull request description:
Added in #14573 but not used, so begone.
ACKs for top commit:
practicalswift:
utACK b078067b9c
hebasto:
ACK b078067b9c
laanwj:
ACK b078067b9c, there's nothing really to test here
Tree-SHA512: 237276dea4d174b5fca34855447146f79c3faaae7179f4245c70e2070b49282d95f886b1be6d2a33713c81a254f4483a4e4bf850053a8dcb18a3a897bd3da08e
c5d3787367 Allow createwallet to take empty passwords to make unencrypted wallets (Andrew Chow)
Pull request description:
Allow createwallet to take the empty string as a password and interpret that as leaving the wallet unencrypted. Also warn when that happens.
This fixes a bug where it was not possible to use the `avoid_reuse` option for new unencrypted wallets without using named arguments.Thus this allows more `createwallet` options to be added that can be set on unencrypted wallets when using positional arguments.
ACKs for top commit:
jnewbery:
code review ACK c5d3787367
meshcollider:
re-utACK c5d3787367
ryanofsky:
utACK c5d3787367. Changes since last review are rebasing, concatenating warning strings to avoid discarding warnings, adding release notes, and choosing an unambiguous wallet name for the test.
Tree-SHA512: 146737a728dd614ba94d4b166b27e8c9e195badd1709ccab2315afe59176d9b493dfba9b61c3ed81090f059c7e464d709deb06d99451b9a3fff667f527d6f7c9
0b1f4b3c66 wallet: Drop unused OldKey (João Barbosa)
Pull request description:
Replaces #16494, `OldKey` (previously `CWalletKey`) was never serialized in the code history which means that unserialization support is not required, so remove the code entirely.
ACKs for top commit:
jnewbery:
ACK 0b1f4b3c66
laanwj:
ACK 0b1f4b3c66
fanquake:
ACK 0b1f4b3c66
Tree-SHA512: 92e9b2d6fc41f2765492d5d69d18fc4302c40ab44f28c8c30ca652c72767fbc484848c51a38ecf1f447849767a583c398784408bb5f64f9c86f9a5872b325ffc
9bc8b28c1d refactor : use RelayTransaction in BroadcastTransaction utility (Antoine Riard)
Pull request description:
Implementing suggestion in https://github.com/bitcoin/bitcoin/pull/15713#discussion_r306571420.
Seems a reason of these node utilities is to glue with already there functions, so we should reuse them.
ACKs for top commit:
MarcoFalke:
ACK 9bc8b28c1d
promag:
ACK 9bc8b28c1d, verified there are no more `PushInventory(CInv(MSG_TX, ...`, nice refactor, 👍 @amitiuttarwar.
jnewbery:
ACK 9bc8b28c1d
jonatack:
ACK 9bc8b28c1d, second @jnewbery's suggestions, my guess is they could be added without risking delaying this PR.
Tree-SHA512: 841c65d5f0d9ead5380814bb2260d7ebf03f2a9bfa58a1025785d353bdb42f9122cc257993e6a7bd2bd3f2c74db19c5978cc14be0d83258124ca22e33d6d0164
80ba4241a6 extract min & max depth onto coin control (Amiti Uttarwar)
Pull request description:
- Refactor `AvailableCoins` to pull min & max depths from coin control.
- Add `m_max_depth` to coin control to support this.
- Addresses issue https://github.com/bitcoin/bitcoin/issues/15823, see thread for further details.
ACKs for top commit:
laanwj:
ACK 80ba4241a6
Tree-SHA512: 8f7c0aa90b3bc3667baf6741b1da2829f3919e1df92ae097d86c6b239f0c024eb410d7100e6251ea8fc49d022fb5a1214bf79b0f8b0014945b7784b2311647d1
05b56d1c93 [wallet] Remove CMerkleTx serialization logic (John Newbery)
783a76f23b [wallet] Flatten CWalletTx class hierarchy (John Newbery)
b3a9d179f2 [wallet] Move CMerkleTx functions into CWalletTx (John Newbery)
Pull request description:
CMerkleTx is only used as a base class for
CWalletTx. It was previously also used for vtxPrev which
was removed in 93a18a3650.
This PR moves all of the CMerkleTx members and logic
into CWalletTx. The CMerkleTx class is kept for deserialization
and serialization of old wallet files.
This makes the refactor in #15931 cleaner.
ACKs for top commit:
laanwj:
ACK 05b56d1c93. Looks good to me.
Tree-SHA512: 3d3a0069ebb536b12a328f1261e7dc55158a71088d445ae4b4ace4142c432dc296f58c8183b1922e54a60b8cc77e9d17c3dce7478294cd68693594baacf2bab3
0000ff0aa7 txmempool: Remove unused default value MemPoolRemovalReason::UNKNOWN (MarcoFalke)
Pull request description:
The `remove*` methods set the removal reason to `UNKNOWN` by default. This is nowhere used; Except in tests, where the value doesn't matter. Fix that by removing the confusing default.
ACKs for top commit:
practicalswift:
utACK 0000ff0aa7
promag:
ACK 0000ff0aa7.
jonasschnelli:
utACK 0000ff0aa7
Tree-SHA512: ffc8b35dd3291a81225171577c743c8bb2645638cab02960b6361174cb68afd739aaab7ab8661d65de5750d37daf16bb7eee9338958d8609093a8d46c2ada1ab
CMerkleTx only exists as a base class for CWalletTx and for wallet file
serialization/deserialization. Move CMerkleTx methods into CWalletTx,
but leave class hierarchy and serialization logic in place.
29ee4c417d Specify AM_CPPFLAGS for ZMQ. (Daniel Kraft)
Pull request description:
When building the ZMQ static library, add `AM_CPPFLAGS` to the library `CPPFLAGS`. Otherwise, we may miss important flags that are specified elsewhere. For instance, if `--enable-debug` is passed and
`-DDEBUG_LOCKORDER` set, then that would not apply to the ZMQ library before (causing potential for hard-to-find bugs).
ACKs for top commit:
laanwj:
utACK 29ee4c417d
Tree-SHA512: 64085d71ed3f435a6e4df6dc42bda8b6159a4d292d0547c5b38c09d6ac95e976ad1728cd65278bffdd57363f60a58eb762b1171dafbe055cf94ffcd4f66da877
fa6f22bf44 wallet: Rename CWalletKey to OldKey (MarcoFalke)
fa6dc7fa5f wallet: Enumerate walletdb keys (MarcoFalke)
Pull request description:
It is nice to see all the keys that exists in a single enum
Also, rename CWalletKey to OldKey and update the outdated documentation
ACKs for top commit:
laanwj:
ACK fa6f22bf44, I'm a big fan of this kind of change as it prevents typos, which can happen with 'magic' strings in the code.
promag:
ACK fa6f22bf44. @jnewbery suggestions are great followups, I think this is good enough.
meshcollider:
utACK fa6f22bf44
achow101:
Code review ACK fa6f22bf44
fanquake:
ACK fa6f22bf44 - I had a quick look over, definitely prefer this to strings floating around everywhere.
Tree-SHA512: 8ac3abd5a0d22dac1d77b8f97fe1e16c2608d650f3e9d6dd1df2fd5aeb35ef6643dfd4cd5c162404bb0100343c927d66df04dc695507ffc84a6c667e603acc54
42a5e912ee [mempool] log correct messages when CPFP fails (John Newbery)
Pull request description:
Fixes a logging issue introduced in #15681
ACKs for top commit:
laanwj:
ACK 42a5e912ee (+utACK from bluematt that isn't registered because it has no commit id)
Tree-SHA512: ff5f423cc4d22838eea00c5b1d39ceda89cd61474c72f256a97c698eb0ec3f2156a97139f537669376132902c1e3943bf84c356a4b98a9a306b4ec57302c2761
4057b7acb7 wallet: Recognize -disablewallet option early (Hennadii Stepanov)
Pull request description:
This PR makes early check for the `-disablewallet` option.
If `-disablewallet=1`, objects `PaymentServer` and `WalletController` are nor created.
ACKs for top commit:
jonasschnelli:
utACK 4057b7acb7
laanwj:
ACK 4057b7acb7
Tree-SHA512: 74633cd1eacd0914c73712e6dff190255b5378595cfee7eaeb91e17671fc9120928034739f4ae1c53b86f46c4b400390877241384376b2fc534de326d3ab0944
59cb722fd0 Update configure to reject unsafe miniUPnPc API ver (Hennadii Stepanov)
ab2190557e doc: Add release notes for 15993 (Hennadii Stepanov)
02709e9560 Align formatting with clang-format (Hennadii Stepanov)
91a1b85083 Use PACKAGE_NAME in UPnP description (Hennadii Stepanov)
9f76e45b9d Drop support of insecure miniUPnPc versions (Hennadii Stepanov)
Pull request description:
1. Minimum supported miniUPnPc API version is set to 10:
- https://packages.ubuntu.com/xenial/libminiupnpc-dev
- https://packages.debian.org/jessie/libminiupnpc-dev
Refs:
- #6583
- #6789
- #10414
2. The hardcoded "Bitcoin" replaced with `PACKAGE_NAME`:
3. Also style-only commit applied.
Pardon: could not reopen my previous PR #15966.
ACKs for top commit:
ryanofsky:
utACK 59cb722fd0. Changes since last review: adding a new commit which updates configure script to fall back to disabling upnp if version is too old, adding a requested comment explaining static_assert condition, and fixing a spelling (jessy/jessie)
Tree-SHA512: 42ed11bc2fb2ec83d5dd58e2383da5444a24fd572707f6cf10b622cb8943e28adfcca4750d06801024c4472625b5ea9279516fbd9d2ccebc9bbaafe1d148e80d
07e01d6258 rpc: sendrawtransaction unconditionality/privacy note (Jon Atack)
Pull request description:
In sendrawtransaction RPCHelpMan, mention unconditionality and privacy as per http://www.erisian.com.au/bitcoin-core-dev/log-2019-07-25.html#l-522
before
```
$ bitcoin-cli help sendrawtransaction
sendrawtransaction "hexstring" ( maxfeerate )
Submits raw transaction (serialized, hex-encoded) to local node and network.
Also see createrawtransaction and signrawtransactionwithkey calls.
(...)
```
after
```
$ bitcoin-cli help sendrawtransaction
sendrawtransaction "hexstring" ( maxfeerate )
Submit a raw transaction (serialized, hex-encoded) to local node and network.
Note that the transaction will be sent unconditionally to all peers, so using this
for manual rebroadcast may degrade privacy by leaking the transaction's origin, as
nodes will normally not rebroadcast non-wallet transactions already in their mempool.
Also see createrawtransaction and signrawtransactionwithkey calls.
(...)
```
ACKs for top commit:
promag:
ACK 07e01d6258.
laanwj:
ACK 07e01d6258
Tree-SHA512: 427b3ca29384eef271eb496b7b14e883220863543a536ddeb31940aaffd52ea0b607d929d50f2b7958514105ef7823fa05c1ee381d4a432808753c06bd97af58
