aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2019-11-06Merge #17340: Tests: speed up fundrawtransaction testMarcoFalke
af7bae734089f6af0029b0887932ccd9a469e12e [tests] Don't stop-start unnecessarily in rpc_fundrawtransaction.py (John Newbery) 9a8505299ba392acbab4647963113b0c29495f1d [tests] Use -whitelist in rpc_fundrawtransaction.py (John Newbery) 646b593bbd0db113c6e45ab92177b8f5251e8710 [tests] Speed up rpc_fundrawtransaction.py (John Newbery) Pull request description: Speed up rpc_fundrawtransaction.py Most of the time in rpc_fundrawtransaction.py is spent waiting for unconfirmed transactions to propagate. Net processing adds a poisson random delay to the time it will INV transactions with a mean interval of 5 seconds. Calls like the following: ``` self.nodes[2].sendrawtransaction(signedTx['hex']) self.sync_all() self.nodes[1].generate(1) ```` will therefore introduce a delay waiting for the mempools to sync. Instead just generate the block on the node that sent the transaction: ``` self.nodes[2].sendrawtransaction(signedTx['hex']) self.nodes[2].generate(1) ``` rpc_fundrawtransaction.py is not intended to be a test for transaction relay, so it's ok to do this. ACKs for top commit: MarcoFalke: ACK af7bae734089f6af0029b0887932ccd9a469e12e 🛴 Tree-SHA512: db3407d871bfdc99a02e7304b07239dd3585ac47f27f020f1a70608b7f6386b134343c01f3e4d1c246ce734676755897671999695068d6388602fb042d178780
2019-11-06[tests] Don't stop-start unnecessarily in rpc_fundrawtransaction.pyJohn Newbery
This was only added in c1dde3a949b36ce9c2155777b3fa1372e7ed97d8 to match behaviour when `encryptwallet` would restart the node. It's not required for the test (and slows things down).
2019-11-06[tests] Use -whitelist in rpc_fundrawtransaction.pyJohn Newbery
Makes tx relay faster
2019-11-06Merge #17381: LegacyScriptPubKeyMan code cleanupsWladimir J. van der Laan
05b224a175065aee4d6d9c471722bc4503f01fdf Add missing SetupGeneration error handling in EncryptWallet (Russell Yanofsky) bfd826a675445801adec86a469040f3ceb8172ee Clean up nested scope in GetReservedDestination (Russell Yanofsky) 491a599b37f3e3a648e52aebed677ca11b0615e2 Get rid of confusing LegacyScriptPubKeyMan::TopUpKeyPool method (Russell Yanofsky) 4a0abf694ee10cf186f25a67ca35c3fce0c10874 Pass CTxDestination to ScriptPubKeyMan::GetMetadata (Russell Yanofsky) b07b07cd8779355ba1dd16e7eb4af42e0ae1c587 Add EnsureLegacyScriptPubKeyMan and use in rpcwallet.cpp (Russell Yanofsky) Pull request description: This PR implements suggested code cleanups from #17300 and #17304 review comments ACKs for top commit: Sjors: re-ACK 05b224a laanwj: Code review ACK 05b224a175065aee4d6d9c471722bc4503f01fdf Tree-SHA512: 12fd86637088515b744c028e0501c5d21a9cf9ee9c9cfd70e9cb65d44611ea5643abd5f6f101105caa5aff015d74de606f074f08af7dae8429f929d21288ab45
2019-11-06Merge #17390: test: Add util_ArgParsing testWladimir J. van der Laan
286f197704e82045c762d332aba5d1ac52e0212d Add util_ArgParsing test (Russell Yanofsky) Pull request description: ArgsManager test coverage for parsing of integer and boolean values is currently very poor and doesn't give us a way of knowing whether changes to ArgsManager may unintentionally break backwards compatibility, so this adds a new test to catch regressions. ACKs for top commit: promag: ACK 286f197, more surprising results 😱 laanwj: ACK 286f197704e82045c762d332aba5d1ac52e0212d Tree-SHA512: 9e1db3ef87e55abbc280af60c088f35765a1f9e2ec20507ad0c1992027b875490016868dcb8cc287e6df279dd0e00f10550901af3de3d36287867249e0bd8207
2019-11-06Merge #17339: doc: Add template for good first issuesWladimir J. van der Laan
7b78b8d3a64503d582af8298241a20ebf582a0c5 doc: Add template for good first issues (Michael Folkson) Pull request description: closes #17317 Attempted to address everyone's suggestions in #17317 without making it too long. The first half is for the benefit of the individual opening the issue and the second half is for the benefit of the new contributor. Ideally we don't want the second half to be deleted by the individual opening the issue but whether they delete the first half or not isn't really a concern ACKs for top commit: MarcoFalke: ACK 7b78b8d3a64503d582af8298241a20ebf582a0c5 jonatack: ACK 7b78b8d3a64503d582af8298241a20ebf582a0c5 Tree-SHA512: 5874b244a52f432637600a73aac493972971568f8d8af10aa731b8a6b221566015827dd82c310c60a76fb01140c3bc56a691206c3442018611c820d4b98d104f
2019-11-06Merge #17353: doc: Add ShellCheck to lint tests dependenciesMarcoFalke
80c9e66ab84f8cecc2bf2eebf508a5aad8911246 build: Remove install command samples (Hennadii Stepanov) 2ad74b78c61697068be5d157785592c0c875a347 doc: Add ShellCheck to lint tests dependencies (Hennadii Stepanov) Pull request description: In master (9641366950276da88af626d0898676195df8d83a) the lint tests dependencies list lacks ShellCheck. This PR fixes it. Also `lint-python.sh` is slightly improved. ACKs for top commit: laanwj: ACK 80c9e66ab84f8cecc2bf2eebf508a5aad8911246 promag: ACK 80c9e66ab84f8cecc2bf2eebf508a5aad8911246, verified internal and external links. Nice looking table. Tree-SHA512: b63718a6c41be93137db70586465d84ca0b1ff33c0f2674147c928cb1bdf903ec7587861c09ad832841264285f99c7b171d5319eef3c989822a7cd01449222ae
2019-11-06build: Remove install command samplesHennadii Stepanov
test/README.md contains comprehensive install instructions.
2019-11-06Merge #17368: cli: fix -getinfo output when compiled with no walletWladimir J. van der Laan
3d05d332693ec860626fc77e6ba50dec94e4e83c cli: fix -getinfo output when compiled with no wallet (fanquake) Pull request description: master (33b155f28732487854cf0ca29ca17c50f8c6872e): ```bash src/bitcoin-cli -getinfo { "version": 199900, "protocolversion": 70015, "blocks": 602348, "headers": 602348, "verificationprogress": 0.9999995592310106, "timeoffset": 0, "connections": 10, "proxy": "", "difficulty": 13691480038694.45, "chain": "main", "walletversion": null, "balance": null, "keypoololdest": null, "keypoolsize": null, "paytxfee": null, "relayfee": 0.00001000, "warnings": "This is a pre-release test build - use at your own risk - do not use for mining or merchant applications" } ``` This PR (3d05d332693ec860626fc77e6ba50dec94e4e83c): ```bash { "version": 199900, "protocolversion": 70015, "blocks": 602348, "headers": 602348, "verificationprogress": 0.9999996313568186, "timeoffset": 0, "connections": 10, "proxy": "", "difficulty": 13691480038694.45, "chain": "main", "relayfee": 0.00001000, "warnings": "This is a pre-release test build - use at your own risk - do not use for mining or merchant applications" } ``` ACKs for top commit: MarcoFalke: ouch ACK 3d05d332693ec860626fc77e6ba50dec94e4e83c laanwj: ACK 3d05d332693ec860626fc77e6ba50dec94e4e83c darosior: ACK 3d05d332693ec860626fc77e6ba50dec94e4e83c Tree-SHA512: 055424e122a082cbfea410da287d9ceb7ed405fd68d53e2f5bef62beea80bc374a7d00366de0479d23faecb7f063b232aca52e9fdbdb97c58ddf46e7749136a9
2019-11-06Merge #17388: Add missing newline in util_ChainMerge testWladimir J. van der Laan
3645e4ca0033bb6365f41ef710111780c139370f Add missing newline in util_ChainMerge test (Russell Yanofsky) Pull request description: This was causing a lot of test cases not not be very meaningful because multiple configuration options were combined into one line. The changes in test output with this fix make sense and look like: ```diff - testnet=1 regtest=1 || test + testnet=1 regtest=1 || error: Invalid combination of -regtest, -testnet and -chain. Can use at most one. ``` Issue was reported and debugged by Wladimir J. van der Laan <laanwj@protonmail.com> in https://github.com/bitcoin/bitcoin/pull/17385#issuecomment-550033222 <!-- *** 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. --> <!-- 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: laanwj: ACK 3645e4ca0033bb6365f41ef710111780c139370f practicalswift: ACK 3645e4ca0033bb6365f41ef710111780c139370f -- diff looks correct Tree-SHA512: ca5bde9b9f553811d4827113f4880d15d7b8f4f1455b95bbf34c9a1512fdd53062f1a2133c50d9b54f94160a1ee77a54bc82681a5f3bf25d2b0d01f8a8e95165
2019-11-05Merge #17370: doc: Update doc/bips.md with recent changes in masterfanquake
fa7f5a4d2a2581cc25125311892a80efc2c494e2 doc: Update doc/bips.md with recent changes in master (MarcoFalke) Pull request description: Follow-up to #17165 ACKs for top commit: jonatack: ACK fa7f5a4d2a2581cc25125311892a80efc2c494e2. Verified markdown view at https://github.com/MarcoFalke/bitcoin-core/blob/1911-docBips/doc/bips.md and the urls in the links. Some of the PRs are indicated with # and some without, but this is the case over the whole document. laanwj: ACK fa7f5a4d2a2581cc25125311892a80efc2c494e2 fanquake: ACK fa7f5a4d2a2581cc25125311892a80efc2c494e2 Tree-SHA512: 31782b5f1f2f10b1189f05f010f908c183dbe723477ca1c46ad1d3bee5ea483335847008a7fe48d72373ccd39b84e0b950d0d1b23e457cb70f34210c5f2dc6aa
2019-11-05Add util_ArgParsing testRussell Yanofsky
ArgsManager test coverage for parsing of integer and boolean values is currently very poor and doesn't give us a way of knowing whether changes to ArgsManager may unintentionally break backwards compatibility, so this adds a new test to catch regressions.
2019-11-06Merge #17382: rpc: Remove unused boost::this_thread::interruption_pointWladimir J. van der Laan
fa5facd3e72b6d61374b0b93b722b55e2b090020 rpc: Remove unused boost::this_thread::interruption_point (MarcoFalke) Pull request description: There are predefined interruption points for `boost::thread`: https://www.boost.org/doc/libs/1_71_0/doc/html/thread/thread_management.html#interruption_points However, the rpc threads are `std::thread`, which does not have an `std::thread::interrupt` member function to request interruption: https://dev.visucore.com/bitcoin/doxygen/httpserver_8cpp.html#ae1a63374e18b9abd348eb74e4243ea34 Thus, the interruption points can be removed. ACKs for top commit: laanwj: ACK fa5facd3e72b6d61374b0b93b722b55e2b090020, this does nothing. practicalswift: ACK fa5facd3e72b6d61374b0b93b722b55e2b090020 jamesob: ACK https://github.com/bitcoin/bitcoin/pull/17382/commits/fa5facd3e72b6d61374b0b93b722b55e2b090020 Tree-SHA512: 4e29a44df1f2702cbd1ffdffa559440a8bb800baab64b4116e2c3d27cd64d8d1e8aafe1dc21b1a4e3988470d03be19cae294bd5669f7abf6d487685dc8fd8d7e
2019-11-05Merge #16805: logs: add timing information to FlushStateToDisk()Wladimir J. van der Laan
dcef9a2922317cb2849f397366b6c56d755db6c9 logs: add timing information to FlushStateToDisk() (James O'Beirne) 41edaf227a69bc4846d5996675e8763fdfe0f367 logs: add BCLog::Timer and related macros (James O'Beirne) Pull request description: It's currently annoying to detect FlushStateToDisk() calls when benchmarking since they have to be inferred from a drop in coins count from the `UpdateTip: ` log messages. This adds a new logging utility, `BCLog::Timer`, and some related macros that are generally useful for printing timing-related logging messages, and a message that is unconditionally written when the coins cache is flushed to disk. ``` 2019-09-04T20:17:51Z FlushStateToDisk: write block and undo data to disk completed (3ms) 2019-09-04T20:17:51Z FlushStateToDisk: write block index to disk completed (370ms) 2019-09-04T20:17:51Z FlushStateToDisk: write coins cache to disk (2068451 coins, 294967kB) completed (21481ms) ``` ACKs for top commit: laanwj: Thanks, ACK dcef9a2922317cb2849f397366b6c56d755db6c9 ryanofsky: Code review ACK dcef9a2922317cb2849f397366b6c56d755db6c9. No changes since last review other than moving code to new timer.h header Tree-SHA512: 6d61e48a062d3edb48d0e056a6f0b1f8031773cc99289ee4544f8349d24526b88519e1e304009d56e428f1eaf76c857bf8e7e1c0b6873a6f270306accb5edc3d
2019-11-05Add missing newline in util_ChainMerge testRussell Yanofsky
This was causing a lot of test cases not not be very meaningful because multiple configuration options were combined into one line. The changes in test output with this fix make sense and look like: ```diff - testnet=1 regtest=1 || test + testnet=1 regtest=1 || error: Invalid combination of -regtest, -testnet and -chain. Can use at most one. ``` Issue was reported and debugged by Wladimir J. van der Laan <laanwj@protonmail.com> in https://github.com/bitcoin/bitcoin/pull/17385#issuecomment-550033222
2019-11-05Merge #17360: gui: Improve "Hide" button tool-tip messageWladimir J. van der Laan
1c26c16065182ca2d2cdbb05fae79cac8c75f17d Improve "Hide" button tool-tip message (Danny-Scott) Pull request description: Cleaned up the tool tip text, it looks as though it just got included back in 2014 when the whole section was added. Changed hide button tool tip within transaction fee settings area from "collapse fee-settings" to "Hide transaction fee settings" to be more user friendly and fit with other tool tips. ![hide-transaction-fee-tool-tip](https://user-images.githubusercontent.com/17258195/68086415-b7b70680-fe43-11e9-82cb-567b9730c1b9.png) ACKs for top commit: laanwj: ACK 1c26c16065182ca2d2cdbb05fae79cac8c75f17d Tree-SHA512: e2c83271c273f785ac625da9f88e095076043e21a9c59792049c271747837d19483e0cae5466c26ef3231947b6245680c4c136a530ba6f1885f9ddc18f2560d6
2019-11-05Merge #16540: test: Add ASSERT_DEBUG_LOG to unit test frameworkMarcoFalke
fa2c44c3ccc3e7a54e2afc862addd555d5a6e51b test: Add ASSERT_DEBUG_LOG to unit test framework (MarcoFalke) fa1936f57bbf5aebb1f8fc18701441d79219d443 logging: Add member for arbitrary print callbacks (MarcoFalke) Pull request description: Similar to `assert_debug_log` in the functional test framework Top commit has no ACKs. Tree-SHA512: aa9eaeca386b61d806867c04a33275f6eb4624fa5bf50f2928d16c83f5634bac96bcac46f9e8eda3b00b4251c5f12d7b01d6ffd84ba8e05c09eeec810cc31251
2019-11-05Improve "Hide" button tool-tip messageDanny-Scott
2019-11-05rpc: Remove unused boost::this_thread::interruption_pointMarcoFalke
2019-11-05Merge #16899: UTXO snapshot creation (dumptxoutset)Wladimir J. van der Laan
92b2f5306ba0b3f031293cb8f415b67cb002c2f1 test: add dumptxoutset RPC test (James O'Beirne) c1ccbc3ddef931896a7e9dcfa6704e305a69fbff devtools: add utxo_snapshot.sh (James O'Beirne) 57cf74c9918d10c69a46e6ceb3cb1a5e04edf5bc rpc: add dumptxoutset (James O'Beirne) 92fafb3a7da66f737e960e541fcfbcadedf6043a coinstats: add coins_count (James O'Beirne) 707fde7b9ba522c22179e2db0ed7b462c65138d9 add unused SnapshotMetadata class (James O'Beirne) Pull request description: This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11): Parent PR: #15606 Issue: #15605 Specification: https://github.com/jamesob/assumeutxo-docs/tree/master/proposal --- This changeset defines the serialization format for UTXO snapshots and adds an RPC command for creating them, `dumptxoutset`. It also adds a convenience script for generating and verifying snapshots at a certain height, since that requires doing a hacky rewind of the chain via `invalidateblock`. All of this is unused at the moment. ACKs for top commit: laanwj: ACK 92b2f5306ba0b3f031293cb8f415b67cb002c2f1 Tree-SHA512: 200dff87767f157d627e99506ec543465d9329860a6cd49363081619c437163a640a46d008faa92b1f44fd403bfc7a7c9e851c658b5a4849efa9a34ca976bf31
2019-11-05test: add dumptxoutset RPC testJames O'Beirne
2019-11-05devtools: add utxo_snapshot.shJames O'Beirne
to allow easy (if not time-consuming) generation and verification of snapshots.
2019-11-05rpc: add dumptxoutsetJames O'Beirne
Allows the creation of a UTXO snapshot to disk.
2019-11-05Merge #17044: init: Remove auto-import of `bootstrap.dat` and associated codeWladimir J. van der Laan
104f7de5934f13b837fcf21f6d6b2559799eabe2 remove old bootstrap relevant code (tryphe) Pull request description: This picks up #15954 I fixed the code and added at a functional test utilizing the scripts in `contrib/linearize` as suggested by @MarcoFalke . ACKs for top commit: laanwj: ACK 104f7de5934f13b837fcf21f6d6b2559799eabe2 Tree-SHA512: acac9f285f9785fcbc3afc78118461e45bec2962f90ab90e9f82f3ad28adc90a44f0443b712458ccf486e46d891eb8a67f53e7bee5fa6d89e4387814fe03f117
2019-11-05Merge #17363: test: add "diamond" unit test to MempoolAncestryTestsMarcoFalke
b2ff500fb3e4fa05de366ab1900825bea1f70377 test: add "diamond" unit test to MempoolAncestryTests (Sebastian Falbesoner) Pull request description: Approaches #17271 (_Missing Unit Test for Ancestors "diamond"_). If ancestors are represented more than once (in this case `ta` and `tb`), check that those are not overcounted. ACKs for top commit: laanwj: ACK b2ff500fb3e4fa05de366ab1900825bea1f70377 Tree-SHA512: 82a6573cc7f0e82bf6fcfe207d7ddecbf297d2a203d22e95b73d887e3cb280f45a3c5f649161561c1be1eb560ff81b9b385868f205d1c12284211c2377e5ad99
2019-11-05Merge #17357: tests: Add fuzzing harness for Bech32 encoding/decodingMarcoFalke
b7541705d0abfbddf682a0134f3fa8a8e1d06cdf tests: Add fuzzing harness for Bech32 encoding/decoding (practicalswift) 85a34b1683233060c4500cfb8e3f5d84c8b8f0da tests: Move CaseInsensitiveEqual to test/util/str (practicalswift) Pull request description: Add fuzzing harness for Bech32 encoding/decoding. **Testing this PR** Run: ``` $ make distclean $ ./autogen.sh $ CC=clang CXX=clang++ ./configure --enable-fuzz \ --with-sanitizers=address,fuzzer,undefined $ make $ src/test/fuzz/bech32 -max_total_time=60 … ``` ACKs for top commit: jonatack: ACK b7541705d0abfbddf682a0134f3fa8a8e1d06cdf Tree-SHA512: ade01d30c6886a083b806dbfff08999cc0d08e687701c670c895e261ed242c789e8a0062d4ebbe8f82676b8f168dc37e83351a88822c9c0eab478572a9e1ec02
2019-11-05Merge #17243: p2p: add PoissonNextSend method that returns mockable timeMarcoFalke
1a8f0d5a74d5cc0000456932babf35301f5c1686 [tools] update nNextInvSend to use mockable time (Amiti Uttarwar) 4de630354fc6808b9b13b9e82da1a82f2f50f26a [tools] add PoissonNextSend method that returns mockable time (Amiti Uttarwar) Pull request description: Introduce a Poisson helper method that wraps the existing method to return `std::chrono::duration` type, which is mockable. Needed for https://github.com/bitcoin/bitcoin/pull/16698. ACKs for top commit: ajtowns: ACK 1a8f0d5a74d5cc0000456932babf35301f5c1686 MarcoFalke: re-ACK 1a8f0d5a74d5cc0000456932babf35301f5c1686 naumenkogs: ACK 1a8f0d5, and let's merge it and come back to it later. Tree-SHA512: 7e2325d7c55fc0b4357cb86b83e0c218ba269f678c1786342d8bc380bfd9696373bc24ff124b9ff17a6e761c62b2b44ff5247c3911e2afdc7cc5c20417e8290b
2019-11-05Merge #17378: TestShell: Fix typos & implement cleanupsMarcoFalke
2493770e365a7e30117dc1b8d228f9cbed97f7e1 TestShell: Return self from setup() (James Chiang) a8dea4552412668c2914b6395ef543341a9898cd TestShell: Simplify default setting of num_nodes (James Chiang) 9c7806e4bf113bee6c32cff7b46493fd1a5aa0ba Doc: Remove backticks in test-shell.md code block (James Chiang) d3ed06e2cdb31dcecf5e647f7e1e52185cc76733 TestShell: Fix typo in TestShell warning printout (James Chiang) Pull request description: This PR follows up on #17288 and fixes typos and implements code clean-ups suggested by reviewers of 19139ee. - Typo in `test_shell.py` warning - Typo in `test-shell.md` code block - Simplified default setting of `num_nodes` in `TestShell.setup()` - Enable initializer chaining: `TestShell().setup()` ACKs for top commit: MarcoFalke: ACK 2493770e365a7e30117dc1b8d228f9cbed97f7e1 instagibbs: tACK https://github.com/bitcoin/bitcoin/pull/17378/commits/2493770e365a7e30117dc1b8d228f9cbed97f7e1 jnewbery: utACK 2493770e365a7e30117dc1b8d228f9cbed97f7e1 Tree-SHA512: 8fa7c2c550dbc3ec899de9dc328cd55cfa6daafe3b888aa5427e72fea69f064d938ec68e15bfa57109c0f6c3583e627ac4bd69303a11575d056941bd253adee0
2019-11-05doc: Add template for good first issuesMichael Folkson
2019-11-05coinstats: add coins_countJames O'Beirne
Also changes existing CCoinsStats attributes to be C++11 initialized.
2019-11-05Add missing SetupGeneration error handling in EncryptWalletRussell Yanofsky
Suggested https://github.com/bitcoin/bitcoin/pull/17304#discussion_r341286026 by me
2019-11-05remove old bootstrap relevant codetryphe
- only load blockfiles when we have paths - add release notes for modified bootstrap functionality - amend documentation on ThreadImport
2019-11-05Clean up nested scope in GetReservedDestinationRussell Yanofsky
Suggested https://github.com/bitcoin/bitcoin/pull/17304#discussion_r341194391 by Gregory Sanders <gsanders87@gmail.com> Reason for keeping the `return true` `return false` verbosity is that more code will be added after the ReserveKeyFromKeyPool() call before returning.
2019-11-05Get rid of confusing LegacyScriptPubKeyMan::TopUpKeyPool methodRussell Yanofsky
Previous discussion https://github.com/bitcoin/bitcoin/pull/17304#discussion_r340307903
2019-11-05Pass CTxDestination to ScriptPubKeyMan::GetMetadataRussell Yanofsky
Pass CTxDestination instead of more ambiguous uint160 hash value. This is more type safe and more efficient since it avoids doing map lookups that will always fail and were not done previously before a18edd7b383d667b15b6d4b87aa3a055a9fa5051 from https://github.com/bitcoin/bitcoin/pull/17304 Change suggested by Andrew Chow <achow101-github@achow101.com> in https://github.com/bitcoin/bitcoin/pull/17304#discussion_r340345745 and https://github.com/bitcoin/bitcoin/pull/17381#issuecomment-549994944
2019-11-05doc: Update doc/bips.md with recent changes in masterMarcoFalke
2019-11-05Add EnsureLegacyScriptPubKeyMan and use in rpcwallet.cppRussell Yanofsky
This also fixes unused variable warnings in rpcdump.cpp
2019-11-05Merge #17365: depends: update README.md with working Android targets and API ↵MarcoFalke
levels 3fe1aba6016c6501c8a02d8bd812f06397279100 depends: move README.md Android instructions to a separate section (Igor Cota) aa9b84acee8beda82861ad69139efeefadcd19bb depends: update README.md with working Android targets and API levels (Igor Cota) Pull request description: Per @Sjors comments in https://github.com/bitcoin/bitcoin/pull/16110#pullrequestreview-310821810 ACKs for top commit: Sjors: ACK 3fe1aba Tree-SHA512: 7a2e676070d51c7a4291b0d4b638f52321c08cc6ebe2bd2c02ba62f6cc3dd8a73227df4693c6ce9201863eb0bf26e0133805347b9016cb0f9a389a49cc9492aa
2019-11-05TestShell: Return self from setup()James Chiang
This allows user to chain setup() to the initializer. test-shell.md code examples have been updated to reflect this.
2019-11-05TestShell: Simplify default setting of num_nodesJames Chiang
2019-11-05Doc: Remove backticks in test-shell.md code blockJames Chiang
2019-11-05TestShell: Fix typo in TestShell warning printoutJames Chiang
2019-11-05[tools] update nNextInvSend to use mockable timeAmiti Uttarwar
2019-11-05[tools] add PoissonNextSend method that returns mockable timeAmiti Uttarwar
2019-11-05depends: move README.md Android instructions to a separate sectionIgor Cota
2019-11-05tests: Add fuzzing harness for Bech32 encoding/decodingpracticalswift
2019-11-05tests: Move CaseInsensitiveEqual to test/util/strpracticalswift
2019-11-05Merge #16766: wallet: Make IsTrusted scan parents recursivelySamuel Dobson
4671fc3d9e669da8b8781f0cbefee43cb9acd527 Expand on wallet_balance.py comment from https://github.com/bitcoin/bitcoin/pull/16766\#issuecomment-527563982 (Jeremy Rubin) 91f3073f08aff395dd813296bf99fd8ccc81bb27 Update release notes to mention changes to IsTrusted and impact on wallet (Jeremy Rubin) 8f174ef112199aa4e98d756039855cc561687c2e Systematize style of IsTrusted single line if (Jeremy Rubin) b49dcbedf79613f0e0f61bfd742ed265213ed280 update variable naming conventions for IsTrusted (Jeremy Rubin) 5ffe0d144923f365cb1c2fad181eca15d1668692 Update comment in test/functional/wallet_balance.py (Jeremy Rubin) a550c58267f50c59c2eea1d46edaa5019a8ad5d8 Update wallet_balance.py test to reflect new behavior (Jeremy Rubin) 5dd7da4ccd1354f09e2d00bab29288db0d5665d0 Reuse trustedParents in looped calls to IsTrusted (Jeremy Rubin) 595f09d6de7f1b94428cdd1310777aa6a4c584e5 Cache tx Trust per-call to avoid DoS (Jeremy Rubin) dce032ce294fe0d531770f540b1de00dc1d13f4b Make IsTrusted scan parents recursively (Jeremy Rubin) Pull request description: This slightly modifies the behavior of IsTrusted to recursively check the parents of a transaction. Otherwise, it's possible that a parent is not IsTrusted but a child is. If a parent is not trusted, then a child should not be either. This recursive scan can be a little expensive, so ~it might be beneficial to have a way of caching IsTrusted state, but this is a little complex because various conditions can change between calls to IsTrusted (e.g., re-org).~ I added a cache which works per call/across calls, but does not store the results semi-permanently. Which reduces DoS risk of this change. There is no risk of untrusted parents causing a resource exploitation, as we immediately return once that is detected. This is a change that came up as a bug-fix esque change while working on OP_SECURETHEBAG. You can see the branch where this change is important here: https://github.com/bitcoin/bitcoin/compare/master...JeremyRubin:stb-with-rpc?expand=1. Essentially, without this change, we can be tricked into accepting an OP_SECURETHEBAG output because we don't properly check the parents. As this was a change which, on its own, was not dependent on OP_SECURETHEBAG, I broke it out as I felt the change stands on its own by fixing a long standing wallet bug. The test wallet_balance.py has been corrected to meet the new behavior. The below comment, reproduced, explains what the issue is and the edge cases that can arise before this change. # Before `test_balance()`, we have had two nodes with a balance of 50 # each and then we: # # 1) Sent 40 from node A to node B with fee 0.01 # 2) Sent 60 from node B to node A with fee 0.01 # # Then we check the balances: # # 1) As is # 2) With transaction 2 from above with 2x the fee # # Prior to #16766, in this situation, the node would immediately report # a balance of 30 on node B as unconfirmed and trusted. # # After #16766, we show that balance as unconfirmed. # # The balance is indeed "trusted" and "confirmed" insofar as removing # the mempool transactions would return at least that much money. But # the algorithm after #16766 marks it as unconfirmed because the 'taint' # tracking of transaction trust for summing balances doesn't consider # which inputs belong to a user. In this case, the change output in # question could be "destroyed" by replace the 1st transaction above. # # The post #16766 behavior is correct; we shouldn't be treating those # funds as confirmed. If you want to rely on that specific UTXO existing # which has given you that balance, you cannot, as a third party # spending the other input would destroy that unconfirmed. # # For example, if the test transactions were: # # 1) Sent 40 from node A to node B with fee 0.01 # 2) Sent 10 from node B to node A with fee 0.01 # # Then our node would report a confirmed balance of 40 + 50 - 10 = 80 # BTC, which is more than would be available if transaction 1 were # replaced. The release notes have been updated to note the new behavior. ACKs for top commit: ariard: Code Review ACK 4671fc3, maybe extend DoS protection in a follow-up PR. fjahr: Code review ACK 4671fc3d9e669da8b8781f0cbefee43cb9acd527 ryanofsky: Code review ACK 4671fc3d9e669da8b8781f0cbefee43cb9acd527. Changes since last review: 2 new commits adding suggested release note and python test comment, also a clean rebase with no changes to the earlier commits. The PR description is more comprehensive now, too. Looks good! promag: Code review ACK 4671fc3d9e669da8b8781f0cbefee43cb9acd527. Tree-SHA512: 6b183ff425304fef49724290053514cb2770f4a2350dcb83660ef24af5c54f7c4c2c345b0f62bba60eb2d2f70625ee61a7fab76a7f491bb5a84be5c4cc86b92f
2019-11-05Merge #17258: Fix issue with conflicted mempool tx in listsinceblockSamuel Dobson
436ad436434b94982bcb7dc1d13a21949263ef73 Fix issue with conflicted mempool tx in listsinceblock (Adam Jonas) Pull request description: Closes #8752 by bringing back abandoned #10470. This now checks that returned transactions are not conflicting with any transactions that are filtered out by the given blockhash and add a functional test to prevent this in the future. For more context, #8757 was closed in favor of #10470. ACKs for top commit: instagibbs: utACK https://github.com/bitcoin/bitcoin/pull/17258/commits/436ad436434b94982bcb7dc1d13a21949263ef73 kallewoof: utACK 436ad436434b94982bcb7dc1d13a21949263ef73 jonatack: I'm not qualifed to give an ACK here but 436ad436434b94982bcb7dc1d13a21949263ef73 appears reasonable. Built/ran tests/verified that this test fails without the change in rpcwallet.cpp: Tree-SHA512: 63d75cd3d3f19fc84dc38899b200c96179b82b24db263cd0116ee5b715265be647157855c2e35912d2fbc49c7b37db9375d6aab0ac672f0f09bece8431de5ea9
2019-11-04Merge #17288: Added TestShell class for interactive Python environments.MarcoFalke
19139ee034d20ebab1b91d3ac13a8eee70b59374 Add documentation for test_shell submodule (JamesC) f5112369cf91451d2d0bf574a9bfdaea04696939 Add TestShell class (James Chiang) 5155602a636c323424f75272ccec38588b3d71cd Move argparse() to init() (JamesC) 2ab01462f48b2d4e0d03ba842c3af8851c67c6f1 Move assert num_nodes is set into main() (JamesC) 614c645643e86c4255b98c663c10f2c227158d4b Clear TestNode objects after shutdown (JamesC) 6f40820757d25ff1ccfdfcbdf2b45b8b65308010 Add closing and flushing of logging handlers (JamesC) 6b71241291a184c9ee197bf5f0c7e1414417a0a0 Refactor TestFramework main() into setup/shutdown (JamesC) ede8b7608e115364b5bb12e7f39d662145733de6 Remove network_event_loop instance in close() (JamesC) Pull request description: This PR refactors BitcoinTestFramework to encapsulate setup and shutdown logic into dedicated methods, and adds a ~~TestWrapper~~ TestShell child class. This wrapper allows the underlying BitcoinTestFramework to run _between user inputs_ in a REPL environment, such as a Jupyter notebook or any interactive Python3 interpreter. The ~~TestWrapper~~ TestShell is motivated by the opportunity to expose the test-framework as a prototyping and educational toolkit. Examples of code prototypes enabled by ~~TestWrapper~~ TestShell can be found in the Optech [Taproot/Schnorr](https://github.com/bitcoinops/taproot-workshop) workshop repository. Usage example: ``` >>> import sys >>> sys.path.insert(0, "/path/to/bitcoin/test/functional") ``` ``` >>> from test_framework.test_wrapper import TestShell >>> test = TestShell() >>> test.setup(num_nodes=2) 20XX-XX-XXTXX:XX:XX.XXXXXXX TestFramework (INFO): Initializing test directory /path/to/bitcoin_func_test_XXXXXXX ``` ``` >>> test.nodes[0].generate(101) >>> test.nodes[0].getblockchaininfo()["blocks"] 101 ``` ``` >>> test.shutdown() 20XX-XX-XXTXX:XX:XX.XXXXXXX TestFramework (INFO): Stopping nodes 20XX-XX-XXTXX:XX:XX.XXXXXXX TestFramework (INFO): Cleaning up /path/to/bitcoin_func_test_XXXXXXX on exit 20XX-XX-XXTXX:XX:XX.XXXXXXX TestFramework (INFO): Tests successful ``` **Overview of changes to BitcoinTestFramework:** - Code moved to `setup()/shutdown()` methods. - Argument parsing logic encapsulated by `parse_args` method. - Success state moved to `BitcoinTestFramework.success`. _During Shutdown_ - `BitcoinTestFramework` logging handlers are flushed and removed. - `BitcoinTestFrameowork.nodes` list is cleared. - `NetworkThread.network_event_loop` is reset. (NetworkThread class). **Behavioural changes:** - Test parameters can now also be set when overriding BitcoinTestFramework.setup() in addition to overriding `set_test_params` method. - Potential exceptions raised in BitcoinTestFramework.setup() will be handled in main(). **Added files:** - ~~test_wrapper.py~~ `test_shell.py` - ~~test-wrapper.md~~ `test-shell.md` ACKs for top commit: jamesob: ACK https://github.com/bitcoin/bitcoin/pull/17288/commits/19139ee034d20ebab1b91d3ac13a8eee70b59374 jonatack: ACK 19139ee034d20ebab1b91d3ac13a8eee70b59374 jnewbery: Rather than invalidate the three ACKs for a minor nit, can you force push back to 19139ee034d20ebab1b91d3ac13a8eee70b59374 please? I think this PR was ready to merge before your last force push. jachiang: > Rather than invalidate the three ACKs for a minor nit, can you force push back to [19139ee](https://github.com/bitcoin/bitcoin/commit/19139ee034d20ebab1b91d3ac13a8eee70b59374) please? I think this PR was ready to merge before your last force push. jnewbery: ACK 19139ee034d20ebab1b91d3ac13a8eee70b59374 Tree-SHA512: 0c24f405f295a8580a9c8f1b9e0182b5d753eb08cc331424616dd50a062fb773d3719db4d08943365b1f42ccb965cc363b4bcc5beae27ac90b3460b349ed46b2