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.
cardano-foundation · Oct 26, 2020 · 0d1f207 · 0d1f207
1 parent 19a0bf6
commit 0d1f207
Show file tree

Hide file tree

Showing 6 changed files with 45 additions and 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
@@ -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
@@ -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 suffix."
                 ]
 
 

diff --git 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
@@ -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)
+            , "an "
+            , show (getIndex @'Soft maxBound)
+            , "with an optional 'H' suffix (e.g. \"1815H\" or \"44\")."
+            , "Indexes without suffix are called 'Soft'"
+            , "Indexes with suffix 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
@@ -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 suffix are called 'Soft' \
+            \Indexes with suffix 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
@@ -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)
+                  , "an "
+                  , show (getIndex @'Soft maxBound)
+                  , "with an optional 'H' suffix (e.g. \"1815H\" or \"44\")."
+                  , "Indexes without suffix are called 'Soft'"
+                  , "Indexes with suffix 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)
+                  , "an "
+                  , show (getIndex @'Soft maxBound)
+                  , "with an optional 'H' suffix (e.g. \"1815H\" or \"44\")."
+                  , "Indexes without suffix are called 'Soft'"
+                  , "Indexes with suffix are called 'Hardened'."
                   ]
+
             let value = show $ succ $ toInteger $ getIndex @'Soft maxBound
             Aeson.parseEither parseJSON [aesonQQ|#{value}|]
                 `shouldBe` Left @String @(ApiT DerivationIndex) message