IntersectMBO / cardano-api

Cardano API
Apache License 2.0
26 stars 23 forks source link

[BUG] - `BalancedTx` contains `UnsignedTx` field type from `Experimental` API, forcing users to use the latter #675

Closed Swordlash closed 1 week ago

Swordlash commented 2 weeks ago

External

Other

Summary Pretty much what the title states. I found this while upgrading to 10.1 for the Chang#2. makeTransactionBodyAutoBalance now uses UnsignedTx which forces API users to use the API in the Experimental namespace (i.e. forces IsBabbageBasedEra although it takes ShelleyBasedEra as parameter etc.).

Compare my previous simple code

signTxWithSKey :: IsShelleyBasedEra era => SKey -> TxBody era -> Tx era
signTxWithSKey sk txb = makeSignedTransaction [mkSKeyWitness sk txb] txb

with the migrated one with a lot of manual wrapping/unwrapping to retain previous signature as much as possible (i.e. return cardano-api Tx instead of the ledger one):

signTxWithSKey :: forall era. IsBabbageBasedEra era => SKey -> UnsignedTx era -> Tx era
signTxWithSKey sk (UnsignedTx tx) =
  let 
    bbe = babbageEraOnwards @era
    txb = obtainCommonConstraints (babbageEraOnwardsToEra bbe) $ getTxBody $ ShelleyTx shelleyBasedEra tx
  in makeSignedTransaction [mkSKeyWitness sk txb] txb

Expected behavior Non-experimental API should not force users to rely on Experimental API. There should be a clear distinction between these two since the latter is described as "it is subject to dramatic changes so use with caution".

Jimbo4350 commented 1 week ago

Although UnsignedTx era is unlikely to change (despite being confusingly placed in the Experimental namespace), we did not anticipate such a significant change in your code. We have decided to revert the change and introduce the new API separately.

Swordlash commented 1 week ago

Thank you!