aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2020-02-09Merge #18032: rpc: Output a descriptor in createmultisig and addmultisigaddressMarcoFalke
19a354b11f85a3c6c81ff83bf702bf7a40cf5046 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 19a354b11f85a3c6c81ff83bf702bf7a40cf5046 MarcoFalke: ACK 19a354b11f85a3c6c81ff83bf702bf7a40cf5046 promag: Code review ACK 19a354b11f85a3c6c81ff83bf702bf7a40cf5046. meshcollider: utACK 19a354b11f85a3c6c81ff83bf702bf7a40cf5046 Tree-SHA512: e813125fbbc358ea8d45b1748de16a29a94efd83175b748fb8fa3b0bfc8e783ed36b6c554d84f5d4ead1ba252a83a3e937b6c3f75da7b8d3b4e55f94d6013771
2020-02-07Qt: pass clientmodel changes from walletframe to walletviewsJonas Schnelli
2020-02-06Convert undo.h to new serialization frameworkPieter Wuille
2020-02-06Make std::vector and prevector reuse the VectorFormatter logicPieter Wuille
2020-02-06Add custom vector-element formatterPieter Wuille
This allows a very compact notation for serialization of vectors whose elements are not serialized using their default encoding.
2020-02-06refactor: Work around GCC 9 `-Wredundant-move` warningRussell Yanofsky
Use std::move workaround for unique_ptr, for when the C++ compiler lacks a fix for this issue: http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1579 Do this in a way that avoids a GCC 9 `-Wredundant-move` warning.
2020-02-06net: Fail instead of truncate command name in CMessageHeaderWladimir J. van der Laan
Replace the memset/strncpy dance in `CMessageHeader::CMessageHeader` with explicit code that copies then name and asserts the length. This removes a warning in g++ 9.1.1 and IMO makes the code more readable by not relying on strncpy padding and silent truncation behavior.
2020-02-06Merge #18062: gui: Fix unintialized WalletView::progressDialogfanquake
acf8abc7f3cf7efa418a46f9f69f23f1a5035582 gui: Fix unintialized WalletView::progressDialog (João Barbosa) Pull request description: #17911 shows that it's possible to read the unintialized `progressDialog` in https://github.com/bitcoin/bitcoin/blob/f32564f0a73c5ad1a107dd112e40516f39d1a51e/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 acf8abc7f3cf7efa418a46f9f69f23f1a5035582, I have reviewed the code and it looks OK, I agree it can be merged. elichai: utACK acf8abc7f3cf7efa418a46f9f69f23f1a5035582 kristapsk: ACK acf8abc7f3cf7efa418a46f9f69f23f1a5035582 MarcoFalke: ACK acf8abc7f3cf7efa418a46f9f69f23f1a5035582 Tree-SHA512: f5e6d873192d08d1a572e66e17c2e06d1ce27d01aa196b2a7ed591008641295bb02cda8ac90919ff2d2fc778316c2e143f8d36599e0d377779758853dfaf0a31
2020-02-06random: don't special case clock usage on macOSfanquake
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
2020-02-05Merge #17482: util: Disallow network-qualified command line optionsWladimir J. van der Laan
900d8f6f70859f528e84c5c38d0332f81d19df55 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 900d8f6f70859f528e84c5c38d0332f81d19df55 Tree-SHA512: ab020a16a86c1e8ec709fbf798d533879d32c565eceeb7eb785c33042c49c6b4d1108c5453d8166e4a2abffc2c8802fbb6d3b895e0ddeefa8f274fd647e3c8ad
2020-02-05Merge #17804: doc: Misc RPC help fixesWladimir J. van der Laan
fa5c6622c8ecf1954e7177888ad8c97a77b16fb7 doc: Use proper RPC help syntax in importmulti (MarcoFalke) fab63111bec73859597e6ce0986f76e5e9959091 doc: Remove duplicate "comment" from listsinceblock RPC help (MarcoFalke) fa04cd6cfc0330b62058ed169d621e08108dc87e doc: Properly document proxy_randomize_credentials as bool in getnetworkinfo (MarcoFalke) fa9dec7c395897e8dbbb6de7a16ec5185a609d41 doc: Fix syntax error (trailing square bracket) in finalizepsbt (MarcoFalke) faff5a60ed328d4c5fdef253e8935a351cb57bd0 doc: Fix syntax error (trailing square bracket) in walletprocesspsbt (MarcoFalke) fa0545901daad32b09511cc61c4af1400c48088d doc: Add missing "optional" to "long" estimaterawfee RPC help (MarcoFalke) Pull request description: This fixes documentation of the following RPCs: * estimaterawfee (hidden) * https://bitcoincore.org/en/doc/0.19.0/rpc/wallet/walletprocesspsbt/ * https://bitcoincore.org/en/doc/0.19.0/rpc/rawtransactions/finalizepsbt/ * https://bitcoincore.org/en/doc/0.19.0/rpc/network/getnetworkinfo/ * https://bitcoincore.org/en/doc/0.19.0/rpc/wallet/listsinceblock/ * https://bitcoincore.org/en/doc/0.19.0/rpc/wallet/importmulti/ <!-- Also, it comes with a scripted diff to normalize whitespace and type names. (Previous attempts: #14601 and #14459) ACKs for top commit: laanwj: ACK fa5c6622c8ecf1954e7177888ad8c97a77b16fb7 Tree-SHA512: 5a10956e12f8ce23e93a2ce8bafd6cae759d8a21658f79397e3bfce3e4aabd9658bdbd40acde49323dca958a9befee7166654994208c182dd60f483109621e17
2020-02-05Merge #18052: Remove false positive GCC warningWladimir J. van der Laan
e9434ee03efdfc0a5a54cf46561e95fd93cba007 Remove false positive GCC warning (Hennadii Stepanov) Pull request description: On master (f05c1ac444e0c893516535bfdf07c5c8cd9bce16) 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 e9434ee03efdfc0a5a54cf46561e95fd93cba007, removes the warning for me (gcc 7.4.0) kristapsk: ACK e9434ee03efdfc0a5a54cf46561e95fd93cba007 Tree-SHA512: 8820a8ba6a75aa6b1ac675a38c883a77f12968b010533b6383180aa66e7e0d570bf6300744903ead91cf9084e5345144959cd6b0cea1b763190b8dd49bacce75
2020-02-05Merge #18023: Fix some asmap issuesWladimir J. van der Laan
c86bc144081f960347232546f7d22deb65d27deb Make asmap Interpret tolerant of malicious map data (Pieter Wuille) 38c2395d7a905c87dc4630031849fd8e403e61bf Use ASNs for mapped IPv4 addresses correctly (Pieter Wuille) 6f8c93731203c111f86c39eaf2102f9a825d1706 Mark asmap const in statistics code (Pieter Wuille) d58bcdc4b569a667b6974c3547b7ff6f665afce9 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 c86bc144081f960347232546f7d22deb65d27deb -- patch looks correct naumenkogs: utACK c86bc14 laanwj: ACK c86bc144081f960347232546f7d22deb65d27deb jonatack: ACK c86bc144081f960347232546f7d22deb65d27deb code looks correct, built/ran tests, bitcoind with -asmap pointed to asmap/demo.map Tree-SHA512: 1036f43152754d621bfbecfd3b7c7276e4670598fcaed42a3d275e51fa2cf3653e2c9e9cfa714f6c7719362541510e92171e076ac4169b55a0cc8908b2d514c0
2020-02-05Merge #18029: tests: Add fuzzing harness for AS-mapping (asmap)Wladimir J. van der Laan
4d2aceaad8d28a54246b6639966e2278d2d795e3 tests: Add fuzzer asmap to FUZZERS_MISSING_CORPORA (temporarily) (practicalswift) 8d07706985a72b105b63efa289121d17d31607a1 tests: Add fuzzing harness for AS-mapping (asmap) (practicalswift) Pull request description: Add fuzzing harness for AS-mapping (`asmap`). To test this PR: ``` $ make distclean $ ./autogen.sh $ CC=clang CXX=clang++ ./configure --enable-fuzz \ --with-sanitizers=address,fuzzer,undefined $ make $ src/test/fuzz/asmap … ``` ACKs for top commit: MarcoFalke: ACK 4d2aceaad8d28a54246b6639966e2278d2d795e3 jonatack: ACK 4d2aceaad8d28a54246b6639966e2278d2d795e3 Tree-SHA512: bc4c63b48cd98c0cec9d10ecb43775b1bf1215241ff821fc7a866c7e2738605641fb88d044eabf2f48a8c16f2ced9ffce5165c9e6a83c73ece004350da7153e7
2020-02-03gui: Fix unintialized WalletView::progressDialogJoão Barbosa
2020-02-03gui: Drop PeerTableModel dependency to ClientModelJoão Barbosa
2020-02-03Merge #16974: Walk pindexBestHeader back to ChainActive().Tip() if it is invalidWladimir J. van der Laan
0a50019fde7781263e0c8f041d1d9dcb0dee77e8 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 0a50019fde7781263e0c8f041d1d9dcb0dee77e8 kallewoof: ACK 0a50019fde7781263e0c8f041d1d9dcb0dee77e8 ariard: utACK 0a50019 Tree-SHA512: 2ecfa973a9878a00313ae7ede94a9bd7710e0caf55b544b10bbc46dc463a0478cbaf477e6cdd072356d5a0c5fb3848e9339284af785a2995c20bae8bd23f23e5
2020-02-03Merge #17925: Improve UpdateTransactionsFromBlock with EpochsWladimir J. van der Laan
bd5a02692853f7240a4fdc593d7d0123d7916e45 Make UpdateTransactionsFromBlock use Epochs (Jeremy Rubin) 2ccb7cca4ac67198ac89bd58f5b4ae41a5163ceb 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 bd5a02692853f7240a4fdc593d7d0123d7916e45 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 bd5a02692853f7240a4fdc593d7d0123d7916e45 (code review) ariard: Code review ACK bd5a026 hebasto: ACK bd5a02692853f7240a4fdc593d7d0123d7916e45, modulo some nits and a typo. Tree-SHA512: f0d2291085019ffb4e1119edeb9f4a89c1a572d1cb5b4bdf5743dd0152e721e1935f5155dcae84e1e5bda5ffdf6224c488c1e200bd33bedca9f5ca22d5f5139f
2020-02-03Merge #18054: net: reference instead of copy in BlockConnected range loopfanquake
9a299a59cc8a9ab516e047356c5bc0e93774b557 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 https://github.com/bitcoin/bitcoin/pull/18054/commits/9a299a59cc8a9ab516e047356c5bc0e93774b557 MarcoFalke: ACK 9a299a59cc8a9ab516e047356c5bc0e93774b557 promag: ACK 9a299a59cc8a9ab516e047356c5bc0e93774b557. elichai: ACK 9a299a59cc8a9ab516e047356c5bc0e93774b557 emilengler: ACK 9a299a5. Tree-SHA512: 9284d1b00684877505454a05071212758c8cea083534e2eec09bfc8a9c3059eea811d2008f6a5a678539444f0d5b3134db1bd23da6514b3d3a1440634c8b53be
2020-02-02net: reference instead of copy in BlockConnected range loopJon Atack
to fix -Wrange-loop-analysis warning introduced in a029e18
2020-02-02Merge #17585: rpc: deprecate getaddressinfo labelSamuel Dobson
d3bc18408146e91b3836f72360ff6fa2420b6887 doc: update release notes with getaddressinfo label deprecation (Jon Atack) 72af93f36479dc12d795f1d05fa3d8fbd9b293bd test: getaddressinfo label deprecation test (Jon Atack) d48875fa20d0b71b978cb3d1f85dd9ec14e664cc rpc: deprecate getaddressinfo label field (Jon Atack) dc0cabeda49a7edbfa71df22846721b6f6224aea test: remove getaddressinfo label tests (Jon Atack) c7654af6f830577a54df12b5d65df93532db0dc2 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 d3bc18408146e91b3836f72360ff6fa2420b6887 laanwj: ACK d3bc18408146e91b3836f72360ff6fa2420b6887 meshcollider: utACK d3bc18408146e91b3836f72360ff6fa2420b6887 Tree-SHA512: f954402884ec54977def332c8160fd892f289b0d2aee1e91fed9ac3220f7e5b1f7fc6421b84cc7a5c824a0582eca4e6fc194e4e33ddd378c733c8941ac45f56d
2020-02-01Remove false positive GCC warningHennadii Stepanov
2020-02-01Merge #17937: gui: Remove WalletView and BitcoinGUI circular dependencyJonas Schnelli
cb8a86d9f952401eaad68b2e3818ce50f7befd91 gui: Remove WalletView and BitcoinGUI circular dependency (João Barbosa) ac3d10777d65b68862c6deb57594c8fc4d21ca77 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 cb8a86d9f952401eaad68b2e3818ce50f7befd91, tested on Linux Mint 19.3. jonasschnelli: utACK cb8a86d9f952401eaad68b2e3818ce50f7befd91 Tree-SHA512: 250316cd3689e51c8cded9ccd75963c836dcafa6db25d684f2aa691dea9738895f9140793e0f925784909e39f8257f7e1c7d611e8bd6d6634e1a50333f4ddb1e
2020-02-01Merge #18036: gui: Break trivial circular dependenciesJonas Schnelli
3aee10b80b9d9a0f5172fc2ee75f03a37d5c3863 gui: Drop ShutdownWindow dependency to BitcoinGUI (João Barbosa) 61eb058cc10592cfa314ba2209fb370706100e8b 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 3aee10b80b9d9a0f5172fc2ee75f03a37d5c3863, since previous review only suggested change `QWidget` --> `QMainWindow` jonasschnelli: utACK 3aee10b80b9d9a0f5172fc2ee75f03a37d5c3863 Tree-SHA512: e15cb6ee274730bd071d3d97b540c5059e5c655248d69a37c3fd00f2aacc6cfcb36b9a65755718027e15482ec8e5e85534c1dc13d0ddb4e0680df03fbf6571f2
2020-01-31Make asmap Interpret tolerant of malicious map dataPieter Wuille
2020-01-31Use ASNs for mapped IPv4 addresses correctlyPieter Wuille
2020-01-31Mark asmap const in statistics codePieter Wuille
2020-01-31Avoid asmap copies in initializationPieter Wuille
2020-01-31Merge #17951: Use rolling bloom filter of recent block txs for AlreadyHave() ↵Jonas Schnelli
check a029e18c2bf67dd00552b0f4bbc85fa2fa5b973b 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 a029e18c2bf67dd00552b0f4bbc85fa2fa5b973b jonatack: Code review ACK a029e18c2bf67dd00552b0f4bbc85fa2fa5b973b 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
2020-01-31tests: Add fuzzing harness for functions taking floating-point types as inputpracticalswift
2020-01-31Add missing includespracticalswift
2020-01-31gui: Drop ShutdownWindow dependency to BitcoinGUIJoão Barbosa
2020-01-31gui: Drop BanTableModel dependency to ClientModelJoão Barbosa
2020-01-30Output a descriptor in createmultisig and addmultisigaddressAndrew Chow
2020-01-31Merge #16115: On bitcoind startup, write config args to debug.logMarcoFalke
b951b0973cfd4e0db4607a00d434a04afb0d6199 on startup, write config options to debug.log (Larry Ruane) Pull request description: When a developer is examining `debug.log` after something goes wrong, it's often useful to know the exact options the failing instance of `bitcoind` was started with. Sometimes the `debug.log` file is all that's available for the analysis. This PR logs the `bitcoin.conf` entries and command-line arguments to `debug.log` on startup. ACKs for top commit: MarcoFalke: ACK b951b0973cfd4e0db4607a00d434a04afb0d6199 🐪 jonatack: ACK b951b0973c reviewed diff, re-code review, built, ran tests, launched bitcoind and reviewed debug log output, verified value of `str` debug log in the added unit test. Tree-SHA512: bbca4fb3d49f99261758302bde0b8b67300ccc72e7380b01f1f66a146ae8a008a045df0ca5ca9664caff034d0ee38ea7ef38a50f38374525608c07ba52790358
2020-01-30tests: Add fuzzing harness for AS-mapping (asmap)practicalswift
2020-01-31Merge #18009: tests: Add fuzzing harness for strprintf(…)MarcoFalke
cc668d06fb71463fd406df761b0e89e25d4de968 tests: Add fuzzing harness for strprintf(...) (practicalswift) ccc3c76e2b5d28a2372ae5752c08256396bf43e6 tests: Add fuzzer strprintf to FUZZERS_MISSING_CORPORA (temporarily) (practicalswift) 6ef04912af7f216f3112e0e9919f67e36415a792 tests: Update FuzzedDataProvider.h from upstream (LLVM) (practicalswift) Pull request description: Add fuzzing harness for `strprintf(…)`. Update `FuzzedDataProvider.h`. Avoid hitting some issues in tinyformat (reported upstreams in https://github.com/c42f/tinyformat/issues/70). --- Found issues in tinyformat: **Issue 1.** The following causes a signed integer overflow followed by an allocation of 9 GB of RAM (or an OOM in memory constrained environments): ``` strprintf("%.777777700000000$", 1.0); ``` **Issue 2.** The following causes a stack overflow: ``` strprintf("%987654321000000:", 1); ``` **Issue 3.** The following causes a stack overflow: ``` strprintf("%1$*1$*", -11111111); ``` **Issue 4.** The following causes a `NULL` pointer dereference: ``` strprintf("%.1s", (char *)nullptr); ``` **Issue 5.** The following causes a float cast overflow: ``` strprintf("%c", -1000.0); ``` **Issue 6.** The following causes a float cast overflow followed by an invalid integer negation: ``` strprintf("%*", std::numeric_limits<double>::lowest()); ``` Top commit has no ACKs. Tree-SHA512: 9b765559281470f4983eb5aeca94bab1b15ec9837c0ee01a20f4348e9335e4ee4e4fecbd7a1a5a8ac96aabe0f9eeb597b8fc9a2c8faf1bab386e8225d5cdbc18
2020-01-31Merge #18018: tests: reset fIsBareMultisigStd after bare-multisig testsMarcoFalke
1b96a3cd1ebe725896f59614903184289fe62cf8 tests: reset fIsBareMultisigStd after bare-multisig tests (fanquake) Pull request description: Fixes: #18015 The bug this fixes is two-part. 1. The `fIsBareMultisigStd` global is being reused by other tests, such as [script_p2sh_tests(set)](https://github.com/bitcoin/bitcoin/blob/master/src/test/script_p2sh_tests.cpp#L150), after being set to false. 2. The order our tests run in doesn't always? seem to be random, which meant that the `script_p2sh` tests would only fail if they were run in an order where the `transaction_tests` ran first, mutating the `fIsBareMultisigStd` global. This doesn't seem to happen when running make check, but if you run `src/test/test_bitcoin and pass --random=99999`, the failure in `script_p2sh` will occur (on most, but maybe not all systems): ```bash src/test/test_bitcoin --random=99999 Running 389 test cases... test/script_p2sh_tests.cpp:200: error: in "script_p2sh_tests/set": txTo[1].IsStandard test/script_p2sh_tests.cpp:200: error: in "script_p2sh_tests/set": txTo[2].IsStandard test/script_p2sh_tests.cpp:200: error: in "script_p2sh_tests/set": txTo[3].IsStandard *** 3 failures are detected in the test module "Bitcoin Core Test Suite" ``` The new test for bare multisig was introduced in #17502. ACKs for top commit: Empact: Code Review ACK https://github.com/bitcoin/bitcoin/pull/18018/commits/1b96a3cd1ebe725896f59614903184289fe62cf8 theStack: ACK https://github.com/bitcoin/bitcoin/pull/18018/commits/1b96a3cd1ebe725896f59614903184289fe62c Tree-SHA512: fd7578f9f3faa44d236cd007fc25e31f061acabdb8458559fde0e67d11ab5cafed15305993270c9943a50326574bc5f5301b09494a5b0d2de69e64978093ed45
2020-01-30gui: Remove WalletView and BitcoinGUI circular dependencyJoão Barbosa
2020-01-30gui: Add transactionClicked and coinsSent signals to WalletViewJoão Barbosa
2020-01-30psbt_wallet_tests: use unique_ptr for GetSigningProviderAnthony Towns
2020-01-29Add a constant for the maximum vector allocation (5 Mbyte)Pieter Wuille
2020-01-30Merge #17261: Make ScriptPubKeyMan an actual interface and the wallet to ↵Samuel Dobson
have multiple 3f373659d732a5b1e5fdc692a45b2b8179f66bec Refactor: Replace SigningProvider pointers with unique_ptrs (Andrew Chow) 3afe53c4039103670cec5f9cace897ead76e20a8 Cleanup: Drop unused GUI learnRelatedScripts method (Andrew Chow) e2f02aa59e3402048269362ff692d49a6df35cfd Refactor: Copy CWallet signals and print function to LegacyScriptPubKeyMan (Andrew Chow) c729afd0a3b74a3943e4c359270beaf3e6ff8a7b Box the wallet: Add multiple keyman maps and loops (Andrew Chow) 4977c30d59e88a3e5ee248144bcc023debcd895b refactor: define a UINT256_ONE global constant (Andrew Chow) 415afcccd3e5583defdb76e3a280f48e98983301 HD Split: Avoid redundant upgrades (Andrew Chow) 01b4511206e399981a77976deb15785d18db46ae Make UpgradeKeyMetadata work only on LegacyScriptPubKeyMan (Andrew Chow) 4a7e43e8460127a40a7895519587399feff3b682 Store p2sh scripts in AddAndGetDestinationForScript (Andrew Chow) 501acb5538008d98abe79288b92040bc186b93f3 Always try to sign for all pubkeys in multisig (Andrew Chow) 81610eddbc57c46ae243f45d73e715d509f53a6c List output types in an array in order to be iterated over (Andrew Chow) eb81fc3ee58d3e88af36d8091b9e4017a8603b3c Refactor: Allow LegacyScriptPubKeyMan to be null (Andrew Chow) fadc08ad944cad42e805228cdd58e0332f4d7184 Locking: Lock cs_KeyStore instead of cs_wallet in legacy keyman (Andrew Chow) f5be479694d4dbaf59eef562d80fbeacb3bb7dc1 wallet: Improve CWallet:MarkDestinationsDirty (João Barbosa) Pull request description: Continuation of wallet boxes project. Actually makes ScriptPubKeyMan an interface which LegacyScriptPubkeyMan. Moves around functions and things from CWallet into LegacyScriptPubKeyMan so that they are actually separate things without circular dependencies. *** Introducing the `ScriptPubKeyMan` (short for ScriptPubKeyManager) for managing scriptPubKeys and their associated scripts and keys. This functionality is moved over from `CWallet`. Instead, `CWallet` will have a pointer to a `ScriptPubKeyMan` for every possible address type, internal and external. It will fetch the correct `ScriptPubKeyMan` as necessary. When fetching new addresses, it chooses the `ScriptPubKeyMan` based on address type and whether it is change. For signing, it takes the script and asks each `ScriptPubKeyMan` for whether that `ScriptPubKeyMan` considers that script `IsMine`, whether it has that script, or whether it is able to produce a signature for it. If so, the `ScriptPubKeyMan` will provide a `SigningProvider` to the caller which will use that in order to sign. There is currently one `ScriptPubKeyMan` - the `LegacyScriptPubKeyMan`. Each `CWallet` will have only one `LegacyScriptPubKeyMan` with the pointers for all of the address types and change pointing to this `LegacyScriptPubKeyMan`. It is created when the wallet is loaded and all keys and metadata are loaded into it instead of `CWallet`. The `LegacyScriptPubKeyMan` is primarily made up of all of the key and script management that used to be in `CWallet`. For convenience, `CWallet` has a `GetLegacyScriptPubKeyMan` which will return the `LegacyScriptPubKeyMan` or a `nullptr` if it does not have one (not yet implemented, but callers will check for the `nullptr`). For purposes of signing, `LegacyScriptPubKeyMan`'s `GetSigningProvider` will return itself rather than a separate `SigningProvider`. This will be different for future `ScriptPubKeyMan`s. The `LegacyScriptPubKeyMan` will also handle the importing and exporting of keys and scripts instead of `CWallet`. As such, a number of RPCs have been limited to work only if a `LegacyScriptPubKeyMan` can be retrieved from the wallet. These RPCs are `sethdseed`, `addmultisigaddress`, `importaddress`, `importprivkey`, `importpubkey`, `importmulti`, `dumpprivkey`, and `dumpwallet`. Other RPCs which relied on the wallet for scripts and keys have been modified in order to take the `SigningProvider` retrieved from the `ScriptPubKeyMan` for a given script. Overall, these changes should not effect how everything actually works and the user should experience no difference between having this change and not having it. As such, no functional tests were changed, and the only unit tests changed were those that were directly accessing `CWallet` functions that have been removed. This PR is the last step in the [Wallet Structure Changes](https://github.com/bitcoin-core/bitcoin-devwiki/wiki/Wallet-Class-Structure-Changes). ACKs for top commit: instagibbs: re-utACK https://github.com/bitcoin/bitcoin/pull/17261/commits/3f373659d732a5b1e5fdc692a45b2b8179f66bec Sjors: re-utACK 3f373659d732a5b1e5fdc692a45b2b8179f66bec (it still compiles on macOS after https://github.com/bitcoin/bitcoin/pull/17261#discussion_r370377070) meshcollider: Tested re-ACK 3f373659d732a5b1e5fdc692a45b2b8179f66bec Tree-SHA512: f8e2b8d9efa750b617691e8702d217ec4c33569ec2554a060141d9eb9b9a3a5323e4216938e2485c44625d7a6e0925d40dea1362b3af9857cf08860c2f344716
2020-01-30tests: reset fIsBareMultisigStd after bare-multisig testsfanquake
The bug this fixes is two-part. 1.The fIsBareMultisigStd global is being reused by other tests, i.e script_p2sh_tests(set), after being set to false. 2. The order our tests run in doesn't always? seem to be random, which meant that the script_p2sh tests would only fail if they were run in an order where transaction_tests ran first, mutating the fIsBareMultisigStd global. This doesn't seem to happen when running make check, but if you run src/test/test_bitcoin and pass --random=99999, the failure in script_p2sh: test/script_p2sh_tests.cpp:200: error: in "script_p2sh_tests/set": txTo[1].IsStandard will occur (on most systems). The new test was introduced in 1bb5d517aa616c1d5b5801d2ea36a2de5fb61eba.
2020-01-29on startup, write config options to debug.logLarry Ruane
2020-01-30Merge #17719: Document better -keypool as a look-ahead safety mechanismSamuel Dobson
f41d58966995fe69df433fa684117fae74a56e66 Document better -keypool as a look-ahead safety mechanism (Antoine Riard) Pull request description: If after a backup, an address is issued beyond the initial keypool range and none of the addresses in this range is seen onchain, if a wallet is restored from backup, even in case of rescan, funds may be loss due to the look-ahead buffer not being incremented and so restored wallet not detecting onchain out-of-range address as derived from its seed. This scenario is theoretically unavoidable due to the requirement of the keypool to have a max size. However, given the default keypool size, this is unlikely. Document better keypool size implications to avoid user setting a too low value. While reviewing #17681, it took me a while to figure out the safety implications of keypool, I find it would be better to document this a bit farther to avoid users shooting themselves in the foot. For further context & discussion, see https://github.com/bitcoin/bitcoin/pull/17681#issuecomment-563613452 ACKs for top commit: ryanofsky: Code review ACK f41d58966995fe69df433fa684117fae74a56e66. Just "Warning:" prefix added since the last review jonatack: ACK f41d58966995fe69df433fa684117fae74a56e66 code review and build/test. The added `Warning:` since last review is a good addition. Tree-SHA512: d3d0ee88fcdfc5c8841a2bd4bada0e4eeb412a0dce5054e5fb023643c2fa57206a0f3efb06890c245528dc4431413ed2fd5645b9319d26245d044c490b7f0db0
2020-01-29Use rolling bloom filter of recent block tx's for AlreadyHave() checkSuhas Daftuar
In order to determine whether to download or process a relayed transaction, we try to determine if we already have the transaction, either in the mempool, in our recently rejected filter, in our orphan pool, or already confirmed in the chain itself. Prior to this commit, the heuristic for checking the chain is based on whether there's an output corresponding to the 0- or 1-index vout in our coin cache. While that is a quick check, it is very imprecise (say if those outputs were already spent in a block) -- we can do better by just keeping a rolling bloom filter of the transactions in recent blocks, which will capture the case of a transaction which has been confirmed and then fully spent already. 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.
2020-01-29Merge #17957: Serialization improvements step 3 (compression.h)Wladimir J. van der Laan
4de934b9b5b4be1bac8fe205f4ee9a79e772dc34 Convert compression.h to new serialization framework (Pieter Wuille) ca34c5cba5fbb9b046b074a234f06ecf6ed5d610 Add FORMATTER_METHODS, similar to SERIALIZE_METHODS, but for formatters (Pieter Wuille) Pull request description: This is the next piece of the puzzle from #10785. It includes: * The `FORMATTER_METHODS` macro, similar to `SERIALIZE_METHODS`, for defining a formatter with a unified serialization/deserialization implementation. * Updating `compression.h` to consist of 3 formatters, rather than old-style wrappers (`ScriptCompression`, `AmountCompression`, `TxOutCompression`). ACKs for top commit: laanwj: code review ACK 4de934b9b5b4be1bac8fe205f4ee9a79e772dc34 ryanofsky: Code review ACK 4de934b9b5b4be1bac8fe205f4ee9a79e772dc34. Only change since last review is removing REF usages Tree-SHA512: d52ca21eb1ce87d9bc3c90d00c905bd4fada522759aaa144c02a58b4d738d5e8647c0558b8ce393c707f6e3c4d20bf93781a2dcc1e1dcbd276d9b5ffd0e02cd6
2020-01-29Merge #16702: p2p: supplying and using asmap to improve IP bucketing in addrmanWladimir J. van der Laan
3c1bc40205a3fcab606e70b0e3c13d68b2860e34 Add extra logging of asmap use and bucketing (Gleb Naumenko) e4658aa8eaf1629dd5af8cf7b9717a8e72028251 Return mapped AS in RPC call getpeerinfo (Gleb Naumenko) ec45646de9e62b3d42c85716bfeb06d8f2b507dc Integrate ASN bucketing in Addrman and add tests (Gleb Naumenko) 8feb4e4b667361bf23344149c01594abebd56fdb Add asmap utility which queries a mapping (Gleb Naumenko) Pull request description: This PR attempts to solve the problem explained in #16599. A particular attack which encouraged us to work on this issue is explained here [[Erebus Attack against Bitcoin Peer-to-Peer Network](https://erebus-attack.comp.nus.edu.sg/)] (by @muoitranduc) Instead of relying on /16 prefix to diversify the connections every node creates, we would instead rely on the (ip -> ASN) mapping, if this mapping is provided. A .map file can be created by every user independently based on a router dump, or provided along with the Bitcoin release. Currently we use the python scripts written by @sipa to create a .map file, which is no larger than 2MB (awesome!). Here I suggest adding a field to peers.dat which would represent a hash of asmap file used while serializing addrman (or 0 for /16 prefix legacy approach). In this case, every time the file is updated (or grouping method changed), all buckets will be re-computed. I believe that alternative selective re-bucketing for only updated ranges would require substantial changes. TODO: - ~~more unit tests~~ - ~~find a way to test the code without including >1 MB mapping file in the repo.~~ - find a way to check that mapping file is not corrupted (checksum?) - comments and separate tests for asmap.cpp - make python code for .map generation public - figure out asmap distribution (?) ~Interesting corner case: I’m using std::hash to compute a fingerprint of asmap, and std::hash returns size_t. I guess if a user updates the OS to 64-bit, then the hash of asap will change? Does it even matter?~ ACKs for top commit: laanwj: re-ACK 3c1bc40205a3fcab606e70b0e3c13d68b2860e34 jamesob: ACK 3c1bc40205a3fcab606e70b0e3c13d68b2860e34 ([`jamesob/ackr/16702.3.naumenkogs.p2p_supplying_and_using`](https://github.com/jamesob/bitcoin/tree/ackr/16702.3.naumenkogs.p2p_supplying_and_using)) jonatack: ACK 3c1bc40205a3fcab606e70b0e3c13d68b2860e34 Tree-SHA512: e2dc6171188d5cdc2ab2c022fa49ed73a14a0acb8ae4c5ffa970172a0365942a249ad3d57e5fb134bc156a3492662c983f74bd21e78d316629dcadf71576800c
2020-01-29Merge #18008: test: only declare a main() when fuzzing with AFLfanquake
b35567fe0ba3e6f8d8f9525088eb8ee62065ad01 test: only declare a main() when fuzzing with AFL (fanquake) Pull request description: This fixes fuzzing using [libFuzzer](https://llvm.org/docs/LibFuzzer.html) on macOS, which caused a few issues during the recent review club. macOS users could only fuzz using afl, or inside a VM. It seems that the `__attribute__((weak))` marking is not quite enough to properly mark `main()` as weak on macOS. See Apples docs on [Frameworks and Weak Linking](https://developer.apple.com/library/archive/documentation/MacOSX/Conceptual/BPFrameworks/Concepts/WeakLinking.html#//apple_ref/doc/uid/20002378-107262-CJBJAEID). Have tested fuzzing using libFuzzer and AFL with this patch. ACKs for top commit: MarcoFalke: ACK b35567fe0ba3e6f8d8f9525088eb8ee62065ad01 fjahr: ACK b35567f Tree-SHA512: b881fdd98c7e1587fcf44debd31f5e7a52df938059ab91c41d0785077b3329b793e051a2bf2eee64488b9f6029d9288c911052ec23ab3ab8c0561a2be1682dae