19a354b11f Output a descriptor in createmultisig and addmultisigaddress (Andrew Chow)
Pull request description:
Give a descriptor from `createmultisig` and `addmultisigaddress`.
Extracted from #16528 with `addmultisgaddress` and tests added.
ACKs for top commit:
Sjors:
tACK 19a354b11f
MarcoFalke:
ACK 19a354b11f
promag:
Code review ACK 19a354b11f.
meshcollider:
utACK 19a354b11f
Tree-SHA512: e813125fbbc358ea8d45b1748de16a29a94efd83175b748fb8fa3b0bfc8e783ed36b6c554d84f5d4ead1ba252a83a3e937b6c3f75da7b8d3b4e55f94d6013771
acf8abc7f3 gui: Fix unintialized WalletView::progressDialog (João Barbosa)
Pull request description:
#17911 shows that it's possible to read the unintialized `progressDialog` in f32564f0a7/src/qt/walletview.cpp (L296-L297).
And the debugger shows
```
(gdb) bt
#0 0x0000555556687c60 in QProgressDialog::wasCanceled() const ()
#1 0x000055555572989f in WalletView::showProgress (this=0x5555577d7a70,
title=..., nProgress=1) at qt/walletview.cpp:322
```
Closes#17911.
ACKs for top commit:
hebasto:
ACK acf8abc7f3, I have reviewed the code and it looks OK, I agree it can be merged.
elichai:
utACK acf8abc7f3
kristapsk:
ACK acf8abc7f3
MarcoFalke:
ACK acf8abc7f3
Tree-SHA512: f5e6d873192d08d1a572e66e17c2e06d1ce27d01aa196b2a7ed591008641295bb02cda8ac90919ff2d2fc778316c2e143f8d36599e0d377779758853dfaf0a31
acd644b83d build: remove --large-address-aware linker flag (fanquake)
Pull request description:
This flag was used when building 32-bit Windows executables, which we no-longer
do, and is not accepted by the linker for any of the hosts we currently build
for. i.e:
```bash
checking whether the linker accepts -Wl,--large-address-aware... no
```
--large-address-aware
If given, the appropriate bit in the "Characteristics" field of the COFF
header is set to indicate that this executable supports virtual addresses
greater than 2 gigabytes. This should be used in conjunction with the /3GB
or /USERVA=value megabytes switch in the "[operating systems]" section of
the BOOT .INI. Otherwise, this bit has no effect. [This option is specific
to PE targeted ports of the linker]
You can check that the appropriate bit in the COFF header of our 64-bit
Windows binaries is still be set using dumpbin. i.e:
```powershell
dumpbin /headers .\bitcoind.exe
FILE HEADER VALUES
<snip>
26 characteristics
Executable
Line numbers stripped
Application can handle large (>2GB) addresses
```
ACKs for top commit:
laanwj:
ACK acd644b83d
Tree-SHA512: 9711e07bc08e843fcefd0517091a59cb7670dd107d03623a146d03fe73054d0e64f78489490b37f4708eab2c4800037f923b9ec92e7f53c3df9a590242f52b55
clock_gettime(), CLOCK_MONOTONIC and CLOCK_REALTIME are all available for use on
macOS (now that we require macOS >=10.12). Use them rather than the deprecated
mach_timespec_t time API.
master:
2019-12-23T20:49:43Z Feeding 216 bytes of dynamic environment data into RNG
2019-12-23T20:50:43Z Feeding 216 bytes of dynamic environment data into RNG
this commit:
2019-12-23T20:32:41Z Feeding 232 bytes of dynamic environment data into RNG
2019-12-23T20:33:42Z Feeding 232 bytes of dynamic environment data into RNG
900d8f6f70 util: Disallow network-qualified command line options (Russell Yanofsky)
Pull request description:
Previously these were allowed but ignored.
This change implements one of the settings simplifications listed in #17508. Change includes release notes.
ACKs for top commit:
laanwj:
ACK 900d8f6f70
Tree-SHA512: ab020a16a86c1e8ec709fbf798d533879d32c565eceeb7eb785c33042c49c6b4d1108c5453d8166e4a2abffc2c8802fbb6d3b895e0ddeefa8f274fd647e3c8ad
e9434ee03e Remove false positive GCC warning (Hennadii Stepanov)
Pull request description:
On master (f05c1ac444) GCC compiler fires a false positive `-Wmaybe-uninitialized`:
```
wallet/wallet.cpp: In static member function ‘static std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(interfaces::Chain&, const WalletLocation&, std::__cxx11::string&, std::vector<std::__cxx11::basic_string<char> >&, uint64_t)’:
wallet/wallet.cpp:3913:27: warning: ‘*((void*)& time_first_key +8)’ may be used uninitialized in this function [-Wmaybe-uninitialized]
Optional<int64_t> time_first_key;
^~~~~~~~~~~~~~
```
The same as #15292.
This PR leverages a workaround and removes the warning.
ACKs for top commit:
laanwj:
ACK e9434ee03e, removes the warning for me (gcc 7.4.0)
kristapsk:
ACK e9434ee03e
Tree-SHA512: 8820a8ba6a75aa6b1ac675a38c883a77f12968b010533b6383180aa66e7e0d570bf6300744903ead91cf9084e5345144959cd6b0cea1b763190b8dd49bacce75
c0bc453135 build: remove deprecated key from macOS Info.plist (fanquake)
Pull request description:
Note that the current release binaries show correct version numbers everywhere in the GUI and macOS info dialogs (except for when you "space" click the app, shown in screenshots), and we haven't reintroduced the issue that #14701 fixed. This is just swapping a deprecated field for a newer one, as well as using the entire version string in two fields that we hadn't been previously.
Follows up discussion in #14701.
0.19.0.1
This PR.
ACKs for top commit:
laanwj:
ACK c0bc453135
Tree-SHA512: 6191056d0cb6072b8a2170c8441ebfe500cf00cd41014bf5ee68fbf60b5bb5642e2fad9541f1c5abfaafdae6db3102c3add6169cefce3fc4a63d8b913ea35865
c86bc14408 Make asmap Interpret tolerant of malicious map data (Pieter Wuille)
38c2395d7a Use ASNs for mapped IPv4 addresses correctly (Pieter Wuille)
6f8c937312 Mark asmap const in statistics code (Pieter Wuille)
d58bcdc4b5 Avoid asmap copies in initialization (Pieter Wuille)
Pull request description:
Here are a few things to improve in the asmap implementation. The first two commits are just code improvements. The last one is a bugfix (the exsting code wouldn't correctly apply ASN lookups to mapped/embedded IPv4 addresses).
ACKs for top commit:
practicalswift:
ACK c86bc14408 -- patch looks correct
naumenkogs:
utACK c86bc14
laanwj:
ACK c86bc14408
jonatack:
ACK c86bc14408 code looks correct, built/ran tests, bitcoind with -asmap pointed to asmap/demo.map
Tree-SHA512: 1036f43152754d621bfbecfd3b7c7276e4670598fcaed42a3d275e51fa2cf3653e2c9e9cfa714f6c7719362541510e92171e076ac4169b55a0cc8908b2d514c0
eca56f8929 test: replace 'regtest' leftovers by self.chain (Sebastian Falbesoner)
Pull request description:
This is a follow-up PR to #16681 (fixes#18068), replacing all remaining hardcoded `"regtest"` strings in functional tests by `self.chain`.
Top commit has no ACKs.
Tree-SHA512: 96524649b33164938e5a95215991103ed7855ebab55ef788d4816b3fa5cbc03d8f3b0d39f2247a87522f289fd7f4daf25e059900b8462b5127eb154bbee89054
6c22315223 build: add additional attributes to Win installer (fanquake)
Pull request description:
Fixes: #17170.
ACKs for top commit:
hebasto:
ACK 6c22315223, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: d2ff2006b8df6a34b3a16270d3eb895b03cf6b3ca69404bc39adeb7d5e3b896ddab6ba831566dc966d8bdfba3f57ddf325762cddf3ad76d1427971d1bcc68255
317fb96de9 Add search for first blk file with pruned node (Rjected)
Pull request description:
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improve coverage are always welcome.
* All other changes should have accompanying unit tests (see `src/test/`) or
functional tests (see `test/`). Contributors should note which tests cover
modified code. If no tests exist for a region of modified code, new tests
should accompany the change.
* Bug fixes are most welcome when they come with steps to reproduce or an
explanation of the potential issue as well as reasoning for the way the bug
was fixed.
* Features are welcome, but might be rejected due to design or scope issues.
If a feature is based on a lot of dependencies, contributors should first
consider building the system outside of Bitcoin Core, if possible.
* Refactoring changes are only accepted if they are required for a feature or
bug fix or otherwise improve developer experience significantly. For example,
most "code style" refactoring changes require a thorough explanation why they
are useful, what downsides they have and why they *significantly* improve
developer experience or avoid serious programming bugs. Note that code style
is often a subjective matter. Unless they are explicitly mentioned to be
preferred in the [developer notes](/doc/developer-notes.md), stylistic code
changes are usually rejected.
-->
When bitcoind is running in pruned mode, producing a hashlist with `./linearize-hashes.py linearize.cfg > hashlist.txt` and then executing `linearize-data.py linearize.cfg` will produce:
```
Read 313001 hashes
Input file /home/dan/.bitcoin/blocks/blk00000.dat
Premature end of block data
```
This happens because `linearize-data` starts by attempting to process `blk00000.dat` regardless of whether or not `blk00000.dat` actually exists - this may not be the case if working with a pruned node.
This PR adds a function which finds the first block file that does exist, and calls that function when the `BlockDataCopier` is initialized.
This is a refactor of #16431.
<!--
Bitcoin Core has a thorough review process and even the most trivial change
needs to pass a lot of eyes and requires non-zero or even substantial time
effort to review. There is a huge lack of active reviewers on the project, so
patches often sit for a long time.
-->
ACKs for top commit:
darosior:
ACK 317fb96de9
laanwj:
Code review ACK 317fb96de9
theStack:
Code review ACK 317fb96de9
Tree-SHA512: fc8014282df6cfe7b267e64db8ce7d82b86b758c302fbfea4a3c39b62d93512f5c2e31a0de4e9c5ec18fc0268c917f011257d37b45afaef6033eec90e4aa585f
1abcecc40c Tests: Use self.chain instead of 'regtest' in almost all current tests (Jorge Timón)
Pull request description:
Simply avoiding the hardcoded string in more places for consistency.
It can also allow for more easily reusing tests for other chains other than regtest.
Separated from #8994 .
Continues #16509 .
It is still not complete (ie to be complete, we need the -chain parameter in #16680 and make whether acceptnonstdtxs is allowed for that chain or not customizable for regtest [or for custom chains like in #8994 ] ). But while being incomplete like #16509 , it's quite simple to review and another step forward IMO.
ACKs for top commit:
Sjors:
re-ACK 1abcecc. I think it's an improvement even if incomplete and if some PR's might accidentally bring "regtest" back. Subsequent improvements hopefully don't have to touch 16 files.
elichai:
Code review ACK 1abcecc40c
ryanofsky:
Code review ACK 1abcecc40c.
ryanofsky:
Code review ACK 1abcecc40c
Tree-SHA512: 5620de6dab235ca8bd8670d6366c7b9f04f0e3ca9c5e7f87765b38e16ed80c17d7d1630c0d5fd7c5526f070830d94dc74cc2096d8ede87dc7180ed20569509ee
ff59bcd321 gui: Drop PeerTableModel dependency to ClientModel (João Barbosa)
Pull request description:
Class `PeerTableModel` doesn't actually depend on `ClientModel`.
ACKs for top commit:
Empact:
Code Review ACK ff59bcd321
hebasto:
ACK ff59bcd321, tested on Linux Mint 19.3. No changes in behavior are observed.
Tree-SHA512: 29fa3c316c05b8f7b9340e5859bbb8c3a0b826aa7c865c892cfa13b5ad30f822fcaae4e01555f7860cd1727f20b7ef555a808235522a04a6eebaaa7b605f8595
0a50019fde Walk pindexBestHeader back to ChainActive().Tip() if it is invalid (Matt Corallo)
Pull request description:
Instead of keeping pindexBestHeader set to the best header we've
ever seen, reset it back to our validated tip if we find an ancestor
of it turns out to be invalid. While the name is now a bit confusing,
this matches much better with how it is used in practice, see below.
Further, this opens up more use-cases for it in the future, namely
aggressively searching for new peers in case we have discovered
(possibly via some covert channel) headers which we do not know to be
invalid, but which we cannot find block data for.
Places pindexBestHeader is used:
* Various GUI displays of the best header and getblockchaininfo["headers"],
I don't think changing this is bad, and if anything this is less confusing
in the presence of an invalid block.
* IsCurrentForFeeEstimation(): If anything I think ensuring pindexBestHeader
isn't some crazy invalid chain is better than the alternative, even in the
case where you are rejecting the current chain due to hardware error (since
hopefully in that case you won't get any new blocks anyway).
* ConnectBlock assumevalid checks: We use pindexBestHeader to check that the
block we're connecting leads to something with nMinimumChainWork (preventing
a user-set assumevalid from having bogus work) and that the block we're
connecting leads to pindexBestHeader (I'm not too worried about this one -
it's nice to "disable" assumevalid if we have a long invalid headers chain,
but I don't see it as a critical protection).
* BlockRequestAllowed() uses pindexBestHeader as its target to ensure the
requested block is within a month of the "current chain". I don't think this
is a meaningful difference, if we're rejecting the current tip we're
trivially fingerprintable anyway, and if the chain really does have a bunch
of invalid crap near the tip, using the best not-invalid header is likely a
better criteria.
* ProcessGetBlockData uses pindexBestHeader as the "current chain" definition
of whether a block request is "historical" for the purpose of bandwidth
limiting. Similarly, I don't see why this is a meaningful change.
* We use pindexBestHeader for requesting missing headers on receipt of a
headers/compact block message or block inv as well as for initial getheaders.
I think this is definitely wrong, using the best not-invalid header for such
requests is much better.
* We use pindexBestHeader to define the "current chain" for deciding when
we're close to done with initial headers sync. I don't think this is a
meaningful change.
* We use pindexBestHeader to decide if initial headers sync has timed out. If
we're rejecting the chain due to hardware error this may result in
additional cases where we ban a peer, but this is already true, so I think
its fine.
ACKs for top commit:
fjahr:
ACK 0a50019fde
kallewoof:
ACK 0a50019fde
ariard:
utACK 0a50019
Tree-SHA512: 2ecfa973a9878a00313ae7ede94a9bd7710e0caf55b544b10bbc46dc463a0478cbaf477e6cdd072356d5a0c5fb3848e9339284af785a2995c20bae8bd23f23e5
bd5a026928 Make UpdateTransactionsFromBlock use Epochs (Jeremy Rubin)
2ccb7cca4a Add Epoch Guards to CTXMemPoolEntry and CTxMemPool (Jeremy Rubin)
Pull request description:
UpdateTransactionsFromBlock is called during a re-org. When a re-org occurs, all of the transactions in the mempool may be descendants from a transaction which is in the pre-reorg block. This can cause us to propagate updates, worst case, to every transaction in the mempool.
Because we construct a `setEntries setChildren`, which is backed by a `std::set`, it is possible that this algorithm is `O(N log N)`.
By using an Epoch visitor pattern, we can limit this to `O(N)` worst case behavior.
Epochs are also less resource intensive than almost any set option (e.g., hash set) because they are allocation free.
This PR is related to https://github.com/bitcoin/bitcoin/pull/17268, it is a small subset of the changes which have been refactored slightly to ease review. If this PR gets review & merge, I will follow up with more PRs (similar to #17268) to improve the mempool
ACKs for top commit:
sdaftuar:
ACK bd5a026928
adamjonas:
Just to summarize for those looking to review - as of bd5a026 there are 3 ACKs (@sdaftuar, @ariard, and @hebasto) and one "looks good" from @ajtowns with no NACKs or any show-stopping concerns raised.
ajtowns:
ACK bd5a026928 (code review)
ariard:
Code review ACK bd5a026
hebasto:
ACK bd5a026928, modulo some nits and a typo.
Tree-SHA512: f0d2291085019ffb4e1119edeb9f4a89c1a572d1cb5b4bdf5743dd0152e721e1935f5155dcae84e1e5bda5ffdf6224c488c1e200bd33bedca9f5ca22d5f5139f
9a299a59cc net: reference instead of copy in BlockConnected range loop (Jon Atack)
Pull request description:
Reference elements in range for loop instead of copying them and
fix Clang `-Wrange-loop-analysis` warning introduced in a029e18
```
net_processing.cpp:1185:25: warning: loop variable 'ptx' of
type 'const std::shared_ptr<const CTransaction>' creates a copy from
type 'const std::shared_ptr<const CTransaction>' [-Wrange-loop-analysis]
for (const auto ptx : pblock->vtx) {
^
net_processing.cpp:1185:14: note: use reference type
'const std::shared_ptr<const CTransaction> &' to prevent copying
for (const auto ptx : pblock->vtx) {
^~~~~~~~~~~~~~~~
1 warning generated.
```
ACKs for top commit:
Empact:
ACK 9a299a59cc
MarcoFalke:
ACK 9a299a59cc
promag:
ACK 9a299a59cc.
elichai:
ACK 9a299a59cc
emilengler:
ACK 9a299a5.
Tree-SHA512: 9284d1b00684877505454a05071212758c8cea083534e2eec09bfc8a9c3059eea811d2008f6a5a678539444f0d5b3134db1bd23da6514b3d3a1440634c8b53be
d3bc184081 doc: update release notes with getaddressinfo label deprecation (Jon Atack)
72af93f364 test: getaddressinfo label deprecation test (Jon Atack)
d48875fa20 rpc: deprecate getaddressinfo label field (Jon Atack)
dc0cabeda4 test: remove getaddressinfo label tests (Jon Atack)
c7654af6f8 doc: address pr17578 review feedback (Jon Atack)
Pull request description:
This PR builds on #17578 (now merged) and deprecates the rpc getaddressinfo `label` field. The deprecated behavior can be re-enabled by starting bitcoind with `-deprecatedrpc=label`.
See http://www.erisian.com.au/bitcoin-core-dev/log-2019-11-22.html#l-622 and https://github.com/bitcoin/bitcoin/pull/17283#issuecomment-554458001 for more context.
Reviewers: This PR may be tested manually by building, then running bitcoind with and without the `-deprecatedrpc=label` flag while verifying the rpc getaddressinfo output and help text.
Next step: add support for multiple labels.
ACKs for top commit:
jnewbery:
ACK d3bc184081
laanwj:
ACK d3bc184081
meshcollider:
utACK d3bc184081
Tree-SHA512: f954402884ec54977def332c8160fd892f289b0d2aee1e91fed9ac3220f7e5b1f7fc6421b84cc7a5c824a0582eca4e6fc194e4e33ddd378c733c8941ac45f56d
cb8a86d9f9 gui: Remove WalletView and BitcoinGUI circular dependency (João Barbosa)
ac3d10777d gui: Add transactionClicked and coinsSent signals to WalletView (João Barbosa)
Pull request description:
Essentially moves the code in `WalletView::setBitcoinGUI` to the only caller. Two new signals are added beforehand in the first commit so that the connections in `WalletFrame` are all from the wallet view.
ACKs for top commit:
hebasto:
ACK cb8a86d9f9, tested on Linux Mint 19.3.
jonasschnelli:
utACK cb8a86d9f9
Tree-SHA512: 250316cd3689e51c8cded9ccd75963c836dcafa6db25d684f2aa691dea9738895f9140793e0f925784909e39f8257f7e1c7d611e8bd6d6634e1a50333f4ddb1e
3aee10b80b gui: Drop ShutdownWindow dependency to BitcoinGUI (João Barbosa)
61eb058cc1 gui: Drop BanTableModel dependency to ClientModel (João Barbosa)
Pull request description:
`ShutdownWindow::showShutdownWindow` just needs a widget to center the shutdown window and to borrow its title.
ACKs for top commit:
hebasto:
ACK 3aee10b80b, since previous review only suggested change `QWidget` --> `QMainWindow`
jonasschnelli:
utACK 3aee10b80b
Tree-SHA512: e15cb6ee274730bd071d3d97b540c5059e5c655248d69a37c3fd00f2aacc6cfcb36b9a65755718027e15482ec8e5e85534c1dc13d0ddb4e0680df03fbf6571f2
a029e18c2b Use rolling bloom filter of recent block tx's for AlreadyHave() check (Suhas Daftuar)
Pull request description:
In order to determine whether to download or process a relayed transaction, we first try to check whether we already have the transaction -- either in the mempool, in our filter of recently rejected transactions, in our orphan pool, or already confirmed in a block.
Prior to this commit, the heuristic for checking whether a transaction was confirmed in a block is based on whether there's a coin cache entry corresponding to the 0- or 1-index vout of the tx. While that is a quick check, it is very imprecise (eg if those outputs were already spent in another block, we wouldn't detect that the transaction has already been confirmed) -- we can do better by just keeping a rolling bloom filter of the transactions in recent blocks, which will better capture the case of a transaction which has been confirmed and then fully spent.
This should reduce the bandwidth that we waste by requesting transactions which will not be accepted to the mempool.
To avoid relay problems for transactions which have been included in a recent block but then reorged out of the chain, we clear the bloom filter whenever a block is disconnected.
ACKs for top commit:
MarcoFalke:
re-ACK a029e18c2b only stylistic and comment fixups 🍴
sipa:
utACK a029e18c2b
jonatack:
Code review ACK a029e18c2b also built/ran tests and am running bitcoind with mempool debug logging and custom logging. Looked a bit into CRollingBloomFilter and also the mempool median time past checks mentioned above; I don't have a deep understanding of those areas yet but the concept here and changes LGTM. Tests and other optimisations could be added as a follow-up. In favor of seeing this move forward if no major immediate concerns.
Tree-SHA512: 784c9a35bcd3af5db469063ac7d26b4bac430e451e5637a34d8a538c3ffd1433abdd3f06e5584e7a84bfa9e791449e61819397b5a6c7890fa59d78ec3ba507b2
