Skip to content

Commit

Permalink
do not enforce non-empty outputs for unsigned tx
Browse files Browse the repository at this point in the history
  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).
  • Loading branch information
KtorZ committed Oct 19, 2020
1 parent 05e59a0 commit c60c5a6
Show file tree
Hide file tree
Showing 6 changed files with 14 additions and 32 deletions.
10 changes: 2 additions & 8 deletions lib/core/src/Cardano/Wallet.hs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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

Expand All @@ -1697,7 +1695,6 @@ data ErrSelectCoinsExternal e
| ErrSelectCoinsExternalForPayment (ErrSelectForPayment e)
| ErrSelectCoinsExternalForDelegation ErrSelectForDelegation
| ErrSelectCoinsExternalUnableToAssignInputs CoinSelection
| ErrSelectCoinsExternalUnableToAssignOutputs CoinSelection
deriving (Eq, Show)

signDelegation
Expand Down Expand Up @@ -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.
Expand All @@ -2283,7 +2279,6 @@ data ErrJoinStakePool
| ErrJoinStakePoolSubmitTx ErrSubmitTx
| ErrJoinStakePoolCannotJoin ErrCannotJoin
| ErrJoinStakePoolUnableToAssignInputs CoinSelection
| ErrJoinStakePoolUnableToAssignOutputs CoinSelection
deriving (Generic, Eq, Show)

data ErrQuitStakePool
Expand All @@ -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
Expand Down
21 changes: 5 additions & 16 deletions lib/core/src/Cardano/Wallet/Api/Server.hs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/core/src/Cardano/Wallet/Api/Types.hs
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
2 changes: 1 addition & 1 deletion lib/core/src/Cardano/Wallet/Primitive/Types.hs
Original file line number Diff line number Diff line change
Expand Up @@ -970,7 +970,7 @@ data UnsignedTx input = UnsignedTx
{ unsignedInputs
:: NonEmpty input
, unsignedOutputs
:: NonEmpty TxOut
:: [TxOut]
}
deriving (Eq, Show)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -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 .
Expand Down
4 changes: 2 additions & 2 deletions specifications/api/swagger.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -2947,7 +2947,7 @@ paths:
Random-Improve coin selection algorithm</a>.
<b>Note: </b> Not supported for Byron random wallets.
parameters:
- *parametersWalletId
requestBody:
Expand Down

0 comments on commit c60c5a6

Please sign in to comment.