diff options
| author | MeshCollider <dobsonsa68@gmail.com> | 2019-09-06 01:28:15 +1200 |
|---|---|---|
| committer | MeshCollider <dobsonsa68@gmail.com> | 2019-09-06 01:28:54 +1200 |
| commit | 5e202382a987351a59d5cda98ea9f2aee99f61a5 (patch) | |
| tree | 50a1d4ca1efc3f5973e4edee39cc69b305ff932b /src/wallet/rpcwallet.cpp | |
| parent | 5667b0d758f119dadc5b41e0bf24e45da1132e54 (diff) | |
| parent | 442a87cc0ae43ebc9b6654a6165778eecb931f74 (diff) | |
| download | bitcoin-5e202382a987351a59d5cda98ea9f2aee99f61a5.tar.xz | |
Merge #16624: wallet: encapsulate transactions state
442a87cc0ae43ebc9b6654a6165778eecb931f74 Add a test wallet_reorgsrestore (Antoine Riard)
40ede992d97df38282919693dfe851c975c3b1d8 Modify wallet tx status if has been reorged out (Antoine Riard)
7e89994133725125dddbfa8d45484e3b9ed51c6e Remove SyncTransaction for conflicted txn in CWallet::BlockConnected (Antoine Riard)
a31be09bfd77eed497a8e251d31358e16e2f2eb1 Encapsulate tx status in a Confirmation struct (Antoine Riard)
Pull request description:
While working on #15931, I've tried to rationalize tx state management to ease integration of block height tracking per-wallet tx. We currently rely on a combination of `hashBlock` and `nIndex` with magic value to determine tx confirmation, conflicted or abandoned state. It's hard to reason and error-prone. To solve that, we encapsulate these fields in a `TxConfirmation` struct and introduce a `TxState` member that we update accordingly at block connection/disconnection.
Following jnewbery [recommendation](https://github.com/bitcoin/bitcoin/pull/15931#discussion_r312576506), I've taken these changes in its own commit, and open a PR to get them first. It would ease review of aforementioned PR, but above all should ease fixing of long-term issues like :
* https://github.com/bitcoin/bitcoin/issues/7315 (but maybe we should abandon abandontransaction or relieve it to only free outpoints not track the transaction as abandoned in itself, need its own discussion)
* https://github.com/bitcoin/bitcoin/issues/8692 where we should cancel conflicted state of transactions chain smoothly
* `MarkConflicted` in `LoadToWallet` is likely useless if we track conflicts rights at block connection
Main changes of this PR to get right are tx update in `AddToWallet` and serialization/deserialization logic.
ACKs for top commit:
meshcollider:
Light re-Code Review ACK 442a87cc0ae43ebc9b6654a6165778eecb931f74
ryanofsky:
utACK 442a87cc0ae43ebc9b6654a6165778eecb931f74. Changes since last review are switching from `hasChain` to `LockChain` and removing chain lock in `WalletBatch::LoadWallet` that's redundant with the new lock still added in `CWallet::LoadWallet`, and fixing python test race condition.
Tree-SHA512: 029209e006de0240436817204e69e548c5665e2b0721b214510e7aba7eba130a5eab441d3a1ad95bd6426114dd27390492c77bf4560a9610009b32cd0a1f72f7
Diffstat (limited to 'src/wallet/rpcwallet.cpp')
| -rw-r--r-- | src/wallet/rpcwallet.cpp | 6 |
1 files changed, 3 insertions, 3 deletions
diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 18be3a6ea5..22a5f7e249 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -134,10 +134,10 @@ static void WalletTxToJSON(interfaces::Chain& chain, interfaces::Chain::Lock& lo entry.pushKV("generated", true); if (confirms > 0) { - entry.pushKV("blockhash", wtx.hashBlock.GetHex()); - entry.pushKV("blockindex", wtx.nIndex); + entry.pushKV("blockhash", wtx.m_confirm.hashBlock.GetHex()); + entry.pushKV("blockindex", wtx.m_confirm.nIndex); int64_t block_time; - bool found_block = chain.findBlock(wtx.hashBlock, nullptr /* block */, &block_time); + bool found_block = chain.findBlock(wtx.m_confirm.hashBlock, nullptr /* block */, &block_time); assert(found_block); entry.pushKV("blocktime", block_time); } else { |