Merge pull request input-output-hk/cardano-sl#3646 from input-output-hk/feature/cbr-401-validate-address

[CBR-401] For the validate-address API endpoint, add an IsOurs field …

Author: adinapoli-iohk

integration/Functions.hs

@@ -560,7 +560,7 @@ runAction wc action = do
 
             let changeAddress = toList (txOutputs newTx) \\ toList paymentDestinations
                 -- NOTE: instead of this manual conversion we could filter WalletAddress from getAddressIndex
-                pdToChangeAddress PaymentDistribution{..} = WalletAddress pdAddress True True
+                pdToChangeAddress PaymentDistribution{..} = WalletAddress pdAddress True True (V1 AddressIsOurs)
                 realChangeAddressId = map addrId addressesAfterTransaction \\ map addrId addressesBeforeTransaction
                 changeWalletAddresses = filter ((`elem` realChangeAddressId) . addrId) addressesAfterTransaction
 

src/Cardano/Wallet/API/V1/Migration/Types.hs

@@ -177,8 +177,13 @@ instance Migrate V0.CAddress V1.WalletAddress where
     eitherMigrate V0.CAddress{..} = do
         addrId <- eitherMigrate cadId
         let addrUsed = cadIsUsed
-        let addrChangeAddress = cadIsChange
+            addrChangeAddress = cadIsChange
+            addrOwnership = addressOwnershipFromUsed addrUsed
         return V1.WalletAddress{..}
+      where
+        addressOwnershipFromUsed :: Bool -> V1 V1.AddressOwnership
+        addressOwnershipFromUsed used =
+            V1 $ if used then V1.AddressIsOurs else V1.AddressAmbiguousOwnership
 
 -- | Migrates to a V1 `SyncProgress` by computing the percentage as
 -- coded here: https://github.com/input-output-hk/daedalus/blob/master/app/stores/NetworkStatusStore.js#L108

src/Cardano/Wallet/API/V1/Swagger.hs

@@ -1051,6 +1051,19 @@ the `spendingPassword` field to `null` as that won't influence the fee
 calculation.
 |]
 
+getAddressDescription :: Text
+getAddressDescription = [text|
+The previous version of this endpoint failed with an HTTP error when the given
+address was unknown to the wallet.
+
+This was misleading since an address that is unknown to the wallet may still
+belong to the wallet (since it could be part of a pending transaction in
+another instance of the same wallet).
+
+To reflect this, the V1 endpoint does not fail when an address is not recognised
+and returns a new field 'is-ours' which indicates either that an address is ours,
+or that it is 'not-recognised' (which may mean 'not recognised yet').
+|]
 
 --
 -- The API
@@ -1083,7 +1096,8 @@ api (compileInfo, curSoftwareVersion) walletAPI mkDescription = toSwagger wallet
     , deSoftwareVersion       = fromString $ show curSoftwareVersion
     }
   & info.license ?~ ("MIT" & url ?~ URL "https://raw.githubusercontent.com/input-output-hk/cardano-sl/develop/lib/LICENSE")
-  & paths %~ (POST, "/api/internal/apply-update") `setDescription` applyUpdateDescription
-  & paths %~ (POST, "/api/internal/postpone-update") `setDescription` postponeUpdateDescription
+  & paths %~ (POST,   "/api/internal/apply-update")       `setDescription` applyUpdateDescription
+  & paths %~ (POST,   "/api/internal/postpone-update")    `setDescription` postponeUpdateDescription
   & paths %~ (DELETE, "/api/internal/reset-wallet-state") `setDescription` resetWalletStateDescription
-  & paths %~ (POST, "/api/v1/transactions/fees") `setDescription` estimateFeesDescription
+  & paths %~ (POST,   "/api/v1/transactions/fees")        `setDescription` estimateFeesDescription
+  & paths %~ (GET,    "/api/v1/addresses/{address}")      `setDescription` getAddressDescription

src/Cardano/Wallet/API/V1/Types.hs

