Skip to content

Commit

Permalink
fixes rescanning wallet without --reset (#3654)
Browse files Browse the repository at this point in the history
the logic of 'connectBlock' filters any account that does not have its 'head'
equal to the previous block. this means that if we don't reset account heads
during rescan nothing will happen.

this is also true of rescanning using the '--from' flag. this flag lets users
start the rescan at a particular sequence. however, if we don't set account
heads to that sequence then scanning won't do anything.

- removes the 'reset' option and always resets accounts
- throws an error if the 'from' sequence is not in the chain or the header at
  that sequence is not on the main chain
- sets account heads equal to the block header before the from sequence if it is
  not the genesis block
  • Loading branch information
hughy authored Mar 20, 2023
1 parent c57ade0 commit 32aea33
Show file tree
Hide file tree
Showing 5 changed files with 108 additions and 23 deletions.
15 changes: 3 additions & 12 deletions ironfish-cli/src/commands/wallet/rescan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { ProgressBar } from '../../types'
import { hasUserResponseError } from '../../utils'

export class RescanCommand extends IronfishCommand {
static description = `Rescan the blockchain for transaction`
static description = `Rescan the blockchain for transactions. Clears wallet disk caches before rescanning.`

static flags = {
...RemoteFlags,
Expand All @@ -20,11 +20,6 @@ export class RescanCommand extends IronfishCommand {
description: 'Follow the rescan live, or attach to an already running rescan',
allowNo: true,
}),
reset: Flags.boolean({
default: false,
description:
'Clear the in-memory and disk caches before rescanning. Note that this removes all pending transactions',
}),
local: Flags.boolean({
default: false,
description: 'Force the rescan to not connect via RPC',
Expand All @@ -37,23 +32,19 @@ export class RescanCommand extends IronfishCommand {

async start(): Promise<void> {
const { flags } = await this.parse(RescanCommand)
const { follow, reset, local, from } = flags
const { follow, local, from } = flags

if (local && !follow) {
this.error('You cannot pass both --local and --no-follow')
}

if (from && !reset) {
this.error('When passing --from, you must also pass --reset.')
}

const client = await this.sdk.connectRpc(local)

CliUx.ux.action.start('Asking node to start scanning', undefined, {
stdout: true,
})

const response = client.rescanAccountStream({ reset, follow, from })
const response = client.rescanAccountStream({ follow, from })

const speed = new Meter()

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
{
"wallet/rescanAccount sets account head to one before the request.from sequence": [
{
"header": {
"sequence": 2,
"previousBlockHash": "AD35193FA159CFD7440A3DFDFBE7ACB720CB4A44A441AB933071E2B9EF5DE90B",
"noteCommitment": {
"type": "Buffer",
"data": "base64:q3Sg5nq5F4FBDGLBQA82adp4zfSm46kPFbJUlcSDmAk="
},
"transactionCommitment": {
"type": "Buffer",
"data": "base64:maskm0r6USIh55DSh6onTCysP+MEpqPV0OYLIXgDy5g="
},
"target": "883423532389192164791648750371459257913741948437809479060803100646309888",
"randomness": "0",
"timestamp": 1678897392755,
"graffiti": "0000000000000000000000000000000000000000000000000000000000000000",
"noteSize": 4,
"work": "0"
},
"transactions": [
{
"type": "Buffer",
"data": "base64:AQAAAAAAAAAAAQAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAGzKiP////8AAAAA9spIe9qq9XCnvXKM0mvzp6x81rA1rFKBY7C5UOsSdiOXc6SZeTXwV9RuWmzGZ56WmcQercJu3bMpHzN1TksuEAHV3GufHIgUaCIX++5VCtqvkztByn+s+faxBiJYo0hwOlmFkt0LV7Q4vKW5s8x+/pk+yicC03fVO/nMfsOfGL8DRg4+vKzg4l8iML3l+1elt9DxYyOWdihkpHtI/Bv4gTHMEyNhm5+H9MuOT5nwr2GWcSue36x/P4sXuKNGfJkUE8T0nhFzUDTRt1awmTUkbfUWuQ5pEhrrJofq3BRbHeci3dvUqhAP/do1gENe9+wv1fyQerLLFnkjd4I9pF1VGcLzUWNgltKH4GUBAuVEGFoevB8+iXtaqkOOcE5mMtInro+vJ6oq9jJWBZCbkvaRe9bM/MEh9W39Odc59cvRGN0CYrrTqRcyjEZbKvI7g0HFpVmQ7sKL4/j0ncNcmdRWBooqo8hBP4gMoGk7w7YkiiUVfVpSXCOZsnffo8qXQ/DhP3fACZt3mEaCzr9CqhJD5XFQ1CwewYkfADYyEJDG2GLp2w8VRh3niDlOVWnJCMmgEos/uXt1lfnpW+0/udyykx8aXssLWpPEniWEALNoE5ZFNhpTSJHkJUlyb24gRmlzaCBub3RlIGVuY3J5cHRpb24gbWluZXIga2V5MDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwel9yFFdsdl1imMjiy6n8Ai6eSAP5QiSx3otK5gvWhpSfIarTiu0HNUY0Fwt+gcwiCFE4raymoKjb7BdUCyS8Bw=="
}
]
}
]
}
56 changes: 55 additions & 1 deletion ironfish/src/rpc/routes/wallet/rescanAccount.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
/* eslint-disable @typescript-eslint/no-explicit-any */

import { v4 as uuid } from 'uuid'
import { useMinerBlockFixture } from '../../../testUtilities'
import { createRouteTest } from '../../../testUtilities/routeTest'
import { Account, ScanState } from '../../../wallet'
import { RescanAccountResponse } from './rescanAccount'
Expand Down Expand Up @@ -92,11 +93,64 @@ describe('wallet/rescanAccount', () => {
await routeTest.client
.request<RescanAccountResponse>('wallet/rescanAccount', {
follow: false,
reset: true,
})
.waitForEnd()

expect(reset).toHaveBeenCalledTimes(1)
expect(scanTransactions).toHaveBeenCalledTimes(1)
})

it('sets account head to one before the request.from sequence', async () => {
const reset = jest.spyOn(routeTest.node.wallet, 'reset').mockReturnValue(Promise.resolve())

const chain = routeTest.node.chain

const block2 = await useMinerBlockFixture(chain, 2)

const scanTransactions = jest
.spyOn(routeTest.node.wallet, 'scanTransactions')
.mockReturnValue(Promise.resolve())

const getHeaderAtSequence = jest
.spyOn(routeTest.node.chain, 'getHeaderAtSequence')
.mockReturnValue(Promise.resolve(block2.header))

const updateHead = jest.spyOn(account, 'updateHead').mockReturnValue(Promise.resolve())

await routeTest.client
.request<RescanAccountResponse>('wallet/rescanAccount', {
follow: false,
from: 2,
})
.waitForEnd()

expect(reset).toHaveBeenCalledTimes(1)
expect(getHeaderAtSequence).toHaveBeenCalledWith(2)
expect(updateHead).toHaveBeenCalledWith({
hash: block2.header.previousBlockHash,
sequence: 1,
})
expect(scanTransactions).toHaveBeenCalledTimes(1)
})

it('does not set account head if the request.from sequence is the genesis block', async () => {
const reset = jest.spyOn(routeTest.node.wallet, 'reset').mockReturnValue(Promise.resolve())

const scanTransactions = jest
.spyOn(routeTest.node.wallet, 'scanTransactions')
.mockReturnValue(Promise.resolve())

const updateHead = jest.spyOn(account, 'updateHead').mockReturnValue(Promise.resolve())

await routeTest.client
.request<RescanAccountResponse>('wallet/rescanAccount', {
follow: false,
from: 1,
})
.waitForEnd()

expect(reset).toHaveBeenCalledTimes(1)
expect(updateHead).not.toHaveBeenCalled()
expect(scanTransactions).toHaveBeenCalledTimes(1)
})
})
28 changes: 19 additions & 9 deletions ironfish/src/rpc/routes/wallet/rescanAccount.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,17 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at https://mozilla.org/MPL/2.0/. */
import * as yup from 'yup'
import { GENESIS_BLOCK_SEQUENCE } from '../../../primitives'
import { ValidationError } from '../../adapters/errors'
import { ApiNamespace, router } from '../router'

export type RescanAccountRequest = { follow?: boolean; from?: number; reset?: boolean }
export type RescanAccountRequest = { follow?: boolean; from?: number }
export type RescanAccountResponse = { sequence: number; startedAt: number; endSequence: number }

export const RescanAccountRequestSchema: yup.ObjectSchema<RescanAccountRequest> = yup
.object({
follow: yup.boolean().optional(),
from: yup.number().optional(),
reset: yup.boolean().optional(),
})
.defined()

Expand All @@ -39,15 +39,25 @@ router.register<typeof RescanAccountRequestSchema, RescanAccountResponse>(
await node.wallet.updateHeadState.abort()
}

if (request.data.reset) {
await node.wallet.reset()
}
await node.wallet.reset()

let fromHash = undefined
if (request.data.from) {
const blockHash = await node.chain.getHashAtSequence(request.data.from)
if (blockHash) {
fromHash = blockHash
if (request.data.from && request.data.from > GENESIS_BLOCK_SEQUENCE) {
const header = await node.chain.getHeaderAtSequence(request.data.from)

if (header === null) {
throw new ValidationError(
`No block header found in the chain at sequence ${request.data.from}`,
)
}

fromHash = header.hash

for (const account of node.wallet.listAccounts()) {
await account.updateHead({
hash: header.previousBlockHash,
sequence: header.sequence - 1,
})
}
}

Expand Down
2 changes: 1 addition & 1 deletion ironfish/src/wallet/wallet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -577,7 +577,7 @@ export class Wallet {
}

this.logger.info(
`Scan starting from earliest found account head hash: ${beginHash.toString('hex')}`,
`Scan starting from block ${beginHash.toString('hex')} (${beginHeader.sequence})`,
)

// Go through every transaction in the chain and add notes that we can decrypt
Expand Down

0 comments on commit 32aea33

Please sign in to comment.