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

Enable using key hashes #193

Merged
merged 15 commits into from
Aug 4, 2022

Conversation

paweljakubas
Copy link
Collaborator

@paweljakubas paweljakubas commented Jul 25, 2022

ADP-1985

PR extends possible bech32 prefixes possible for key hashes, now also non shared ones also.
Also Credential is extended and can be constructed from key hash, for both payment and delegation credential.
PaymentFromKeyHash and DelegationFromKeyHash were added. Thanks to that payment and base address, as well as reward address now can be constructed from key hashes. The new functionality was checked on golden tests. Also proper support in command lines was added, along with testing and documentation.

Copy link
Contributor

@HeinrichApfelmus HeinrichApfelmus left a comment

Choose a reason for hiding this comment

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

Looks good to me! 😊 Two requests concerning the description — if the notion of "base address" is not defined somewhere (e.g. ledger spec), I'd suggest to leave it out.

README.md Outdated


<details>
<summary>How to generate a delegated payment address, ie. base address, from a stake key (<strong>base.addr</strong>)</summary>
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<summary>How to generate a delegated payment address, ie. base address, from a stake key (<strong>base.addr</strong>)</summary>
<summary>How to generate a delegated payment address (<strong>base.addr</strong>)</summary> from a stake key and a payment address

I'm not sure — did we define the notion of "base address" somewhere? If not, then the phrase about "base address" is probably more confusing than helpful. 🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, it is defined in shelley spec, reward account, enterprise address and base address and used for example in ledger CDDL - see https://github.com/input-output-hk/cardano-ledger/blob/master/eras/alonzo/test-suite/cddl-files/alonzo.cddl#L148-L151

README.md Outdated
</details>

<details>
<summary>How to generate a delegated payment address, ie. base address, from a stake key hash (<strong>base.addr</strong>)</summary>
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<summary>How to generate a delegated payment address, ie. base address, from a stake key hash (<strong>base.addr</strong>)</summary>
How to generate a delegated payment address (<strong>base.addr</strong>)</summary> from a stake key hash and a payment address

(as above)

@paweljakubas paweljakubas merged commit 8f57479 into master Aug 4, 2022
@paweljakubas paweljakubas deleted the paweljakubas/adp-1985/enable-using-key-hashes branch August 4, 2022 13:22
iohk-bors bot added a commit to cardano-foundation/cardano-wallet that referenced this pull request Aug 4, 2022
3420: Construct address with hashes r=paweljakubas a=paweljakubas

<!--
Detail in a few bullet points the work accomplished in this PR.

Before you submit, don't forget to:

* Make sure the GitHub PR fields are correct:
   ✓ Set a good Title for your PR.
   ✓ Assign yourself to the PR.
   ✓ Assign one or more reviewer(s).
   ✓ Link to a Jira issue, and/or other GitHub issues or PRs.
   ✓ In the PR description delete any empty sections
     and all text commented in <!--, so that this text does not appear
     in merge commit messages.

* Don't waste reviewers' time:
   ✓ If it's a draft, select the Create Draft PR option.
   ✓ Self-review your changes to make sure nothing unexpected slipped through.

* Try to make your intent clear:
   ✓ Write a good Description that explains what this PR is meant to do.
   ✓ Jira will detect and link to this PR once created, but you can also
     link this PR in the description of the corresponding Jira ticket.
   ✓ Highlight what Testing you have done.
   ✓ Acknowledge any changes required to the Documentation.
-->


- [x] used CredentialFromKeyHash in ApiCredential
- [x] added extended public key in ApiCredential
- [x] cleaned parser (better reuse), also updated swagger
- [x] added more golden in ANY_ADDRESS   

### Comments
depends on IntersectMBO/cardano-addresses#193
<!-- Additional comments, links, or screenshots to attach, if any. -->

