From 729c665f962348ea0508eb43f44f1690495782e2 Mon Sep 17 00:00:00 2001 From: KtorZ Date: Mon, 26 Oct 2020 10:57:14 +0100 Subject: [PATCH 1/2] revise error messages and error code for DerivationIndex The current errors were saying black and white at the same time. Understanding what was a valid a value for derivation index and what was not was unclear. --- .../Scenario/API/Shelley/Wallets.hs | 4 +-- lib/core/src/Cardano/Wallet/Api/Server.hs | 7 +++-- lib/core/src/Cardano/Wallet/Api/Types.hs | 1 + .../Wallet/Primitive/AddressDerivation.hs | 22 ++++++++------- .../test/unit/Cardano/Wallet/Api/Malformed.hs | 12 ++++---- .../test/unit/Cardano/Wallet/Api/TypesSpec.hs | 28 +++++++++++++------ 6 files changed, 45 insertions(+), 29 deletions(-) diff --git a/lib/core-integration/src/Test/Integration/Scenario/API/Shelley/Wallets.hs b/lib/core-integration/src/Test/Integration/Scenario/API/Shelley/Wallets.hs index df4612c57c2..2872595e59a 100644 --- a/lib/core-integration/src/Test/Integration/Scenario/API/Shelley/Wallets.hs +++ b/lib/core-integration/src/Test/Integration/Scenario/API/Shelley/Wallets.hs @@ -1194,7 +1194,7 @@ spec = describe "SHELLEY_WALLETS" $ do r <- request @ApiVerificationKey ctx link Default Empty verify r - [ expectResponseCode @IO HTTP.status400 + [ expectResponseCode @IO HTTP.status403 , expectErrorMessage "It looks like you've provided a derivation index that is out of bound." ] @@ -1270,7 +1270,7 @@ spec = describe "SHELLEY_WALLETS" $ do (Json payload) verify r - [ expectResponseCode @IO HTTP.status400 + [ expectResponseCode @IO HTTP.status403 , expectErrorMessage "It looks like you've provided a derivation index that is out of bound." ] diff --git a/lib/core/src/Cardano/Wallet/Api/Server.hs b/lib/core/src/Cardano/Wallet/Api/Server.hs index 26c094b6678..29bfc04e322 100644 --- a/lib/core/src/Cardano/Wallet/Api/Server.hs +++ b/lib/core/src/Cardano/Wallet/Api/Server.hs @@ -2764,10 +2764,11 @@ instance LiftHandler ErrDerivePublicKey where instance LiftHandler ErrInvalidDerivationIndex where handler = \case ErrIndexTooHigh maxIx _ix -> - apiError err400 BadRequest $ mconcat + apiError err403 SoftDerivationRequired $ mconcat [ "It looks like you've provided a derivation index that is " - , "out of bound. I can only derive keys up to #" - , pretty maxIx, "." + , "out of bound. The index is well-formed, but I require " + , "indexes valid for soft derivation only. That is, indexes " + , "between 0 and ", pretty maxIx, " without a suffix." ] diff --git a/lib/core/src/Cardano/Wallet/Api/Types.hs b/lib/core/src/Cardano/Wallet/Api/Types.hs index 758c68a520c..ce88dd3a021 100644 --- a/lib/core/src/Cardano/Wallet/Api/Types.hs +++ b/lib/core/src/Cardano/Wallet/Api/Types.hs @@ -817,6 +817,7 @@ data ApiErrorCode | WithdrawalNotWorth | PastHorizon | UnableToAssignInputOutput + | SoftDerivationRequired deriving (Eq, Generic, Show) -- | Defines a point in time that can be formatted as and parsed from an diff --git a/lib/core/src/Cardano/Wallet/Primitive/AddressDerivation.hs b/lib/core/src/Cardano/Wallet/Primitive/AddressDerivation.hs index 70b071b3bfa..8f2fc853c18 100644 --- a/lib/core/src/Cardano/Wallet/Primitive/AddressDerivation.hs +++ b/lib/core/src/Cardano/Wallet/Primitive/AddressDerivation.hs @@ -269,27 +269,29 @@ instance FromText DerivationIndex where where firstHardened = getIndex @'Hardened minBound + errMalformed = TextDecodingError $ unwords + [ "A derivation index must be a natural number between" + , show (getIndex @'Soft minBound) + , "and" + , show (getIndex @'Soft maxBound) + , "with an optional 'H' suffix (e.g. '1815H' or '44')." + , "Indexes without suffixes are called 'Soft'" + , "Indexes with suffixes are called 'Hardened'." + ] + parseAsScientific :: Scientific -> Either TextDecodingError DerivationIndex parseAsScientific x = case toBoundedInteger x of Just ix | ix < firstHardened -> pure $ DerivationIndex ix _ -> - Left $ TextDecodingError $ mconcat - [ "A derivation index must be a natural number between " - , show (getIndex @'Soft minBound) - , " and " - , show (getIndex @'Soft maxBound) - , "." - ] + Left errMalformed castNumber :: Text -> Either TextDecodingError Scientific castNumber txt = case readMay (T.unpack txt) of Nothing -> - Left $ TextDecodingError - "expected a number as string with an optional 'H' \ - \suffix (e.g. \"1815H\" or \"44\")." + Left errMalformed Just s -> pure s diff --git a/lib/core/test/unit/Cardano/Wallet/Api/Malformed.hs b/lib/core/test/unit/Cardano/Wallet/Api/Malformed.hs index 06a7a240599..209f67cc079 100644 --- a/lib/core/test/unit/Cardano/Wallet/Api/Malformed.hs +++ b/lib/core/test/unit/Cardano/Wallet/Api/Malformed.hs @@ -214,17 +214,17 @@ instance Malformed (PathParam (ApiT DerivationIndex)) where malformed = first PathParam <$> [ ( "patate", msgMalformed ) , ( "💩", msgMalformed ) - , ( "2147483648", msgOutOfBounds ) + , ( "2147483648", msgMalformed ) , ( "1234H1234", msgMalformed ) , ( "H", msgMalformed ) ] where msgMalformed = - "expected a number as string with an optional 'H' suffix \ - \(e.g. '1815H' or '44')." - - msgOutOfBounds = - "A derivation index must be a natural number between 0 and 2147483647." + "A derivation index must be a natural number between \ + \0 and 2147483647 with an optional 'H' suffix \ + \(e.g. '1815H' or '44'). \ + \Indexes without suffixes are called 'Soft' \ + \Indexes with suffixes are called 'Hardened'." -- -- Class instances (BodyParam) diff --git a/lib/core/test/unit/Cardano/Wallet/Api/TypesSpec.hs b/lib/core/test/unit/Cardano/Wallet/Api/TypesSpec.hs index e1a0d46c785..372ad0e2f87 100644 --- a/lib/core/test/unit/Cardano/Wallet/Api/TypesSpec.hs +++ b/lib/core/test/unit/Cardano/Wallet/Api/TypesSpec.hs @@ -471,21 +471,33 @@ spec = do it "ApiT DerivationIndex (too small)" $ do - let message = mconcat - [ "Error in $: " - , "A derivation index must be a natural number " - , "between 0 and 2147483647." + let message = unwords + [ "Error in $:" + , "A derivation index must be a natural number between" + , show (getIndex @'Soft minBound) + , "and" + , show (getIndex @'Soft maxBound) + , "with an optional 'H' suffix (e.g. '1815H' or '44')." + , "Indexes without suffixes are called 'Soft'" + , "Indexes with suffixes are called 'Hardened'." ] + let value = show $ pred $ toInteger $ getIndex @'Soft minBound Aeson.parseEither parseJSON [aesonQQ|#{value}|] `shouldBe` Left @String @(ApiT DerivationIndex) message it "ApiT DerivationIndex (too large)" $ do - let message = mconcat - [ "Error in $: " - , "A derivation index must be a natural number " - , "between 0 and 2147483647." + let message = unwords + [ "Error in $:" + , "A derivation index must be a natural number between" + , show (getIndex @'Soft minBound) + , "and" + , show (getIndex @'Soft maxBound) + , "with an optional 'H' suffix (e.g. '1815H' or '44')." + , "Indexes without suffixes are called 'Soft'" + , "Indexes with suffixes are called 'Hardened'." ] + let value = show $ succ $ toInteger $ getIndex @'Soft maxBound Aeson.parseEither parseJSON [aesonQQ|#{value}|] `shouldBe` Left @String @(ApiT DerivationIndex) message From bb6528d30562a2554f05b860fd36b9d698fd3867 Mon Sep 17 00:00:00 2001 From: KtorZ Date: Mon, 26 Oct 2020 10:57:42 +0100 Subject: [PATCH 2/2] remove 409 Conflict from possible errors of the new signature endpoint. --- specifications/api/swagger.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/specifications/api/swagger.yaml b/specifications/api/swagger.yaml index 892f0ad2b4c..256ff94b242 100644 --- a/specifications/api/swagger.yaml +++ b/specifications/api/swagger.yaml @@ -2325,7 +2325,6 @@ x-responsesPostSignatures: &responsesPostSignatures <<: *responsesErr400 <<: *responsesErr405 <<: *responsesErr406 - <<: *responsesErr409 <<: *responsesErr415 200: description: OK