493a166948 bench: Don't return a bool from main (Wladimir J. van der Laan)
Pull request description:
Return `1` from `main()` on error, not the bool `false` (introduced in #13112). This is the correct value to return on error, and also shuts up a clang warning.
Tree-SHA512: 52a0f1b2f6ae2697555f71ee2019ce657046f7f379f1f4faf3cce9d5f3fb21fcdc43a4c84895a2a8b6929997ba70bbe87c231f2f9553215b84c22333810d58d9
903055730b Test gArgs erroring on unknown args (Andrew Chow)
4f8704d57f Give an error and exit if there are unknown parameters (Andrew Chow)
174f7c8080 Use a struct for arguments and nested map for categories (Andrew Chow)
Pull request description:
Following #13190, gArgs is aware of all of the command line arguments. This PR has gArgs check whether the arguments provided are actually valid arguments. When an unknown argument is encountered, an error is printed to stderr and the program exist.
Since gArgs is used for everything that has command line arguments, `bitcoind`, `bitcoin-cli`, `bitcoin-qt`, `bitcoin-tx`, and `bench_bitcoin` are all effected by this change and all now have the same argument checking behavior.
Closes#1044
Tree-SHA512: 388201319a7d6493204bb5433da47e8e6c8266882e809f6df45f86d925f1f320f2fd13edb3e57ffc6a37415dfdfc689f83929452bca224229783accb367032e7
4b62bdf5136c174621509bf7866fbd89b61cc66a Wallet: Refactor ReserveKeyFromKeyPool for safety (Ben Woosley)
Pull request description:
ReserveKeyFromKeyPool's previous behaviour is to set nIndex to -1 if the keypool is
empty, OR throw an exception for technical failures. Instead, we now return false
if the keypool is empty, true if the operation succeeded.
This is to make failure more easily detectable by calling code.
Tree-SHA512: 753f057ad13bd4c28d121f426bf0967ed72b827d97fb24582f9326ec60072abc5482e3db69ccada7c5fc66de9957fc59098432dd223fc4116991cab44c6d7aef
c814e2e7e81fd01fcb07f4a28435741bdc463801 Remove template matching and pseudo opcodes (Pieter Wuille)
Pull request description:
The current code contains a rather complex script template matching engine, which is only used for 3 particular script types (P2PK, P2PKH, multisig). The first two of these are trivial to match for otherwise, and a specialized matcher for multisig is both more compact and more efficient than a generic one.
The goal is being more flexible, so that for example larger standard multisigs inside SegWit outputs are easier to implement.
As a side-effect, it also gets rid of the pseudo opcodes hack.
Tree-SHA512: 643b409c5c36821519f613a43efd399af0ec99b6131f35cd4024decfb2d483d719e0e921cd088bc9832a7ac797cb4a6b1158b8574c82f7fbebb75f1b31b359df
c004ffc9b42a738043e19e4c812fc7e0566119c5 Make handling of invalid in IsMine more uniform (Pieter Wuille)
a53f0feff8d42b7a40d417f77dc8de682dd88fd9 Add some checks for invalid recursion in IsMine (Pieter Wuille)
b5802a9f5f69815d3290361fd8c96d76a037832f Simplify IsMine logic (Pieter Wuille)
4e91820531889e309dc4335fe0de8229c6426040 Make IsMine stop distinguishing solvable/unsolvable (Pieter Wuille)
6d714c3419b368671bd071a8992950c3dc00e613 Make coincontrol use IsSolvable to determine solvability (Pieter Wuille)
Pull request description:
Our current `IsMine` logic does several things with outputs:
* Determine "spendability" (roughly corresponding to "could we sign for this")
* Determine "watching" (is this an output directly or indirectly a watched script)
* Determine invalidity (is this output definitely not legally spendable, detecting accidental uncompressed pubkeys in witnesses)
* Determine "solvability" (would we be able to sign for this ignoring the fact that we may be missing some private keys).
The last item (solvability) is mostly unrelated and only rarely needed (there is just one instance, inside the wallet's coin control logic). This PR changes that instance to use the separate `IsSolvable` function, and stop `IsMine` from distinguishing between solvable and unsolvable.
As an extra, this also simplifies the `IsMine` logic and adds some extra checks (which wouldn't be hit unless someone adds already invalid scripts to their wallet).
Tree-SHA512: 95a6ef75fbf2eedc5ed938c48a8e5d77dcf09c933372acdd0333129fb7301994a78498f9aacce2c8db74275e19260549dd67a83738e187d40b5090cc04f33adf
fac1223a568fa1ad6dd602350598eed278d115e8 Cache witness hash in CTransaction (MarcoFalke)
faab55fbb17f2ea5080bf02bc59eeef5ca746f07 Make CMutableTransaction constructor explicit (MarcoFalke)
Pull request description:
This speeds up:
* compactblocks (v2)
* ATMP
* validation and miner (via `BlockWitnessMerkleRoot`)
* sigcache (see also unrelated #13204)
* rpc and rest (nice, but irrelevant)
This presumably slows down rescan, which uses a `CTransaction` and its `GetHash`, but never uses the `GetWitnessHash`. The slow down is proportional to the number of witness transactions in the rescan window. I.e. early in the chain there should be no measurable slow down. Later in the chain, there should be a slow down, but acceptable given the speedups in the modules mentioned above.
Tree-SHA512: 443e86acfcceb5af2163e68840c581d44159af3fd1fce266cab3504b29fcd74c50812b69a00d41582e7e1c5ea292f420ce5e892cdfab691da9c24ed1c44536c7
6249021d1 [docs] Add release notes for HD master key -> HD seed rename (John Newbery)
79053a5f2 [rpc] [wallet] Add 'hdmasterkeyid' alias return values. (John Newbery)
c75c35141 [refactor] manually change remaining instances of master key to seed. (John Newbery)
131d4450b scripted-diff: Rename master key to seed (John Newbery)
Pull request description:
Addresses #12084 and #8684
This renames a couple of functions and members (no functional changes, expect log prints):
- Rename CKey::SetMaster to CKey::SetSeed
- Rename CHDChain::masterKeyId to CHDChain::seedID
- Rename CHDChain::hdMasterKeyID to CHDChain::hdSeedID
- Rename CWallet::GenerateNewHDMasterKey to CWallet::GenerateNewHDSeed
- Rename CWallet::SetHDMasterKey to CWallet::SetHDSeed
As well it introduces a tiny API change:
- RPC API change: Rename "hdmasterkeyid" to "hdseedid", rename "hdmaster" in wallet-dump output to "hdseed"
Fixes also a bug:
- Bugfix: use "s" instead of the incorrect "m" for the seed-key hd-keypath key metadata
Tree-SHA512: c913252636f213135a3b64df5de5d21844fb9c2d646567c1aad0ec65745188587de26119de99492c67e559bd49fdd9606b54276f00dddb84301785beba58f281
2352aa9 test: Ensure that recursive -includeconf produces appropriate warnings (Karl-Johan Alm)
c5bcc7d util: warn about recursive -includeconf arguments in configuration files (Karl-Johan Alm)
Pull request description:
This is a follow-up PR to #10267, and addresses https://github.com/bitcoin/bitcoin/pull/10267#issuecomment-387546144.
~~I am adding extra work for @jnewbery in #12755 here -- maybe I should just rebase on top of that, but not sure what the appropriate approach is here.~~
Tree-SHA512: 87f0c32436b70424e33616ffb88d7cb699f90d6a583a10237e224b28fc936d6a9df95536c8c52ee8546b3942da92b2a357e61bf87e00d1462bc10d46d3bee352
beee49b [tests] Allow stderr to be tested against specified string (John Newbery)
e503671 [Tests] Use LIBC_FATAL_STDERR_=1 in tests (John Newbery)
c22ce8a [Tests] Write stdout/stderr to datadir instead of temp file. (John Newbery)
Pull request description:
**Due to a merge conflict, this is now based on #10267. Please review that PR first!**
Subset of #12379 now that parts of that PR have been merged.
#12362 was only observed when running the functional tests locally because:
- by defatul libc logs to `/dev/tty` instead of stderr
- the functional tests only check for substring inclusion in stderr when we're expecting bitcoind to fail.
This PR tightens our checking of stderr and will cause tests to fail if there is any unexpected message in stderr:
- commit *Write stdout/stderr to datadir instead of temp file* writes stderr to a file in the datadir instead of a temporary file. This helps with debugging in the case of failure.
- commit *Use LIBC_FATAL_STDERR=1 in tests* ensures that libc failures are logged to stderr instead of the terminal.
commit *Assert that bitcoind stdout is empty on shutdown* asserts that stderr is empty on bitcoind shutdown.
Tree-SHA512: 21111030e667b3b686f2a7625c2b625ebcfb6998e1cccb4f3932e8b5d21fb514b19a73ac971595d049343430e9a63155986a7f5648cad55b8f36f3c58b1c7048
59cb722fd050393a69f1e0df97d857c893d19d80 Update configure to reject unsafe miniUPnPc API ver (Hennadii Stepanov)
ab2190557ec2757fa48b52855b05561854af49af doc: Add release notes for 15993 (Hennadii Stepanov)
02709e95601c6020a87a6a05ee1d00c13fc38f9b Align formatting with clang-format (Hennadii Stepanov)
91a1b8508358d04685391651aea303ebce1c3d05 Use PACKAGE_NAME in UPnP description (Hennadii Stepanov)
9f76e45b9d6671e2074fb7a3885db703045a791f 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 59cb722fd050393a69f1e0df97d857c893d19d80. 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
fa8548c5d1 net: Remove unused unsanitized user agent string CNode::strSubVer (MarcoFalke)
Pull request description:
I fail to see a use case for this unsanitized byte array. In fact this can easily be confused with `cleanSubVer` and be displayed to the user (or logged) by a simple typo that is hard to find in review.
Further reading: https://btcinformation.org/en/developer-reference#version
ACKs for commit fa8548:
promag:
utACK fa8548c, good catch.
practicalswift:
utACK fa8548c5d13957f57f9b1e20e03002600962f7f0
sipa:
utACK fa8548c5d13957f57f9b1e20e03002600962f7f0
Tree-SHA512: 3c3ff1504d1583ad099df9a6aa761458a82ec48a58ef7aaa9b5679a5281dd1b59036ba2932ed708488951a565b669a3083ef70be5a58472ff8677b971162ae2f
8a2656702b4b5d53d1b8343c3215302e4305a038 torcontrol: Use the default/standard network port for Tor hidden services, even if the internal port is set differently (Luke Dashjr)
Pull request description:
Currently, the hidden service is published on the same port as the public listening port.
But if a non-standard port is configured, this can be used to guess (pretty reliably) that the public IP and the hidden service are the same node.
ACKs for top commit:
practicalswift:
utACK 8a2656702b4b5d53d1b8343c3215302e4305a038
naumenkogs:
utACK 8a26567
laanwj:
utACK 8a2656702b4b5d53d1b8343c3215302e4305a038
Tree-SHA512: 737c8da4f7c3f0bb22a338647d357987f5808156e3f38864168d0d8c2e2b171160812f7da4de11eef602902b304e357d76052950b72d7b3b83535b0fdd05fadc
ef0019e054734a14214dfbce56611ce4db1688a5 Generate log entry when blocks messages are received unexpectedly. (Patrick Strateman)
Pull request description:
Currently these are incorrectly logged as an unknown command.
Tree-SHA512: dd272388a90b79897f8c1ea6d4c949323fcf75493f3a5b2ec9a26a2cf6a8ee743b497941702f21df8fae0f5b9481444363643379832dbd5053b0cc0b0363de04
6170ec5 Do not query all DNS seed at once (Pieter Wuille)
Pull request description:
Before this PR, when we don't have enough connections after 11 seconds, we proceed to query all DNS seeds in a fixed order, loading responses from all of them.
Change this to to only query three randomly-selected DNS seed. If 11 seconds later we still don't have enough connections, try again with another one, and so on.
This reduces the amount of information DNS seeds can observe about the requesters by spreading the load over all of them.
ACKs for top commit:
Sjors:
ACK 6170ec5
sdaftuar:
ACK 6170ec5
jonasschnelli:
utACK 6170ec5 - I think the risk of a single seeder codebase is orthogonal to this PR. Such risks could also be interpreted differently (diversity could also increase the risk based on the threat model).
fanquake:
ACK 6170ec5 - Agree with the reasoning behind the change. Did some testing with and without `-forcednsseed` and/or a `peers.dat` and monitored the DNS activity.
Tree-SHA512: 33f6be5f924a85d312303ce272aa8f8d5e04cb616b4b492be98832e3ff37558d13d2b16ede68644ad399aff2bf5ff0ad33844e55eb40b7f8e3fddf9ae43add57
20e6ea259b222b10f066f22695a5f56c52071f63 [addrman] Improve collision logging and address nits (Suhas Daftuar)
f71fdda3bc2e7acd2a8b74e882364866b8b0f55b [addrman] Ensure collisions eventually get resolved (Suhas Daftuar)
4991e3c813c9848d3b3957ea3ad433f02fca9e81 [net] feeler connections can be made to outbound peers in same netgroup (Suhas Daftuar)
4d834018e368c3481a5421891395f64aa9002185 [addrman] Improve tried table collision logging (Suhas Daftuar)
Pull request description:
The restriction on outbound peers sharing the same network group is not intended to apply to feeler connections, so fix this.
This fixes an issue where a tried table collision with an entry to a netgroup we already have an outbound connection to could cause feelers to stop working, because the tried collision buffer (`m_tried_collisions`) would never be drained.
Also, ensure that all entries don't linger in `m_tried_collisions` by evicting an old entry if its collisions is unresolved after 40 minutes.
Tree-SHA512: 553fe2b01b82cd7f0f62f90c6781e373455a45b254e3bec085b5e6b16690aa9f3938e8c50e7136f19dafa250ed4578a26227d944b76daf9ce4ef0c75802389b6
* instantsend: make stuff const where possible
Signed-off-by: pasta <pasta@dashboost.org>
* instantsend: remove unused `params`
Signed-off-by: pasta <pasta@dashboost.org>
* instantsend: combine two nested if's into one
Signed-off-by: pasta <pasta@dashboost.org>
* instantsend: use auto in spots where possible and clear
Signed-off-by: pasta <pasta@dashboost.org>
* coinjoin: make IsValidStructure const
Signed-off-by: pasta <pasta@dashboost.org>
* coinjoin: divide by an integer to avoid double implicit conversions
Signed-off-by: pasta <pasta@dashboost.org>
* coinjoin: make unused parameter unnamed
Signed-off-by: pasta <pasta@dashboost.org>
* quorums.* use const and use references where possible
Signed-off-by: pasta <pasta@dashboost.org>
* quorums.h don't return const when returning by value
Signed-off-by: pasta <pasta@dashboost.org>
* quorums_blockprocessor.cpp remove redundant casts and combine two if statements
Signed-off-by: pasta <pasta@dashboost.org>
* quorums_blockprocessor.cpp make values const
Signed-off-by: pasta <pasta@dashboost.org>
* quorums_chainlocks.cpp access static function statically
Signed-off-by: pasta <pasta@dashboost.org>
* quorums_chainlocks.h remove commented out include
Signed-off-by: pasta <pasta@dashboost.org>
* quorums_commitment.cpp remove redundant casts
Signed-off-by: pasta <pasta@dashboost.org>
* quorums_debug.cpp remove redundant casts and add const
Signed-off-by: pasta <pasta@dashboost.org>
* quorums_dkgsession.cpp use const where possible
Signed-off-by: pasta <pasta@dashboost.org>
* quorums_dkgsessionhandler.cpp use const where possible
Signed-off-by: pasta <pasta@dashboost.org>
* quorums_dkgsessionhandler.cpp don't use std::move which apparently prevents "copy elision" in this instance
Signed-off-by: pasta <pasta@dashboost.org>
* quorums_dkgsessionhandler.cpp use const
Signed-off-by: pasta <pasta@dashboost.org>
* quorums_dkgsessionmgr.cpp misc refactoring
Signed-off-by: pasta <pasta@dashboost.org>
* quorums_signing.* misc refactoring
Signed-off-by: pasta <pasta@dashboost.org>
* quorums_signing_shares.* misc refactoring
Signed-off-by: pasta <pasta@dashboost.org>
* quorums_utils.cpp misc refactoring
Signed-off-by: pasta <pasta@dashboost.org>
ee5e896 Organise Linux build instructions to be categorised by distro (Alex Vear)
4c85517 Add NetBSD build instruction links (Alex Vear)
Pull request description:
* Added references to the newly created [`doc/build-netbsd.md`] (#12294) instructions in the [`doc/README.md`] and the [`doc/build-unix.md`] files.
* Organise [`doc/build-unix.md`] dependency build instructions by Linux distribution. This will help discoverability of dependency build instructions for specific distros. Future instructions will also be able to be added easier.
I am not quite sure about the FreeBSD instructions being in the [`doc/build-unix.md`], while both the OpenBSD and NetBSD instructions are contained within separate files ([`doc/build-openbsd.md`] and [`doc/build-netbsd.md`] respectively).
Feedback is greatly appreciated. 😄
[`doc/build-netbsd.md`]:https://github.com/bitcoin/bitcoin/blob/master/doc/build-netbsd.md
[`doc/build-unix.md`]:https://github.com/bitcoin/bitcoin/blob/master/doc/build-unix.md
[`doc/build-openbsd.md`]:https://github.com/bitcoin/bitcoin/blob/master/doc/build-openbsd.md
[`doc/README.md`]:https://github.com/bitcoin/bitcoin/blob/master/doc/README.md
Tree-SHA512: ebe2604d1802795851bbfce2335f159b53ea696bc9afb309be7825c697b992cc3963270fe945ca3e449b18522046e227fde3fae1b9c01bd49c3a7a513b5bd40c
be5ca825a38bc71c3a79ef35335e9c2e597ad225 doc: update NetBSD build instructions for 8.0 (fanquake)
Pull request description:
Updates the NetBSD build documentation for 8.0.
Use Python37 and add pkg-config.
Switches to using our `contrib/install_db4.sh` script for installing bdb.
Tree-SHA512: c0ac1a89349a752d9d4d87e2d134fd402e9beaac84e471ec9a0b507ebc5e762e973c8d2821db3824dea82841e38c39b0bb0a0d97f7e58318f2b15e93e81bf654
107623c26c2113428446effaa24edb986a3e780c net: Correct comparison of addr count (Carl Dong)
Pull request description:
`LOCAL_NONE` is supposed to be an enum indicating the `nScore` of a
`LocalServiceInfo` rather than the count of an addr in `mapLocalHost`.
Tree-SHA512: a47a0859dd11c991d75b54e96b08c502e3d235f7a6522a2355053f377d05e7853483996919292f458d917a561b23951e6945d6bf0ff5a2f29513c477c640bdd2
d48f664440e7bb3ff7a90b6d706a3ac2cfaec95a tests: Fix fs_tests for unknown locales (Daki Carnhof)
Pull request description:
Fix by removing "L" as suggested by meeDamian in
https://github.com/bitcoin/bitcoin/issues/14948#issuecomment-522355441
```
# all in .../bitcoin/src/test
$ uname -m
x86_64
$ export LC_ALL=randomnonexistentlocale
$ ./test_bitcoin
Running 369 test cases...
unknown location(0): fatal error: in "fs_tests/fsbridge_fstream": boost::system::system_error: boost::filesystem::path codecvt to string: error
test/fs_tests.cpp(13): last checkpoint: "fsbridge_fstream" test entry
*** 1 failure is detected in the test module "Bitcoin Core Test Suite"
```
After the patch is applied, the same test under the same conditions runs fine.
```
$ export LC_ALL=randomnonexistentlocale
$ ./test_bitcoin
Running 369 test cases...
*** No errors detected
```
Co-Authored-By: bugs@meedamian.com
ACKs for top commit:
laanwj:
ACK d48f664440e7bb3ff7a90b6d706a3ac2cfaec95a
Tree-SHA512: a9910252b8ce6a05cab5530874549c2999ca2c28e835fc18aa8e5468fb417bd7d245864ec71d9233dd53e02940a9f0691b247430257f27eb0d7c20745d1c846d
232f96f5c8a3920c09db92f4dbac2ad7d10ce8cf doc: Add release notes for -avoidpartialspends (Karl-Johan Alm)
e00b4699cc6d2ee5697d38dd6607eb2631c9b77a clean-up: Remove no longer used ivars from CInputCoin (Karl-Johan Alm)
43e04d13b1ffc02b1082176e87f420198b40c7b1 wallet: Remove deprecated OutputEligibleForSpending (Karl-Johan Alm)
0128121101fb3ee82f3abd3973a967a4226ffe0e test: Add basic testing for wallet groups (Karl-Johan Alm)
59d6f7b4e2f847ec1f2ff46c84e6157655984f85 wallet: Switch to using output groups instead of coins in coin selection (Karl-Johan Alm)
87ebce25d66952f5ce565bb5130dcf5e24049872 wallet: Add output grouping (Karl-Johan Alm)
bb629cb9dc567cc819724d9f4852652926e60cbf Add -avoidpartialspends and m_avoid_partial_spends (Karl-Johan Alm)
65b3eda458221644616d0fdd6ba0fe01bdbce893 wallet: Add input bytes to CInputCoin (Karl-Johan Alm)
a443d7a0ca333b0bae63e04b5d476f9ad9c7aeac moveonly: CoinElegibilityFilter into coinselection.h (Karl-Johan Alm)
173e18a289088c6087ba6fac708e322aa63b7a94 utils: Add insert() convenience templates (Karl-Johan Alm)
Pull request description:
This PR adds an optional (off by default) `-avoidpartialspends` flag, which changes coin select to use output groups rather than outputs, where each output group corresponds to all outputs with the same destination.
It is a privacy improvement, as each time you spend some output, any other output that is publicly associated with the destination (address) will also be spent at the same time, at the cost of fee increase for cases where coin select without group restriction would find a more optimal set of coins (see example below).
For regular use without address reuse, this PR should have no effect on the user experience whatsoever; it only affects users who, for some reason, have multiple outputs with the same destination (i.e. address reuse).
Nodes with this turned off will still try to avoid partial spending, if the fee of the resulting transaction is not greater than the fee of the original transaction.
Example: a node has four outputs linked to two addresses `A` and `B`:
* 1.0 btc to `A`
* 0.5 btc to `A`
* 1.0 btc to `B`
* 0.5 btc to `B`
The node sends 0.2 btc to `C`. Without `-avoidpartialspends`, the following coin selection will occur:
* 0.5 btc to `A` or `B` is picked
* 0.2 btc is output to `C`
* 0.3 - fee is output to (unique change address)
With `-avoidpartialspends`, the following will instead happen:
* Both of (0.5, 1.0) btc to `A` or `B` is picked (one or the other pair)
* 0.2 btc is output to `C`
* 1.3 - fee is output to (unique change address)
As noted, the pro here is that, assuming nobody sends to the address after you spend from it, you will only ever use one address once. The con is that the transaction becomes slightly larger in this case, because it is overpicking outputs to adhere to the no partial spending rule.
This complements #10386, in particular it addresses @luke-jr and @gmaxwell's concerns in https://github.com/bitcoin/bitcoin/pull/10386#issuecomment-300667926 and https://github.com/bitcoin/bitcoin/pull/10386#issuecomment-302361381.
Together with `-avoidreuse`, this fully addresses the concerns in #10065 I believe.
Tree-SHA512: 24687a4490ba59cf4198ed90052944ff4996653a4257833bb52ed24d058b3e924800c9b3790aeb6be6385b653b49e304453e5d7ff960e64c682fc23bfc447621
# Conflicts:
# src/Makefile.am
# src/bench/coin_selection.cpp
# src/wallet/coincontrol.h
# src/wallet/coinselection.cpp
# src/wallet/coinselection.h
# src/wallet/init.cpp
# src/wallet/test/coinselector_tests.cpp
# src/wallet/wallet.cpp
# src/wallet/wallet.h
# test/functional/test_runner.py
