This repository has been archived by the owner on Mar 1, 2019. It is now read-only.
Derivation wallet flag #324
Open
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
akegalj
commented
Feb 7, 2019
|
|
||
| derivationScheme :: Core.Address -> DerivationScheme |
There was a problem hiding this comment.
this can be removed now as this information is contained in wallet type and in db - will remove
There was a problem hiding this comment.
maybe we can have that as an invariant check somewhere 🤔 ?
There was a problem hiding this comment.
yes - this could be used in tests at least. Additionally we could throw an exception if (or an error) if that invariant is not satisfied as a "failsafe" if something whent wrong.
akegalj
commented
Feb 7, 2019
KtorZ
suggested changes
Feb 7, 2019
|
|
||
| derivationScheme :: Core.Address -> DerivationScheme |
There was a problem hiding this comment.
maybe we can have that as an invariant check somewhere 🤔 ?
This commit adds derivation scheme flag to Wallet datatype from V1.Types and stores this flag to db. This flag will be use in future to determine which derivation scheme to use with specific wallet.
akegalj
force-pushed
the
akegalj/45/derivation-wallet-flag
branch
from
9d46e90 to
915a6e1
Compare
KtorZ
reviewed
Feb 8, 2019
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
45#
Overview
DerivationSchemeVersiontoWalletto distinguish between sequential and random derivation schemesDerivationSchemetoHdRootto store this data to dbI have added a db migration to set all old wallets to random derivation scheme- not needed for 1.6 release
Comments
I am not sure how to test this feature? I can test migration but as this new flag is not fully wired yet I think we will need to postpone the testing until everything is in place. This adds additional flag to the db but doesn't do any logic with the flag yet.
I believe @uroboros work will use this flag. Also work from few of my PRs will also benefit the flag (as testing transaction signer is closer to be working now)
If you have some idea how to test - please sare.