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`:
![Screenshot from 2019-05-06 23-10-29](https://user-images.githubusercontent.com/32963518/57253178-afc60780-7056-11e9-83c9-e85670c58c1e.png)
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
35e60e790f Remove ReadVersion and WriteVersion (Andrew Chow)
b3d4f6c961 Log the actual wallet file version (Andrew Chow)
c88e87c3b2 Remove nFileVersion from CWalletScanState (Andrew Chow)
Pull request description:
The wallet file version is stored in the "minversion" record, not the "version" record. However "version" is no longer used anywhere except to record the highest versioned client which has opened a wallet file (which is currently only used to check whether this was most recently opened by a 0.4.0 or 0.5.0rc1 client which had a broken wallet encryption implementation). Furthermore, "version" was logged to the debug.log which is confusing because it is not the actual wallet file version.
This PR changes it so that this confusion largely no longer exists. The wallet file version logging is changed to use "minversion" and reading and writing the "version" record is no longer publicly exposed to prevent potential confusion about whether the actual file version is being read or written. Lastly, in the one place it is actually used, the variable name is changed from nFileVersion to last_client to better reflect what that record actually represents.
ACKs for top commit:
jb55:
ACK 35e60e7, I compiled locally as a quick sanity check.
ryanofsky:
utACK 35e60e790f. This code still pretty confusing, but a little simpler now. And the previous log statement was really misleading and useless compared to the new one here.
meshcollider:
Looks good, thanks! utACK 35e60e790f
Tree-SHA512: f782b2f215d07fbc9b806322bda8085445b81c02b65ca674a8c6a3e1de505a0abd050669afe0ead4778816144a1c18462e13930071cedb7227a058aeb39493f7
4d94916f0d Get rid of PendingWalletTx class. (Russell Yanofsky)
Pull request description:
No reason for this class to exist if it doesn't have any code to run in the destructor. e10e1e8db0 from https://github.com/bitcoin/bitcoin/pull/16208 recently removed the destructor code that would return an unused key if the transaction wasn't committed.
This is just cleanup, there's no change in behavior.
ACKs for top commit:
ariard:
utACK 4d94916. Successfully built both `bitcoind` and `bitcoin-qt`. `PendingWalletTx` was only a wrapper to enforce call to `ReturnDestination` if `CommitTransaction` doesn't `KeepDestination` before.
promag:
ACK 4d94916f0d, refactor looks good to me.
meshcollider:
utACK 4d94916f0d
Tree-SHA512: f3f93d2f2f5d8f1e7810d609d881c1b1cbbaa8629f483f4293e20b3210292605e947bc4903fde9d2d8736277ca3bd6de182f7eac1e13515d5a327f2ebc130839
fa4a605a4c Remove wallet settings from chainparams (MarcoFalke)
Pull request description:
Feels a bit odd to have wallet setting in the chainparams, so remove them from there
ACKs for top commit:
promag:
ACK fa4a605a4c, missed s/2018/2019?
practicalswift:
utACK fa4a605a4c
darosior:
ACK fa4a605a4c
Tree-SHA512: 2b3a5ee85d36af290d7db80bed1339e3c684607f1ce61cc65c906726e9174e40325fb1f67a34d8780f2a61fa39a1785e7c3a1cef5b6d6c364f38db5300cdbe3a
2f7eb772f6 Add RPC bumpfee totalFee deprecation test (Jon Atack)
a92d9ce8cf deprecate totalFee argument in bumpfee RPC call (Gregory Sanders)
Pull request description:
totalFee argument is of questionable use, and should be removed in favor of feerate-based features.
I first moved IsDeprecatedRPCEnabled because `bitcoin-wallet` doesn't link `libbitcoin_server`.
ACKs for top commit:
ryanofsky:
utACK 2f7eb772f6. Only change since last review is leaving IsDeprecatedRPCEnabled in its happy home, and switching to rpcEnableDeprecated instead. (Thanks!)
jonatack:
ACK 2f7eb772f6. Built locally, manually tested rpc bumpfee, help output ([gist](https://gist.github.com/jonatack/863673eacc02f9da39ff6d6712f9d837)), all tests pass. Travis failures appears to be unrelated, the [bitcoin builds are green](https://bitcoinbuilds.org/index.php?build=121).
meshcollider:
Code Review ACK 2f7eb772f6
Tree-SHA512: c97465205ee59575df37894bcbb6c4ecf8858dd8fe9d89503f9342b226768c1dcb553153bc9eb3055f7bf5eb41573e48b8efa57e083cd255793cbe5280f0026a
40ad2f6a58 Have importwallet use ImportPrivKeys and ImportScripts (Andrew Chow)
78941da5ba Optionally allow ImportScripts to set script creation timestamp (Andrew Chow)
94bf156f39 Have importaddress use ImportScripts and ImportScriptPubKeys (Andrew Chow)
a00d1e5ec5 Have importpubkey use CWallet's ImportScriptPubKeys and ImportPubKeys functions (Andrew Chow)
c6a8274247 Have importprivkey use CWallet's ImportPrivKeys, ImportScripts, and ImportScriptPubKeys (Andrew Chow)
fae7a5befd Log when an import is being skipped because we already have it (Andrew Chow)
ab28e31c95 Change ImportScriptPubKeys' internal to apply_label (Andrew Chow)
Pull request description:
#15741 introduced `ImportPrivKeys`, `ImportPubKeys`, `ImportScripts`, and `ImportScriptPubKeys` in `CWallet` which are used by `importmulti`. This PR changes the remaining `import*` RPCs (`importaddress`, `importprivkey`, `importpubkey`, and `importwallet`) to use these functions as well instead of directly adding the imported items to the wallet.
ACKs for top commit:
MarcoFalke:
ACK 40ad2f6a58 (checked that behavior changes are mentioned in the commit body)
ryanofsky:
utACK 40ad2f6a58. Only change since last review is a tweaked commit message (mentioning label update in importpubkey commit)
Sjors:
ACK 40ad2f6a5. Those extra tests also pass.
Tree-SHA512: 910e3bbe20b6f8809a47b7293775db234125615d886c7fd99c194f4cdf00c765eb1e24b1799260f1213b98c88f9bbe696796f36087c182925e567d44e9194c98
a47df13471 [qa] Test disconnect block failure -> shutdown (Suhas Daftuar)
4433ed0f73 [validation] Crash if disconnecting a block fails (Suhas Daftuar)
Pull request description:
If we're unable to disconnect a block during normal operation, then that is a
failure of our local system (such as disk failure) or the chain that we are on
(eg CVE-2018-17144), but cannot be due to failure of the (more work) chain that
we're trying to validate.
We should abort rather than stay on a less work chain.
Fixes#14341.
ACKs for top commit:
practicalswift:
utACK a47df13471
TheBlueMatt:
utACK a47df13471. Didn't bother to review the test in detail, it looked fine. Debated whether invalidateblock should ever crash the node, but *not* crashing in the case of hitting a pruned block (which is the only change here) is clearly better, even if there are other cases I'd argue we should crash in.
ryanofsky:
utACK a47df13471. Only change since last review is new comment.
promag:
ACK a47df1347, it takes awhile to quit (RPC connection timeouts) but that's unrelated - hope to fix that soon.
fanquake:
ACK a47df13471
Tree-SHA512: 4dec8cef6e7dbbe513c138fc5821a7ceab855e603ece3c16185b51a3830ab7ebbc844a28827bf64e75326f45325991dcb672f13bd7baede53304f27289c4af8d
This prevents premature tries to access or create the default datadir.
This is useful when the -datadir option is specified and the default
datadir is unreachable.
This prevents premature tries to access or create the default datadir.
This is useful when the -datadir option is specified and the default
datadir is unreachable.
This prevents premature tries to access or create the default datadir.
This is useful when the -datadir option is specified and the default
datadir is unreachable.
Also removes the now unused ImportAddress and ImportScript from rpcdump.cpp
Behavior changes:
* No errors will be thrown when the script or key already exists in the wallet.
* If the key or script is already in the wallet, their labels will be updated.
-BEGIN VERIFY SCRIPT-
sed -i 's/inline std::string _(const char\* psz)/inline bilingual_str _(const char\* psz)/' src/util/translation.h
sed -i 's/return G_TRANSLATION_FUN ? (G_TRANSLATION_FUN)(psz) : psz;/return bilingual_str{psz, G_TRANSLATION_FUN ? (G_TRANSLATION_FUN)(psz) : psz};/' src/util/translation.h
sed -i 's/\b_("\([^"]\|\\"\)*")/&.translated/g' $(git grep --files-with-matches '\b_("' src)
echo Hard cases - multiline strings.
sed -i 's/"Visit %s for further information about the software.")/&.translated/g' src/init.cpp
sed -i "s/\"Only rebuild the block database if you are sure that your computer's date and time are correct\")/&.translated/g" src/init.cpp
sed -i 's/" restore from a backup.")/&.translated/g' src/wallet/db.cpp
sed -i 's/" or address book entries might be missing or incorrect.")/&.translated/g' src/wallet/wallet.cpp
echo Special case.
sed -i 's/_(COPYRIGHT_HOLDERS)/&.translated/' src/util/system.cpp test/lint/lint-format-strings.py
-END VERIFY SCRIPT-
fa6f402bde Call node->initError instead of InitError from GUI code (Russell Yanofsky)
fad2502240 init: Use InitError for all errors in bitcoind/qt (MarcoFalke)
Pull request description:
Using the same InitError for startup error in the daemon and the gui makes it possible to run the tests with the gui again:
```sh
BITCOIND=bitcoin-qt ./test/functional/test_runner.py feature_includeconf feature_config_args
ACKs for top commit:
hebasto:
ACK fa6f402bde
ryanofsky:
utACK fa6f402bde. Only changes since last review are removing more includes and adding Node::initError method to avoid accessing node `InitError` function and global variables from GUI code.
Tree-SHA512: bd19e08dcea4019dfe40356bc5c63cb583cefed54b6c9dcfb82f1b5b00308d8e2b363549afcaea5e93bf83864dbe0917400c3b70f43a8a5bdff45c9cd34cc294
4f050b91c7 move-onlyish: move CCoinsViewErrorCatcher out of init.cpp (James O'Beirne)
Pull request description:
This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11):
Parent PR: #15606
Issue: #15605
Specification: https://github.com/jamesob/assumeutxo-docs/tree/2019-04-proposal/proposal
---
This change moves `CCoinsViewErrorCatcher` out of `init` and into `coins` so that it can later be included in [a `CoinsView` instance](91284964ef (diff-349fbb003d5ae550a2e8fa658e475880R504)) under `CChainState`.
Instead of hardcoding read failure behavior that has knowledge of qt, it accepts error callbacks via `AddReadErrCallback()`.
ACKs for top commit:
dongcarl:
re-ACK 4f050b91c7
ryanofsky:
utACK 4f050b91c7. Only change since last review is fixing const.
Tree-SHA512: eaba21606d15d2b8d0e3db7cec57779ce181af953db1ef4af80a0bc1dfb57923d0befde9d61b7be55c32224744f7fb6bd47d4e4c72f3ccfe6eaf0f4ae3765c17
The "version" record that these functions read and write are not
used anywhere in the code except for one place. There is no reason
to expose these functions publicly. Furthermore, this avoids potential
confusion as developers may mistake these functions for actually
reading and writing the wallet version when they do not.
nFileVersion is not the actual file version and is not used except
in one place. So it is removed from CWalletScanState and changed so
that it is just read at the place it is needed. Furthermore, the
"version" record now only indicates the version of the highest
versioned client that has opened a wallet file so the variable
name is changed accordingly
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).
and into coins.cpp. This move is necessary so that we can later include a
CCoinsViewErrorCatcher instance under CChainState.
Co-authored-by: MarcoFalke <falke.marco@gmail.com>