From c486ba2f5166237a9d931e9e126f6aab8c345e92 Mon Sep 17 00:00:00 2001 From: Pawel Jakubas Date: Fri, 1 Feb 2019 16:23:05 +0100 Subject: [PATCH 1/2] Support UtxoNotEnoughFragmented error Add multioutput integration tests Fix Lens accesses and confusion between WalAddress vs WalletAddress vs V1 Address --- .../Cardano/Wallet/API/V1/ReifyWalletError.hs | 3 + wallet/src/Cardano/Wallet/API/V1/Swagger.hs | 36 ++++- wallet/src/Cardano/Wallet/API/V1/Types.hs | 30 ++++- .../src/Cardano/Wallet/Kernel/Transactions.hs | 32 +++++ .../Test/Integration/Framework/DSL.hs | 24 ++++ .../Test/Integration/Scenario/Transactions.hs | 123 +++++++++++++++++- 6 files changed, 244 insertions(+), 4 deletions(-) diff --git a/wallet/src/Cardano/Wallet/API/V1/ReifyWalletError.hs b/wallet/src/Cardano/Wallet/API/V1/ReifyWalletError.hs index ad22df3083a..1c576f4615a 100644 --- a/wallet/src/Cardano/Wallet/API/V1/ReifyWalletError.hs +++ b/wallet/src/Cardano/Wallet/API/V1/ReifyWalletError.hs @@ -306,6 +306,9 @@ newTransactionError e = case e of Kernel.NewTransactionInvalidTxIn -> V1.SignedTxSubmitError "NewTransactionInvalidTxIn" + (Kernel.NewTransactionNotEnoughUtxoFragmentation (Kernel.NumberOfMissingUtxos missingUtxo)) -> + V1.UtxoNotEnoughFragmented (V1.ErrUtxoNotEnoughFragmented missingUtxo V1.msgUtxoNotEnoughFragmented) + redeemAdaError :: RedeemAdaError -> V1.WalletError redeemAdaError e = case e of (RedeemAdaError e') -> case e' of diff --git a/wallet/src/Cardano/Wallet/API/V1/Swagger.hs b/wallet/src/Cardano/Wallet/API/V1/Swagger.hs index 2d8af778773..9a233dd273b 100644 --- a/wallet/src/Cardano/Wallet/API/V1/Swagger.hs +++ b/wallet/src/Cardano/Wallet/API/V1/Swagger.hs @@ -335,7 +335,7 @@ $errors -- 'UnsupportedMimeTypeError' , mkRow fmtErr $ UnsupportedMimeTypePresent "Expected Content-Type's main MIME-type to be 'application/json'." - + , mkRow fmtErr $ UtxoNotEnoughFragmented (ErrUtxoNotEnoughFragmented 1 msgUtxoNotEnoughFragmented) -- TODO 'MnemonicError' ? ] mkRow fmt err = T.intercalate "|" (fmt err) @@ -684,6 +684,40 @@ curl -X POST https://localhost:8090/api/v1/transactions \ ``` +About UTXO Fragmentation +------------------------ + +As described in [Sending Money to Multiple Recipients](#section/Common-Use-Cases/Sending-Money-to-Multiple-Recipients), it is possible to send ada to more than one destination. Cardano only allows a given UTXO to cover at most one single transaction output. As a result, +when the number of transaction outputs is greater than the number the API returns a `UtxoNotEnoughFragmented` error which +looks like the following +``` +{ + "status": "error", + "diagnostic": { + "details": { + "help": "Utxo is not enough fragmented to handle the number of outputs of this transaction. Query /api/v1/wallets/{walletId}/statistics/utxos endpoint for more information", + "missingUtxos": 1 + } + }, + "message": "UtxoNotEnoughFragmented" +} +``` + +To make sure the source account has a sufficient level of UTXO fragmentation (i.e. number of UTXOs), +please monitor the state of the UTXOs as described in [Getting UTXO Statistics](#section/Common-Use-Cases/Getting-Utxo-Statistics). The +number of wallet UTXOs should be no less than the transaction outputs, and the sum of all UTXOs should be enough to cover the total +transaction amount, including fees. + +Contrary to a classic accounting model, there's no such thing as spending _part of a UTXO_, and one has to wait for a transaction to be included in a +block before spending the remaining change. This is very similar to using bank notes: one can't spend a USD 20 bill at two different shops at the same time, +even if it is enough to cover both purchases — one has to wait for change from the first transaction before making the second one. +There's no "ideal" level of fragmentation; it depends on one's needs. However, the more UTXOs that are available, the higher the concurrency capacity +of one's wallet, allowing multiple transactions to be made at the same time. + +Similarly, there's no practical maximum number of UTXOs, but there is nevertheless a maximum transaction size. By having many small UTXOs, +one is taking the risk of hitting that restriction, should too many inputs be selected to fill a transaction. The only way to +work around this is to make multiple smaller transactions. + Estimating Transaction Fees --------------------------- diff --git a/wallet/src/Cardano/Wallet/API/V1/Types.hs b/wallet/src/Cardano/Wallet/API/V1/Types.hs index 3bfd722e3c6..efbc83248b8 100644 --- a/wallet/src/Cardano/Wallet/API/V1/Types.hs +++ b/wallet/src/Cardano/Wallet/API/V1/Types.hs @@ -106,6 +106,8 @@ module Cardano.Wallet.API.V1.Types ( -- * Wallet Errors , WalletError(..) , ErrNotEnoughMoney(..) + , ErrUtxoNotEnoughFragmented(..) + , msgUtxoNotEnoughFragmented , toServantError , toHttpErrorStatus , module Cardano.Wallet.Types.UtxoStatistics @@ -1883,6 +1885,21 @@ instance FromJSON ErrNotEnoughMoney where when (msg /= sformat build (ErrAvailableBalanceIsInsufficient 0)) mempty ErrAvailableBalanceIsInsufficient <$> (o .: "availableBalance") +data ErrUtxoNotEnoughFragmented = ErrUtxoNotEnoughFragmented { + theMissingUtxos :: !Int + , theHelp :: !Text + } deriving (Eq, Generic, Show) + + +msgUtxoNotEnoughFragmented :: Text +msgUtxoNotEnoughFragmented = "Utxo is not enough fragmented to handle the number of outputs of this transaction. Query /api/v1/wallets/{walletId}/statistics/utxos endpoint for more information" + +deriveJSON Aeson.defaultOptions ''ErrUtxoNotEnoughFragmented + +instance Buildable ErrUtxoNotEnoughFragmented where + build (ErrUtxoNotEnoughFragmented missingUtxos _ ) = + bprint ("Missing "%build%" utxo(s) to accommodate all outputs of the transaction") missingUtxos + -- | Type representing any error which might be thrown by wallet. -- @@ -1948,6 +1965,9 @@ data WalletError = | RequestThrottled !Word64 -- ^ The request has been throttled. The 'Word64' is a count of microseconds -- until the user should retry. + | UtxoNotEnoughFragmented !ErrUtxoNotEnoughFragmented + -- ^ available Utxo is not enough fragmented, ie., there is more outputs of transaction than + -- utxos deriving (Generic, Show, Eq) deriveGeneric ''WalletError @@ -1990,6 +2010,9 @@ instance Arbitrary WalletError where , NodeIsStillSyncing <$> arbitrary , CannotCreateAddress <$> arbitraryText , RequestThrottled <$> arbitrary + , UtxoNotEnoughFragmented <$> Gen.oneof + [ ErrUtxoNotEnoughFragmented <$> Gen.choose (1, 10) <*> arbitrary + ] ] where arbitraryText :: Gen Text @@ -2046,7 +2069,8 @@ instance Buildable WalletError where bprint "Cannot create derivation path for new address, for external wallet." RequestThrottled _ -> bprint "You've made too many requests too soon, and this one was throttled." - + UtxoNotEnoughFragmented x -> + bprint build x -- | Convert wallet errors to Servant errors instance ToServantError WalletError where @@ -2089,6 +2113,8 @@ instance ToServantError WalletError where err500 RequestThrottled{} -> err400 { errHTTPCode = 429 } + UtxoNotEnoughFragmented{} -> + err403 -- | Declare the key used to wrap the diagnostic payload, if any instance HasDiagnostic WalletError where @@ -2131,3 +2157,5 @@ instance HasDiagnostic WalletError where "msg" RequestThrottled{} -> "microsecondsUntilRetry" + UtxoNotEnoughFragmented{} -> + "details" diff --git a/wallet/src/Cardano/Wallet/Kernel/Transactions.hs b/wallet/src/Cardano/Wallet/Kernel/Transactions.hs index ae627d356e4..4327dacac47 100644 --- a/wallet/src/Cardano/Wallet/Kernel/Transactions.hs +++ b/wallet/src/Cardano/Wallet/Kernel/Transactions.hs @@ -10,6 +10,7 @@ module Cardano.Wallet.Kernel.Transactions ( , PaymentError(..) , EstimateFeesError(..) , RedeemAdaError(..) + , NumberOfMissingUtxos(..) , cardanoFee , mkStdTx , prepareUnsignedTxWithSources @@ -85,6 +86,16 @@ import UTxO.Util (shuffleNE) Generating payments and estimating fees -------------------------------------------------------------------------------} +data NumberOfMissingUtxos = NumberOfMissingUtxos Int + +instance Buildable NumberOfMissingUtxos where + build (NumberOfMissingUtxos number) = + bprint ("NumberOfMissingUtxos " % build) number + +instance Arbitrary NumberOfMissingUtxos where + arbitrary = oneof [ NumberOfMissingUtxos <$> arbitrary + ] + data NewTransactionError = NewTransactionUnknownAccount UnknownHdAccount | NewTransactionUnknownAddress UnknownHdAddress @@ -92,6 +103,7 @@ data NewTransactionError = | NewTransactionErrorCreateAddressFailed Kernel.CreateAddressError | NewTransactionErrorSignTxFailed SignTransactionError | NewTransactionInvalidTxIn + | NewTransactionNotEnoughUtxoFragmentation NumberOfMissingUtxos instance Buildable NewTransactionError where build (NewTransactionUnknownAccount err) = @@ -106,6 +118,9 @@ instance Buildable NewTransactionError where bprint ("NewTransactionErrorSignTxFailed " % build) err build NewTransactionInvalidTxIn = bprint "NewTransactionInvalidTxIn" + build (NewTransactionNotEnoughUtxoFragmentation err) = + bprint ("NewTransactionNotEnoughUtxoFragmentation" % build) err + instance Arbitrary NewTransactionError where arbitrary = oneof [ @@ -117,6 +132,7 @@ instance Arbitrary NewTransactionError where , NewTransactionErrorCreateAddressFailed <$> arbitrary , NewTransactionErrorSignTxFailed <$> arbitrary , pure NewTransactionInvalidTxIn + , NewTransactionNotEnoughUtxoFragmentation <$> arbitrary ] data PaymentError = PaymentNewTransactionError NewTransactionError @@ -214,6 +230,9 @@ newUnsignedTransaction ActiveWallet{..} options accountId payees = runExceptT $ availableUtxo <- withExceptT NewTransactionUnknownAccount $ exceptT $ currentAvailableUtxo snapshot accountId + withExceptT NewTransactionNotEnoughUtxoFragmentation $ exceptT $ + checkUtxoFragmentation payees availableUtxo + -- STEP 1: Run coin selection. CoinSelFinalResult inputs outputs coins <- withExceptT NewTransactionErrorCoinSelectionFailed $ ExceptT $ @@ -247,6 +266,19 @@ newUnsignedTransaction ActiveWallet{..} options accountId payees = runExceptT $ toTxOut :: (Address, Coin) -> TxOutAux toTxOut (a, c) = TxOutAux (TxOut a c) + checkUtxoFragmentation + :: NonEmpty (Address, Coin) + -> Utxo + -> Either NumberOfMissingUtxos () + checkUtxoFragmentation outputs inputs = + let numberOfUtxo = Map.size inputs + numberOfOutputs = NonEmpty.length outputs + diff = numberOfOutputs - numberOfUtxo + in if diff > 0 then + Left $ NumberOfMissingUtxos diff + else + Right () + -- | Creates a new unsigned transaction. -- -- NOTE: this function does /not/ perform a payment, it just creates a new diff --git a/wallet/test/integration/Test/Integration/Framework/DSL.hs b/wallet/test/integration/Test/Integration/Framework/DSL.hs index 2378f2ea22e..cc024997663 100644 --- a/wallet/test/integration/Test/Integration/Framework/DSL.hs +++ b/wallet/test/integration/Test/Integration/Framework/DSL.hs @@ -37,6 +37,7 @@ module Test.Integration.Framework.DSL , defaultAccountId , defaultAssuranceLevel , defaultDistribution + , customDistribution , defaultGroupingPolicy , defaultPage , defaultPerPage @@ -72,6 +73,8 @@ module Test.Integration.Framework.DSL , ($-) , () , (!!) + , addresses + , walAddresses , amount , assuranceLevel , backupPhrase @@ -106,6 +109,7 @@ import Data.Generics.Internal.VL.Lens (lens) import Data.Generics.Product.Fields (field) import Data.Generics.Product.Typed (HasType, typed) import Data.List ((!!)) +import qualified Data.List.NonEmpty as NonEmpty import qualified Data.Map.Strict as Map import qualified Data.Text as T import qualified Data.Text.Encoding as T @@ -268,6 +272,21 @@ defaultDistribution defaultDistribution c s = pure $ PaymentDistribution (V1 $ head $ s ^. typed) (V1 $ mkCoin c) +customDistribution + :: NonEmpty (Account, Word64) + -> NonEmpty PaymentDistribution +customDistribution payees = + let recepientWalAddresses = + NonEmpty.fromList + $ map (view walAddresses) + $ concatMap (view addresses . fst) + $ payees + + in NonEmpty.zipWith + PaymentDistribution + recepientWalAddresses + (map ((\coin -> V1 $ mkCoin coin) . snd) payees) + defaultGroupingPolicy :: Maybe (V1 InputSelectionPolicy) defaultGroupingPolicy = Nothing @@ -386,6 +405,11 @@ walletName = typed spendingPasswordLastUpdate :: Lens' Wallet (V1 Timestamp) spendingPasswordLastUpdate = field @"walSpendingPasswordLastUpdate" +addresses :: HasType [WalletAddress] s => Lens' s [WalletAddress] +addresses = typed + +walAddresses :: HasType (V1 Address) s => Lens' s (V1 Address) +walAddresses = typed -- -- EXPECTATIONS diff --git a/wallet/test/integration/Test/Integration/Scenario/Transactions.hs b/wallet/test/integration/Test/Integration/Scenario/Transactions.hs index 7b79c6a0b01..7ebcafbefdf 100644 --- a/wallet/test/integration/Test/Integration/Scenario/Transactions.hs +++ b/wallet/test/integration/Test/Integration/Scenario/Transactions.hs @@ -4,12 +4,95 @@ module Test.Integration.Scenario.Transactions import Universum +import qualified Data.List.NonEmpty as NonEmpty + import qualified Cardano.Wallet.Client.Http as Client import Test.Integration.Framework.DSL - +import Test.Integration.Framework.Scenario (Scenario) spec :: Scenarios Context spec = do + + -- estimated fee amount for this transaction is 187946 + multioutputTransactionScenario + "proper fragmentation of utxo - both utxos can seperately cover fee and outputs" + [200000, 200000] + [10,11] + [ expectTxStatusEventually [InNewestBlocks] ] + + -- estimated fee amount for this transaction is 187946 + multioutputTransactionScenario + "proper fragmentation of utxo - 2 utxos available, each cannot seperately cover fee and outputs, but jointly can" + [100000, 100000] + [10,11] + [ expectTxStatusEventually [InNewestBlocks] ] + + -- estimated fee amount for this transaction is 196076 + multioutputTransactionScenario + "proper fragmentation of utxo - 3 utxos available, each cannot seperately cover fee and outputs, but jointly can" + [70000, 70000, 70000] + [10,11] + [ expectTxStatusEventually [InNewestBlocks] ] + + -- estimated fee amount for this transaction is 187946 + multioutputTransactionScenario + "proper fragmentation of utxo - 2 utxos available, one can cover fee, cannot cover any output seperately, but jointly can" + [187950, 50] + [10,11] + [ expectTxStatusEventually [InNewestBlocks] ] + + multioutputTransactionScenario + "not enough fragmentation of utxo although available utxo can cover both outputs and fee" + [400000] + [10,11] + [ expectWalletError (UtxoNotEnoughFragmented (Client.ErrUtxoNotEnoughFragmented 1 Client.msgUtxoNotEnoughFragmented)) ] + + multioutputTransactionScenario + "not enough fragmentation of utxo and available utxo cannot cover the sum of outputs and fee" + [100000] + [10,11] + [ expectWalletError (UtxoNotEnoughFragmented (Client.ErrUtxoNotEnoughFragmented 1 Client.msgUtxoNotEnoughFragmented)) ] + + + scenario "cannot send subsequent transaction when the first one is pending" $ do + fixtureSource <- setup $ defaultSetup + & initialCoins .~ [10000000] + & rawPassword .~ "raw password" + + fixtureDest <- setup $ defaultSetup + + -- Running two transactions one after another. Not waiting for the first transaction to be "completed". + -- The second transaction returns UtxoNotEnoughFragmented because the first one is still "pending" + resp1 <- request $ Client.postTransaction $- Payment + (defaultSource fixtureSource) + (defaultDistribution 1 fixtureDest) + defaultGroupingPolicy + (Just $ fixtureDest ^. spendingPassword) + verify resp1 + [ expectSuccess + ] + + resp2 <- request $ Client.postTransaction $- Payment + (defaultSource fixtureSource) + (defaultDistribution 1 fixtureDest) + defaultGroupingPolicy + (Just $ fixtureDest ^. spendingPassword) + verify resp2 + [ expectWalletError (UtxoNotEnoughFragmented (Client.ErrUtxoNotEnoughFragmented 1 Client.msgUtxoNotEnoughFragmented)) + ] + + -- only after the first transaction completes the next one can be successfully sent + expectTxStatusEventually [InNewestBlocks, Persisted] resp1 + + resp3 <- request $ Client.postTransaction $- Payment + (defaultSource fixtureSource) + (defaultDistribution 1 fixtureDest) + defaultGroupingPolicy + (Just $ fixtureDest ^. spendingPassword) + verify resp3 + [ expectTxStatusEventually [InNewestBlocks, Persisted] + ] + scenario "successful payment appears in the history" $ do fixture <- setup $ defaultSetup & initialCoins .~ [1000000] @@ -49,7 +132,7 @@ spec = do noSpendingPassword verify response - [ expectWalletError (NotEnoughMoney (ErrAvailableBalanceIsInsufficient 0)) + [ expectWalletError (UtxoNotEnoughFragmented (Client.ErrUtxoNotEnoughFragmented 1 Client.msgUtxoNotEnoughFragmented)) ] scenario "payment fails when wallet has insufficient funds" $ do @@ -257,3 +340,39 @@ spec = do verify response [ expectTxStatusEventually [InNewestBlocks, Persisted] ] + + multioutputTransactionScenario + :: String + -> [Word64] + -> [Word64] + -> [Either Client.ClientError Client.Transaction -> Scenario Context IO ()] + -> Scenarios Context + multioutputTransactionScenario title startCoins outputDistribution expectations = + scenario ("multi-output transaction: " <> title) $ do + fixtureSource <- setup $ defaultSetup + & initialCoins .~ startCoins + + fixtureDest1 <- setup $ defaultSetup & walletName .~ "destinationWallet1" + + accountDest1 <- successfulRequest $ Client.getAccount + $- (fixtureDest1 ^. wallet . walletId) + $- defaultAccountId + + fixtureDest2 <- setup $ defaultSetup & walletName .~ "destinationWallet2" + + accountDest2 <- successfulRequest $ Client.getAccount + $- (fixtureDest2 ^. wallet . walletId) + $- defaultAccountId + + response <- request $ Client.postTransaction $- Payment + (defaultSource fixtureSource) + (customDistribution $ + NonEmpty.zipWith + (,) + (accountDest1 :| [accountDest2]) + (NonEmpty.fromList outputDistribution) + ) + defaultGroupingPolicy + noSpendingPassword + + verify response expectations From d58e4bcf8e5bcb05c5130ebd8b93f4ab336521e3 Mon Sep 17 00:00:00 2001 From: Pawel Jakubas Date: Fri, 1 Feb 2019 18:06:37 +0100 Subject: [PATCH 2/2] CHANGELOG.md update --- CHANGELOG.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e7324191fe..11e27f659eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # CHANGELOG -## Cardano SL 3.0.0 +## Cardano SL 3.0.0 ### Fixes @@ -29,7 +29,7 @@ - Additional node settings exposed through the wallet backend API in `/api/v1/node-settings`. This is in order to align and be on-par with the new node monitoring API. [#4045](https://github.com/input-output-hk/cardano-sl/pull/4045) Added settings: - - `slotId`: The current slot and epoch + - `slotId`: The current slot and epoch - `slotCount`: The number of slots per epoch - `maxTxSize`: The largest allowed transaction size in bytes - `feePolicy`: The fee policy, in flat Lovelace and variable Lovelace/byte @@ -269,6 +269,9 @@ - Add a test which checks if the configuration can be correctly parsed - [CDEC-405](https://iohk.myjetbrains.com/youtrack/issue/CDEC-405) [#3175](https://github.com/input-output-hk/cardano-sl/pull/3175) +- Add Utxo not enough fragmentation error handling and multi-output transaction tests + - [cardano-wallet#190](https://github.com/input-output-hk/cardano-wallet/issues/190) [#4058](https://github.com/input-output-hk/cardano-sl/pull/4058) + ### Documentation - Make an inventory of existing wallet errors and exceptions ([CBR-307](https://iohk.myjetbrains.com/youtrack/issue/CO-307))