Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new address discovery schemes for testing and benchmarking #160

Merged
merged 4 commits into from
Apr 26, 2019

Conversation

rvl
Copy link
Contributor

@rvl rvl commented Apr 8, 2019

Relates to issue #100

Overview

This adds supplementary address discovery schemes for benchmarking.

Some things related to creating wallets needed to be moved around to accommodate.

@rvl rvl self-assigned this Apr 8, 2019
@rvl rvl force-pushed the rvl/100/bench branch from 18901ea to 9c5f6df Compare April 9, 2019 05:52
@rvl rvl force-pushed the rvl/100/address-discovery branch from b04b8cb to ea4d55b Compare April 9, 2019 09:20
@rvl rvl changed the title wip: Add new address discovery schemes for testing and benchmarking Add new address discovery schemes for testing and benchmarking Apr 9, 2019
@rvl rvl marked this pull request as ready for review April 9, 2019 09:24
@KtorZ KtorZ force-pushed the rvl/100/bench branch 2 times, most recently from 97b16d1 to 714319b Compare April 9, 2019 12:13
@KtorZ KtorZ changed the base branch from rvl/100/bench to master April 9, 2019 12:32
@KtorZ KtorZ force-pushed the rvl/100/address-discovery branch from ea4d55b to 639e8c5 Compare April 9, 2019 12:46
@KtorZ
Copy link
Member

KtorZ commented Apr 9, 2019

@rvl I've changed the base the master and resolved conflicts.

} deriving (Show, Generic)
, addressDiscoveryConfig
:: AddressDiscoveryConfig s
}
Copy link
Member

@KtorZ KtorZ Apr 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A suggestion here: let's remove the NewWallet type wrapper here and simply modify the wallet layer signature as follows:

        :: WalletName
        -> Key scheme 'Root0 XPrv
        -> Passphrase "encryption"
        -> s
        -> ExceptT (ErrWalletAlreadyExists "createWallet") IO WalletId

Leave the creation of the state to the caller 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK good idea. Turns out NewWallet is kind of helpful, so I have added a createWalletSeq :: WalletLayer SeqState -> NewWallet ExceptT ErrWalletAlreadyExists IO WalletId.

initState :: NewWallet s -> (WalletId, s)

instance InitState SeqState where
initState w = (wid, seqState)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We won't require a type-class here (such abstraction for the wallet scheme is actually hard to have in practice because the random cardano wallet and the sequential have rather different and not-so-compatible setup :(), but rather, let's make use of a helper function which yield a SeqState from a seed, generation passphrase and address pool gap.

initSeqState 
    :: (Passphrase "seed", Passphrase "generation") 
    -> AddressPoolGap 
    -> (Key 'Seq Root0 XPrv, SeqState)

This will play nicely with the previous comment and should be also flexible enough for testing 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK added, but the testing AD scheme(s) don't have any key. So I let the initSeqState function calculate the wallet ID.

@rvl rvl force-pushed the rvl/100/address-discovery branch 5 times, most recently from eadb5dc to b47eea2 Compare April 11, 2019 00:08
@KtorZ KtorZ force-pushed the rvl/100/address-discovery branch from b47eea2 to 854c571 Compare April 26, 2019 15:41
Copy link
Member

@KtorZ KtorZ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reviewed & rebased on top of master

KtorZ and others added 4 commits April 26, 2019 17:52
This creates a new 'special' address scheme that accept a given
proportion of addresses on the chain (e.g. 50%). This allows us to
artificially create very big wallets and observe how the wallet backend
survive and behave.

Co-authored-by: Rodney Lorrimar <[email protected]>
@KtorZ KtorZ force-pushed the rvl/100/address-discovery branch from 854c571 to e3964f2 Compare April 26, 2019 15:52
@KtorZ KtorZ merged commit 9d81287 into master Apr 26, 2019
@KtorZ KtorZ deleted the rvl/100/address-discovery branch April 26, 2019 16:33
parsonsmatt added a commit to parsonsmatt/cardano-sl that referenced this pull request May 14, 2019
3987: Parse required CLI params in cluster r=parsonsmatt a=parsonsmatt

## Description

<!--- A brief description of this PR and the problem is trying to solve -->

## Linked issue

<!--- Put here the relevant issue from YouTrack -->

cardano-foundation/cardano-wallet#160



Co-authored-by: Moritz Angermann <[email protected]>
Co-authored-by: parsonsmatt <[email protected]>
Co-authored-by: KtorZ <[email protected]>
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