Skip to content

Commit

Permalink
walletdb: make markState part of the setTip.
Browse files Browse the repository at this point in the history
Fixes two markState issues:
  - On close/crash of walletdb on add block wdb state would have
    incorrect state for the startHeight/startHash and mark. WalletDB
    could end up having incorrect startHeight/startHash for the rest
    of the walletDB until rescan before height. Check the
    wallet-chainstate-test case for corruption.
  - On the specific height where the walletDB received first
    transaction, state.height would also be incorrect (only for one
    block).
  • Loading branch information
nodech committed Nov 3, 2023
1 parent 949a03e commit 2601f3c
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 32 deletions.
48 changes: 17 additions & 31 deletions lib/wallet/walletdb.js
Original file line number Diff line number Diff line change
Expand Up @@ -1839,13 +1839,24 @@ class WalletDB extends EventEmitter {
/**
* Sync the current chain state to tip.
* @param {BlockMeta} tip
* @param {Boolean} checkMark - should we check startHeight/mark. This should
* only happen if we are progressing forward in history and have txs.
* @returns {Promise}
*/

async setTip(tip) {
async setTip(tip, checkMark = false) {
const b = this.db.batch();
const state = this.state.clone();

// mark state if state has not been marked, we are moving forward
// and we have txs. If state is marked, it means we already found
// first tx for the whole wdb, so no longer move it forward.
if (checkMark && !state.marked) {
state.startHeight = tip.height;
state.startHash = tip.hash;
state.marked = true;
}

if (tip.height < state.height) {
// Hashes ahead of our new tip
// that we need to delete.
Expand Down Expand Up @@ -1890,26 +1901,6 @@ class WalletDB extends EventEmitter {
return height;
}

/**
* Mark current state.
* @param {BlockMeta} block
* @returns {Promise}
*/

async markState(block) {
const state = this.state.clone();
state.startHeight = block.height;
state.startHash = block.hash;
state.marked = true;

const b = this.db.batch();
b.put(layout.R.encode(), state.encode());
await b.write();

this.state = state;
this.height = state.height;
}

/**
* Get a wallet map.
* @param {Buffer} key
Expand Down Expand Up @@ -2209,7 +2200,7 @@ class WalletDB extends EventEmitter {
assert(tip);

await this.revert(tip.height);
await this.setTip(tip);
await this.setTip(tip, false);
}

/**
Expand Down Expand Up @@ -2309,7 +2300,8 @@ class WalletDB extends EventEmitter {
}

// Sync the state to the new tip.
await this.setTip(tip);
// If we encountered wallet txs, we also trigger mark check.
await this.setTip(tip, walletTxs.length > 0);
} finally {
this.confirming = false;
}
Expand Down Expand Up @@ -2370,7 +2362,7 @@ class WalletDB extends EventEmitter {
const map = await this.getBlockMap(tip.height);

if (!map) {
await this.setTip(prev);
await this.setTip(prev, false);
this.emit('block disconnect', entry);
return 0;
}
Expand All @@ -2384,7 +2376,7 @@ class WalletDB extends EventEmitter {
}

// Sync the state to the previous tip.
await this.setTip(prev);
await this.setTip(prev, false);

this.logger.warning('Disconnected wallet block %x (tx=%d).',
tip.hash, total);
Expand Down Expand Up @@ -2454,12 +2446,6 @@ class WalletDB extends EventEmitter {
if (!wids)
return null;

// This is better place than _addBlock because here above check
// makes sure we really own the txs and it was not a false positive
// from the filter.
if (block && !this.state.marked)
await this.markState(block);

this.logger.info(
'Incoming transaction for %d wallets in WalletDB (%s).',
wids.size, tx.txid());
Expand Down
2 changes: 1 addition & 1 deletion test/wallet-chainstate-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -506,7 +506,7 @@ describe('WalletDB ChainState', function() {
assert.strictEqual(wdb.state.marked, true);
});

it.skip('should recover to the proper mark/startHeight after corruption', async () => {
it('should recover to the proper mark/startHeight after corruption', async () => {
// If we receive a block that has TXs (meaning wdb should care) but it
// DB/Node closes/crashes and restarted node does not have txs in the blocks.
// startHeight and mark will be set incorrectly.
Expand Down

0 comments on commit 2601f3c

Please sign in to comment.