From 156d85ccc0fce4fe150aff8a35cb5e694c9a1f93 Mon Sep 17 00:00:00 2001 From: Jonathan Knowles Date: Wed, 15 May 2024 11:28:12 +0000 Subject: [PATCH 1/8] Rename error constructor to `ErrSubmitTransactionMissingWitnesses`. This more closely matches the API error name: `ApiErrorMissingWitnessesInTransaction`. --- lib/api/src/Cardano/Wallet/Api/Http/Server/Error.hs | 2 +- lib/api/src/Cardano/Wallet/Api/Http/Shelley/Server.hs | 4 ++-- lib/wallet/src/Cardano/Wallet.hs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/api/src/Cardano/Wallet/Api/Http/Server/Error.hs b/lib/api/src/Cardano/Wallet/Api/Http/Server/Error.hs index 625d68acbed..abfe99e2ad5 100644 --- a/lib/api/src/Cardano/Wallet/Api/Http/Server/Error.hs +++ b/lib/api/src/Cardano/Wallet/Api/Http/Server/Error.hs @@ -646,7 +646,7 @@ instance IsServerError ErrSubmitTransaction where , "wallet and cannot be sent. Submit a transaction that has " , "either an input or a withdrawal belonging to the wallet." ] - ErrSubmitTransactionPartiallySignedOrNoSignedTx + ErrSubmitTransactionMissingWitnesses expectedWitsNo foundWitsNo -> flip (apiError err403) message $ MissingWitnessesInTransaction diff --git a/lib/api/src/Cardano/Wallet/Api/Http/Shelley/Server.hs b/lib/api/src/Cardano/Wallet/Api/Http/Shelley/Server.hs index 32c8dc6f5fa..1a89c0c651f 100644 --- a/lib/api/src/Cardano/Wallet/Api/Http/Shelley/Server.hs +++ b/lib/api/src/Cardano/Wallet/Api/Http/Shelley/Server.hs @@ -3749,7 +3749,7 @@ submitTransaction ctx apiw@(ApiT wid) apitx = do when (witsRequiredForInputs > totalNumberOfWits) $ liftHandler . throwE - $ ErrSubmitTransactionPartiallySignedOrNoSignedTx + $ ErrSubmitTransactionMissingWitnesses witsRequiredForInputs totalNumberOfWits void $ withWorkerCtx ctx wid liftE liftE $ \wrk -> do @@ -3899,7 +3899,7 @@ submitSharedTransaction ctx apiw@(ApiT wid) apitx = do paymentWitsRequired + fromIntegral delegationWitsRequired when (allWitsRequired > totalNumberOfWits) $ liftHandler $ throwE $ - ErrSubmitTransactionPartiallySignedOrNoSignedTx + ErrSubmitTransactionMissingWitnesses allWitsRequired totalNumberOfWits let txCtx = defaultTransactionCtx diff --git a/lib/wallet/src/Cardano/Wallet.hs b/lib/wallet/src/Cardano/Wallet.hs index 07eb6164376..295c1aecd18 100644 --- a/lib/wallet/src/Cardano/Wallet.hs +++ b/lib/wallet/src/Cardano/Wallet.hs @@ -3651,7 +3651,7 @@ data ErrSignPayment -- | Errors that can occur when submitting a transaction. data ErrSubmitTransaction = ErrSubmitTransactionForeignWallet - | ErrSubmitTransactionPartiallySignedOrNoSignedTx Int Int + | ErrSubmitTransactionMissingWitnesses Int Int | ErrSubmitTransactionMultidelegationNotSupported deriving (Show, Eq) From 384e2425707d4ee4ac4690126e386fdaa95727d9 Mon Sep 17 00:00:00 2001 From: Jonathan Knowles Date: Wed, 15 May 2024 11:36:03 +0000 Subject: [PATCH 2/8] Add type `ErrSubmitTransactionMissingWitnessCounts`. --- lib/api/src/Cardano/Wallet/Api/Http/Server/Error.hs | 6 +++++- lib/api/src/Cardano/Wallet/Api/Http/Shelley/Server.hs | 5 ++++- lib/wallet/src/Cardano/Wallet.hs | 11 ++++++++++- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/lib/api/src/Cardano/Wallet/Api/Http/Server/Error.hs b/lib/api/src/Cardano/Wallet/Api/Http/Server/Error.hs index abfe99e2ad5..1d6d720a74b 100644 --- a/lib/api/src/Cardano/Wallet/Api/Http/Server/Error.hs +++ b/lib/api/src/Cardano/Wallet/Api/Http/Server/Error.hs @@ -64,6 +64,7 @@ import Cardano.Wallet , ErrStakePoolDelegation (..) , ErrStartTimeLaterThanEndTime (..) , ErrSubmitTransaction (..) + , ErrSubmitTransactionMissingWitnessCounts (..) , ErrSubmitTx (..) , ErrUpdatePassphrase (..) , ErrWalletAlreadyExists (..) @@ -647,7 +648,10 @@ instance IsServerError ErrSubmitTransaction where , "either an input or a withdrawal belonging to the wallet." ] ErrSubmitTransactionMissingWitnesses - expectedWitsNo foundWitsNo -> + ErrSubmitTransactionMissingWitnessCounts + { expectedNumberOfKeyWits = expectedWitsNo + , detectedNumberOfKeyWits = foundWitsNo + } -> flip (apiError err403) message $ MissingWitnessesInTransaction ApiErrorMissingWitnessesInTransaction diff --git a/lib/api/src/Cardano/Wallet/Api/Http/Shelley/Server.hs b/lib/api/src/Cardano/Wallet/Api/Http/Shelley/Server.hs index 1a89c0c651f..1bc90cf7962 100644 --- a/lib/api/src/Cardano/Wallet/Api/Http/Shelley/Server.hs +++ b/lib/api/src/Cardano/Wallet/Api/Http/Shelley/Server.hs @@ -189,6 +189,7 @@ import Cardano.Wallet , ErrReadRewardAccount (..) , ErrSignPayment (..) , ErrSubmitTransaction (..) + , ErrSubmitTransactionMissingWitnessCounts (..) , ErrUpdatePassphrase (..) , ErrWalletAlreadyExists (..) , ErrWalletNotResponding (..) @@ -3750,6 +3751,7 @@ submitTransaction ctx apiw@(ApiT wid) apitx = do when (witsRequiredForInputs > totalNumberOfWits) $ liftHandler . throwE $ ErrSubmitTransactionMissingWitnesses + $ ErrSubmitTransactionMissingWitnessCounts witsRequiredForInputs totalNumberOfWits void $ withWorkerCtx ctx wid liftE liftE $ \wrk -> do @@ -3899,7 +3901,8 @@ submitSharedTransaction ctx apiw@(ApiT wid) apitx = do paymentWitsRequired + fromIntegral delegationWitsRequired when (allWitsRequired > totalNumberOfWits) $ liftHandler $ throwE $ - ErrSubmitTransactionMissingWitnesses + ErrSubmitTransactionMissingWitnesses $ + ErrSubmitTransactionMissingWitnessCounts allWitsRequired totalNumberOfWits let txCtx = defaultTransactionCtx diff --git a/lib/wallet/src/Cardano/Wallet.hs b/lib/wallet/src/Cardano/Wallet.hs index 295c1aecd18..08d612563ae 100644 --- a/lib/wallet/src/Cardano/Wallet.hs +++ b/lib/wallet/src/Cardano/Wallet.hs @@ -161,6 +161,7 @@ module Cardano.Wallet , ErrCannotQuit (..) , ErrCannotVote (..) , ErrSubmitTransaction (..) + , ErrSubmitTransactionMissingWitnessCounts (..) -- ** Migration , createMigrationPlan @@ -3651,10 +3652,18 @@ data ErrSignPayment -- | Errors that can occur when submitting a transaction. data ErrSubmitTransaction = ErrSubmitTransactionForeignWallet - | ErrSubmitTransactionMissingWitnesses Int Int + | ErrSubmitTransactionMissingWitnesses + !ErrSubmitTransactionMissingWitnessCounts | ErrSubmitTransactionMultidelegationNotSupported deriving (Show, Eq) +data ErrSubmitTransactionMissingWitnessCounts = + ErrSubmitTransactionMissingWitnessCounts + { expectedNumberOfKeyWits :: !Int + , detectedNumberOfKeyWits :: !Int + } + deriving (Show, Eq) + -- | Errors that can occur when constructing an unsigned transaction. data ErrConstructTx = ErrConstructTxWrongPayload From e2a1ea3af19c8d7d9dd47c40df17541be844a212 Mon Sep 17 00:00:00 2001 From: Jonathan Knowles Date: Wed, 15 May 2024 11:43:21 +0000 Subject: [PATCH 3/8] Change type of `{detected,expected}NumberOfKeyWits` fields to `Natural`. --- lib/api/src/Cardano/Wallet/Api/Http/Server/Error.hs | 4 ++-- lib/api/src/Cardano/Wallet/Api/Http/Shelley/Server.hs | 6 ++++-- lib/wallet/src/Cardano/Wallet.hs | 4 ++-- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/lib/api/src/Cardano/Wallet/Api/Http/Server/Error.hs b/lib/api/src/Cardano/Wallet/Api/Http/Server/Error.hs index 1d6d720a74b..df0186346c1 100644 --- a/lib/api/src/Cardano/Wallet/Api/Http/Server/Error.hs +++ b/lib/api/src/Cardano/Wallet/Api/Http/Server/Error.hs @@ -655,8 +655,8 @@ instance IsServerError ErrSubmitTransaction where flip (apiError err403) message $ MissingWitnessesInTransaction ApiErrorMissingWitnessesInTransaction - { expectedNumberOfKeyWits = fromIntegral expectedWitsNo - , detectedNumberOfKeyWits = fromIntegral foundWitsNo + { expectedNumberOfKeyWits = expectedWitsNo + , detectedNumberOfKeyWits = foundWitsNo } where message = mconcat diff --git a/lib/api/src/Cardano/Wallet/Api/Http/Shelley/Server.hs b/lib/api/src/Cardano/Wallet/Api/Http/Shelley/Server.hs index 1bc90cf7962..d257c62666c 100644 --- a/lib/api/src/Cardano/Wallet/Api/Http/Shelley/Server.hs +++ b/lib/api/src/Cardano/Wallet/Api/Http/Shelley/Server.hs @@ -3752,7 +3752,8 @@ submitTransaction ctx apiw@(ApiT wid) apitx = do $ liftHandler . throwE $ ErrSubmitTransactionMissingWitnesses $ ErrSubmitTransactionMissingWitnessCounts - witsRequiredForInputs totalNumberOfWits + (fromIntegral witsRequiredForInputs) + (fromIntegral totalNumberOfWits) void $ withWorkerCtx ctx wid liftE liftE $ \wrk -> do let tx = walletTx $ decodeTx tl era sealedTx @@ -3903,7 +3904,8 @@ submitSharedTransaction ctx apiw@(ApiT wid) apitx = do liftHandler $ throwE $ ErrSubmitTransactionMissingWitnesses $ ErrSubmitTransactionMissingWitnessCounts - allWitsRequired totalNumberOfWits + (fromIntegral allWitsRequired) + (fromIntegral totalNumberOfWits) let txCtx = defaultTransactionCtx { txValidityInterval = (Nothing, ttl) diff --git a/lib/wallet/src/Cardano/Wallet.hs b/lib/wallet/src/Cardano/Wallet.hs index 08d612563ae..b168b42b0e4 100644 --- a/lib/wallet/src/Cardano/Wallet.hs +++ b/lib/wallet/src/Cardano/Wallet.hs @@ -3659,8 +3659,8 @@ data ErrSubmitTransaction data ErrSubmitTransactionMissingWitnessCounts = ErrSubmitTransactionMissingWitnessCounts - { expectedNumberOfKeyWits :: !Int - , detectedNumberOfKeyWits :: !Int + { expectedNumberOfKeyWits :: !Natural + , detectedNumberOfKeyWits :: !Natural } deriving (Show, Eq) From af0a8a39e8783bc8d1a8e4ba4ac2992af68c9dd6 Mon Sep 17 00:00:00 2001 From: Jonathan Knowles Date: Wed, 15 May 2024 11:48:39 +0000 Subject: [PATCH 4/8] Don't alias `{detected,expected}NumberOfKeyWits` in pattern match. --- lib/api/src/Cardano/Wallet/Api/Http/Server/Error.hs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/api/src/Cardano/Wallet/Api/Http/Server/Error.hs b/lib/api/src/Cardano/Wallet/Api/Http/Server/Error.hs index df0186346c1..94098e79054 100644 --- a/lib/api/src/Cardano/Wallet/Api/Http/Server/Error.hs +++ b/lib/api/src/Cardano/Wallet/Api/Http/Server/Error.hs @@ -649,21 +649,21 @@ instance IsServerError ErrSubmitTransaction where ] ErrSubmitTransactionMissingWitnesses ErrSubmitTransactionMissingWitnessCounts - { expectedNumberOfKeyWits = expectedWitsNo - , detectedNumberOfKeyWits = foundWitsNo + { expectedNumberOfKeyWits + , detectedNumberOfKeyWits } -> flip (apiError err403) message $ MissingWitnessesInTransaction ApiErrorMissingWitnessesInTransaction - { expectedNumberOfKeyWits = expectedWitsNo - , detectedNumberOfKeyWits = foundWitsNo + { expectedNumberOfKeyWits + , detectedNumberOfKeyWits } where message = mconcat [ "The transaction expects " - , toText expectedWitsNo + , toText expectedNumberOfKeyWits , " witness(es) to be fully-signed but " - , toText foundWitsNo + , toText detectedNumberOfKeyWits , " was provided." , " Please submit a fully-signed transaction." ] From 5b7add5b829352598ab7b1351dec29186fed7a5d Mon Sep 17 00:00:00 2001 From: Jonathan Knowles Date: Thu, 16 May 2024 03:32:20 +0000 Subject: [PATCH 5/8] Extract out function `validateWitnessCounts`. Use `intCastMaybe` and `fromJustNote` with unique and identifiable error message instead of `fromIntegral`. --- .../Cardano/Wallet/Api/Http/Shelley/Server.hs | 42 +++++++++++++------ 1 file changed, 30 insertions(+), 12 deletions(-) diff --git a/lib/api/src/Cardano/Wallet/Api/Http/Shelley/Server.hs b/lib/api/src/Cardano/Wallet/Api/Http/Shelley/Server.hs index d257c62666c..530589d7883 100644 --- a/lib/api/src/Cardano/Wallet/Api/Http/Shelley/Server.hs +++ b/lib/api/src/Cardano/Wallet/Api/Http/Shelley/Server.hs @@ -764,6 +764,9 @@ import Data.Generics.Labels import Data.Generics.Product ( typed ) +import Data.IntCast + ( intCastMaybe + ) import Data.List ( isInfixOf , sortOn @@ -856,6 +859,9 @@ import Network.Wai.Middleware.ServerError import Numeric.Natural ( Natural ) +import Safe + ( fromJustNote + ) import Servant ( Application , NoContent (..) @@ -3702,6 +3708,24 @@ toOut ((TxOut addr (TokenBundle c tmap)), (Just path)) = , derivationPath = NE.map ApiT path } +validateWitnessCounts + :: Int + -- ^ expected number of key witnesses + -> Int + -- ^ detected number of key witnesses + -> Either ErrSubmitTransaction () +validateWitnessCounts expected detected + | expected > detected = Left $ + ErrSubmitTransactionMissingWitnesses $ + ErrSubmitTransactionMissingWitnessCounts + { expectedNumberOfKeyWits = toNatural expected + , detectedNumberOfKeyWits = toNatural detected + } + | otherwise = Right () + where + toNatural :: Int -> Natural + toNatural = fromJustNote "validateWitnessCounts.toNatural" . intCastMaybe + submitTransaction :: forall ctx s k n . ( ctx ~ ApiLayer s @@ -3748,12 +3772,9 @@ submitTransaction ctx apiw@(ApiT wid) apitx = do when (countJoinsQuits (apiDecoded ^. #certificates) > 1) $ liftHandler $ throwE ErrSubmitTransactionMultidelegationNotSupported - when (witsRequiredForInputs > totalNumberOfWits) - $ liftHandler . throwE - $ ErrSubmitTransactionMissingWitnesses - $ ErrSubmitTransactionMissingWitnessCounts - (fromIntegral witsRequiredForInputs) - (fromIntegral totalNumberOfWits) + liftHandler $ except $ validateWitnessCounts + witsRequiredForInputs + totalNumberOfWits void $ withWorkerCtx ctx wid liftE liftE $ \wrk -> do let tx = walletTx $ decodeTx tl era sealedTx @@ -3900,12 +3921,9 @@ submitSharedTransaction ctx apiw@(ApiT wid) apitx = do fromIntegral pWitsPerInput * witsRequiredForInputs let allWitsRequired = paymentWitsRequired + fromIntegral delegationWitsRequired - when (allWitsRequired > totalNumberOfWits) $ - liftHandler $ throwE $ - ErrSubmitTransactionMissingWitnesses $ - ErrSubmitTransactionMissingWitnessCounts - (fromIntegral allWitsRequired) - (fromIntegral totalNumberOfWits) + liftHandler $ except $ validateWitnessCounts + allWitsRequired + totalNumberOfWits let txCtx = defaultTransactionCtx { txValidityInterval = (Nothing, ttl) From ac48c37b86177feff3bf15b52b68c52d8c4177a9 Mon Sep 17 00:00:00 2001 From: Jonathan Knowles Date: Thu, 16 May 2024 05:01:08 +0000 Subject: [PATCH 6/8] Use `ExceptT` in `validateWitnessCounts`. --- .../Cardano/Wallet/Api/Http/Shelley/Server.hs | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/lib/api/src/Cardano/Wallet/Api/Http/Shelley/Server.hs b/lib/api/src/Cardano/Wallet/Api/Http/Shelley/Server.hs index 530589d7883..f855cc7d8dd 100644 --- a/lib/api/src/Cardano/Wallet/Api/Http/Shelley/Server.hs +++ b/lib/api/src/Cardano/Wallet/Api/Http/Shelley/Server.hs @@ -3709,19 +3709,20 @@ toOut ((TxOut addr (TokenBundle c tmap)), (Just path)) = } validateWitnessCounts - :: Int + :: Monad m + => Int -- ^ expected number of key witnesses -> Int -- ^ detected number of key witnesses - -> Either ErrSubmitTransaction () + -> ExceptT ErrSubmitTransaction m () validateWitnessCounts expected detected - | expected > detected = Left $ + | expected > detected = throwE $ ErrSubmitTransactionMissingWitnesses $ ErrSubmitTransactionMissingWitnessCounts { expectedNumberOfKeyWits = toNatural expected , detectedNumberOfKeyWits = toNatural detected } - | otherwise = Right () + | otherwise = pure () where toNatural :: Int -> Natural toNatural = fromJustNote "validateWitnessCounts.toNatural" . intCastMaybe @@ -3772,9 +3773,7 @@ submitTransaction ctx apiw@(ApiT wid) apitx = do when (countJoinsQuits (apiDecoded ^. #certificates) > 1) $ liftHandler $ throwE ErrSubmitTransactionMultidelegationNotSupported - liftHandler $ except $ validateWitnessCounts - witsRequiredForInputs - totalNumberOfWits + liftHandler $ validateWitnessCounts witsRequiredForInputs totalNumberOfWits void $ withWorkerCtx ctx wid liftE liftE $ \wrk -> do let tx = walletTx $ decodeTx tl era sealedTx @@ -3921,9 +3920,7 @@ submitSharedTransaction ctx apiw@(ApiT wid) apitx = do fromIntegral pWitsPerInput * witsRequiredForInputs let allWitsRequired = paymentWitsRequired + fromIntegral delegationWitsRequired - liftHandler $ except $ validateWitnessCounts - allWitsRequired - totalNumberOfWits + liftHandler $ validateWitnessCounts allWitsRequired totalNumberOfWits let txCtx = defaultTransactionCtx { txValidityInterval = (Nothing, ttl) From 78ceba78b4ead148745af75c02ba9125b5228185 Mon Sep 17 00:00:00 2001 From: Jonathan Knowles Date: Thu, 16 May 2024 05:03:15 +0000 Subject: [PATCH 7/8] Use `when` in `validateWitnessCounts`. --- lib/api/src/Cardano/Wallet/Api/Http/Shelley/Server.hs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/api/src/Cardano/Wallet/Api/Http/Shelley/Server.hs b/lib/api/src/Cardano/Wallet/Api/Http/Shelley/Server.hs index f855cc7d8dd..88fdd84134b 100644 --- a/lib/api/src/Cardano/Wallet/Api/Http/Shelley/Server.hs +++ b/lib/api/src/Cardano/Wallet/Api/Http/Shelley/Server.hs @@ -3715,14 +3715,13 @@ validateWitnessCounts -> Int -- ^ detected number of key witnesses -> ExceptT ErrSubmitTransaction m () -validateWitnessCounts expected detected - | expected > detected = throwE $ +validateWitnessCounts expected detected = + when (expected > detected) $ throwE $ ErrSubmitTransactionMissingWitnesses $ ErrSubmitTransactionMissingWitnessCounts { expectedNumberOfKeyWits = toNatural expected , detectedNumberOfKeyWits = toNatural detected } - | otherwise = pure () where toNatural :: Int -> Natural toNatural = fromJustNote "validateWitnessCounts.toNatural" . intCastMaybe From 0bf79f7b6ffbe332d037b8c9a3bc6221fc5c6eae Mon Sep 17 00:00:00 2001 From: Jonathan Knowles Date: Thu, 16 May 2024 05:17:04 +0000 Subject: [PATCH 8/8] Save space in `IsServerError` instance for `ErrSubmitTransaction`. --- lib/api/src/Cardano/Wallet/Api/Http/Server/Error.hs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/lib/api/src/Cardano/Wallet/Api/Http/Server/Error.hs b/lib/api/src/Cardano/Wallet/Api/Http/Server/Error.hs index 94098e79054..e136aac3694 100644 --- a/lib/api/src/Cardano/Wallet/Api/Http/Server/Error.hs +++ b/lib/api/src/Cardano/Wallet/Api/Http/Server/Error.hs @@ -649,15 +649,11 @@ instance IsServerError ErrSubmitTransaction where ] ErrSubmitTransactionMissingWitnesses ErrSubmitTransactionMissingWitnessCounts - { expectedNumberOfKeyWits - , detectedNumberOfKeyWits - } -> + {expectedNumberOfKeyWits, detectedNumberOfKeyWits} -> flip (apiError err403) message $ MissingWitnessesInTransaction ApiErrorMissingWitnessesInTransaction - { expectedNumberOfKeyWits - , detectedNumberOfKeyWits - } + {expectedNumberOfKeyWits, detectedNumberOfKeyWits} where message = mconcat [ "The transaction expects "