Skip to content

Commit 38fa4b2

Browse files
committed
TODROP: Fix redeemer pointer indexing in the experimental witness machinery
Proposal redeemer pointers were computed against Ord-sorted order instead of the ledger's OSet insertion order, and certifying pointers did not count unwitnessed certificates' index slots. Both now follow the ledger's positional resolution, with regression tests pinning the behaviour and property tests checking every witnessable category's pointer against the ledger's own Indexable resolution. The same fix is applied to the deprecated legacy transaction builder (createTransactionBody); end-to-end and legacy-bridge regression properties now cover the deprecated path too. Also harden getVotes against voter-map gaps, remove the unused StakeCredential field from the Witnessable WitTxCert constructor, and correct the redeemer-index ordering documentation.
1 parent 3ab3f29 commit 38fa4b2

9 files changed

Lines changed: 1005 additions & 18 deletions

File tree

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
project: cardano-api
2+
pr: 1288
3+
kind:
4+
- bugfix
5+
- breaking
6+
- test
7+
description: |
8+
Fix plutus redeemer pointer indexing: proposal pointers now follow the transaction's insertion order and certificate pointers count unwitnessed certificates, in both the experimental and the deprecated transaction builders. Remove the unused StakeCredential field from the WitTxCert constructor. Add property tests checking every redeemer pointer against the ledger's own resolution.

cardano-api/cardano-api.cabal

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,7 @@ test-suite cardano-api-test
368368
cardano-crypto,
369369
cardano-crypto-class:{cardano-crypto-class, testlib} ^>=2.3,
370370
cardano-crypto-wrapper:testlib,
371+
cardano-data >=1.0,
371372
cardano-ledger-alonzo,
372373
cardano-ledger-api ^>=1.13,
373374
cardano-ledger-babbage,
@@ -420,6 +421,7 @@ test-suite cardano-api-test
420421
Test.Cardano.Api.Orphans
421422
Test.Cardano.Api.RawBytes
422423
Test.Cardano.Api.Transaction.Autobalance
424+
Test.Cardano.Api.Transaction.Body.Plutus.RedeemerIndex
423425
Test.Cardano.Api.Transaction.Body.Plutus.Scripts
424426
Test.Cardano.Api.TxBody
425427
Test.Cardano.Api.Value

cardano-api/src/Cardano/Api/Experimental/Plutus/Internal/IndexedPlutusScriptWitness.hs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,6 @@ data Witnessable (thing :: WitnessableItem) era where
7676
WitTxCert
7777
:: (L.EraTxCert era, L.AlonzoEraScript era)
7878
=> L.TxCert era
79-
-> StakeCredential
8079
-> Witnessable CertItem era
8180
WitMint
8281
:: L.AlonzoEraScript era
@@ -108,11 +107,16 @@ compareWitnesses :: Witnessable thing era -> Witnessable thing era -> Ordering
108107
compareWitnesses a b =
109108
case (a, b) of
110109
(WitTxIn txinA, WitTxIn txinB) -> compare txinA txinB
111-
(WitTxCert{}, WitTxCert{}) -> LT -- Certificates in the ledger are in an `OSet` therefore we preserve the order.
110+
-- Certificates are stored in an `OSet` but resolved positionally, via
111+
-- `certsTxBodyL`'s `findIndexL`. `EQ` lets the stable sort in
112+
-- `createIndexedPlutusScriptWitnesses` preserve insertion order.
113+
(WitTxCert{}, WitTxCert{}) -> EQ
112114
(WitMint polIdA _, WitMint polIdB _) -> compare polIdA polIdB
113115
(WitWithdrawal stakeAddrA _, WitWithdrawal stakeAddrB _) -> compare stakeAddrA stakeAddrB
114116
(WitVote voterA, WitVote voterB) -> compare voterA voterB
115-
(WitProposal propA, WitProposal propB) -> compare propA propB
117+
-- Proposals are also stored in an `OSet` and resolved positionally
118+
-- (`StrictSeq.findIndexL`), same as `WitTxCert` above.
119+
(WitProposal{}, WitProposal{}) -> EQ
116120

117121
data WitnessableItem
118122
= TxInItem

