aboutsummaryrefslogtreecommitdiff
path: root/src/wallet/wallet.h
AgeCommit message (Collapse)Author
2019-12-06Remove SetCrypted() and fUseCrypto; Change IsCrypted()'s implementationAndrew Chow
Removes SetCrypted() and fUseCrypto as we don't need them anymore. SetCrypted calls in LegacyScriptPubKeyMan are replaced with mapKeys.empty() IsCrypted() is changed to just call HasEncryptionKeys()
2019-12-06Rename EncryptKeys to Encrypt and pass in the encrypted batch to useAndrew Chow
2019-12-06Move fDecryptionThoroughlyChecked from CWallet to LegacyScriptPubKeyManAndrew Chow
2019-12-06Add GetEncryptionKey() and HasEncryptionKeys() to WalletStorageAndrew Chow
Adds functions in WalletStorage that allow ScriptPubKeyMans to check and get encryption keys from the wallet.
2019-12-06Merge #17373: wallet: Various fixes and cleanup to keypool handling in ↵fanquake
LegacyScriptPubKeyMan and CWallet 886f1731bec4393dd342403ac34069a3a4f95eea Key pool: Fix omitted pre-split count in GetKeyPoolSize (Andrew Chow) 386a994b853bc5b3a2ed0d812673465b8ffa4849 Key pool: Change ReturnDestination interface to take address instead of key (Andrew Chow) ba41aa4969169cd73d6b4f57444ed7d8d875de10 Key pool: Move LearnRelated and GetDestination calls (Andrew Chow) 65833a74076cddf986037c6eb3b29a9b9dbe31c5 Add OutputType and CPubKey parameters to KeepDestination (Andrew Chow) 9fcf8ce7ae02bf170b9bf0c2887fd709d752cbf7 Rename Keep/ReturnKey to Keep/ReturnDestination and remove the wrapper (Andrew Chow) 596f6460f9fd8273665c8754ccd673d93a4f25f0 Key pool: Move CanGetAddresses call (Andrew Chow) Pull request description: * The `pwallet->CanGetAddresses()` call in `ReserveDestination::GetReservedDestination` to `LegacyScriptPubKeyMan::GetReservedDestination` so that the sanity check results in a failure when a `ScriptPubKeyMan` individually cannot get a destination, not when any of the `ScriptPubKeyMan`s can't. * `ScriptPubKeyMan::GetReservedDestination` is changed to return the destination so that future `ScriptPubKeyMan`s can return destinations constructed in other ways. This is implemented for `LegacyScriptPubKeyMan` by moving key-to-destination code from `CWallet` to `LegacyScriptPubKeyMan` * In order for `ScriptPubKeyMan` to be generic and work with future `ScriptPubKeyMan`s, `ScriptPubKeyMan::ReturnDestination` is changed to take a `CTxDestination` instead of a `CPubKey`. Since `LegacyScriptPubKeyMan` still deals with keys internally, a new map `m_reserved_key_to_index` is added in order to track the keypool indexes that have been reserved. * A bug is fixed in how the total keypool size is calculated as it was omitting `set_pre_split_keypool` which is a bug. Split from #17261 ACKs for top commit: ryanofsky: Code review ACK 886f1731bec4393dd342403ac34069a3a4f95eea. Only change is moving earlier fix to a better commit (same end result). promag: Code review ACK 886f1731bec4393dd342403ac34069a3a4f95eea. instagibbs: code review re-ACK https://github.com/bitcoin/bitcoin/pull/17373/commits/886f1731bec4393dd342403ac34069a3a4f95eea Sjors: Code review re-ACK 886f1731bec4393dd342403ac34069a3a4f95eea Tree-SHA512: f4be290759f63fdc920d5c02bd0d09acc4b06a5f053787d4afcd3c921b2e35d2bd97617fadae015da853dc189f559fb8d2c6e58d53e4cabfac9af151cd97ad19
2019-12-02Key pool: Change ReturnDestination interface to take address instead of keyAndrew Chow
In order for ScriptPubKeyMan to be generic and work with future ScriptPubKeyMans, ScriptPubKeyMan::ReturnDestination is changed to take a CTxDestination instead of a CPubKey. Since LegacyScriptPubKeyMan still deals with keys internally, a new map m_reserved_key_to_index is added in order to track the keypool indexes that have been reserved. The CPubKey argument of KeepDestination is also removed so that it is more generic. Instead of taking a CPubKey or a CTxDestination, we just use the nIndex given to find the pubkey.
2019-12-02Key pool: Move LearnRelated and GetDestination callsAndrew Chow
Addresses are determined by LegacyScriptPubKeyMan::GetReservedDestination instead of ReserveDestination::GetReservedDestination as other ScriptPubKeyMan implementations may construct addresses differently This does not change behavior.
2019-11-23refactor: Nuke coincontrol circular dependencyHennadii Stepanov
2019-11-23Merge #17237: wallet: LearnRelatedScripts only if KeepDestinationSamuel Dobson
3958295bc8a3787b66b0269190218a3764088f79 wallet: LearnRelatedScripts only if KeepDestination (João Barbosa) 55295fba4cbff36e9a8c3fed9c38e82ebe3c48b7 wallet: Lock address type in ReserveDestination (João Barbosa) Pull request description: Only mutates the wallet if the reserved key is kept. First commit is a refactor that makes the address type a class member. The second commit moves `LearnRelatedScripts` from `GetReservedDestination` to `KeepDestination` to avoid an unnecessary call to `AddCScript` - which in turn prevents multiple entries of the same script in the wallet DB. ACKs for top commit: achow101: Re-ACK 3958295bc8a3787b66b0269190218a3764088f79 Sjors: ACK 3958295bc8a3787b66b0269190218a3764088f79 ryanofsky: Code review ACK 3958295bc8a3787b66b0269190218a3764088f79. I like this change. The new behavior makes more sense, and the change makes the code clearer, since the current LearnRelatedScripts call is hard to understand and explain. (Personally, I'd like it if this PR were merged before #17373 or that PR was rebased on top of this one so it would be less confusing.) meshcollider: utACK 3958295bc8a3787b66b0269190218a3764088f79 Tree-SHA512: 49a5f4b022b28042ad37ea309b28378a3983cb904e234a25795b5a360356652e0f8e60f15e3e64d85094ea63af9be01812d90ccfc08ca4f1dd927fdd8566e33f
2019-11-23Merge #17371: Refactor: Require scriptPubKey to get wallet SigningProviderSamuel Dobson
d0dab897afaac0a18aa47d3ce673a4a43a69178a Refactor: Require scriptPubKey to get wallet SigningProvider (Andrew Chow) 4b0c718f8f48c678cbe4575e9a9cf9e62a30f0da Accumulate result UniValue in SignTransaction (Andrew Chow) Pull request description: 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. Split from #17261 ACKs for top commit: instagibbs: utACK https://github.com/bitcoin/bitcoin/pull/17371/commits/d0dab897afaac0a18aa47d3ce673a4a43a69178a ryanofsky: Code review ACK d0dab897afaac0a18aa47d3ce673a4a43a69178a. Thanks for the SignTransaction update. No other changes since last review Sjors: Code review ACK d0dab897afaac0a18aa47d3ce673a4a43a69178a promag: Code review ACK d0dab897afaac0a18aa47d3ce673a4a43a69178a. meshcollider: Code review ACK d0dab897afaac0a18aa47d3ce673a4a43a69178a Tree-SHA512: c3f52df20fd9d6b3b5aa65562cf5f7dce7b7f44c148b0f988f8b578fce2a28e9b7bf010f5f04bb5bf60f5272b2899f1dbbfb8aee81579c21c9cba559d1d2bb70
2019-11-23Merge #17290: Enable BnB coin selection for preset inputs and subtract fee ↵Samuel Dobson
from outputs b007efdf1910db1d38671d6435d2f379bbf847d2 Allow BnB when subtract fee from outputs (Andrew Chow) db15e71e79b24601853703bebd1c92f4b523fd5f Use BnB when preset inputs are selected (Andrew Chow) Pull request description: Currently we explicitly disable BnB when there are preset inputs selected or when the subtract fee from outputs option is enabled. This PR enables BnB for both cases. Kind of an alternative to #17246 (implements the subtract fee from outputs part of it) and borrows a test from there too. ACKs for top commit: instagibbs: reACK https://github.com/bitcoin/bitcoin/pull/17290/commits/b007efdf1910db1d38671d6435d2f379bbf847d2 Sjors: re-ACK b007efdf1910db1d38671d6435d2f379bbf847d2 Tree-SHA512: 933276b09b2fa2ab43db7f0b98762f06f6f5fa8606195f96aca9fa1cb71ae4ee7156028dd482b1cada82ddd0996a9daf12ea5c152589fdf192cd96cbc51e99df
2019-11-20Allow BnB when subtract fee from outputsAndrew Chow
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-08Merge #17354: wallet: Tidy CWallet::SetUsedDestinationStatefanquake
0b75a7f0680d16a41043864a897470324917b1e8 wallet: Reuse existing batch in CWallet::SetUsedDestinationState (João Barbosa) 01f45dd00eb032a19d142026e4d019944192da19 wallet: Avoid recursive lock in CWallet::SetUsedDestinationState (João Barbosa) Pull request description: This PR makes 2 distinct changes around `CWallet::SetUsedDestinationState`: - 1st the recursive lock is removed and now it requires the lock to be held; - 2nd change is to support, in the best case, just a wallet database flush when transaction is added to the wallet. ACKs for top commit: achow101: ACK 0b75a7f0680d16a41043864a897470324917b1e8 MarcoFalke: ACK 0b75a7f0680d16a41043864a897470324917b1e8 ryanofsky: Code review ACK 0b75a7f0680d16a41043864a897470324917b1e8. Code changes looks fine but PR description should be updated to say what benefits of the change are. I might have missed something, but I didn't see a place where multiple batches were used previously and a single batch was used now. So the main benefit of this change appears to be removing a recursive lock? And maybe moving toward a consistent convention for passing batch instances? Tree-SHA512: abcf23a5850d29990668db20d6f624cca3e89629cc9ed003e0d05cde1b58ab2ff365034f156684ad13e55764b54c6c0c2bc7d5f96b8af7dc5e45a3be955d6b15
2019-11-06Remove locked_chain from GetDepthInMainChain and its callersAntoine Riard
We don't remove yet Chain locks as we need to preserve lock order with CWallet one until swapping at once to avoid deadlock failures (spotted by --enable-debug)
2019-11-06Use CWallet::m_last_block_processed_height in GetDepthInMainChainAntoine Riard
Avoid to lock chain to query state thanks to tracking last block height in CWallet.
2019-11-06Add block_height field in struct ConfirmationAntoine Riard
At wallet loading, we rely on chain state querying to retrieve height of txn, to do so we ensure that lock order is respected between cs_main and cs_wallet. If wallet loaded is the wallet-tool one, all wallet txn will show up with a height of zero. It doesn't matter as confirmation height is not used by wallet-tool. Reorder arguments and document Confirmation calls to avoid ambiguity. Fixes nits left from #16624
2019-11-06Replace CWalletTx::SetConf by Confirmation initialization listAntoine Riard
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-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-04wallet: Lock address type in ReserveDestinationJoão Barbosa
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 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: 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-01Remove SetWalletFlag from WalletStorageAndrew Chow
SetWalletFlag is unused. Does not change any behavior
2019-10-25Refactor: Split up CWallet and LegacyScriptPubKeyMan and classesAndrew Chow
This moves CWallet members and methods dealing with keys to a new LegacyScriptPubKeyMan class, and updates calling code to reference the new class instead of CWallet. Most of the changes are simple text replacements and variable substitutions easily verified with: git log -p -n1 -U0 --word-diff-regex=. The only nontrivial chunk of code added is the new LegacyScriptPubKeyMan class declaration, but this code isn't new and is just selectively copied and moved from the previous CWallet class declaration. This can be verified with: git log -p -n1 --color-moved=dimmed_zebra src/wallet/scriptpubkeyman.h src/wallet/wallet.h or git diff HEAD~1:src/wallet/wallet.h HEAD:src/wallet/scriptpubkeyman.h This commit does not change behavior.
2019-10-25MOVEONLY: Move key handling code out of wallet to keyman fileAndrew Chow
Start moving wallet and ismine code to scriptpubkeyman.h, scriptpubkeyman.cpp The easiest way to review this commit is to run: git log -p -n1 --color-moved=dimmed_zebra And check that everything is a move (other than includes and copyrights comments). This commit is move-only and doesn't change code or affect behavior.
2019-10-25Move wallet enums to walletutil.hAndrew Chow
2019-10-24Merge #17154: wallet: Remove return value from CommitTransactionWladimir J. van der Laan
9e95931865186d7a9a6dc54b64bd96507e9fea4b [wallet] Remove `state` argument from CWallet::CommitTransaction (John Newbery) d1734f9a3b138ab046f38ee44a09bc3847bf938a [wallet] Remove return value from CommitTransaction() (John Newbery) b6f486a02b463ffeaf82ec11fc6f74f439c037ae [wallet] Add doxygen comment to CWallet::CommitTransaction() (John Newbery) 8bba91b22d22a8dfea7c947b542b1022bfc1c0ea [wallet] Fix whitespace in CWallet::CommitTransaction() (John Newbery) Pull request description: `CommitTransaction()` returns a bool to indicate success, but since commit b3a7410 (#9302) it only returns true, even if the transaction was not successfully broadcast. This commit changes CommitTransaction() to return void. All dead code in `if (!CommitTransaction())` branches has been removed. Two additional commits fix up the idiosyncratic whitespace in `CommitTransaction` and add a doxygen comment for the function. ACKs for top commit: laanwj: ACK 9e95931865186d7a9a6dc54b64bd96507e9fea4b Tree-SHA512: a55a2c20369a45222fc0e02d0891495655a926e71c4f52cb72624768dd7b9c1dca716ea67d38420afb90f40c6e0fd448caa60c18fd693bb10ecb110b641820e6
2019-10-21update variable naming conventions for IsTrustedJeremy Rubin
2019-10-21Cache tx Trust per-call to avoid DoSJeremy Rubin
2019-10-21Merge #17070: wallet: Avoid showing GUI popups on RPC errorsWladimir J. van der Laan
facec1c643105d0ae74b5d32cf33d593f9e82a36 wallet: Avoid showing GUI popups on RPC errors (MarcoFalke) Pull request description: RPC errors and warnings are shown as popups in the GUI instead of being returned to the RPC caller. For example, ``` $ ./src/bitcoin-cli loadwallet $(pwd)/./test/functional/data/wallets/high_minversion/ error code: -4 error message: Wallet loading failed. ``` gives me a GUI popup and no reason why loading the wallet failed. After this pull request: ``` $ ./src/bitcoin-cli loadwallet $(pwd)/./test/functional/data/wallets/high_minversion/ error code: -4 error message: Wallet loading failed: Error loading /home/marco/workspace/btc_bitcoin_core/./test/functional/data/wallets/high_minversion/wallet.dat: Wallet requires newer version of Bitcoin Core ACKs for top commit: laanwj: Code review ACK facec1c643105d0ae74b5d32cf33d593f9e82a36 Tree-SHA512: c8274bbb02cfcf71676eeec1e773e51fb3538cf93f82e7cb8536f4716d44ed819cdc162dfc039ac7386a4db381a734cdb27fd32567043a1180c02519fbcba194
2019-10-20wallet: Remove unused GetLabelNameSebastian Falbesoner
2019-10-18[wallet] Remove `state` argument from CWallet::CommitTransactionJohn Newbery
The `state` return argument has not been set since commit 611291c198. Remove it (and the one place that it's used in a calling function).
2019-10-18[wallet] Remove return value from CommitTransaction()John Newbery
CommitTransaction returns a bool to indicate success, but since commit b3a74100b8 it only returns true, even if the transaction was not successfully broadcast. This commit changes CommitTransaction() to return void. All dead code in `if (!CommitTransaction())` branches has been removed.
2019-10-18[wallet] Add doxygen comment to CWallet::CommitTransaction()John Newbery
2019-10-15Remove unused includespracticalswift
2019-10-08wallet: Avoid showing GUI popups on RPC errorsMarcoFalke
2019-10-02Merge #16524: Wallet: Disable -fallbackfee by defaultMarcoFalke
ea4cc3a7b36a9c77dbf0aff439da3ef0ea58e6e4 Truly decouple wallet from chainparams for -fallbackfee (Jorge Timón) Pull request description: Before it was 0 by default for main and 20000 for test and regtest. Now it is 0 by default for all chains, thus there's no need to call Params(). Also now the default for main is properly documented. Suggestion for release notes: -fallbackfee was 0 (disabled) by default for the main chain, but 20000 by default for the test chains. Now it is 0 by default for all chains. Testnet and regtest users will have to add fallbackfee=20000 to their configuration if they weren't setting it and they want it to keep working like before. Should I propose them to the wiki for the release notes or only after merge? For more context, see https://github.com/bitcoin/bitcoin/pull/16402#issuecomment-515701042 ACKs for top commit: MarcoFalke: ACK ea4cc3a7b36a9c77dbf0aff439da3ef0ea58e6e4 Tree-SHA512: fdfaba5d813da4221e405e0988bef44f3856d10f897a94f9614386d14b7716f4326ab8a6646e26d41ef3f4fa61b936191e216b1b605e9ab0520b0657fc162e6c
2019-10-02Truly decouple wallet from chainparams for -fallbackfeeJorge Timón
Before it was 0 by default for main and 20000 for test and regtest. Now it is 0 by default for all chains, thus there's no need to call Params(). Also now the default for main is properly documented
2019-09-26Change default address type to bech32Gregory Sanders
2019-08-29Modify wallet tx status if has been reorged outAntoine Riard
Add a LockChain method to CWallet to know if we can lock or query chain state safely. At tx loading, we rely on chain to know if hashBlock of tx is still in main chain. If not, we set its status to unconfirmed and reset its hashBlock/nIndex. If wallet loaded is the wallet-tool one, all wallet txn will show up with a height of zero. It doesn't matter as status is not used by wallet-tool. We take lock prematurely in CWallet::LoadWallet and CWallet::Verify to ensure that lock order is respected between cs_main an cs_wallet.
2019-08-23Encapsulate tx status in a Confirmation structAntoine Riard
Instead of relying on combination of hashBlock and nIndex values to manage tx in its lifecycle, we introduce 4 status : CONFIRMED, UNCONFIRMED, CONFLICTED, ABANDONED. hashBlock and nIndex magic values should only be used at serialization/deserialization for backward-compatibility. At block disconnection, we know flag txn as UNCONFIRMED where previously they kept their states until being override by a block connection or abandontransaction call. This is a change in behavior for which user may have to call abandon twice if transaction is disconnected and not accepted back in the mempool. We assert status transitioning right in AddToWallet. Doing so flagged a misbehavior in ComputeTimeSmart unit test where same tx is confirmed twice in different block. To avoid inconsistencies we unconfirmed tx before new connection in different block. We also remove a cs_main lock in test, as AddToWallet and its callees don't rely on locked chain.
2019-08-21Merge #16572: wallet: Fix Char as Bool in Walletfanquake
2dbfb37b407ed23b517f507d78fb77334142dce5 Fix Char as Bool in interfaces (Jeremy Rubin) Pull request description: In a few places in src/wallet/wallet.h, we use a char when semantically we want a bool. This is kind of an issue because it means we can unserialize the same transaction with different fFromMe flags (as differing chars) and evaluate the following section in wallet/wallet.cpp ```c++ if (wtxIn.fFromMe && wtxIn.fFromMe != wtx.fFromMe) { wtx.fFromMe = wtxIn.fFromMe; fUpdated = true; } ``` incorrectly (triggering an fUpdated where both fFromMe values represent true, via different chars). I don't think this is a vulnerability, but it's just a little messy and unsemantic, and could lead to issues with stored wtxIns not being findable in a map by their hash. The serialize/unserialize code for bool internally uses a char, so it should be safe to make this substitution. NOTE: Technically, this is a behavior change -- I haven't checked too closely that nowhere is depending on storing information in this char. Theoretically, this could break something because after this change a tx unserialized with such a char would preserve it's value, but now it is converted to a ~true~ canonical bool. ACKs for top commit: achow101: Code review ACK 2dbfb37b407ed23b517f507d78fb77334142dce5 meshcollider: Code review ACK 2dbfb37b407ed23b517f507d78fb77334142dce5 Tree-SHA512: 8c0dc9cf672aa2276c694facbf50febe7456eaa8bf2bd2504f81a61052264b8b30cdb5326e1936893adc3d33504667aee3c7e207a194c71d87b3e7b5fe199c9d
generated by cgit v1.2.3 (git 2.26.2) at 2024-11-02 22:47:29 +0000