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.

Sign up for GitHub

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

Jump to bottom

Construct address with key hashes #3420

Merged
merged 11 commits into from
Aug 4, 2022
Merged

Construct address with key hashes #3420

merged 11 commits into from
Aug 4, 2022

Conversation

paweljakubas
Copy link
Contributor

@paweljakubas paweljakubas commented Aug 2, 2022 •
edited
Loading

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

Comments

depends on IntersectMBO/cardano-addresses#193

Issue Number

adp-1985

@paweljakubas paweljakubas self-assigned this Aug 2, 2022
@paweljakubas paweljakubas force-pushed the paweljakubas/adp-1985/construct-address-with-hashes branch from 9672cac to 84b08cf Compare August 3, 2022 12:11
@paweljakubas paweljakubas marked this pull request as ready for review August 3, 2022 12:11
HeinrichApfelmus
HeinrichApfelmus approved these changes Aug 4, 2022
View reviewed changes
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, thanks! 😊

@piotr-iohk may want to take another look, but the integration tests give me confidence that the code works as advertised.

@paweljakubas paweljakubas force-pushed the paweljakubas/adp-1985/construct-address-with-hashes branch from 84b08cf to 02fccba Compare August 4, 2022 13:32
@paweljakubas
Copy link
Contributor Author

bors r+

iohk-bors bot added a commit that referenced this pull request Aug 4, 2022
@iohk-bors @paweljakubas @jonathanknowles
Merge #3420
fc1355f 
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]>
paweljakubas and others added 11 commits August 4, 2022 15:34
@paweljakubas
bump cardano-addresses
d27e686
@paweljakubas
add integration test
67b7c92
@paweljakubas
extend ApiCredential - extended public keys
6397ec8
@paweljakubas
extend ApiCredential - key hashes
d90fd85
@paweljakubas
update swagger
e1d9655
@paweljakubas
fix parser
db06261
@jonathanknowles @paweljakubas
Update golden JSON test data for API types.
624a7c2 
Steps:

1. Identify the set of types for which there are no JSON golden data files:

```
$ cabal run cardano-wallet-core:test:unit -- --match "produces the same JSON" | grep -i missing
Missing golden file: lib/core/test/data/Cardano/Wallet/Api/ApiAddressData.json
Missing golden file: lib/core/test/data/Cardano/Wallet/Api/ApiCredential.json
```

2. Identify the set of types for which JSON golden data files are out-of-date:

```
$ cabal run cardano-wallet-core:test:unit -- --match "produces the same JSON" | grep -i warning
WARNING: Encoding new random samples do not match lib/core/test/data/Cardano/Wallet/Api/ApiNetworkInformation.json.
WARNING: Encoding new random samples do not match lib/core/test/data/Cardano/Wallet/Api/WalletPutPassphraseData.json.
```

3. Delete the out-of-date files:
```
rm lib/core/test/data/Cardano/Wallet/Api/ApiNetworkInformation.json
rm lib/core/test/data/Cardano/Wallet/Api/WalletPutPassphraseData.json
```

4. Generate new golden files for both sets (missing and out-of-date):
```
CREATE_MISSING_GOLDEN=TRUE cabal run cardano-wallet-core:test:unit -- --match "produces the same JSON"
```

5. Add updated files to git:
```
git add lib/core/test/data
```
@paweljakubas
extend enterprise address testing
8f31d3e
@paweljakubas
extend reward account address testing
80b2138
@paweljakubas
extend base address testing
9eb1c07
@paweljakubas
bump to cardano-addresses and hlint
02fccba
@paweljakubas paweljakubas changed the title Construct address with hashes Construct address with key hashes Aug 4, 2022
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Aug 4, 2022 •
edited by Anviking
Loading

Build failed:

         )
       
       Waited longer than 300s to resolve action: "metadata is fetched".
       expected: [PoolId {getPoolId = "\180Wh\193\162\218K\209>\188\170\RS\165\DC4\b\237\163\GS\204!v\\\203\212\a\205\169\242"}]
        but got: []

  To rerun use: --match "/API Specifications/SHELLEY_STAKE_POOLS/STAKE_POOLS_SMASH_01 - fetching metadata from SMASH works with delisted pools/"

Randomized with seed 1170280285

Finished in 1783.0975 seconds, used 1187.2460 seconds of CPU time
973 examples, 2 failures, 35 pending

#2331

@paweljakubas
Copy link
Contributor Author

bors r+

iohk-bors bot added a commit that referenced this pull request Aug 4, 2022
@iohk-bors @paweljakubas @jonathanknowles
Merge #3420
d47a2ac 
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
Copy link
Contributor

iohk-bors bot commented Aug 4, 2022 •
edited by Anviking
Loading

Build failed:

  src/Test/Integration/Framework/DSL.hs:1202:21: 
  1) API Specifications, SHELLEY_MIGRATIONS, SHELLEY_MIGRATE_09 - Can migrate a wallet that has rewards.
       Waited longer than 90s to resolve action: "waitForNextEpoch: goes to next epoch". Error condition: Just getFromResponse failed to get item
       CallStack (from HasCallStack):
         error, called at src/Test/Integration/Framework/DSL.hs:2110:16 in cardano-wallet-core-integration-2022.7.1-J5Z5HGgKfnT9G6NpDdbvog:Test.Integration.Framework.DSL
         getFromResponse, called at src/Test/Integration/Framework/DSL.hs:1047:14 in cardano-wallet-core-integration-2022.7.1-J5Z5HGgKfnT9G6NpDdbvog:Test.Integration.Framework.DSL

  To rerun use: --match "/API Specifications/SHELLEY_MIGRATIONS/SHELLEY_MIGRATE_09 - Can migrate a wallet that has rewards./"

Randomized with seed 1650868241

#3441

@paweljakubas
Copy link
Contributor Author

bors r+

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Aug 4, 2022

Build succeeded:

@iohk-bors iohk-bors bot merged commit 8bb2a1f into master Aug 4, 2022
@iohk-bors iohk-bors bot deleted the paweljakubas/adp-1985/construct-address-with-hashes branch August 4, 2022 17:17
WilliamKingNoel-Bot pushed a commit that referenced this pull request Aug 4, 2022
docs: Merge #3420
cf8f046
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@HeinrichApfelmus HeinrichApfelmus HeinrichApfelmus approved these changes

@Unisay Unisay Awaiting requested review from Unisay

@piotr-iohk piotr-iohk Awaiting requested review from piotr-iohk

@jonathanknowles jonathanknowles Awaiting requested review from jonathanknowles

@paolino paolino Awaiting requested review from paolino

Assignees

@paweljakubas paweljakubas

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@paweljakubas @HeinrichApfelmus @jonathanknowles