Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Swordlash opened this issue Nov 7, 2024 · 2 comments
Assignees

Comments

@Swordlash
Copy link
Contributor

Swordlash commented Nov 7, 2024

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".

@carbolymer carbolymer self-assigned this Nov 13, 2024
@Jimbo4350
Copy link
Contributor

Jimbo4350 commented Nov 13, 2024

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
Copy link
Contributor Author

Thank you!

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

No branches or pull requests

3 participants