aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2019-12-09Merge #17093: tests: Add fuzzing harness for various CTx{In,Out} related ↵MarcoFalke
functions d5766f223f627bf2eb731ce8552dfafa2b824378 tests: Add corpora suppression (FUZZERS_MISSING_CORPORA) for fuzzers missing in https://github.com/bitcoin-core/qa-assets/tree/master/fuzz_seed_corpus (practicalswift) e75ecb91c730115290e1201371492c2cd334e9b4 tests: Add fuzzing harness for various CTxOut related functions (practicalswift) ce935292c041162e160d95fc6afeda3dceded2cf tests: Add fuzzing harness for various CTxIn related functions (practicalswift) Pull request description: Add fuzzing harness for various `CTx{In,Out}` related functions. **Testing this PR** Run: ``` $ CC=clang CXX=clang++ ./configure --enable-fuzz --with-sanitizers=address,fuzzer,undefined $ make $ src/test/fuzz/tx_in … $ src/test/fuzz/tx_out … # And to to quickly verify that the relevant code regions are triggered, that the # fuzzing throughput seems reasonable, etc. $ contrib/devtools/test_fuzzing_harnesses.sh '^tx_' ``` `test_fuzzing_harnesses.sh` can be found in PR #17000. Top commit has no ACKs. Tree-SHA512: f1374307a2581ebc3968d012ea2438061bbb84ece068e584fae9750669a6cd003723dde14db88e77c9579281ecd4eaa2a7ff0614f253d8c075e6dd16dd2e68d5
2019-12-09Merge #17225: tests: Test serialisation as part of deserialisation fuzzing. ↵MarcoFalke
Test round-trip equality where possible. 709afb2a7de283a9188e7df51476830012e0a4e5 tests: Test serialisation as part of deserialisation fuzzing. Test round-trip equality where possible. Avoid code repetition. (practicalswift) Pull request description: Test serialisation as part of deserialisation fuzzing. Test round-trip equality where possible. ACKs for top commit: MarcoFalke: ACK 709afb2a7de283a9188e7df51476830012e0a4e5 🍲 Tree-SHA512: b8c9c24538ee516607608ac685d2e9b01eca5c15213def3fd096b16516db84bfd45516fbee43e25b28cb3481a5d4ec3f7a34713e2da35b2902081ed42b85224d
2019-12-09Merge #17682: util: Update tinyformat to upstreamMarcoFalke
978b25528c5f336e0aade73bd1b320500f257f70 util: Update tinyformat to upstream (Wladimir J. van der Laan) Pull request description: Last update was in 2017. Updates tinyformat to upstream commit c42f/tinyformat@705e3f4e1de922069bf715746d35bd2364b1f98f. Re-apply (and mark) bitcoin core specific changes. No changes that affect our use, as far as I can see, but this gets rid of the gcc `-Wimplicit-fallthrough` warnings, at least. ACKs for top commit: MarcoFalke: ACK 978b25528c5f336e0aade73bd1b320500f257f70, extracted our patches based on the last update, did the update to v2.3.0 myself and re-applied the patches. Only diff is NULL/nullptr and explicit 🔝 Tree-SHA512: 2ba09e1095878d088520f379d545b40c7286ef199ecbbc17fdd5c85bca447d9b0c7a1829d4038bb6d432cd1ff92ad7bba75c0f2f96c71aeb6fa6031002f1ea1d
2019-12-09Merge #17702: gui: Move static placeholder texts to formsMarcoFalke
a652dc5521e2caf5734ffb797c7f2fc80685fef1 qt: Normalize placeholder to avoid using "address book" in sendcoinsentry (Wladimir J. van der Laan) 67f36e0b2ce0f99b90578e7e1dd9e0624026bcfa gui: Move static placeholder texts to forms (Wladimir J. van der Laan) Pull request description: There was an issue around the time of Qt 4.6 when placeholder text was introduced, that caused a compile failure when it was specified in the form. As a workaround the placeholder texts were moved to the code. Qt 4 hasn't been relevant to us for ages. So move all (non-parametrized) placeholder texts to the form files instead. It's better to keep this kind of text content together. Translate/no-translate status is kept as it is. Proof that they still work: ![win1](https://user-images.githubusercontent.com/126646/70428014-0e80b300-1a76-11ea-9a6d-be78a0bf14ed.png) ![win2](https://user-images.githubusercontent.com/126646/70428019-10e30d00-1a76-11ea-8016-ffa0c4eafe34.png) ![win3](https://user-images.githubusercontent.com/126646/70428021-13456700-1a76-11ea-9449-9413487e39f6.png) ![win4](https://user-images.githubusercontent.com/126646/70428025-150f2a80-1a76-11ea-92ad-be5f3c171c43.png) ACKs for top commit: hebasto: Re-ACK a652dc5521e2caf5734ffb797c7f2fc80685fef1, `tooltip` and `placeholderText` are identical now. MarcoFalke: ACK a652dc5521e2caf5734ffb797c7f2fc80685fef1 🚿 fanquake: ACK a652dc5521e2caf5734ffb797c7f2fc80685fef1 - checked that placeholder text still appears. Tree-SHA512: 7d3c1faeef2eb5d4b195d9d78f2a3f161296d869e5059b5e8d308167e3c6c668a3ebabec93dc592762ba15bfc86d51985e20c4e17f1065c8dce84fec036ff5ee
2019-12-09Merge #17694: ui: disable 3rd-party tx-urls when wallet disabledfanquake
48a5c92f9ef6634375a3f52812cf3d511c37699d ui: disable 3rd-party tx-urls when wallet disabled (Harris) Pull request description: This PR closes #17683 by removing 3rd-party Url-Label and -TextBox from Display Options in wallet-disabled mode. ACKs for top commit: laanwj: Code review ACK 48a5c92f9ef6634375a3f52812cf3d511c37699d fanquake: ACK 48a5c92f9ef6634375a3f52812cf3d511c37699d - tested with and without wallet (compiled out and `-disablewallet`). Tree-SHA512: 3cc89825409fc0a3eec501c4dab5ff1caaa4ce410746a4b6ab200222fff986f4483eab90cda53a98a144be6acf1b6ca8650ab18242c39446f3335b3a9a537066
2019-12-09Merge #17695: gui: disable File->CreateWallet during startupfanquake
d65fafc2f7d98ab2be0a0961e7a3ebe7850c1dca gui: disable File->CreateWallet during startup (fanquake) Pull request description: Same as #16118. Early calls to Create Wallet will crash bitcoin-qt. ```bash lldb /Applications/Bitcoin-Qt.app/Contents/MacOS/Bitcoin-Qt -- --regtest -debug Process 18143 launched: '/Users/michael/github/bitcoin/src/qt/bitcoin-qt' (x86_64) 2019-12-07 15:49:37.823867-0500 bitcoin-qt[18143:5696499] MessageTracer: Falling back to default whitelist Process 18143 stopped * thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x18) frame #0: 0x00000001000d2d9d bitcoin-qt`CreateWalletActivity::createWallet() + 381 bitcoin-qt`CreateWalletActivity::createWallet: -> 0x1000d2d9d <+381>: movq 0x18(%rax), %r14 0x1000d2da1 <+385>: movq %r15, -0xa8(%rbp) 0x1000d2da8 <+392>: leaq -0xa0(%rbp), %r12 0x1000d2daf <+399>: leaq -0x80(%rbp), %rsi Target 0: (bitcoin-qt) stopped. (lldb) bt * thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x18) * frame #0: 0x00000001000d2d9d bitcoin-qt`CreateWalletActivity::createWallet() + 381 frame #1: 0x0000000100833e6f bitcoin-qt`QMetaObject::activate(QObject*, int, int, void**) + 1631 frame #2: 0x0000000100a1fc47 bitcoin-qt`QDialog::done(int) + 247 frame #3: 0x0000000100833ef5 bitcoin-qt`QMetaObject::activate(QObject*, int, int, void**) + 1765 frame #4: 0x00000001009e04c2 bitcoin-qt`QDialogButtonBoxPrivate::_q_handleButtonClicked() + 786 ``` ACKs for top commit: jonasschnelli: utACK d65fafc2f7d98ab2be0a0961e7a3ebe7850c1dca promag: ACK d65fafc2f7d98ab2be0a0961e7a3ebe7850c1dca. Tree-SHA512: 12d7f9e8772508bffbb0163849d9eceec5b1c80068c5d377a4d0973c713dc5f8ad38be8f793fec843d7fb604f0e60a72398b0c95f0a8b775dab39d25b29ac046
2019-12-09qt: Normalize placeholder to avoid using "address book" in sendcoinsentryWladimir J. van der Laan
2019-12-09gui: Move static placeholder texts to formsWladimir J. van der Laan
There was an issue around the time of Qt 4.6 when placeholder text was introduced, that caused a compile failure when it was specified in the form. As a workaround the placeholder texts were moved to the code. Qt 4 hasn't been relevant to us for ages. So move all (non-parametrized) placeholder texts to the form files instead. It's better to keep this kind of text content together. Makes sure translate/no-translate status is kept as it is.
2019-12-09ui: disable 3rd-party tx-urls when wallet disabledHarris
2019-12-08Make env data logging optionalPieter Wuille
2019-12-08Merge #17685: tests: Fix bug in the descriptor parsing fuzzing harness ↵MarcoFalke
(descriptor_parse) 6338c0203416a5f86e9422b6cd479da8af277f2f tests: Fix fuzzing harness for descriptor parsing (descriptor_parse) (practicalswift) Pull request description: Fix bug in the descriptor parsing fuzzing harness (`descriptor_parse`) by making sure `secp256k1_context_verify` is properly initialized (via `ECCVerifyHandle`). Background: When fuzzing `Parse(…)` with `libFuzzer` I eventually reached the test case `combo(020000000000000000000000000000000000000000000000000000000000000000)`. That input triggers a call to `CPubKey::IsFullyValid()` which in turns requires an initialized `secp256k1_context_verify`. The fuzzing harness did not fulfil that pre-condition prior to this commit (sorry, my fault!) :) Before: ``` $ mkdir descriptors/ $ echo -n 'combo(020000000000000000000000000000000000000000000000000000000000000000)' > descriptors/input $ UBSAN_OPTIONS="print_stacktrace=1:halt_on_error=1" src/test/fuzz/descriptor_parse -runs=1 descriptors/ … pubkey.cpp:210:38: runtime error: null pointer passed as argument 1, which is declared to never be null secp256k1/include/secp256k1.h:305:3: note: nonnull attribute specified here #0 0x561c032ccf25 in CPubKey::IsFullyValid() const src/pubkey.cpp:210:12 #1 0x561c022139c3 in (anonymous namespace)::ParsePubkeyInner(Span<char const> const&, bool, FlatSigningProvider&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&) src/script/descriptor.cpp:674:24 #2 0x561c02207680 in (anonymous namespace)::ParsePubkey(Span<char const> const&, bool, FlatSigningProvider&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&) src/script/descriptor.cpp:730:42 #3 0x561c0220080e in (anonymous namespace)::ParseScript(Span<char const>&, (anonymous namespace)::ParseScriptContext, FlatSigningProvider&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&) src/script/descriptor.cpp:774:23 #4 0x561c021ffb07 in Parse(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, FlatSigningProvider&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, bool) src/script/descriptor.cpp:994:16 #5 0x561c0218d5d4 in test_one_input(std::vector<unsigned char, std::allocator<unsigned char> > const&) src/test/fuzz/descriptor_parse.cpp:20:9 … $ ``` After: ``` $ mkdir descriptors/ $ echo -n 'combo(020000000000000000000000000000000000000000000000000000000000000000)' > descriptors/input $ UBSAN_OPTIONS="print_stacktrace=1:halt_on_error=1" src/test/fuzz/descriptor_parse -runs=1 descriptors/ … Done 2 runs in 0 second(s) $ ``` ACKs for top commit: paymog: ACK 6338c0203416a5f86e9422b6cd479da8af277f2f MarcoFalke: ACK 6338c0203416a5f86e9422b6cd479da8af277f2f 🕊 Tree-SHA512: bf24c404e1f64183761b057d2f210c3db85277f4415122977c315d7d6835acb5e897b5d64032615e9e44ad4a16dfe857e94481f6e4b57b6dfa8cb37adb2528a5
2019-12-07gui: disable File->CreateWallet during startupfanquake
2019-12-06Merge #17373: wallet: Various fixes and cleanup to keypool handling in ↵fanquake
LegacyScriptPubKeyMan and CWallet 886f1731bec4393dd342403ac34069a3a4f95eea Key pool: Fix omitted pre-split count in GetKeyPoolSize (Andrew Chow) 386a994b853bc5b3a2ed0d812673465b8ffa4849 Key pool: Change ReturnDestination interface to take address instead of key (Andrew Chow) ba41aa4969169cd73d6b4f57444ed7d8d875de10 Key pool: Move LearnRelated and GetDestination calls (Andrew Chow) 65833a74076cddf986037c6eb3b29a9b9dbe31c5 Add OutputType and CPubKey parameters to KeepDestination (Andrew Chow) 9fcf8ce7ae02bf170b9bf0c2887fd709d752cbf7 Rename Keep/ReturnKey to Keep/ReturnDestination and remove the wrapper (Andrew Chow) 596f6460f9fd8273665c8754ccd673d93a4f25f0 Key pool: Move CanGetAddresses call (Andrew Chow) Pull request description: * The `pwallet->CanGetAddresses()` call in `ReserveDestination::GetReservedDestination` to `LegacyScriptPubKeyMan::GetReservedDestination` so that the sanity check results in a failure when a `ScriptPubKeyMan` individually cannot get a destination, not when any of the `ScriptPubKeyMan`s can't. * `ScriptPubKeyMan::GetReservedDestination` is changed to return the destination so that future `ScriptPubKeyMan`s can return destinations constructed in other ways. This is implemented for `LegacyScriptPubKeyMan` by moving key-to-destination code from `CWallet` to `LegacyScriptPubKeyMan` * In order for `ScriptPubKeyMan` to be generic and work with future `ScriptPubKeyMan`s, `ScriptPubKeyMan::ReturnDestination` is changed to take a `CTxDestination` instead of a `CPubKey`. Since `LegacyScriptPubKeyMan` still deals with keys internally, a new map `m_reserved_key_to_index` is added in order to track the keypool indexes that have been reserved. * A bug is fixed in how the total keypool size is calculated as it was omitting `set_pre_split_keypool` which is a bug. Split from #17261 ACKs for top commit: ryanofsky: Code review ACK 886f1731bec4393dd342403ac34069a3a4f95eea. Only change is moving earlier fix to a better commit (same end result). promag: Code review ACK 886f1731bec4393dd342403ac34069a3a4f95eea. instagibbs: code review re-ACK https://github.com/bitcoin/bitcoin/pull/17373/commits/886f1731bec4393dd342403ac34069a3a4f95eea Sjors: Code review re-ACK 886f1731bec4393dd342403ac34069a3a4f95eea Tree-SHA512: f4be290759f63fdc920d5c02bd0d09acc4b06a5f053787d4afcd3c921b2e35d2bd97617fadae015da853dc189f559fb8d2c6e58d53e4cabfac9af151cd97ad19
2019-12-06Merge #17275: pubkey: Assert CPubKey's ECCVerifyHandle preconditionMarcoFalke
d8daa8f3711909223b117b8faa82daca87fc942d pubkey: Assert CPubKey's ECCVerifyHandle precondition (practicalswift) Pull request description: Assert `CPubKey`'s `ECCVerifyHandle` precondition. This makes it more clear for fuzzing harness writers and others that `ECCVerifyHandle` is expected to be held when interacting with `CPubKey`. Related PR #17274. ACKs for top commit: sipa: ACK d8daa8f3711909223b117b8faa82daca87fc942d Tree-SHA512: 9e74086599799dc9b5c3fb8357445b662e5bf896d826af63d6d6b6ddb616612966f3bb5de3bd3ae0e692c47de85672f64b8ab6d3a1c45899dc25ba46990b5ec7
2019-12-06tests: Fix fuzzing harness for descriptor parsing (descriptor_parse)practicalswift
2019-12-06tests: Test serialisation as part of deserialisation fuzzing. Test ↵practicalswift
round-trip equality where possible. Avoid code repetition.
2019-12-06tests: Add fuzzing harness for various CTxOut related functionspracticalswift
2019-12-06tests: Add fuzzing harness for various CTxIn related functionspracticalswift
2019-12-06util: Update tinyformat to upstreamWladimir J. van der Laan
Last update was in 2017. Updates tinyformat to upstream commit 705e3f4e1de922069bf715746d35bd2364b1f98f. Re-apply bitcoin core specific changes. No changes that affect our use, as far as I can see, but this gets rid of the gcc `-Wimplicit-fallthrough` warnings, at least.
2019-12-06Merge #17051: tests: Add deserialization fuzzing harnessesWladimir J. van der Laan
897849d8c225045f0dd3a2fe99b5d69bdf84b4e2 tests: Add deserialization fuzzing harnesses (practicalswift) 16f0a186dcee563bb1000e1ffc51da87e7623bc6 tests: Add corpora suppression (FUZZERS_MISSING_CORPORA) for fuzzers missing in https://github.com/bitcoin-core/qa-assets/tree/master/fuzz_seed_corpus (practicalswift) Pull request description: Add deserialization fuzzing harnesses. **Testing this PR** Run: ``` $ CC=clang CXX=clang++ ./configure --enable-fuzz --with-sanitizers=address,fuzzer,undefined $ make $ contrib/devtools/test_fuzzing_harnesses.sh 'addr_info|block_file_info|block_filter|block_header|ext_key|ext_pub_key|fee_rate|flat_file|key_origin|merkle_block|mutable_transaction|out_point|partial_merkle_tree|partially_signed_transaction|prefilled_transaction|psbt_input|psbt_output|pub_key|script_deserialize|sub_net|tx_in' 10 ``` `test_fuzzing_harnesses.sh` can be found in PR #17000. ACKs for top commit: laanwj: thanks, ACK 897849d8c225045f0dd3a2fe99b5d69bdf84b4e2 Tree-SHA512: 5a270a3002cc23b725f7b35476a43777b2b00b4d089cc006372e2fcc7afa430afaa3c1430f778ae08fc53dd85a13e7bd2fab0449c319f676423226e189a417f6
2019-12-05Move events_hasher into RNGState()Pieter Wuille
2019-12-05Merge #17507: random: mark RandAddPeriodic and SeedPeriodic as noexceptWladimir J. van der Laan
55b2cb199c276781b6daa5438af2da57dea3ac52 random: mark RandAddPeriodic and SeedPeriodic as noexcept (fanquake) 461e547877da0c04db69e067c923cc4540aab03a doc: correct random.h docs after #17270 (fanquake) Pull request description: The usage of `MilliSleep()` in SeedPeriodic (previously SeedSleep) was [removed](https://github.com/bitcoin/bitcoin/pull/17270/commits/d61f2bb076d8f17840a8e79f1583d7f6e3e6d09a) in #17270, meaning it, and its users can now be marked `noexcept`. This also corrects the docs in random.h for some of the changes in #17270. ACKs for top commit: practicalswift: ACK 55b2cb199c276781b6daa5438af2da57dea3ac52 laanwj: ACK 55b2cb199c276781b6daa5438af2da57dea3ac52 sipa: ACK 55b2cb199c276781b6daa5438af2da57dea3ac52 Tree-SHA512: 672d369796e7c4f9b4d98dc545e5454999fa1bef373871994a26041d6163c58909e2255e4f820d3ef011679aa3392754eb57477306a89f5fd3d57e2bd7f0811a
2019-12-04Merge #17650: util: remove unwanted fields from bitcoin-cli -getinfofanquake
01c87015597021bf1c0856f7f6be175bdde844b2 util: remove unwanted fields from bitcoin-cli -getinfo (malevolent) Pull request description: Removed the following fields from -getinfo: protocolversion, walletversion and keypoololdest. This change closes #17314 . ACKs for top commit: laanwj: ACK 01c87015597021bf1c0856f7f6be175bdde844b2 achow101: ACK 01c87015597021bf1c0856f7f6be175bdde844b2 practicalswift: ACK 01c87015597021bf1c0856f7f6be175bdde844b2 -- diff looks correct Tree-SHA512: c98f2e8a3fee04d46766f70cb88f4e49e892a4424eff8940a7d48e9e808597b702427225788f984f5c3641591fd8d86acee56774afde1d57a4259c31d971ea08
2019-12-04Merge #17517: ci: Bump to clang-8 for asan build to avoid segfaults on ppc64leMarcoFalke
fa40e48c50d8ccf42ce5e66c12390e2ed4b60e75 ci: Remove unparseable lines from supp file for old xenial clang tsan (MarcoFalke) fa1bfc476c9208a4c412c8ca74d05f52bb47766f ci: ubsan report_error_type=1 and add suppressions (MarcoFalke) fa69cef13e5aab8264339eb3d50a9e89d59efd87 test: Print stderr when subprocess fails (MarcoFalke) 2222c305866a77065ab5be24c1c252bae252bb59 test: Use char instead of unsigned char (MarcoFalke) faa8023ce9a47b282e1fac3ca8b3a7bb0042935a ci: Bump to clang-8 for asan build to avoid segfaults on ppc64le (MarcoFalke) Pull request description: Use clang-8 instead of default clang (which is clang-6 on Bionic) to avoid spurious segfaults when running the ci system on ppc64le ACKs for top commit: practicalswift: ACK fa40e48c50d8ccf42ce5e66c12390e2ed4b60e75 assuming Travis is happy -- diff looks correct :) Tree-SHA512: f4f26232d3a0ef38da245869340f723d279a3db9823befbc735fb5a00024dae041c7306d7ae55d2488e6f86aa96cdea155b007aefb561fba505141e8dbc717dc
2019-12-04test: Use char instead of unsigned charMarcoFalke
2019-12-04Merge #17573: Seed RNG with precision timestamps on receipt of net messages.Wladimir J. van der Laan
02d8c56a18b9a2960888d6ec1209955105bae847 Seed RNG with precision timestamps on receipt of net messages. (Matt Corallo) Pull request description: See title. Exposes a generic dead-simple "SeedEvent" interface, but currently just used for net messages. ACKs for top commit: sipa: utACK https://github.com/bitcoin/bitcoin/commit/02d8c56a18b9a2960888d6ec1209955105bae847 laanwj: ACK 02d8c56a18b9a2960888d6ec1209955105bae847 meshcollider: utACK 02d8c56a18b9a2960888d6ec1209955105bae847 Tree-SHA512: 28eb39a201ee2b13393c5c64dbf7c1913f3482f095969ef5141bfe549ce77dd63bb5f14738f6eedb296c686ea36014aa157b9c5e8059710a318590f30e9caa14
2019-12-04Merge #17648: doc: rename wallet-tool references to bitcoin-walletWladimir J. van der Laan
e7ad4a2f8c07a82d6424b473f0d51dbd8f897b10 doc: rename wallet-tool references to bitcoin-wallet (Wilson Ccasihue S) Pull request description: Fix. text reference to executable bitcoin-wallet instead of wallet-tool, there is not a wallet-tool at bin/ folder. ACKs for top commit: fanquake: ACK e7ad4a2f8c07a82d6424b473f0d51dbd8f897b10 - thanks for following up. Tree-SHA512: aed41b08947728a4ff3a97a62858ee7c86e2e5d57dcbbd0aab492dae3d8a548bb60541924e68cf3a0aa3d53d7db0012b489462b466919cd83f05b2aa88b7fff7
2019-12-04util: remove unwanted fields from bitcoin-cli -getinfomalevolent
In accordance with #17314, Removing noisy fields from -getinfo. Fields removed: protocolversion, walletversion and keypoololdest. In addition to changing bitcoin-cli -getinfo, there is another change to test/functional/interface_bitcoin_cli.py. This change deletes tests that utilize removed -getinfo calls.
2019-12-03Merge #17555: test: add unit test for non-standard txs with wrong nVersionMarcoFalke
76303f65f92a0fbe9a90c0e807554a6daa860636 test: add unit test for non-standard txs with wrong nVersion (Dominik Spicher) Pull request description: Takes care of one of the missing cases of #17394: nVersion must be within the allowed range. ACKs for top commit: instagibbs: ACK https://github.com/bitcoin/bitcoin/pull/17555/commits/76303f65f92a0fbe9a90c0e807554a6daa860636 Tree-SHA512: 94464f781cf70a5616f7cea2014ae0a97a338c34411cc989c60389de2ce00368374811db78c919bda30e0ebf341fb830998a5e97c124dd8afc8feb726cedfd3a
2019-12-03Merge #17641: Add unit test for leveldb creation with unicode pathMarcoFalke
70ed2ab7ef9e7ebf56f77b7c410a345ff455938f Add unit test for DB creation with unicode path (Aaron Clauson) Pull request description: An issue arose when attempting to switch back to the main repo version of leveldb when the bitcoin data directory uses a unicode path. The leveldb windows file IO wrapper was using the *A ANSI win32 calls instead of the Unicode *W ones. This unit test will catch if the path created by leveldb doesn't match what we're expecting. For more info see https://github.com/google/leveldb/issues/755. ACKs for top commit: laanwj: ACK 70ed2ab7ef9e7ebf56f77b7c410a345ff455938f Tree-SHA512: fc6dbd3aa26a439016e63e8d4d931f218ce99094fc7887a13b54562ad4133047020288ecbcd622a8309f422ee1eda5df50bcb8c8e44442af36ed57b22c069004
2019-12-03Merge #17643: wallet: Fix origfee return for bumpfee with feerate argMarcoFalke
02afb0c550dc8529918460c845d1da3adf236eed Fix origfee return for bumpfee with feerate arg (Gregory Sanders) Pull request description: fixes https://github.com/bitcoin/bitcoin/issues/17642 and adds a simple test that would have caught it ACKs for top commit: achow101: ACK 02afb0c550dc8529918460c845d1da3adf236eed Tree-SHA512: 303e392e05407f204dffe360689b5bb5dc77fd462dd0e489bc0b6c8f94f89ab7fe2bd8cb47e4dc6dc5c23a619826d15f3bf6b02b2c8e96402fbb51953c462e2d
2019-12-02doc: rename wallet-tool references to bitcoin-walletWilson Ccasihue S
2019-12-02Key pool: Fix omitted pre-split count in GetKeyPoolSizeAndrew Chow
This is a bugfix: https://github.com/bitcoin/bitcoin/pull/16341#discussion_r330669214
2019-12-02Key pool: Change ReturnDestination interface to take address instead of keyAndrew Chow
In order for ScriptPubKeyMan to be generic and work with future ScriptPubKeyMans, ScriptPubKeyMan::ReturnDestination is changed to take a CTxDestination instead of a CPubKey. Since LegacyScriptPubKeyMan still deals with keys internally, a new map m_reserved_key_to_index is added in order to track the keypool indexes that have been reserved. The CPubKey argument of KeepDestination is also removed so that it is more generic. Instead of taking a CPubKey or a CTxDestination, we just use the nIndex given to find the pubkey.
2019-12-02Key pool: Move LearnRelated and GetDestination callsAndrew Chow
Addresses are determined by LegacyScriptPubKeyMan::GetReservedDestination instead of ReserveDestination::GetReservedDestination as other ScriptPubKeyMan implementations may construct addresses differently This does not change behavior.
2019-12-02Merge #17634: qt: Fix comparison function signaturefanquake
98fbd1cdffaa69357091cc67e959ac21119dfa16 Use correct C++11 header for std::swap() (Hennadii Stepanov) b66861e2e5e8a49e11e7489cf22c3007bc7082cc Fix comparison function signature (Hennadii Stepanov) Pull request description: This PR fixes build on CentOS 7 with GCC 4.8.5: ``` ... In file included from /usr/include/c++/4.8.2/algorithm:62:0, from ./serialize.h:11, from ./qt/sendcoinsrecipient.h:13, from ./qt/recentrequeststablemodel.h:8, from qt/recentrequeststablemodel.cpp:5: /usr/include/c++/4.8.2/bits/stl_algo.h: In instantiation of ‘_RandomAccessIterator std::__unguarded_partition(_RandomAccessIterator, _RandomAccessIterator, const _Tp&, _Compare) [with _RandomAccessIterator = QList<RecentRequestEntry>::iterator; _Tp = RecentRequestEntry; _Compare = RecentRequestEntryLessThan]’: /usr/include/c++/4.8.2/bits/stl_algo.h:2296:78: required from ‘_RandomAccessIterator std::__unguarded_partition_pivot(_RandomAccessIterator, _RandomAccessIterator, _Compare) [with _RandomAccessIterator = QList<RecentRequestEntry>::iterator; _Compare = RecentRequestEntryLessThan]’ /usr/include/c++/4.8.2/bits/stl_algo.h:2337:62: required from ‘void std::__introsort_loop(_RandomAccessIterator, _RandomAccessIterator, _Size, _Compare) [with _RandomAccessIterator = QList<RecentRequestEntry>::iterator; _Size = int; _Compare = RecentRequestEntryLessThan]’ /usr/include/c++/4.8.2/bits/stl_algo.h:5499:44: required from ‘void std::sort(_RAIter, _RAIter, _Compare) [with _RAIter = QList<RecentRequestEntry>::iterator; _Compare = RecentRequestEntryLessThan]’ qt/recentrequeststablemodel.cpp:208:82: required from here /usr/include/c++/4.8.2/bits/stl_algo.h:2263:35: error: no match for call to ‘(RecentRequestEntryLessThan) (RecentRequestEntry&, const RecentRequestEntry&)’ while (__comp(*__first, __pivot)) ^ In file included from qt/recentrequeststablemodel.cpp:5:0: ./qt/recentrequeststablemodel.h:43:7: note: candidate is: class RecentRequestEntryLessThan ^ qt/recentrequeststablemodel.cpp:217:6: note: bool RecentRequestEntryLessThan::operator()(RecentRequestEntry&, RecentRequestEntry&) const bool RecentRequestEntryLessThan::operator()(RecentRequestEntry &left, RecentRequestEntry &right) const ^ qt/recentrequeststablemodel.cpp:217:6: note: no known conversion for argument 2 from ‘const RecentRequestEntry’ to ‘RecentRequestEntry&’ In file included from /usr/include/c++/4.8.2/algorithm:62:0, from ./serialize.h:11, from ./qt/sendcoinsrecipient.h:13, from ./qt/recentrequeststablemodel.h:8, from qt/recentrequeststablemodel.cpp:5: /usr/include/c++/4.8.2/bits/stl_algo.h:2266:34: error: no match for call to ‘(RecentRequestEntryLessThan) (const RecentRequestEntry&, RecentRequestEntry&)’ while (__comp(__pivot, *__last)) ^ In file included from qt/recentrequeststablemodel.cpp:5:0: ./qt/recentrequeststablemodel.h:43:7: note: candidate is: class RecentRequestEntryLessThan ^ qt/recentrequeststablemodel.cpp:217:6: note: bool RecentRequestEntryLessThan::operator()(RecentRequestEntry&, RecentRequestEntry&) const bool RecentRequestEntryLessThan::operator()(RecentRequestEntry &left, RecentRequestEntry &right) const ^ qt/recentrequeststablemodel.cpp:217:6: note: no known conversion for argument 1 from ‘const RecentRequestEntry’ to ‘RecentRequestEntry&’ CXX qt/qt_libbitcoinqt_a-sendcoinsentry.o make[2]: *** [qt/qt_libbitcoinqt_a-recentrequeststablemodel.o] Error 1 ``` Also for `std::swap()` header `<algorithm>` is replaced with `<utility>` one. Refs: - [`std::swap()`](https://en.cppreference.com/w/cpp/algorithm/swap) - [standard library header `<utility>`](https://en.cppreference.com/w/cpp/header/utility) ACKs for top commit: promag: Code review ACK 98fbd1cdffaa69357091cc67e959ac21119dfa16. jonasschnelli: utACK 98fbd1cdffaa69357091cc67e959ac21119dfa16 fanquake: ACK 98fbd1cdffaa69357091cc67e959ac21119dfa16 Tree-SHA512: 91324490c1bdb98f186d233418e7e72ae7bee507876e94fb8c038bee031cea9e1046900f21156da4b7c33abcd726796867b124c4132d9ae3759877e90a8527db
2019-12-01Fix origfee return for bumpfee with feerate argGregory Sanders
2019-12-01Merge #17568: wallet: fix when sufficient preset inputs and ↵fanquake
subtractFeeFromOutputs eadd1304c81e0b89178e4cc7630bd31650850c85 tests: Add a test for funding with sufficient preset inputs and subtractFeeFromOutputs (Andrew Chow) ff330badd45067cb520b1cfa1844f60a4c9f2031 Default to bnb_used = false as there are many cases where BnB is not used (Andrew Chow) Pull request description: #17290 introduced a bug where, when we had preset inputs that covered the amount being sent and subtractFeeFrromOutputs was being used, transaction funding would result in a `Fee exceeds maximum configured by -maxtxfee` error. This was happening because we weren't setting `bnb_used = false` when the preset inputs were used as it should have been. This resulted in a too high fee because the change would go to fees accidentally. Apparently this particular case doesn't have a test, so I've added one as well. ACKs for top commit: Sjors: ACK eadd130. I can't get this new test to fail on macOS (without this PR). It passes whether or not I compile with `--enable-debug`. It does fail on Ubuntu. Yay undefined behavior... Anyway, it's a useful test. fanquake: ACK eadd1304c81e0b89178e4cc7630bd31650850c85 instagibbs: utACK https://github.com/bitcoin/bitcoin/pull/17568/commits/eadd1304c81e0b89178e4cc7630bd31650850c85 Tree-SHA512: 7286c321f78666eea558cc591174630d210263594df41cab1065417510591ee514ade0e1d0cec8af09a785757da68de82592b013e8fe8d4966cec3254368706e
2019-11-30Add unit test for DB creation with unicode pathAaron Clauson
An issue arose when attempting to switch back to the main repo version of leveldb when the bitcoin data directory uses a unicode path. The leveldb windows file IO wrapper was using the *A ANSI win32 calls instead of the Unicode *W ones. This unit test will catch if the path created by leveldb doesn't match what we're expecting. For more info see https://github.com/google/leveldb/issues/755.
2019-11-29Use correct C++11 header for std::swap()Hennadii Stepanov
2019-11-29Fix comparison function signatureHennadii Stepanov
This commit fixes build on CentOS 7 with GCC 4.8.5
2019-11-29Merge #17587: gui: show watch-only balance in send screenSamuel Dobson
4a96e459d733f1b6427221aaa1874ea00f79988a [gui] send: show watch-only balance in send screen (Sjors Provoost) 2689c8fd7159f47248c5fc365463be8b0e8b039c [test] qt: add send screen balance test (Sjors Provoost) Pull request description: Now that we can create a PSBT from a watch-only wallet (#16944), we should also display the watch-only balance on the send screen. Before: <img width="1008" alt="before" src="https://user-images.githubusercontent.com/10217/69533384-030e9180-0f78-11ea-9748-c32c957e822e.png"> After: <img width="1009" alt="Schermafbeelding 2019-11-26 om 11 44 17" src="https://user-images.githubusercontent.com/10217/69622879-19811f80-1042-11ea-8279-091012f39b38.png"> I added a test to check the balance on the send screen, but it only covers regular wallets. A better would add a watch-only only wallet. ACKs for top commit: meshcollider: utACK 4a96e459d733f1b6427221aaa1874ea00f79988a jb55: utACK 4a96e459d733f1b6427221aaa1874ea00f79988a promag: reACK 4a96e45, rebased and label change since last review. instagibbs: code review and light test ACK https://github.com/bitcoin/bitcoin/pull/17587/commits/4a96e459d733f1b6427221aaa1874ea00f79988a Tree-SHA512: 4213549888bd309f72bdbba1453218f4a2b07e809100d786a3791897c75468f9092b06fe4b971942b1c228aa75ee7c04971f262ca9a478b42756e056eb534620
2019-11-28Merge #17624: net: Fix an uninitialized read in ProcessMessage(…, "tx", ↵Wladimir J. van der Laan
…) when receiving a transaction we already have 73b96c94cb6c2afdee7f151768a96944ecaf9d9b net: Fix uninitialized read in ProcessMessage(...) (practicalswift) Pull request description: Fix an uninitialized read in `ProcessMessage(…, "tx", …)` when receiving a transaction we already have. The uninitialized value is read and used on [L2526 in the case of `AlreadyHave(inv) == true`](https://github.com/bitcoin/bitcoin/blob/d8a66626d63135fd245d5afc524b88b9a94d208b/src/net_processing.cpp#L2494-L2526). Proof of concept being run against a `bitcoind` built with MemorySanitizer (`-fsanitize=memory`): ``` $ ./p2p-uninit-read-in-conditional-poc.py Usage: ./p2p-uninit-read-in-conditional-poc.py <dstaddr> <dstport> <net> $ bitcoind -regtest & $ ./p2p-uninit-read-in-conditional-poc.py 127.0.0.1 18444 regtest SUMMARY: MemorySanitizer: use-of-uninitialized-value [1]+ Exit 77 bitcoind -regtest $ ``` Proof of concept being run against a `bitcoind` running under Valgrind (`valgrind --exit-on-first-error`): ``` $ valgrind -q --exit-on-first-error=yes --error-exitcode=1 bitcoind -regtest & $ ./p2p-uninit-read-in-conditional-poc.py 127.0.0.1 18444 regtest ==27351== Conditional jump or move depends on uninitialised value(s) [1]+ Exit 1 valgrind -q --exit-on-first-error=yes --error-exitcode=1 bitcoind -regtest $ ``` Proof of concept script: ``` #!/usr/bin/env python3 import sys from test_framework.mininode import NetworkThread from test_framework.mininode import P2PDataStore from test_framework.messages import CTransaction, CTxIn, CTxOut, msg_tx def send_duplicate_tx(dstaddr="127.0.0.1", dstport=18444, net="regtest"): network_thread = NetworkThread() network_thread.start() node = P2PDataStore() node.peer_connect(dstaddr=dstaddr, dstport=dstport, net=net)() node.wait_for_verack() tx = CTransaction() tx.vin.append(CTxIn()) tx.vout.append(CTxOut()) node.send_message(msg_tx(tx)) node.send_message(msg_tx(tx)) node.peer_disconnect() network_thread.close() if __name__ == "__main__": if len(sys.argv) != 4: print("Usage: {} <dstaddr> <dstport> <net>".format(sys.argv[0])) sys.exit(0) send_duplicate_tx(sys.argv[1], int(sys.argv[2]), sys.argv[3]) ``` Note that the transaction in the proof of concept is the simplest possible, but really any transaction can be used. It does not have to be a valid transaction. This bug was introduced in #15921 ("validation: Tidy up ValidationState interface") which was merged in to `master` 28 days ago. Luckily this bug was caught before being part of any Bitcoin Core release :) ACKs for top commit: jnewbery: utACK 73b96c94cb6c2afdee7f151768a96944ecaf9d9b laanwj: ACK 73b96c94cb6c2afdee7f151768a96944ecaf9d9b, thanks for discovering and reporting this before it ended up in a release. Tree-SHA512: 7ce6b8f260bcdd9b2ec4ff4b941a891bbef578acf4456df33b7a8d42b248237ec4949e65e2445b24851d1639b10681c701ad500b1c0b776ff050ef8c3812c795
2019-11-28Merge #17604: util: make ScheduleBatchPriority advisory onlyWladimir J. van der Laan
d2a3a5cadbe58c0fe363bbc6acac293d41eedf7e util: make ScheduleBatchPriority advisory only (fanquake) Pull request description: ACKs for top commit: laanwj: ACK d2a3a5cadbe58c0fe363bbc6acac293d41eedf7e Tree-SHA512: 14e44360bc6b0c0bfd794cb8a744af7d64fb01aa5602fdb392d6c54799a721ef04426e8379b157dd40f2a33c0b6a5248b09d59c865c453ff1f6e3abbafff524e
2019-11-27net: Fix uninitialized read in ProcessMessage(...)practicalswift
2019-11-26Default to bnb_used = false as there are many cases where BnB is not usedAndrew Chow
2019-11-26Add OutputType and CPubKey parameters to KeepDestinationAndrew Chow
These need to be added so that LearnRelatedScripts can be called from within KeepDestination later.
2019-11-26Rename Keep/ReturnKey to Keep/ReturnDestination and remove the wrapperAndrew Chow
There is no reason to have Keep/ReturnDestination to be a wrapper for Keep/ReturnKey. Instead just make them the same function.
2019-11-26Merge #17283: rpc: improve getaddressinfo test coverage, help, code docsWladimir J. van der Laan
33f5fc32e5bfbe1e89c4d20ce455bcc6dc194151 test: add rpc getaddressinfo labels test coverage (Jon Atack) 0f3539ac6d772fc646b5f184fa1efe77bf632f6a test: add listlabels test in wallet_labels.py (Jon Atack) 1388de83900eaced906d369fe9e8887ae74b2dcf rpc: add getaddressinfo code documentation (Jon Atack) 2ee0cb3330ccf70f0540cb42370796e32eff1569 rpc: update getaddressinfo RPCExamples to bech32 (Jon Atack) 8d1ed0c263f8cdff7189f02040b5d02238d93da0 rpc: clarify label vs labels in getaddressinfo RPCHelpman (Jon Atack) 5a0ed850700dfb19167d40b38f80313bd5e427ca rpc: improve getaddressinfo RPCHelpman content (Jon Atack) 70cda342cd20d0e0cd9f28405457544036968f2d rpc: improve getaddressinfo RPCHelpman formatting (Jon Atack) Pull request description: This PR is a continuation of the work in https://github.com/bitcoin/bitcoin/pull/12892. Main motivations: - There is currently no test coverage for the getaddressinfo `labels` response. Coverage here is a prerequisite before deprecating the `label` response or adding multiple labels per address. - `bitcoin-cli help getaddressinfo` returns a few content errors, difficult-to-read formatting, and no explanation why it returns both `label` and `labels` and how they relate, which can be confusing for application developers. Changes by order of commits: - [x] improve/fix getaddressinfo RPCHelpman layout formatting - [x] improve/fix getaddressinfo RPCHelpman content - [x] clarify the `label` and `labels` fields in getaddressinfo RPCHelpman - [x] update getaddressinfo RPCExamples addresses to bech32 - [x] add getaddressinfo code docs - [x] add a `listlabels` test assertion in wallet_labels.py - [x] add missing getaddressinfo `labels` test coverage and improve the existing `label` tests Here are gists of the CLI help output: [`bitcoin-cli help getaddressinfo` before this PR](https://gist.github.com/jonatack/022af5221a85c069780359a22643c810) [`bitcoin-cli help getaddressinfo` after this PR](https://gist.github.com/jonatack/4ee5f6abc62a3d99269570206a5f90ba) It seems we ought to begin a deprecation process for the getaddressinfo `label` field? If yes, I have a follow-up ready. _--> EDIT: Deprecation follow-ups #17578 and #17585 now build on this PR._ ACKs for top commit: fjahr: Re-ACK 33f5fc32e5bfbe1e89c4d20ce455bcc6dc194151 jnewbery: ACK 33f5fc32e5bfbe1e89c4d20ce455bcc6dc194151. Tree-SHA512: a001aa863090ec2566a31059477945b1c303ebeb430b33472f8b150e420fa5742fc33bca9d95571746395b607f43f6078dd5b53e238ac1f3fc648b51c8f79a07
2019-11-26Merge #17567: gui: remove macOS start on login codefanquake
27d82b63fb8869716d2f103fd381c2413bde4d1b gui: remove macOS start on login code (fanquake) Pull request description: The macOS startup item code was disabled for builds targeting macOS > `10.11` in #15208. Now that we require macOS `10.12` as a minimum (#17550), we can remove the startup item code entirely. The API we were using, `LSSharedFileListItemCopyResolvedURL`, `LSSharedFileListCopySnapshot` etc, was removed in macOS `10.12` SDK. ACKs for top commit: jonasschnelli: utACK 27d82b63fb8869716d2f103fd381c2413bde4d1b jonasschnelli: Tested ACK 27d82b63fb8869716d2f103fd381c2413bde4d1b - successfully compiled on 10.15.1 Tree-SHA512: 7420757b91c7820e6a63280887155394547134a9cebcf3721af0284da23292627f94cd431241e033075b3fd86d79ace3ebf1b25d17763acbf71e07a742395409