Skip to content
This repository has been archived by the owner on Aug 18, 2020. It is now read-only.

Commit

Permalink
[CBR-401] For the validate-address API endpoint, add an IsOurs field …
Browse files Browse the repository at this point in the history
…to clarify the ambiguity of an address not being recognised
  • Loading branch information
uroboros committed Sep 21, 2018
1 parent ea0dbbd commit 0c9994c
Show file tree
Hide file tree
Showing 7 changed files with 65 additions and 9 deletions.
2 changes: 1 addition & 1 deletion wallet-new/integration/Functions.hs
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
3 changes: 2 additions & 1 deletion wallet-new/src/Cardano/Wallet/API/V1/Migration/Types.hs
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,8 @@ instance Migrate V0.CAddress V1.WalletAddress where
eitherMigrate V0.CAddress{..} = do
addrId <- eitherMigrate cadId
let addrUsed = cadIsUsed
let addrChangeAddress = cadIsChange
addrChangeAddress = cadIsChange
addrIsOurs = V1.addressIsOursFromUsed addrUsed
return V1.WalletAddress{..}

-- | Migrates to a V1 `SyncProgress` by computing the percentage as
Expand Down
20 changes: 17 additions & 3 deletions wallet-new/src/Cardano/Wallet/API/V1/Swagger.hs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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") `setDescription` getAddressDescription
40 changes: 40 additions & 0 deletions wallet-new/src/Cardano/Wallet/API/V1/Types.hs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ module Cardano.Wallet.API.V1.Types (
, NewExternalWallet (..)
, WalletAndTxHistory (..)
-- * Addresses
, AddressIsOurs (..)
, AddressIndex
, AddressValidity (..)
, AddressAsBase58
Expand All @@ -67,6 +68,7 @@ module Cardano.Wallet.API.V1.Types (
, AddressPath
, AddressLevel
, addressLevelToWord32
, addressIsOursFromUsed
, word32ToAddressLevel
, IsChangeAddress (..)
, mkAddressPathBIP44
Expand Down Expand Up @@ -1202,6 +1204,41 @@ 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, `AddressNotRecognised` makes an inconclusive statement about
-- an address, whereas `AddressIsOurs` is unambiguous.
data AddressIsOurs = AddressIsOurs
| AddressNotRecognised
deriving (Show, Eq, Generic, Ord)

addressIsOursFromUsed :: Bool -> V1 AddressIsOurs
addressIsOursFromUsed used =
V1 $ if used then AddressIsOurs else AddressNotRecognised

instance ToJSON (V1 AddressIsOurs) where
toJSON (V1 AddressIsOurs) = String "is-ours"
toJSON (V1 AddressNotRecognised) = String "not-recognised"

instance FromJSON (V1 AddressIsOurs) where
parseJSON (String "is-ours") = pure (V1 AddressIsOurs)
parseJSON (String "not-recognised") = pure (V1 AddressNotRecognised)
parseJSON x = typeMismatch "Not a valid AddressIsOurs" x

instance ToSchema (V1 AddressIsOurs) where
declareNamedSchema _ =
pure $ NamedSchema (Just "V1AddressIsOurs") $ mempty
& type_ .~ SwaggerString
& enum_ ?~ ["is-ours", "not-recognised"]

instance Arbitrary (V1 AddressIsOurs) where
arbitrary = fmap V1 $ oneof
[ pure AddressIsOurs
, pure AddressNotRecognised
]

--------------------------------------------------------------------------------
-- Accounts
--------------------------------------------------------------------------------
Expand All @@ -1211,6 +1248,7 @@ data WalletAddress = WalletAddress
{ addrId :: !(V1 Core.Address)
, addrUsed :: !Bool
, addrChangeAddress :: !Bool
, addrIsOurs :: !(V1 AddressIsOurs)
} deriving (Show, Eq, Generic, Ord)

deriveJSON Serokell.defaultOptions ''WalletAddress
Expand All @@ -1221,12 +1259,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.")
& ("isOurs" --^ "'is-ours' if this address is recognised as ours, 'not-recognised' if it has not (yet) been encountered")
)

instance Arbitrary WalletAddress where
arbitrary = WalletAddress <$> arbitrary
<*> arbitrary
<*> arbitrary
<*> arbitrary

newtype AccountIndex = AccountIndex { getAccIndex :: Word32 }
deriving (Show, Eq, Ord, Generic)
Expand Down
7 changes: 3 additions & 4 deletions wallet-new/src/Cardano/Wallet/WalletLayer/Kernel/Addresses.hs
Original file line number Diff line number Diff line change
Expand Up @@ -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.AddressNotRecognised)



Expand Down Expand Up @@ -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.AddressNotRecognised'.
return V1.WalletAddress {
addrId = V1 cardanoAddress
, addrUsed = False
, addrChangeAddress = False
, addrIsOurs = (V1 V1.AddressNotRecognised)
}
1 change: 1 addition & 0 deletions wallet-new/src/Cardano/Wallet/WalletLayer/Kernel/Conv.hs
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ toAddress acc hdAddress =
V1.WalletAddress (V1 cardanoAddress)
(addressMeta ^. addressMetaIsUsed)
(addressMeta ^. addressMetaIsChange)
(V1.addressIsOursFromUsed $ addressMeta ^. addressMetaIsUsed)
where
cardanoAddress = hdAddress ^. HD.hdAddressAddress . fromDb
addressMeta = acc ^. HD.hdAccountState . HD.hdAccountStateCurrent . cpAddressMeta cardanoAddress
Expand Down
1 change: 1 addition & 0 deletions wallet-new/test/unit/Test/Spec/Addresses.hs
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,7 @@ spec = describe "Addresses" $ do
addrId = V1.V1 randomAddr
, addrUsed = False
, addrChangeAddress = False
, addrIsOurs = V1.V1 V1.AddressNotRecognised
}
withAddressFixtures 1 $ \_ layer _ _ -> do
res <- WalletLayer.validateAddress layer (sformat build randomAddr)
Expand Down

0 comments on commit 0c9994c

Please sign in to comment.