wallet: Replace -zapwallettxes with wallet tool command #19700

1. 
   achow101 commented at 0:49 am on August 12, 2020: member

   Replaces the -zapwallettxes startup option with a zapwallettxes command in the wallet tool.

   To preserve the transaction metadata, using zapwallettxes rewrites the transactions as a zaptx record and removes the original tx record. bestblock is also reset to null to trigger a rescan. On loading the wallet, a rescan will be triggered. If any zaptx records were found, then the metadata from those records is copied to the new tx. All zaptx records are removed after loading. If metadata is not kept (by using -keepmeta=0 in the wallet tool), then no zaptx records will be created.

   Lastly -zapwallettxes is replaced with an error telling users to use the wallet tool command instead.

   Alternative to #19653 and #19671

2. fanquake added the label Wallet on Aug 12, 2020
3. 
   laanwj commented at 11:04 am on August 12, 2020: member

   Concept ACK!

   I think this is the best solution if we decide to not remove the functionality wholesale.

4. achow101 force-pushed on Aug 12, 2020
5. achow101 force-pushed on Aug 12, 2020
6. achow101 force-pushed on Aug 12, 2020
7. 
   meshcollider commented at 0:08 am on August 13, 2020: contributor

   As I just commented on #19671, I think it is better to just remove the option, so I prefer #19671
8. 
   jonasschnelli commented at 7:59 am on August 14, 2020: contributor

   Concept ACK and light core review ACK 5739465627a5ed9fb8cefa87e1fa026475f602e5.

   The questions in the past popped up if we have to bump the wallet version (not to mix with the minwalletversion) if we add a key/value type. I don’t think so but can remember @luke-jr had some arguments for bumping it.

9. DrahtBot added the label Needs rebase on Aug 14, 2020
10. walletdb: Have FindWalletTx return a map instead of a list 41e30f22b7
11. walletdb: Add zaptx record 36e758393b
12. walletdb: Write zapped txs as zaptx records before erasing tx record

    If we are keeping metadata, write the zapped txs in zaptx records.
    Remove them after fetching the metadata.

    6246c40873
13. wallet: Keep zapped tx metadata using zaptx record 14915b4f0c
14. wallettool: Add zapwallettxes command 362797f757
15. test: zapwallettxes metadata ff7e6f20b5
16. Remove -zapwallettxes startup option

    Removes the startup option and replaces it with an error message telling
    users to use the RPC instead.

    f7604a568d
17. achow101 force-pushed on Aug 14, 2020
18. DrahtBot removed the label Needs rebase on Aug 14, 2020
19. 
    DrahtBot commented at 8:39 pm on August 20, 2020: member

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #19752 (test: Update wait_until usage in tests not to use the one from utils by slmtpz)
    • #19671 (wallet: Remove -zapwallettxes by achow101)
    • #19619 (Remove wallet.dat path handling from wallet.cpp, rpcwallet.cpp by ryanofsky)
    • #19602 (wallet: Migrate legacy wallets to descriptor wallets by achow101)
    • #19168 (Refactor: Improve setup_clean_chain semantics by fjahr)
    • #19137 (wallettool: Add dump and createfromdump commands by achow101)
    • #19077 (wallet: Add sqlite as an alternative wallet database and use it for new descriptor wallets by achow101)
    • #18836 (wallet: upgradewallet fixes and additional tests by achow101)
    • #18242 (Add BIP324 encrypted p2p transport de-/serializer (only used in tests) by jonasschnelli)
    • #15719 (Wallet passive startup by ryanofsky)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

20. DrahtBot added the label Needs rebase on Aug 27, 2020
21. 
    DrahtBot commented at 8:01 am on August 27, 2020: member

    🐙 This pull request conflicts with the target branch and needs rebase.

    Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a “draft”.

22. 
    fanquake commented at 1:59 pm on August 27, 2020: member

    Seems the we have consensus to move forward in #19671, so I’m going to close this PR.
23. fanquake closed this on Aug 27, 2020

    ---

24. fanquake referenced this in commit a1d14f522c on Sep 1, 2020
25. sidhujag referenced this in commit d996825ddb on Sep 3, 2020
26. DrahtBot locked this on Feb 15, 2022

Contributors
achow101 laanwj meshcollider jonasschnelli DrahtBot fanquake

Labels
Wallet Needs rebase