fix: set wallet start scan height to birthday and not 0 (see issue #4807) #4911
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
SWvheerden
requested changes
Nov 14, 2022
base_layer/wallet/src/utxo_scanner_service/utxo_scanner_task.rs
Outdated
Show resolved
Hide resolved
AaronFeickert
requested changes
Nov 15, 2022
base_layer/wallet/src/utxo_scanner_service/utxo_scanner_task.rs
Outdated
Show resolved
Hide resolved
jorgeantonio21
force-pushed
the
ja-wallet-scans-at-birthday-not-height-0
branch
from
f439d24
to
b27c1da
Compare
jorgeantonio21
requested review from
SWvheerden,
AaronFeickert and
stringhandler
and removed request for
AaronFeickert
SWvheerden
previously approved these changes
Nov 15, 2022
base_layer/wallet/src/utxo_scanner_service/utxo_scanner_task.rs
Outdated
Show resolved
Hide resolved
AaronFeickert
previously approved these changes
Nov 15, 2022
There was a problem hiding this comment.
Just a few nits for clarity, particularly because of unit conversions between the CipherSeed birthday and Unix time.
Are there tests that exercise the expected behavior? Namely:
- a birthday corresponding to a block that is not the genesis block
- a birthday corresponding to the genesis block
If not, recommend adding them if feasible.
Otherwise, did not specifically test, but the changes to the math appear to be doing what you want.
base_layer/wallet/src/utxo_scanner_service/utxo_scanner_task.rs
Outdated
Show resolved
Hide resolved
|
Any reason for the large changes to |
|
May be useful to include a couple of unit tests to hit the saturating subtraction cases. |
jorgeantonio21
dismissed stale reviews from AaronFeickert and SWvheerden
via
f57c801
jorgeantonio21
force-pushed
the
ja-wallet-scans-at-birthday-not-height-0
branch
from
aaab208
to
f57c801
Compare
|
Added unit tests and documentation that convey, more precisely, the points made by @AaronFeickert. I tried to make unit tests to be as expressive as possible. In particular, we use hardcoded values for second duration from specific dates. Also added a test on a mocked chain to check consistency of birthday and block timestamps, altogether. |
stringhandler
approved these changes
Nov 16, 2022
stringhandler
added a commit
that referenced
this pull request
Nov 17, 2022
* fix: updates for SafePassword API change (#4927) Description --- Minor updates for a corresponding [change](tari-project/tari_utilities#52) to the `SafePassword` API. Motivation and Context --- A pending change in `tari_utilities` to the handling of sensitive data includes a new implementation of `SafePassword`. One change removes derived traits for equality testing in favor of equality testing on references to the underlying passphrase data. This work makes the minor but necessary changes to address this API change. How Has This Been Tested? --- Tests pass after applying the linked `tari_utilities` PR. Note that tests will pass using the current `SafePassword` API as well, as the dependency change PR restricts the API. * chore: remove unused methods (#4922) Delete some unused methods * v0.40.0 * fix: set wallet start scan height to birthday and not 0 (see issue #4807) (#4911) Description --- When wallet starts new UTXO scanning service it should start at the birthday. Currently, it starts at height 0. This PR addresses this issue Motivation and Context --- Addresses #4807. How Has This Been Tested? --- * chore: fix naming of ffi functions and comments (#4930) Description --- Fixing the comments of one function in the wallet FFI. Fixes the names of 4 functions in the wallet FFI. * chore: fix depreciated timestamp clippy (#4929) Description --- Fixes depreciated timestamp warnings * feat: implement validator node lmdb store * feat: add stateless vn reg utxo validation rules and merkle root * rename vn validaty period consensus constant * use commitment instead of block hash so that lmdb uses canonical ordering * DRY up vn_mmr calculation, add vn reg utxo validations * fix clippies * wallet: use amount from consensus constants for VN reg * adds validator node signature to common types This is to allow the validator node to use the same signature * remove commitment from signature for now * Remove unneeded validator node kernel feature * ad epoch_length to grpc consensus constants * fix epoch interval check and add test * reduce target time for igor * clippy * change shard key hash label Co-authored-by: Aaron Feickert <[email protected]> Co-authored-by: stringhandler <[email protected]> Co-authored-by: jorgeantonio21 <[email protected]> Co-authored-by: SW van Heerden <[email protected]>
sdbondi
added a commit
to sdbondi/tari
that referenced
this pull request
Nov 23, 2022
* development: fix: add hidden types and seed words to key manager (tari-project#4925) feat: timestamp validation (tari-project#4887) fix: deleted_txo_mmr_position_to_height_index already exists error (tari-project#4924) feat: add default grpc for localnet (tari-project#4937) first commit (tari-project#4926) v0.40.2 fix(dht): use limited ban period for invalid peer (tari-project#4933) feat: upgrade tari_crypto sign api (tari-project#4932) v0.40.1 chore: fix depreciated timestamp clippy (tari-project#4929) chore: fix naming of ffi functions and comments (tari-project#4930) fix: set wallet start scan height to birthday and not 0 (see issue tari-project#4807) (tari-project#4911) v0.40.0 chore: remove unused methods (tari-project#4922) fix: updates for SafePassword API change (tari-project#4927)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
When wallet starts new UTXO scanning service it should start at the birthday. Currently, it starts at height 0. This PR addresses this issue
Motivation and Context
Addresses #4807.
How Has This Been Tested?