wallet: Add birthday. #2347

JoeGruffins · 2024-04-20T06:49:07Z

closes #2346
closes #320

JoeGruffins · 2024-04-20T07:10:31Z

wallet/udb/txmined.go

+//
+//	[0:32]  Block header hash (32 bytes)
+//	[32:40] Birthday time (8 bytes)
+func (s *Store) SetBirthday(dbtx walletdb.ReadWriteTx, birthday *time.Time) (*chainhash.Hash, error) {


This looks like a mess, so if there is anything simpler to do, please make a suggestion. It goes backwards using an approximation based on the amount of time to the birthday compared to the lifetime of the chain. Then it goes forward one at a time until it goes past the time we want and uses the header before that.

I really think the birthday should internally be just "birthblock" (height+hash), and then that can be specified and recorded precisely.

You can then have multiple ways of specifying the birthblock when creating the wallet: by height or hash directly, by timestamp (like you do here), by "decred day" (like aezeed does) or any other method.

As you notice, you'll need two sets of states: the user-specified birthday, set when invoking with --create (e.g. --birthtimestamp, --birthheight, etc) and then a separate state that records the actual determined birthday hash and height (because you can't determine the hash before sync and sync may take an arbitrary amount of time to complete).

Instead of doing this process here, this can probably (haven't thought this through completely) be done inside the main db tx of wallet.ChainSwitch(): if the birthblock hash is not set, then go through the logic of determining the birthday within the set of the batch of headers being processed. This has the advantage of working both for SPV and RPC sync modes, avoids going through this process multiple times and meshes perfectly with the initial sync process, such that by the end of the initial sync, the birthblock will be set and ready to be used as the initial rescan point.

Thank you! Doing what you said in wallet.ChainSwitch() I think. Please look again, the changes also https://github.com/decred/dcrwallet/compare/e52f2f26d5e210dde5d0b1c01aa1b8a46f4d4099..80145bce22dd5604c93e2c6e25cd5c72a0063885

Now allowing the consumer to set the intent to set a birthday, so two separate values in the db now.

If this looks ok, I guess we still want to use the birthday for rescans with default values, so I will add that if I can.

spv/sync.go

matheusd · 2024-04-22T11:40:32Z

wallet/udb/txmined.go

+//
+//	[0:32]  Block header hash (32 bytes)
+//	[32:40] Birthday time (8 bytes)
+func (s *Store) SetBirthday(dbtx walletdb.ReadWriteTx, birthday *time.Time) (*chainhash.Hash, error) {


I really think the birthday should internally be just "birthblock" (height+hash), and then that can be specified and recorded precisely.

You can then have multiple ways of specifying the birthblock when creating the wallet: by height or hash directly, by timestamp (like you do here), by "decred day" (like aezeed does) or any other method.

As you notice, you'll need two sets of states: the user-specified birthday, set when invoking with --create (e.g. --birthtimestamp, --birthheight, etc) and then a separate state that records the actual determined birthday hash and height (because you can't determine the hash before sync and sync may take an arbitrary amount of time to complete).

Instead of doing this process here, this can probably (haven't thought this through completely) be done inside the main db tx of wallet.ChainSwitch(): if the birthblock hash is not set, then go through the logic of determining the birthday within the set of the batch of headers being processed. This has the advantage of working both for SPV and RPC sync modes, avoids going through this process multiple times and meshes perfectly with the initial sync process, such that by the end of the initial sync, the birthblock will be set and ready to be used as the initial rescan point.

walletsetup.go

wallet/udb/txdb.go

wallet/udb/txmined.go

JoeGruffins · 2024-04-24T06:05:44Z

I'm not sure if I need to do anything for the rescans. They will start from zero by default but you can set a block header or height. Should the birthday info be included in walletinfo or a new rpc call to help there?

matheusd · 2024-04-24T11:00:22Z

internal/prompt/prompt.go

+// Birthday prompts for a wallet birthday.
+func Birthday(reader *bufio.Reader) (*time.Time, error) {
+	for {
+		fmt.Printf("Do you have a wallet birthday we should rescan from? (enter date as YYYY-MM-DD, or 'no') [no]: ")


Personally, I'd also add support to accept a block height here, so the user can specify either a date or the exact block height, but this should have input from @jrick to double check.

this seems reasonable, although it's not too difficult to look up a similar date using the block explorer for whatever height you want to use.

the prompt could accept either input, if it's an integer, use block height, if it can be parsed as a date, use that instead.

wallet/udb/txmined.go

walletsetup.go

matheusd · 2024-04-24T12:22:16Z

I'm not sure if I need to do anything for the rescans. They will start from zero by default but you can set a block header or height. Should the birthday info be included in walletinfo or a new rpc call to help there?

Yeah, I'm pretty sure you want both account/address discovery and rescans to start from the birthday (when it is set) for performance reasons, unless a specific block height/hash has been specified on a particular rescan call.

JoeGruffins · 2024-04-25T07:45:51Z

Yeah, I'm pretty sure you want both account/address discovery and rescans to start from the birthday (when it is set) for performance reasons, unless a specific block height/hash has been specified on a particular rescan call.

Hard to do this without changing a lot I think. Default height is zero so there's not a way to differentiate between default height and if a user really wants to rescan from zero. They may want to rescan before the birthday for example if some data was imported into the wallet.

I guess we can have -1 be a special case? Would that be ok?

JoeGruffins · 2024-04-25T08:16:09Z

https://github.com/decred/dcrwallet/compare/3da2ec7844f042ca78d71eee63219cbc7c08ba03..1486597870ae26b392ded4fd46648f96e8c3db10

@matheusd Just one birthstate now. Added the 24 hour buffer. It is missing a lot of comments.

Still unsure:

Is this better in ChainSwitch or done inside sync (chain/spv) during the initial sync?
Do we need to change up the rescan rpc/grpc requests to allow a "default" alongside allowing from zero? imo giving the user a way to see the birth block is enough, then one can rescan from the birth block after they know it.

wallet/chainntfns.go

walletsetup.go

matheusd · 2024-04-26T10:43:15Z

wallet/chainntfns.go

+					// In the case that the birthday will not be reached,
+					// store a hash for initial sync to start from. Do not
+					// store the tip hash as that will cause w.rescanPoint
+					// to return nil.
+					bh := n.Header.PrevBlock
+					if err := w.txStore.UpdateProcessedTxsBlockMarker(dbtx, &bh); err != nil {
+						return err
+					}


I'm not sure you need this case. At this point in the code, you know the birthday will bet set at some point in the future (in which case one of the prior branches will be reached), so what's the need to do this for every batch of blocks until you reach the birthday height?

If the birthday is set in the future then, although we don't need to, we will scan from genesis.

I will remove the check anyway though since this is user error I suppose. If the birthday is set to a time or block in the future, it is not fatal.

wallet/chainntfns.go

JoeGruffins · 2024-04-29T04:46:45Z

I guess I will try adding the birthblock to walletinfo? imo that is enough, but we can do more... now or later.

JoeGruffins · 2024-04-29T10:40:00Z

~~I haven't had a chance to test with the last changes so converted to draft temporarily.~~

Added allowing restoring from block to the prompt. Moved the birthday verification there. Added methods for rpc and grpc to see the birthday https://github.com/decred/dcrwallet/compare/1486597870ae26b392ded4fd46648f96e8c3db10..9449b2c2fe8fb3ce846b87f891209f1b0a8dc7c5

matheusd · 2024-05-02T18:19:48Z

Is this ready for testing or are you planning on adding any other features to this? Asking so I can start functional testing on it...

JoeGruffins · 2024-05-03T01:51:50Z

Is this ready for testing or are you planning on adding any other features to this? Asking so I can start functional testing on it...

Yes it is ready. I don't plan to add anything unless requested.

JoeGruffins · 2024-05-03T01:53:37Z

I could add a unit test for the db functions SetBirthState and BirthState. I have one half made.

matheusd

Functional testing worked. Only have a couple more nits on the code, and then a few more general questions:

Thoughts on adding a db migration to add the birthday state to existing wallets? This would ensure every wallet has the birthday state (with existing wallets pointing to genesis)
Just to double check, but additional rescanwallet with no arguments will NOT rely on the birthday to perform the rescan, right? It will be up to the users to call walletinfo and then use the birthblock as argument to ensure the rescan only goes through blocks after the birthblock.
We should probably modify the account and address discovery procedures in SPV mode to use blocks only after the birthblock. This would make these procedures significantly faster, specially for new wallets (which will have a very recent birthblock). The relevant place to start is here

rpc/api.proto

wallet/udb/txmined.go

JoeGruffins · 2024-05-06T01:12:44Z

Just to double check, but additional rescanwallet with no arguments will NOT rely on the birthday to perform the rescan, right? It will be up to the users to call walletinfo and then use the birthblock as argument to ensure the rescan only goes through blocks after the birthblock.

Yes that keeps this pr simple. I think this work can still be done though if necessary. As I said I think this will require changes to both the grpc and rpc api to distinguish between a rescan from the height 0 and from birthday.

JoeGruffins · 2024-05-06T02:02:17Z

Just rebased.

JoeGruffins · 2024-05-06T09:51:07Z

Added a db upgrade https://github.com/decred/dcrwallet/compare/475cabd342ed69e40b9b31831a187837c24358f3..70da522eb5cb5163e44d3091a217c0bde51f9804

I changed that one spot for spv wallets that you showed and looking if there are others... Are there other spots that need changing in chainntfs.go?

wallet/udb/txmined_test.go

matheusd · 2024-05-06T11:30:55Z

This is looking pretty good:

Time to discover accounts and addresses (mainnet, empty wallet)
Without birthday: 1m38s
   With birthday: 315ms

One more thing I remembered is adding the birthday ({time,height}) to the CreateWallet gRPC endpoint. This will then allow me to pass this info from dcrlnd when creating new wallets from there.

wallet/chainntfns.go

JoeGruffins · 2024-05-07T03:09:23Z

Add time and height params to the createwallet grpc request https://github.com/decred/dcrwallet/compare/70da522eb5cb5163e44d3091a217c0bde51f9804..47e49d626db7d68ed51d51fc409e04fa5bd66116

Also change the main if block to exclude block zero although it seems to always start from block one anyhow

internal/prompt/prompt.go

JoeGruffins · 2024-05-08T01:35:12Z

Using time.Since https://github.com/decred/dcrwallet/compare/df07037476bc1978502d9d67857b8d2feb632beb..2d9989e86ae7c4c52d11ef3f3ee15c9feb88a846

matheusd

LGTM!

Nice work!

internal/rpc/rpcserver/server.go

JoeGruffins · 2024-05-10T01:34:27Z

Avoid possible panic https://github.com/decred/dcrwallet/compare/2d9989e86ae7c4c52d11ef3f3ee15c9feb88a846..32e15c4cd02a3b89545cae29a29904cc69aab177

Add a birthday state to the database than can be used when creating a new wallet to restore from a certain date or time.

jrick

great work

JoeGruffins force-pushed the addbirthday branch 2 times, most recently from 592825b to e52f2f2 Compare April 20, 2024 07:10

JoeGruffins commented Apr 20, 2024

View reviewed changes

matheusd reviewed Apr 22, 2024

View reviewed changes

JoeGruffins force-pushed the addbirthday branch from e52f2f2 to 80145bc Compare April 23, 2024 10:49

JoeGruffins commented Apr 23, 2024

View reviewed changes

walletsetup.go Outdated Show resolved Hide resolved

JoeGruffins commented Apr 23, 2024

View reviewed changes

wallet/udb/txdb.go Outdated Show resolved Hide resolved

JoeGruffins commented Apr 23, 2024

View reviewed changes

wallet/udb/txmined.go Outdated Show resolved Hide resolved

JoeGruffins force-pushed the addbirthday branch 2 times, most recently from aa31212 to 3da2ec7 Compare April 24, 2024 06:03

JoeGruffins marked this pull request as ready for review April 24, 2024 06:04

matheusd reviewed Apr 24, 2024

View reviewed changes

JoeGruffins force-pushed the addbirthday branch from 3da2ec7 to 1486597 Compare April 25, 2024 08:00

matheusd reviewed Apr 26, 2024

View reviewed changes

JoeGruffins force-pushed the addbirthday branch from 1486597 to 9449b2c Compare April 29, 2024 10:37

JoeGruffins marked this pull request as draft April 29, 2024 10:37

JoeGruffins force-pushed the addbirthday branch 3 times, most recently from db16f9f to c1e6035 Compare April 30, 2024 03:33

JoeGruffins marked this pull request as ready for review April 30, 2024 03:34

matheusd reviewed May 3, 2024

View reviewed changes

rpc/api.proto Outdated Show resolved Hide resolved

wallet/udb/txmined.go Outdated Show resolved Hide resolved

wallet/udb/txmined.go Outdated Show resolved Hide resolved

wallet/udb/txmined.go Outdated Show resolved Hide resolved

wallet/udb/txmined.go Outdated Show resolved Hide resolved

JoeGruffins force-pushed the addbirthday branch from c1e6035 to 475cabd Compare May 6, 2024 02:02

JoeGruffins force-pushed the addbirthday branch from 475cabd to 70da522 Compare May 6, 2024 09:49

JoeGruffins commented May 6, 2024

View reviewed changes

wallet/udb/txmined_test.go Outdated Show resolved Hide resolved

JoeGruffins commented May 7, 2024

View reviewed changes

wallet/chainntfns.go Show resolved Hide resolved

JoeGruffins force-pushed the addbirthday branch from 70da522 to 47e49d6 Compare May 7, 2024 03:07

JoeGruffins force-pushed the addbirthday branch from 47e49d6 to df07037 Compare May 7, 2024 03:17

JoeGruffins commented May 7, 2024

View reviewed changes

internal/prompt/prompt.go Outdated Show resolved Hide resolved

JoeGruffins force-pushed the addbirthday branch from df07037 to 2d9989e Compare May 8, 2024 01:34

matheusd approved these changes May 9, 2024

View reviewed changes

JoeGruffins commented May 10, 2024

View reviewed changes

internal/rpc/rpcserver/server.go Outdated Show resolved Hide resolved

JoeGruffins force-pushed the addbirthday branch from 2d9989e to d7007a6 Compare May 10, 2024 01:33

wallet: Add birthday.

32e15c4

Add a birthday state to the database than can be used when creating a new wallet to restore from a certain date or time.

JoeGruffins force-pushed the addbirthday branch from d7007a6 to 32e15c4 Compare May 10, 2024 01:35

jrick approved these changes May 13, 2024

View reviewed changes

jrick merged commit 5e8f132 into decred:master May 13, 2024
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

wallet: Add birthday. #2347

wallet: Add birthday. #2347

JoeGruffins commented Apr 20, 2024 •

edited

JoeGruffins Apr 20, 2024

matheusd Apr 22, 2024

JoeGruffins Apr 23, 2024

matheusd Apr 22, 2024

JoeGruffins commented Apr 24, 2024

matheusd Apr 24, 2024

jrick Apr 26, 2024

matheusd commented Apr 24, 2024

JoeGruffins commented Apr 25, 2024

JoeGruffins commented Apr 25, 2024

matheusd Apr 26, 2024

JoeGruffins Apr 29, 2024

JoeGruffins commented Apr 29, 2024 •

edited

JoeGruffins commented Apr 29, 2024 •

edited

matheusd commented May 2, 2024

JoeGruffins commented May 3, 2024

JoeGruffins commented May 3, 2024

matheusd left a comment

JoeGruffins commented May 6, 2024

JoeGruffins commented May 6, 2024

JoeGruffins commented May 6, 2024

matheusd commented May 6, 2024

JoeGruffins commented May 7, 2024

JoeGruffins commented May 8, 2024

matheusd left a comment

JoeGruffins commented May 10, 2024 •

edited

jrick left a comment

wallet: Add birthday. #2347

wallet: Add birthday. #2347

Conversation

JoeGruffins commented Apr 20, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JoeGruffins commented Apr 24, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matheusd commented Apr 24, 2024

JoeGruffins commented Apr 25, 2024

JoeGruffins commented Apr 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JoeGruffins commented Apr 29, 2024 • edited

JoeGruffins commented Apr 29, 2024 • edited

matheusd commented May 2, 2024

JoeGruffins commented May 3, 2024

JoeGruffins commented May 3, 2024

matheusd left a comment

Choose a reason for hiding this comment

JoeGruffins commented May 6, 2024

JoeGruffins commented May 6, 2024

JoeGruffins commented May 6, 2024

matheusd commented May 6, 2024

JoeGruffins commented May 7, 2024

JoeGruffins commented May 8, 2024

matheusd left a comment

Choose a reason for hiding this comment

JoeGruffins commented May 10, 2024 • edited

jrick left a comment

Choose a reason for hiding this comment

JoeGruffins commented Apr 20, 2024 •

edited

JoeGruffins commented Apr 29, 2024 •

edited

JoeGruffins commented Apr 29, 2024 •

edited

JoeGruffins commented May 10, 2024 •

edited