wallet: fix missing tx when rescan with filter update

[chikeichan]

CHANGE:

• add migration-id 1 to change lookahead value from uint8 to uint32
• when either changeDepth or receiveDepth exceed current lookahead value during rescan, increment lookahead value by 20, and then send updated filter and block height to to node/http.js
• when filter is updated during rescan, node/http will halt the current rescan and start a new one with the new filter from the new height

Summary

This borrow heavily from this issue in bcoin but with a simplified approach in node/http.

This version of hsd and rescan functionality can be tested in this branch in Bob. I have tested this with a wallet with 5k+ transactions and lookahead value at 3k+ and have no issue.

[pinheadmz]

Are you sure we need all this work on the lookahead value?
I was hoping that if we merge #506 (bump lookahead to 200, still within the U8 value) and the filter update commit from bcoin we'd basically be done with this. There is a test as well in this other commit: bcoin-org/bcoin@46967f8

Unfortunately my open source time for hsd is very limited for now. If you or someone at Kyokan has the bandwidth to try this suggestion I think it'll be much cleaner and easier to get community approval.

[tynes]

It would be nice to have some tests and also squash the commits so that they follow the conventions of the codebase

[tynes]

Skipping this on purpose?

[chikeichan]

Are you sure we need all this work on the lookahead value?
I was hoping that if we merge #506 (bump lookahead to 200, still within the U8 value) and the filter update commit from bcoin we'd basically be done with this. There is a test as well in this other commit: bcoin-org/bcoin@46967f8

I see what you mean now - after some digging i believe there is a bug in the syncDepth method where some new account keys are not being saved. To workaround, I basically was just calling setLookahead with account depth to force account keys to be generated. This has since been modified to match closer to the bcoin commit.

Let me try to fix the bug in syncDepth instead -- that should allow us to leave lookahead value unchanged.

[chikeichan]

Are you sure we need all this work on the lookahead value?

So after more digging, I can confirm that there is at least one user wallet with a lookahead value of 500, testing using the rescan filter fix in this PR + hardcoding lookahead value.

@pinheadmz This issue from you helped lot! I will open a separate PR for lookahead value migration and see if there are any community support for it. For Bob, I will just maintain a simple fork with a hardcode lookahead value of 1000 for now so that lookahead datatype stays the same.

@tynes squashed and added unit test!

However, I am having trouble getting them to run locally. The tests seems to be stucked when it tries to generate blocks. Is that a common issue when running test?

[pinheadmz]

This might need to be changed to HSD_WALLET_HTTP_PORT :-)

[pinheadmz]

I think in hsd the coinbase maturity for regtest is much smaller so we can reduce this here.

hsd/lib/protocol/networks.js

Line 745 in c85d9b4

regtest.coinbaseMaturity = 2;

[pinheadmz]

note, Braydon doesn't like to be tagged in Handshake-org stuff but this is mostly his work, I'm not sure if we need to be explicit about the license or anything but you might consider just adding his name to the commit message.

[pinheadmz]

@chikeichan finally getting to more deeply review this. I'm playing with the test locally, converting it more hsd-friendly parameters etc. Looks like the main patch isn't just Braydon's commit but some extra work as well, including calling rollback() -- were you able to solve the issue here? Can you provide a test script with bash or anything to evaluate the changes? This issue is going to be my next top priority, I may start a new branch with my modified test and Braydon's bcoin commit and then add back in some of your changes.

[chikeichan]

hello @pinheadmz - nice to see you active again!

For this PR, the general changes to scan is:

• wallet client will call setFilter with current height
• wallet node receive set filter in the socket event
• wallet node stop current scan progress (kinda hacky now by throwing an error)
• wallet db rollback to current height minus 1
• wallet node restart new scan from new filter height minus 1

I do not have test for this changes unfortunately; I have been using my personal wallet with a lookahead value of around 500 to test during development.

I am more than happy to review new branch + merge back to Bob when it's ready!