### Issue Number
adp-1985
<!-- Reference the Jira/GitHub issue that this PR relates to, and which requirements it tackles.
  Note: Jira issues of the form ADP- will be auto-linked. -->


Co-authored-by: Pawel Jakubas <[email protected]>
Co-authored-by: Jonathan Knowles <[email protected]>
iohk-bors bot added a commit to cardano-foundation/cardano-wallet that referenced this pull request Aug 4, 2022
3420: Construct address with key hashes r=paweljakubas a=paweljakubas

<!--
Detail in a few bullet points the work accomplished in this PR.

Before you submit, don't forget to:

* Make sure the GitHub PR fields are correct:
   ✓ Set a good Title for your PR.
   ✓ Assign yourself to the PR.
   ✓ Assign one or more reviewer(s).
   ✓ Link to a Jira issue, and/or other GitHub issues or PRs.
   ✓ In the PR description delete any empty sections
     and all text commented in <!--, so that this text does not appear
     in merge commit messages.

* Don't waste reviewers' time:
   ✓ If it's a draft, select the Create Draft PR option.
   ✓ Self-review your changes to make sure nothing unexpected slipped through.

* Try to make your intent clear:
   ✓ Write a good Description that explains what this PR is meant to do.
   ✓ Jira will detect and link to this PR once created, but you can also
     link this PR in the description of the corresponding Jira ticket.
   ✓ Highlight what Testing you have done.
   ✓ Acknowledge any changes required to the Documentation.
-->


- [x] used CredentialFromKeyHash in ApiCredential
- [x] added extended public key in ApiCredential
- [x] cleaned parser (better reuse), also updated swagger
- [x] added more golden in ANY_ADDRESS   

### Comments
depends on IntersectMBO/cardano-addresses#193
<!-- Additional comments, links, or screenshots to attach, if any. -->

### Issue Number
adp-1985
<!-- Reference the Jira/GitHub issue that this PR relates to, and which requirements it tackles.
  Note: Jira issues of the form ADP- will be auto-linked. -->


Co-authored-by: Pawel Jakubas <[email protected]>
Co-authored-by: Jonathan Knowles <[email protected]>
iohk-bors bot added a commit to cardano-foundation/cardano-wallet that referenced this pull request Aug 4, 2022
3420: Construct address with key hashes r=paweljakubas a=paweljakubas

<!--
Detail in a few bullet points the work accomplished in this PR.

Before you submit, don't forget to:

* Make sure the GitHub PR fields are correct:
   ✓ Set a good Title for your PR.
   ✓ Assign yourself to the PR.
   ✓ Assign one or more reviewer(s).
   ✓ Link to a Jira issue, and/or other GitHub issues or PRs.
   ✓ In the PR description delete any empty sections
     and all text commented in <!--, so that this text does not appear
     in merge commit messages.

* Don't waste reviewers' time:
   ✓ If it's a draft, select the Create Draft PR option.
   ✓ Self-review your changes to make sure nothing unexpected slipped through.

* Try to make your intent clear:
   ✓ Write a good Description that explains what this PR is meant to do.
   ✓ Jira will detect and link to this PR once created, but you can also
     link this PR in the description of the corresponding Jira ticket.
   ✓ Highlight what Testing you have done.
   ✓ Acknowledge any changes required to the Documentation.
-->


- [x] used CredentialFromKeyHash in ApiCredential
- [x] added extended public key in ApiCredential
- [x] cleaned parser (better reuse), also updated swagger
- [x] added more golden in ANY_ADDRESS   

### Comments
depends on IntersectMBO/cardano-addresses#193
<!-- Additional comments, links, or screenshots to attach, if any. -->

### Issue Number
adp-1985
<!-- Reference the Jira/GitHub issue that this PR relates to, and which requirements it tackles.
  Note: Jira issues of the form ADP- will be auto-linked. -->


Co-authored-by: Pawel Jakubas <[email protected]>
Co-authored-by: Jonathan Knowles <[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