Fix supplemental datum propagation in transaction build#1341
Fix supplemental datum propagation in transaction build#1341
Conversation
Jimbo4350
requested review from
carbolymer,
disassembler,
kevinhammond,
newhoggy and
palas
as code owners
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where supplemental datums (datum hashes referenced in transaction outputs) were not being propagated into the transaction body during transaction construction. The fix collects datums from both transaction outputs and return collateral, then passes them to the transaction body via Exp.setTxSupplementalDatums.
Changes:
- Modified
toTxOutInEraandtoTxOutInShelleyBasedErato return supplemental datums alongside theTxOut - Applied the fix to all three transaction build commands:
build,build-estimate, andbuild-raw - Pinned
cardano-apito a specific commit that includes thesetTxSupplementalDatumsAPI
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| cardano-cli/test/cardano-cli-golden/files/golden/shelley/build-raw-tx-body-out-6.json | Updated golden test CBOR encoding to reflect new supplemental datum inclusion |
| cardano-cli/src/Cardano/CLI/EraBased/Transaction/Run.hs | Core implementation: collects supplemental datums from outputs/collateral and passes them through transaction body construction |
| cabal.project | Pins cardano-api to specific commit with required API support |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import Cardano.Api.Experimental.AnyScriptWitness qualified as Exp | ||
| import Cardano.Api.Experimental.Tx qualified as Exp | ||
| import Cardano.Api.Ledger qualified as L | ||
| import Cardano.Ledger.Hashes (DataHash) |
There was a problem hiding this comment.
The new import for DataHash is inserted between two qualified imports with different prefixes (Cardano.Api.Experimental.Tx as Exp and Cardano.Api.Network as Consensus), breaking the grouping pattern. Consider placing this import with other Cardano.Ledger imports or in a separate unqualified import group to maintain consistency with the existing import organization.
| fromExceptTCli @ProtocolParamsError | ||
| (obtainCommonConstraints era $ readProtocolParameters protocolParamsFile) | ||
| out <- obtainCommonConstraints era $ toTxOutInShelleyBasedEra txOut | ||
| (out, _suppDatums :: Map.Map DataHash (L.Data (Exp.LedgerEra era))) <- |
There was a problem hiding this comment.
The variable name _suppDatums uses an underscore prefix typically reserved for intentionally unused variables in Haskell, but includes an explicit type annotation. If the datums are truly unused here, remove the type annotation. If they might be used in the future, use a name without the underscore prefix like suppDatums.
| (out, _suppDatums :: Map.Map DataHash (L.Data (Exp.LedgerEra era))) <- | |
| (out, _suppDatums) <- |
|
Your PR is not using description template |
Summary
Exp.setTxSupplementalDatumstoTxOutInEraandtoTxOutInShelleyBasedErato return supplemental datums alongside theTxOutbuild,build-estimate, andbuild-rawcardano-apito a commit that includes thesetTxSupplementalDatumsAPIContext
Previously, supplemental datums (datum hashes referenced in transaction outputs) were not being propagated into the transaction body. This meant that when a transaction output included a datum hash, the corresponding datum was not included in the transaction's supplemental data map, causing validation failures for consumers expecting to resolve those datum hashes.
Test plan
build-raw-tx-body-out-6.jsonto reflect the new CBOR encoding with supplemental datums