Merge #1806

1806: Remove duplicate transaction check from tryAddTxs in mempool r=intricate a=intricate

Closes #1801 

Co-authored-by: Luke Nadur <[email protected]>

Author: iohk-bors[bot]

cabal.project

@@ -207,29 +207,29 @@ source-repository-package
 source-repository-package
   type: git
   location: https://github.com/input-output-hk/cardano-ledger
-  tag: 1b0d9ff9c7e201f0877ccec2a992c58e2b19c5ec
-  --sha256: 05vlmhxqsjn2y8ff864mm0jjbkl9pmb5qscbhwk6nrngqvxg00ys
+  tag: 90b14c056059d0082cb2641f9c77cb1b097be329
+  --sha256: 029lqlf33a3vvy933fil8qglv3hjaprankllr80h0acspqjzahk0
   subdir: cardano-ledger
 
 source-repository-package
   type: git
   location: https://github.com/input-output-hk/cardano-ledger
-  tag: 1b0d9ff9c7e201f0877ccec2a992c58e2b19c5ec
-  --sha256: 05vlmhxqsjn2y8ff864mm0jjbkl9pmb5qscbhwk6nrngqvxg00ys
+  tag: 90b14c056059d0082cb2641f9c77cb1b097be329
+  --sha256: 029lqlf33a3vvy933fil8qglv3hjaprankllr80h0acspqjzahk0
   subdir: crypto
 
 source-repository-package
   type: git
   location: https://github.com/input-output-hk/cardano-ledger
-  tag: 1b0d9ff9c7e201f0877ccec2a992c58e2b19c5ec
-  --sha256: 05vlmhxqsjn2y8ff864mm0jjbkl9pmb5qscbhwk6nrngqvxg00ys
+  tag: 90b14c056059d0082cb2641f9c77cb1b097be329
+  --sha256: 029lqlf33a3vvy933fil8qglv3hjaprankllr80h0acspqjzahk0
   subdir: cardano-ledger/test
 
 source-repository-package
   type: git
   location: https://github.com/input-output-hk/cardano-ledger
-  tag: 1b0d9ff9c7e201f0877ccec2a992c58e2b19c5ec
-  --sha256: 05vlmhxqsjn2y8ff864mm0jjbkl9pmb5qscbhwk6nrngqvxg00ys
+  tag: 90b14c056059d0082cb2641f9c77cb1b097be329
+  --sha256: 029lqlf33a3vvy933fil8qglv3hjaprankllr80h0acspqjzahk0
   subdir: crypto/test
 
 source-repository-package

ouroboros-consensus/src/Ouroboros/Consensus/Mempool/API.hs

