-
Notifications
You must be signed in to change notification settings - Fork 217
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
Allow specifying purpose for acc x pub #2693
Allow specifying purpose for acc x pub #2693
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good - with 2 requested changes. Thanks
lib/core/src/Cardano/Wallet.hs
Outdated
when (isJust purposeM) $ do | ||
purposeGuarded <- runExceptT $ guardHardIndex (fromJust purposeM) | ||
case purposeGuarded of | ||
Left err -> throwE $ ErrReadAccountPublicKeyInvalidPurposeIndex err | ||
Right _ -> pure () |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something like this can avoid the need for isJust/fromJust/runExceptT/throwE.
when (isJust purposeM) $ do | |
purposeGuarded <- runExceptT $ guardHardIndex (fromJust purposeM) | |
case purposeGuarded of | |
Left err -> throwE $ ErrReadAccountPublicKeyInvalidPurposeIndex err | |
Right _ -> pure () | |
maybe (pure ()) (withExceptT ErrReadAccountPublicKeyInvalidPurposeIndex . guardHardIndex) purpose |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indeed!
specifications/api/swagger.yaml
Outdated
the wallet must have been created from mnemonic. In request body, arbitrary purpose can be specified. | ||
Omitting the purpose segment means default purpose, ie., 1852H, will be used. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the wallet must have been created from mnemonic. In request body, arbitrary purpose can be specified. | |
Omitting the purpose segment means default purpose, ie., 1852H, will be used. | |
the wallet must have been created from mnemonic. | |
It is possible to use the optional `purpose` field to override that branch of the derivation path | |
with different hardened derivation index. If that field is omitted, the default purpose | |
for Cardano wallets (`1852H`) will be used. |
bors r+ |
2692: Test the generation of change maps for non-user-specified assets. r=jonathanknowles a=jonathanknowles # Issue Number ADP-346 # Overview This PR: - extracts out function `collateNonUserSpecifiedAssetQuantities`. - extracts out function `makeChangeForNonUserSpecifiedAssets`. - adds property tests to verify the expected behaviour of each function. - adds unit tests to illustrate the expected behaviour of each function. The `collateNonUserSpecifiedAssetQuantities` function is designed to produce a map of all assets that do **NOT** appear in the user-specified outputs of a coin selection. Each asset `a` is mapped to the complete list of discrete quantities of `a` found in the selected inputs. The `makeChangeForNonUserSpecifiedAssets` function is designed to make a list of change maps for all assets that do **NOT** appear in the user-specified outputs of coin selection. The number of change maps is intended to be exactly equal to the number of user-specified outputs. 2693: Allow specifying purpose for acc x pub r=paweljakubas a=paweljakubas # Issue Number <!-- Put here a reference to the issue that this PR relates to and which requirements it tackles. Jira issues of the form ADP- will be auto-linked. --> adp-950 # Overview <!-- Detail in a few bullet points the work accomplished in this PR --> - [x] updated swagger - [x] enable passing purpose - [x] adjust core unit tests - [x] add integration test - [x] guard purpose with integration test # 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) ✓ Jira will detect and link to this PR once created, but you can also link this PR in the description of the corresponding ticket ✓ Acknowledge any changes required to the Wiki ✓ Finally, in the PR description delete any empty sections and all text commented in <!--, so that this text does not appear in merge commit messages. --> Co-authored-by: Jonathan Knowles <[email protected]> Co-authored-by: Pawel Jakubas <[email protected]>
Build failed (retrying...): |
2693: Allow specifying purpose for acc x pub r=paweljakubas a=paweljakubas # Issue Number <!-- Put here a reference to the issue that this PR relates to and which requirements it tackles. Jira issues of the form ADP- will be auto-linked. --> adp-950 # Overview <!-- Detail in a few bullet points the work accomplished in this PR --> - [x] updated swagger - [x] enable passing purpose - [x] adjust core unit tests - [x] add integration test - [x] guard purpose with integration test # 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) ✓ Jira will detect and link to this PR once created, but you can also link this PR in the description of the corresponding ticket ✓ Acknowledge any changes required to the Wiki ✓ Finally, in the PR description delete any empty sections and all text commented in <!--, so that this text does not appear in merge commit messages. --> Co-authored-by: Pawel Jakubas <[email protected]>
Build failed:
|
23a18ff
to
d66fa2e
Compare
bors r+ |
2693: Allow specifying purpose for acc x pub r=paweljakubas a=paweljakubas # Issue Number <!-- Put here a reference to the issue that this PR relates to and which requirements it tackles. Jira issues of the form ADP- will be auto-linked. --> adp-950 # Overview <!-- Detail in a few bullet points the work accomplished in this PR --> - [x] updated swagger - [x] enable passing purpose - [x] adjust core unit tests - [x] add integration test - [x] guard purpose with integration test # 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) ✓ Jira will detect and link to this PR once created, but you can also link this PR in the description of the corresponding ticket ✓ Acknowledge any changes required to the Wiki ✓ Finally, in the PR description delete any empty sections and all text commented in <!--, so that this text does not appear in merge commit messages. --> 2694: Factor `getAccountBalance` into `getCachedAccountBalance` and `fetchAccountBalances` r=Anviking a=Anviking # Issue Number <!-- Put here a reference to the issue that this PR relates to and which requirements it tackles. Jira issues of the form ADP- will be auto-linked. --> Split off from #2684 # Overview <!-- Detail in a few bullet points the work accomplished in this PR --> - [x] Rename `getAccountBalance` to `getCachedAccountBalance` for clarity - [x] Add `fetchAccountBalances` function for un-cached behaviour - [x] `fetchAccountBalances` from `listStakeKeys` # 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) ✓ Jira will detect and link to this PR once created, but you can also link this PR in the description of the corresponding ticket ✓ Acknowledge any changes required to the Wiki ✓ Finally, in the PR description delete any empty sections and all text commented in <!--, so that this text does not appear in merge commit messages. --> Co-authored-by: Pawel Jakubas <[email protected]> Co-authored-by: Johannes Lund <[email protected]>
Build failed (retrying...): #expected |
2693: Allow specifying purpose for acc x pub r=paweljakubas a=paweljakubas # Issue Number <!-- Put here a reference to the issue that this PR relates to and which requirements it tackles. Jira issues of the form ADP- will be auto-linked. --> adp-950 # Overview <!-- Detail in a few bullet points the work accomplished in this PR --> - [x] updated swagger - [x] enable passing purpose - [x] adjust core unit tests - [x] add integration test - [x] guard purpose with integration test # 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) ✓ Jira will detect and link to this PR once created, but you can also link this PR in the description of the corresponding ticket ✓ Acknowledge any changes required to the Wiki ✓ Finally, in the PR description delete any empty sections and all text commented in <!--, so that this text does not appear in merge commit messages. --> Co-authored-by: Pawel Jakubas <[email protected]>
Build failed:
|
bors r+ |
Build succeeded: |
2691: Additional checks for listing stake keys in e2e tests r=piotr-iohk a=piotr-iohk # Issue Number <!-- Put here a reference to the issue that this PR relates to and which requirements it tackles. Jira issues of the form ADP- will be auto-linked. --> # Overview <!-- Detail in a few bullet points the work accomplished in this PR --> - [ ] Additional checks for listing stake keys in e2e tests # 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) ✓ Jira will detect and link to this PR once created, but you can also link this PR in the description of the corresponding ticket ✓ Acknowledge any changes required to the Wiki ✓ Finally, in the PR description delete any empty sections and all text commented in <!--, so that this text does not appear in merge commit messages. --> 2693: Allow specifying purpose for acc x pub r=paweljakubas a=paweljakubas # Issue Number <!-- Put here a reference to the issue that this PR relates to and which requirements it tackles. Jira issues of the form ADP- will be auto-linked. --> adp-950 # Overview <!-- Detail in a few bullet points the work accomplished in this PR --> - [x] updated swagger - [x] enable passing purpose - [x] adjust core unit tests - [x] add integration test - [x] guard purpose with integration test # 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) ✓ Jira will detect and link to this PR once created, but you can also link this PR in the description of the corresponding ticket ✓ Acknowledge any changes required to the Wiki ✓ Finally, in the PR description delete any empty sections and all text commented in <!--, so that this text does not appear in merge commit messages. --> Co-authored-by: Piotr Stachyra <[email protected]> Co-authored-by: Pawel Jakubas <[email protected]> ded045e
Issue Number
adp-950
Overview
Comments