Add -zapwallettx function, a diagnostic tool to assist in wallet repair

jgarzik commented at 2:23 pm on February 12, 2014: contributor

It is somewhat suboptimal in that it loads every wallet record, when it really should just scan keys.

laanwj commented at 2:36 pm on February 12, 2014: member

02014-02-12 14:32:44 init message: Zapping all transactions from wallet...
12014-02-12 14:32:44 init message: Loading wallet...

Worked for me on regtest. Before: wallet with two unconfirmed mutated transactions After: wallet with only confirmed transactions

(Did not test with dependent chains of unconfirmed transactions, just direct duplicates)

in src/init.cpp: in 7ce1e68ffa outdated

903@@ -897,6 +904,15 @@ bool AppInit2(boost::thread_group& threadGroup)
904         pwalletMain = NULL;
905         LogPrintf("Wallet disabled!\n");
906     } else {
907+        uiInterface.InitMessage(_("Zapping all transactions from wallet..."));

laanwj commented at 2:53 pm on February 12, 2014:

Please don’t log/display this message if not zapping, it caught me unaware for a minute

jgarzik commented at 3:00 pm on February 12, 2014:

heh, whoops!

Diapolo commented at 3:21 pm on February 12, 2014: none

What does this really do or help with which problem? From the description and comments in the code I don’t understand it’s purpose ;).

laanwj commented at 3:25 pm on February 12, 2014: member

@Diapolo This gets rid of all unconfirmed transactions. Currently used for cleaning up extra transactions inserted by the malleability DOS. It’s less drastic than -salvagewallet which also loses all label information and other metadata.

Diapolo commented at 3:56 pm on February 12, 2014: none

So it “just” clears (for Qt) the list of unconfirmed transactions? Does this also remove unconfirmed ones that a users created by himself but where the TX fee was too low for example? IMHO the description for the switch should be a little more user friendly/descriptive perhaps.

jgarzik commented at 3:58 pm on February 12, 2014: contributor

All transactions are removed. Confirmed transactions are then added.

No unconfirmed transactions will be present after this is run.

It might be best just to not document this in command line help. It’s a rare-use utility.

laanwj commented at 3:58 pm on February 12, 2014: member

@diapolo It’s not meant for willy-nilly use. Wrong use of this feature can result in double-spending (transactions that you get rid of in this way may still be in the mempool somewhere else and get mined at some point). It’s only meant to get rid of transactions that can never confirm.

sipa commented at 4:00 pm on February 12, 2014: member

Does this clear the vfSpent flag in other transactions? Otherwise it seems pretty useless…

laanwj commented at 4:03 pm on February 12, 2014: member

@sipa It removes all transactions and forces a rescan.

cozz commented at 4:17 pm on February 12, 2014: contributor

As far as I see does this also remove the information of CWalletTx.strFromAccount. So we should store all strFromAccount by txhash and then restore this information after rescan.

Also I dont like the name and description of the parameter. They are really very confusing in several ways. (If even we are confused, how can a normal user understand this?) My suggestion is “-cleanwallet”. We already have disablewallet,salvagewallet,upgradewallet so it should be -XXXwallet. And description “Remove all unconfirmed transactions (implies -rescan)”

Edit: + “on startup”, so “Remove all unconfirmed transactions on startup (implies -rescan)”

sipa commented at 5:26 pm on February 12, 2014: member

So how is this different from -salvagewallet?

laanwj commented at 5:27 pm on February 12, 2014: member

@sipa It doesn’t lose label information. It keeps everything but transactions, instead of only keys. But yes, maybe it could be defined as a mode of salvagewallet instead of a seperate option.

sipa commented at 5:36 pm on February 12, 2014: member

Got it. Sounds useful indeed, as salvagewallet is overkill here…

gmaxwell commented at 5:00 am on February 14, 2014: contributor

Salvage wallet currently totally corrupts my testnet wallet. I don’t know why. Start off with a zillion tnbtc, salvage, end up with a balance of 150 and a wallet that won’t unlock. God knows what abusive things I’ve done to this wallet— I’m not aware of anyone having a similar issue with salvage.

Zapwallet works fine on it. ACK.

maaku commented at 6:51 am on February 14, 2014: contributor

Armchair architecting here but… when I saw the pull request name I thought it would be a JSON-RPC command to surgically remove a specific unconfirmed transaction (identified by hash). Which, btw, is probably a good feature to have but is a separate issue. Maybe zapwallettx should be renamed just zapwallet or flushwallet to avoid the confusion?

laanwj commented at 7:08 am on February 14, 2014: member

@maaku IMO, plural -zapwallettxes would be best. Just -zapwallet would imply the whole wallet is desintegrated.

laanwj commented at 4:21 pm on February 14, 2014: member

ACK in any case

Add -zapwallettxes cli/config option, used for wallet recovery

This diagnostic tool removes all "tx" records from the wallet db,
then forces a full rescan, to rebuild "tx" records accurately.

518f3bdae3

jgarzik commented at 4:35 pm on February 14, 2014: contributor

Updated to use -zapwallettxes, and collapse bug fixes into the main commit. Given ACKs, will merge, once pull tester is happy.

ghost commented at 4:54 pm on February 14, 2014: none

I think the plural of tx is more likely to be txs, so -zapwallettxs

BitcoinPullTester commented at 5:08 pm on February 14, 2014: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/518f3bdae3415fdb60cef984b69b36f2633c1fe1 for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

jgarzik referenced this in commit e051e65c21 on Feb 14, 2014

jgarzik merged this on Feb 14, 2014

jgarzik closed this on Feb 14, 2014

jgarzik deleted the branch on Aug 24, 2014

in src/walletdb.cpp:744 in 518f3bdae3

739+    }
740+    catch (...) {
741+        result = DB_CORRUPT;
742+    }
743+
744+    if (fNoncriticalErrors && result == DB_LOAD_OK)

promag commented at 2:40 am on December 18, 2017:

fNoncriticalErrors is always false as such this is dead code. Cleaned in #11923.

DrahtBot locked this on Sep 8, 2021

Add -zapwallettx function, a diagnostic tool to assist in wallet repair #3659