@@ -17,8 +17,8 @@ module Ouroboros.Consensus.Mempool.API (
   , HasTxId(..)
   , GenTxId
   , MempoolAddTxResult (..)
-  , isTxAddedOrAlreadyInMempool
-  , isTxRejected
+  , isMempoolTxAdded
+  , isMempoolTxRejected
   , MempoolSize (..)
   , TraceEventMempool(..)
   , HasTxs(..)
@@ -235,26 +235,20 @@ data Mempool m blk idx = Mempool {
 data MempoolAddTxResult blk
   = MempoolTxAdded
     -- ^ The transaction was added to the mempool.
-  | MempoolTxAlreadyInMempool
-    -- ^ The transaction already exists within the mempool and therefore could
-    -- not be added.
-    --
-    -- Note that we don't treat this like an error/rejection case.
   | MempoolTxRejected !(ApplyTxErr blk)
     -- ^ The transaction was rejected and could not be added to the mempool
     -- for the specified reason.
 
 deriving instance Eq (ApplyTxErr blk) => Eq (MempoolAddTxResult blk)
 deriving instance Show (ApplyTxErr blk) => Show (MempoolAddTxResult blk)
 
-isTxAddedOrAlreadyInMempool :: MempoolAddTxResult blk -> Bool
-isTxAddedOrAlreadyInMempool MempoolTxAdded            = True
-isTxAddedOrAlreadyInMempool MempoolTxAlreadyInMempool = True
-isTxAddedOrAlreadyInMempool _                         = False
+isMempoolTxAdded :: MempoolAddTxResult blk -> Bool
+isMempoolTxAdded MempoolTxAdded = True
+isMempoolTxAdded _              = False
 
-isTxRejected :: MempoolAddTxResult blk -> Bool
-isTxRejected (MempoolTxRejected _) = True
-isTxRejected _                     = False
+isMempoolTxRejected :: MempoolAddTxResult blk -> Bool
+isMempoolTxRejected (MempoolTxRejected _) = True
+isMempoolTxRejected _                     = False
 
 -- | Wrapper around 'implTryAddTxs' that blocks until all transaction have
 -- either been added to the Mempool or rejected.

ouroboros-consensus/src/Ouroboros/Consensus/Mempool/Impl.hs

@@ -273,13 +273,6 @@ implTryAddTxs mpEnv = go []
         join $ atomically $ readTVar mpEnvStateVar >>= tryAdd
       where
         tryAdd is
-          -- This transaction already exists within the mempool.
-          -- Note that we don't treat this as an error/rejection case.
-          | implSnapshotHasTx is (txId firstTx)
-          = return $ go
-              ((firstTx, MempoolTxAdded):acc)
-              toAdd'
-
           -- No space in the Mempool.
           | let firstTxSize = txSize firstTx
                 curSize = msNumBytes $ isMempoolSize is

ouroboros-consensus/src/Ouroboros/Consensus/MiniProtocol/LocalTxSubmission/Server.hs

@@ -34,7 +34,6 @@ localTxSubmissionServer tracer mempool =
         case res of
           [(_tx, addTxRes)] -> case addTxRes of
             MempoolTxAdded             -> return (Nothing, server)
-            MempoolTxAlreadyInMempool  -> return (Nothing, server)
             MempoolTxRejected addTxErr -> return (Just addTxErr, server)
           -- The output list of addTxs has the same length as the input list.
           _                 -> error "addTxs: unexpected result"

ouroboros-consensus/src/Ouroboros/Consensus/NodeKernel.hs

@@ -630,6 +630,6 @@ getMempoolWriter
 getMempoolWriter mempool = Inbound.TxSubmissionMempoolWriter
     { Inbound.txId          = txId
     , mempoolAddTxs = \txs ->
-        map (txId . fst) . filter (isTxAddedOrAlreadyInMempool . snd) <$>
+        map (txId . fst) . filter (isMempoolTxAdded . snd) <$>
         addTxs mempool txs
     }

ouroboros-consensus/test-consensus/Test/Consensus/Mempool.hs

@@ -114,8 +114,8 @@ prop_Mempool_addTxs_result setup =
     withTestMempool (testSetup setup) $ \TestMempool { mempool } -> do
       result <- addTxs mempool (allTxs setup)
       return $ counterexample (ppTxs (txs setup)) $
-        [(tx, isTxAddedOrAlreadyInMempool res) | (tx, res) <- result] ===
-        [(testTx, valid)                       | (testTx, valid) <- txs setup]
+        [(tx, isMempoolTxAdded res) | (tx, res) <- result] ===
+        [(testTx, valid)            | (testTx, valid) <- txs setup]
 
 -- | Test that invalid transactions are never added to the 'Mempool'.
 prop_Mempool_InvalidTxsNeverAdded :: TestSetupWithTxs -> Property
@@ -186,13 +186,13 @@ prop_Mempool_Capacity (MempoolCapTestSetup testSetupWithTxs) =
     MempoolCapacityBytes capacity = testMempoolCap testSetup
 
     -- | Convert 'MempoolAddTxResult' into a 'Bool':
-    -- isTxAddedOrAlreadyInMempool -> True, isTxRejected -> False.
+    -- isMempoolTxAdded -> True, isMempoolTxRejected -> False.
     blindErrors
       :: ([(GenTx TestBlock, MempoolAddTxResult blk)], [GenTx TestBlock])
       -> ([(GenTx TestBlock, Bool)], [GenTx TestBlock])
     blindErrors (processed, toAdd) = (processed', toAdd)
       where
-        processed' = [ (tx, isTxAddedOrAlreadyInMempool txAddRes)
+        processed' = [ (tx, isMempoolTxAdded txAddRes)
                      | (tx, txAddRes) <- processed ]
 
     expectedResult
@@ -670,8 +670,8 @@ withTestMempool setup@TestSetup { testLedgerState, testInitialTxs, testMempoolCa
       result  <- addTxs mempool testInitialTxs
       -- the invalid transactions are reported in the same order they were
       -- added, so the first error is not the result of a cascade
-      whenJust (find (isTxRejected . snd) result) $ \(invalidTx, _) -> error $
-        "Invalid initial transaction: " <> condense invalidTx
+      whenJust (find (isMempoolTxRejected . snd) result) $ \(invalidTx, _) ->
+        error $ "Invalid initial transaction: " <> condense invalidTx
 
       -- Clear the trace
       atomically $ writeTVar varEvents []

stack.yaml

@@ -65,7 +65,7 @@ extra-deps:
   - gray-code-0.3.1
 
   - git: https://github.com/input-output-hk/cardano-ledger
-    commit: 1b0d9ff9c7e201f0877ccec2a992c58e2b19c5ec
+    commit: 90b14c056059d0082cb2641f9c77cb1b097be329
     subdirs:
       - cardano-ledger
       - cardano-ledger/test