Fix mapScriptWitnessesCertificates silently dropping key-witnessed certs#1136
Merged
Fix mapScriptWitnessesCertificates silently dropping key-witnessed certs#1136
Conversation
Jimbo4350
requested review from
CarlosLopezDeLara,
carbolymer,
dcoutts,
disassembler,
erikd,
kevinhammond,
newhoggy and
palas
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a bug in experimental transaction fee balancing where rebuilding TxCertificates during exec-unit substitution could silently drop certificates that had no script witness (Nothing), causing key-witnessed / witnessless certs (e.g., some stake registration-style certs) to disappear from the balanced transaction body.
Changes:
- Update
mapScriptWitnessesCertificatesto iterate over all certificates in theTxCertificatesOMap, passing throughNothingwitnesses unchanged. - Retain exec-unit substitution behavior for witnessed certificates via
substituteExecUnits.
Comment on lines
+1026
to
+1030
| -- We iterate over all certs, not just the script-witnessed ones from | ||
| -- indexTxCertificates. TxCertificates holds certs both with and without | ||
| -- script witnesses; certs without a witness (Nothing) must pass through | ||
| -- unchanged. Iterating only over the indexed subset would silently drop | ||
| -- all key-witnessed certs from the rebuilt TxCertificates. |
Comment on lines
+1010
to
+1014
| let witnessLookup = | ||
| Map.fromList | ||
| [ (cert, (ix, stakeCred, wit)) | ||
| | (ix, cert, stakeCred, wit) <- indexTxCertificates txCertificates' | ||
| ] |
| -> Either (TxBodyErrorAutoBalance (LedgerEra era)) (TxCertificates (LedgerEra era)) | ||
| mapScriptWitnessesCertificates txCertificates' = do | ||
| let mappedScriptWitnesses | ||
| mapScriptWitnessesCertificates txCertificates'@(TxCertificates certsMap) = do |
Previously, the function iterated only over script-witnessed certs (via indexTxCertificates), so certs with no witness (Nothing) were silently omitted from the rebuilt TxCertificates. Now all certs are traversed; those without a script witness are preserved as-is with Right Nothing.
Jimbo4350
force-pushed
the
fix/cert-witnesses-drop-none
branch
from
8bfb09a to
a53807d
Compare
Test that both Nothing-witnessed and Just-witnessed certificates in TxCertificates survive mapScriptWitnessesCertificates unchanged when substituteExecutionUnits is called with an empty execution unit map. Also exports substituteExecutionUnits from Cardano.Api.Experimental.
palas
approved these changes
Mar 18, 2026
Comment on lines
+529
to
+534
| inputCerts = | ||
| Exp.mkTxCertificates | ||
| Exp.ConwayEra | ||
| [ (regCert, Exp.AnyKeyWitnessPlaceholder) | ||
| , (unRegCert, Exp.AnyKeyWitnessPlaceholder) | ||
| ] |
Contributor
There was a problem hiding this comment.
Maybe it would be more interesting to randomise the cert list too, in case there is a bug where they only get lost on a certain order, for example
Contributor
Author
There was a problem hiding this comment.
Will do so in a follow up PR. Want to get this in before the 10.7 integration.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changelog
Context
During auto-balance,
mapScriptWitnessesCertificatescalledindexTxCertificates(which filters to script-witnessed certs only) and reconstructed aTxCertificatesfrom that subset. Any cert stored with no script witness —(cert, Nothing)in theOMap— was silently dropped, so it never appeared in the final transaction body.Fix: iterate over all certs in the
OMap. Certs without a script witness (Nothing) pass through asRight Nothing; script-witnessed certs go throughsubstituteExecUnitsas before.Fixes the bug reported in IntersectMBO/cardano-cli#1347 where a shelley-era stake registration certificate was silently omitted from a Conway transaction built with
cardano-cli conway transaction build.Checklist