diff --git a/wallet-new/integration/Functions.hs b/wallet-new/integration/Functions.hs index cdce0774dce..266157ba274 100644 --- a/wallet-new/integration/Functions.hs +++ b/wallet-new/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 diff --git a/wallet-new/src/Cardano/Wallet/API/V1/Migration/Types.hs b/wallet-new/src/Cardano/Wallet/API/V1/Migration/Types.hs index 73b8313ed3f..9711d7e43da 100644 --- a/wallet-new/src/Cardano/Wallet/API/V1/Migration/Types.hs +++ b/wallet-new/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 diff --git a/wallet-new/src/Cardano/Wallet/API/V1/Swagger.hs b/wallet-new/src/Cardano/Wallet/API/V1/Swagger.hs index 70b2f2786d1..8a805e8cf18 100644 --- a/wallet-new/src/Cardano/Wallet/API/V1/Swagger.hs +++ b/wallet-new/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 diff --git a/wallet-new/src/Cardano/Wallet/API/V1/Types.hs b/wallet-new/src/Cardano/Wallet/API/V1/Types.hs index 4613f080ca2..deafe1c6ae4 100644 --- a/wallet-new/src/Cardano/Wallet/API/V1/Types.hs +++ b/wallet-new/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) diff --git a/wallet-new/src/Cardano/Wallet/Util.hs b/wallet-new/src/Cardano/Wallet/Util.hs index a1bf5f28c61..c3232fa414e 100644 --- a/wallet-new/src/Cardano/Wallet/Util.hs +++ b/wallet-new/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 diff --git a/wallet-new/src/Cardano/Wallet/WalletLayer/Kernel/Addresses.hs b/wallet-new/src/Cardano/Wallet/WalletLayer/Kernel/Addresses.hs index 73a2a693ea9..bd1f6cfe8cd 100644 --- a/wallet-new/src/Cardano/Wallet/WalletLayer/Kernel/Addresses.hs +++ b/wallet-new/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) } diff --git a/wallet-new/src/Cardano/Wallet/WalletLayer/Kernel/Conv.hs b/wallet-new/src/Cardano/Wallet/WalletLayer/Kernel/Conv.hs index 195df9eaf86..457d1a5050c 100644 --- a/wallet-new/src/Cardano/Wallet/WalletLayer/Kernel/Conv.hs +++ b/wallet-new/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 diff --git a/wallet-new/test/MarshallingSpec.hs b/wallet-new/test/MarshallingSpec.hs index e9f6f5d4bc7..d5e04e7f3a3 100644 --- a/wallet-new/test/MarshallingSpec.hs +++ b/wallet-new/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 diff --git a/wallet-new/test/unit/Test/Spec/Addresses.hs b/wallet-new/test/unit/Test/Spec/Addresses.hs index a7d6c42bafc..25e297f1fdb 100644 --- a/wallet-new/test/unit/Test/Spec/Addresses.hs +++ b/wallet-new/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)