Skip to content

Commit 6b6b409

Browse files
committed
cardano-testnet | Test transaction autobalancing with withdrawal
1 parent 6c0373b commit 6b6b409

10 files changed

Lines changed: 1664 additions & 5 deletions

File tree

WithdrawalReward-analysis.md

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
# Test analysis: `WithdrawalReward.hs`
2+
3+
Regression test for [cardano-cli#965](https://github.com/IntersectMBO/cardano-cli/issues/965).
4+
Transaction auto-balance crashes with "Illegal Value in TxOut" when withdrawals are present and the change output computation produces a negative value.
5+
6+
## Revision history
7+
8+
- **v1**: Initial analysis against the original test (fictional withdrawal amounts, no error message check).
9+
- **v2**: Re-analysis after the test was rewritten to use real on-chain rewards, add an `isInfixOf` assertion, and verify the output file.
10+
Found critical arithmetic bug: `outputLovelace = inputLovelace + rewardLovelace` consumed all available value, leaving nothing for fees.
11+
- **v3**: Re-analysis after the arithmetic fix (`outputLovelace = inputLovelace + 1_000_000`) and reward floor guard.
12+
13+
## Core logic assessment
14+
15+
The test is now logically sound and well-structured.
16+
17+
The arithmetic is correct:
18+
19+
```
20+
change = input + withdrawal - output - fee
21+
= inputLovelace + rewardLovelace - (inputLovelace + 1_000_000) - fee
22+
= rewardLovelace - 1_000_000 - fee (positive, guarded by rewardCoin > 1.5 ADA)
23+
```
24+
25+
Without the withdrawal (the bug):
26+
27+
```
28+
change = inputLovelace - (inputLovelace + 1_000_000) - fee
29+
= -1_000_000 - fee (hugely negative, triggers crash)
30+
```
31+
32+
The output exceeds the input by a fixed 1 ADA, forcing the auto-balancer to rely on the withdrawal.
33+
The explicit guard at line 127 (`H.assertWith rewardCoin (> L.Coin 1500000)`) ensures the reward comfortably covers the excess + fee, and fails cleanly with a descriptive message if rewards are too small.
34+
35+
API usage is consistent with the codebase: `getAccountsStates`, `retryUntilJustM`, `H.assertWith`, and `balanceAccountStateL` are all used in existing tests and library code (e.g. `Testnet.Process.Cli.SPO`).
36+
37+
## What the test gets right
38+
39+
- **Real rewards**: queries on-chain reward balance via `getAccountsStates` and `retryUntilJustM`, making the test robust against future CLI withdrawal-amount validation.
40+
- **Specific error assertion**: line 164 checks `not . isInfixOf "Illegal Value in TxOut"` on stderr, distinguishing the regression crash from other failures.
41+
- **ExitSuccess assertion**: line 166 verifies the command succeeds end-to-end.
42+
- **Reward floor guard**: line 127 makes the minimum-reward assumption explicit and fails cleanly if not met.
43+
- **Output file verification**: line 169 reads back the transaction body file.
44+
- **Clear comments**: lines 136-143 explain the arithmetic for both the correct and buggy cases.
45+
46+
## Remaining holes
47+
48+
### 1. Potential flakiness if rewards accumulate slowly
49+
50+
The test waits up to 10 epochs (`WaitForEpochs $ L.EpochInterval 10`) and then asserts `rewardCoin > L.Coin 1500000`.
51+
Whether rewards reach 1.5 ADA in time depends on the testnet's monetary expansion parameters (`rho`, `tau`) and the delegator's stake proportion.
52+
If the default genesis configuration yields slow rewards, the test could time out or fail at the guard.
53+
54+
This is a clean failure (not a false positive), and the guard gives a descriptive message.
55+
But it could make the test unreliable across different configurations or CI environments.
56+
57+
Mitigation: if this proves flaky, either increase the wait period or lower the excess (e.g. to 500_000 lovelace with a guard of `> 1_000_000`).
58+
59+
### 2. No coverage for Plutus script-based withdrawals
60+
61+
KtorZ's reproduction (the most recent confirmation the bug exists, from 2026-04-17) uses `--withdrawal-plutus-script-v3` with `--withdrawal-reference-tx-in-redeemer-value`.
62+
63+
For Plutus script withdrawals, `evaluateTransactionExecutionUnits` calls the node to evaluate the script.
64+
This exercises a different code path through the auto-balancer where `exUnitsMap` is non-empty and `substituteExecutionUnits` modifies `txbodycontent1`.
65+
The current test only covers key-based (non-script) withdrawals, which is the simpler path.
66+
67+
### 3. `checkNonNegative` is dead code for the negative case in the experimental path
68+
69+
The CLI calls `Exp.makeTransactionBodyAutoBalance` (line 1120 of `Run.hs`), the experimental auto-balance path in `Experimental/Tx/Internal/Fee.hs`.
70+
In this path, the initial change output is constructed at lines 1437-1439 as:
71+
72+
```haskell
73+
TxOut (L.mkBasicTxOut (toShelleyAddr changeaddr) initialChangeTxOutValue)
74+
```
75+
76+
Then `checkNonNegative` at line 1443 is supposed to catch negative values.
77+
But `TxOut` is a newtype wrapper, and when `checkNonNegative` forces it (line 477: `balance ^. L.valueTxOutL`), this evaluates `L.mkBasicTxOut`, which calls `L.mkTxOut`, which throws `error "Illegal Value in TxOut: ..."` for negative values before `checkNonNegative` can return `Left TxBodyErrorBalanceNegative`.
78+
79+
`error` throws an uncaught `ErrorCall` exception that the `Either` monad cannot intercept.
80+
So `checkNonNegative` never gets to run its logic for the negative case.
81+
This is a secondary bug in the auto-balance code itself (the check is dead code), though it does not affect whether the test detects the regression.
82+
83+
### 4. Test does not exercise the final balance check
84+
85+
The auto-balancer has two balance checks:
86+
87+
- Initial: line 1443 (`checkNonNegative` on initial change before fee estimation).
88+
- Final: line 1548 (`when (adaBalance < 0) $ Left $ BalanceIsNegative ...` after fee estimation).
89+
90+
The test's arithmetic ensures both checks yield the same sign (`rewardLovelace - 1_000_000 - fee` is positive in both cases).
91+
A complementary test case with a very small input (where `inputLovelace < fee`) would exercise the final balance check independently.
92+
93+
## Summary
94+
95+
| Hole | Severity | Impact |
96+
|------|----------|--------|
97+
| Potential flakiness from reward timing | Low-Medium | Could fail in slow-reward configurations |
98+
| No Plutus script withdrawal case | Medium | KtorZ's actual reproduction path untested |
99+
| `checkNonNegative` is dead code | Low | Secondary bug in auto-balance, not the test |
100+
| Final balance check not exercised | Low | Incomplete coverage of auto-balance code paths |

cardano-testnet/cardano-testnet.cabal

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,7 @@ test-suite cardano-testnet-test
218218
Cardano.Testnet.Test.Cli.Transaction
219219
Cardano.Testnet.Test.Cli.Transaction.BuildEstimate
220220
Cardano.Testnet.Test.Cli.Transaction.RegisterDeregisterStakeAddress
221+
Cardano.Testnet.Test.Cli.Transaction.WithdrawalReward
221222
Cardano.Testnet.Test.DumpConfig
222223
Cardano.Testnet.Test.FoldEpochState
223224
Cardano.Testnet.Test.Gov.CommitteeAddNew
Lines changed: 196 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,196 @@
1+
{-# LANGUAGE NamedFieldPuns #-}
2+
{-# LANGUAGE OverloadedStrings #-}
3+
{-# LANGUAGE ScopedTypeVariables #-}
4+
{-# LANGUAGE TypeApplications #-}
5+
6+
-- | Smoke tests for @transaction build@ with a @--withdrawal@ flag,
7+
-- covering both key-witnessed and Plutus-script-witnessed withdrawals.
8+
-- Loosely related to https://github.com/IntersectMBO/cardano-cli/issues/965.
9+
module Cardano.Testnet.Test.Cli.Transaction.WithdrawalReward
10+
( hprop_tx_withdrawal_reward
11+
, hprop_tx_withdrawal_reward_plutus_v3
12+
) where
13+
14+
import Cardano.Api as Api
15+
import qualified Cardano.Api.Ledger as L
16+
17+
import Cardano.Testnet
18+
19+
import Prelude
20+
21+
import Control.Monad (void)
22+
import Data.Default.Class
23+
import qualified Data.Text as Text
24+
import System.Exit (ExitCode (..))
25+
import System.FilePath ((</>))
26+
27+
import Testnet.Components.Configuration
28+
import Testnet.Components.Query
29+
import Testnet.Defaults (plutusV3Script)
30+
import Testnet.Process.Run (execCli, execCliAny, mkExecConfig)
31+
import Testnet.Property.Util (integrationRetryWorkspace)
32+
import Testnet.Start.Types
33+
import Testnet.Types
34+
35+
import Hedgehog
36+
import qualified Hedgehog as H
37+
import qualified Hedgehog.Extras as H
38+
39+
40+
-- | Smoke test: @transaction build@ builds a transaction whose output
41+
-- exceeds the input, with a @--withdrawal@ covering the shortfall
42+
-- (@output = 2 * input@, @withdrawal = 2 * input@). The auto-balancer
43+
-- sums the withdrawal into consumed value, producing
44+
-- @change = inputs + withdrawal - outputs - fee = input - fee@ (positive).
45+
--
46+
-- The withdrawal amount is fictional; the CLI does not validate it against
47+
-- on-chain reward balances at build time, and the test only exercises the
48+
-- build step.
49+
--
50+
-- Execute me with:
51+
-- @DISABLE_RETRIES=1 cabal test cardano-testnet-test --test-options '-p "/transaction build with withdrawal/"'@
52+
hprop_tx_withdrawal_reward :: Property
53+
hprop_tx_withdrawal_reward = integrationRetryWorkspace 2 "tx-withdrawal-reward" $ \tempAbsBasePath' -> H.runWithDefaultWatchdog_ $ do
54+
conf@Conf { tempAbsPath } <- mkConf tempAbsBasePath'
55+
let tempAbsPath' = unTmpAbsPath tempAbsPath
56+
tempBaseAbsPath = makeTmpBaseAbsPath tempAbsPath
57+
58+
work <- H.createDirectoryIfMissing $ tempAbsPath' </> "work"
59+
60+
let ceo = ConwayEraOnwardsConway
61+
sbe = convert ceo
62+
eraName = eraToString sbe
63+
fastTestnetOptions = def { cardanoNodeEra = AnyShelleyBasedEra sbe }
64+
shelleyOptions = def { genesisEpochLength = 100 }
65+
66+
TestnetRuntime
67+
{ testnetMagic
68+
, testnetNodes
69+
, wallets = wallet0:_
70+
, configurationFile
71+
}
72+
<- createAndRunTestnet fastTestnetOptions shelleyOptions conf
73+
74+
node <- H.headM testnetNodes
75+
poolSprocket1 <- H.noteShow $ nodeSprocket node
76+
execConfig <- mkExecConfig tempBaseAbsPath poolSprocket1 testnetMagic
77+
let socketPath = nodeSocketPath node
78+
79+
epochStateView <- getEpochStateView configurationFile socketPath
80+
81+
let delegatorStakeVKeyFp = tempAbsPath' </> "stake-delegators" </> "delegator1" </> "staking.vkey"
82+
83+
delegatorStakeAddress <- filter (/= '\n') <$>
84+
execCli
85+
[ "latest", "stake-address", "build"
86+
, "--stake-verification-key-file", delegatorStakeVKeyFp
87+
, "--testnet-magic", show @Int testnetMagic
88+
]
89+
H.note_ $ "Delegator stake address: " <> delegatorStakeAddress
90+
91+
(txinForWithdrawal, TxOut _ txinValue _ _) <- H.nothingFailM $
92+
findLargestUtxoWithAddress epochStateView sbe $ paymentKeyInfoAddr wallet0
93+
let L.Coin inputLovelace = txOutValueToLovelace txinValue
94+
H.note_ $ "Input UTxO: " <> show txinForWithdrawal <> " = " <> show inputLovelace <> " lovelace"
95+
96+
let withdrawalLovelace = inputLovelace * 2
97+
outputLovelace = withdrawalLovelace
98+
99+
let withdrawalTxBodyFp = work </> "withdrawal.txbody"
100+
101+
(exitCode, _stdout, stderr) <- execCliAny execConfig
102+
[ eraName, "transaction", "build"
103+
, "--change-address", Text.unpack $ paymentKeyInfoAddr wallet0
104+
, "--tx-in", Text.unpack $ renderTxIn txinForWithdrawal
105+
, "--tx-out", Text.unpack (paymentKeyInfoAddr wallet0) <> "+" <> show outputLovelace
106+
, "--withdrawal", delegatorStakeAddress <> "+" <> show withdrawalLovelace
107+
, "--witness-override", show @Int 2
108+
, "--out-file", withdrawalTxBodyFp
109+
]
110+
H.note_ stderr
111+
112+
exitCode H.=== ExitSuccess
113+
114+
-- Verify the transaction body was written and can be read back
115+
void $ H.readFile withdrawalTxBodyFp
116+
117+
-- | Plutus-script-witnessed withdrawal variant of 'hprop_tx_withdrawal_reward'.
118+
-- A Plutus V3 always-succeeds script acts as the stake credential, exercising
119+
-- the script-witness branch of the auto-balancer. Same balance arithmetic
120+
-- (output exceeds input, withdrawal covers the difference), and the
121+
-- withdrawal amount is likewise fictional.
122+
--
123+
-- Execute me with:
124+
-- @DISABLE_RETRIES=1 cabal test cardano-testnet-test --test-options '-p "/transaction build with plutus withdrawal/"'@
125+
hprop_tx_withdrawal_reward_plutus_v3 :: Property
126+
hprop_tx_withdrawal_reward_plutus_v3 = integrationRetryWorkspace 2 "tx-withdrawal-reward-plutus-v3" $ \tempAbsBasePath' -> H.runWithDefaultWatchdog_ $ do
127+
conf@Conf { tempAbsPath } <- mkConf tempAbsBasePath'
128+
let tempAbsPath' = unTmpAbsPath tempAbsPath
129+
tempBaseAbsPath = makeTmpBaseAbsPath tempAbsPath
130+
131+
work <- H.createDirectoryIfMissing $ tempAbsPath' </> "work"
132+
133+
let ceo = ConwayEraOnwardsConway
134+
sbe = convert ceo
135+
eraName = eraToString sbe
136+
fastTestnetOptions = def { cardanoNodeEra = AnyShelleyBasedEra sbe }
137+
shelleyOptions = def { genesisEpochLength = 100 }
138+
139+
TestnetRuntime
140+
{ testnetMagic
141+
, testnetNodes
142+
, wallets = wallet0:wallet1:_
143+
, configurationFile
144+
}
145+
<- createAndRunTestnet fastTestnetOptions shelleyOptions conf
146+
147+
node <- H.headM testnetNodes
148+
poolSprocket1 <- H.noteShow $ nodeSprocket node
149+
execConfig <- mkExecConfig tempBaseAbsPath poolSprocket1 testnetMagic
150+
let socketPath = nodeSocketPath node
151+
152+
epochStateView <- getEpochStateView configurationFile socketPath
153+
154+
-- Write the always-succeeds Plutus V3 script used as stake credential
155+
plutusScriptFp <- H.note $ work </> "always-succeeds.plutusV3"
156+
H.writeFile plutusScriptFp $ Text.unpack plutusV3Script
157+
158+
-- Build the bech32 stake address for the script credential
159+
scriptStakeAddr <- filter (/= '\n') <$> execCli
160+
[ eraName, "stake-address", "build"
161+
, "--stake-script-file", plutusScriptFp
162+
, "--testnet-magic", show @Int testnetMagic
163+
]
164+
H.note_ $ "Script stake address: " <> scriptStakeAddr
165+
166+
(txinForWithdrawal, TxOut _ txinValue _ _) <- H.nothingFailM $
167+
findLargestUtxoWithAddress epochStateView sbe $ paymentKeyInfoAddr wallet0
168+
let L.Coin inputLovelace = txOutValueToLovelace txinValue
169+
H.note_ $ "Input UTxO: " <> show txinForWithdrawal <> " = " <> show inputLovelace <> " lovelace"
170+
171+
-- Plutus scripts require collateral
172+
txinCollateral <- findLargestUtxoForPaymentKey epochStateView sbe wallet1
173+
174+
let withdrawalLovelace = inputLovelace * 2
175+
outputLovelace = withdrawalLovelace
176+
177+
let withdrawalTxBodyFp = work </> "withdrawal.txbody"
178+
179+
(exitCode, _stdout, stderr) <- execCliAny execConfig
180+
[ eraName, "transaction", "build"
181+
, "--change-address", Text.unpack $ paymentKeyInfoAddr wallet0
182+
, "--tx-in-collateral", Text.unpack $ renderTxIn txinCollateral
183+
, "--tx-in", Text.unpack $ renderTxIn txinForWithdrawal
184+
, "--tx-out", Text.unpack (paymentKeyInfoAddr wallet0) <> "+" <> show outputLovelace
185+
, "--withdrawal", scriptStakeAddr <> "+" <> show withdrawalLovelace
186+
, "--withdrawal-script-file", plutusScriptFp
187+
, "--withdrawal-redeemer-value", "0"
188+
, "--witness-override", show @Int 2
189+
, "--out-file", withdrawalTxBodyFp
190+
]
191+
H.note_ stderr
192+
193+
exitCode H.=== ExitSuccess
194+
195+
-- Verify the transaction body was written and can be read back
196+
void $ H.readFile withdrawalTxBodyFp

cardano-testnet/test/cardano-testnet-test/cardano-testnet-test.hs

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,17 @@ import qualified Cardano.Testnet.Test.Api.TxReferenceInputDatum
99
import qualified Cardano.Testnet.Test.Cli.KesPeriodInfo
1010
import qualified Cardano.Testnet.Test.Cli.Plutus.BuildRaw
1111
import qualified Cardano.Testnet.Test.Cli.Plutus.CostCalculation
12+
import qualified Cardano.Testnet.Test.Cli.Plutus.MultiAssetReturnCollateral
1213
import qualified Cardano.Testnet.Test.Cli.Plutus.Scripts
1314
import qualified Cardano.Testnet.Test.Cli.Query
1415
import qualified Cardano.Testnet.Test.Cli.QuerySlotNumber
16+
import qualified Cardano.Testnet.Test.Cli.Scripts.Simple.CostCalculation
17+
import qualified Cardano.Testnet.Test.Cli.Scripts.Simple.Mint
1518
import qualified Cardano.Testnet.Test.Cli.StakeSnapshot
1619
import qualified Cardano.Testnet.Test.Cli.Transaction
20+
import qualified Cardano.Testnet.Test.Cli.Transaction.BuildEstimate
1721
import qualified Cardano.Testnet.Test.Cli.Transaction.RegisterDeregisterStakeAddress
22+
import qualified Cardano.Testnet.Test.Cli.Transaction.WithdrawalReward
1823
import qualified Cardano.Testnet.Test.DumpConfig
1924
import qualified Cardano.Testnet.Test.FoldEpochState
2025
import qualified Cardano.Testnet.Test.Gov.CommitteeAddNew as Gov
@@ -23,7 +28,6 @@ import qualified Cardano.Testnet.Test.Gov.DRepRetirement as Gov
2328
import qualified Cardano.Testnet.Test.Gov.GovActionTimeout as Gov
2429
import qualified Cardano.Testnet.Test.Gov.InfoAction as LedgerEvents
2530
import qualified Cardano.Testnet.Test.Gov.PParamChangeFailsSPO as Gov
26-
import qualified Cardano.Testnet.Test.Cli.Scripts.Simple.Mint
2731
import qualified Cardano.Testnet.Test.Gov.ProposeNewConstitution as Gov
2832
import qualified Cardano.Testnet.Test.Gov.Transaction.HashMismatch as WrongHash
2933
import qualified Cardano.Testnet.Test.Gov.TreasuryDonation as Gov
@@ -37,9 +41,6 @@ import qualified Cardano.Testnet.Test.SanityCheck
3741
import qualified Cardano.Testnet.Test.SanityCheck as LedgerEvents
3842
import qualified Cardano.Testnet.Test.SubmitApi.Transaction
3943
import qualified Cardano.Testnet.Test.UpdateTimeStamps
40-
import qualified Cardano.Testnet.Test.Cli.Scripts.Simple.CostCalculation
41-
import qualified Cardano.Testnet.Test.Cli.Transaction.BuildEstimate
42-
import qualified Cardano.Testnet.Test.Cli.Plutus.MultiAssetReturnCollateral
4344

4445
import Prelude
4546

@@ -89,7 +90,7 @@ tests = do
8990
, T.testGroup "Simple Script"
9091
[ ignoreOnWindows "Simple Script Mint" Cardano.Testnet.Test.Cli.Scripts.Simple.Mint.hprop_simple_script_mint
9192
, ignoreOnWindows "Simple Reference Script Mint" Cardano.Testnet.Test.Cli.Scripts.Simple.CostCalculation.hprop_ref_simple_script_mint
92-
93+
9394
]
9495
, T.testGroup "Plutus"
9596
[ ignoreOnWindows "PlutusV3 purposes" Cardano.Testnet.Test.Cli.Plutus.Scripts.hprop_plutus_purposes_v3
@@ -115,6 +116,8 @@ tests = do
115116
, ignoreOnWindows "simple transaction build" Cardano.Testnet.Test.Cli.Transaction.hprop_transaction
116117
, ignoreOnWindows "Transaction Build Estimate" Cardano.Testnet.Test.Cli.Transaction.BuildEstimate.hprop_tx_build_estimate
117118
, ignoreOnWindows "register deregister stake address in transaction build" Cardano.Testnet.Test.Cli.Transaction.RegisterDeregisterStakeAddress.hprop_tx_register_deregister_stake_address
119+
, ignoreOnWindows "transaction build with withdrawal" Cardano.Testnet.Test.Cli.Transaction.WithdrawalReward.hprop_tx_withdrawal_reward
120+
, ignoreOnWindows "transaction build with plutus withdrawal" Cardano.Testnet.Test.Cli.Transaction.WithdrawalReward.hprop_tx_withdrawal_reward_plutus_v3
118121
-- FIXME
119122
-- , ignoreOnMacAndWindows "leadership-schedule" Cardano.Testnet.Test.Cli.LeadershipSchedule.hprop_leadershipSchedule
120123

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
[
2+
{
3+
"executionUnits": {
4+
"memory": 500,
5+
"steps": 64100
6+
},
7+
"lovelaceCost": 34,
8+
"scriptHash": "186e32faa80a26810392fda6d559c7ed4721a65ce1c9d4ef3e1c87b4"
9+
}
10+
]
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
[]

0 commit comments

Comments
 (0)