Support Range and SortOrder in DB.readTxHistory

I made DB.readTxHistory take and apply sortOrder and range filtering with implementations in Sqlite, MVar and Sqlite's StateMachine tests.

Squashed from:

Ensure readTxHistory tests pass with `Descending noFilter`
[WIP] Make tests concider sorting and filtering
Do actual Sqlite sorting and filtering
Apply suggestions from code review
Introduce defaultTxSortDirection
Temporary overflow fix to make StateMachine tests pass
Rename Filter to Range`
Fixup: more polish

lib/core/src/Cardano/Wallet.hs

@@ -133,10 +133,12 @@ import Cardano.Wallet.Primitive.Types
     , WalletPassphraseInfo (..)
     , WalletState (..)
     , computeUtxoStatistics
+    , defaultTxSortOrder
     , log10
     , slotDifference
     , slotRatio
     , slotStartTime
+    , wholeRange
     )
 import Cardano.Wallet.Transaction
     ( ErrMkStdTx (..), ErrValidateSelection, TransactionLayer (..) )
@@ -685,7 +687,8 @@ newWalletLayer tracer bp db nw tl = do
     _listAddresses wid = do
         (s, txs) <- DB.withLock db $ (,)
             <$> (getState <$> _readWalletCheckpoint wid)
-            <*> liftIO (DB.readTxHistory db (PrimaryKey wid))
+            <*> liftIO (DB.readTxHistory db
+                (PrimaryKey wid) defaultTxSortOrder wholeRange)
         let maybeIsOurs (TxOut a _) = if fst (isOurs a s)
                 then Just a
                 else Nothing
@@ -758,7 +761,9 @@ newWalletLayer tracer bp db nw tl = do
                     ErrStartTimeLaterThanEndTime start end
             _ -> pure ()
         let tip = currentTip w ^. #slotId
-        liftIO $ assemble tip <$> DB.readTxHistory db (PrimaryKey wid)
+        liftIO $ assemble tip
+            <$> DB.readTxHistory db (PrimaryKey wid)
+                defaultTxSortOrder wholeRange
       where
         -- This relies on DB.readTxHistory returning all necessary transactions
         -- to assemble coin selection information for outgoing payments.

lib/core/src/Cardano/Wallet/DB.hs

@@ -26,7 +26,15 @@ import Cardano.Wallet.Primitive.AddressDerivation
 import Cardano.Wallet.Primitive.Model
     ( Wallet )
 import Cardano.Wallet.Primitive.Types
-    ( DefineTx (..), Hash, TxMeta, WalletId, WalletMetadata )
+    ( DefineTx (..)
+    , Hash
+    , Range (..)
+    , SlotId (..)
+    , SortOrder (..)
+    , TxMeta
+    , WalletId
+    , WalletMetadata
+    )
 import Control.Monad.Trans.Except
     ( ExceptT, runExceptT )
 import Data.Map.Strict
@@ -102,6 +110,8 @@ data DBLayer m s t = DBLayer
 
     , readTxHistory
         :: PrimaryKey WalletId
+        -> SortOrder
+        -> Range SlotId
         -> m [(Hash "Tx", (Tx t, TxMeta))]
         -- ^ Fetch the current transaction history of a known wallet, ordered by
         -- descending slot number.

lib/core/src/Cardano/Wallet/DB/MVar.hs

@@ -27,7 +27,14 @@ import Cardano.Wallet.Primitive.AddressDerivation
 import Cardano.Wallet.Primitive.Model
     ( Wallet )
 import Cardano.Wallet.Primitive.Types
-    ( Hash, Tx, TxMeta (slotId), WalletId, WalletMetadata )
+    ( Hash
+    , SortOrder (..)
+    , Tx
+    , TxMeta (slotId)
+    , WalletId
+    , WalletMetadata
+    , isWithinRange
+    )
 import Control.Concurrent.MVar
     ( MVar, modifyMVar, newMVar, readMVar, withMVar )
 import Control.DeepSeq
@@ -37,11 +44,11 @@ import Control.Monad
 import Control.Monad.Trans.Except
     ( ExceptT (..), runExceptT )
 import Data.List
-    ( sortOn )
+    ( sortBy )
 import Data.Map.Strict
     ( Map )
 import Data.Ord
-    ( Down (..) )
+    ( Down (..), comparing )
 
 import qualified Data.Map.Strict as Map
 
@@ -125,10 +132,15 @@ newDBLayer = do
                         Right $ Just $ Database cp meta (txs' <> txs) k
             txs' `deepseq` alterMVar db alter key
 
-        , readTxHistory = \key -> let
-                sortedTxHistory = sortOn slot . Map.toList . txHistory
-                slot = Down . slotId . snd . snd
-            in maybe mempty sortedTxHistory . Map.lookup key <$> readMVar db
+        , readTxHistory = \key order range -> let
+                order' = case order of
+                    Ascending -> comparing slot
+                    Descending -> comparing $ Down . slot
+                result =
+                    filter (isWithinRange range . slot)
+                    . sortBy order' . Map.toList . txHistory
+                slot = slotId . snd . snd
+            in maybe mempty result . Map.lookup key <$> readMVar db
 
         {-----------------------------------------------------------------------
                                        Keystore

lib/core/src/Cardano/Wallet/DB/Sqlite.hs

@@ -109,7 +109,7 @@ import Data.List.Split
 import Data.Map.Strict
     ( Map )
 import Data.Maybe
-    ( fromMaybe )
+    ( catMaybes, fromMaybe )
 import Data.Quantity
     ( Quantity (..) )
 import Data.Text
@@ -137,8 +137,10 @@ import Database.Persist.Sql
     , selectList
     , updateWhere
     , (<-.)
+    , (<=.)
     , (=.)
     , (==.)
+    , (>=.)
     )
 import Database.Persist.Sqlite
     ( SqlBackend, SqlPersistT, mkSqliteConnectionInfo, wrapConnectionInfo )
@@ -306,7 +308,8 @@ newDBLayer logConfig trace fp = do
               selectLatestCheckpoint wid >>= \case
                   Just cp -> do
                       utxo <- selectUTxO cp
-                      txs <- selectTxHistory @t wid [TxMetaStatus ==. W.Pending]
+                      txs <- selectTxHistory @t wid
+                          W.defaultTxSortOrder [TxMetaStatus ==. W.Pending]
                       s <- selectState (checkpointId cp)
                       pure (checkpointFromEntity @s @t cp utxo txs <$> s)
                   Nothing -> pure Nothing
@@ -347,9 +350,12 @@ newDBLayer logConfig trace fp = do
                       pure $ Right ()
                   Nothing -> pure $ Left $ ErrNoSuchWallet wid
 
-        , readTxHistory = \(PrimaryKey wid) ->
+        , readTxHistory = \(PrimaryKey wid) order range ->
               runQuery $
-              selectTxHistory @t wid []
+              selectTxHistory @t wid order $ catMaybes
+                [ (TxMetaSlotId >=.) <$> W.rStart range
+                , (TxMetaSlotId <=.) <$> W.rEnd range
+                ]
 
         {-----------------------------------------------------------------------
                                        Keystore
@@ -739,18 +745,22 @@ selectTxs txids = do
 selectTxHistory
     :: forall t. PersistTx t
     => W.WalletId
+    -> W.SortOrder
     -> [Filter TxMeta]
     -> SqlPersistT IO [(W.Hash "Tx", (W.Tx t, W.TxMeta))]
-selectTxHistory wid conditions = do
-    -- Note: there are sorted indices on these columns.
-    -- The secondary sort by TxId is to make the ordering stable
-    -- so that testing with random data always works.
-    let sortOpt = [ Desc TxMetaSlotId, Asc TxMetaTxId ]
+selectTxHistory wid order conditions = do
     metas <- fmap entityVal <$> selectList
         ((TxMetaWalletId ==. wid) : conditions) sortOpt
     let txids = map txMetaTxId metas
     (ins, outs) <- selectTxs txids
     pure $ txHistoryFromEntity @t metas ins outs
+  where
+    -- Note: there are sorted indices on these columns.
+    -- The secondary sort by TxId is to make the ordering stable
+    -- so that testing with random data always works.
+    sortOpt = case order of
+        W.Ascending -> [Asc TxMetaSlotId, Desc TxMetaTxId]
+        W.Descending -> [Desc TxMetaSlotId, Asc TxMetaTxId]
 
 ---------------------------------------------------------------------------
 -- DB queries for address discovery state

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

@@ -97,6 +97,10 @@ module Cardano.Wallet.Primitive.Types
 
     -- * Querying
     , SortOrder (..)
+    , Range (..)
+    , wholeRange
+    , isWithinRange
+    , defaultTxSortOrder
 
     -- * Polymorphic
     , Hash (..)
@@ -326,6 +330,39 @@ instance ToText SortOrder where
 instance FromText SortOrder where
     fromText = fromTextToBoundedEnum SnakeLowerCase
 
+defaultTxSortOrder :: SortOrder
+defaultTxSortOrder =  Descending
+
+-- |'Range a' is used to filter data with optional `>=` and `<=` bounds.
+--
+-- When both bounds are 'Just', it functions like the closed range
+-- '[start, end]'. When both bounds are 'Nothing' it functions like
+-- '(-∞,∞)', including everything (see 'wholeRange'.)
+--
+-- The full four interesting cases with the corresponding predicate in the third
+-- column:
+--
+-- - [start, end]  Range (Just start) (Just end)   \x -> x >= start && x <= end
+-- - [start,∞)     Range (Just start) Nothing      \x -> x >= start
+-- - (-∞,end)      Range Nothing (Just end)        \x -> x <= end
+-- - (-∞,∞)        Range Nothing Nothing           \_x -> True
+data Range a = Range
+    { rStart :: Maybe a
+    , rEnd :: Maybe a
+    } deriving (Eq, Show)
+
+-- | The range that includes everything.
+wholeRange :: Range a
+wholeRange = Range Nothing Nothing
+
+isWithinRange :: Ord a => Range a -> a -> Bool
+isWithinRange (Range low high) x =
+    (maybe True (x >=) low) &&
+    (maybe True (x <=) high)
+
+-- NOTE: We could imagine replacing 'Range (Just 3) Nothing' with something
+-- more magic like: `(Including 3) ... NoBound`
+
 {-------------------------------------------------------------------------------
                                   Stake Pools
 -------------------------------------------------------------------------------}

lib/core/test/unit/Cardano/Wallet/DB/SqliteFileModeSpec.hs

@@ -49,6 +49,8 @@ import Cardano.Wallet.Primitive.Types
     , WalletName (..)
     , WalletPassphraseInfo (..)
     , WalletState (..)
+    , defaultTxSortOrder
+    , wholeRange
     )
 import Cardano.Wallet.Unsafe
     ( unsafeRunExceptT )
@@ -131,7 +133,9 @@ spec =  do
             unsafeRunExceptT $ createWallet db testWid testCp testMetadata
             unsafeRunExceptT $ putTxHistory db testWid (Map.fromList testTxs)
             destroyDBLayer ctx
-            testOpeningCleaning f (`readTxHistory` testWid) testTxs mempty
+            testOpeningCleaning f
+                (\db' -> readTxHistory db' testWid defaultTxSortOrder wholeRange)
+                testTxs mempty
 
         it "put and read checkpoint" $ \f -> do
             (ctx, db) <- newDBLayer' (Just f)

lib/core/test/unit/Cardano/Wallet/DB/SqliteSpec.hs

@@ -63,6 +63,8 @@ import Cardano.Wallet.Primitive.Types
     , WalletName (..)
     , WalletPassphraseInfo (..)
     , WalletState (..)
+    , defaultTxSortOrder
+    , wholeRange
     )
 import Cardano.Wallet.Unsafe
     ( unsafeRunExceptT )
@@ -169,15 +171,17 @@ simpleSpec = do
             unsafeRunExceptT $ createWallet db testPk testCp testMetadata
             runExceptT (putTxHistory db testPk (Map.fromList testTxs))
                 `shouldReturn` Right ()
-            readTxHistory db testPk `shouldReturn` testTxs
+            readTxHistory db testPk
+                defaultTxSortOrder wholeRange `shouldReturn` testTxs
 
         it "put and read tx history - regression case" $ \db -> do
             unsafeRunExceptT $ createWallet db testPk testCp testMetadata
             unsafeRunExceptT $ createWallet db testPk1 testCp testMetadata
             runExceptT (putTxHistory db testPk1 (Map.fromList testTxs))
                 `shouldReturn` Right ()
             runExceptT (removeWallet db testPk) `shouldReturn` Right ()
-            readTxHistory db testPk1 `shouldReturn` testTxs
+            readTxHistory db testPk1
+                defaultTxSortOrder wholeRange `shouldReturn` testTxs
 
         it "put and read checkpoint" $ \db -> do
             unsafeRunExceptT $ createWallet db testPk testCp testMetadata