aboutsummaryrefslogtreecommitdiff
path: root/src/wallet
AgeCommit message (Collapse)Author
2019-11-06Replace CWalletTx::SetConf by Confirmation initialization listAntoine Riard
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-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-05Add m_last_block_processed_height field in CWalletAntoine Riard
At BlockConnected/BlockDisconnected, we rely on height of block itself to know current height of wallet
2019-11-05Pass block height in Chain::BlockConnected/Chain::BlockDisconnectedAntoine Riard
To do so we update CValidationInterface::BlockDisconnect to take a CBlockIndex pointing to the block being disconnected. This new parameter will be use in the following commit to establish wallet height.
2019-11-05Add missing SetupGeneration error handling in EncryptWalletRussell Yanofsky
Suggested https://github.com/bitcoin/bitcoin/pull/17304#discussion_r341286026 by me
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-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 #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-04wallet: LearnRelatedScripts only if KeepDestinationJoão Barbosa
2019-11-04wallet: Lock address type in ReserveDestinationJoão Barbosa
2019-11-04test: Add ASSERT_DEBUG_LOG to unit test frameworkMarcoFalke
2019-11-02wallet: Reuse existing batch in CWallet::SetUsedDestinationStateJoão Barbosa
2019-11-02wallet: Avoid recursive lock in CWallet::SetUsedDestinationStateJoão Barbosa
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-10-31Merge #17300: LegacyScriptPubKeyMan code cleanupsMarcoFalke
53fe0b70adeffe4cb94e6fa18a9abbdf674a2cd0 Fix missing strFailReason in CreateTransaction (Russell Yanofsky) 4b28a05f080de8acefaaa74f1204829995611d9e Fix misplaced AssertLockHeld (Russell Yanofsky) 2632b1f12466f970b667b42f462a6503df88e128 doc: Clarify WalletStorage / Wallet relation (Russell Yanofsky) 628d11b2ba0f1ba715c2358373e603a56d9f69ff Add back mistakenly removed AssertLockHeld (Russell Yanofsky) 52cf68f7ffa1dd3a6850606a75c32b77b1308c4b Refactor: Add GetLegacyScriptPubKeyMan helper (Russell Yanofsky) Pull request description: This PR implements suggested code cleanups from https://github.com/bitcoin/bitcoin/pull/17260 review comments ACKs for top commit: Sjors: ACK 53fe0b70adeffe4cb94e6fa18a9abbdf674a2cd0 achow101: ACK 53fe0b70adeffe4cb94e6fa18a9abbdf674a2cd0 MarcoFalke: ACK 53fe0b70adeffe4cb94e6fa18a9abbdf674a2cd0 Tree-SHA512: a577b96cb21a9aa7185d7d900e4db0665c302adcd12097957b9d8e838a8548c7de8f901bcb83e7c46d231b841221345c9264f5e29ed069f3d9236896430f959b
2019-10-30Merge #17316: refactor: Replace all uses of boost::optional with our own ↵fanquake
Optional type d314e8a818d4c162b1c7201533e6b600dcab2d91 refactor: Replace all uses of boost::optional with our own Optional type (Wladimir J. van der Laan) Pull request description: Replace all uses of boost::optional with our own Optional type. Luckily, there aren't so many. After this: - `boost::optional` is no longer used directly (only through `Optional` which is an alias for it) - `boost/optional.hpp` is only included in one place ACKs for top commit: MarcoFalke: ACK d314e8a818d4c162b1c7201533e6b600dcab2d91 practicalswift: ACK d314e8a818d4c162b1c7201533e6b600dcab2d91 -- diff looks correct + satisfying to see incremental progress towards the goal of a Boost free future :) jtimon: ACK d314e8a818d4c162b1c7201533e6b600dcab2d91 fanquake: ACK d314e8a818d4c162b1c7201533e6b600dcab2d91 Tree-SHA512: b43e0017af81b07b5851377cd09624f114510ac5b9018d037664b58ad0fc8e893e30946b61f8f5e21e39125925bf9998a81f2226b468aab2df653ee57ed3213d
2019-10-30replace asserts in RPC code with CHECK_NONFATAL and add linterAdam Jonas
2019-10-30refactor: Replace all uses of boost::optional with our own Optional typeWladimir J. van der Laan
After this: - `boost::optional` is no longer used directly (only through `Optional` which is an alias for it) - `boost/optional.hpp` is only included in one place
2019-10-30Merge #16839: Replace Connman and BanMan globals with NodeContext localWladimir J. van der Laan
362ded410b8cb1104b7ef31ff8488fec4824a7d5 Avoid using g_rpc_node global in wallet code (Russell Yanofsky) 8922d7f6b751a3e6b3b9f6fb7961c442877fb65a scripted-diff: Remove g_connman, g_banman globals (Russell Yanofsky) e6f4f895d5e42feaf7bfa5f41e80292aaa73cd7d Pass NodeContext, ConnMan, BanMan references more places (Russell Yanofsky) 4d5448c76b71c9d91399c31b043237091be2e5e7 MOVEONLY: Move NodeContext struct to node/context.h (Russell Yanofsky) 301bd41a2e6765b185bd55f4c541f9e27aeea29d scripted-diff: Rename InitInterfaces to NodeContext (Russell Yanofsky) Pull request description: This change is mainly a naming / organization change intended to simplify #10102. It: - Renames struct InitInterfaces to struct NodeContext and moves it from src/init.h to src/node/context.h. This is a cosmetic change intended to make the point of the struct more obvious. - Gets rid of BanMan and ConnMan globals making them NodeContext members instead. Getting rid of these globals has been talked about in past as a way to implement testing and simulations. Making them NodeContext members is a way of keeping them accessible without the globals. - Splits g_rpc_interfaces global into g_rpc_node and g_rpc_chain globals. This better separates node and wallet rpc methods. Node RPC methods should have access NodeContext, while wallet RPC methods should only have indirect access to node functionality via interfaces::Chain. - Adds NodeContext& references to interfaces::Chain class and the interfaces::MakeChain() function. This is needed to access ConnMan and BanMan instances without the globals. - Gets rid of redundant Node and Chain instances in Qt tests. This is needed due to the previous MakeChain change, and also makes test setup a little more straightforward. More cleanup could be done in the future, but it will require deduplication of bitcoind, bitcoin-qt, and TestingSetup init code. ACKs for top commit: laanwj: ACK 362ded410b8cb1104b7ef31ff8488fec4824a7d5 Tree-SHA512: 9ae6ff1e33423291d1e52056bac95e0874538390892a6e83c4c115b3c73155a8827c0191b46eb3d14e3b3f6c23ccb08095490880fbc3188026319c71739f7db2
2019-10-30Merge #17280: refactor: Change occurences of c_str() used with size() to data()Wladimir J. van der Laan
f3b51eb9352d7a7c5dfa15615efc8bc0a52ffecf Fix occurences of c_str() used with size() to data() (Wladimir J. van der Laan) Pull request description: Using `data()` better communicates the intent here. ~~Also, depending on how `c_str()` is implemented, this fixes undefined behavior: The part of the string after the first NULL character might have undefined contents (or even be inaccessible, worst case).~~ Apparently [this is no longer an issue with C++11](https://github.com/bitcoin/bitcoin/pull/17281#discussion_r339742128). ACKs for top commit: fjahr: Code review ACK f3b51eb practicalswift: ACK f3b51eb9352d7a7c5dfa15615efc8bc0a52ffecf -- diff looks correct, `data()` more idiomatic ryanofsky: Code review ACK f3b51eb9352d7a7c5dfa15615efc8bc0a52ffecf. Most of these calls (including one in crypter.cpp) are passing text strings, not binary strings likely to contain `\0` and were probably safe before, but much better to avoid the possibility of bugs like this. Tree-SHA512: 842e1bdd37efc4ece2ecb87ca34962aafef0a192180051def630607e349dc9c8b4e562481fff3de474515f493b4ee3ea53b00269a801a66e625326a38dfce5b8
2019-10-29Fix missing strFailReason in CreateTransactionRussell Yanofsky
Suggested by MarcoFalke <falke.marco@gmail.com> https://github.com/bitcoin/bitcoin/pull/17260#discussion_r340036269
2019-10-29Fix misplaced AssertLockHeldRussell Yanofsky
Suggestion from MarcoFalke <falke.marco@gmail.com> https://github.com/bitcoin/bitcoin/pull/17260#discussion_r340033021
2019-10-29doc: Clarify WalletStorage / Wallet relationRussell Yanofsky
Suggested by MarcoFalke <falke.marco@gmail.com> https://github.com/bitcoin/bitcoin/pull/17260#discussion_r340031507
2019-10-29Add back mistakenly removed AssertLockHeldRussell Yanofsky
Suggestion from MarcoFalke <falke.marco@gmail.com> https://github.com/bitcoin/bitcoin/pull/17260#discussion_r340029481
2019-10-29Refactor: Add GetLegacyScriptPubKeyMan helperRussell Yanofsky
Suggested by João Barbosa <joao.paulo.barbosa@gmail.com> https://github.com/bitcoin/bitcoin/pull/17260#discussion_r339505236
2019-10-29Merge #17260: Split some CWallet functions into new LegacyScriptPubKeyManMarcoFalke
f201ba59ffd2e071a36a688b80d2cff9a9c44bb2 Refactor: Split up CWallet and LegacyScriptPubKeyMan and classes (Andrew Chow) 6702048f91089d7a565e5ca5f7c8dcd2ca405a85 MOVEONLY: Move key handling code out of wallet to keyman file (Andrew Chow) ab053ec6d1e766402f88947d29cd875a285e7280 Move wallet enums to walletutil.h (Andrew Chow) Pull request description: Moves key management functions into a new class LegacyScriptPubKeyMan. First two commits are move-only commits which move stuff out of wallet.{h/cpp} and into newly created scriptpubkeyman.{h/cpp}. Third commit changes several things in CWallet to use LegacyScriptPubKeyMan. First step in the wallet boxes refactor. Note that LegacyScriptPubKeyMan and ScriptPubKeyMan cannot be used standalone yet and are still very much tied into CWallet with both accessing functions within each other. This PR is to help reduce review burden. ACKs for top commit: Sjors: Code review ACK f201ba5. promag: Code review ACK f201ba59ffd2e071a36a688b80d2cff9a9c44bb2. ryanofsky: Code review ACK f201ba59ffd2e071a36a688b80d2cff9a9c44bb2 MarcoFalke: ACK f201ba59ffd2e071a36a688b80d2cff9a9c44bb2 Tree-SHA512: bdc0d8595a06233fe003afcf968a38e0e8cc584a6a89c5bcd05309ac29dca852391802d46763ef81a108d146d0f40c79ea5438e87234ed12b4b8360c9aec94c0
2019-10-28Merge #17279: refactor: Remove redundant c_str() calls in formattingMarcoFalke
c72906dcc11a73fa06a0adf97557fa756b551bee refactor: Remove redundant c_str() calls in formatting (Wladimir J. van der Laan) Pull request description: Our formatter, tinyformat, *never* needs `c_str()` for strings. Still, many places call it redundantly, resulting in longer code and a slight overhead. Remove redundant `c_str()` calls for: - `strprintf` - `LogPrintf` - `tfm::format` (also, combined with #17095, I think this improves logging in case of unexpected embedded NULL characters) ACKs for top commit: ryanofsky: Code review ACK c72906dcc11a73fa06a0adf97557fa756b551bee. Easy to review with `git log -p -n1 --word-diff-regex=. -U0 c72906dcc11a73fa06a0adf97557fa756b551bee` Tree-SHA512: 9e21e7bed8aaff59b8b8aa11571396ddc265fb29608c2545b1fcdbbb36d65b37eb361db6688dd36035eab0c110f8de255375cfda50df3d9d7708bc092f67fefc
2019-10-28Avoid using g_rpc_node global in wallet codeRussell Yanofsky
Wallet code should use interfaces::Chain and not directly access to node state. Add a g_rpc_chain replacement global for wallet code to use, and move g_rpc_node definition to a libbitcoin_server source file so there are link errors if wallet code tries to access it.