Merge #2255

2255: do not enforce non-empty outputs for unsigned tx r=KtorZ a=KtorZ

# Issue Number

<!-- Put here a reference to the issue this PR relates to and which requirements it tackles -->

#2200 

# Overview

<!-- Detail in a few bullet points the work accomplished in this PR -->

- c60c5a6
  📍 **do not enforce non-empty outputs for unsigned tx**
    Actually, there are many use-cases for empty outputs. This occurs in
  particular often when performing a selection for delegation: in this
  scenario, there are typically no outputs at all and, depending on the
  size of the selected input(s), there might be no change at all
  (because of the minUTxO value).


# Comments

<!-- Additional comments or screenshots to attach if any -->

Should fix: 

- #2213 (comment)
- #2213 (comment)

Thanks @piotr-iohk 🙏 


<!-- 
Don't forget to:

 ✓ Self-review your changes to make sure nothing unexpected slipped through
 ✓ Assign yourself to the PR
 ✓ Assign one or several reviewer(s)
 ✓ Once created, link this PR to its corresponding ticket
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: KtorZ <matthias.benkort@gmail.com>

lib/core/src/Cardano/Wallet.hs

@@ -1634,7 +1634,7 @@ signTx
     -> Maybe TxMetadata
     -> UnsignedTx (TxIn, TxOut)
     -> ExceptT ErrSignPayment IO (Tx, TxMeta, UTCTime, SealedTx)
-signTx ctx wid pwd md (UnsignedTx inpsNE outsNE) = db & \DBLayer{..} -> do
+signTx ctx wid pwd md (UnsignedTx inpsNE outs) = db & \DBLayer{..} -> do
     withRootKey @_ @s ctx wid pwd ErrSignPaymentWithRootKey $ \xprv scheme -> do
         let pwdP = preparePassphrase scheme pwd
         nodeTip <- withExceptT ErrSignPaymentNetwork $ currentNodeTip nl
@@ -1658,7 +1658,6 @@ signTx ctx wid pwd md (UnsignedTx inpsNE outsNE) = db & \DBLayer{..} -> do
     tl = ctx ^. transactionLayer @t @k
     nl = ctx ^. networkLayer @t
     inps = NE.toList inpsNE
-    outs = NE.toList outsNE
 
 -- | Makes a fully-resolved coin selection for the given set of payments.
 selectCoinsExternal
