search

iron-fish / ironfish

A novel cryptocurrency focused on privacy and accessibility.
https://ironfish.network
Mozilla Public License 2.0
964 stars 577 forks source link

avoids resetting account birthday #5036

Closed hughy closed 3 weeks ago

hughy commented 4 weeks ago

Summary

an account's 'createdAt' field contains the sequence and hash of the block at which that account was created. we use it to skip decryption of notes during rescans on blocks that came before the 'createdAt' block

we check that the 'createdAt' block is in the chain in several places to protect against block references that may be from other networks or from pruned forks. however, it should only be necessary to check for the account birthday when the account is imported, and not when the wallet starts or in the middle of a scan.

by reducing the number of places that we might reset an account's 'createdAt' we can reduce the sources of error from resetting it to null

Testing Plan

unit 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.

[ ] Yes

Breaking Change

Is this a breaking change? If yes, add notes below on why this is breaking and label it with breaking-change-rpc or breaking-change-sdk.

[ ] Yes
danield9tqh commented 4 weeks ago

One edge case I can think of. created at exists in the chain on import but then there is a re-org away from that account birthday before the account is synced. Very rare but still possible. Maybe we could keep the check inside shouldDecryptForAccount but remove it from start still

hughy commented 4 weeks ago

One edge case I can think of. created at exists in the chain on import but then there is a re-org away from that account birthday before the account is synced. Very rare but still possible. Maybe we could keep the check inside shouldDecryptForAccount but remove it from start still

I think the birthday sequence will still almost always be reliable in that case -- there would need to be a re-org away from the account birthday and the account would need to receive a transaction at an earlier sequence than the birthday during the reorg.

I think using sequence - confirmations will almost entirely fix this, but that will probably require a migration of existing birthdays.