From cd5ccc3b4a609733370c2adbd6b2a3946956b03d Mon Sep 17 00:00:00 2001 From: Johannes Lund Date: Thu, 12 Nov 2020 14:37:43 +0100 Subject: [PATCH] Revert "Merge #2262" This reverts commit 970bebbf10a0a984477e14f48033757f6726ce5b, reversing changes made to eff3c57abc15bc76e3fee060393532f16310530c. --- .../Test/Integration/Framework/TestData.hs | 8 +-- .../Scenario/API/Shelley/Transactions.hs | 44 +----------- .../Scenario/CLI/Shelley/Transactions.hs | 4 +- lib/core/src/Cardano/Wallet.hs | 22 +++--- lib/core/src/Cardano/Wallet/Api/Server.hs | 16 ++--- lib/core/src/Cardano/Wallet/Api/Types.hs | 2 +- lib/core/src/Cardano/Wallet/DB.hs | 16 ++--- lib/core/src/Cardano/Wallet/DB/MVar.hs | 23 ++++--- lib/core/src/Cardano/Wallet/DB/Model.hs | 54 +++++++-------- lib/core/src/Cardano/Wallet/DB/Sqlite.hs | 68 +++++++++---------- .../src/Cardano/Wallet/Primitive/Types.hs | 6 -- .../unit/Cardano/Wallet/DB/StateMachine.hs | 29 ++++---- .../Jormungandr/Scenario/CLI/Transactions.hs | 4 +- specifications/api/swagger.yaml | 10 +-- 14 files changed, 125 insertions(+), 181 deletions(-) diff --git a/lib/core-integration/src/Test/Integration/Framework/TestData.hs b/lib/core-integration/src/Test/Integration/Framework/TestData.hs index f766e98748d..6fcbc5c4ebc 100644 --- a/lib/core-integration/src/Test/Integration/Framework/TestData.hs +++ b/lib/core-integration/src/Test/Integration/Framework/TestData.hs @@ -44,7 +44,7 @@ module Test.Integration.Framework.TestData , errMsg403NotEnoughMoney , errMsg403NotEnoughMoney_ , errMsg403WrongPass - , errMsg403AlreadyInLedger + , errMsg403NoPendingAnymore , errMsg404NoSuchPool , errMsg403PoolAlreadyJoined , errMsg403NotDelegating @@ -351,9 +351,9 @@ errMsg404NoEndpoint = "I couldn't find the requested endpoint. If the endpoint\ \ contains path parameters, please ensure they are well-formed, otherwise I\ \ won't be able to route them correctly." -errMsg403AlreadyInLedger :: Text -> String -errMsg403AlreadyInLedger tid = "The transaction with id: " ++ unpack tid ++ - " cannot be forgotten as it is already in the ledger." +errMsg403NoPendingAnymore :: Text -> String +errMsg403NoPendingAnymore tid = "The transaction with id: " ++ unpack tid ++ + " cannot be forgotten as it is not pending anymore." errMsg404NoSuchPool :: Text -> String errMsg404NoSuchPool pid = "I couldn't find any stake pool with the given id: " diff --git a/lib/core-integration/src/Test/Integration/Scenario/API/Shelley/Transactions.hs b/lib/core-integration/src/Test/Integration/Scenario/API/Shelley/Transactions.hs index 9189becb63e..c24464d6c00 100644 --- a/lib/core-integration/src/Test/Integration/Scenario/API/Shelley/Transactions.hs +++ b/lib/core-integration/src/Test/Integration/Scenario/API/Shelley/Transactions.hs @@ -149,9 +149,9 @@ import Test.Integration.Framework.TestData ( errMsg400MinWithdrawalWrong , errMsg400StartTimeLaterThanEndTime , errMsg400TxMetadataStringTooLong - , errMsg403AlreadyInLedger , errMsg403Fee , errMsg403InputsDepleted + , errMsg403NoPendingAnymore , errMsg403NotAShelleyWallet , errMsg403NotEnoughMoney , errMsg403NotEnoughMoney_ @@ -2210,7 +2210,7 @@ spec = describe "SHELLEY_TRANSACTIONS" $ do let ep = Link.deleteTransaction @'Shelley wSrc (ApiTxId txid) rDel <- request @ApiTxId ctx ep Default Empty expectResponseCode HTTP.status403 rDel - let err = errMsg403AlreadyInLedger (toUrlPiece (ApiTxId txid)) + let err = errMsg403NoPendingAnymore (toUrlPiece (ApiTxId txid)) expectErrorMessage err rDel describe "TRANS_DELETE_03 - checking no transaction id error for " $ do @@ -2222,46 +2222,6 @@ spec = describe "SHELLEY_TRANSACTIONS" $ do txDeleteFromDifferentWalletTest emptyWallet "wallets" txDeleteFromDifferentWalletTest emptyRandomWallet "byron-wallets" - it "TRANS_TTL_DELETE_01 - Shelley: can remove expired tx" $ \ctx -> runResourceT $ do - liftIO $ pendingWith "#1840 this is flaky -- need a better approach" - (wa, wb) <- (,) <$> fixtureWallet ctx <*> emptyWallet ctx - let amt = minUTxOValue :: Natural - - -- this transaction is going to expire really soon. - basePayload <- mkTxPayload ctx wb amt fixturePassphrase - let payload = addTxTTL 0.1 basePayload - - ra <- request @(ApiTransaction n) ctx - (Link.createTransaction @'Shelley wa) Default payload - - expectSuccess ra - - let txid = ApiTxId (getFromResponse #id ra) - let linkSrc = Link.getTransaction @'Shelley wa txid - - rb <- eventually "transaction is no longer pending" $ do - rr <- request @(ApiTransaction n) ctx linkSrc Default Empty - verify rr - [ expectSuccess - , expectField (#status . #getApiT) (`shouldNotBe` Pending) - ] - pure rr - - -- it should be expired - expectField (#status . #getApiT) (`shouldBe` Expired) rb - - -- remove it - let linkDel = Link.deleteTransaction @'Shelley wa txid - request @(ApiTransaction n) ctx linkDel Default Empty - >>= expectResponseCode HTTP.status204 - - -- it should be gone - request @(ApiTransaction n) ctx linkSrc Default Empty - >>= expectResponseCode HTTP.status404 - -- yes, gone - request @(ApiTransaction n) ctx linkDel Default Empty - >>= expectResponseCode HTTP.status404 - it "BYRON_TRANS_DELETE -\ \ Cannot delete tx on Byron wallet using shelley ep" $ \ctx -> runResourceT $ do w <- emptyRandomWallet ctx diff --git a/lib/core-integration/src/Test/Integration/Scenario/CLI/Shelley/Transactions.hs b/lib/core-integration/src/Test/Integration/Scenario/CLI/Shelley/Transactions.hs index f014bf30a88..c5feb22cc4f 100644 --- a/lib/core-integration/src/Test/Integration/Scenario/CLI/Shelley/Transactions.hs +++ b/lib/core-integration/src/Test/Integration/Scenario/CLI/Shelley/Transactions.hs @@ -103,7 +103,7 @@ import Test.Integration.Framework.DSL ) import Test.Integration.Framework.TestData ( arabicWalletName - , errMsg403AlreadyInLedger + , errMsg403NoPendingAnymore , errMsg403WrongPass , errMsg404CannotFindTx , errMsg404NoWallet @@ -749,7 +749,7 @@ spec = describe "SHELLEY_CLI_TRANSACTIONS" $ do -- Try Forget transaction once it's no longer pending (Exit c2, Stdout out2, Stderr err2) <- deleteTransactionViaCLI @t ctx wSrcId txId - err2 `shouldContain` errMsg403AlreadyInLedger (T.pack txId) + err2 `shouldContain` errMsg403NoPendingAnymore (T.pack txId) out2 `shouldBe` "" c2 `shouldBe` ExitFailure 1 diff --git a/lib/core/src/Cardano/Wallet.hs b/lib/core/src/Cardano/Wallet.hs index 7ee0e115aff..888c6291895 100644 --- a/lib/core/src/Cardano/Wallet.hs +++ b/lib/core/src/Cardano/Wallet.hs @@ -146,7 +146,7 @@ module Cardano.Wallet , handleCannotCover -- ** Transaction - , forgetTx + , forgetPendingTx , listTransactions , getTransaction , submitExternalTx @@ -155,7 +155,7 @@ module Cardano.Wallet , ErrMkTx (..) , ErrSubmitTx (..) , ErrSubmitExternalTx (..) - , ErrRemoveTx (..) + , ErrRemovePendingTx (..) , ErrPostTx (..) , ErrDecodeSignedTx (..) , ErrListTransactions (..) @@ -198,7 +198,7 @@ import Cardano.Slotting.Slot import Cardano.Wallet.DB ( DBLayer (..) , ErrNoSuchWallet (..) - , ErrRemoveTx (..) + , ErrRemovePendingTx (..) , ErrWalletAlreadyExists (..) , PrimaryKey (..) , SparseCheckpointsConfig (..) @@ -1946,22 +1946,18 @@ submitExternalTx ctx bytes = do nw = ctx ^. networkLayer @t tl = ctx ^. transactionLayer @t @k --- | Remove a pending or expired transaction from the transaction history. This --- happens at the request of the user. If the transaction is already on chain, --- or is missing from the transaction history, an error will be returned. --- --- If a 'Pending' transaction is removed, but later appears in a block, it will --- be added back to the transaction history. -forgetTx +-- | Forget pending transaction. This happens at the request of the user and +-- will remove the transaction from the history. +forgetPendingTx :: forall ctx s k. ( HasDBLayer s k ctx ) => ctx -> WalletId -> Hash "Tx" - -> ExceptT ErrRemoveTx IO () -forgetTx ctx wid tid = db & \DBLayer{..} -> do - mapExceptT atomically $ removePendingOrExpiredTx (PrimaryKey wid) tid + -> ExceptT ErrRemovePendingTx IO () +forgetPendingTx ctx wid tid = db & \DBLayer{..} -> do + mapExceptT atomically $ removePendingTx (PrimaryKey wid) tid where db = ctx ^. dbLayer @s @k diff --git a/lib/core/src/Cardano/Wallet/Api/Server.hs b/lib/core/src/Cardano/Wallet/Api/Server.hs index fee9e79348d..59009f0162b 100644 --- a/lib/core/src/Cardano/Wallet/Api/Server.hs +++ b/lib/core/src/Cardano/Wallet/Api/Server.hs @@ -125,7 +125,7 @@ import Cardano.Wallet , ErrPostTx (..) , ErrQuitStakePool (..) , ErrReadChimericAccount (..) - , ErrRemoveTx (..) + , ErrRemovePendingTx (..) , ErrSelectCoinsExternal (..) , ErrSelectForDelegation (..) , ErrSelectForMigration (..) @@ -1406,7 +1406,7 @@ deleteTransaction -> Handler NoContent deleteTransaction ctx (ApiT wid) (ApiTxId (ApiT (tid))) = do withWorkerCtx ctx wid liftE liftE $ \wrk -> liftHandler $ - W.forgetTx wrk wid tid + W.forgetPendingTx wrk wid tid return NoContent listTransactions @@ -2519,18 +2519,18 @@ instance LiftHandler ErrSubmitExternalTx where , errReasonPhrase = errReasonPhrase err400 } -instance LiftHandler ErrRemoveTx where +instance LiftHandler ErrRemovePendingTx where handler = \case - ErrRemoveTxNoSuchWallet wid -> handler wid - ErrRemoveTxNoSuchTransaction tid -> + ErrRemovePendingTxNoSuchWallet wid -> handler wid + ErrRemovePendingTxNoSuchTransaction tid -> apiError err404 NoSuchTransaction $ mconcat [ "I couldn't find a transaction with the given id: " , toText tid ] - ErrRemoveTxAlreadyInLedger tid -> - apiError err403 TransactionAlreadyInLedger $ mconcat + ErrRemovePendingTxTransactionNoMorePending tid -> + apiError err403 TransactionNotPending $ mconcat [ "The transaction with id: ", toText tid, - " cannot be forgotten as it is already in the ledger." + " cannot be forgotten as it is not pending anymore." ] instance LiftHandler ErrPostTx where diff --git a/lib/core/src/Cardano/Wallet/Api/Types.hs b/lib/core/src/Cardano/Wallet/Api/Types.hs index 9bc8e961292..33da68cadf9 100644 --- a/lib/core/src/Cardano/Wallet/Api/Types.hs +++ b/lib/core/src/Cardano/Wallet/Api/Types.hs @@ -883,7 +883,7 @@ newtype ApiVerificationKey = ApiVerificationKey data ApiErrorCode = NoSuchWallet | NoSuchTransaction - | TransactionAlreadyInLedger + | TransactionNotPending | WalletAlreadyExists | NoRootKey | WrongEncryptionPassphrase diff --git a/lib/core/src/Cardano/Wallet/DB.hs b/lib/core/src/Cardano/Wallet/DB.hs index d3616a04094..c99a5ef563a 100644 --- a/lib/core/src/Cardano/Wallet/DB.hs +++ b/lib/core/src/Cardano/Wallet/DB.hs @@ -27,7 +27,7 @@ module Cardano.Wallet.DB , gapSize -- * Errors - , ErrRemoveTx (..) + , ErrRemovePendingTx (..) , ErrNoSuchWallet(..) , ErrWalletAlreadyExists(..) ) where @@ -253,10 +253,10 @@ data DBLayer m s k = forall stm. (MonadIO stm, MonadFail stm) => DBLayer -- ^ Removes any expired transactions from the pending set and marks -- their status as expired. - , removePendingOrExpiredTx + , removePendingTx :: PrimaryKey WalletId -> Hash "Tx" - -> ExceptT ErrRemoveTx stm () + -> ExceptT ErrRemovePendingTx stm () -- ^ Manually remove a pending transaction. , putPrivateKey @@ -311,11 +311,11 @@ newtype ErrNoSuchWallet = ErrNoSuchWallet WalletId -- Wallet is gone or doesn't exist yet deriving (Eq, Show) --- | Can't remove pending or expired transaction. -data ErrRemoveTx - = ErrRemoveTxNoSuchWallet ErrNoSuchWallet - | ErrRemoveTxNoSuchTransaction (Hash "Tx") - | ErrRemoveTxAlreadyInLedger (Hash "Tx") +-- | Can't perform removing pending transaction +data ErrRemovePendingTx + = ErrRemovePendingTxNoSuchWallet ErrNoSuchWallet + | ErrRemovePendingTxNoSuchTransaction (Hash "Tx") + | ErrRemovePendingTxTransactionNoMorePending (Hash "Tx") deriving (Eq, Show) -- | Can't perform given operation because there's no transaction diff --git a/lib/core/src/Cardano/Wallet/DB/MVar.hs b/lib/core/src/Cardano/Wallet/DB/MVar.hs index 9afab0382c2..d90c60b6ec3 100644 --- a/lib/core/src/Cardano/Wallet/DB/MVar.hs +++ b/lib/core/src/Cardano/Wallet/DB/MVar.hs @@ -23,13 +23,14 @@ import Cardano.Address.Derivation import Cardano.Wallet.DB ( DBLayer (..) , ErrNoSuchWallet (..) - , ErrRemoveTx (..) + , ErrRemovePendingTx (..) , ErrWalletAlreadyExists (..) , PrimaryKey (..) ) import Cardano.Wallet.DB.Model ( Database , Err (..) + , ErrErasePendingTx (..) , ModelOp , emptyDatabase , mCheckWallet @@ -50,7 +51,7 @@ import Cardano.Wallet.DB.Model , mReadProtocolParameters , mReadTxHistory , mReadWalletMeta - , mRemovePendingOrExpiredTx + , mRemovePendingTx , mRemoveWallet , mRollbackTo , mUpdatePendingTxForExpiry @@ -179,8 +180,8 @@ newDBLayer timeInterpreter = do , updatePendingTxForExpiry = \pk tip -> ExceptT $ do alterDB errNoSuchWallet db (mUpdatePendingTxForExpiry pk tip) - , removePendingOrExpiredTx = \pk tid -> ExceptT $ do - alterDB errCannotRemovePendingTx db (mRemovePendingOrExpiredTx pk tid) + , removePendingTx = \pk tid -> ExceptT $ do + alterDB errCannotRemovePendingTx db (mRemovePendingTx pk tid) {----------------------------------------------------------------------- Protocol Parameters @@ -239,13 +240,13 @@ errNoSuchWallet :: Err (PrimaryKey WalletId) -> Maybe ErrNoSuchWallet errNoSuchWallet (NoSuchWallet (PrimaryKey wid)) = Just (ErrNoSuchWallet wid) errNoSuchWallet _ = Nothing -errCannotRemovePendingTx :: Err (PrimaryKey WalletId) -> Maybe ErrRemoveTx -errCannotRemovePendingTx (NoSuchWallet (PrimaryKey wid)) = - Just (ErrRemoveTxNoSuchWallet (ErrNoSuchWallet wid)) -errCannotRemovePendingTx (NoSuchTx _ tid) = - Just (ErrRemoveTxNoSuchTransaction tid) -errCannotRemovePendingTx (CantRemoveTxInLedger _ tid) = - Just (ErrRemoveTxAlreadyInLedger tid) +errCannotRemovePendingTx :: Err (PrimaryKey WalletId) -> Maybe ErrRemovePendingTx +errCannotRemovePendingTx (CannotRemovePendingTx (ErrErasePendingTxNoSuchWallet (PrimaryKey wid))) = + Just (ErrRemovePendingTxNoSuchWallet (ErrNoSuchWallet wid)) +errCannotRemovePendingTx (CannotRemovePendingTx (ErrErasePendingTxNoTx tid)) = + Just (ErrRemovePendingTxNoSuchTransaction tid) +errCannotRemovePendingTx (CannotRemovePendingTx (ErrErasePendingTxNoPendingTx tid)) = + Just (ErrRemovePendingTxTransactionNoMorePending tid) errCannotRemovePendingTx _ = Nothing errWalletAlreadyExists diff --git a/lib/core/src/Cardano/Wallet/DB/Model.hs b/lib/core/src/Cardano/Wallet/DB/Model.hs index 3a35ba27906..5530bc6f506 100644 --- a/lib/core/src/Cardano/Wallet/DB/Model.hs +++ b/lib/core/src/Cardano/Wallet/DB/Model.hs @@ -38,6 +38,7 @@ module Cardano.Wallet.DB.Model -- * Model Operation Types , ModelOp , Err (..) + , ErrErasePendingTx (..) -- * Model database functions , mCleanDB , mInitializeWallet @@ -54,7 +55,7 @@ module Cardano.Wallet.DB.Model , mPutTxHistory , mReadTxHistory , mUpdatePendingTxForExpiry - , mRemovePendingOrExpiredTx + , mRemovePendingTx , mPutPrivateKey , mReadPrivateKey , mPutProtocolParameters @@ -183,8 +184,13 @@ type ModelOp wid s xprv a = data Err wid = NoSuchWallet wid | WalletAlreadyExists wid - | NoSuchTx wid (Hash "Tx") - | CantRemoveTxInLedger wid (Hash "Tx") + | CannotRemovePendingTx (ErrErasePendingTx wid) + deriving (Show, Eq, Functor, Foldable, Traversable) + +data ErrErasePendingTx wid + = ErrErasePendingTxNoSuchWallet wid + | ErrErasePendingTxNoTx (Hash "Tx") + | ErrErasePendingTxNoPendingTx (Hash "Tx") deriving (Show, Eq, Functor, Foldable, Traversable) {------------------------------------------------------------------------------- @@ -267,15 +273,20 @@ mUpdatePendingTxForExpiry wid currentTip = alterModel wid $ \wal -> _ -> txMeta -mRemovePendingOrExpiredTx :: Ord wid => wid -> Hash "Tx" -> ModelOp wid s xprv () -mRemovePendingOrExpiredTx wid tid = alterModelErr wid $ \wal -> - case Map.lookup tid (txHistory wal) of +mRemovePendingTx :: Ord wid => wid -> (Hash "Tx") -> ModelOp wid s xprv () +mRemovePendingTx wid tid db@(Database wallets txs) = case Map.lookup wid wallets of + Nothing -> + ( Left (CannotRemovePendingTx (ErrErasePendingTxNoSuchWallet wid)), db ) + Just wal -> case Map.lookup tid (txHistory wal) of Nothing -> - ( Left (NoSuchTx wid tid), wal ) - Just txMeta | txMeta ^. #status == InLedger -> - ( Left (CantRemoveTxInLedger wid tid), wal ) - Just _ -> - ( Right (), wal { txHistory = Map.delete tid (txHistory wal) } ) + ( Left (CannotRemovePendingTx (ErrErasePendingTxNoTx tid)), db ) + Just txMeta -> + if (status :: TxMeta -> TxStatus) txMeta == Pending then + ( Right (), Database updateWallets txs ) + else ( Left (CannotRemovePendingTx (ErrErasePendingTxNoPendingTx tid)), db ) + where + updateWallets = Map.adjust changeTxMeta wid wallets + changeTxMeta meta = meta { txHistory = Map.delete tid (txHistory meta) } mRollbackTo :: Ord wid => wid -> SlotNo -> ModelOp wid s xprv SlotNo mRollbackTo wid requested db@(Database wallets txs) = case Map.lookup wid wallets of @@ -498,29 +509,14 @@ mReadDelegationRewardBalance wid db@(Database wallets _) = Model function helpers -------------------------------------------------------------------------------} --- | Create a 'ModelOp' which mutates the database for a certain wallet id. --- --- The given function returns a value and a modified wallet database. alterModel :: Ord wid => wid -> (WalletDatabase s xprv -> (a, WalletDatabase s xprv)) -> ModelOp wid s xprv a -alterModel wid f = alterModelErr wid (first Right . f) - --- | Create a 'ModelOp' which mutates the database for a certain wallet id. --- --- The given function returns a either a value or error, and a modified wallet --- database. -alterModelErr - :: Ord wid - => wid - -> (WalletDatabase s xprv -> (Either (Err wid) a, WalletDatabase s xprv)) - -> ModelOp wid s xprv a -alterModelErr wid f db@Database{wallets,txs} = - case f <$> Map.lookup wid wallets of - Just (a, wal) -> (a, Database (Map.insert wid wal wallets) txs) - Nothing -> (Left (NoSuchWallet wid), db) +alterModel wid f db@Database{wallets,txs} = case f <$> Map.lookup wid wallets of + Just (a, wal) -> (Right a, Database (Map.insert wid wal wallets) txs) + Nothing -> (Left (NoSuchWallet wid), db) -- | Apply optional filters on slotNo and sort using the default sort order -- (first time/slotNo, then by TxId) to a 'TxHistory'. diff --git a/lib/core/src/Cardano/Wallet/DB/Sqlite.hs b/lib/core/src/Cardano/Wallet/DB/Sqlite.hs index fea40c05294..3e0643eb600 100644 --- a/lib/core/src/Cardano/Wallet/DB/Sqlite.hs +++ b/lib/core/src/Cardano/Wallet/DB/Sqlite.hs @@ -57,7 +57,7 @@ import Cardano.Wallet.DB ( DBFactory (..) , DBLayer (..) , ErrNoSuchWallet (..) - , ErrRemoveTx (..) + , ErrRemovePendingTx (..) , ErrWalletAlreadyExists (..) , PrimaryKey (..) , defaultSparseCheckpointsConfig @@ -166,7 +166,6 @@ import Database.Persist.Sql , Update (..) , deleteCascadeWhere , deleteWhere - , deleteWhereCount , insertMany_ , insert_ , rawExecute @@ -919,22 +918,24 @@ newDBLayer trace defaultFieldValues mDatabaseFile timeInterpreter = do updatePendingTxForExpiryQuery wid tip pure $ Right () - , removePendingOrExpiredTx = \(PrimaryKey wid) tid -> ExceptT $ do + , removePendingTx = \(PrimaryKey wid) tid -> ExceptT $ do let errNoSuchWallet = - Left $ ErrRemoveTxNoSuchWallet $ ErrNoSuchWallet wid + Left $ ErrRemovePendingTxNoSuchWallet $ ErrNoSuchWallet wid let errNoMorePending = - Left $ ErrRemoveTxAlreadyInLedger tid + Left $ ErrRemovePendingTxTransactionNoMorePending tid let errNoSuchTransaction = - Left $ ErrRemoveTxNoSuchTransaction tid + Left $ ErrRemovePendingTxNoSuchTransaction tid selectWallet wid >>= \case Nothing -> pure errNoSuchWallet - Just _ -> selectTxMeta wid tid >>= \case - Nothing -> pure errNoSuchTransaction - Just _ -> do - count <- deletePendingOrExpiredTx wid tid - pure $ if count == 0 - then errNoMorePending - else Right () + Just _ -> do + metas <- selectPendingTxs wid (TxId tid) + let isPending meta = txMetaStatus meta == W.Pending + case metas of + [] -> pure errNoSuchTransaction + txs | any isPending txs -> do + deletePendingTx wid (TxId tid) + pure $ Right () + _ -> pure errNoMorePending , getTx = \(PrimaryKey wid) tid -> ExceptT $ do selectLatestCheckpoint wid >>= \case @@ -1566,31 +1567,26 @@ selectTxHistory cp ti wid minWithdrawal order conditions = do W.Ascending -> [Asc TxMetaSlot, Desc TxMetaTxId] W.Descending -> [Desc TxMetaSlot, Asc TxMetaTxId] -selectTxMeta +selectPendingTxs :: W.WalletId - -> W.Hash "Tx" - -> SqlPersistT IO (Maybe TxMeta) -selectTxMeta wid tid = - fmap entityVal <$> selectFirst - [ TxMetaWalletId ==. wid, TxMetaTxId ==. (TxId tid)] - [ Desc TxMetaSlot ] - --- | Delete the transaction, but only if it's not in ledger. --- Returns non-zero if this was a success. -deletePendingOrExpiredTx + -> TxId + -> SqlPersistT IO [TxMeta] +selectPendingTxs wid tid = + fmap entityVal <$> selectList + [TxMetaWalletId ==. wid, TxMetaTxId ==. tid] [] + +deletePendingTx :: W.WalletId - -> W.Hash "Tx" - -> SqlPersistT IO Int -deletePendingOrExpiredTx wid tid = do - let filt = [ TxMetaWalletId ==. wid, TxMetaTxId ==. (TxId tid) ] - selectFirst ((TxMetaStatus ==. W.InLedger):filt) [] >>= \case - Just _ -> pure 0 -- marked in ledger - refuse to delete - Nothing -> fromIntegral <$> deleteWhereCount - ((TxMetaStatus <-. [W.Pending, W.Expired]):filt) - --- | Mutates all pending transaction entries which have exceeded their TTL so --- that their status becomes expired. Transaction expiry is not something which --- can be rolled back. + -> TxId + -> SqlPersistT IO () +deletePendingTx wid tid = do + deleteWhere + [ TxMetaWalletId ==. wid, TxMetaTxId ==. tid + , TxMetaStatus ==. W.Pending ] + +-- Mutates all pending transaction entries which have exceeded their TTL so that +-- their status becomes expired. Transaction expiry is not something which can +-- be rolled back. updatePendingTxForExpiryQuery :: W.WalletId -> W.SlotNo diff --git a/lib/core/src/Cardano/Wallet/Primitive/Types.hs b/lib/core/src/Cardano/Wallet/Primitive/Types.hs index f665a2ec2e2..d9ef7ff1db4 100644 --- a/lib/core/src/Cardano/Wallet/Primitive/Types.hs +++ b/lib/core/src/Cardano/Wallet/Primitive/Types.hs @@ -963,9 +963,6 @@ instance Buildable TxOut where instance Buildable (TxIn, TxOut) where build (txin, txout) = build txin <> " ==> " <> build txout --- | Additional information about a transaction, derived from the transaction --- and ledger state. This should not be confused with 'TxMetadata' which is --- application-specific data included with the transaction. data TxMeta = TxMeta { status :: !TxStatus , direction :: !Direction @@ -989,11 +986,8 @@ instance Buildable TxMeta where data TxStatus = Pending - -- ^ Created, but not yet in a block. | InLedger - -- ^ Has been found in a block. | Expired - -- ^ Time to live (TTL) has passed. deriving (Show, Eq, Ord, Bounded, Enum, Generic) instance NFData TxStatus diff --git a/lib/core/test/unit/Cardano/Wallet/DB/StateMachine.hs b/lib/core/test/unit/Cardano/Wallet/DB/StateMachine.hs index 1dce3c77a5b..cb06be080d8 100644 --- a/lib/core/test/unit/Cardano/Wallet/DB/StateMachine.hs +++ b/lib/core/test/unit/Cardano/Wallet/DB/StateMachine.hs @@ -55,7 +55,7 @@ import Cardano.Address.Derivation import Cardano.Wallet.DB ( DBLayer (..) , ErrNoSuchWallet (..) - , ErrRemoveTx (..) + , ErrRemovePendingTx (..) , ErrWalletAlreadyExists (..) , PrimaryKey (..) , cleanDB @@ -65,6 +65,7 @@ import Cardano.Wallet.DB.Arbitrary import Cardano.Wallet.DB.Model ( Database , Err (..) + , ErrErasePendingTx (..) , TxHistory , WalletDatabase (..) , emptyDatabase @@ -86,7 +87,7 @@ import Cardano.Wallet.DB.Model , mReadProtocolParameters , mReadTxHistory , mReadWalletMeta - , mRemovePendingOrExpiredTx + , mRemovePendingTx , mRemoveWallet , mRollbackTo , mUpdatePendingTxForExpiry @@ -394,7 +395,7 @@ runMock = \case RollbackTo wid sl -> first (Resp . fmap Point) . mRollbackTo wid sl RemovePendingTx wid tid -> - first (Resp . fmap Unit) . mRemovePendingOrExpiredTx wid tid + first (Resp . fmap Unit) . mRemovePendingTx wid tid UpdatePendingTxForExpiry wid sl -> first (Resp . fmap Unit) . mUpdatePendingTxForExpiry wid sl where @@ -447,8 +448,8 @@ runIO db@DBLayer{..} = fmap Resp . go mapExceptT atomically $ putTxHistory (PrimaryKey wid) txs ReadTxHistory wid minWith order range status -> Right . TxHistory <$> atomically (readTxHistory (PrimaryKey wid) minWith order range status) - RemovePendingTx wid tid -> (catchCannotRemovePendingTx wid) Unit $ - mapExceptT atomically $ removePendingOrExpiredTx (PrimaryKey wid) tid + RemovePendingTx wid tid -> catchCannotRemovePendingTx Unit $ + mapExceptT atomically $ removePendingTx (PrimaryKey wid) tid UpdatePendingTxForExpiry wid sl -> catchNoSuchWallet Unit $ mapExceptT atomically $ updatePendingTxForExpiry (PrimaryKey wid) sl PutPrivateKey wid pk -> catchNoSuchWallet Unit $ @@ -470,8 +471,8 @@ runIO db@DBLayer{..} = fmap Resp . go fmap (bimap errWalletAlreadyExists f) . runExceptT catchNoSuchWallet f = fmap (bimap errNoSuchWallet f) . runExceptT - catchCannotRemovePendingTx wid f = - fmap (bimap (errCannotRemovePendingTx wid) f) . runExceptT + catchCannotRemovePendingTx f = + fmap (bimap errCannotRemovePendingTx f) . runExceptT errNoSuchWallet :: ErrNoSuchWallet -> Err WalletId errNoSuchWallet (ErrNoSuchWallet wid) = NoSuchWallet wid @@ -479,13 +480,13 @@ runIO db@DBLayer{..} = fmap Resp . go errWalletAlreadyExists :: ErrWalletAlreadyExists -> Err WalletId errWalletAlreadyExists (ErrWalletAlreadyExists wid) = WalletAlreadyExists wid - errCannotRemovePendingTx :: WalletId -> ErrRemoveTx -> Err WalletId - errCannotRemovePendingTx _ (ErrRemoveTxNoSuchWallet e) = - errNoSuchWallet e - errCannotRemovePendingTx wid (ErrRemoveTxNoSuchTransaction tid) = - NoSuchTx wid tid - errCannotRemovePendingTx wid (ErrRemoveTxAlreadyInLedger tid) = - CantRemoveTxInLedger wid tid + errCannotRemovePendingTx :: ErrRemovePendingTx -> Err WalletId + errCannotRemovePendingTx (ErrRemovePendingTxNoSuchWallet (ErrNoSuchWallet wid)) = + CannotRemovePendingTx (ErrErasePendingTxNoSuchWallet wid) + errCannotRemovePendingTx (ErrRemovePendingTxNoSuchTransaction tid) = + CannotRemovePendingTx (ErrErasePendingTxNoTx tid) + errCannotRemovePendingTx (ErrRemovePendingTxTransactionNoMorePending tid) = + CannotRemovePendingTx (ErrErasePendingTxNoPendingTx tid) unPrimaryKey :: PrimaryKey key -> key unPrimaryKey (PrimaryKey key) = key diff --git a/lib/jormungandr/test/integration/Test/Integration/Jormungandr/Scenario/CLI/Transactions.hs b/lib/jormungandr/test/integration/Test/Integration/Jormungandr/Scenario/CLI/Transactions.hs index fd1d35f8b19..54602acff86 100644 --- a/lib/jormungandr/test/integration/Test/Integration/Jormungandr/Scenario/CLI/Transactions.hs +++ b/lib/jormungandr/test/integration/Test/Integration/Jormungandr/Scenario/CLI/Transactions.hs @@ -66,7 +66,7 @@ import Test.Integration.Framework.DSL import Test.Integration.Framework.TestData ( errMsg400MalformedTxPayload , errMsg400WronglyEncodedTxPayload - , errMsg403AlreadyInLedger + , errMsg403NoPendingAnymore ) import Test.Integration.Jormungandr.Scenario.API.Transactions ( ExternalTxFixture (..), fixtureExternalTx, getWalletBalance ) @@ -205,6 +205,6 @@ spec = do -- Try to forget external tx (Exit c2, Stdout out2, Stderr err2) <- deleteTransactionViaCLI @t ctx (T.unpack $ w ^. walletId) txid - err2 `shouldContain` errMsg403AlreadyInLedger (T.pack txid) + err2 `shouldContain` errMsg403NoPendingAnymore (T.pack txid) out2 `shouldBe` "" c2 `shouldBe` ExitFailure 1 diff --git a/specifications/api/swagger.yaml b/specifications/api/swagger.yaml index 953c099bea2..5931752706e 100644 --- a/specifications/api/swagger.yaml +++ b/specifications/api/swagger.yaml @@ -2169,16 +2169,16 @@ x-errNoSuchTransaction: &errNoSuchTransaction type: string enum: ['no_such_transaction'] -x-errTransactionAlreadyInLedger: &errTransactionAlreadyInLedger +x-errTransactionNotPending: &errTransactionNotPending <<: *responsesErr - title: transaction_already_in_ledger + title: transaction_not_pending properties: message: type: string - description: Occurs when attempting to delete a transaction which is neither pending nor expired. + description: May occur when trying to forget a transaction that is not pending. code: type: string - enum: ['transaction_already_in_ledger'] + enum: ['transaction_not_pending'] x-errWalletAlreadyExists: &errWalletAlreadyExists <<: *responsesErr @@ -2879,7 +2879,7 @@ x-responsesDeleteTransaction: &responsesDeleteTransaction description: Forbidden content: application/json: - schema: *errTransactionAlreadyInLedger + schema: *errTransactionNotPending 404: description: Not Found content: