Fix for the listAssets API which shows assets unrelated to a wallet

lib/core/src/Cardano/Wallet.hs

@@ -158,6 +158,7 @@ module Cardano.Wallet
     -- ** Transaction
     , forgetTx
     , listTransactions
+    , listAssets
     , getTransaction
     , submitExternalTx
     , submitTx
@@ -508,7 +509,7 @@ import Data.List.NonEmpty
 import Data.Map.Strict
     ( Map )
 import Data.Maybe
-    ( fromMaybe, mapMaybe )
+    ( fromMaybe, isJust, mapMaybe )
 import Data.Proxy
     ( Proxy )
 import Data.Quantity
@@ -2489,6 +2490,28 @@ listTransactions ctx wid mMinWithdrawal mStart mEnd order = db & \DBLayer{..} ->
                 $ slotRangeFromTimeRange
                 $ Range mStart mEnd
 
+-- | Extract assets associated with a given wallet from its transaction history.
+listAssets
+    :: forall s k ctx. (HasDBLayer IO s k ctx, IsOurs s Address)
+    => ctx
+    -> WalletId
+    -> ExceptT ErrNoSuchWallet IO (Set TokenMap.AssetId)
+listAssets ctx wid = db & \DBLayer{..} -> do
+    cp <- mapExceptT atomically $ withNoSuchWallet wid $ readCheckpoint wid
+    txs <- lift . atomically $
+        let noMinWithdrawal = Nothing
+            allTxStatuses = Nothing
+        in readTxHistory wid noMinWithdrawal Ascending wholeRange allTxStatuses
+    let txAssets :: TransactionInfo -> Set TokenMap.AssetId
+        txAssets = Set.unions
+            . map (TokenBundle.getAssets . view #tokens)
+            . filter ourOut
+            . txInfoOutputs
+        ourOut TxOut{address} = ourAddress address
+        ourAddress addr = isJust . fst . isOurs addr $ getState cp
+    pure $ Set.unions $ map txAssets txs
+  where
+    db = ctx ^. dbLayer @IO @s @k
 
 -- | Get transaction and metadata from history for a given wallet.
 getTransaction

lib/core/src/Cardano/Wallet/Api/Server.hs

@@ -1731,34 +1731,26 @@ newtype ErrListAssets = ErrListAssetsNoSuchWallet ErrNoSuchWallet
 listAssets
     :: forall ctx s k.
         ( ctx ~ ApiLayer s k
+        , IsOurs s Address
         , HasTokenMetadataClient ctx
         )
     => ctx
     -> ApiT WalletId
     -> Handler [ApiAsset]
 listAssets ctx wid = do
     assets <- listAssetsBase ctx wid
-    liftIO $ fillMetadata client assets toApiAsset
+    liftIO $ fillMetadata client (Set.toList assets) toApiAsset
   where
     client = ctx ^. tokenMetadataClient
 
 -- | Return a list of all AssetIds involved in the transaction history of this
 -- wallet.
 listAssetsBase
-    :: forall ctx s k.
-        ( ctx ~ ApiLayer s k
-        )
-    => ctx
-    -> ApiT WalletId
-    -> Handler [AssetId]
-listAssetsBase ctx (ApiT wid) = withWorkerCtx ctx wid liftE liftE $ \wrk ->
-    liftHandler $ allTxAssets <$>
-    W.listTransactions @_ @_ @_ wrk wid Nothing Nothing Nothing Descending
-  where
-    allTxAssets = Set.toList . Set.unions . map txAssets
-    txAssets = Set.unions
-        . map (TokenBundle.getAssets . view #tokens)
-        . W.txInfoOutputs
+    :: forall s k. IsOurs s Address =>
+    ApiLayer s k -> ApiT WalletId -> Handler (Set AssetId)
+listAssetsBase ctx (ApiT wallet) =
+    withWorkerCtx ctx wallet liftE liftE $ \wctx ->
+        liftHandler $ W.listAssets wctx wallet
 
 -- | Look up a single asset and its metadata.
 --
@@ -1767,6 +1759,7 @@ listAssetsBase ctx (ApiT wid) = withWorkerCtx ctx wid liftE liftE $ \wrk ->
 getAsset
     :: forall ctx s k.
         ( ctx ~ ApiLayer s k
+        , IsOurs s Address
         , HasTokenMetadataClient ctx
         )
     => ctx
@@ -1786,6 +1779,7 @@ getAsset ctx wid (ApiT policyId) (ApiT assetName) = do
 getAssetDefault
     :: forall ctx s k.
         ( ctx ~ ApiLayer s k
+        , IsOurs s Address
         , HasTokenMetadataClient ctx
         )
     => ctx

lib/core/src/Cardano/Wallet/Primitive/Model.hs

@@ -271,11 +271,11 @@ applyBlock !block (Wallet !u0 _ s0) =
 --
 applyBlocks
     :: (IsOurs s Address, IsOurs s RewardAccount)
-    => NonEmpty (Block)
+    => NonEmpty Block
     -> Wallet s
     -> NonEmpty (FilteredBlock, Wallet s)
-applyBlocks (block0 :| blocks) cp =
-    NE.scanl (flip applyBlock . snd) (applyBlock block0 cp) blocks
+applyBlocks (block0 :| blocks) walletState =
+    NE.scanl (flip applyBlock . snd) (applyBlock block0 walletState) blocks
 
 {-------------------------------------------------------------------------------
                                    Accessors
@@ -379,7 +379,7 @@ changeUTxO pending = evalState $
                                 UTxO operations
 -------------------------------------------------------------------------------}
 
--- | Applies a transaction to a UTxO, moving it from one state from another.
+-- | Applies a transaction to a UTxO, moving it from one state to another.
 --
 -- When applying a transaction to a UTxO:
 --   1. We need to remove any unspents that have been spent in the transaction.
@@ -426,7 +426,7 @@ spendTx tx !u =
 -- | Construct a 'UTxO' corresponding to a given transaction.
 --
 -- It is important for the transaction outputs to be ordered correctly,
--- as their index within this ordering determines how 
+-- as their index within this ordering determines how
 -- they are referenced as transaction inputs in subsequent blocks.
 --
 -- > balance (utxoFromTx tx) = foldMap tokens (outputs tx)
@@ -463,7 +463,7 @@ discoverAddresses block s0 = s2
     -- NOTE: Only outputs and withdrawals can potentially
     -- result in the extension of the address pool and
     -- the learning of new addresses.
-    -- 
+    --
     -- Inputs and collateral are forced to use existing addresses.
     discoverTx s tx = discoverWithdrawals (discoverOutputs s tx) tx
     discoverOutputs s tx =
@@ -529,10 +529,7 @@ applyBlockToUTxO Block{header,transactions,delegations} s u0 = (fblock, u1)
       }
     (txs1, u1) = L.foldl' applyOurTx (mempty, u0) transactions
 
-    applyOurTx
-        :: ([(Tx, TxMeta)], UTxO)
-        -> Tx
-        -> ([(Tx, TxMeta)], UTxO)
+    applyOurTx :: ([(Tx, TxMeta)], UTxO) -> Tx -> ([(Tx, TxMeta)], UTxO)
     applyOurTx (!txs, !u) !tx =
         case applyOurTxToUTxO slotNo blockHeight s tx u of
             Nothing -> (txs, u)
@@ -557,59 +554,59 @@ applyOurTxToUTxO
     -> UTxO
     -> Maybe ((Tx, TxMeta), UTxO)
 applyOurTxToUTxO !slotNo !blockHeight !s !tx !prevUTxO =
-    let
-        -- The next UTxO state (apply a state transition) (e.g. remove
-        -- transaction outputs we've spent)
-        ourNextUTxO =
-            spendTx tx prevUTxO
-            <> UTxO.filterByAddress (ours s) (utxoFromTx tx)
-        ourWithdrawalSum = ourWithdrawalSumFromTx s tx
-
-        -- Balance of the UTxO that we received and that we spent
-        received = balance (ourNextUTxO `excluding` dom prevUTxO)
-        spent =
-            balance (prevUTxO `excluding` dom ourNextUTxO)
-            `TB.add` TB.fromCoin ourWithdrawalSum
-
-        adaSpent = TB.getCoin spent
-        adaReceived = TB.getCoin received
-        dir = if adaSpent > adaReceived then Outgoing else Incoming
-        amount = distance adaSpent adaReceived
-
-        -- Transaction metadata computed from the above information
-        txmeta = TxMeta
-            { status = InLedger
-            , direction = dir
-            , slotNo
-            , blockHeight
-            , amount = amount
-            , expiry = Nothing
-            }
-
-        hasKnownWithdrawal = ourWithdrawalSum /= mempty
-
-        -- NOTE 1: The only case where fees can be 'Nothing' is when dealing with
-        -- a Byron transaction. In which case fees can actually be calculated as
-        -- the delta between inputs and outputs.
-        --
-        -- NOTE 2: We do not have in practice the actual input amounts, yet we
-        -- do make the assumption that if one input is ours, then all inputs are
-        -- necessarily ours and therefore, known as part of our current UTxO.
-        actualFee direction = case (tx ^. #fee, direction) of
-            (Just x, Outgoing) ->
-                -- Shelley and beyond:
-                Just x
-            (Nothing, Outgoing) ->
-                -- Byron:
-                let totalOut = F.fold (txOutCoin <$> outputs tx)
-                    totalIn = TB.getCoin spent
-                in
-                Just $ distance totalIn totalOut
-            (_, Incoming) ->
-                Nothing
-    in if hasKnownWithdrawal || prevUTxO /= ourNextUTxO
-        then Just ((tx {fee = actualFee dir}, txmeta), ourNextUTxO)
+    if ourWithdrawalSum /= mempty || prevUTxO /= ourNextUTxO
+        then
+            let updatedTx = tx { fee = actualFee dir }
+            in Just ((updatedTx, txmeta), ourNextUTxO)
         else Nothing
+  where
+    -- The next UTxO state (apply a state transition) (e.g. remove
+    -- transaction outputs we've spent)
+    ourNextUTxO =
+        spendTx tx prevUTxO
+        <> UTxO.filterByAddress (ours s) (utxoFromTx tx)
+    ourWithdrawalSum = ourWithdrawalSumFromTx s tx
+
+    -- Balance of the UTxO that we received and that we spent
+    received = balance (ourNextUTxO `excluding` dom prevUTxO)
+    spent =
+        balance (prevUTxO `excluding` dom ourNextUTxO)
+        `TB.add` TB.fromCoin ourWithdrawalSum
+
+    adaSpent = TB.getCoin spent
+    adaReceived = TB.getCoin received
+    dir = if adaSpent > adaReceived then Outgoing else Incoming
+    amount = distance adaSpent adaReceived
+
+    -- Transaction metadata computed from the above information
+    txmeta = TxMeta
+        { status = InLedger
+        , direction = dir
+        , slotNo
+        , blockHeight
+        , amount = amount
+        , expiry = Nothing
+        }
+
+    -- NOTE 1: The only case where fees can be 'Nothing' is when dealing with
+    -- a Byron transaction. In which case fees can actually be calculated as
+    -- the delta between inputs and outputs.
+    --
+    -- NOTE 2: We do not have in practice the actual input amounts, yet we
+    -- do make the assumption that if one input is ours, then all inputs are
+    -- necessarily ours and therefore, known as part of our current UTxO.
+    actualFee direction = case (tx ^. #fee, direction) of
+        (Just x, Outgoing) ->
+            -- Shelley and beyond:
+            Just x
+        (Nothing, Outgoing) ->
+            -- Byron:
+            let totalOut = F.fold (txOutCoin <$> outputs tx)
+                totalIn = TB.getCoin spent
+            in
+            Just $ distance totalIn totalOut
+        (_, Incoming) ->
+            Nothing
 
 ourWithdrawalSumFromTx
     :: IsOurs s RewardAccount

lib/core/test/unit/Cardano/Wallet/Primitive/ModelSpec.hs

@@ -10,6 +10,7 @@
 {-# LANGUAGE TypeApplications #-}
 {-# LANGUAGE TypeFamilies #-}
 {-# OPTIONS_GHC -fno-warn-orphans #-}
+{-# LANGUAGE TupleSections #-}
 
 module Cardano.Wallet.Primitive.ModelSpec
     ( spec
@@ -563,7 +564,7 @@ prop_changeUTxO_inner pendingTxs =
 data AllOurs = AllOurs
 
 instance IsOurs AllOurs a where
-    isOurs _ s = (Just shouldNotEvaluate, s)
+    isOurs _ = (Just shouldNotEvaluate,)
       where
         shouldNotEvaluate = error "AllOurs: unexpected evaluation"
 
@@ -732,34 +733,20 @@ prop_applyOurTxToUTxO_allOurs
     -> Property
 prop_applyOurTxToUTxO_allOurs slotNo blockHeight tx utxo =
     checkCoverage $
-    cover 50  (    haveResult)        "have result" $
+    cover 50 haveResult "have result" $
     cover 0.1 (not haveResult) "do not have result" $
-    report
-        (utxo)
-        "utxo" $
-    report
-        (utxoFromTx tx)
-        "utxoFromTx tx" $
-    report
-        (haveResult)
-        "haveResult" $
-    report
-        (shouldHaveResult)
-        "shouldHaveResult" $
+    report utxo "utxo" $
+    report (utxoFromTx tx) "utxoFromTx tx" $
+    report haveResult "haveResult" $
+    report shouldHaveResult "shouldHaveResult" $
     case maybeResult of
         Nothing ->
-            verify
-                (not shouldHaveResult)
-                "not shouldHaveResult" $
+            verify (not shouldHaveResult) "not shouldHaveResult" $
             property True
         Just utxo' ->
-            cover 10 (utxo /= utxo')
-                "utxo /= utxo'" $
-            verify
-                (shouldHaveResult)
-                "shouldHaveResult" $
-            verify
-                (utxo' == applyTxToUTxO tx utxo)
+            cover 10 (utxo /= utxo') "utxo /= utxo'" $
+            verify shouldHaveResult "shouldHaveResult" $
+            verify (utxo' == applyTxToUTxO tx utxo)
                 "utxo' == applyTxToUTxO tx utxo" $
             property True
   where
@@ -785,32 +772,19 @@ prop_applyOurTxToUTxO_someOurs
     -> Property
 prop_applyOurTxToUTxO_someOurs ourState slotNo blockHeight tx utxo =
     checkCoverage $
-    cover 50  (    haveResult)        "have result" $
+    cover 50 haveResult "have result" $
     cover 0.1 (not haveResult) "do not have result" $
-    report
-        (utxo)
-        "utxo" $
-    report
-        (utxoFromTx tx)
-        "utxoFromTx tx" $
-    report
-        (haveResult)
-        "haveResult" $
-    report
-        (shouldHaveResult)
-        "shouldHaveResult" $
+    report utxo "utxo" $
+    report (utxoFromTx tx) "utxoFromTx tx" $
+    report haveResult "haveResult" $
+    report shouldHaveResult "shouldHaveResult" $
     case maybeResult of
         Nothing ->
-            verify
-                (not shouldHaveResult)
-                "not shouldHaveResult" $
+            verify (not shouldHaveResult) "not shouldHaveResult" $
             property True
         Just utxo' ->
-            cover 10 (utxo /= utxo')
-                "utxo /= utxo'" $
-            verify
-                (shouldHaveResult)
-                "shouldHaveResult" $
+            cover 10 (utxo /= utxo') "utxo /= utxo'" $
+            verify shouldHaveResult "shouldHaveResult" $
             property True
   where
     haveResult :: Bool