Skip to content

Birthday#243

Merged
cygnet3 merged 13 commits intodevfrom
birthday
Mar 1, 2026
Merged

Birthday#243
cygnet3 merged 13 commits intodevfrom
birthday

Conversation

@Sosthene00
Copy link
Collaborator

  • Give user a date for birthday on seed screen
  • Ask user for a birthday on recover
  • We're using the blockheight the wallet was created in and take the day that block was mined at 12am as an easy to remember proxy for the user. At worst we will scan a whole day worth of data for nothing.

@Sosthene00 Sosthene00 force-pushed the birthday branch 3 times, most recently from 574a00c to a6ab3fe Compare February 5, 2026 09:09
@Sosthene00
Copy link
Collaborator Author

Sosthene00 commented Feb 5, 2026

Lots of improvements to handle offline wallet creation, I think that's an interesting feature to have, and no matter what it's also useful if services like mempool or blindbit or nameserver are temporarily down.

Basically the idea is that user can create a new wallet while having 0 network, but will get stuck at the dana address register step with an offline screen that just let him share payment code with a qr code and copy paste it so that it can still be paid on the spot. As soon as network's back normal flow resume.

@Sosthene00 Sosthene00 force-pushed the birthday branch 3 times, most recently from 530885f to 88a7535 Compare February 11, 2026 21:19
cygnet3 added a commit that referenced this pull request Feb 18, 2026
This was residue from back when we used to store the json-serialized
SpWallet struct in the flutter secure storage directly. Having this
makes #243 more difficult, so we remove it here.
cygnet3 added a commit that referenced this pull request Feb 18, 2026
This was residue from back when we used to store the json-serialized
SpWallet struct in the flutter secure storage directly. Having this
makes #243 more difficult, so we remove it here.
cygnet3 added a commit that referenced this pull request Feb 19, 2026
This was residue from back when we used to store the json-serialized
SpWallet struct in the flutter secure storage directly. Having this
makes #243 more difficult, so we remove it here.
@Sosthene00 Sosthene00 force-pushed the birthday branch 5 times, most recently from 7ddaa33 to d91fb3d Compare February 23, 2026 15:45
@cygnet3
Copy link
Owner

cygnet3 commented Feb 24, 2026

Two general comments:

  1. I think using lockTimeThreshold is a clever solution to allow storing both the block height as well as a DateTime in a single int, but I would prefer to just store them separately to make it more explicit they are different things:
const String _keyBirthdayLegacy = "birthday";
const String _keyBirthdayTimestamp = "birthdaytimestamp";
  1. Looks like the WalletBackup struct currently requires the birthday block height. I think we should make this optional first, probably in a separate PR

I will try to work on these points tomorrow

@Sosthene00
Copy link
Collaborator Author

Two general comments:

1. I think using `lockTimeThreshold` is a clever solution to allow storing both the block height as well as a DateTime in a single `int`, but I would prefer to just store them separately to make it more explicit they are different things:
const String _keyBirthdayLegacy = "birthday";
const String _keyBirthdayTimestamp = "birthdaytimestamp";
2. Looks like the `WalletBackup` struct currently requires the birthday block height. I think we should make this optional first, probably in a separate PR

I will try to work on these points tomorrow

  1. the point was to not pollute the code for ever with a key that has been in used for a very short time, and it kind of squat "birthday" and force us to use another key like "birthdaytimestamp" which is a bit ugly.

It may be a bit more readable now though.

  1. I'm perfectly aware that backup is not satisfactory at all, but indeed I think it needs its own PR, there's a lot more to fix about it.

As for the other comments I agree with it and will make the necessary changes.

@Sosthene00
Copy link
Collaborator Author

Addressed your comments, except for having a separate key in storage and a separate getter for legacy birthday stored as block height, it was easier for me to keep things as they are now. What I do is that I still convert a block height into a DateTime, obviously a block height will give a Date long in the past, close to beginning of epoch time. If that date is before defaultBirthday, then it is interpreted as a block height.

