Negative values are not checked when building tx and ledger exception is returned

[mkoura]

Description

When building a tx with negative value in txout, we used to have a user friendly error message like "Negative quantity..." returned by cardano-cli. Now (in cli budled with node 9.2.0) we get a ledger exception like

cardano-cli: Illegal Value in TxOut: MaryValue (Coin (-1)) (MultiAsset (fromList []))
CallStack (from HasCallStack):
  error, called at src/Cardano/Ledger/Babbage/TxOut.hs:401:25 in cardano-ledger-babbage-1.9.0.0-HU2WvbcuCJvAecg4qks84x:Cardano.Ledger.Babbage.TxOut
  mkTxOut, called at src/Cardano/Ledger/Babbage/TxOut.hs:372:44 in cardano-ledger-babbage-1.9.0.0-HU2WvbcuCJvAecg4qks84x:Cardano.Ledger.Babbage.TxOut
  BabbageTxOut, called at src/Cardano/Ledger/Conway/TxOut.hs:40:26 in cardano-ledger-conway-1.16.1.0-8pKv0OtXssQEEfAAG2pHEP:Cardano.Ledger.Conway.TxOut
  mkBasicTxOut, called at internal/Cardano/Api/Tx/Body.hs:3084:13 in cardano-api-9.3.0.0-P2Vcj3w08j9AcD72DWi0Z-internal:Cardano.Api.Tx.Body
  toShelleyTxOutAny, called at internal/Cardano/Api/Tx/Body.hs:2339:41 in cardano-api-9.3.0.0-P2Vcj3w08j9AcD72DWi0Z-internal:Cardano.Api.Tx.Body

Steps to Reproduce

1. try build a tx

   cardano-cli conway transaction build-raw --fee 200000 --tx-in "e25450233e4bedd00c8bda15c48c2d4018223bd88271e194052294c4e5be7d55#0" --tx-out "addr_test1vqfxq2s8yce3tuhjq9ulu2awuk623hzvtft9z8fh6qelzts49vuqw+-1" --tx-out "addr_test1vq7q25klctxxpz9x07qjx3m4qpt640a4hateh0ncyaqt4fqw2hcs3+2000000" --out-file test_transfer_negative_amount.body

[smelc]

@CarlosLopezDeLara and I tried the oldest release version of cardano-cli we easily have access to, it's 8.21 from April 2024, and indeed the error message was better:

Command failed: transaction build-raw  Error: Transaction validaton error: Negative quantity (-1) in transaction output: TxOutInAnyEra ConwayEra (TxOut (AddressInEra (ShelleyAddressInEra ShelleyBasedEraConway) (ShelleyAddress Testnet (KeyHashObj (KeyHash "12602a07263315f2f20179fe2baee5b4a8dc4c5a56511d37d033f12e")) StakeRefNull)) (TxOutValueShelleyBased ShelleyBasedEraConway (MaryValue (Coin (-1)) (MultiAsset (fromList [])))) TxOutDatumNone ReferenceScriptNone)

At this time, we were using cardano-api 8.42.0.0.

[smelc]

Alright, here's how it was working in 8.21:

1. build-raw was calling API's createAndValidateTransactionBody
2. createAndValidateTransactionBody was calling makeShelleyTransactionBody, that calls validateTxBodyContent
3. validateTxBodyContent is doing the positive verification by calling validateTxOuts.

[smelc]

Now, the situation is as follows:

1. build-raw calls API's createTransactionBody.
2. createTransactionBody doesn't do validation of transaction outputs.

And actually @palas caught the issue: https://github.com/IntersectMBO/cardano-cli/pull/853#discussion_r1746086547 :upside_down_face:

So this shows that this issue started appearing in cardano-cli 9.4.0.0

[smelc]

https://github.com/IntersectMBO/cardano-cli/commits/smelc/forbid-negative-transaction-output/ shows a possible solution:

1. Pass the ShelleyBasedEra era witness to parseTxOutAnyEra
2. Within parseTxOutAnyEra, do some validation (here) by calling validateTxOuts from API

The problem with that is validateTxOuts and friends require the TxOut _ _ value to be available already. That's because validateTxOuts can return errors like TxBodyOutputNegative that print the entire TxOut when being rendered.

I think requiring the entire TxOut to be there already when raising error is bad practice, because you aren't supposed to have the big thing ready when validating the small thing. So one way forward would be to change validateTxOuts to only operate on the things they actually require, so that we can call them during parseTxOutAnyEra.

[edit] Or directly parse a TxOutValue (even better?)

[github-actions[bot]]

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 120 days.