Skip to content

Include native assets in estimateBalancedTxBody's declared UTxO value - #1366

Open
carbolymer wants to merge 1 commit into
masterfrom
mgalazyn/fix/build-estimate-multiasset
Open

carbolymer wants to merge 1 commit into
masterfrom
mgalazyn/fix/build-estimate-multiasset

Conversation

@carbolymer

@carbolymer carbolymer commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Context

Follow-up to
#1365
which fixed estimateBalancedTxBody for transactions with no outputs.
Copilot's review on that PR pointed out that the fake UTxO entry used for balance evaluation carried only ADA, so native assets in the declared total UTxO value were dropped.
That limitation predates PR 1365 (it was marked with a TODO: Include multiassets) and also affected transactions that do have outputs: only the tokens present in the first output were represented in the fake entry.

createFakeUTxO in both Cardano.Api.Tx.Internal.Fee and Cardano.Api.Experimental.Tx.Internal.Fee now builds the single fake entry at the change address holding the full available value, ADA and native assets together.
The first-output template is gone, and the now-unused legacy helper updateTxOut was removed.
The public totalUTxOValue Haddock now says the value includes native assets.
cardano-cli transaction build-estimate already parses a multi-asset --total-utxo-value, so it benefits from this fix without any changes on its side.

This is a behaviour change worth calling out: tokens present in outputs but missing from the declared total now fail with a negative balance error, instead of passing by accident when they happened to sit in the first output.

The negative balance check for native assets now runs before the fee-estimation body is built.
Previously, when an explicit output spent tokens that were not on the fake input, the estimate aborted with the ledger's "Illegal Value in TxOut" exception instead of returning an error, because Conway outputs are compacted eagerly and reject negative quantities.

Two more paths that reached the same ledger exception are guarded as well.
checkNonNegative in both modules now rejects negative token quantities before the zero-ADA branch, which computed a minimum UTxO from a still-negative value.
The legacy estimateBalancedTxBody now rejects deposits that exceed the declared ADA with a balance error, as the experimental one already did.

How to trust this PR

Four regression properties cover this:

  • prop_estimateBalancedTxBody_keeps_native_assets_in_change and prop_estimateBalancedTxBody_fails_on_undeclared_native_assets in Test.Cardano.Api.Experimental.Fee
  • prop_estimate_balanced_tx_body_keeps_native_assets_in_change and prop_estimate_balanced_tx_body_fails_on_undeclared_native_assets in Test.Cardano.Api.Transaction.Autobalance

The first property in each pair declares a total UTxO value of 150 ADA plus 10 tokens, spends 10 ADA plus 3 tokens to an explicit output, and asserts the change output carries the remaining ADA minus fee and the remaining 7 tokens.
The second property in each pair declares an ADA-only total but spends tokens, and asserts the result is the negative balance error rather than an exception.

Three further properties cover the guarded paths: prop_make_transaction_body_auto_balance_fails_on_zero_ada_negative_assets and prop_estimate_balanced_tx_body_fails_when_deposits_exceed_declared_ada in Test.Cardano.Api.Transaction.Autobalance, and prop_makeTransactionBodyAutoBalance_fails_on_zero_ada_negative_assets in Test.Cardano.Api.Experimental.Fee.
Each forces the returned error, so a ledger exception would fail the test.

Checklist

  • Commit sequence broadly makes sense and commits have useful messages
  • New tests are added if needed and existing tests are updated. See Running tests for more details
  • Self-reviewed the diff
  • Changelog fragment added in .changes/

🤖 Generated with Claude Code

@carbolymer carbolymer changed the title Mgalazyn/fix/build estimate multiasset Include native assets in estimateBalancedTxBody's declared UTxO value Oct 5, 2026
@carbolymer
carbolymer force-pushed the mgalazyn/fix/build-estimate-multiasset branch from bb6305b to 8b6a97b Compare October 5, 2026 16:18
@carbolymer carbolymer self-assigned this Oct 5, 2026
@carbolymer
carbolymer force-pushed the mgalazyn/fix/build-estimate-multiasset branch 2 times, most recently from 0bb1736 to d8f558d Compare October 6, 2026 09:46
@carbolymer
carbolymer marked this pull request as ready for review October 6, 2026 09:59
Copilot AI balanced review requested due to automatic review settings October 6, 2026 09:59

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The public behavioral change must be classified as breaking in the changelog fragment.

Review effort: Balanced
Findings: 1 Low severity

Open (1)
What changed in this PR

Updates transaction fee estimation to preserve native assets and return balance errors instead of ledger exceptions.

Changes:

  • Builds fake UTxOs using the full declared value.
  • Validates negative ADA and native-asset balances earlier.
  • Adds regression tests and updates documentation/changelog.
File Description
.changes/​estimate-balanced-tx-body-native-assets.yml Documents the behavioral fix.
cardano-api/​src/​Cardano/​Api/​Tx/​Internal/​Fee.hs Fixes legacy balancing and validation.
cardano-api/​src/​Cardano/​Api/​Experimental/​Tx/​Internal/​Fee.hs Fixes experimental balancing and validation.
cardano-api/​test/​cardano-api-test/​Test/​Cardano/​Api/​Transaction/​Autobalance.hs Adds legacy regression coverage.
cardano-api/​test/​cardano-api-test/​Test/​Cardano/​Api/​Experimental/​Fee.hs Adds experimental regression coverage.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

project: cardano-api
pr: 1366
kind:
- bugfix
@carbolymer
carbolymer force-pushed the mgalazyn/fix/build-estimate-multiasset branch from d8f558d to f5849a3 Compare October 6, 2026 10:06

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants