WalletScanner skips accounts with createdAt until createdAt sequence is reached #5652
Conversation
danield9tqh
changed the title
danield9tqh
changed the title
danield9tqh
force-pushed
the
dont-scan-empty-accounts-with-birthday
branch
from
79100c2 to
6c2f87a
Compare
| for (const { scanFrom: head } of this.scanningAccounts) { | ||
| if (!head) { | ||
| return null | ||
| private async getEarliestHead(): Promise<HeadValue | null | 'none'> { |
There was a problem hiding this comment.
Does 'none' mean that all of the scanFroms are ahead of the chain head?
There was a problem hiding this comment.
Would it work to return the chain head in that case instead of the special 'none' value?
There was a problem hiding this comment.
I think its either
- earliest is ahead of the chain
- there are no scanning accounts
There was a problem hiding this comment.
I'm not sure if returning the head would work for this case where we break https://github.com/iron-fish/ironfish/pull/5652/files#diff-4a43e4a9c484bea01d7248d51bc40948e7bbdd13c5bef3fdbfe0a02c7210f85dR138. If we set chain processor to head of the chain then maybe that's not the same as the chain processor's head (because it was set previously)
Idk its a good thought. I think I'll keep it as none for now though so don't have to think through / refactor that break statement
| private scanningAccounts = new Array<{ account: Account; scanFrom: HeadValue | null }>() | ||
| private scanningAccounts = new Array<{ | ||
| account: Account | ||
| scanFrom: { sequence: number; hash?: Buffer } | 'cursor' |
There was a problem hiding this comment.
Does 'cursor' mean that scanFrom is equal to the head of the wallet scanner?
There was a problem hiding this comment.
yea, any thoughts on better name?
There was a problem hiding this comment.
I think cursor is good, but it does make me wonder whether we should have an explicit head value on the walletScanner that we set and compare these to
Mostly I'm worried that we'll forget what these special string values mean 😅
Summary
When an account has the following
createdAt: 10
head: null
the wallet scanner should not take any action on this account until it reaches block 10. Currently the wallet scanner will still connect blocks 1-10 (without decrypting transactions) but it still takes some time and it doesn't provide any value over just connecting block 10 directly
Testing Plan
Unit tests + running local wallet tests
Documentation
Does this change require any updates to the Iron Fish Docs (ex. the RPC API
Reference)? If yes, link a
related documentation pull request for the website.
Breaking Change
Is this a breaking change? If yes, add notes below on why this is breaking and label it with
breaking-change-rpcorbreaking-change-sdk.