Sort pending wallet transactions before reaccepting #5511
Conversation
|
oh erp. Yea, good find. |
|
I don't believe that chrnological order (really nOrderPos order in your patch) always means dependency order, however. Do you have a way to reproduce the behavior you're fixing? |
|
I assumed that transactions are not added out of order to the wallet, because it appears as if they have to pass certain checks to be added to the wallet in the first place, and that nOrderPos reflects correct dependency order. If this is questionable, then this approach might not cover all situations. A simple test to check, if a client adds unconfirmed transactions back to the memory pool in correct order:
As RPC test: https://gist.github.com/dexX7/06df91d1a7f99190d8d6 |
|
I'd say that transactions generated by the wallet itself, which are the ones useful to insert into the mempool, are always in dependency order by nOrderPos, so this makes sense. |
dexX7
force-pushed
the
reaccept-sorted-wtx
branch
from
3358e55 to
daa3c3d
Compare
|
@zander: I did not move the lock, but I believe both should work. I reworded the comment from "chronological order" to "based on initial wallet insertion order". Hope this is more accurate. |
During startup, when adding pending wallet transactions, which spend outputs of other pending wallet transactions, back to the memory pool, and when they are added out of order, it appears as if they are orphans with missing inputs. Those transactions are then rejected and flagged as "conflicting" (= not in the memory pool, not in the block chain). To prevent this, transactions are explicitly sorted.
dexX7
force-pushed
the
reaccept-sorted-wtx
branch
from
daa3c3d to
e9c3215
Compare
|
Rebased ( Tested with the above linked test a few hundred times, although I can't say in which case |
|
utACK |
1 similar comment
|
utACK |
e9c3215 [Wallet] sort pending wallet transactions before reaccepting (dexX7)
|
As a wallet user who has this issue and a lot of BTC in this status what can be done to get them back? |
|
Hey @willstg, fortunally this is only a local issue, and your BTC are not lost. Please create a backup of the wallet nevertheless. Shutdown Bitcoin Core before continuing. Depending on the OS, the default wallet locations are:
To clear the conflicted transactions, start Alternatively,
If you had a bunch of unconfirmed transactions, then it could be that not all, but only some, or maybe even none, are going to be confirmed. This depends on whether other nodes received the transactions (and ultimately the miners). There is also a chance that other nodes received the unconfirmed transactions, but your local client doesn't know anything about them after zapping, so please wait some time, ideally a few blocks, until you resend the transactions, which didn't go through. |
|
Thumbs up @dexX7 thanks. |
91b48c7 [Build] Add new merkle files to CMake lists (warrows) 48a3aff [Wallet] Ignore coinbase and zc tx "conflicts" (warrows) 3572354 [Wallet] Fix an error in tx depth computation (warrows) 6928369 [Tests] Enable abandonconflict functional test (warrows) 34cd496 Fix that CWallet::AbandonTransaction would only traverse one level (Ben Woosley) aba5b75 Fix calculation of balances and available coins. (Alex Morcos) 48d705f Remove stale wallet transactions on initial load. (presstab) 12985ae Flush wallet after abandontransaction (Alex Morcos) 8f87956 [Wallet] sort pending wallet transactions before reaccepting (dexX7) 9c2f445 [Wallet] Call notification signal when a transaction is abandoned (Jonas Schnelli) 778ebf3 Add new rpc call: abandontransaction (Alex Morcos) 0e86c3e Make wallet descendant searching more efficient (Alex Morcos) d0083a8 Make sure conflicted wallet tx's update balances (Alex Morcos) 6a50e03 [Wallet] Keep track of explicit wallet conflicts instead of using mempool (warrows) 7ccb2b5 [Wallet] Do not flush the wallet in AddToWalletIfInvolvingMe(..) (warrows) 47345be [Refactor] Move wallet functions out of header (warrows) ab9efb8 [Wallet] Switch to a constant-space Merkle root/branch algorithm (warrows) 5447622 [Wallet] Do not store Merkle branches in the wallet (warrows) Pull request description: This pull request is a happy melting pot of improvements regarding transactions handling. Most of them are backports from bitcoin. I advise reviewers to check the code of the different commits independently to understand them more easily. However, testing is probably better done all at once. I am making a single pull request because these changes are all entangled and introducing some of them without others would probably introduce temporary bugs. ## Commits details ## - 6c3e2ac backport of bitcoin#6550 - 5304fdf backport of bitcoin#6508 - c3eeeac simple code move from the header to the cpp file. It contains no functional change. - 6cc4d37 backport of bitcoin#4805 - 10be1db backport of bitcoin#7105 - 8a34c32 backport of bitcoin#7306 - 3caf123, 9e17178 and 240f5b4 are the backport for bitcoin#7312 - ad6d0b1 backport of bitcoin#5511 - fcc07c3562 backport of bitcoin#9311 - 5ed5e266794 is an update of #825 - 392d504 backport of bitcoin#7715 - 7199f3a backport of bitcoin#13652 - f09d999 enables and fixes the test from bitcoin#7312 - 4fd43c5 fixes an oversight in bitcoin#7105 backport ACKs for top commit: random-zebra: ACK 91b48c7 Fuzzbawls: ACK 91b48c7 Tree-SHA512: 2628cebe98805b8048b920b51ee26fd4f0c53643d78da9b8cb265aede52dfe1d40c8c19d34293c232c5c35be7f1ab89ff5b4a07073a4b27c371ea70eb8708669
6 participants
During startup, when adding pending wallet transactions, which spend outputs of
other pending wallet transactions, back to the memory pool, and when they are
added out of order, it appears as if they are orphans with missing inputs.
Those transactions are then rejected and flagged as "conflicting" (= not in the
memory pool, not in the block chain).
To prevent this, transactions are explicitly sorted.