cardano-api/src/Cardano/Api/Experimental/Tx/Internal/BodyContent/New.hs

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -859,6 +859,13 @@ extractWitnessableTxIns tIns =
859859
obtainCommonConstraints (useEra @era) $
860860
List.nub [(WitTxIn txin, wit) | (txin, wit) <- tIns]
861861

862+
-- | Wrap every certificate as a 'Witnessable', paired with its witness.
863+
--
864+
-- Unwitnessed certificates are included too, paired with
865+
-- 'AnyKeyWitnessPlaceholder'. An unwitnessed certificate still occupies a
866+
-- redeemer index slot: the ledger indexes the 'Certifying' purpose by
867+
-- position in the full certificate sequence, not just the witnessed
868+
-- subset.
862869
extractWitnessableCertificates
863870
:: forall era
864871
. IsEra era
@@ -867,10 +874,8 @@ extractWitnessableCertificates
867874
extractWitnessableCertificates txCerts =
868875
obtainCommonConstraints (useEra @era) $
869876
List.nub
870-
[ ( WitTxCert cert stakeCred
871-
, wit
872-
)
873-
| (Exp.Certificate cert, Just (stakeCred, wit)) <- getCertificates txCerts
877+
[ (WitTxCert cert, maybe AnyKeyWitnessPlaceholder snd mCredAndWit)
878+
| (Exp.Certificate cert, mCredAndWit) <- getCertificates txCerts
874879
]
875880
where
876881
getCertificates (TxCertificates txcs) = toList txcs
@@ -918,13 +923,20 @@ extractWitnessableVotes (Just txVoteProc) =
918923
| (vote, wit) <- getVotes txVoteProc
919924
]
920925
where
926+
-- Uses a total 'Map.findWithDefault' (placeholder witness on a miss),
927+
-- not a lookup that skips missing voters. A skipped voter would shrink
928+
-- this list and shift every later voter's redeemer index.
929+
--
930+
-- 'mkTxVotingProcedures' builds 'scriptWitnessedVotes' in lockstep with
931+
-- 'allVotingProcedures', assuming exactly one voter per merged
932+
-- 'L.VotingProcedures' value, so a miss should not normally happen.
921933
getVotes
922934
:: TxVotingProcedures (LedgerEra era)
923935
-> [(L.Voter, AnyWitness (LedgerEra era))]
924936
getVotes (TxVotingProcedures allVotingProcedures scriptWitnessedVotes) =
925937
[ (voter, wit)
926938
| (voter, _) <- toList $ L.unVotingProcedures allVotingProcedures
927-
, wit <- maybe [] return (Map.lookup voter scriptWitnessedVotes)
939+
, let wit = Map.findWithDefault AnyKeyWitnessPlaceholder voter scriptWitnessedVotes
928940
]
929941

930942
extractWitnessableProposals

cardano-api/src/Cardano/Api/Tx.hs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -845,13 +845,13 @@ module Cardano.Api.Tx
845845
, fromShelleyMetadata
846846
, toShelleyMetadatum
847847
, fromShelleyMetadatum
848-
-- Exported for testing
849-
, extractWitnessableCertificates
848+
-- Exported for testing and advanced use
850849
, extractWitnessableMints
851850
, extractWitnessableProposals
852851
, extractWitnessableTxIns
853852
, extractWitnessableVotes
854853
, extractWitnessableWithdrawals
854+
, extractWitnessableCertificates
855855
-- Exporting for testing. Deprecate in the future.
856856
, legacyKeyWitnessEncode
857857

cardano-api/src/Cardano/Api/Tx/Internal/Body.hs

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -830,8 +830,8 @@ mkTxProposalProcedures proposals = do
830830
fromList $
831831
map (second pure) proposals
832832

833-
-- | Index proposal procedures by their order ('Ord').
834-
-- | and filter out the ones that do not have a witness.
833+
-- | Index proposal procedures by the order they appear in the transaction,
834+
-- and filter out the ones that do not have a witness.
835835
indexTxProposalProcedures
836836
:: TxProposalProcedures BuildTx era
837837
-> [(ScriptWitnessIndex, L.ProposalProcedure (ShelleyLedgerEra era), ScriptWitness WitCtxStake era)]
@@ -840,7 +840,7 @@ indexTxProposalProcedures proposals =
840840
| (proposal, Just (ix, scriptWitness)) <- indexWitnessedTxProposalProcedures proposals
841841
]
842842

