Skip to content

Commit

Permalink
make sure accXPub invariance is also enforced on pending shared walle…
Browse files Browse the repository at this point in the history
…t and add tests
  • Loading branch information
paweljakubas committed Apr 15, 2021
1 parent cc6ca5e commit ef2f5d9
Show file tree
Hide file tree
Showing 7 changed files with 95 additions and 59 deletions.
Expand Up @@ -84,6 +84,7 @@ module Test.Integration.Framework.TestData
, errMsg403KeyAlreadyPresent
, errMsg403NoSuchCosigner
, errMsg403CannotUpdateThisCosigner
, errMsg403CreateIllegal
) where

import Prelude
Expand Down Expand Up @@ -534,3 +535,10 @@ errMsg403CannotUpdateThisCosigner = mconcat
[ "It looks like you've tried to update the key of a cosigner having "
, "the shared wallet's account key. Only other cosigner key(s) can be updated."
]

errMsg403CreateIllegal :: String
errMsg403CreateIllegal = mconcat
[ "It looks like you've tried to create a shared wallet "
, "with a missing account key in the script template(s). This cannot be done "
, "as the wallet's account key must be always present for each script template."
]
Expand Up @@ -61,8 +61,6 @@ import Data.Bifunctor
( second )
import Data.ByteString
( ByteString )
import Data.Either
( isRight )
import Data.Either.Extra
( eitherToMaybe )
import Data.Generics.Internal.VL.Lens
Expand Down Expand Up @@ -101,6 +99,7 @@ import Test.Integration.Framework.DSL
)
import Test.Integration.Framework.TestData
( errMsg403CannotUpdateThisCosigner
, errMsg403CreateIllegal
, errMsg403KeyAlreadyPresent
, errMsg403NoDelegationTemplate
, errMsg403NoSuchCosigner
Expand Down Expand Up @@ -163,17 +162,21 @@ spec = describe "SHARED_WALLETS" $ do
]

it "SHARED_WALLETS_CREATE_02 - Create a pending shared wallet from root xprv" $ \ctx -> runResourceT $ do
m15 <- liftIO $ genMnemonics M15
m12 <- liftIO $ genMnemonics M12
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 accXPubDerived = accPubKeyFromMnemonics m15 (Just m12) 30 passphrase
let payload = Json [json| {
"name": "Shared Wallet",
"mnemonic_sentence": #{m15},
"mnemonic_second_factor": #{m12},
"mnemonic_sentence": #{m15txt},
"mnemonic_second_factor": #{m12txt},
"passphrase": #{fixturePassphrase},
"account_index": "30H",
"payment_script_template":
{ "cosigners":
{ "cosigner#0": #{accXPubTxt0} },
{ "cosigner#0": #{accXPubDerived} },
"template":
{ "all":
[ "cosigner#0",
Expand Down Expand Up @@ -421,7 +424,7 @@ spec = describe "SHARED_WALLETS" $ do
expectResponseCode HTTP.status403 rPatch
expectErrorMessage errMsg403NoDelegationTemplate rPatch

it "SHARED_WALLETS_PATCH_05 - Can update key of cosigner in pending state" $ \ctx -> runResourceT $ do
it "SHARED_WALLETS_PATCH_05 - Cannot create shared wallet when missing wallet's account public key in template" $ \ctx -> runResourceT $ do
let payloadCreate = Json [json| {
"name": "Shared Wallet",
"account_public_key": #{accXPubTxt0},
Expand All @@ -439,18 +442,8 @@ spec = describe "SHARED_WALLETS" $ do
}
} |]
rPost <- postSharedWallet ctx Default payloadCreate
expectResponseCode HTTP.status201 rPost
let wal@(ApiSharedWallet (Left _pendingWal)) = getFromResponse id rPost

let payloadPatch = Json [json| {
"cosigner#0": #{accXPub0}
} |]

rPatch <- patchSharedWallet ctx wal Payment payloadPatch
expectResponseCode HTTP.status200 rPatch
let (ApiSharedWallet (Left pendingWal)) = getFromResponse id rPatch
let cosignerKeysPatch = pendingWal ^. #paymentScriptTemplate
liftIO $ cosigners cosignerKeysPatch `shouldBe` Map.fromList [(Cosigner 0,accXPub0)]
expectResponseCode HTTP.status403 rPost
expectErrorMessage errMsg403CreateIllegal rPost

it "SHARED_WALLETS_PATCH_06 - Can add the same cosigner key for delegation script template but not payment one" $ \ctx -> runResourceT $ do
let payloadCreate = Json [json| {
Expand All @@ -459,7 +452,7 @@ spec = describe "SHARED_WALLETS" $ do
"account_index": "30H",
"payment_script_template":
{ "cosigners":
{ "cosigner#0": #{accXPubTxt2} },
{ "cosigner#0": #{accXPubTxt0} },
"template":
{ "all":
[ "cosigner#0",
Expand All @@ -470,7 +463,7 @@ spec = describe "SHARED_WALLETS" $ do
},
"delegation_script_template":
{ "cosigners":
{ "cosigner#0": #{accXPubTxt1} },
{ "cosigner#0": #{accXPubTxt0} },
"template":
{ "all":
[ "cosigner#0",
Expand All @@ -492,21 +485,21 @@ spec = describe "SHARED_WALLETS" $ do
expectResponseCode HTTP.status200 rPatch1

let payloadPatch2 = Json [json| {
"cosigner#0": #{accXPub2}
"cosigner#0": #{accXPub0}
} |]
rPatch2 <- patchSharedWallet ctx wal Payment payloadPatch2
expectResponseCode HTTP.status403 rPatch2
expectErrorMessage (errMsg403KeyAlreadyPresent (toText Payment)) rPatch2
expectErrorMessage errMsg403CannotUpdateThisCosigner rPatch2

let payloadPatch3 = Json [json| {
"cosigner#1": #{accXPub2}
"cosigner#1": #{accXPub0}
} |]
rPatch3 <- patchSharedWallet ctx wal Payment payloadPatch3
expectResponseCode HTTP.status403 rPatch3
expectErrorMessage (errMsg403KeyAlreadyPresent (toText Payment)) rPatch3

let payloadPatch4 = Json [json| {
"cosigner#1": #{accXPub1}
"cosigner#1": #{accXPub2}
} |]
rPatch4 <- patchSharedWallet ctx wal Delegation payloadPatch4
expectResponseCode HTTP.status403 rPatch4
Expand Down Expand Up @@ -546,35 +539,6 @@ spec = describe "SHARED_WALLETS" $ do
rPatch <- patchSharedWallet ctx wal Payment payloadPatch
expectResponseCode HTTP.status403 rPatch
expectErrorMessage errMsg403CannotUpdateThisCosigner rPatch

it "SHARED_WALLETS_PATCH_08 - Active wallet needs shared wallet' account key in template" $ \ctx -> runResourceT $ do
let payloadCreate = Json [json| {
"name": "Shared Wallet",
"account_public_key": #{accXPubTxt0},
"account_index": "30H",
"payment_script_template":
{ "cosigners":
{ "cosigner#0": #{accXPubTxt1} },
"template":
{ "all":
[ "cosigner#0",
{ "active_from": 120 }
]
}
}
} |]
rPost <- postSharedWallet ctx Default payloadCreate
expectResponseCode HTTP.status201 rPost
let wal@(ApiSharedWallet (Left _pendingWal)) = getFromResponse id rPost

let payloadPatch = Json [json| {
"cosigner#0": #{accXPub0}
} |]

rPatch <- patchSharedWallet ctx wal Payment payloadPatch
expectResponseCode HTTP.status201 rPost
let (ApiSharedWallet walletConstructor) = getFromResponse id rPatch
liftIO $ isRight walletConstructor `shouldBe` True
where
xpubFromText :: Text -> Maybe XPub
xpubFromText = fmap eitherToMaybe fromHexText >=> xpubFromBytes
Expand Down
6 changes: 6 additions & 0 deletions lib/core/src/Cardano/Wallet.hs
Expand Up @@ -92,6 +92,7 @@ module Cardano.Wallet
-- * Shared Wallet
, updateCosigner
, ErrAddCosignerKey (..)
, ErrConstructSharedWallet (..)

-- ** Address
, createChangeAddress
Expand Down Expand Up @@ -2152,6 +2153,11 @@ data ErrAddCosignerKey
-- ^ Error adding this co-signer to the shared wallet.
deriving (Eq, Show)

data ErrConstructSharedWallet
= ErrConstructSharedWalletMissingKey
-- ^ The shared wallet' script template doesn't have the wallet's account public key
deriving (Eq, Show)

data ErrReadAccountPublicKey
= ErrReadAccountPublicKeyNoSuchWallet ErrNoSuchWallet
-- ^ The wallet doesn't exist?
Expand Down
18 changes: 17 additions & 1 deletion lib/core/src/Cardano/Wallet/Api/Server.hs
Expand Up @@ -123,6 +123,7 @@ import Cardano.Wallet
( ErrAddCosignerKey (..)
, ErrCannotJoin (..)
, ErrCannotQuit (..)
, ErrConstructSharedWallet (..)
, ErrCreateRandomAddress (..)
, ErrDecodeSignedTx (..)
, ErrDerivePublicKey (..)
Expand Down Expand Up @@ -315,6 +316,7 @@ import Cardano.Wallet.Primitive.AddressDiscovery.SharedState
, SharedStatePending (..)
, mkSharedStateFromAccountXPub
, mkSharedStateFromRootXPrv
, walletCreationInvariant
)
import Cardano.Wallet.Primitive.CoinSelection.MA.RoundRobin
( SelectionError (..)
Expand Down Expand Up @@ -405,7 +407,7 @@ import Control.Arrow
import Control.DeepSeq
( NFData )
import Control.Monad
( forM, forever, join, void, when, (>=>) )
( forM, forever, join, unless, void, when, (>=>) )
import Control.Monad.IO.Class
( MonadIO, liftIO )
import Control.Monad.Trans.Except
Expand Down Expand Up @@ -872,6 +874,8 @@ postSharedWalletFromRootXPrv
-> ApiSharedWalletPostDataFromMnemonics
-> Handler ApiSharedWallet
postSharedWalletFromRootXPrv ctx generateKey body = do
unless (walletCreationInvariant accXPub pTemplate dTemplateM) $
liftHandler $ throwE ErrConstructSharedWalletMissingKey
let state = mkSharedStateFromRootXPrv (rootXPrv, pwd) accIx g pTemplate dTemplateM
void $ liftHandler $ createWalletWorker @_ @s @k ctx wid
(\wrk -> W.createWallet @(WorkerCtx ctx) @s @k wrk wid wName state)
Expand All @@ -890,6 +894,7 @@ postSharedWalletFromRootXPrv ctx generateKey body = do
pTemplate = body ^. #paymentScriptTemplate
dTemplateM = body ^. #delegationScriptTemplate
wName = getApiT (body ^. #name)
accXPub = publicKey $ deriveAccountPrivateKey pwd rootXPrv accIx

postSharedWalletFromAccountXPub
:: forall ctx s k n.
Expand All @@ -908,6 +913,8 @@ postSharedWalletFromAccountXPub
-> ApiSharedWalletPostDataFromAccountPubX
-> Handler ApiSharedWallet
postSharedWalletFromAccountXPub ctx liftKey body = do
unless (walletCreationInvariant (liftKey accXPub) pTemplate dTemplateM) $
liftHandler $ throwE ErrConstructSharedWalletMissingKey
let state = mkSharedStateFromAccountXPub (liftKey accXPub) accIx g pTemplate dTemplateM
void $ liftHandler $ createWalletWorker @_ @s @k ctx wid
(\wrk -> W.createWallet @(WorkerCtx ctx) @s @k wrk wid wName state)
Expand Down Expand Up @@ -3088,6 +3095,15 @@ instance IsServerError ErrAddCosignerKey where
, "the shared wallet's account key. Only other cosigner key(s) can be updated."
]

instance IsServerError ErrConstructSharedWallet where
toServerError = \case
ErrConstructSharedWalletMissingKey ->
apiError err403 SharedWalletCreateNotAllowed $ mconcat
[ "It looks like you've tried to create a shared wallet "
, "with a missing account key in the script template(s). This cannot be done "
, "as the wallet's account key must be always present for each script template."
]

instance IsServerError (ErrInvalidDerivationIndex 'Soft level) where
toServerError = \case
ErrIndexOutOfBound minIx maxIx _ix ->
Expand Down
1 change: 1 addition & 0 deletions lib/core/src/Cardano/Wallet/Api/Types.hs
Expand Up @@ -1163,6 +1163,7 @@ data ApiErrorCode
| SharedWalletKeyAlreadyExists
| SharedWalletNoSuchCosigner
| SharedWalletCannotUpdateKey
| SharedWalletCreateNotAllowed
deriving (Eq, Generic, Show, Data, Typeable)
deriving anyclass NFData

Expand Down
Expand Up @@ -33,6 +33,7 @@ module Cardano.Wallet.Primitive.AddressDiscovery.SharedState
, purposeCIP1854
, isShared
, retrieveAllCosigners
, walletCreationInvariant
) where

import Prelude
Expand Down Expand Up @@ -276,6 +277,26 @@ sharedStateFromPending (SharedStatePending accXPub pT dT g)
mkAddressPool @n (ParentContextMultisigScript accXPub pT dT) g []
| otherwise = Nothing

accountXPubCondition
:: WalletKey k
=> k 'AccountK XPub
-> ScriptTemplate
-> Bool
accountXPubCondition accXPub (ScriptTemplate cosignerKeys _) =
getRawKey accXPub `F.elem` cosignerKeys

walletCreationInvariant
:: WalletKey k
=> k 'AccountK XPub
-> ScriptTemplate
-> Maybe ScriptTemplate
-> Bool
walletCreationInvariant accXPub pTemplate dTemplate =
isValid pTemplate && maybe True isValid dTemplate
where
isValid template' =
accountXPubCondition accXPub template'

templatesComplete
:: WalletKey k
=> k 'AccountK XPub
Expand All @@ -285,9 +306,9 @@ templatesComplete
templatesComplete accXPub pTemplate dTemplate =
isValid pTemplate && maybe True isValid dTemplate
where
isValid template'@(ScriptTemplate cosignerKeys _) =
isValid template' =
isRight (validateScriptTemplate RequiredValidation template')
&& (getRawKey accXPub `F.elem` cosignerKeys)
&& (accountXPubCondition accXPub template')

-- | Possible errors from adding a co-signer key to the shared wallet state.
data ErrAddCosigner
Expand Down
22 changes: 21 additions & 1 deletion specifications/api/swagger.yaml
Expand Up @@ -2989,6 +2989,19 @@ x-errSharedWalletCannotUpdateKey: &errSharedWalletCannotUpdateKey
type: string
enum: ['shared_wallet_cannot_update_key']

x-errSharedWalletCreateNotAllowed: &errSharedWalletCreateNotAllowed
<<: *responsesErr
title: shared_wallet_create_not_allowed
properties:
message:
type: string
description: |
Returned when a user tries to create a shared wallet with missing wallet's
account key in all the script templates.
code:
type: string
enum: ['shared_wallet_create_not_allowed']

# TODO: Map this error to the place it belongs.
x-errNetworkUnreachable: &errNetworkUnreachable
<<: *responsesErr
Expand Down Expand Up @@ -3485,6 +3498,13 @@ x-responsesPostSharedWallet: &responsesPostSharedWallet
<<: *responsesErr406
<<: *responsesErr409WalletAlreadyExists
<<: *responsesErr415UnsupportedMediaType
403:
description: Forbidden
content:
application/json:
schema:
oneOf:
- <<: *errSharedWalletCreateNotAllowed
201:
description: Created
content:
Expand Down Expand Up @@ -5112,7 +5132,7 @@ paths:
tags: ["Shared Wallets"]
summary: Create
description: |
<p align="right">status: <strong>unstable</strong></p>
<p align="right">status: <strong>stable</strong></p>
Create a shared wallet from an account public key, its index and script templates.
requestBody:
Expand Down

0 comments on commit ef2f5d9

Please sign in to comment.