aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2019-11-18Refactor: Require scriptPubKey to get wallet SigningProviderAndrew Chow
Easier to review ignoring whitespace: git log -p -n1 -w This commit does not change behavior. It passes new CScript arguments to signing functions, but the arguments aren't currently used.
2019-11-18Accumulate result UniValue in SignTransactionAndrew Chow
SignTransaction will be called multiple times in the future. Pass it a result UniValue so that it can accumulate the results of multiple SignTransaction passes.
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
2019-11-04Merge #17366: test: Reset global args between test suitesMarcoFalke
fa07b8beb598642655b1207afd275b801ff8cec2 test: Reset global args between test suites (MarcoFalke) Pull request description: Ideally there wouldn't be any globals in Bitcoin Core. However, as we still have globals, they need to be reset between runs of test cases. One way to do this is to run each suite in a different process. `make check` does that. However, `./src/test/test_bitcoin` when run manually or on appveyor is a single process, where all globals are preserved between test cases. This leads to hard to debug issues such as https://github.com/bitcoin/bitcoin/pull/15845#pullrequestreview-310852164. Fix that by resetting the global arg for each test suite. Note that this wont reset the arg between test cases, as the constructor/destructor is not called for them. Addendum: This is not a general fix, only for `-segwitheight`. I don't know if clearing all args can be done with today's argsmanager. Nor do I know if it makes sense. Maybe we want datadir set to a temp path to not risk accidentally corrupting the default data dir? ACKs for top commit: laanwj: ACK fa07b8beb598642655b1207afd275b801ff8cec2 practicalswift: ACK fa07b8beb598642655b1207afd275b801ff8cec2 mzumsande: ACK fa07b8beb598642655b1207afd275b801ff8cec2, I also tested that this fixes the issue in #15845. Tree-SHA512: 1e30b06f0d2829144a61cc1bc9bdd6a694cbd911afff83dd3ad2a3f15b577fd30acdf9f1469f8cb724d0642ad5d297364fd5a8a2a9c8619a7a71fa9ae2837cdc
2019-11-04Merge #17318: replace asserts in RPC code with CHECK_NONFATAL and add linterMarcoFalke
c98bd13e675fbf5641ed64d551b63aaf55a1a8e9 replace asserts in RPC code with CHECK_NONFATAL and add linter (Adam Jonas) Pull request description: - Replace instances of assert in /rpc files and rpcwallet with CHECK_NONFATAL(condition) - Add a linter to prevent future usage of assert being used in RPC code ref https://github.com/bitcoin/bitcoin/pull/17192 ACKs for top commit: practicalswift: ACK c98bd13e675fbf5641ed64d551b63aaf55a1a8e9 -- diff looks correct Tree-SHA512: a16036b6bbcca73a5334665f66e17e1756377d582317568291da1d727fc9cf8c84bac9d9bd099534e1be315345336e5f7b66b93793135155f320dc5862a2d875
2019-11-04Merge #17199: test: use default address type (bech32) for wallet_bumpfee testsMarcoFalke
8d8e5a79d09d9027e5b68fecfc713e7b135320ba test: use default address type (bech32) for wallet_bumpfee tests (Sebastian Falbesoner) Pull request description: The use of native segwit addresses (pure p2wpkh instead of p2sh-p2wpkh) leads to smaller transaction sizes, needing adaption of some constants in the following test cases: - `test_dust_to_fee()`: adaption of dust calculation (p2wpkh spend estimate of 67 is taken from `src/policy/policy.cpp:GetDustThreshold()`) - `test_maxtxfee_fails()`: lowering `-maxtxfee` setting to trigger fail Top commit has no ACKs. Tree-SHA512: b4163700d56c11955f811bc5fe6edaf7aec69931d7db741c03b055fb518bb9825c031fb931c513b37a1968085cb8c2f263adf664b357aff8ee42795fd0f88d2d
2019-11-04Merge #17164: p2p: Avoid allocating memory for addrKnown where we don't need itMarcoFalke
b6d2183858975abc961207c125c15791e531edcc Minor refactoring to remove implied m_addr_relay_peer. (User) a552e8477c5bcd22a5457f4f73a2fd6db8acd2c2 added asserts to check m_addr_known when it's used (User) 090b75c14be6b9ba2efe38a17d141c6e6af575cb p2p: Avoid allocating memory for addrKnown where we don't need it (User) Pull request description: We should allocate memory for addrKnown filter only for those peers which are expected to participate in address relay. Currently, we do it for all peers (including SPV and block-relay-only), which results in extra RAM where it's not needed. Upd: In future, we would still allow SPVs to ask for addrs, so allocation still will be done by default. However, they will be able to opt-out via [this proposal](https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2019-October/017428.html) and then we could save some more memory. This PR still saves memory for block-relay-only peers immediately after merging. Top commit has no ACKs. Tree-SHA512: e84d93b2615556d466f5ca0e543580fde763911a3bfea3127c493ddfaba8f05c8605cb94ff795d165af542b594400995a2c51338185c298581408687e7812463
2019-11-04test: Reset global args between test suitesMarcoFalke
2019-11-04Add documentation for test_shell submoduleJamesC
2019-11-04Merge #17304: refactor: Move many functions into LegacyScriptPubKeyMan and ↵Wladimir J. van der Laan
further separate it from CWallet 152b0a00d8e681dd098f6b548447b82ab54ebe3c Refactor: Move nTimeFirstKey accesses out of CWallet (Andrew Chow) 7ef47b88e67718766c92d23973742d08436176e0 Refactor: Move GetKeypoolSize code out of CWallet (Andrew Chow) 089e17d45c8147bd0303bcbf02dc0f7d6b387f2a Refactor: Move RewriteDB code out of CWallet (Andrew Chow) 0eac7088ab878372a72f85f9c5f7662a14199083 Refactor: Move SetupGeneration code out of CWallet (Andrew Chow) f45d12b36cee05aa3c2685b951d27bd8a58539ae Refactor: Move HavePrivateKeys code out of CWallet::CreateWalletFromFile (Andrew Chow) 8b0d82bb428de9e7f1da7c61574e7a8376a62d43 Refactor: Move Upgrade code out of CWallet::CreateWalletFromFile (Andrew Chow) 46865ec958b6b9bde04a827de598975f14bdb5e7 Refactor: Move MarkUnusedAddresses code out of CWallet::AddToWalletIfInvolvingMe (Andrew Chow) a18edd7b383d667b15b6d4b87aa3a055a9fa5051 Refactor: Move GetMetadata code out of getaddressinfo (Andrew Chow) 9716bbe0f8081814cbf052e8211d1c6a838e5262 Refactor: Move LoadKey LegacyScriptPubKeyMan method definition (Andrew Chow) 67be6b9e213da1fd7b2e8438c445c5a64092e831 Refactor: Move SetAddressBookWithDB call out of LegacyScriptPubKeyMan::ImportScriptPubKeys (Andrew Chow) fc2867fdf50f47e5ad5f43445e500e3a477a8074 refactor: Replace UnsetWalletFlagWithDB with UnsetBlankWalletFlag in ScriptPubKeyMan (Andrew Chow) 78e7cbc7ba5938ab2ae6347141ca793a8fc09853 Refactor: Remove UnsetWalletFlag call from LegacyScriptPubKeyMan::SetHDSeed (Andrew Chow) 0391aba52dcc33613295fd6099c804ac7134b063 Remove SetWalletFlag from WalletStorage (Andrew Chow) 4c5491f99c6b8ca779c48be63eb2ba18d98ee52a Refactor: Move SetWalletFlag out of LegacyScriptPubKeyMan::UpgradeKeyMetadata (Andrew Chow) 769acef85730a6c0e2f5603fbd673b176c2d34d0 Refactor: Move SetAddressBook call out of LegacyScriptPubKeyMan::GetNewDestination (Andrew Chow) acedc5b8230ed9ad07f96f51f0ef862ab3a41d5e Refactor: Add new ScriptPubKeyMan virtual methods (Andrew Chow) 533d8b364f4e589aa1acb28cdea5e6e6e80d34dc Refactor: Declare LegacyScriptPubKeyMan methods as virtual (Andrew Chow) b4cb18bce3c9a6adab2fa32f3dad4ad966b33be0 MOVEONLY: Reorder LegacyScriptPubKeyMan methods (Andrew Chow) Pull request description: Moves several more key management and metadata functions into LegacyScriptPubKeyMan from CWallet to further separate the two. Note to reviewers: All of the `if (auto spk_man = walletInstance->m_spk_man.get()) {` blocks will be replaced with for loops in the next PR so you may see some things in those blocks that don't necessarily make sense with an `if` but will with a `for`. ACKs for top commit: laanwj: code review ACK 152b0a00d8e681dd098f6b548447b82ab54ebe3c Sjors: re-ACK 152b0a00d8e681dd098f6b548447b82ab54ebe3c promag: Code review ACK 152b0a00d8e681dd098f6b548447b82ab54ebe3c. Tree-SHA512: ff9872a3ef818922166cb15d72363004ec184e1015a3928a66091bddf48995423602ccd7e55b814de85d25ad7c69058280b1fde2e633570c680dc7d6084b3122
2019-11-04Merge #17349: Remove redundant copy constructorsMarcoFalke
fa8919889f3c1bd3e2700ecbb56493e3cd1e25ad bench: Remove redundant copy constructor in mempool_stress (MarcoFalke) 29f84343681831baf02a17d3af566c5c57ecf3c2 refactor: Remove redundant PSBT copy constructor (Hennadii Stepanov) Pull request description: I fail to see why people add these copy constructors manually without explanation, when the compiler can generate them at least as good automatically with less code. ACKs for top commit: promag: ACK fa8919889f3c1bd3e2700ecbb56493e3cd1e25ad. hebasto: ACK fa8919889f3c1bd3e2700ecbb56493e3cd1e25ad, nit s/constructor/operator/ in commit fa8919889f3c1bd3e2700ecbb56493e3cd1e25ad message, as @promag [mentioned](https://github.com/bitcoin/bitcoin/pull/17349#discussion_r341776389) above. jonatack: ACK fa8919889f3c1bd3e2700ecbb56493e3cd1e25ad Tree-SHA512: ce024fdb894328f41037420b881169b8b1b48c87fbae5f432edf371a35c82e77e21468ef97cda6f54d34f1cf9bb010235d62904bb0669793457ed1c3b2a89723
2019-11-04Merge #17233: travis: Run unit and functional tests on native armMarcoFalke
facc0da63a8fa4bd6fc2782cbe92eb9f920f2256 travis: Run unit and functional tests on native arm (MarcoFalke) fafa064d2a8dbe24303545ab582ec84cde52ab5b ci: Remove ccache requirement on the host (MarcoFalke) Pull request description: This keeps the cross-compilation to make it easy to run the ci on non-arm hardware. To run this locally in qemu-user as it used to be, just `export QEMU_USER_CMD="qemu-arm -L /usr/arm-linux-gnueabihf/"`. ACKs for top commit: laanwj: LGTM ACK facc0da63a8fa4bd6fc2782cbe92eb9f920f2256 practicalswift: ACK facc0da63a8fa4bd6fc2782cbe92eb9f920f2256 -- diff looks correct and Travis seems happy Tree-SHA512: 0dc1bc82eb93e2bd8b159e044f20fe3055f8cdfd73aaa238bd2e178397582144dfc0c6a87bd8270115dafea1a623e642bde5d5f30254f94140f1a2cdb12fc2da
2019-11-04Merge #17228: test: Add RegTestingSetup to setup_commonMarcoFalke
fa0a731d007a8e76d2740a2e6ead2289de77e475 test: Add RegTestingSetup to setup_common (MarcoFalke) fa54b3e2485f701aa420c37f09a2859a5b805161 test: move-only ComputeFilter to src/test/lib/blockfilter (MarcoFalke) Pull request description: The default chain for `TestingSetup` is the main chain. However, any test that wants to mine blocks on demand needs to switch to regtest. This is done manually and in-line right now. Fix that by creating an explicit `RegTestingSetup` and use it where appropriate. Also, add a move-only commit to move `ComputeFilter` into the newly created unit test library. Both commits are part of #15845, but split up because they are useful on their own. ACKs for top commit: practicalswift: ACK fa0a731d007a8e76d2740a2e6ead2289de77e475 -- diff looks correct Tree-SHA512: 02b9765580b355ed8d1be555f8ae11fa6e3d575f5cb177bbdda0319378837e29de5555c126c477dc8a1e8a5be47335afdcff152cf2dea2fbdd1a988ddde3689b
2019-11-04Merge #17351: doc: Fix some misspellingsMarcoFalke
ac831339cbfa65b1f7576c53b5d9a94841db9868 doc: Fix some misspellings (randymcmillan) Pull request description: Here is a more thorough lint-spelling update. This PR takes care of easy to fix spelling errors to clean up the linting stages. There are misspellings coded into the functional tests. That is a whole separate job within itself. ACKs for top commit: practicalswift: ACK ac831339cbfa65b1f7576c53b5d9a94841db9868 -- diff looks correct Tree-SHA512: d8fad83fed083715655f148263ddeffc6752c8007d568fcf3dc2c418ccd5db70089ce3ccfd3994fcbd78043171402eb9cca5bdd5125287e22c42ea305aaa6e9d
2019-11-04Merge #16110: depends: Add Android NDK supportWladimir J. van der Laan
f9af3ced1c69d65c5c530ec5526f5eefaf786126 Android: add all arch support (Block Mechanic) d419ca7e32bfc71e8dd1f1b91870463eacd6ad8e depends: export dynamic JNI symbols from static qtforandroid.a (Igor Cota) ed30684d03d3a1d5496e69c488d848fe92ae6fb4 Qt: patch androidjnimain.cpp to make sure JNI is initialised when statically compiled (Igor Cota) e4c319e8a1c6e40a953036b942bd5d732b0bbf69 builds: remove superfluous config_opts_aarch64_android (Igor Cota) 24ffef0c271739a2ca75feecb816f3218c1850bf Patch libevent when building for Android (fix arc4random_addrandom) (Igor Cota) f1e40b3e7114b18bc03f45c3a97f9f673543ddef Update bitcoin_qt.m4 (BlockMechanic) b4057d82618a21720f39f448b689cebf475cc2dc Define TARGET_OS when host is android (Igor Cota) 80b475f159525737e242161397f35d0729449545 Fix Android zlib cross compilation issue (https://stackoverflow.com/questions/21396988/zlib-build-not-configuring-properly-with-cross-compiler-ignores-ar) (Igor Cota) 45f82190150b3feef333724ea7395ba080e700b1 Add full Android build example command and instructions on getting SDK/NDK (Igor Cota) b68f2a68c211aa2264e9ca824d10a90f4a5a5af4 Add config opts and patch for aarch64_android build of Qt (Igor Cota) 9c4cb0166e801471f8cb3d82656c86bacf051db6 Add ranlib to android.mk hosts file (fix OSX Android NDK build) (Igor Cota) c2a749c9c16697e744ecfb283fdf4095d0278066 Add example Android host-platform-triplet and options (Igor Cota) 0b0cff3c61610fb56f8c5c9451ace01598117a8d Add support for building Android dependencies (Igor Cota) Pull request description: This allows one to build the dependencies with the Android SDK and goes towards fixing #11844. It has been tested to work with: `make HOST=aarch64-linux-android ANDROID_API_LEVEL=28 ANDROID_TOOLCHAIN_BIN=/home/user/Android/Sdk/ndk-bundle/toolchains/llvm/prebuilt/linux-x86_64/bin NO_QT=1 NO_WALLET=1` ACKs for top commit: Sjors: ACK f9af3ce. I'm OK with merging and then improving later. Tree-SHA512: cb805115ebe5c9e33db2bf3eab8628808fe3f50052053d8877d8b8e4406d6fea1ed9e5c4dff85d777fb99c81be6ffb9d95a0e6d32344e728e5e0da6c653e2ce7
2019-11-04Merge #17297: refactor: Remove addrdb.h dependency from node.hWladimir J. van der Laan
f44abe4bed25a40145ab168adc1589f5df4146f3 refactor: Remove addrdb.h dependency from node.h (Hennadii Stepanov) Pull request description: `node.h` includes `addrdb.h` just for the sake of `banmap_t` type. This PR makes dependencies simpler and explicit. ~Also needless `typedef` has been removed from `enum BanReason`.~ ACKs for top commit: laanwj: ACK f44abe4bed25a40145ab168adc1589f5df4146f3 practicalswift: ACK f44abe4bed25a40145ab168adc1589f5df4146f3 Tree-SHA512: 33a1be20e5c629daf4a61ebbf93ea6494b9256887cebd4974de4782f6d324404b6cc84909533d9502b2cc19902083f1f9307d4fb7231e67db5b412b842d13072
2019-11-04doc: Fix some misspellingsrandymcmillan
2019-11-04Add TestShell classJames Chiang
A BitcoinTestFramework child class which can be imported by an external user or project. TestShell.setup() initiates an underlying BitcoinTestFramework object with bitcoind subprocesses, rpc interfaces and test logging. TestShell.shutdown() safely tears down the BitcoinTestFramework object.
2019-11-03Move argparse() to init()JamesC
This ensures TestFramework default parameters are set before setup is called. A child class will therefore have access to defaults when overriding setup.
2019-11-03Move assert num_nodes is set into main()JamesC
This allows a BitcoinTestFramework child class to set test parameters in an overridden setup() rather than in an overridden set_test_params().
2019-11-03Clear TestNode objects after shutdownJamesC
TestNode objects need to be removed during shutdown, as setup_nodes does not remove previous TestNode objects from previous test runs during setup.
2019-11-03Add closing and flushing of logging handlersJamesC
In order for BitcoinTestFramework to correctly restart after shutdown, the previous logging handlers need to be removed, or else logging will continue in the previous temp directory. "Flush" ensures buffers are emptied, and "close" ensures file handler close logging file.
2019-11-03Refactor TestFramework main() into setup/shutdownJamesC
Setup and shutdown code now moved into dedicated methods. Test "success" is added as a BitcoinTestFramework member, which can be accessed outside of main. Argument parsing also moved into separate method and called from main.
2019-11-03Remove network_event_loop instance in close()JamesC
The asyncio.new_event_loop() instance is now removed from the NetworkThread class during shutdown. This enables a NetworkThread instance to be restarted after being closed. The current NetworkThread class guards against an existing new_event_loop during initialization.
2019-11-02Merge #17285: doc: Bip70 removal follow-upWladimir J. van der Laan
3ed8e3d079a3860dcdf944f7c1aa37765a53da32 doc: Remove explicit network name references (Fabian Jahr) d6e493f0c2850b522a676a005935163beddaa2cc wallet: Remove left-over BIP70 comment (Fabian Jahr) Pull request description: A small follow-up to #17165 which removed BIP70 support. 1. Removes one leftover mention of BIP70 in a comment. 2. Removes BIP70 reference in comments on network/chain name strings. These can be removed as they are not really helpful and also incorrect: BIP70 only defines "main" and "test" but not "regtest". If/When signet gets merged we will add another name to the list that is not defined in BIP70. Mostly there is also an exhaustive list of the options included in the comment anyway. If we would like to keep an identifier for this naming scheme, I would suggest switching to something more generic, like 'short chain name'. Happy to implement that if that is preferred. Alternatively, we could add a reference to `CBaseChainParams`. That would also mean we don't have to change these lines again for signet. ACKs for top commit: MarcoFalke: ACK 3ed8e3d079a3860dcdf944f7c1aa37765a53da32 Tree-SHA512: 9a7c0b9cacbb67bd31a089ffdc6f1ebc7f336493e2c8266eb697da34dce2b505a431d5639a3e4fc34f9287361343e861b55dc2662e0a1d2095cc1046db77d6ee
2019-11-02Merge #17293: Add assertion to randrange that input is not 0Wladimir J. van der Laan
a35b6824f3a0bdb68c5aef599c0f17562689970e Add assertion to randrange that input is not 0 (Jeremy Rubin) Pull request description: From the comment in randrange, their is an implicit argument that randrange cannot accept an argument of 0. If the argument is 0, then we have to return {}, which is not possible in a uint64_t. The current code takes a very interesting approach, which is to return [0..std::numeric_limits<uint64_t>]. This can cause all sorts of fun problems, like allocating a lot of memory, accessing random memory (maybe with your private keys), and crashing the computer entirely. This gives us three choices of how to make it "safe": 1) return Optional<uint64_t> 2) Change the return type to [0..range] 3) Return 0 if 0 4) Assert(range) So which solution is best? 1) seems a bit overkill, as it makes any code using randrange worse. 2) Changing the return type as in 2 could be acceptable, but it imposes the potential overflow checking on the caller (which is what we want). 3) An interesting option -- effective makes the return type in {0} U [0..range]. But this is a bad choice, because it leads to code like `vec[randrange(vec.size())]`, which is incorrect for an empty vector. Null set should mean null set. 4) Assert(range) stands out as the best mitigation for now, with perhaps a future change to solution 2. It prevents the error from propagating at the earliest possible time, so the program crashes cleanly rather than by freezing the computer or accessing random memory. ACKs for top commit: instagibbs: Seems reasonable for now, ACK https://github.com/bitcoin/bitcoin/pull/17293/commits/a35b6824f3a0bdb68c5aef599c0f17562689970e laanwj: ACK a35b6824f3a0bdb68c5aef599c0f17562689970e promag: ACK a35b6824f3a0bdb68c5aef599c0f17562689970e. Tree-SHA512: 8fc626cde4b04b918100cb7af28753f25ec697bd077ce0e0c640be0357626322aeea233e3c8fd964ba1564b0fda830b7f5188310ebbb119c113513a4b89952dc
2019-11-01Refactor: Move nTimeFirstKey accesses out of CWalletAndrew Chow
This commit does not change behavior.
2019-11-01Refactor: Move GetKeypoolSize code out of CWalletAndrew Chow
This commit does not change behavior.
2019-11-01Refactor: Move RewriteDB code out of CWalletAndrew Chow
This commit does not change behavior.
2019-11-01Refactor: Move SetupGeneration code out of CWalletAndrew Chow
This commit does not change behavior.
2019-11-01Refactor: Move HavePrivateKeys code out of CWallet::CreateWalletFromFileAndrew Chow
This commit does not change behavior.
2019-11-01Refactor: Move Upgrade code out of CWallet::CreateWalletFromFileAndrew Chow
This commit does not change behavior.
2019-11-01Refactor: Move MarkUnusedAddresses code out of CWallet::AddToWalletIfInvolvingMeAndrew Chow
This commit does not change behavior.
2019-11-01Refactor: Move GetMetadata code out of getaddressinfoAndrew Chow
Easier to review ignoring whitespace: git log -p -n1 -w This commit does not change behavior.
2019-11-01Refactor: Move LoadKey LegacyScriptPubKeyMan method definitionAndrew Chow
This commit does not change behavior.
2019-11-01Refactor: Move SetAddressBookWithDB call out of ↵Andrew Chow
LegacyScriptPubKeyMan::ImportScriptPubKeys This commit does not change behavior.
2019-11-01refactor: Replace UnsetWalletFlagWithDB with UnsetBlankWalletFlag in ↵Andrew Chow
ScriptPubKeyMan ScriptPubKeyMan is only using UnsetWalletFlagWithDB to unset the blank wallet flag. Just make that it's own function and not expose the flag writing directly. This does not change behavior.
2019-11-01Refactor: Remove UnsetWalletFlag call from LegacyScriptPubKeyMan::SetHDSeedAndrew Chow
This commit does not change behavior.
2019-11-01Remove SetWalletFlag from WalletStorageAndrew Chow
SetWalletFlag is unused. Does not change any behavior
2019-11-01Refactor: Move SetWalletFlag out of LegacyScriptPubKeyMan::UpgradeKeyMetadataAndrew Chow
This commit does not change behavior.
2019-11-01Refactor: Move SetAddressBook call out of ↵Andrew Chow
LegacyScriptPubKeyMan::GetNewDestination This commit does not change behavior.
2019-11-01Refactor: Add new ScriptPubKeyMan virtual methodsAndrew Chow
This commit does not change behavior.
2019-11-01Refactor: Declare LegacyScriptPubKeyMan methods as virtualAndrew Chow
This commit does not change behavior.
2019-11-01MOVEONLY: Reorder LegacyScriptPubKeyMan methodsAndrew Chow
Can verify move-only with: git log -p -n1 --color-moved This commit is move-only and doesn't change code or affect behavior.
2019-11-01bench: Remove redundant copy constructor in mempool_stressMarcoFalke
2019-11-01refactor: Remove redundant PSBT copy constructorHennadii Stepanov
The default (i.e., generated by a compiler) copy constructor does the same things. Also this prevents -Wdeprecated-copy warning for implicitly declared operator= in GCC 9.
2019-11-01Merge #17292: Add new mempool benchmarks for a complex poolMarcoFalke
b0c774b48a3198584e61429ef053844bdde2f9ae Add new mempool benchmarks for a complex pool (Jeremy Rubin) Pull request description: This PR is related to #17268. It adds a mempool stress test which makes a really big complicated tx graph, and then, similar to mempool_eviction test, trims the size. The test setup is to make 100 original transactions with Rand(10)+2 outputs each. Then, 800 times: we create a new transaction with Rand(10) + 1 parents that are randomly sampled from all existing transactions (with unspent outputs). From each such parent, we then select Rand(remaining outputs) +1 50% of the time, or 1 outputs 50% of the time. Then, we trim the size to 3/4. Then we trim it to just a single transaction. This creates, hopefully, a big bundle of transactions with lots of complex structure, that should really put a strain on the mempool graph algorithms. This ends up testing both the descendant and ancestor tracking. I don't love that the test is "unstable". That is, in order to compare this test to another, you really can't modify any of the internal state because it will have a different order of invocations of the deterministic randomness. However, it certainly suffices for comparing branches. Top commit has no ACKs. Tree-SHA512: cabe96b849b9885878e20eec558915e921d49e6ed1e4b011b22ca191b4c99aa28930a8b963784c9adf78cc8b034a655513f7a0da865e280a1214ae15ebb1d574
2019-11-01Merge #17254: test: fix script_p2sh_tests OP_PUSHBACK2/4 missingMarcoFalke
5710dadf9b282524fddf42c682351cd8022ed7bf test: fix script_p2sh_tests OP_PUSHBACK2/4 missing (kodslav) Pull request description: Cleans up #15140 which fixes commit 6b25f29a91 where opcodes were lost in translation. ACKs for top commit: laanwj: code review ACK 5710dadf9b282524fddf42c682351cd8022ed7bf Tree-SHA512: 3f7fbcaf0dd199626d9ec9fdf3c5b5c5c2a91c4cfe81fae5b1d5662a48e52cf4bd27c94f8f42ebdfe7a076c5d600ada5661a6902b03eb5dc3dc953f4524345ac
2019-11-01Merge #17309: doc: update MSVC instructions to remove Qt OpenSSL linkingMarcoFalke
162d0038e772bf6210b9218df35c14fd9719d3f7 doc: compiling with Visual Studio is now supported on Windows (fanquake) b1f1fb5f1dbdeb329c8c764f92a891329dc10863 doc: update MSVC instructions to remove Qt configuration (fanquake) Pull request description: Follow up from #17165. Flips `-openssl-linked` to `-no-openssl`. Also adds some missing packages to the vcpkg install instructions. ACKs for top commit: sipsorcery: tACK 162d0038e772bf6210b9218df35c14fd9719d3f7. Tree-SHA512: 40577a3759a30170a14fd27e3eeac5a78a7852ae88daacecf584ad46c685708c167153d39357aa77a4f65bfd5a349f7589f20aa16fdf3d2895b4076b381e2c9c
2019-11-01Merge #17345: test: Do not instantiate CAddrDB for static call CAddrDB::Read()fanquake
59853c33f1c8781480549effc39baee60ffd837c test: Do not instantiate CAddrDB for static call (Hennadii Stepanov) Pull request description: Ref: https://github.com/bitcoin/bitcoin/blob/6a7c40bee403cadedeecd4b1c6528575522094eb/src/addrdb.h#L84-L94 ACKs for top commit: MarcoFalke: unsigned ACK 59853c33f1c8781480549effc39baee60ffd837c naumenkogs: ACK 59853c3 Tree-SHA512: 06b2e8e4f2b4a4f20454a4828a786878fc2bd441b0ee53f1128a7a0b6ad54dd3ca7598cee000ec60e5e4ef02570a09c01974d2d8260bd11fa9baf16b3ff9ba08