Address derivation for shared wallets #2624

paweljakubas · 2021-04-26T19:12:17Z

Issue Number

ADP-866

Overview

Adding Shared style
Accommodate new cardano-addresses, align code, change MutableAccount -> Stake
Deal with prefixes of shared wallet vs shelley wallet by adding SharedWalletKeys
Align all unit tests
Align all integration tests plus add test checking newly added validation in cardano-addresses
Add integration testing
Enable receiving hashed verification keys, now only verification keys are present which needs to be hashed to be used in script
Sneak SharedKey in shared wallet rather than ShelleyKey
Removed MultisigScript from Role and refactored ParentContext

Comments

Rendered API docs

KtorZ · 2021-04-27T07:50:27Z

lib/core/src/Cardano/Wallet/Primitive/AddressDerivation.hs

@@ -184,7 +185,7 @@ data Depth
 data Role
    = UtxoExternal
    | UtxoInternal
-    | MutableAccount
+    | Stake


This will cause a breaking change in the API which generically derive path parameters from these constructors. I agree with the change here, but the JSON instance of the API should be modified to also still accept mutable_account if provided to not break existing integrations.

after some thinking I reverted this change

KtorZ · 2021-04-27T07:53:29Z

lib/core/src/Cardano/Wallet/DB/Sqlite/Types.hs

+                _ -> Left $ TextDecodingError "KeyHash should begin with either '0' or '1'."
+            Nothing -> Left $ TextDecodingError "KeyHash should not be empty."
+        bimap textDecodingError (KeyHash keyRole)
+            $ convertFromBase Base16 $ T.encodeUtf8 txt'


This is a red-herring to me 🤔 ...

Why would you need to differentiate them in the database? Surely, the context when deserializing from the database is enough to assign the right role to the key hash. If it's not, then it should likely go in a separate column, instead of being serialized all at once.

Regardless, this will require a database migration and I fail to see how existing databases can be migrated 🤔 ? (or said differently, why is this distinction only occurs now?)

I checked that KeyHash is not stored. These instances are not needed in the end. so removed them. They are some remnants of past version of shared wallet I forgot to erase.

paweljakubas · 2021-04-27T12:36:53Z

lib/core/src/Cardano/Wallet/Primitive/AddressDerivation/Shared.hs

+        SharedKey
+
+    keyTypeDescriptor _ =
+        "shared"


I have doubts what should be here to be honest

This is the name then used to prefix database's names. In other wallet scheme, we typically use a 3-letter identifier:

she

ica

rnd

I'd say that for consistency, we probably want to also have a 3-letter name here but I am perhaps being too maniac 😬

went for sha

KtorZ · 2021-04-28T08:21:56Z

lib/core/src/Cardano/Wallet.hs

+
+    return addrK
+  where
+    db = ctx ^. dbLayer @IO @s @k


This code looks very similar to derivePublicKeyShelley. Both function could be unified rather easily by providing the account key lookup as a function:

derivePublicKey :: forall ctx s k n. ( HasDBLayer IO s k ctx , SoftDerivation k ) => ctx -> WalletId -> (s -> k 'AccountK XPub) -> Role -> DerivationIndex -> ExceptT ErrDerivePublicKey IO (k 'AddressK XPub) derivePublicKey ctx wid getAccountKey role_ ix = ...

lib/core/src/Cardano/Wallet/Api.hs

KtorZ · 2021-04-28T08:25:49Z

lib/core/src/Cardano/Wallet/Primitive/AddressDerivation/Shared.hs

+        SharedKey
+
+    keyTypeDescriptor _ =
+        "shared"


This is the name then used to prefix database's names. In other wallet scheme, we typically use a 3-letter identifier:

she

ica

rnd

I'd say that for consistency, we probably want to also have a 3-letter name here but I am perhaps being too maniac 😬

paweljakubas · 2021-04-29T17:18:49Z

@KtorZ @rvl I added PaymentAddress instances to make it pass - at this level (we are not discovering, sending txs) it is not needed. But this is to be changed.. This bits should be used https://github.com/input-output-hk/cardano-wallet/blob/master/lib/core/src/Cardano/Wallet/Primitive/AddressDiscovery/Script.hs#L189 I need to think how to approach it. Would prefer to do it in the next PR addressing discovery ..

paweljakubas · 2021-04-29T19:53:07Z

specifications/api/swagger.yaml

@@ -5199,3 +5231,40 @@ paths:
          application/json:
            schema: *ApiSharedWalletPatchData
      responses: *responsesPatchSharedWallet
+
+  /shared-wallets/{walletId}/keys/{role}/{index}?hash=false:


I am not sure if query params should be exposed like this.Nevertheless,

openapi-spec-validator --schema 3.0.0 specifications/api/swagger.yaml

passes

I fixed it for you.

KtorZ · 2021-05-04T13:17:28Z

lib/core-integration/src/Test/Integration/Framework/DSL.hs

+    request @ApiVerificationKeyShared ctx link Default Empty
+getSharedWalletKey ctx (ApiSharedWallet (Right w)) role ix hashed = do
+    let link = Link.getSharedWalletKey w role ix hashed
+    request @ApiVerificationKeyShared ctx link Default Empty


Minor note: this implicitly suggests to me that both implementations are different, whereas they aren't. I would favour a prismatic accessor to extract the wallet from the ApiSharedWallet instead, or simply pattern match on that field only inside the function body, e.g.

let w = case apiSharedWalletOf ... let link = ... request @...

KtorZ · 2021-05-04T13:18:23Z

lib/core-integration/src/Test/Integration/Framework/DSL.hs

+    request @ApiAccountKeyShared ctx link headers payload
+postAccountKeyShared ctx (ApiSharedWallet (Right w)) ix headers payload = do
+    let link = Link.postAccountKeyShared w ix
+    request @ApiAccountKeyShared ctx link headers payload


Same here ☝️

done, also for other Shared-related helpers

KtorZ · 2021-05-04T13:28:45Z

lib/core-integration/src/Test/Integration/Scenario/API/Shelley/Addresses.hs

+                        "from" : [
+                            "addr_shared_vkh1zxt0uvrza94h3hv4jpv0ttddgnwkvdgeyq8jf9w30mcs6y8w3nq",
+                            "stake_shared_vkh1nac0awgfa4zjsh4elnjmsscz0huhss8q2g0x3n7m539mwaa5m7s"
+                            ],


KtorZ · 2021-05-04T13:29:18Z

lib/core-integration/src/Test/Integration/Scenario/API/Shelley/Addresses.hs

+            }|]
+        r <- request @AnyAddress ctx Link.postAnyAddress Default payload
+        expectResponseCode HTTP.status400 r
+        let msg = "All keys of a script must have the same role: either payment or delegation."


This should not be hard-coded here but rather defined in the Framework/TestData module for lack of a better mechanism.

done, also for other errors connected with scripts

KtorZ · 2021-05-04T13:34:55Z

lib/core-integration/src/Test/Integration/Scenario/API/Shelley/SharedWallets.hs

@@ -119,8 +127,8 @@ spec = describe "SHARED_WALLETS" $ do
                          }
                    }
                } |]


Note that, I think this doesn't generally reflect a real use-case. Users don't generally know how to get their account public key from their recovery phrase. A typical scenario when creating from a mnemonic would be to:

Create a shared wallet with an empty mapping of cosigners

Query the API to get the account key associated with the wallet

Complete the script template by providing the mapping for my own key

Likely, the step 2 & 3 would automatically be done by Daedalus on the behalf of the end-user; still I'd love to see our scenarios reflecting this and not resort on custom accPubKeyFromMnemonics ;) ... In the end, the goal of the API scenarios is to described capabilities doable only via the API.

I imagine that this may be conflicting with the invariant we are trying to enforce, right? So it may be necessary to either:

Provide a way to easily derive keys from a given mnemonic phrase via the API (less preferred approach)

Allow maybe for assigning a special string "self" to cosigners, such that one could submit:
{ "cosigner#0": "self" },
And let the server replace it with the correct account key internally.

yes, it is conflicting with invariant we introduced in the previous shared wallet PR. I will try with self though

done, added illustrative integration tests, aligned swagger and core unit tests

KtorZ · 2021-05-04T14:39:36Z

lib/core/src/Cardano/Wallet/Primitive/AddressDerivation.hs

+    keyRole = case role' of
+        UtxoExternal -> CA.Payment
+        MutableAccount -> CA.Delegation
+        _ -> error "verification keys make sense only for payment (role=0) and delegation (role=2)"


And role=1; The type signature is not restricted to purpose=1854 so role=1 should be possible here, or the type signature should be further restricted.

KtorZ · 2021-05-04T14:41:44Z

lib/core/src/Cardano/Wallet/Primitive/AddressDerivation/Shared.hs

+    paymentKeyFingerprint (_, paymentK) =
+        Right $ KeyFingerprint $ blake2b224 $ xpubPublicKey $ getKey paymentK
+
+instance PaymentAddress 'Mainnet SharedKey where


I don't think this instance make sense for SharedKey. An address (payment or delegation) can in principle only be constructed from the state using the script template!

there was unnecessary Payment n SharedKeyconstraint in Sqlite's PersistentState. Now it is removed

KtorZ · 2021-05-04T15:17:04Z

lib/core/src/Cardano/Wallet/Api/Types.hs

+    , hashed :: VerificationKeyHashing
+    } deriving (Eq, Generic, Show)
+      deriving anyclass NFData
+
 data ApiPostAccountKeyData = ApiPostAccountKeyData
    { passphrase :: ApiT (Passphrase "raw")
    , extended :: Bool


Only seeing this now, and I think we can do better than Bool here.

introduced data KeyFormat = Extended | NonExtended as a replacement

KtorZ · 2021-05-04T15:19:39Z

lib/shelley/src/Cardano/Wallet/Shelley/Api/Server.hs

-        :<|> patchSharedWallet multisig ShelleyKey Delegation
-        :<|> deleteWallet multisig
+             postSharedWallet @_ @_ @SharedKey multisig Shared.generateKeyFromSeed SharedKey
+        :<|> (\wid -> withMultisigLayer wid


Cf earlier comment, I don't think this indirection is necessary.

KtorZ · 2021-05-04T15:20:18Z

lib/shelley/src/Cardano/Wallet/Shelley/Transaction.hs

@@ -247,6 +249,9 @@ instance TxWitnessTagFor IcarusKey where
 instance TxWitnessTagFor ByronKey where
    txWitnessTagFor = TxWitnessByronUTxO Byron

+instance TxWitnessTagFor SharedKey where


This instance looks very suspicious to me. This should not be needed for Shared wallets since they are signed independently and using a different process. Plus, TxWitnessTagFor is used for calculating the size of the resulting transaction; in the case of shared scripts this also needs to acknowledge for the script in the witness which require more changes to the code down the line.

removed newTransactionLayer from apiLayer for the time being. Will return to it when txs are to be supported for shared wallets

piotr-iohk · 2021-05-05T13:32:50Z

specifications/api/swagger.yaml

+  /shared-wallets/{walletId}/keys/{role}/{index}?hash=false:
+    get:
+      operationId: getSharedWalletKey
+      tags: ["Keys"]


fixed in c68a6bc.

piotr-iohk · 2021-05-05T13:32:10Z

Question:
For shared wallet created from acc pub key one cannot get acc pub key:

curl -X POST http://localhost:8090/v2/shared-wallets/f70d591d06a83517e7e46a69a3da58cb52baf60e/keys/0H \
-d '{"passphrase":"Secure Passphrase","extended":true}' \
-H "Content-Type: application/json"

{"code":"no_root_key","message":"I couldn't find a root private key for the given wallet: f70d591d06a83517e7e46a69a3da58cb52baf60e. However, this operation requires that I do have such a key. Either there's no such wallet, or I don't fully own it."}

The same behavior is actually for shelley wallet from acc pub key and it's corresponding endpoint, however I'm wondering if it is in fact desired behavior? 🤔

rvl · 2021-05-07T07:31:16Z

For shared/shelley wallet created from acc pub key one cannot get acc pub key.

That seems like a bug to me @piotr-iohk, although the impact is low given that the user would presumably already know their acc pub key.

KtorZ

Not a small PR 🙃 , it would be good to try to limit the number of changes which goes in a single PR in the future to make iterations less painful. For examples:

Tackle only one endpoint at a time
Work on core changes first in isolation (e.g. the SharedKey module) before wiring it up into all layers of the code
Do database changes separately

Modulo some final minor remarks, I think the PR is okay now. Though quite large also and therefore, it is possible that I have missed something. Before we completely close the shared story and call it done I'd like to have some time to play with the API from a user perspective as a form of acceptance test and I invite @piotr-iohk to do the same. It may be a good opportunity to also write an extensive user guide about that!

KtorZ · 2021-05-12T06:25:12Z

lib/core/src/Cardano/Wallet/Api/Server.hs

@@ -668,6 +680,7 @@ postWallet
        , IsOurs s RewardAccount
        , Typeable s
        , Typeable n
+        , (k == SharedKey) ~ 'False


Interesting. I thought GHC would figure it out 🤷

KtorZ · 2021-05-12T06:27:29Z

lib/core/src/Cardano/Wallet/Primitive/AddressDiscovery.hs

+    getPurpose :: Index 'Hardened 'PurposeK
+
+-- It is used for geting account public key for a given state.
+class GetAccount s (key :: Depth -> Type -> Type) | s -> key  where


Why the functional dependency here 🤔 ?

we have s ~ SomethingKey n key, and return key ~Account XPub, and I wanted to enforce that key is from s...

KtorZ · 2021-05-12T06:30:53Z

lib/shelley/src/Cardano/Wallet/Shelley.hs

@@ -288,7 +290,7 @@ serveWallet
                    Server.idleWorker
                shelleyApi <- apiLayer (newTransactionLayer net) nl
                    (Server.manageRewardBalance proxy)
-                multisigApi <- apiLayer (newTransactionLayer net) nl
+                multisigApi <- apiLayer undefined nl


Whoops? I understand you want to do this in a second PR and there's currently nothing wired into the transaction layer but may we use a more descriptive error than undefined here 😬 ?

KtorZ · 2021-05-12T06:32:42Z

specifications/api/swagger.yaml

@@ -4109,6 +4186,7 @@ x-tagGroups:
  - name: Shelley (Shared Wallets)
    tags:
    - Shared Wallets
+    - Shared Keys


Small note: I'd put the Shelley (Shared Wallets) section before Byron (Random). Byron is legacy so it makes more sense to have it presented last 👍

rvl

It's looking good - almost there!

rvl · 2021-05-12T06:43:09Z

lib/core/src/Cardano/Wallet.hs

@@ -2186,6 +2186,7 @@ updateCosigner
        , Typeable n
        , WalletKey k
        , HasDBLayer IO s k ctx
+        , k ~ SharedKey


Just wondering aloud, could the constraints in these functions be made shorter because the types are basically fixed?

we can think of introducing constriant groups probably but I am not sure and do not know if they will be more clear. Personally, I am accustomed to explicit constrains. But I was one day looking at ledger repo and they introduced those composite constraints and I think they serve well, but they have situation that one the same composite constraints is in many places...

rvl · 2021-05-12T06:43:10Z

lib/core/src/Cardano/Wallet/Api/Server.hs

@@ -668,6 +679,7 @@ postWallet
        , IsOurs s RewardAccount
        , Typeable s
        , Typeable n
+        , (k == SharedKey) ~ 'False


I am a bit confused by this.

so we use this here https://hackage.haskell.org/package/base-4.15.0.0/docs/src/Data-Type-Equality.html#%3D%3D and we needed as we introduced different ParentContext for SharedKey and the rest (ShelleyKey/IcarusKey..). So we want here to say that k must not be SharedKey. for SharedKey we just specify k ~ SharedKey

rvl · 2021-05-12T06:43:10Z

lib/core/src/Cardano/Wallet/Api/Server.hs

+    -> ApiT DerivationIndex
+    -> ApiPostAccountKeyData
+    -> Handler ApiAccountKeyShared
+postAccountPublicKeyShared ctx (ApiT wid) (ApiT ix) (ApiPostAccountKeyData (ApiT pwd) extd) = do


This function looks almost idential to postAccountPublicKeyShelley. Likewise for the JSON instances of ApiAccountKeyShared.

indeed, good point. adopted postAccountPublicKey in both Shelley and Shared cases

rvl · 2021-05-12T06:43:10Z

lib/core/src/Cardano/Wallet/Api/Server.hs

+    -> ApiT DerivationIndex
+    -> ApiPostAccountKeyData
+    -> Handler ApiAccountKeyShared
+postAccountPublicKeyShared ctx (ApiT wid) (ApiT ix) (ApiPostAccountKeyData (ApiT pwd) extd) = do
    withWorkerCtx @_ @s @k ctx wid liftE liftE $ \wrk -> do


I suggest combining them and adding a hrp prefix string as a type parameter or something like that.

rvl · 2021-05-12T06:43:10Z

lib/shelley/src/Cardano/Wallet/Shelley.hs

@@ -288,7 +290,7 @@ serveWallet
                    Server.idleWorker
                shelleyApi <- apiLayer (newTransactionLayer net) nl
                    (Server.manageRewardBalance proxy)
-                multisigApi <- apiLayer (newTransactionLayer net) nl
+                multisigApi <- apiLayer undefined nl


I'm guessing there is some tricky type error?

no no...there is required TxWintnessTag instance and I want to leave it for ADP-686. And for the current endpoints tx later is not engaged

rvl · 2021-05-12T06:43:11Z

lib/shelley/src/Cardano/Wallet/Shelley/Api/Server.hs

-        :<|> patchSharedWallet multisig ShelleyKey Delegation
-        :<|> deleteWallet multisig
+    sharedWallets
+        :: ApiLayer (SharedState n SharedKey) SharedKey


The other server functions get their ctx from the function scope.
These ones could also, right?

the problem is that it takes ctx type from the above context and we have ApiLayer (SharedStaten ShelleyKey) ShelleyKey ... so I needed to do this trick to make it tick...Nevertheless, I will relook if I can do something in coming PR....

rvl · 2021-05-12T06:43:11Z

specifications/api/swagger.yaml

+        Create a shared wallet from an account public key and script
+        template index.
+
+        **TODO:** ADP-866 is it also restored? If not, when?


Would it be possible to describe what happens when the shared wallet is created? Is it restored immediately, or must further API requests be made, etc, etc.

rvl · 2021-05-12T06:43:11Z

specifications/api/swagger.yaml

@@ -5259,6 +5340,8 @@ paths:
      summary: Get
      description: |
        <p align="right">status: <strong>⚠ under development</strong></p>
+
+        **TODO:** ADP-866 document this endpoint.


I put these TODO messages in for you ;-)

paweljakubas · 2021-05-12T12:26:00Z

bors r+

2624: Address derivation for shared wallets r=paweljakubas a=paweljakubas # Issue Number ADP-866 # Overview - [x] Adding Shared style - [x] Accommodate new cardano-addresses, align code, change MutableAccount -> Stake - [x] Deal with prefixes of shared wallet vs shelley wallet by adding SharedWalletKeys - [x] Align all unit tests - [x] Align all integration tests plus add test checking newly added validation in cardano-addresses - [x] Add integration testing - [x] Enable receiving hashed verification keys, now only verification keys are present which needs to be hashed to be used in script - [x] Sneak SharedKey in shared wallet rather than ShelleyKey - [x] Removed MultisigScript from Role and refactored ParentContext # Comments [Rendered API docs](https://redocly.github.io/redoc/?url=https://raw.githubusercontent.com/input-output-hk/cardano-wallet/paweljakubas/adp-866/add-shared-style/specifications/api/swagger.yaml) Co-authored-by: Pawel Jakubas <[email protected]> Co-authored-by: IOHK <[email protected]> Co-authored-by: Piotr Stachyra <[email protected]> Co-authored-by: Rodney Lorrimar <[email protected]>

iohk-bors · 2021-05-12T12:33:50Z

Build failed:

ci/hydra-build:required

import fix

paweljakubas · 2021-05-12T12:39:31Z

bors r+

2624: Address derivation for shared wallets r=paweljakubas a=paweljakubas # Issue Number ADP-866 # Overview - [x] Adding Shared style - [x] Accommodate new cardano-addresses, align code, change MutableAccount -> Stake - [x] Deal with prefixes of shared wallet vs shelley wallet by adding SharedWalletKeys - [x] Align all unit tests - [x] Align all integration tests plus add test checking newly added validation in cardano-addresses - [x] Add integration testing - [x] Enable receiving hashed verification keys, now only verification keys are present which needs to be hashed to be used in script - [x] Sneak SharedKey in shared wallet rather than ShelleyKey - [x] Removed MultisigScript from Role and refactored ParentContext # Comments [Rendered API docs](https://redocly.github.io/redoc/?url=https://raw.githubusercontent.com/input-output-hk/cardano-wallet/paweljakubas/adp-866/add-shared-style/specifications/api/swagger.yaml) Co-authored-by: Pawel Jakubas <[email protected]> Co-authored-by: IOHK <[email protected]> Co-authored-by: Piotr Stachyra <[email protected]> Co-authored-by: Rodney Lorrimar <[email protected]>

iohk-bors · 2021-05-12T13:01:37Z

Build failed:

ci/hydra-build:required

paweljakubas · 2021-05-12T14:39:09Z

bors r+

2624: Address derivation for shared wallets r=paweljakubas a=paweljakubas # Issue Number ADP-866 # Overview - [x] Adding Shared style - [x] Accommodate new cardano-addresses, align code, change MutableAccount -> Stake - [x] Deal with prefixes of shared wallet vs shelley wallet by adding SharedWalletKeys - [x] Align all unit tests - [x] Align all integration tests plus add test checking newly added validation in cardano-addresses - [x] Add integration testing - [x] Enable receiving hashed verification keys, now only verification keys are present which needs to be hashed to be used in script - [x] Sneak SharedKey in shared wallet rather than ShelleyKey - [x] Removed MultisigScript from Role and refactored ParentContext # Comments [Rendered API docs](https://redocly.github.io/redoc/?url=https://raw.githubusercontent.com/input-output-hk/cardano-wallet/paweljakubas/adp-866/add-shared-style/specifications/api/swagger.yaml) Co-authored-by: Pawel Jakubas <[email protected]> Co-authored-by: IOHK <[email protected]> Co-authored-by: Piotr Stachyra <[email protected]> Co-authored-by: Rodney Lorrimar <[email protected]>

iohk-bors · 2021-05-12T19:02:10Z

Timed out.

paweljakubas · 2021-05-12T19:04:20Z

bors retry

2624: Address derivation for shared wallets r=paweljakubas a=paweljakubas # Issue Number ADP-866 # Overview - [x] Adding Shared style - [x] Accommodate new cardano-addresses, align code, change MutableAccount -> Stake - [x] Deal with prefixes of shared wallet vs shelley wallet by adding SharedWalletKeys - [x] Align all unit tests - [x] Align all integration tests plus add test checking newly added validation in cardano-addresses - [x] Add integration testing - [x] Enable receiving hashed verification keys, now only verification keys are present which needs to be hashed to be used in script - [x] Sneak SharedKey in shared wallet rather than ShelleyKey - [x] Removed MultisigScript from Role and refactored ParentContext # Comments [Rendered API docs](https://redocly.github.io/redoc/?url=https://raw.githubusercontent.com/input-output-hk/cardano-wallet/paweljakubas/adp-866/add-shared-style/specifications/api/swagger.yaml) Co-authored-by: Pawel Jakubas <[email protected]> Co-authored-by: IOHK <[email protected]> Co-authored-by: Piotr Stachyra <[email protected]> Co-authored-by: Rodney Lorrimar <[email protected]>

iohk-bors · 2021-05-12T19:39:44Z

Build failed:

ci/hydra-build:required

paweljakubas · 2021-05-12T19:41:23Z

bors retry

iohk-bors · 2021-05-12T20:24:26Z

Build succeeded:

paweljakubas self-assigned this Apr 26, 2021

KtorZ reviewed Apr 27, 2021

View reviewed changes

paweljakubas force-pushed the paweljakubas/adp-866/add-shared-style branch from f4443f6 to 178732d Compare April 27, 2021 12:26

paweljakubas requested a review from KtorZ April 27, 2021 12:35

paweljakubas commented Apr 27, 2021

View reviewed changes

KtorZ reviewed Apr 28, 2021

View reviewed changes

paweljakubas force-pushed the paweljakubas/adp-866/add-shared-style branch from 5643486 to 8fed84a Compare April 28, 2021 19:47

rvl mentioned this pull request Apr 29, 2021

Minting and burning draft #2627

Closed

5 tasks

paweljakubas force-pushed the paweljakubas/adp-866/add-shared-style branch 2 times, most recently from 6cccabd to db78467 Compare April 29, 2021 17:15

paweljakubas requested review from KtorZ and rvl April 29, 2021 17:16

paweljakubas marked this pull request as ready for review April 29, 2021 17:21

paweljakubas force-pushed the paweljakubas/adp-866/add-shared-style branch from db78467 to 2eede29 Compare April 29, 2021 18:08

paweljakubas commented Apr 29, 2021

View reviewed changes

paweljakubas force-pushed the paweljakubas/adp-866/add-shared-style branch from f97d0d4 to ddeafcc Compare April 30, 2021 18:56

KtorZ reviewed May 4, 2021

View reviewed changes

piotr-iohk reviewed May 5, 2021

View reviewed changes

paweljakubas force-pushed the paweljakubas/adp-866/add-shared-style branch 2 times, most recently from c491cad to 0ddcab9 Compare May 6, 2021 14:23

paweljakubas force-pushed the paweljakubas/adp-866/add-shared-style branch from 610320b to f446059 Compare May 7, 2021 11:44

paweljakubas requested review from KtorZ and piotr-iohk May 7, 2021 19:52

paweljakubas added 3 commits May 7, 2021 21:58

add Shared to style

13ca2aa

absorb new cardano-addresses plus change naming MutableAccount -> Stake

41b4087

correct prefixes for ApiVerificationKeyShelley

8b56d20

paweljakubas added 3 commits May 11, 2021 16:16

introduce GetAccount type class

71ca86c

more code cleaning

1393e6e

remove AddressDiscovery.Script module

3736064

paweljakubas requested a review from KtorZ May 11, 2021 15:31

paweljakubas and others added 3 commits May 11, 2021 18:02

hlint

2102d0f

Bump cardano-addresses to 3.4.0 tag

872d611

fixup! Regenerate nix

4de9934

rvl changed the title ~~Add shared style~~ Address derivation for shared wallets May 12, 2021

KtorZ approved these changes May 12, 2021

View reviewed changes

rvl reviewed May 12, 2021

View reviewed changes

final code improvements

e11d3b0

import fix

paweljakubas force-pushed the paweljakubas/adp-866/add-shared-style branch from 0bee2c4 to e11d3b0 Compare May 12, 2021 12:38

correct Cardano.CLISpec

61c5934

iohk-bors bot merged commit 0921f0f into master May 12, 2021

iohk-bors bot deleted the paweljakubas/adp-866/add-shared-style branch May 12, 2021 20:24

piotr-iohk mentioned this pull request May 25, 2021

Improve shared wallets #2663

Merged

8 tasks

@@ @@ -119,8 +127,8 @@ spec = describe "SHARED_WALLETS" $ do @@
                                         }
                                   }
                               } |]

Address derivation for shared wallets #2624

Address derivation for shared wallets #2624

Conversation

paweljakubas commented Apr 26, 2021 • edited Loading

Issue Number

Overview

Comments

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paweljakubas commented Apr 29, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as resolved.

Choose a reason for hiding this comment

piotr-iohk commented May 5, 2021

rvl commented May 7, 2021

KtorZ left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rvl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paweljakubas commented May 12, 2021

iohk-bors bot commented May 12, 2021

paweljakubas commented May 12, 2021

iohk-bors bot commented May 12, 2021

paweljakubas commented May 12, 2021

iohk-bors bot commented May 12, 2021

paweljakubas commented May 12, 2021

iohk-bors bot commented May 12, 2021

paweljakubas commented May 12, 2021

paweljakubas commented Apr 26, 2021 •

edited

Loading

paweljakubas commented Apr 29, 2021 •

edited

Loading