Merge #2269

2269: revise error messages with regards to DerivationIndex r=KtorZ a=KtorZ

# Issue Number

<!-- Put here a reference to the issue this PR relates to and which requirements it tackles -->

ADP-509

# Overview

<!-- Detail in a few bullet points the work accomplished in this PR -->

- 0d1f207
  📍 **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.

- 96c62fb
  📍 **remove 409 Conflict from possible errors of the new signature endpoint.**

# Comments

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

<!-- 
Don't forget to:

 ✓ Self-review your changes to make sure nothing unexpected slipped through
 ✓ Assign yourself to the PR
 ✓ Assign one or several reviewer(s)
 ✓ Once created, link this PR to its corresponding ticket
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: KtorZ <[email protected]>

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."
             ]

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."
                 ]
 
 

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

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
 

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)

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

specifications/api/swagger.yaml

@@ -2325,7 +2325,6 @@ x-responsesPostSignatures: &responsesPostSignatures
     <<: *responsesErr400
     <<: *responsesErr405
     <<: *responsesErr406
-    <<: *responsesErr409
     <<: *responsesErr415
     200:
       description: OK