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

CAD-2337 API extensions for address and credentials #2127

Merged

iohk-bors merged 2 commits into master from cad-2337-api

Nov 26, 2020

Contributor

deepfire commented Nov 25, 2020

toAddressAny :: Address addr -> AddressAny
fromShelleyPaymentCredential :: Shelley.PaymentCredential StandardShelley -> PaymentCredential
fromShelleyStakeReference :: Shelley.StakeReference StandardShelley -> StakeAddressReference
export fromShelleyStakeCredential

deepfire requested review from dcoutts, erikd, intricate, Jimbo4350, mrBliss and newhoggy as code owners

November 25, 2020 18:57

intricate reviewed

View reviewed changes

cardano-api/src/Cardano/API.hs Outdated

Comment on lines 68 to 71

+                  fromShelleyPaymentCredential,
+                  fromShelleyStakeCredential,
                   StakeAddressReference(..),
+                  fromShelleyStakeReference,

Contributor

intricate Nov 25, 2020

w.r.t. these exports you've added, if you need them, could we export them from Cardano.Api.Shelley instead?

Contributor

dcoutts Nov 25, 2020

Yes, these should only be exported from Cardano.Api.Shelley and not from Cardano.Api

Contributor Author

deepfire Nov 25, 2020

Done.

dcoutts requested changes

View reviewed changes

Contributor

dcoutts left a comment

Yes, fine to add these definitions and export them from appropriate modules.

Comments below. Please follow the local style.

cardano-api/src/Cardano/Api/Address.hs Outdated

Comment on lines 393 to 398

+              toAddressAny = \case
+                a@ShelleyAddress{} -> AddressShelley a
+                a@ByronAddress{}   -> AddressByron a

Contributor

dcoutts Nov 25, 2020

local style would be

toAddressAny a@ShelleyAddress{} = AddressShelley a
toAddressAny a@ByronAddress{}   = AddressByron a

no need for case or lambda case at all.

Contributor

dcoutts Nov 25, 2020

This needs to be exported, both from this module and from Cardano.API and Cardano.Api.Typed

Contributor Author

deepfire Nov 25, 2020

Done.

cardano-api/src/Cardano/Api/Address.hs Outdated

@@ @@ -1,5 +1,6 @@ @@
               {-# LANGUAGE FlexibleInstances #-}
               {-# LANGUAGE GADTs #-}
+              {-# LANGUAGE LambdaCase #-}

Contributor

dcoutts Nov 25, 2020

This isn't the local style.

Contributor Author

deepfire Nov 25, 2020

Done.

cardano-api/src/Cardano/Api/Address.hs

@@ @@ -32,6 +33,7 @@ module Cardano.Api.Address ( @@
                   shelleyAddressInEra,
                   anyAddressInShelleyBasedEra,
                   anyAddressInEra,
+                  toAddressAny,

Contributor

dcoutts Nov 25, 2020

Put this in the previous patch, where it gets defined.

Contributor Author

deepfire Nov 25, 2020

Done.

cardano-api/src/Cardano/Api/Address.hs

@@ @@ -51,6 +53,10 @@ module Cardano.Api.Address ( @@
                   fromShelleyStakeAddr,
                   fromShelleyStakeCredential,
+                  fromShelleyPaymentCredential,
+                  fromShelleyStakeCredential,

Contributor

dcoutts Nov 25, 2020

Huh? This is already exported.

Contributor Author

deepfire Nov 25, 2020

Done, thank you, missed it.

cardano-api/src/Cardano/Api/Address.hs Outdated

Comment on lines 562 to 573

+              fromShelleyPaymentCredential ::
+                Shelley.PaymentCredential StandardShelley -> PaymentCredential
+              fromShelleyPaymentCredential = \case
+                Shelley.KeyHashObj    kh -> PaymentCredentialByKey (PaymentKeyHash kh)
+                Shelley.ScriptHashObj sh -> PaymentCredentialByScript (ScriptHash sh)
+              fromShelleyStakeReference ::
+                Shelley.StakeReference StandardShelley -> StakeAddressReference
+              fromShelleyStakeReference = \case
+                Shelley.StakeRefBase stakecred -> StakeAddressByValue (fromShelleyStakeCredential stakecred)
+                Shelley.StakeRefPtr ptr -> StakeAddressByPointer ptr
+                Shelley.StakeRefNull -> NoStakeAddress

Contributor

dcoutts Nov 25, 2020

Can we follow the local style (e.g. the functions immediately above) and use

foo (Shelley.Blah ..) = Blah ...

rather than lambda case.

Contributor Author

deepfire Nov 25, 2020

Done.

cardano-api/src/Cardano/API.hs Outdated

Comment on lines 68 to 71

+                  fromShelleyPaymentCredential,
+                  fromShelleyStakeCredential,
                   StakeAddressReference(..),
+                  fromShelleyStakeReference,

Contributor

dcoutts Nov 25, 2020

Yes, these should only be exported from Cardano.Api.Shelley and not from Cardano.Api

deepfire force-pushed the cad-2337-api branch 3 times, most recently from b7e7704 to 8661c8c Compare

November 25, 2020 20:58

Contributor Author

deepfire commented Nov 25, 2020

All suggestions addressed.

deepfire force-pushed the cad-2337-api branch from 8661c8c to 7af20aa Compare

November 25, 2020 22:02

deepfire added 2 commits

November 26, 2020 12:47


          CAD-2337 api: toAddressAny :: Address addr -> AddressAny

627e76c


          CAD-2337 api: fromShelley{Payment,Stake}{Credential,Reference}

8c1afb3

deepfire force-pushed the cad-2337-api branch from 7af20aa to 8c1afb3 Compare

November 26, 2020 09:48

dcoutts approved these changes

View reviewed changes

Contributor

dcoutts left a comment

LGTM

Contributor Author

deepfire commented Nov 26, 2020

bors r+

iohk-bors bot added a commit that referenced this pull request


          Merge #2125 #2127 #2130

dd973bc

2125: Update cardano-node-cli-reference.md r=kevinhammond a=kevinhammond

Added additional documentation for query ledger/protocol-state

2127: CAD-2337 API extensions for address and credentials r=deepfire a=deepfire

- `toAddressAny :: Address addr -> AddressAny`
- `fromShelleyPaymentCredential ::  Shelley.PaymentCredential StandardShelley -> PaymentCredential`
- `fromShelleyStakeReference ::  Shelley.StakeReference StandardShelley -> StakeAddressReference`
- export `fromShelleyStakeCredential`

2130: Testnet with more configurable parameters r=newhoggy a=newhoggy



Co-authored-by: Kevin Hammond <12563287+kevinhammond@users.noreply.github.com>
Co-authored-by: Kosyrev Serge <serge.kosyrev@iohk.io>
Co-authored-by: John Ky <john.ky@iohk.io>

Contributor

iohk-bors bot commented Nov 26, 2020

This PR was included in a batch that timed out, it will be automatically retried

intricate approved these changes

View reviewed changes

Contributor

iohk-bors bot commented Nov 26, 2020

Build succeeded:

iohk-bors bot merged commit 63ba413 into master

iohk-bors bot deleted the cad-2337-api branch

November 26, 2020 16:24

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

dcoutts dcoutts approved these changes

intricate intricate approved these changes

erikd Awaiting requested review from erikd

Jimbo4350 Awaiting requested review from Jimbo4350

mrBliss Awaiting requested review from mrBliss

newhoggy Awaiting requested review from newhoggy

Labels

None yet