Hardcode a default `DateTime` for all network, 1st June of 2025 for now.
* `getBlockForHash`: needed to retrieve the timestamp of a block
* `getBlockHashForHeight`: retrieve the block hash from its height
* `getBlockFromTimestamp`: used to get the height of the closest block from a timestamp
Add `toSeconds()` method to `DateTime` and a `toDate()` method to `int`
Birthday becomes a DateTime (stored as unix timestamp) instead of a block height integer. Includes migration logic for existing wallets that still have a block height stored.
* add a ticker box on Seed Phrase Recovery screen
* if ticked open birthday picker
* if user does'nt provice a birthday set network default
Minimum allowed birthday can be used for two purposes:

- In date picker screen, to set a sensible minimum
- During wallet recovery, we parse Datetimes before this timestamp as a
legacy birthday (block height)
@Sosthene00
Copy link
Collaborator Author

2c55de4: ACK, if I really wanted to nit I'd say why not choose Dana creation as minimum birthday too, but nevermind
300e7aa: I was a bit reluctant to do that, because users that upgrade and can't get the mempool api for some reasons will end up with a degraded experience. Otoh I agree it was adding lasting complexity just for a edge case. Actually just upgrading will probably be fine because you still have the lastScan, but maybe we should just let user know ("Some Error occured and your wallet birthday was reset to 1st june of 2025, you can change it in the settings")
314656a: Most of those changes are simply consequence of previous commit, as for adding the lastScan argument on setupWallet() it's only used in 2 places and one will always be null and let synchronization service do the initialization anyway... But I guess it makes wallet creation better so let's go

@cygnet3
Copy link
Owner

cygnet3 commented Feb 28, 2026

300e7aa: I was a bit reluctant to do that, because users that upgrade and can't get the mempool api for some reasons will end up with a degraded experience. Otoh I agree it was adding lasting complexity just for a edge case. Actually just upgrading will probably be fine because you still have the lastScan, but maybe we should just let user know ("Some Error occured and your wallet birthday was reset to 1st june of 2025, you can change it in the settings")

Thinking about this some more, I propose that we actually do allow birthday to be null, and treat this as the 'I don't know my wallet birthday' case. So we don't write the default birthday to storage either. That change will also require the walletBackup birthday to be nullable, so probably best to do separate from this PR (but before next release).

@Sosthene00
Copy link
Collaborator Author

300e7aa: I was a bit reluctant to do that, because users that upgrade and can't get the mempool api for some reasons will end up with a degraded experience. Otoh I agree it was adding lasting complexity just for a edge case. Actually just upgrading will probably be fine because you still have the lastScan, but maybe we should just let user know ("Some Error occured and your wallet birthday was reset to 1st june of 2025, you can change it in the settings")

Thinking about this some more, I propose that we actually do allow birthday to be null, and treat this as the 'I don't know my wallet birthday' case. So we don't write the default birthday to storage either. That change will also require the walletBackup birthday to be nullable, so probably best to do separate from this PR (but before next release).

Let's keep it simple and move on, that's a very limited edge case and fixable if user can change birthday.

cygnet3 and others added 3 commits March 1, 2026 01:02
Make birthday variable in WalletState non-nullable.
In case we can't get the birthday variable, use a default birthday. This
only happens when wallets upgrade from legacy birthday format (using
block height) and there's a connection error.
When creating a new wallet, set lastScan to current block height.
Before, this would be lazily initialized by the chain synchronization
service, but I think setting it immediately on wallet creation is
preferable.
@cygnet3 cygnet3 merged commit a24b501 into dev Mar 1, 2026
@cygnet3 cygnet3 deleted the birthday branch March 1, 2026 00:11
cygnet3 added a commit that referenced this pull request Mar 1, 2026
This was residue from back when we used to store the json-serialized
SpWallet struct in the flutter secure storage directly. Having this
makes #243 more difficult, so we remove it here.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants