Address Derivation (Sequential) #46
Merged
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
KtorZ
force-pushed
the
KtorZ/#21/ed25519-address-derivation
branch
from
b5208c1 to
d27eb91
Compare
KtorZ
force-pushed
the
KtorZ/#21/ed25519-address-derivation
branch
2 times, most recently
from
80c2b68 to
7263f47
Compare
KtorZ
force-pushed
the
KtorZ/#21/ed25519-address-derivation
branch
from
7263f47 to
35cb00e
Compare
KtorZ
changed the title
paweljakubas
approved these changes
Mar 13, 2019
There was a problem hiding this comment.
I am really fond of extensive usage of phantom-type diambiguation to Key, Index, Passphrase types. It makes comparing BIP-32/44 with this implementation really straightforward, especially reading the intention and the way of deriving different keys
The PR needs rebase and squash
|
|
||
| -- | An encapsulated passphrase. The inner format is free, but the wrapper helps | ||
| -- readability in function signatures. | ||
| newtype Passphrase (goal :: Symbol) = Passphrase ScrubbedBytes |
There was a problem hiding this comment.
just wonder why for Index and Key there is Depth and DerivationType phantom-type used, respectively. And here, you are relying on Symbol (in a sense making phantom-type like specialization open) rather than on data. Using :
data PassphrasePurpose = Encryption | Generation
would have some drawbacks? Maybe we want to extend it somewhere outside ant making not visible in this module?
There was a problem hiding this comment.
Exactly for the reason you mention, using a symbol leaves the type quite open which makes sense for types that are rather general like "Passphrase". On the contrary, the Index and Key have a very precise semantic for which we already know the spectrum, and therefore, we can be explicit and have a closed typed.
So, it's just slightly more convenient to have symbols than sum types and we get almost the same guarantee. Typos are still possible but in such case, GHC will probably yell at you that types don't match.
| keyToAddress (Key xpub) = | ||
| Address $ CBOR.toStrictByteString $ encodeAddress xpub encodeAttributes | ||
| where | ||
| encodeAttributes = mempty <> CBOR.encodeMapLen 0 |
There was a problem hiding this comment.
and we do not want to hide encodeAttributes in the implementation of encodeAddress and just choose the proper implementation by adding in the Cardano.Wallet.Binary :
data AddressScheme = Random | Sequential
and just declare what we want at this this level?
There was a problem hiding this comment.
Nope because the implementation for encoding attributes of random addresses doesn't solely depends on the public key unfortunately. There are extra params that we don't want to bring in the context of the Binary module. The inversion of control is therefore desirable here (cf the prototype).
There was a problem hiding this comment.
ah, ok, I see now 👍 still maybe some testing of encodeAddress would be desirable
There was a problem hiding this comment.
We have golden tests testing just that actually :)
rvl
approved these changes
Mar 13, 2019
| -> Index 'Hardened 'AccountK | ||
| -> Key 'AccountK XPrv | ||
| deriveAccountPrivateKey (Passphrase pwd) (Key rootXPrv) (Index accIx) = | ||
| let |
There was a problem hiding this comment.
I notice there was a passphrase check in the previous version.
This function is no longer failable (partly because the isHardened check is obviated by the Index type).
What happens when the key encryption passphrase is wrong?
There was a problem hiding this comment.
I left a comment about that but I'll extend it. This has been left to the caller as controlling the password would require to either keep a hash of the password in memory (which was a wrong design decision) like before, or have an access to the keystore / password vault. This is a responsibility of the wallet layer calling those derivation primitives.
| deriveXPub DerivationScheme2 changeXPub addrIx | ||
| return $ Key addrXPub | ||
| where | ||
| errWrongIndex = error $ |
There was a problem hiding this comment.
So it's ok to crash with error when the index is greater than a billion or so?
There was a problem hiding this comment.
In practice it shouldn't happen because we can't create such index. Our Index type is completely opaque and the only way to create indexes is through the Bounded and Enum instance (Enum which also throws at runtime if succ is called on maxBound, as specified in the base built-in). Again, the caller would have to check that prior to calling those primitives whether it makes sense.
We know it can't happen, but it's better to have a rather clear invariant here than doing a partial pattern match like Just res <- ... , which would not be very helpful if we ever violate this invariant.
| keyToAddress (Key xpub) = | ||
| Address $ CBOR.toStrictByteString $ encodeAddress xpub encodeAttributes | ||
| where | ||
| encodeAttributes = mempty <> CBOR.encodeMapLen 0 |
There was a problem hiding this comment.
mempty <> should not be necessary according to the semigroup law, or am I missing something?
There was a problem hiding this comment.
It's not indeed. This is just the result of copy/pasting + tweaking.
| -- let encodeAttributes = mempty <> CBOR.encodeMapLen 0 | ||
| -- let addr = encodeAddress xpub encodeAttributes | ||
| -- @ | ||
| encodeAddress :: XPub -> CBOR.Encoding -> CBOR.Encoding |
There was a problem hiding this comment.
Yes, we have golden tests comparing with Yoroi addresses.
| -> Index 'Hardened 'AccountK | ||
| -> Property | ||
| prop_accountKeyDerivation (Seed seed, recPwd) encPwd ix = | ||
| accXPub `seq` property () |
There was a problem hiding this comment.
Is this checking for absence of error?
There was a problem hiding this comment.
Yes. Making sure that this code can run just fine.
|
Removed the unnecessary |
This is just to minimize merge conflicts as we rebase. It makes sense in the context of a PR only
KtorZ
force-pushed
the
KtorZ/#21/ed25519-address-derivation
branch
from
35cb00e to
63e9070
Compare
akegalj
reviewed
Mar 19, 2019
| instance NFData (Index derivationType level) | ||
|
|
||
| instance Bounded (Index 'Hardened level) where | ||
| minBound = Index 0x80000000 |
There was a problem hiding this comment.
nice idea for sequential types to deal with "Word31" type
| -- let accountPubKey = Key 'AccountK XPub | ||
| -- let addressPubKey = Key 'AddressK XPub | ||
| -- @ | ||
| newtype Key (level :: Depth) key = Key key |
Anviking
added a commit
that referenced
this pull request
Aug 25, 2020
Bump includes: - [Increase default mnemonic phrase length](IntersectMBO/cardano-addresses#53) - [Allow to inspect reward accounts wrt #46](IntersectMBO/cardano-addresses#56) - [Allow to generate stake addresses #58](IntersectMBO/cardano-addresses#58) - [Make 'payment' subcommand generate testnet hrp, wrt #55](IntersectMBO/cardano-addresses#59)
iohk-bors bot
added a commit
that referenced
this pull request
Aug 25, 2020
2066: Bump cardano-addresses r=Anviking a=Anviking # Issue Number Release. # Overview <!-- Detail in a few bullet points the work accomplished in this PR --> - [x] Bump cardano-addresses # Comments - I believe this purely affects CLI usage of the cardano-addresses binary that will get included in the wallet release ``` Bump includes: - [Increase default mnemonic phrase length](IntersectMBO/cardano-addresses#53) - [Allow to inspect reward accounts wrt #46](IntersectMBO/cardano-addresses#56) - [Allow to generate stake addresses #58](IntersectMBO/cardano-addresses#58) - [Make 'payment' subcommand generate testnet hrp, wrt #55](IntersectMBO/cardano-addresses#59) ``` <!-- Additional comments or screenshots to attach if any --> <!-- Don't forget to: ✓ Self-review your changes to make sure nothing unexpected slipped through ✓ Assign yourself to the PR ✓ Assign one or several reviewer(s) ✓ Once created, link this PR to its corresponding ticket ✓ Assign the PR to a corresponding milestone ✓ Acknowledge any changes required to the Wiki --> Co-authored-by: Johannes Lund <[email protected]> Co-authored-by: IOHK <[email protected]>
Anviking
added a commit
that referenced
this pull request
Aug 25, 2020
Bump includes: - [Increase default mnemonic phrase length](IntersectMBO/cardano-addresses#53) - [Allow to inspect reward accounts wrt #46](IntersectMBO/cardano-addresses#56) - [Allow to generate stake addresses #58](IntersectMBO/cardano-addresses#58) - [Make 'payment' subcommand generate testnet hrp, wrt #55](IntersectMBO/cardano-addresses#59)
iohk-bors bot
added a commit
that referenced
this pull request
Aug 25, 2020
2066: Bump cardano-addresses r=Anviking a=Anviking # Issue Number Release. # Overview <!-- Detail in a few bullet points the work accomplished in this PR --> - [x] Bump cardano-addresses # Comments - I believe this purely affects CLI usage of the cardano-addresses binary that will get included in the wallet release ``` Bump includes: - [Increase default mnemonic phrase length](IntersectMBO/cardano-addresses#53) - [Allow to inspect reward accounts wrt #46](IntersectMBO/cardano-addresses#56) - [Allow to generate stake addresses #58](IntersectMBO/cardano-addresses#58) - [Make 'payment' subcommand generate testnet hrp, wrt #55](IntersectMBO/cardano-addresses#59) ``` <!-- Additional comments or screenshots to attach if any --> <!-- Don't forget to: ✓ Self-review your changes to make sure nothing unexpected slipped through ✓ Assign yourself to the PR ✓ Assign one or several reviewer(s) ✓ Once created, link this PR to its corresponding ticket ✓ Assign the PR to a corresponding milestone ✓ Acknowledge any changes required to the Wiki --> Co-authored-by: Johannes Lund <[email protected]> Co-authored-by: IOHK <[email protected]> Co-authored-by: Rodney Lorrimar <[email protected]>
iohk-bors bot
added a commit
that referenced
this pull request
Aug 25, 2020
2066: Bump cardano-addresses r=rvl a=Anviking # Issue Number Release. # Overview <!-- Detail in a few bullet points the work accomplished in this PR --> - [x] Bump cardano-addresses # Comments - I believe this purely affects CLI usage of the cardano-addresses binary that will get included in the wallet release ``` Bump includes: - [Increase default mnemonic phrase length](IntersectMBO/cardano-addresses#53) - [Allow to inspect reward accounts wrt #46](IntersectMBO/cardano-addresses#56) - [Allow to generate stake addresses #58](IntersectMBO/cardano-addresses#58) - [Make 'payment' subcommand generate testnet hrp, wrt #55](IntersectMBO/cardano-addresses#59) ``` <!-- Additional comments or screenshots to attach if any --> <!-- Don't forget to: ✓ Self-review your changes to make sure nothing unexpected slipped through ✓ Assign yourself to the PR ✓ Assign one or several reviewer(s) ✓ Once created, link this PR to its corresponding ticket ✓ Assign the PR to a corresponding milestone ✓ Acknowledge any changes required to the Wiki --> Co-authored-by: Johannes Lund <[email protected]> Co-authored-by: IOHK <[email protected]> Co-authored-by: Rodney Lorrimar <[email protected]>
iohk-bors bot
added a commit
that referenced
this pull request
Aug 25, 2020
2066: Bump cardano-addresses r=Anviking a=Anviking # Issue Number Release. # Overview <!-- Detail in a few bullet points the work accomplished in this PR --> - [x] Bump cardano-addresses # Comments - I believe this purely affects CLI usage of the cardano-addresses binary that will get included in the wallet release ``` Bump includes: - [Increase default mnemonic phrase length](IntersectMBO/cardano-addresses#53) - [Allow to inspect reward accounts wrt #46](IntersectMBO/cardano-addresses#56) - [Allow to generate stake addresses #58](IntersectMBO/cardano-addresses#58) - [Make 'payment' subcommand generate testnet hrp, wrt #55](IntersectMBO/cardano-addresses#59) ``` <!-- Additional comments or screenshots to attach if any --> <!-- Don't forget to: ✓ Self-review your changes to make sure nothing unexpected slipped through ✓ Assign yourself to the PR ✓ Assign one or several reviewer(s) ✓ Once created, link this PR to its corresponding ticket ✓ Assign the PR to a corresponding milestone ✓ Acknowledge any changes required to the Wiki --> Co-authored-by: Johannes Lund <[email protected]> Co-authored-by: IOHK <[email protected]> Co-authored-by: Rodney Lorrimar <[email protected]>
iohk-bors bot
added a commit
that referenced
this pull request
Aug 26, 2020
2066: Bump cardano-addresses r=Anviking a=Anviking # Issue Number Release. # Overview <!-- Detail in a few bullet points the work accomplished in this PR --> - [x] Bump cardano-addresses # Comments - I believe this purely affects CLI usage of the cardano-addresses binary that will get included in the wallet release ``` Bump includes: - [Increase default mnemonic phrase length](IntersectMBO/cardano-addresses#53) - [Allow to inspect reward accounts wrt #46](IntersectMBO/cardano-addresses#56) - [Allow to generate stake addresses #58](IntersectMBO/cardano-addresses#58) - [Make 'payment' subcommand generate testnet hrp, wrt #55](IntersectMBO/cardano-addresses#59) ``` <!-- Additional comments or screenshots to attach if any --> <!-- Don't forget to: ✓ Self-review your changes to make sure nothing unexpected slipped through ✓ Assign yourself to the PR ✓ Assign one or several reviewer(s) ✓ Once created, link this PR to its corresponding ticket ✓ Assign the PR to a corresponding milestone ✓ Acknowledge any changes required to the Wiki --> Co-authored-by: Johannes Lund <[email protected]> Co-authored-by: IOHK <[email protected]> Co-authored-by: Rodney Lorrimar <[email protected]>
iohk-bors bot
added a commit
that referenced
this pull request
Aug 26, 2020
2066: Bump cardano-addresses r=Anviking a=Anviking # Issue Number Release. # Overview <!-- Detail in a few bullet points the work accomplished in this PR --> - [x] Bump cardano-addresses # Comments - I believe this purely affects CLI usage of the cardano-addresses binary that will get included in the wallet release ``` Bump includes: - [Increase default mnemonic phrase length](IntersectMBO/cardano-addresses#53) - [Allow to inspect reward accounts wrt #46](IntersectMBO/cardano-addresses#56) - [Allow to generate stake addresses #58](IntersectMBO/cardano-addresses#58) - [Make 'payment' subcommand generate testnet hrp, wrt #55](IntersectMBO/cardano-addresses#59) ``` <!-- Additional comments or screenshots to attach if any --> <!-- Don't forget to: ✓ Self-review your changes to make sure nothing unexpected slipped through ✓ Assign yourself to the PR ✓ Assign one or several reviewer(s) ✓ Once created, link this PR to its corresponding ticket ✓ Assign the PR to a corresponding milestone ✓ Acknowledge any changes required to the Wiki --> Co-authored-by: Johannes Lund <[email protected]> Co-authored-by: IOHK <[email protected]> Co-authored-by: Rodney Lorrimar <[email protected]>
iohk-bors bot
added a commit
that referenced
this pull request
Aug 26, 2020
2066: Bump cardano-addresses r=KtorZ a=Anviking # Issue Number Release. # Overview <!-- Detail in a few bullet points the work accomplished in this PR --> - [x] Bump cardano-addresses # Comments - I believe this purely affects CLI usage of the cardano-addresses binary that will get included in the wallet release ``` Bump includes: - [Increase default mnemonic phrase length](IntersectMBO/cardano-addresses#53) - [Allow to inspect reward accounts wrt #46](IntersectMBO/cardano-addresses#56) - [Allow to generate stake addresses #58](IntersectMBO/cardano-addresses#58) - [Make 'payment' subcommand generate testnet hrp, wrt #55](IntersectMBO/cardano-addresses#59) ``` <!-- Additional comments or screenshots to attach if any --> <!-- Don't forget to: ✓ Self-review your changes to make sure nothing unexpected slipped through ✓ Assign yourself to the PR ✓ Assign one or several reviewer(s) ✓ Once created, link this PR to its corresponding ticket ✓ Assign the PR to a corresponding milestone ✓ Acknowledge any changes required to the Wiki --> 2080: Also trace the times of stake distribution LSQ queries r=Anviking a=Anviking # Issue Number #2005 / new issue # Overview <!-- Detail in a few bullet points the work accomplished in this PR --> - [x] Also measure and trace the times of the LSQ queries in the `stakeDistribution` function # Comments <!-- Additional comments or screenshots to attach if any --> <!-- Don't forget to: ✓ Self-review your changes to make sure nothing unexpected slipped through ✓ Assign yourself to the PR ✓ Assign one or several reviewer(s) ✓ Once created, link this PR to its corresponding ticket ✓ Assign the PR to a corresponding milestone ✓ Acknowledge any changes required to the Wiki --> Co-authored-by: Johannes Lund <[email protected]> Co-authored-by: IOHK <[email protected]> Co-authored-by: Rodney Lorrimar <[email protected]>
iohk-bors bot
added a commit
that referenced
this pull request
Aug 26, 2020
2066: Bump cardano-addresses r=KtorZ a=Anviking # Issue Number Release. # Overview <!-- Detail in a few bullet points the work accomplished in this PR --> - [x] Bump cardano-addresses # Comments - I believe this purely affects CLI usage of the cardano-addresses binary that will get included in the wallet release ``` Bump includes: - [Increase default mnemonic phrase length](IntersectMBO/cardano-addresses#53) - [Allow to inspect reward accounts wrt #46](IntersectMBO/cardano-addresses#56) - [Allow to generate stake addresses #58](IntersectMBO/cardano-addresses#58) - [Make 'payment' subcommand generate testnet hrp, wrt #55](IntersectMBO/cardano-addresses#59) ``` <!-- Additional comments or screenshots to attach if any --> <!-- Don't forget to: ✓ Self-review your changes to make sure nothing unexpected slipped through ✓ Assign yourself to the PR ✓ Assign one or several reviewer(s) ✓ Once created, link this PR to its corresponding ticket ✓ Assign the PR to a corresponding milestone ✓ Acknowledge any changes required to the Wiki --> Co-authored-by: Johannes Lund <[email protected]> Co-authored-by: IOHK <[email protected]> Co-authored-by: Rodney Lorrimar <[email protected]>
iohk-bors bot
added a commit
that referenced
this pull request
Aug 28, 2020
2066: Bump cardano-addresses r=rvl a=Anviking # Issue Number Release. # Overview <!-- Detail in a few bullet points the work accomplished in this PR --> - [x] Bump cardano-addresses # Comments - I believe this purely affects CLI usage of the cardano-addresses binary that will get included in the wallet release ``` Bump includes: - [Increase default mnemonic phrase length](IntersectMBO/cardano-addresses#53) - [Allow to inspect reward accounts wrt #46](IntersectMBO/cardano-addresses#56) - [Allow to generate stake addresses #58](IntersectMBO/cardano-addresses#58) - [Make 'payment' subcommand generate testnet hrp, wrt #55](IntersectMBO/cardano-addresses#59) ``` <!-- Additional comments or screenshots to attach if any --> <!-- Don't forget to: ✓ Self-review your changes to make sure nothing unexpected slipped through ✓ Assign yourself to the PR ✓ Assign one or several reviewer(s) ✓ Once created, link this PR to its corresponding ticket ✓ Assign the PR to a corresponding milestone ✓ Acknowledge any changes required to the Wiki --> 2071: Minor nix cleanups r=rvl a=rvl ### Issue Number #2046 #2054 ### Overview - Minor cleanups of nix code for migration tests and latency benchmark. ### Comments Tested with: ``` nix-build -A benchmarks.cardano-wallet.latency nix-build nix/migration-tests.nix ``` 2080: Also trace the times of stake distribution LSQ queries r=rvl a=Anviking # Issue Number #2005 / new issue # Overview <!-- Detail in a few bullet points the work accomplished in this PR --> - [x] Also measure and trace the times of the LSQ queries in the `stakeDistribution` function # Comments <!-- Additional comments or screenshots to attach if any --> <!-- Don't forget to: ✓ Self-review your changes to make sure nothing unexpected slipped through ✓ Assign yourself to the PR ✓ Assign one or several reviewer(s) ✓ Once created, link this PR to its corresponding ticket ✓ Assign the PR to a corresponding milestone ✓ Acknowledge any changes required to the Wiki --> 2087: Transaction metadata in swagger API spec r=rvl a=rvl ### Issue Number ADP-307 / #2073 / #2074 ### Overview Updates the swagger spec for the new metadata field when: - listing transactions (metadata field is always present but may be null) - posting a transaction (metadata field is optional) - estimating fee (as above) ### Comments [Rendered spec](https://redocly.github.io/redoc/?url=https://raw.githubusercontent.com/input-output-hk/cardano-wallet/rvl/2073/swagger/specifications/api/swagger.yaml) Co-authored-by: Johannes Lund <[email protected]> Co-authored-by: IOHK <[email protected]> Co-authored-by: Rodney Lorrimar <[email protected]> Co-authored-by: KtorZ <[email protected]>
iohk-bors bot
added a commit
that referenced
this pull request
Aug 28, 2020
2066: Bump cardano-addresses r=rvl a=Anviking # Issue Number Release. # Overview <!-- Detail in a few bullet points the work accomplished in this PR --> - [x] Bump cardano-addresses # Comments - I believe this purely affects CLI usage of the cardano-addresses binary that will get included in the wallet release ``` Bump includes: - [Increase default mnemonic phrase length](IntersectMBO/cardano-addresses#53) - [Allow to inspect reward accounts wrt #46](IntersectMBO/cardano-addresses#56) - [Allow to generate stake addresses #58](IntersectMBO/cardano-addresses#58) - [Make 'payment' subcommand generate testnet hrp, wrt #55](IntersectMBO/cardano-addresses#59) ``` <!-- Additional comments or screenshots to attach if any --> <!-- Don't forget to: ✓ Self-review your changes to make sure nothing unexpected slipped through ✓ Assign yourself to the PR ✓ Assign one or several reviewer(s) ✓ Once created, link this PR to its corresponding ticket ✓ Assign the PR to a corresponding milestone ✓ Acknowledge any changes required to the Wiki --> 2071: Minor nix cleanups r=rvl a=rvl ### Issue Number #2046 #2054 ### Overview - Minor cleanups of nix code for migration tests and latency benchmark. ### Comments Tested with: ``` nix-build -A benchmarks.cardano-wallet.latency nix-build nix/migration-tests.nix ``` Co-authored-by: Johannes Lund <[email protected]> Co-authored-by: IOHK <[email protected]> Co-authored-by: Rodney Lorrimar <[email protected]>
iohk-bors bot
added a commit
that referenced
this pull request
Aug 28, 2020
2066: Bump cardano-addresses r=rvl a=Anviking # Issue Number Release. # Overview <!-- Detail in a few bullet points the work accomplished in this PR --> - [x] Bump cardano-addresses # Comments - I believe this purely affects CLI usage of the cardano-addresses binary that will get included in the wallet release ``` Bump includes: - [Increase default mnemonic phrase length](IntersectMBO/cardano-addresses#53) - [Allow to inspect reward accounts wrt #46](IntersectMBO/cardano-addresses#56) - [Allow to generate stake addresses #58](IntersectMBO/cardano-addresses#58) - [Make 'payment' subcommand generate testnet hrp, wrt #55](IntersectMBO/cardano-addresses#59) ``` <!-- Additional comments or screenshots to attach if any --> <!-- Don't forget to: ✓ Self-review your changes to make sure nothing unexpected slipped through ✓ Assign yourself to the PR ✓ Assign one or several reviewer(s) ✓ Once created, link this PR to its corresponding ticket ✓ Assign the PR to a corresponding milestone ✓ Acknowledge any changes required to the Wiki --> Co-authored-by: Johannes Lund <[email protected]> Co-authored-by: IOHK <[email protected]> Co-authored-by: Rodney Lorrimar <[email protected]>
iohk-bors bot
added a commit
that referenced
this pull request
Aug 30, 2020
2066: Bump cardano-addresses r=Anviking a=Anviking # Issue Number Release. # Overview <!-- Detail in a few bullet points the work accomplished in this PR --> - [x] Bump cardano-addresses # Comments - I believe this purely affects CLI usage of the cardano-addresses binary that will get included in the wallet release ``` Bump includes: - [Increase default mnemonic phrase length](IntersectMBO/cardano-addresses#53) - [Allow to inspect reward accounts wrt #46](IntersectMBO/cardano-addresses#56) - [Allow to generate stake addresses #58](IntersectMBO/cardano-addresses#58) - [Make 'payment' subcommand generate testnet hrp, wrt #55](IntersectMBO/cardano-addresses#59) ``` <!-- Additional comments or screenshots to attach if any --> <!-- Don't forget to: ✓ Self-review your changes to make sure nothing unexpected slipped through ✓ Assign yourself to the PR ✓ Assign one or several reviewer(s) ✓ Once created, link this PR to its corresponding ticket ✓ Assign the PR to a corresponding milestone ✓ Acknowledge any changes required to the Wiki --> Co-authored-by: Johannes Lund <[email protected]> Co-authored-by: IOHK <[email protected]> Co-authored-by: Rodney Lorrimar <[email protected]>
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.
Issue Number
#21
Overview
Comments
This was actually needed for #22 🤷♂️