aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
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
2020-01-29Merge #17971: refactor: Remove redundant conditionalfanquake
e80317be5fc6f6a04ea3b35bfe9991b3a5d29f7a refactor: Remove redundant conditional (Bushstar) Pull request description: Conditional check against fMaster is now redundant as it is already checked as true. This originally made sense as the outer conditional was: https://github.com/bitcoin/bitcoin/blob/f9cae832e6f56c6abe89b3bf05d1f176c2a7c913/src/checkqueue.h#L86 Removal of fQuit happened in the commit below. https://github.com/bitcoin/bitcoin/commit/30ded3e3d888f75b6fa8b2e55a3883f799e00775#diff-88316c9aa9514c038c9304297e672da5 ACKs for top commit: theStack: ACK https://github.com/bitcoin/bitcoin/commit/e80317be5fc6f6a04ea3b35bfe9991b3a5d29f7a hebasto: ACK e80317be5fc6f6a04ea3b35bfe9991b3a5d29f7a, I have reviewed the code, and it looks OK, I agree it can be merged. promag: ACK e80317be5fc6f6a04ea3b35bfe9991b3a5d29f7a. emilengler: re-ACK e80317be5fc6f6a04ea3b35bfe9991b3a5d29f7a practicalswift: ACK e80317be5fc6f6a04ea3b35bfe9991b3a5d29f7a Empact: ACK https://github.com/bitcoin/bitcoin/pull/17971/commits/e80317be5fc6f6a04ea3b35bfe9991b3a5d29f7a Tree-SHA512: 136ea1d02e3d65100a8758730617ccede7864e08e8404e42e65d45d4bf95a3bfea2ab9895c6e8833abd654557d3efbba02b25297a2a5eefc36a11e97bbe9134f
2020-01-29Merge #17156: psbt: check that various indexes and amounts are within boundsfanquake
deaa6dd144f5650b385658a0c4f9a014aff8dde2 psbt: check output index is within bounds before accessing (Andrew Chow) f1ef7f0aa46338f4cd8de79696027a1bf868f359 Don't calculate tx fees for PSBTs with invalid money values (Andrew Chow) Pull request description: Fixes #17149 Two classes of issues were found by the psbt fuzzer: values out of range and causing overflows, and prevout indexes being out of range. This PR fixes both. When accessing a specific output using the index given in the tx, check that it is actually a possible output before trying to access the output. When summing and checking amounts for `decodepsbt` and `analyzepsbt`, make sure that the values are actually valid money values.. Otherwise, stop summing and don't show the fee. For `analyzepsbt`, return that the next role is the Creator since the Creator needs to remake the transaction to be valid. ACKs for top commit: practicalswift: ACK deaa6dd144f5650b385658a0c4f9a014aff8dde2 -- only change since last ACK was the addition of tests gwillen: tested ACK deaa6dd, would also like to see this merged! Tree-SHA512: 06c36720bbb5a7ab1c29f7d15878bf9f0d3e5760c06bff479d412e1bf07bb3e0e9ab6cca820a4bfedaab71bfd7af813807e87cbcdf0af25cc3f66a53a06dbcfd
2020-01-29test: only declare a main() when fuzzing with AFLfanquake
libFuzzer will provide a main(). This also fixes a weak linking issue when fuzzing with libFuzzer on macOS.
2020-01-28build: CRC32C build system integrationWladimir J. van der Laan
2020-01-28build: Update build system for new leveldbWladimir J. van der Laan
Upstream leveldb switched build systems, which means we need to define a few different values.
2020-01-28Import crc32c using subtree merge as as 'src/crc32c'Wladimir J. van der Laan
2020-01-28Squashed 'src/crc32c/' content from commit ↵Wladimir J. van der Laan
224988680f7673cd7c769963d4035cb315aa3388 git-subtree-dir: src/crc32c git-subtree-split: 224988680f7673cd7c769963d4035cb315aa3388
2020-01-28Update to leveldb upstream using subtree mergeWladimir J. van der Laan
2020-01-28Fix benchmarks filtersElichai Turkel
2020-01-27test: rename test suite name "tx_validationcache_tests" to match filenameSebastian Falbesoner
Quoting src/test/README.md, 'Adding test cases': "The file naming convention is `<source_filename>_tests.cpp` and such files should wrap their tests in a test suite called `<source_filename>_tests`." Currently the unit test source file txvalidationcache_tests.cpp contains a unit test suite with the name tx_validationcache_tests, which is fixed by this commit. The following shell script shows that this is the only mismatch and for all other unit test source files the test suite names are correct: #!/bin/bash shopt -s globstar for test_full_filename in **/*_tests.cpp; do test_name_file=`basename $test_full_filename .cpp` test_name_suite=`sed -n "s/^.*TEST_SUITE(\(.*_tests\).*$/\1/p" $test_full_filename` if [ $test_name_file != $test_name_suite ]; then echo "TestFilename: $test_name_file != TestSuitname: $test_name_suite" fi done
2020-01-27tests: Add fuzzing harness for strprintf(...)practicalswift
2020-01-27tests: Update FuzzedDataProvider.h from upstream (LLVM)practicalswift
Upstream revision: https://github.com/llvm/llvm-project/blob/a44ef027ebca1598892ea9b104d6189aeb3bc2f0/compiler-rt/include/fuzzer/FuzzedDataProvider.h
2020-01-27Merge #17453: gui: Fix intro dialog labels when the prune button is toggledJonas Schnelli
4f7127d1e3a51f0f55d42a08439c516dcc8d1a26 gui: Make Intro consistent with prune checkbox (Hennadii Stepanov) 4824a7d36cf47e766865e0fefe952ec860eb82dd gui: Add Intro::UpdateFreeSpaceLabel() (Hennadii Stepanov) daa3f3fa9071a229275dd6a1b8445237ddc3fa97 refactor: Add Intro::UpdatePruneLabels() (Hennadii Stepanov) e4caa82a03df5c6a6d5d29f34ab006d732c6dac1 refactor: Replace static variable with data member (Hennadii Stepanov) 2bede28cd9ec638d8bb32c187ccf12d89345218e util: Add PruneGBtoMiB() function (Hennadii Stepanov) e35e4b2ba052c9a533626286026dbe0a2d546c5b util: Add PruneMiBtoGB() function (Hennadii Stepanov) Pull request description: On master (a6f6333ba253cda83221ee529810cacf930e413f) and on 0.19.0.1 the intro dialog with prune enabled (checkbox "Discard blocks..." is checked) provides a user with wrong info about the required disk space: ![DeepinScreenshot_bitcoin-qt_20191208112228](https://user-images.githubusercontent.com/32963518/70387510-8daab400-19ae-11ea-9338-29add9c31118.png) Also the paragraph "If you have chosen to limit..." is missed. --- With this PR when prune checkbox is toggled, the related text labels and the amount of required space shown are updated (previously they were only updated when the data directory was updated): ![Screenshot from 2019-12-08 11-34-53](https://user-images.githubusercontent.com/32963518/70387542-eed28780-19ae-11ea-9565-49d8a64b2f33.png) --- This PR is an alternative to #17035. **ryanofsky**'s [suggestion](https://github.com/bitcoin/bitcoin/pull/17035#discussion_r337594268) also has been implemented. ACKs for top commit: emilengler: ACK 4f7127d1e3a51f0f55d42a08439c516dcc8d1a26 Sjors: tACK 4f7127d1e3a51f0f55d42a08439c516dcc8d1a26 ryanofsky: Code review ACK 4f7127d1e3a51f0f55d42a08439c516dcc8d1a26. It seems like there are a few visible changes here: jonasschnelli: utACK 4f7127d1e3a51f0f55d42a08439c516dcc8d1a26 Tree-SHA512: fa0bbdcfafde97d7906cda066cbd4608b936a71cae1b4cda3ee3aa2eed3a9795f279f14c6b1b4997278e094db891c7d3bb695368ba0882347aa42165a86e5172
2020-01-27Merge #18007: Bugfix: GUI: Hide the HD/encrypt icons earlier so they get ↵Jonas Schnelli
re-shown if another wallet is open 4c524f0aad11b44baa56d2b2e432bbddffff74c2 Bugfix: GUI: Hide the HD/encrypt icons earlier so they get re-shown if another wallet is open (Luke Dashjr) Pull request description: To reproduce bug, open 2 wallets, and close 1. You end up left without the HD/encrypt icons, despite having a wallet open still. This works because the icons are re-shown after we remove the current wallet (if there's another wallet still open). ACKs for top commit: promag: Tested ACK 4c524f0aad11b44baa56d2b2e432bbddffff74c2. jonasschnelli: utACK 4c524f0aad11b44baa56d2b2e432bbddffff74c2 hebasto: ACK 4c524f0aad11b44baa56d2b2e432bbddffff74c2, tested on Linux Mint 19.3. Tree-SHA512: 4ef1bd4a0ae2f20ace9d02bc5d778640c11e46a86f30b762f8502e577f85114f0644d51a70cfbc4c23b51869c3caf20e94548aa64f51fdb85aea5f194a23fca6
2020-01-27Merge #17096: gui: rename debug windowJonas Schnelli
44f15cfdcfc64d5a0c36fded39e4aef49415b11b gui: renamed 'debug window' to 'node window' (Zero) Pull request description: **Edit**: I have now limited the change in this PR to only renaming the window title from `Debug Window` to `Node Window`. Check [this comment](https://github.com/bitcoin/bitcoin/pull/17096#issuecomment-542837511) for more details. This PR is in response to #17082, which aims to rename the `Debug window` title to a more user friendly term; `Node window`. Closes #17082 ACKs for top commit: hebasto: ACK 44f15cfdcfc64d5a0c36fded39e4aef49415b11b, tested on Linux Mint 19.3: theStack: ACK https://github.com/bitcoin/bitcoin/commit/44f15cfdcfc64d5a0c36fded39e4aef49415b11b, tested on Linux (Lubuntu 16.04): Tree-SHA512: 9fc73f2e67badb38525c550ce4c313288858b3fde30ef17fee85230be5bf31cf94408c699265b5e1256dfed60f8d04f48927d9b2831ba9f25498b98e6fa7180f
2020-01-26Bugfix: GUI: Hide the HD/encrypt icons earlier so they get re-shown if ↵Luke Dashjr
another wallet is open
2020-01-24gui: Shortcut to close ModalOverlayEmil Engler
2020-01-23Refactor: Replace SigningProvider pointers with unique_ptrsAndrew Chow
Needed for future ScriptPubKeyMans which may need to create SigningProviders dynamically and thus a normal pointer is not enough This commit does not change behavior.
2020-01-23Cleanup: Drop unused GUI learnRelatedScripts methodAndrew Chow
This commit does not change behavior.
2020-01-23Refactor: Copy CWallet signals and print function to LegacyScriptPubKeyManAndrew Chow
This commit does not change behavior.
2020-01-23Box the wallet: Add multiple keyman maps and loopsAndrew Chow
Add wallet logic for dealing with multiple ScriptPubKeyMan instances. This doesn't change current behavior because there is still only a single LegacyScriptPubKeyMan. But in the future the new logic will be used to support descriptor wallets.
2020-01-23refactor: define a UINT256_ONE global constantAndrew Chow
Instead of having a uint256 representations of one scattered throughout where it is used, define it globally in uint256.h
2020-01-23HD Split: Avoid redundant upgradesAndrew Chow
This avoids repeaded upgrades when support for more multiple keyman references is added in the next commit: https://github.com/bitcoin/bitcoin/pull/16341#discussion_r322370108
2020-01-23Make UpgradeKeyMetadata work only on LegacyScriptPubKeyManAndrew Chow
2020-01-23Store p2sh scripts in AddAndGetDestinationForScriptAndrew Chow
2020-01-23Always try to sign for all pubkeys in multisigAndrew Chow
2020-01-23List output types in an array in order to be iterated overAndrew Chow
2020-01-23Refactor: Allow LegacyScriptPubKeyMan to be nullAndrew Chow
In CWallet::LoadWallet, use this to detect and empty wallet with no keys This commit does not change behavior.
2020-01-23Locking: Lock cs_KeyStore instead of cs_wallet in legacy keymanAndrew Chow
This commit only affects locking behavior and doesn't have other changes.
2020-01-23wallet: Improve CWallet:MarkDestinationsDirtyJoão Barbosa
2020-01-23Add extra logging of asmap use and bucketingGleb Naumenko
2020-01-23Return mapped AS in RPC call getpeerinfoGleb Naumenko
If ASN bucketing is used, return a corresponding AS used in bucketing for a given peer.
2020-01-23doc: Use proper RPC help syntax in importmultiMarcoFalke
2020-01-23doc: Remove duplicate "comment" from listsinceblock RPC helpMarcoFalke
Also, properly document all (json object) and (json array)
2020-01-23doc: Properly document proxy_randomize_credentials as bool in getnetworkinfoMarcoFalke
Also, fix indentation of some fields
2020-01-23doc: Fix syntax error (trailing square bracket) in finalizepsbtMarcoFalke
2020-01-23doc: Fix syntax error (trailing square bracket) in walletprocesspsbtMarcoFalke
2020-01-23doc: Add missing "optional" to "long" estimaterawfee RPC helpMarcoFalke
Also, use multi-line for arrays and objects
2020-01-23src/init: correct a typodarosior
2020-01-22Refactor message transport packagingJonas Schnelli
2020-01-22Merge #17863: scripts: Add MACHO dylib checks to symbol-check.pyWladimir J. van der Laan
c491368d8cfddf3a5b6d574f10ed67492fcecbed scripts: add MACHO dylib checking to symbol-check.py (fanquake) 76bf97213f4b153dd3ccf1314088a73c4804601d scripts: fix check-symbols & check-security argument passing (fanquake) Pull request description: Based on #17857. This adds dynamic library checks for MACHO executables to symbol-check.py. The script has been modified to function more like `security-check.py`. The error output is now also slightly different. i.e: ```bash # Linux x86 bitcoin-cli: symbol operator new[](unsigned long) from unsupported version GLIBCXX_3.4 bitcoin-cli: export of symbol vtable for std::basic_ios<char, std::char_traits<char> > not allowed bitcoin-cli: NEEDED library libstdc++.so.6 is not allowed bitcoin-cli: failed IMPORTED_SYMBOLS EXPORTED_SYMBOLS LIBRARY_DEPENDENCIES # RISCV (skips exported symbols checks) bitcoin-tx: symbol operator new[](unsigned long) from unsupported version GLIBCXX_3.4 bitcoin-tx: NEEDED library libstdc++.so.6 is not allowed bitcoin-tx: failed IMPORTED_SYMBOLS LIBRARY_DEPENDENCIES # macOS Checking macOS dynamic libraries... libboost_filesystem.dylib is not in ALLOWED_LIBRARIES! bitcoind: failed DYNAMIC_LIBRARIES ``` Compared to `v0.19.0.1` the macOS allowed dylibs has been slimmed down somewhat: ```diff src/qt/bitcoin-qt: /usr/lib/libSystem.B.dylib -/System/Library/Frameworks/DiskArbitration.framework/Versions/A/DiskArbitration /System/Library/Frameworks/IOKit.framework/Versions/A/IOKit /System/Library/Frameworks/Foundation.framework/Versions/C/Foundation /System/Library/Frameworks/CoreServices.framework/Versions/A/CoreServices /System/Library/Frameworks/AppKit.framework/Versions/C/AppKit /System/Library/Frameworks/ApplicationServices.framework/Versions/A/ApplicationServices /System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation -/System/Library/Frameworks/Security.framework/Versions/A/Security -/System/Library/Frameworks/SystemConfiguration.framework/Versions/A/SystemConfiguration /System/Library/Frameworks/CoreGraphics.framework/Versions/A/CoreGraphics -/System/Library/Frameworks/OpenGL.framework/Versions/A/OpenGL -/System/Library/Frameworks/AGL.framework/Versions/A/AGL /System/Library/Frameworks/Carbon.framework/Versions/A/Carbon /usr/lib/libc++.1.dylib -/System/Library/Frameworks/CFNetwork.framework/Versions/A/CFNetwork /System/Library/Frameworks/CoreText.framework/Versions/A/CoreText /System/Library/Frameworks/ImageIO.framework/Versions/A/ImageIO /usr/lib/libobjc.A.dylib ``` ACKs for top commit: laanwj: ACK c491368d8cfddf3a5b6d574f10ed67492fcecbed Tree-SHA512: f8624e4964e80b3e0d34e8d3cc33f3107938f3ef7a01c07828f09b902b5ea31a53c50f9be03576e1896ed832cf2c399e03a7943a4f537a1e1c705f3804aed979
2020-01-22Merge #17754: net: Don't allow resolving of std::string with embedded NUL ↵Wladimir J. van der Laan
characters. Add tests. 7a046cdc1423963bdcbcf9bb98560af61fa90b37 tests: Avoid using C-style NUL-terminated strings as arguments (practicalswift) fefb9165f23fe9d10ad092ec31715f906e0d2ee7 tests: Add tests to make sure lookup methods fail on std::string parameters with embedded NUL characters (practicalswift) 9574de86ad703ad942cdd0eca79f48c0d42b102b net: Avoid using C-style NUL-terminated strings as arguments in the netbase interface (practicalswift) Pull request description: Don't allow resolving of `std::string`:s with embedded `NUL` characters. Avoid using C-style `NUL`-terminated strings as arguments in the `netbase` interface Add tests. The only place in where C-style `NUL`-terminated strings are actually needed is here: ```diff + if (!ValidAsCString(name)) { + return false; + } ... - int nErr = getaddrinfo(pszName, nullptr, &aiHint, &aiRes); + int nErr = getaddrinfo(name.c_str(), nullptr, &aiHint, &aiRes); if (nErr) return false; ``` Interface changes: ```diff -bool LookupHost(const char *pszName, std::vector<CNetAddr>& vIP, unsigned int nMaxSolutions, bool fAllowLookup); +bool LookupHost(const std::string& name, std::vector<CNetAddr>& vIP, unsigned int nMaxSolutions, bool fAllowLookup); -bool LookupHost(const char *pszName, CNetAddr& addr, bool fAllowLookup); +bool LookupHost(const std::string& name, CNetAddr& addr, bool fAllowLookup); -bool Lookup(const char *pszName, CService& addr, int portDefault, bool fAllowLookup); +bool Lookup(const std::string& name, CService& addr, int portDefault, bool fAllowLookup); -bool Lookup(const char *pszName, std::vector<CService>& vAddr, int portDefault, bool fAllowLookup, unsigned int nMaxSolutions); +bool Lookup(const std::string& name, std::vector<CService>& vAddr, int portDefault, bool fAllowLookup, unsigned int nMaxSolutions); -bool LookupSubNet(const char *pszName, CSubNet& subnet); +bool LookupSubNet(const std::string& strSubnet, CSubNet& subnet); -CService LookupNumeric(const char *pszName, int portDefault = 0); +CService LookupNumeric(const std::string& name, int portDefault = 0); -bool ConnectThroughProxy(const proxyType &proxy, const std::string& strDest, int port, const SOCKET& hSocketRet, int nTimeout, bool *outProxyConnectionFailed); +bool ConnectThroughProxy(const proxyType &proxy, const std::string& strDest, int port, const SOCKET& hSocketRet, int nTimeout, bool& outProxyConnectionFailed); ``` It should be noted that the `ConnectThroughProxy` change (from `bool *outProxyConnectionFailed` to `bool& outProxyConnectionFailed`) has nothing to do with `NUL` handling but I thought it was worth doing when touching this file :) ACKs for top commit: EthanHeilman: ACK 7a046cdc1423963bdcbcf9bb98560af61fa90b37 laanwj: ACK 7a046cdc1423963bdcbcf9bb98560af61fa90b37 Tree-SHA512: 66556e290db996917b54091acd591df221f72230f6b9f6b167b9195ee870ebef6e26f4cda2f6f54d00e1c362e1743bf56785d0de7cae854e6bf7d26f6caccaba
2020-01-22Merge #17887: bug-fix macos: give free bytes to F_PREALLOCATEWladimir J. van der Laan
75163f4729c10c40d2843da28a8c79ab89193f6a bug-fix macos: give free bytes to F_PREALLOCATE (Karl-Johan Alm) Pull request description: The macos manpage for `fcntl` (for `F_PEOFPOSMODE`) states: > Allocate from the physical end of file. In this case, fst_length indicates the number of newly allocated bytes desired. This would result in the rev files being essentially pre-allocating 2x their necessary size (this is the case for block files as well, but these are flushed down to their right sizes every time) as they would pre-allocate `pos + length` **free** bytes, rather than allocating `length` bytes after `pos`, as expected. Fixes #17827. ACKs for top commit: eriknylund: ACK 75163f4729c10c40d2843da28a8c79ab89193f6a built locally. All tests passing. Manual test as per my previous comment above on an older commit, using an APFS unencrypted disk image with 3 GB. laanwj: code review ACK 75163f4729c10c40d2843da28a8c79ab89193f6a Tree-SHA512: 105c8d56c20acad8febdf0583f1e5721b63376ace325a7a62c2e4b15a442c7131404ed604c32c0cda716791d7ca5aa9f5b6a774ff86e39838bc7e87ca3c42760
2020-01-22Merge #17965: qt: Revert changes of pr17943Wladimir J. van der Laan
70e4706093fd7b08a32f9638dace178852a9d249 Revert "refactor: Remove never used default parameter" (Hennadii Stepanov) 219417b388a0373f9eb71446e1b0499ab55dd3e2 Revert "refactor: Simplify connection syntax" (Hennadii Stepanov) Pull request description: The code, the `bool* ret = nullptr` parameter in the `BitcoinGUI::message()` slot, removed in #17943 is not dead actually. It is used in `ThreadSafeMessageBox()` function: https://github.com/bitcoin/bitcoin/blob/a654626f076a72416a3d354218d7107571d6caaf/src/qt/bitcoingui.cpp#L1363-L1368 Now in master (a654626f076a72416a3d354218d7107571d6caaf): ``` $ ./src/qt/bitcoin-qt -prune=-1 Error: Prune cannot be configured with a negative value. bitcoin-qt: qt/bitcoingui.cpp:1369: bool ThreadSafeMessageBox(BitcoinGUI*, const string&, const string&, unsigned int): Assertion `invoked' failed. Aborted (core dumped) ``` This PR reverts all commits of #17943 Additional notes: the bug was missed due to dynamic function call `QMetaObject::invokeMethod()` which cannot be checked at compile time. See #16348 for more discussion. Sorry for introducing a bug. ACKs for top commit: Sjors: ACK 70e4706093fd7b08a32f9638dace178852a9d249 laanwj: ACK 70e4706093fd7b08a32f9638dace178852a9d249 Tree-SHA512: b968a026eaa4f5f39fd36ddc715d8e233f3c6420e6580f11d4ca422a5ff5d1d9d3df9ac11b353c3d4f434d67d6a69e37d2e26b8248d72bedd14ecba0a545a327