Add extraCoin{In,Out} to SelectionParams

[Anviking]

Overview

• Add extraCoin{In,Out} fields to SelectionParams
  • Not currently used, but with the goal of in a future PR constructing SelectionParams from a balance :: Cardano.Value and fee of a partial tx to be balanced.

Comments

• These are the only Primitive.CoinSelection{,.*} changes I anticipate are needed for ADP-1372
• This should be the least invasive change to support a evaluateTransactionBalance-based coin-selection, and allow the existing TransactionCtx approach to co-exist for the time being.
• I'd be happy to discuss the direction of this in a call

---

To re-write balanceTransaction using evaluateTransactionBalance, we need
a way to pass the resulting balance to coin-selection.

The existing tokensTo{Mint, Burn} can be used for non-ada assets. For
ada, we need two new fields: extraCoin{In, Out}. Two fields are needed
because Coin cannot be negative. The extraCoin name was chosen to
avoid conflicts with the more internal extraCoin{Source,Sink} terms.

Then we should be able to construct selection params like these:

let ( TokenBundle.TokenBundle positiveAda positiveTokens
    , TokenBundle.TokenBundle negativeAda negativeTokens
    ) = posAndNegFromCardanoValue balance

let selectionParams = SelectionParams
          { assetsToMint = positiveTokens balance
          , assetsToBurn = negativeTokens balance
          , extraCoinIn = positiveAda <> adaInPreselectedOutputs
          , extraCoinOut = negativeAda <> adaInPreselectedInputs
            -- Coin selection will try to count the preselected inputs
            -- and outputs towards the balance. We need to counteract
            -- this by subtracting the inputs and adding the outputs.

        -- The following fields are already taken into account in the
        -- balance, and are set to zero.
        , rewardWithdrawal = Coin 0
        , certificateDepositsReturned = 0
        , certificateDepositsTaken = 0
        , ...

Issue Number

ADP-1372

[Anviking]

@jonathanknowles here are some examples regarding what SelectionParams values to to use from balanceTransaction:

I had forgotten about some details (tokensInPreselected{Inputs, Outputs}), so it was good that you asked! (but property tests should also have caught it as well in the end)

• ⚠️ assuming no fees
• ⚠️ this comment is more for intuition than for specifying all details of the upcoming balanceTransaction re-write with 100% accuracy
• Using the make-shift syntax y = x (=> 2) to mean "y = x, and x is here 2"

Example 1

balanceTransaction is called with the following partialTx:

partialTx = 
  input0 -- 10 ada
  input1 -- 10 ada
  stake key de-reg certificate -- 2 ada

We begin by evaluating the balance of the partialTx:

(balance0, minfee0) <- ExceptT $ pure $ balanceAfterSettingMinFee partialTx 
-- (22, 0) -- assuming no fees for simplicity

Because it's not 0 we need to run CoinSelection.performSelection to select inputs/and or generate change.

We use the following SelectionParams based upon the balance:

let selectionParams = SelectionParams
          { assetsToMint = positiveTokens balance <> tokensInPreselectedOutputs -- all empty
          , assetsToBurn = negativeTokens balance <> tokensInPreselectedInputs -- all empty
          , extraCoinIn = positiveAda (=> 22) <> adaInPreselectedOutputs (=> 0)
          , extraCoinOut = negativeAda (=> 0) <> adaInPreselectedInputs (=> 20)
            -- Coin selection will try to count the preselected inputs
            -- and outputs towards the balance. We need to counteract
            -- this by subtracting the inputs and adding the outputs.
        , rewardWithdrawal = Coin 0
        , certificateDepositsReturned = 0 -- The 0 is intentional! The refund is already included in balance0.
        , certificateDepositsTaken = 0
        , outputsToCover = extractOutputsFromTx partialTx
        , ...

(see the final section of this comment regarding why we are adding/subtracting adaInPreselected{Inputs,Outputs})

We get a selection (extraInputs, extraOutputs, extraCollateral) and we append it to the partialTx.

Now tx should have a balance of a small amount of ada (because coin-selection does slighty overestimations). We can try
to assign the minimum needed amount to the fee, and add the rest to a change output, making the tx perfectly balanced.

Note: because the minfee can change when changing change output coin values, this needs some care.

Example 2

partialTx = 
  input0 # 10 ada + 1 apple
  output0 # 20 ada + 1 orange
  mint 1 apple
  burn 1 banana

(balance0, minfee0) <- ExceptT $ pure $ balanceAfterSettingMinFee partialTx 
-- (-10 ada, -1 banana, 2 apple, -1 orange, 0)

let selectionParams = SelectionParams
          { assetsToMint = positiveTokens balance (=> 2 apple)
                   <> tokensInPreselectedOutputs (=> 1 orange)
          , assetsToBurn = 
               negativeTokens balance (=> 1 banana, 1 orange) 
                   <> tokensInPreselectedInputs (=> 1 apple)
          , extraCoinIn = positiveAda (=> 0) <> adaInPreselectedOutputs (=> 20)
          , extraCoinOut = negativeAda (=> 10) <> adaInPreselectedInputs (=> 10)
            -- Coin selection will try to count the preselected inputs
            -- and outputs towards the balance. We need to counteract
            -- this by subtracting the inputs and adding the outputs.
        , rewardWithdrawal = Coin 0
        , certificateDepositsReturned = 0
        , certificateDepositsTaken = 0
        , outputsToCover = extractOutputsFromTx partialTx
        , ...

Coin-selection will use the four "extra" fields at the top of SelectionParams as well as the inputs and outputs to determine the balance of the partial tx.

extra = 2 apple + 1 orange - 1 banana - 1 orange - 1 apple + 20 ada - 10 ada - 10 ada
inputOutputBalance = -10 ada +  1 apple - 1 orange

balance according to coin selection = extra + inputOutputBalance =
   20 ada - 10 ada - 10 ada - 10 ada 
   + 2 apple - 1 apple + 1 apple 
   + 1 orange - 1 orange - 1 orange
   + 1 banana 
  
= -10 ada + 2 apple - 1 orange + 1 banana

I.e. coin-selection has the same notion of balance as the actual balance of the partialTx, which should allow it to properly select inputs and generate change 🎉

Why and how are we setting the extraCoin{In,Out}, assetsTo{Mint,Burn} fields?

We are passing the partialTx's inputs and outputs to CoinSelection via outputsToCover and utxoAvailableForInputs.

Looking at the following code:

https://github.com/input-output-hk/cardano-wallet/blob/master/lib/core/src/Cardano/Wallet/Primitive/CoinSelection/Balance.hs#L566-L596

We see that coin selection wants:

  tokensToMint 
+ extraCoinSource 
+ sum inputs
- tokensToBurn
- extraCoinSink
- sum outputsCovered
- sum changeGenerated

to have an ada-only surplus. If we call the surplus fee we have:

  tokensToMint 
+ extraCoinSource 
+ sum inputs
- tokensToBurn
- extraCoinSink
- sum outputsCovered
- sum changeGenerated
- fee
= 0

Let's extract (tokensToMint + extraCoinSource - tokensToBurn - extraCoinSink), and call it extra:

  extra
+ sum inputs
- sum outputsCovered
- sum changeGenerated
- fee
= 0

Now, let's redefine
inputs = predefinedInputs + selectedInputs

  extra
+ sum predefinedInputs 
+ sum selectedInputs
- sum outputsCovered
- sum changeGenerated
- fee
= 0                         (1)

When balancing a transaction the balance should be modified according to:

balance = balance0 - sum changeGenerated + selectedInputs (2)

We want to choose extra such that (1) and (2) holds with balance = 0. Combining this we obtain:

  extra
+ sum predefinedInputs 
+ sum selectedInputs
- sum outputsCovered
- sum changeGenerated
- fee
= balance0 - sum generatedChange + sum selectedInputs

extra
+ sum predefinedInputs 
- sum outputsCovered
- fee
= balance0

And finally:

extra = balance0 - sum predefinedInputs + sum outputsCovered + fee            (3)

I.e. to make coin selection balance any transaction with balance0, we need to set the SelectionParams
according to (3).

[jonathanknowles]

Hi @Anviking

Happy to approve this, as it does not alter the internal consistency of the CoinSelection module.

My main concern is that the fields extraCoinIn and extraCoinOut are at a lower level than other fields such as certificateDepositsTaken, which are quite descriptive.

The original intent was to provide an API that didn't require much thought from the caller: the caller can specify things like certificates and reward withdrawals without much thought, and then rely on coin selection to "do the right thing". This PR takes it in a direction where we offset more responsibility to the caller than before.

However, this doesn't seem completely unreasonable. We may eventually wish to take the internal coin selection API in a direction where these higher level fields are all replaced by some form of "extra value in" and "extra value out", and place the responsibility for mapping higher level concepts onto lower level values onto callers.

So happy to approve for now.

We can always continue to revise this API as we go forward into future.

[Anviking]

bors r+

[iohk-bors]

Build failed:

• buildkite/cardano-wallet

src/Test/Integration/Framework/DSL.hs:2446:7:
--
  | 1) API Specifications, NEW_SHELLEY_TRANSACTIONS, Plutus scenarios, ping-pong
  | expected a successful response but got an error: ClientError (Object (fromList [("code",String "missing_witnesses_in_transaction"),("message",String "The transaction has 2 inputs and 1 witnesses included. Submit fully-signed transaction.")]))
  | While verifying value:
  | ( Status
  | { statusCode = 403
  | , statusMessage = "Forbidden"
  | }
  | , Left
  | ( ClientError
  | ( Object
  | ( fromList
  | [
  | ( "code"
  | , String "missing_witnesses_in_transaction"
  | )
  | ,
  | ( "message"
  | , String "The transaction has 2 inputs and 1 witnesses included. Submit fully-signed transaction."
  | )
  | ]
  | )
  | )
  | )
  | )
  |  
  | To rerun use: --match "/API Specifications/NEW_SHELLEY_TRANSACTIONS/Plutus scenarios/ping-pong/"
  |  
  | Randomized with seed 1893593444

Interesting / slightly alarming...

#3122

[Anviking]

bors r+

[iohk-bors]

Build failed:

• buildkite/cardano-wallet

Encountered error while migrating Pantry database:
--
  | SQLite3 returned ErrorNotFound while attempting to perform prepare "SELECT sql FROM sqlite_master WHERE type='table' AND name=?": database disk image is malformed
  | Please report this on https://github.com/commercialhaskell/stack/issues
  | As a workaround you may delete Pantry database in /build/cardano-wallet.stack/pantry/pantry.sqlite3 triggering its recreation.
  | error: Command exited with code 1!

#3120

[Anviking]

bors r+

[iohk-bors]

Build succeeded:

• ci/hydra-build:required
• buildkite/cardano-wallet