@@ -46,6 +46,7 @@ module Cardano.Wallet.API.V1.Types (
   , NewExternalWallet (..)
   , WalletAndTxHistory (..)
   -- * Addresses
+  , AddressOwnership (..)
   , AddressIndex
   , AddressValidity (..)
   , AddressAsBase58
@@ -183,7 +184,7 @@ import           Cardano.Wallet.API.V1.Swagger.Example (Example, example,
                      genExample)
 import           Cardano.Wallet.Orphans.Aeson ()
 import           Cardano.Wallet.Types.UtxoStatistics
-import           Cardano.Wallet.Util (showApiUtcTime)
+import           Cardano.Wallet.Util (mkJsonKey, showApiUtcTime)
 
 import qualified Pos.Binary.Class as Bi
 import qualified Pos.Chain.Txp as Txp
@@ -265,6 +266,11 @@ genericSchemaDroppingPrefix prfx extraDoc proxy = do
           _               -> err
 
 
+optsADTCamelCase :: A.Options
+optsADTCamelCase =
+    defaultOptions { A.constructorTagModifier = mkJsonKey }
+
+
 --
 -- Versioning
 --
@@ -1202,6 +1208,35 @@ instance BuildableSafeGen AddressValidity where
     buildSafeGen _ AddressValidity{..} =
         bprint ("{ valid="%build%" }") isValid
 
+-- | An address is either recognised as "ours" or not. An address that is not
+--   recognised may still be ours e.g. an address generated by another wallet instance
+--   will not be considered "ours" until the relevant transaction is confirmed.
+--
+--   In other words, `AddressAmbiguousOwnership` makes an inconclusive statement about
+--   an address, whereas `AddressOwnership` is unambiguous.
+data AddressOwnership
+    = AddressIsOurs
+    | AddressAmbiguousOwnership
+    deriving (Show, Eq, Generic, Ord)
+
+instance ToJSON (V1 AddressOwnership) where
+    toJSON = genericToJSON optsADTCamelCase . unV1
+
+instance FromJSON (V1 AddressOwnership) where
+    parseJSON = fmap V1 . genericParseJSON optsADTCamelCase
+
+instance ToSchema (V1 AddressOwnership) where
+    declareNamedSchema _ =
+        pure $ NamedSchema (Just "V1AddressOwnership") $ mempty
+            & type_ .~ SwaggerString
+            & enum_ ?~ ["isOurs", "ambiguousOwnership"]
+
+instance Arbitrary (V1 AddressOwnership) where
+    arbitrary = fmap V1 $ oneof
+        [ pure AddressIsOurs
+        , pure AddressAmbiguousOwnership
+        ]
+
 --------------------------------------------------------------------------------
 -- Accounts
 --------------------------------------------------------------------------------
@@ -1211,6 +1246,7 @@ data WalletAddress = WalletAddress
     { addrId            :: !(V1 Core.Address)
     , addrUsed          :: !Bool
     , addrChangeAddress :: !Bool
+    , addrOwnership     :: !(V1 AddressOwnership)
     } deriving (Show, Eq, Generic, Ord)
 
 deriveJSON Serokell.defaultOptions ''WalletAddress
@@ -1221,12 +1257,14 @@ instance ToSchema WalletAddress where
             & ("id"            --^ "Actual address.")
             & ("used"          --^ "True if this address has been used.")
             & ("changeAddress" --^ "True if this address stores change from a previous transaction.")
+            & ("ownership"     --^ "'isOurs' if this address is recognised as ours, 'ambiguousOwnership' if the node doesn't have information to make a unambiguous statement.")
         )
 
 instance Arbitrary WalletAddress where
     arbitrary = WalletAddress <$> arbitrary
                               <*> arbitrary
                               <*> arbitrary
+                              <*> arbitrary
 
 newtype AccountIndex = AccountIndex { getAccIndex :: Word32 }
     deriving (Show, Eq, Ord, Generic)

src/Cardano/Wallet/Util.hs

@@ -27,7 +27,7 @@ headToLower []     = Nothing
 headToLower (x:xs) = Just $ toLower x : xs
 
 stripFieldPrefix :: String -> String
-stripFieldPrefix = dropWhile (not . isUpper)
+stripFieldPrefix = dropWhile (not . isUpper) . drop 1
 
 mkJsonKey :: String -> String
 mkJsonKey s = fromMaybe s . headToLower $ stripFieldPrefix s

src/Cardano/Wallet/WalletLayer/Kernel/Addresses.hs

@@ -51,7 +51,7 @@ createAddress wallet
     -- | Creates a new 'WalletAddress'. As this is a brand new, fresh Address,
     -- it's fine to have 'False' for both 'isUsed' and 'isChange'.
     mkAddress :: Address -> WalletAddress
-    mkAddress addr = WalletAddress (V1 addr) False False
+    mkAddress addr = WalletAddress (V1 addr) False False (V1 V1.AddressIsOurs)
 
 
 
@@ -179,11 +179,10 @@ validateAddress rawText db = runExcept $ do
         -- If the address is unknown to the wallet, it's possible that it's ours
         -- but not yet used (at least as far as we know, it may be pending in
         -- another instance of the same wallet of course), or it may be that
-        -- it's not even ours. In both cases we return that it is "not used"
-        -- (here) and "not a change address" (here). In the future we may want
-        -- to extend this endpoint with an "is ours" field.
+        -- it's not even ours. In both cases we return 'V1.AddressAmbiguousOwnership'.
         return V1.WalletAddress {
             addrId            = V1 cardanoAddress
           , addrUsed          = False
           , addrChangeAddress = False
+          , addrOwnership     = (V1 V1.AddressAmbiguousOwnership)
           }

src/Cardano/Wallet/WalletLayer/Kernel/Conv.hs

@@ -193,9 +193,17 @@ toAddress acc hdAddress =
     V1.WalletAddress (V1 cardanoAddress)
                      (addressMeta ^. addressMetaIsUsed)
                      (addressMeta ^. addressMetaIsChange)
+                     (V1 addressOwnership)
   where
-    cardanoAddress = hdAddress ^. HD.hdAddressAddress . fromDb
-    addressMeta    = acc ^. HD.hdAccountState . HD.hdAccountStateCurrent . cpAddressMeta cardanoAddress
+    cardanoAddress   = hdAddress ^. HD.hdAddressAddress . fromDb
+    addressMeta      = acc ^. HD.hdAccountState . HD.hdAccountStateCurrent .  cpAddressMeta cardanoAddress
+    -- NOTE
+    -- In this particular case, the address had to be known by us. As a matter
+    -- of fact, to construct a 'WalletAddress', we have to be aware of pieces
+    -- of informations we can only have if the address is ours (like the
+    -- parent's account derivation index required to build the 'HdAddress' in
+    -- a first place)!
+    addressOwnership = V1.AddressIsOurs
 
 -- | Converts a Kernel 'HdAddress' into a Cardano 'Address'.
 toCardanoAddress :: HD.HdAddress -> Address

test/MarshallingSpec.hs

@@ -87,6 +87,7 @@ spec = parallel $ describe "Marshalling & Unmarshalling" $ do
         aesonRoundtripProp @SyncProgress Proxy
         aesonRoundtripProp @SyncThroughput Proxy
         aesonRoundtripProp @AccountIndex Proxy
+        aesonRoundtripProp @(V1 AddressOwnership) Proxy
 
         -- HttpApiData roundtrips
         httpApiDataRoundtripProp @AccountIndex Proxy

test/unit/Test/Spec/Addresses.hs

@@ -534,6 +534,7 @@ spec = describe "Addresses" $ do
                             addrId            = V1.V1 randomAddr
                           , addrUsed          = False
                           , addrChangeAddress = False
+                          , addrOwnership     = V1.V1 V1.AddressAmbiguousOwnership
                           }
                     withAddressFixtures 1 $ \_ layer _ _ -> do
                         res <- WalletLayer.validateAddress layer (sformat build randomAddr)