aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
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-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-05Add EnsureLegacyScriptPubKeyMan and use in rpcwallet.cppRussell Yanofsky
This also fixes unused variable warnings in rpcdump.cpp
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-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-04cli: fix -getinfo output when compiled with no walletfanquake
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-04logs: add timing information to FlushStateToDisk()James O'Beirne
2019-11-04logs: add BCLog::Timer and related macrosJames O'Beirne
Makes logging timing information about a block of code easier.
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 #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-04wallet: LearnRelatedScripts only if KeepDestinationJoão Barbosa
2019-11-04wallet: Lock address type in ReserveDestinationJoão Barbosa
2019-11-04test: Reset global args between test suitesMarcoFalke
2019-11-04test: Add ASSERT_DEBUG_LOG to unit test frameworkMarcoFalke
2019-11-04logging: Add member for arbitrary print callbacksMarcoFalke
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-04test: add "diamond" unit test to MempoolAncestryTestsSebastian Falbesoner
Approaches #17271. If ancestors are represented more than once, check that those are not overcounted.
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 #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 #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-02wallet: Reuse existing batch in CWallet::SetUsedDestinationStateJoão Barbosa
2019-11-02wallet: Avoid recursive lock in CWallet::SetUsedDestinationStateJoão Barbosa
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
generated by cgit v1.2.3 (git 2.26.2) at 2025-03-18 03:17:46 +0000