Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wallet: deep clean #511

Merged
merged 3 commits into from
Jan 12, 2021
Merged

wallet: deep clean #511

merged 3 commits into from
Jan 12, 2021

Conversation

pinheadmz
Copy link
Member

@pinheadmz pinheadmz commented Sep 3, 2020

Closes #467

Requires #512 (CI tests will fail until rebase)

This will likely be required for users with walletDB corruption due to the CLAIM->REGISTER bug (#454) and/or the FINALIZE bug (#464)

walletDB deepClean():

  • deletes all mapping of blocks, outpoints and tx hashes to wallet ids
  • deletes (almost) the entire txdb (all history, all balance data, for all wallets)
  • keeps all BID blinds (otherwise a human must remember the hidden values)
  • keeps all account metadata (account name, type, address depth, etc)
  • keeps all name->wallet ID mapping (although this may be unnecessary)
  • keeps all address-hash path mapping (addresses from all accounts)

wallet HTTP /deepclean:

  • requires admin token, like /rescan
  • requires setting parameter I_HAVE_BACKED_UP_MY_WALLET to true

A deep clean does not automatically initiate a rescan -- but one must be initiated by the user after deep clean is complete.

TODO:

  • add bmocha tests
  • test with shell script that corrupts wallet on unpatched version of hsd, then upgrades to master branch

@pinheadmz pinheadmz requested a review from tynes September 3, 2020 16:56
@coveralls
Copy link

coveralls commented Sep 3, 2020

Pull Request Test Coverage Report for Build 461467296

  • 28 of 36 (77.78%) changed or added relevant lines in 2 files are covered.
  • 6 unchanged lines in 4 files lost coverage.
  • Overall coverage decreased (-0.001%) to 59.414%

Changes Missing Coverage Covered Lines Changed/Added Lines %
lib/wallet/http.js 1 9 11.11%
Files with Coverage Reduction New Missed Lines %
lib/covenants/rules.js 1 67.45%
lib/protocol/consensus.js 1 82.79%
lib/script/script.js 1 63.07%
lib/net/pool.js 3 32.49%
Totals Coverage Status
Change from base Build 377501416: -0.001%
Covered Lines: 19453
Relevant Lines: 30508

💛 - Coveralls

@pinheadmz
Copy link
Member Author

pinheadmz commented Sep 8, 2020

I wrote a bash script here: https://gist.github.com/pinheadmz/a25a58a4bea13a88035cf1899f7c3e15

Here's the exciting conclusion:

Previous HEAD position was 97737f15 v2.2.0
Switched to branch 'finalize-test'
50136
{
  "account": -1,
  "tx": 108,
  "coin": 104,
  "unconfirmed": 199998959660,
  "confirmed": 199998959660,
  "lockedUnconfirmed": 1000000,
  "lockedConfirmed": 1000000
}
{
  "success": true
}
{
  "account": -1,
  "tx": 108,
  "coin": 104,
  "unconfirmed": 199998959660,
  "confirmed": 199998959660,
  "lockedUnconfirmed": 0,
  "lockedConfirmed": 0
}

@pinheadmz pinheadmz added this to the v2.3.0 milestone Sep 17, 2020
Copy link

@RevCBH RevCBH left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense to me and looks generally reasonable. Not sure what the approve process is, so just leaving the review as 'Comment', but there's nothing blocking as far as I can tell.

@turbomaze asked me to take a look, btw, if you're wondering about the random reviewer.

lib/wallet/http.js Show resolved Hide resolved
lib/wallet/walletdb.js Outdated Show resolved Hide resolved
test/wallet-deeprescan-test.js Outdated Show resolved Hide resolved
@turbomaze
Copy link
Contributor

Just to add my 2c, have tested deep rescans somewhat extensively on local regtest wallets, on small mainnet wallets, and also some larger mainnet wallets. Have not encountered any issues!

walletDB deepClean():
- deletes all mapping of blocks, outpoints and tx hashes to wallet ids
- deletes ENTIRE txdb (all history, all balance data, for all wallets)
- keeps all account metadata (account name, type, address depth, etc)
- keeps all name->wallet ID mapping (although this may be unnecessary)
- keeps all address-hash path mapping (addresses from all accounts)

wallet HTTP /deepclean:
- requires admin token, like /rescan
- requires setting parameter `I_HAVE_BACKED_UP_MY_WALLET` to `true`
@pinheadmz pinheadmz changed the title wallet: deep rescan wallet: deep clean Jan 4, 2021
@pinheadmz
Copy link
Member Author

@RevCBH @turbomaze This PR has been updated! (finally!)

It's not deep rescan anymore - now its just deep clean and it does NOT automatically start a rescan. Meaning this operation is now a two-step process. Changelog has been updated to reflect this as well as the integration test and also the bash script I describe here

@pinheadmz
Copy link
Member Author

This is a sample of the debug log output when Deep Clean is run:

[warning] (wallet) Initiating Deep Clean...
[warning] (wallet) Clearing block map, tx map and outpoint map...
[warning] (wallet) Clearing all tx history for wallet: primary (0)
[warning] (wallet) Clearing all tx history for wallet: 5V1xGufkA88e1pbPGZyFiy58oKg1pVk (1)
[warning] (wallet) Clearing all tx history for wallet: 5V1ybvoPa4vWJA17ftSKrvbpiVNm3AX (2)
[warning] (wallet) Deep Clean complete. A rescan is now required.

@HDardenne-HNS
Copy link

Just tried it on my mainnet wallet, with a lot of transfer and finalize in it and negative balance. Seems to work well.
Before :

"balance": {
        "account": -1,
        "tx": 1768,
        "coin": 185,
        "unconfirmed": 9116650351,
        "confirmed": 9116650351,
        "lockedUnconfirmed": 9513310000,
        "lockedConfirmed": 6106000011
    }

after

"balance": {
        "account": -1,
        "tx": 1768,
        "coin": 185,
        "unconfirmed": 9116650351,
        "confirmed": 9116650351,
        "lockedUnconfirmed": 6106000011,
        "lockedConfirmed": 6106000011
    }

@pinheadmz pinheadmz merged commit c5c47a8 into handshake-org:master Jan 12, 2021
const key = 'v'.charCodeAt();
const prefix = layout.t.encode(wid);
await removeRange({
gte: Buffer.concat([prefix, Buffer.alloc(1)]),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this supposed to be a 1 or a 0?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It "should" be a single 0x00 byte buffer. This code will remove everything in the txdb up to the blinds. The next snippet removes everything in the txdb after the blinds:

Removed: 0x00 -> v and v -> 0xff

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Proposal: Deep Rescan
6 participants