aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2020-02-05Merge #16392: build: macOS toolchain updateWladimir J. van der Laan
7e2104433cd0905ccf94632511b3ca0ce5b0463b build: use macOS 10.14 SDK (fanquake) ca5055a5aa07aba81a87cf12f6f0526a63c423b5 depends: native_cctools 921, ld64 409.12, libtapi 1000.10.8 (fanquake) 1de8c067c74cd171144c8a900a8a20efe3072c43 depends: clang 6.0.1 (fanquake) Pull request description: TLDR: This updates our macOS toolchain to use a newer version of Clang, cctools (including new [dependency on libtapi](https://github.com/tpoechtrager/cctools-port/tree/master#dependencies)), LD64 and the macOS SDK. I've been testing depends builds (`HOST=x86_64-apple-darwin16`) inside a Debian Buster [Docker container](https://github.com/fanquake/core-review/blob/master/docker/debian.dockerfile), and running the resultant `bitcoind` and `bitcoin-qt` binaries on a macOS `10.14.4` system. The `.dmg` generated by a `make deploy` also mounts correctly on the same macOS system. #### Clang Upgraded from `3.7.1` to [`6.0.1`](https://releases.llvm.org/6.0.1/docs/ReleaseNotes.html) #### cctools * cctools `877.8` -> [`921`](https://opensource.apple.com/tarballs/cctools/) * LD64 `253.9` -> [`409.12`](https://opensource.apple.com/source/ld64/) * TAPI [`1000.10.8`](https://opensource.apple.com/tarballs/tapi/) See [tpoechtrager/cctools-port](https://github.com/tpoechtrager/cctools-port/) and [tpoechtrager/apple-libtapi](https://github.com/tpoechtrager/apple-libtapi/). #### macOS SDK Upgraded from building against the macOS `10.11` SDK to the macOS `10.14` SDK. #### TODO - [x] Make the `10.14` SDK available to Travis. Fixes: #16052 Closes: #14797 ACKs for top commit: Sjors: re-ACK 7e2104433cd0905ccf94632511b3ca0ce5b0463b (rebased from 248526e) dongcarl: ACK 7e21044 Tree-SHA512: fd36a33dbfb98c144240f8c69b77343e3f5bc18d8cf7d40fff61f51ad48925ec1872e6daba34c4045b18b4c2c84c22c744ebf4cba11061a0305eed13975ceefe
2020-02-05Merge #17660: build: remove deprecated key from macOS Info.plistWladimir J. van der Laan
c0bc453135b3f549f800545075cb7bdb310c3ad4 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 ![0 19 0 1](https://user-images.githubusercontent.com/863730/70089170-a0576e80-15e5-11ea-975c-a6902a1ed95a.png) This PR. ![master](https://user-images.githubusercontent.com/863730/70089178-a3525f00-15e5-11ea-9d63-7db67de014a5.png) ACKs for top commit: laanwj: ACK c0bc453135b3f549f800545075cb7bdb310c3ad4 Tree-SHA512: 6191056d0cb6072b8a2170c8441ebfe500cf00cd41014bf5ee68fbf60b5bb5642e2fad9541f1c5abfaafdae6db3102c3add6169cefce3fc4a63d8b913ea35865
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 #18069: test: replace 'regtest' leftovers by self.chainMarcoFalke
eca56f89293b74f11ca631ff2a0793e970e65841 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
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-05test: replace 'regtest' leftovers by self.chainSebastian Falbesoner
Commit 1abcecc40c518a98b7d17880657ec0247abdf125 replaced 'regtest' by self.chain 'regtest' "in almost all current tests", this commit takes care of the remaining ones.
2020-02-05Merge #18059: build: add missing attributes to Win installerfanquake
6c223152238d2e818e38357b03f38a4dbe9de016 build: add additional attributes to Win installer (fanquake) Pull request description: Fixes: #17170. ACKs for top commit: hebasto: ACK 6c223152238d2e818e38357b03f38a4dbe9de016, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: d2ff2006b8df6a34b3a16270d3eb895b03cf6b3ca69404bc39adeb7d5e3b896ddab6ba831566dc966d8bdfba3f57ddf325762cddf3ad76d1427971d1bcc68255
2020-02-05Merge #17336: scripts: search for first block file for linearize-data with ↵fanquake
some block files pruned 317fb96de9c6257972f1213b4ef2c3fe87dde99f 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 317fb96de9c6257972f1213b4ef2c3fe87dde99f laanwj: Code review ACK 317fb96de9c6257972f1213b4ef2c3fe87dde99f theStack: Code review ACK https://github.com/bitcoin/bitcoin/pull/17336/commits/317fb96de9c6257972f1213b4ef2c3fe87dde99f Tree-SHA512: fc8014282df6cfe7b267e64db8ce7d82b86b758c302fbfea4a3c39b62d93512f5c2e31a0de4e9c5ec18fc0268c917f011257d37b45afaef6033eec90e4aa585f
2020-02-04Merge #16681: Tests: Use self.chain instead of 'regtest' in all current testsMarcoFalke
1abcecc40c518a98b7d17880657ec0247abdf125 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 1abcecc40c518a98b7d17880657ec0247abdf125 ryanofsky: Code review ACK 1abcecc40c518a98b7d17880657ec0247abdf125. ryanofsky: Code review ACK 1abcecc40c518a98b7d17880657ec0247abdf125 Tree-SHA512: 5620de6dab235ca8bd8670d6366c7b9f04f0e3ca9c5e7f87765b38e16ed80c17d7d1630c0d5fd7c5526f070830d94dc74cc2096d8ede87dc7180ed20569509ee
2020-02-04Merge #18060: gui: Drop PeerTableModel dependency to ClientModelfanquake
ff59bcd3213ef61f2167c0aa60fcaf5afbc20c61 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 https://github.com/bitcoin/bitcoin/pull/18060/commits/ff59bcd3213ef61f2167c0aa60fcaf5afbc20c61 hebasto: ACK ff59bcd3213ef61f2167c0aa60fcaf5afbc20c61, tested on Linux Mint 19.3. No changes in behavior are observed. Tree-SHA512: 29fa3c316c05b8f7b9340e5859bbb8c3a0b826aa7c865c892cfa13b5ad30f822fcaae4e01555f7860cd1727f20b7ef555a808235522a04a6eebaaa7b605f8595
2020-02-04build: add additional attributes to Win installerfanquake
2020-02-03gui: Drop PeerTableModel dependency to ClientModelJoão Barbosa
2020-02-03build: use macOS 10.14 SDKfanquake
Co-Authored-By: Carl Dong <accounts@carldong.me>
2020-02-03depends: native_cctools 921, ld64 409.12, libtapi 1000.10.8fanquake
This also removes the obsolete mlinker-version option Co-Authored-By: Cory Fields <cory-nospam-@coryfields.com>
2020-02-03depends: clang 6.0.1fanquake
This also removes some now-unnecessary cctools hacks. Co-Authored-By: Cory Fields <cory-nospam-@coryfields.com>
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-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-31gui: Drop ShutdownWindow dependency to BitcoinGUIJoão Barbosa
2020-01-31gui: Drop BanTableModel dependency to ClientModelJoão Barbosa
2020-01-31Merge #18025: doc: Add missing supported rpcs to doc/descriptors.mdfanquake
c7ec9a18888e040a2e1de2cc740a6ef0372d336d Add missing supported rpcs to doc/descriptors.md (Andrew Toth) Pull request description: Improve descriptor docs by adding missing rpcs. ACKs for top commit: fanquake: ACK c7ec9a18888e040a2e1de2cc740a6ef0372d336d - I think this has been bikeshed enough. jonatack: ACK c7ec9a18888e040a2e1de2cc740a6ef0372d336d Tree-SHA512: 783219928ed7edc904b507bb30e2eefd8ca9f11225e1460fedecd755f9511055adcc52cc49f66ba840e121883e40753061db76a243ee6e0091daf1fc396ae59a
2020-01-31Merge #18031: Remove GitHub Actions CI workflow.fanquake
085423b978c4cb6a2d1385e091e8e4151599d5e0 Remove GitHub Actions CI workflow. (Aaron Clauson) Pull request description: While the GitHub Action CI workflow has permissions to make commits it's not suitable. As per #17803. ACKs for top commit: fanquake: ACK 085423b978c4cb6a2d1385e091e8e4151599d5e0 Tree-SHA512: 62c19dda6164a563dc87e394314afc57ef2494b7e4af148b83220f1b2b0bef9ff309b5cde5d9bed3633429b77041871993ac5c73b0cbe7f720d6a4e60e3e816a
2020-01-30Add missing supported rpcs to doc/descriptors.mdAndrew Toth
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-30Remove GitHub Actions CI workflow.Aaron Clauson
2020-01-30tests: Add fuzzer asmap to FUZZERS_MISSING_CORPORA (temporarily)practicalswift
2020-01-30tests: Add fuzzing harness for AS-mapping (asmap)practicalswift
2020-01-30Merge #17984: test: Add p2p test for forcerelay permissionWladimir J. van der Laan
aaaae4d0ebd5ef34d81997a73ab9839ba7b4b9e4 test: Add p2p test for forcerelay permission (MarcoFalke) fa6b57bcaaf4dc65d78316353033b03d171a3beb test: Fix whitespace in p2p_permissions.py (MarcoFalke) faf40810d7b7f42f3588bfa8a663095aa24001b1 test: Make msg_tx a witness tx (MarcoFalke) Pull request description: The commit `test: Make msg_tx a witness tx` is needed so that the python mininode does not strip the witness from transactions before sending them over p2p. The commit should also be done to keep symmetry with msg_block. See: * tests: Make msg_block a witness block #15982 ACKs for top commit: laanwj: ACK aaaae4d0ebd5ef34d81997a73ab9839ba7b4b9e4 Tree-SHA512: b4b546c88f7f0576cb512f0872bc6bef9d4df65783803f226986e56175937f418aa1ed906417ac909f27f1fd521d64629621fda83250fa925c46ef9513db0e4c
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-30Merge #18026: psbt_wallet_tests: use unique_ptr for GetSigningProviderfanquake
1115ba693b6f6e216cd8417aa499fd018a7c016e psbt_wallet_tests: use unique_ptr for GetSigningProvider (Anthony Towns) Pull request description: #17261 changed GetSigningProvider to return a unique_ptr, but #17156 made psbt_wallet_tests use it as well, and wasn't correspondingly updated. ACKs for top commit: fanquake: ACK 1115ba693b6f6e216cd8417aa499fd018a7c016e meshcollider: Thanks! utACK 1115ba693b6f6e216cd8417aa499fd018a7c016e Tree-SHA512: f0191c9b00780e6d1445fa4ec531456758b468b5bca8660474d22b1edb5f48a636a940656c9bdbe466b8bffad7af1e57e0756239906e901d60c69c3124d3bff4
2020-01-30psbt_wallet_tests: use unique_ptr for GetSigningProviderAnthony Towns
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-29Merge #18022: test: Fix appveyor test_bitcoin build of *.rawMarcoFalke
fa1a46e7f4631a001bc28d415f27aae5ee324ccc build: Fix appveyor test_bitcoin build of *.raw (MarcoFalke) Pull request description: Fixes #18020 Top commit has no ACKs. Tree-SHA512: c0b3ca4f95b46543bb3bc6d254300c832a69feca79f5de4e13cafd4c962ae53903069ec7a8c9573761eefa5cec617992b70750b067ee42231dc74170ba6c3b10
2020-01-29on startup, write config options to debug.logLarry Ruane
2020-01-29build: Fix appveyor test_bitcoin build of *.rawMarcoFalke
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.
generated by cgit v1.2.3 (git 2.26.2) at 2024-09-03 20:56:18 +0000