843-
-- | Index proposal procedures by their order ('Ord').
843+
-- | Index proposal procedures by the order they appear in the transaction.
844844
indexWitnessedTxProposalProcedures
845845
:: TxProposalProcedures BuildTx era
846846
-> [ ( L.ProposalProcedure (ShelleyLedgerEra era)
@@ -2419,17 +2419,28 @@ extractWitnessableWithdrawals aeon txWithdrawals =
24192419
getWithdrawals TxWithdrawalsNone = []
24202420
getWithdrawals (TxWithdrawals _ txws) = txws
24212421

2422+
-- | Convert every certificate to a 'Witnessable', paired with its witness.
2423+
--
2424+
-- Every certificate must stay in the result, witnessed or not: an
2425+
-- unwitnessed certificate still occupies a redeemer index slot, since the
2426+
-- ledger indexes the 'Certifying' purpose by position in the full
2427+
-- certificate sequence, not just the witnessed subset. See
2428+
-- 'indexCertificatesWith' for the same rule applied to the deprecated
2429+
-- indexing path. An unwitnessed certificate is paired with the old API's
2430+
-- inert stake witness, 'KeyWitness' 'KeyWitnessForStakeAddr' (the same
2431+
-- default 'mkTxCertificates' uses), from which
2432+
-- 'legacyWitnessToScriptRequirements' extracts no script requirement.
24222433
extractWitnessableCertificates
24232434
:: AlonzoEraOnwards era
24242435
-> TxCertificates BuildTx era
24252436
-> [(Witnessable CertItem (ShelleyLedgerEra era), BuildTxWith BuildTx (Witness WitCtxStake era))]
24262437
extractWitnessableCertificates aeon txCertificates =
24272438
alonzoEraOnwardsConstraints aeon $
24282439
List.nub
2429-
[ ( WitTxCert cert stakeCred
2430-
, BuildTxWith wit
2440+
[ ( WitTxCert cert
2441+
, BuildTxWith $ maybe (KeyWitness KeyWitnessForStakeAddr) snd mCredAndWit
24312442
)
2432-
| (Exp.Certificate cert, BuildTxWith (Just (stakeCred, wit))) <- getCertificates txCertificates
2443+
| (Exp.Certificate cert, BuildTxWith mCredAndWit) <- getCertificates txCertificates
24332444
]
24342445
where
24352446
getCertificates TxCertificatesNone = []

cardano-api/test/cardano-api-test/Test/Cardano/Api/Experimental.hs

Lines changed: 149 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,11 @@ where
1717
import Cardano.Api qualified as Api
1818
import Cardano.Api.Experimental qualified as Exp
1919
import Cardano.Api.Experimental.AnyScriptWitness
20-
( AnyPlutusScriptWitness (AnyPlutusSpendingScriptWitness)
20+
( AnyPlutusScriptWitness
21+
( AnyPlutusCertifyingScriptWitness
22+
, AnyPlutusProposingScriptWitness
23+
, AnyPlutusSpendingScriptWitness
24+
)
2125
, PlutusSpendingScriptWitness (PlutusSpendingScriptWitnessV3)
2226
)
2327
import Cardano.Api.Experimental.Era (convert)
@@ -30,6 +34,7 @@ import Cardano.Api.Plutus qualified as Script
3034
import Cardano.Api.Tx (Tx (ShelleyTx))
3135

3236
import Cardano.Ledger.Address qualified as L
37+
import Cardano.Ledger.Alonzo.TxWits qualified as Alonzo
3338
import Cardano.Ledger.Api qualified as UnexportedLedger
3439
import Cardano.Ledger.Babbage.TxBody qualified as L
3540
import Cardano.Ledger.Conway qualified as L
@@ -55,7 +60,13 @@ import Data.Time.Clock.POSIX qualified as Time
5560
import Lens.Micro
5661

5762
import Test.Gen.Cardano.Api.Experimental (genAnyScript)
58-
import Test.Gen.Cardano.Api.Typed (genAddressInEra, genTx, genTxIn)
63+
import Test.Gen.Cardano.Api.Typed
64+
( genAddressInEra
65+
, genProposal
66+
, genStakeCredential
67+
, genTx
68+
, genTxIn
69+
)
5970

6071
import Hedgehog (Gen, Property)
6172
import Hedgehog qualified as H
@@ -100,6 +111,12 @@ tests =
100111
[ testProperty
101112
"Plutus scripts without protocol params returns MakeUnsignedTxMissingProtocolParams"
102113
prop_makeUnsignedTx_plutus_without_pparams
114+
, testProperty
115+
"Proposal-procedure redeemer pointers follow OMap insertion order, not Ord order"
116+
prop_makeUnsignedTx_proposal_redeemer_indices_follow_insertion_order
117+
, testProperty
118+
"Certifying redeemer indices count unwitnessed certs preceding a plutus-witnessed one"
119+
prop_makeUnsignedTx_cert_redeemer_indices_count_unwitnessed_certs
103120
]
104121
, testGroup
105122
"calcMinFeeRecursive"
@@ -619,6 +636,136 @@ prop_makeUnsignedTx_plutus_without_pparams = H.propertyOnce $ do
619636
Exp.makeUnsignedTx Exp.ConwayEra txBodyContent
620637
H.=== Left Exp.MakeUnsignedTxMissingProtocolParams
621638

639+
-- | 'makeUnsignedTx' must index plutus-witnessed governance proposals'
640+
-- redeemer pointers ('L.ConwayProposing') by insertion order, never by
641+
-- 'Ord' order. Insertion order is what the ledger's 'OSet'-backed
642+
-- 'proposalProceduresTxBodyL' stores them in.
643+
--
644+
-- 'propA' and 'propB' only differ in 'pProcDeposit' (the first field
645+
-- 'Ord' compares), chosen so 'propB' sorts before 'propA' by 'Ord' but is
646+
-- inserted after it. A regression to 'Ord'-sorted indexing would swap
647+
-- which redeemer lands at which index.
648+
prop_makeUnsignedTx_proposal_redeemer_indices_follow_insertion_order :: Property
649+
prop_makeUnsignedTx_proposal_redeemer_indices_follow_insertion_order = H.property $ do
650+
scriptTxIn <- H.forAll genTxIn
651+
baseA <- H.forAll (genProposal Api.ConwayEraOnwardsConway)
652+
baseB <- H.forAll (genProposal Api.ConwayEraOnwardsConway)
653+
let propA = baseA{L.pProcDeposit = 2_000_000}
654+
propB = baseB{L.pProcDeposit = 1_000_000}
655+
656+
mkRedeemer :: Integer -> Script.HashableScriptData
657+
mkRedeemer n = Script.unsafeHashableScriptData $ Script.ScriptDataConstructor n []
658+
659+
mkProposingWitness redeemer =
660+
Exp.AnyPlutusScriptWitness $
661+
AnyPlutusProposingScriptWitness $
662+
Exp.PlutusScriptWitness
663+
Plutus.SPlutusV3
664+
(Exp.PReferenceScript scriptTxIn)
665+
Exp.NoScriptDatum
666+
redeemer
667+
(Script.ExecutionUnits 0 0)
668+
669+
txBodyContent =
670+
Exp.defaultTxBodyContent
671+
& Exp.setTxProtocolParams exampleProtocolParams
672+
& Exp.setTxProposalProcedures
673+
( Exp.mkTxProposalProcedures
674+
[ (propA, mkProposingWitness (mkRedeemer 1))
675+
, (propB, mkProposingWitness (mkRedeemer 2))
676+
]
677+
)
678+
& Exp.setTxFee 0
679+
680+
Exp.UnsignedTx ledgerTx <- H.evalEither $ Exp.makeUnsignedTx Exp.ConwayEra txBodyContent
681+
682+
-- Sanity check: the body itself is in insertion order regardless of the
683+
-- bug under test (the bug only affects redeemer indexing, not the body).
684+
let bodyProposals = toList $ ledgerTx ^. L.bodyTxL . UnexportedLedger.proposalProceduresTxBodyL
685+
bodyProposals H.=== [propA, propB]
686+
687+
-- The redeemer map must key 'propA''s witness to index 0 and 'propB''s
688+
-- to index 1 (insertion order). 'Ord'-sorted indexing would give the
689+
-- opposite, since 'propB' has the smaller deposit and sorts first.
690+
let redeemers = ledgerTx ^. L.witsTxL . Alonzo.rdmrsTxWitsL
691+
expectedRedeemers =
692+
L.Redeemers $
693+
Map.fromList
694+
[
695+
( L.ConwayProposing (L.AsIx 0)
696+
, (Api.toAlonzoData (mkRedeemer 1), Api.toAlonzoExUnits (Script.ExecutionUnits 0 0))
697+
)
698+
,
699+
( L.ConwayProposing (L.AsIx 1)
700+
, (Api.toAlonzoData (mkRedeemer 2), Api.toAlonzoExUnits (Script.ExecutionUnits 0 0))
701+
)
702+
]
703+
redeemers H.=== expectedRedeemers
704+
705+
-- | 'makeUnsignedTx' must index a plutus-witnessed certificate's
706+
-- 'L.ConwayCertifying' redeemer pointer by its position among all
707+
-- certificates, witnessed and unwitnessed alike, never just among the
708+
-- witnessed subset.
709+
--
710+
-- 'unwitnessedCert' (a plain stake registration, which the ledger never
711+
-- requires a witness for) is placed before 'witnessedCert'. If unwitnessed
712+
-- certs were skipped when assigning indices, 'witnessedCert' would land
713+
-- at index 0 instead of the correct index 1.
714+
prop_makeUnsignedTx_cert_redeemer_indices_count_unwitnessed_certs :: Property
715+
prop_makeUnsignedTx_cert_redeemer_indices_count_unwitnessed_certs = H.property $ do
716+
stakeCred1 <- H.forAll genStakeCredential
717+
stakeCred2 <- H.forAll genStakeCredential
718+
scriptTxIn <- H.forAll genTxIn
719+
let shelleyCred1 = Api.toShelleyStakeCredential stakeCred1
720+
shelleyCred2 = Api.toShelleyStakeCredential stakeCred2
721+
722+
-- Unwitnessed: a plain stake registration cert needs no witness.
723+
unwitnessedCert =
724+
Exp.Certificate $ L.ConwayTxCertDeleg (L.ConwayRegCert shelleyCred1 L.SNothing)
725+
726+
-- Plutus-witnessed: a stake delegation cert witnessed by a plutus script.
727+
witnessedCert =
728+
Exp.Certificate $
729+
L.ConwayTxCertDeleg (L.ConwayDelegCert shelleyCred2 (L.DelegVote L.DRepAlwaysAbstain))
730+
731+
redeemer = Script.unsafeHashableScriptData $ Script.ScriptDataConstructor 0 []
732+
733+
certWitness =
734+
Exp.AnyPlutusScriptWitness $
735+
AnyPlutusCertifyingScriptWitness $
736+
Exp.PlutusScriptWitness
737+
Plutus.SPlutusV3
738+
(Exp.PReferenceScript scriptTxIn)
739+
Exp.NoScriptDatum
740+
redeemer
741+
(Script.ExecutionUnits 0 0)
742+
743+
certs =
744+
Exp.mkTxCertificates
745+
Exp.ConwayEra
746+
[ (unwitnessedCert, Exp.AnyKeyWitnessPlaceholder)
747+
, (witnessedCert, certWitness)
748+
]
749+
750+
txBodyContent =
751+
Exp.defaultTxBodyContent
752+
& Exp.setTxCertificates certs
753+
& Exp.setTxProtocolParams exampleProtocolParams
754+
& Exp.setTxFee 0
755+
756+
Exp.UnsignedTx ledgerTx <- H.evalEither $ Exp.makeUnsignedTx Exp.ConwayEra txBodyContent
757+
758+
let redeemers = ledgerTx ^. L.witsTxL . Alonzo.rdmrsTxWitsL
759+
expectedRedeemers =
760+
L.Redeemers $
761+
Map.fromList
762+
[
763+
( L.ConwayCertifying (L.AsIx 1)
764+
, (Api.toAlonzoData redeemer, Api.toAlonzoExUnits (Script.ExecutionUnits 0 0))
765+
)
766+
]
767+
redeemers H.=== expectedRedeemers
768+
622769
-- ---------------------------------------------------------------------------
623770
-- Property tests for calcMinFeeRecursive
624771
-- ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)