@@ -1687,8 +1686,7 @@ selectCoinsExternal ctx wid argGenChange selectCoins = do
     UnsignedTx
         <$> (fullyQualifiedInputs s' cs'
                 (ErrSelectCoinsExternalUnableToAssignInputs cs'))
-        <*> ensureNonEmpty (outputs cs')
-                (ErrSelectCoinsExternalUnableToAssignOutputs cs')
+        <*> pure (outputs cs')
   where
     db = ctx ^. dbLayer @s @k
 
@@ -1697,7 +1695,6 @@ data ErrSelectCoinsExternal e
     | ErrSelectCoinsExternalForPayment (ErrSelectForPayment e)
     | ErrSelectCoinsExternalForDelegation ErrSelectForDelegation
     | ErrSelectCoinsExternalUnableToAssignInputs CoinSelection
-    | ErrSelectCoinsExternalUnableToAssignOutputs CoinSelection
     deriving (Eq, Show)
 
 signDelegation
@@ -2265,7 +2262,6 @@ data ErrSelectForDelegation
     = ErrSelectForDelegationNoSuchWallet ErrNoSuchWallet
     | ErrSelectForDelegationFee ErrAdjustForFee
     | ErrSelectForDelegationUnableToAssignInputs ErrNoSuchWallet
-    | ErrSelectForDelegationUnableToAssignOutputs ErrNoSuchWallet
     deriving (Show, Eq)
 
 -- | Errors that can occur when signing a delegation certificate.
@@ -2283,7 +2279,6 @@ data ErrJoinStakePool
     | ErrJoinStakePoolSubmitTx ErrSubmitTx
     | ErrJoinStakePoolCannotJoin ErrCannotJoin
     | ErrJoinStakePoolUnableToAssignInputs CoinSelection
-    | ErrJoinStakePoolUnableToAssignOutputs CoinSelection
     deriving (Generic, Eq, Show)
 
 data ErrQuitStakePool
@@ -2293,7 +2288,6 @@ data ErrQuitStakePool
     | ErrQuitStakePoolSubmitTx ErrSubmitTx
     | ErrQuitStakePoolCannotQuit ErrCannotQuit
     | ErrQuitStakePoolUnableToAssignInputs CoinSelection
-    | ErrQuitStakePoolUnableToAssignOutputs CoinSelection
     deriving (Generic, Eq, Show)
 
 -- | Errors that can occur when fetching the reward balance of a wallet

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

@@ -1659,7 +1659,7 @@ migrateWallet ctx (ApiT wid) migrateData = do
             ti
             (txId tx)
             (fmap Just <$> NE.toList (W.unsignedInputs cs))
-            (NE.toList (W.unsignedOutputs cs))
+            (W.unsignedOutputs cs)
             (tx ^. #withdrawals)
             (meta, time)
             Nothing
@@ -1699,7 +1699,7 @@ assignMigrationAddresses addrs selections =
     makeTx :: CoinSelection -> [Address] -> UnsignedTx (TxIn, TxOut)
     makeTx sel addrsSelected = UnsignedTx
         (NE.fromList (sel ^. #inputs))
-        (NE.fromList (zipWith TxOut addrsSelected (sel ^. #change)))
+        (zipWith TxOut addrsSelected (sel ^. #change))
 
 {-------------------------------------------------------------------------------
                                     Network
@@ -1916,6 +1916,7 @@ mkApiCoinSelection mcerts (UnsignedTx inputs outputs) =
                 ]
       where
         apiStakePath = ApiT <$> xs
+
     mkAddressAmount :: TxOut -> AddressAmount (ApiT Address, Proxy n)
     mkAddressAmount (TxOut addr (Coin c)) =
         AddressAmount (ApiT addr, Proxy @n) (Quantity $ fromIntegral c)
@@ -2273,11 +2274,8 @@ instance Buildable e => LiftHandler (ErrSelectCoinsExternal e) where
         ErrSelectCoinsExternalUnableToAssignInputs e ->
             apiError err500 UnableToAssignInputOutput $ mconcat
                 [ "I'm unable to assign inputs from coin selection: "
-                , pretty e]
-        ErrSelectCoinsExternalUnableToAssignOutputs e ->
-            apiError err500 UnableToAssignInputOutput $ mconcat
-                [ "I'm unable to assign outputs from coin selection: "
-                , pretty e]
+                , pretty e
+                ]
 
 instance Buildable e => LiftHandler (ErrCoinSelection e) where
     handler = \case
@@ -2543,7 +2541,6 @@ instance LiftHandler ErrSelectForDelegation where
                 , "delegation certificate. I need: ", showT cost, " Lovelace."
                 ]
         ErrSelectForDelegationUnableToAssignInputs e -> handler e
-        ErrSelectForDelegationUnableToAssignOutputs e -> handler e
 
 instance LiftHandler ErrSignDelegation where
     handler = \case
@@ -2582,10 +2579,6 @@ instance LiftHandler ErrJoinStakePool where
             apiError err500 UnableToAssignInputOutput $ mconcat
                 [ "I'm unable to assign inputs from coin selection: "
                 , pretty e]
-        ErrJoinStakePoolUnableToAssignOutputs  e ->
-            apiError err500 UnableToAssignInputOutput $ mconcat
-                [ "I'm unable to assign outputs from coin selection: "
-                , pretty e]
 
 instance LiftHandler ErrFetchRewards where
     handler = \case
@@ -2626,10 +2619,6 @@ instance LiftHandler ErrQuitStakePool where
             apiError err500 UnableToAssignInputOutput $ mconcat
                 [ "I'm unable to assign inputs from coin selection: "
                 , pretty e]
-        ErrQuitStakePoolUnableToAssignOutputs e ->
-            apiError err500 UnableToAssignInputOutput $ mconcat
-                [ "I'm unable to assign outputs from coin selection: "
-                , pretty e]
 
 instance LiftHandler ErrCreateRandomAddress where
     handler = \case

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

@@ -410,7 +410,7 @@ data ApiCertificate
 
 data ApiCoinSelection (n :: NetworkDiscriminant) = ApiCoinSelection
     { inputs :: !(NonEmpty (ApiCoinSelectionInput n))
-    , outputs :: !(NonEmpty (AddressAmount (ApiT Address, Proxy n)))
+    , outputs :: ![AddressAmount (ApiT Address, Proxy n)]
     , certificates :: Maybe (NonEmpty ApiCertificate)
     } deriving (Eq, Generic, Show)
 

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

@@ -969,8 +969,20 @@ instance ToText TxStatus where
 data UnsignedTx input = UnsignedTx
     { unsignedInputs
         :: NonEmpty input
+        -- Inputs are *necessarily* non-empty because Cardano requires at least
+        -- one UTxO input per transaction to prevent replayable transactions.
+        -- (each UTxO being unique, including at least one UTxO in the
+        -- transaction body makes it seemingly unique).
+
     , unsignedOutputs
-        :: NonEmpty TxOut
+        :: [TxOut]
+        -- Unlike inputs, it is perfectly reasonable to have empty outputs. The
+        -- main scenario where this might occur is when constructing a
+        -- delegation for the sake of submitting a certificate. This type of
+        -- transaction does not typically include any target output and,
+        -- depending on which input(s) get selected to fuel the transaction, it
+        -- may or may not include a change output should its value be less than
+        -- the minimal UTxO value set by the network.
     }
     deriving (Eq, Show)
 

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

@@ -145,7 +145,7 @@ prop_coinValuesPreserved (CoinSelectionsSetup cs addrs) = do
     let selsCoinValue =
             sum $ getCoinValueFromInp . inputs . getCS <$> cs
     let getCoinValueFromTxOut (UnsignedTx _ txouts) =
-            sum $ map (\(TxOut _ (Coin c)) -> c) $ NE.toList txouts
+            sum $ map (\(TxOut _ (Coin c)) -> c) txouts
     let txsCoinValue =
             sum . map getCoinValueFromTxOut
     txsCoinValue (assignMigrationAddresses addrs sels) === selsCoinValue
@@ -164,7 +164,7 @@ prop_coinValuesPreservedPerTx f (CoinSelectionsSetup cs addrs) = do
             f . map (\(_, TxOut _ (Coin c)) -> c)
     let selsCoinValue = getCoinValueFromInp . inputs . getCS <$> cs
     let getCoinValueFromTxOut (UnsignedTx _ txouts) =
-            f $ map (\(TxOut _ (Coin c)) -> c) $ NE.toList txouts
+            f $ map (\(TxOut _ (Coin c)) -> c) txouts
     let txsCoinValue = map getCoinValueFromTxOut
     txsCoinValue (assignMigrationAddresses addrs sels) === selsCoinValue
 
@@ -202,8 +202,7 @@ prop_fairAddressesRecycled
 prop_fairAddressesRecycled (CoinSelectionsSetup cs addrs) = do
     let sels = getCS <$> cs
     let getAllAddrPerTx (UnsignedTx _ txouts) =
-            map (\(TxOut addr _) -> addr) $
-            NE.toList txouts
+            map (\(TxOut addr _) -> addr) txouts
     let getAllAddrCounts =
             Map.elems .
             foldr (\x -> Map.insertWith (+) x (1::Int)) Map.empty .

specifications/api/swagger.yaml

@@ -520,7 +520,7 @@ x-transactionInputs: &transactionInputs
 x-transactionOutputs: &transactionOutputs
   description: A list of target outputs
   type: array
-  minItems: 1
+  minItems: 0
   items:
     type: object
     required:
@@ -2947,7 +2947,7 @@ paths:
         Random-Improve coin selection algorithm</a>.
 
         <b>Note: </b> Not supported for Byron random wallets.
-        
+
       parameters:
         - *parametersWalletId
       requestBody: