Skip to content
This repository has been archived by the owner on Jan 12, 2022. It is now read-only.

feat!: KeyChainStore, ChainStore, WalletStore and path registration feature #351

Open
wants to merge 40 commits into
base: v0.22-dev
Choose a base branch
from

Conversation

Alex-Werner
Copy link
Contributor

@Alex-Werner Alex-Werner commented Nov 4, 2021

Issue being fixed or feature implemented

This Pull Request is pretty dense in the amount of internals that it modifies.
The goal is to modifies the way keys (and subsequently, addresses) are handled internally.

It comes with the addition of a KeyChainStore that manage one of multiple KeyChains.
It improve KeyChain so that it is possible to mark a specific key as watched.
Doing so allow to easily get other components to register path for being watched, by registering keys to the keychain, and adding keychain to the keystore.
One exemple of such usage is DashPay which create a HDPublicKey initialized KeyChain, of which their is a requirement to get the watched address to submit a bloomfilter that includes them.

About Storage :

  • ChainStore holds all chain-related information: addresses, transactions, instantLocks, blockheaders, and fees. It doesn't care about wallets, but when addresses are imported, it will update state via transactions already added or imported later.
  • WalletStore holds internal paths / addresses sets to provide linking with ChainStore or KeyChainStore.

About Bloomfilter and Address state :

As we now hold addresses state (balance, tx, utxos) in ChainStore, and WalletStore only keep some mapping of internal (eg: for unusedAddress purpose), we can allow any addition of new KeyChain to be added and their addresses to be added to watched address.
Therefore, we will be able to submit to bloomfilter all addresses from master keychain and afferent one that were generated (this is typically the case on DashPay Contacts).
It also allow plugins to have some limited control to which addresses get supplied to the SPV process, they will still need to access the information (e.g: getting TxHistory of those addresses).
It would be an option to generalise this on a Wallet level instead of an Account level, but this PR only implements watched address from Account keychainStore.

What was done?

  • feat: introduce KeyChainStore
  • feat!: modifies how KeyChain works
  • feat!: introduce ChainStore, WalletStore and IdentitiesStore and modifies Storage.
  • fix: export and import of Storage adapter's persistence layer.
  • feat: add watching keys functionalities (add, remove, get)

How Has This Been Tested?

  • specific unit test added.

Breaking Changes

  • keyChain in wallet and account is now accessed via keyChainStore.getWalletKeyChain.
  • removed generateKeyForPath, normalize base key to single rootKey, rename type to rootKeyType, remove unnecessary getKeyForChild duplicate of getKeyForType.
  • feat!: Storage now only holds Account, Chain or Wallet Store.
  • Account.storage.store won't return any store this way (use storage.getWalletStore, storage.getChainStore instead).
  • Transaction's metadata chainLocked and instantLocked properties renamed to isChainLocked and isInstantLocked.

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

Alex-Werner and others added 8 commits November 2, 2021 02:29
BREAKING-CHANGES : removed generateKetForPath, normalize base key to single rootKey, rename type to rootKeyType, remove unnecessary getKeyForChild duplicate of getKeyForType
BREAKING-CHANGES: keyChain in wallet and account is now accessed via keyChainStore.getWalletKeyChain
@Alex-Werner Alex-Werner changed the title Feat/registration path feat: KeyChainStore, and path registration Nov 5, 2021
@Alex-Werner Alex-Werner marked this pull request as ready for review November 5, 2021 09:53
@lgtm-com
Copy link

lgtm-com bot commented Nov 15, 2021

This pull request introduces 3 alerts when merging 07690fb into 9a67d0d - view on LGTM.com

new alerts:

  • 3 for Unused variable, import, function or class

@lgtm-com
Copy link

lgtm-com bot commented Nov 15, 2021

This pull request introduces 7 alerts when merging afb46ef into 9a67d0d - view on LGTM.com

new alerts:

  • 5 for Unused variable, import, function or class
  • 2 for Useless assignment to local variable

@lgtm-com
Copy link

lgtm-com bot commented Nov 18, 2021

This pull request introduces 7 alerts when merging a43369a into 9a67d0d - view on LGTM.com

new alerts:

  • 5 for Unused variable, import, function or class
  • 2 for Useless assignment to local variable

@lgtm-com
Copy link

lgtm-com bot commented Nov 27, 2021

This pull request introduces 38 alerts when merging 86a1b2d into 9a67d0d - view on LGTM.com

new alerts:

  • 37 for Unused variable, import, function or class
  • 1 for Unreachable statement

@Alex-Werner Alex-Werner changed the title feat: KeyChainStore, and path registration feat: KeyChainStore, ChainStore, WalletStore and path registration feature Nov 28, 2021
@Alex-Werner Alex-Werner changed the title feat: KeyChainStore, ChainStore, WalletStore and path registration feature feat!: KeyChainStore, ChainStore, WalletStore and path registration feature Nov 28, 2021
@lgtm-com
Copy link

lgtm-com bot commented Nov 29, 2021

This pull request introduces 38 alerts when merging f9eb15d into 9a67d0d - view on LGTM.com

new alerts:

  • 37 for Unused variable, import, function or class
  • 1 for Unreachable statement

@lgtm-com
Copy link

lgtm-com bot commented Nov 30, 2021

This pull request introduces 37 alerts when merging 426e919 into 9a67d0d - view on LGTM.com

new alerts:

  • 37 for Unused variable, import, function or class

@lgtm-com
Copy link

lgtm-com bot commented Dec 1, 2021

This pull request introduces 37 alerts when merging 6968c8f into 9a67d0d - view on LGTM.com

new alerts:

  • 37 for Unused variable, import, function or class

@lgtm-com
Copy link

lgtm-com bot commented Dec 2, 2021

This pull request introduces 37 alerts when merging 09d6b9a into 9a67d0d - view on LGTM.com

new alerts:

  • 37 for Unused variable, import, function or class

@lgtm-com
Copy link

lgtm-com bot commented Dec 2, 2021

This pull request introduces 37 alerts when merging ec7cc24 into 9a67d0d - view on LGTM.com

new alerts:

  • 37 for Unused variable, import, function or class

@lgtm-com
Copy link

lgtm-com bot commented Dec 9, 2021

This pull request introduces 39 alerts when merging 46ac52e into 9a67d0d - view on LGTM.com

new alerts:

  • 39 for Unused variable, import, function or class

@lgtm-com
Copy link

lgtm-com bot commented Dec 15, 2021

This pull request introduces 31 alerts when merging 1b5b1e4 into 9a67d0d - view on LGTM.com

new alerts:

  • 31 for Unused variable, import, function or class

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants