Skip to content

Commit

Permalink
Merge #3275
Browse files Browse the repository at this point in the history
3275: Unify calculation of wallet id for multisig wallets 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] I have unified how wallet id is calculated in multisig wallets. Basically I wanted to achieve the following:
1. for a given wallet created from mnemonic and account pubic key (and having the same payment/staking script) we end up with the same wallet id. This is not the case for shelley wallets where wallet id from mnemonic and account public key are different as the first is calculated on pub key of rootK, and the second is calculated based on account public key. I think this is wrong, and for normal wallet account public key should be the source of wallet id calculation! As multisig are not operational yet, I propose to use the unified calculation of wallet id 
2. As we can have situation that for a given account public key we can have many different multisig wallet, I propose to add script(s) imprint to accXPub and after that hash it. thanks to that we could delete a given shared wallet (using wallet id), rather than many shared wallets sharing accXPub, we have distinct differentiation of them. 
3. Why to take imprint from Script Cosigner rather than Script KeyHash? Because we can have pending shared wallet and not completed Script KeyHash value, but we need wallet id even in pending stage as DB is affected and the shared walet already started lifecycle

### Comments

<!-- Additional comments, links, or screenshots to attach, if any. -->

### Issue Number
adp-1621
<!-- 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]>
  • Loading branch information
iohk-bors[bot] and paweljakubas authored May 19, 2022
2 parents 4a62cef + 9318550 commit 3202968
Show file tree
Hide file tree
Showing 9 changed files with 178 additions and 11 deletions.
4 changes: 2 additions & 2 deletions cabal.project
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ source-repository-package
type: git
location: https://github.com/biocad/servant-openapi3
tag: 4165b837d3a71debd1059c3735460075840000b5

-- TODO: ADP-1713
source-repository-package
type: git
Expand All @@ -80,7 +80,7 @@ source-repository-package
source-repository-package
type: git
location: https://github.com/input-output-hk/cardano-addresses
tag: 8bf98905b903455196495e231b23613ad2264cb0
tag: b33e0f365550bd9d329bdbb0a0d2dfe2b23a3dcf
subdir: command-line
core

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,11 @@ import Cardano.Mnemonic
( MkSomeMnemonic (..) )
import Cardano.Wallet.Api.Types
( ApiAccountKeyShared (..)
, ApiActiveSharedWallet
, ApiAddress
, ApiFee (..)
, ApiSharedWallet (..)
, ApiT (..)
, ApiTransaction
, ApiWallet
, DecodeAddress
Expand Down Expand Up @@ -135,6 +137,7 @@ spec :: forall n.
, EncodeAddress n
) => SpecWith Context
spec = describe "SHARED_WALLETS" $ do

it "SHARED_WALLETS_CREATE_01 - Create an active shared wallet from root xprv" $ \ctx -> runResourceT $ do
m15txt <- liftIO $ genMnemonics M15
m12txt <- liftIO $ genMnemonics M12
Expand Down Expand Up @@ -189,6 +192,36 @@ spec = describe "SHARED_WALLETS" $ do
(`shouldBe` DerivationIndex 2147483678)
]

it "SHARED_WALLETS_CREATE_01 - Compare wallet ids" $ \ctx -> runResourceT $ do
m15txt <- liftIO $ genMnemonics M15
m12txt <- liftIO $ genMnemonics M12
let (Right m15) = mkSomeMnemonic @'[ 15 ] m15txt
let (Right m12) = mkSomeMnemonic @'[ 12 ] m12txt
let passphrase = Passphrase $ BA.convert $ T.encodeUtf8 fixturePassphrase
let index = 30
let accXPubDerived = accPubKeyFromMnemonics m15 (Just m12) index passphrase
let payloadPost = Json [json| {
"name": "Shared Wallet",
"mnemonic_sentence": #{m15txt},
"mnemonic_second_factor": #{m12txt},
"passphrase": #{fixturePassphrase},
"account_index": "30H",
"payment_script_template":
{ "cosigners":
{ "cosigner#0": #{accXPubDerived} },
"template":
{ "all":
[ "cosigner#0",
{ "active_from": 120 }
]
}
}
} |]
rPost <- postSharedWallet ctx Default payloadPost
verify (fmap (view #wallet) <$> rPost)
[ expectResponseCode HTTP.status201
]

let wal = getFromResponse id rPost

let payloadKey = Json [json|{
Expand All @@ -214,6 +247,118 @@ spec = describe "SHARED_WALLETS" $ do
let ApiAccountKeyShared bytes' _ _ = getFromResponse id aKey
hexText bytes' `shouldBe` accXPubDerived

let (ApiSharedWallet (Right walActive)) = wal

rDel <- request @ApiActiveSharedWallet ctx
(Link.deleteWallet @'Shared walActive) Default Empty
expectResponseCode HTTP.status204 rDel

(_, accXPubTxt):_ <- liftIO $ genXPubs 1
let payloadAcctOther = Json [json| {
"name": "Shared Wallet",
"account_public_key": #{accXPubTxt},
"account_index": "30H",
"payment_script_template":
{ "cosigners":
{ "cosigner#0": #{accXPubTxt} },
"template":
{ "all":
[ "cosigner#0",
{ "active_from": 120 }
]
}
}
} |]
rPostAcctOther <- postSharedWallet ctx Default payloadAcctOther
verify (fmap (view #wallet) <$> rPostAcctOther)
[ expectResponseCode HTTP.status201 ]
let walAcctOther = getFromResponse id rPostAcctOther
let (ApiSharedWallet (Right walAcctOtherActive)) = walAcctOther

(walAcctOtherActive ^. #id) `shouldNotBe` (walActive ^. #id)

let payloadAcctSame = Json [json| {
"name": "Shared Wallet",
"account_public_key": #{accXPubDerived},
"account_index": "30H",
"payment_script_template":
{ "cosigners":
{ "cosigner#0": #{accXPubDerived} },
"template":
{ "all":
[ "cosigner#0",
{ "active_from": 120 }
]
}
}
} |]
rPostAcctSame <- postSharedWallet ctx Default payloadAcctSame
verify (fmap (view #wallet) <$> rPostAcctSame)
[ expectResponseCode HTTP.status201 ]
let walAcctSame = getFromResponse id rPostAcctSame
let (ApiSharedWallet (Right walAcctSameActive)) = walAcctSame

(walAcctSameActive ^. #id) `shouldBe` (walActive ^. #id)

let payloadAcctSameOtherScript = Json [json| {
"name": "Shared Wallet",
"account_public_key": #{accXPubDerived},
"account_index": "30H",
"payment_script_template":
{ "cosigners":
{ "cosigner#0": #{accXPubDerived} },
"template":
{ "all":
[ "cosigner#0",
{ "active_from": 100 }
]
}
}
} |]
rPostAcctSameOtherScript <- postSharedWallet ctx Default payloadAcctSameOtherScript
verify (fmap (view #wallet) <$> rPostAcctSameOtherScript)
[ expectResponseCode HTTP.status201 ]
let walAcctSameOtherScript = getFromResponse id rPostAcctSameOtherScript
let (ApiSharedWallet (Right walAcctSameOtherScriptActive)) = walAcctSameOtherScript

(walAcctSameOtherScriptActive ^. #id) `shouldNotBe` (walActive ^. #id)

-- In cardano-addresses
-- $ cat phrase.prv
-- rib kiwi begin other second pool raise prosper inspire forum keep stereo option ride region
--
-- $ cardano-address key from-recovery-phrase Shared < phrase.prv > root.shared_xsk
--
-- $ cardano-address key child 1854H/1815H/0H < root.shared_xsk > acct.shared_xsk
--
-- $ cardano-address key walletid --spending "all [cosigner#0]" < acct.shared_xsk
-- 654a69cd246ab08aeb4d44837ff5d5ceddfbce20
it "SHARED_WALLETS_CREATE_01 - golden test comparing wallet id" $ \ctx -> runResourceT $ do
let payloadPost = Json [json| {
"name": "Shared Wallet",
"mnemonic_sentence": ["rib","kiwi","begin","other","second","pool","raise","prosper","inspire","forum","keep","stereo","option","ride","region"],
"passphrase": #{fixturePassphrase},
"account_index": "0H",
"payment_script_template":
{ "cosigners":
{ "cosigner#0": "self" },
"template":
{ "all": ["cosigner#0"]
}
}
} |]
rPost <- postSharedWallet ctx Default payloadPost
verify (fmap (view #wallet) <$> rPost)
[ expectResponseCode HTTP.status201
]

let wal = getFromResponse id rPost
let (ApiSharedWallet (Right walActive)) = wal

toText (getApiT $ walActive ^. #id) `shouldBe`
"654a69cd246ab08aeb4d44837ff5d5ceddfbce20"


it "SHARED_WALLETS_CREATE_02 - Create a pending shared wallet from root xprv" $ \ctx -> runResourceT $ do
m15txt <- liftIO $ genMnemonics M15
m12txt <- liftIO $ genMnemonics M12
Expand Down
5 changes: 3 additions & 2 deletions lib/core/src/Cardano/Wallet/Api/Server.hs
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,7 @@ import Cardano.Wallet.Primitive.AddressDiscovery.Shared
, SharedState (..)
, mkSharedStateFromAccountXPub
, mkSharedStateFromRootXPrv
, toSharedWalletId
, validateScriptTemplates
)
import Cardano.Wallet.Primitive.Delegation.UTxO
Expand Down Expand Up @@ -1054,11 +1055,11 @@ postSharedWalletFromRootXPrv ctx generateKey body = do
rootXPrv = generateKey (seed, secondFactor) pwdP
g = defaultAddressPoolGap
ix = getApiT (body ^. #accountIndex)
wid = WalletId $ digest $ publicKey rootXPrv
pTemplate = scriptTemplateFromSelf (getRawKey accXPub) $ body ^. #paymentScriptTemplate
dTemplateM = scriptTemplateFromSelf (getRawKey accXPub) <$> body ^. #delegationScriptTemplate
wName = getApiT (body ^. #name)
accXPub = publicKey $ deriveAccountPrivateKey pwdP rootXPrv (Index $ getDerivationIndex ix)
wid = WalletId $ toSharedWalletId accXPub pTemplate dTemplateM
scriptValidation = maybe RecommendedValidation getApiT (body ^. #scriptValidation)

postSharedWalletFromAccountXPub
Expand Down Expand Up @@ -1098,7 +1099,7 @@ postSharedWalletFromAccountXPub ctx liftKey body = do
wName = getApiT (body ^. #name)
(ApiAccountPublicKey accXPubApiT) = body ^. #accountPublicKey
accXPub = getApiT accXPubApiT
wid = WalletId $ digest (liftKey accXPub)
wid = WalletId $ toSharedWalletId (liftKey accXPub) pTemplate dTemplateM
scriptValidation = maybe RecommendedValidation getApiT (body ^. #scriptValidation)

scriptTemplateFromSelf :: XPub -> ApiScriptTemplateEntry -> ScriptTemplate
Expand Down
23 changes: 22 additions & 1 deletion lib/core/src/Cardano/Wallet/Primitive/AddressDiscovery/Shared.hs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ module Cardano.Wallet.Primitive.AddressDiscovery.Shared
, isShared
, retrieveAllCosigners
, validateScriptTemplates
, toSharedWalletId

, CredentialType (..)
, liftPaymentAddress
Expand All @@ -60,10 +61,12 @@ import Cardano.Address.Script
, toScriptHash
, validateScriptTemplate
)
import Cardano.Address.Script.Parser
( scriptToText )
import Cardano.Address.Style.Shelley
( Credential (..), delegationAddress, paymentAddress )
import Cardano.Crypto.Wallet
( XPrv, XPub )
( XPrv, XPub, unXPub )
import Cardano.Wallet.Primitive.AddressDerivation
( Depth (..)
, DerivationIndex (..)
Expand Down Expand Up @@ -108,6 +111,8 @@ import Control.DeepSeq
( NFData )
import Control.Monad
( unless )
import Crypto.Hash
( Blake2b_160, Digest, hash )
import Data.Either
( isRight )
import Data.Either.Combinators
Expand All @@ -134,6 +139,7 @@ import qualified Data.Foldable as F
import qualified Data.List.NonEmpty as NE
import qualified Data.Map.Strict as Map
import qualified Data.Text as T
import qualified Data.Text.Encoding as T

-- | Convenient alias for commonly used class contexts on keys.
type SupportsDiscovery (n :: NetworkDiscriminant) k =
Expand Down Expand Up @@ -597,3 +603,18 @@ liftDelegationAddress ix dTemplate (KeyFingerprint fingerprint) =
delegationCredential = DelegationFromScript . toScriptHash
dScript =
replaceCosignersWithVerKeys CA.Stake dTemplate ix

toSharedWalletId
:: (WalletKey k, k ~ SharedKey)
=> k 'AccountK XPub
-> ScriptTemplate
-> Maybe ScriptTemplate
-> Digest Blake2b_160
toSharedWalletId accXPub pTemplate dTemplateM =
hash $
(unXPub . getRawKey $ accXPub) <>
serializeScriptTemplate pTemplate <>
maybe mempty serializeScriptTemplate dTemplateM
where
serializeScriptTemplate (ScriptTemplate _ script) =
T.encodeUtf8 $ scriptToText script
4 changes: 2 additions & 2 deletions nix/materialized/stack-nix/.stack-to-nix.cache

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion nix/materialized/stack-nix/.stack-to-nix.cache.0

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion nix/materialized/stack-nix/.stack-to-nix.cache.1

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion nix/sha256map.nix
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"https://github.com/input-output-hk/cardano-addresses"."8bf98905b903455196495e231b23613ad2264cb0" = "0jzlm1gnlbvz9kify2z74v9iydy6vf39nk954r73yaddknhxrq2r";
"https://github.com/input-output-hk/cardano-addresses"."b33e0f365550bd9d329bdbb0a0d2dfe2b23a3dcf" = "05vj2fc6n8rmn3ch6115pc6l5vw6pqbv2kwbzxf3y41sa40k50vc";
"https://github.com/biocad/servant-openapi3"."4165b837d3a71debd1059c3735460075840000b5" = "1dngrr353kjhmwhn0b289jzqz5rf32llwcv79zcyq15ldpqpbib9";
"https://github.com/paolino/openapi3"."c30d0de6875d75edd64d1aac2272886528bc492d" = "0b0fzj5vrnfrc8qikabxhsnp4p8lrjpssblbh2rb7aji5hzzfli9";
"https://github.com/input-output-hk/optparse-applicative"."7497a29cb998721a9068d5725d49461f2bba0e7a" = "1gvsrg925vynwgqwplgjmp53vj953qyh3wbdf34pw21c8r47w35r";
Expand Down
2 changes: 1 addition & 1 deletion stack.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ extra-deps:

# cardano-addresses-3.9.0
- git: https://github.com/input-output-hk/cardano-addresses
commit: 8bf98905b903455196495e231b23613ad2264cb0
commit: b33e0f365550bd9d329bdbb0a0d2dfe2b23a3dcf
subdirs:
- command-line
- core
Expand Down

0 comments on commit 3202968

Please sign in to comment.