Skip to content

feat: finish SealedTx cardano-api decommission (follow-up to #5271)#5272

Merged
paolino merged 7 commits into
masterfrom
feat/drop-cardano-api-sealedtx-remainder
May 8, 2026
Merged

feat: finish SealedTx cardano-api decommission (follow-up to #5271)#5272
paolino merged 7 commits into
masterfrom
feat/drop-cardano-api-sealedtx-remainder

Conversation

@paolino
Copy link
Copy Markdown
Collaborator

@paolino paolino commented Apr 24, 2026

Summary

Follow-up to #5271. This PR finishes the Cardano.Wallet.Primitive.Types.Tx.SealedTx cardano-api decommission: the primitive SealedTx module no longer imports Cardano.Api, and the deprecated cardano-api-facing SealedTx exports are removed.

Base branch: feat/drop-cardano-api-sealedtx (#5271).
Rebased after #5270 merged, #5236 was rebased onto master, and #5271 was rebased onto #5236.

What changed

# SHA Purpose
1 734f624 Plan for finishing the SealedTx cardano-api decommission
2 80a3de4 Port metadata extraction in Shared/Transactions to ledger-native
3 8faf76e Drop the dead cardano-api body in addRequiredSigners
4 a929e8e Migrate buildAndSignTransactionPure to ledger-native seal
5 e2e72af Drop the local sealWriteTx cardano-api bridge in Server.hs
6 0b619dd Remove the SealedTx cardano-api surface and move remaining unit-test adapters local to tests
7 b4da418 Keep default sealed transaction byte decoding capped at the supported Conway era, and make signing/decoding re-read sealed bytes with the caller's preferred latest era

SealedTx surface removed

Removed from Cardano.Wallet.Primitive.Types.Tx.SealedTx and the umbrella Cardano.Wallet.Primitive.Types.Tx export list:

  • cardanoTxIdeallyNoLaterThan
  • sealedTxFromCardano
  • sealedTxFromCardano'
  • sealedTxFromCardanoBody
  • getSealedTxBody
  • getSealedTxWitnesses
  • internal cardanoTxFromBytes
  • internal cardanoApiTxToReadTx

sealedTxFromBytes' now takes Read.EraValue Read.Era instead of AnyCardanoEra, which lets the primitive module avoid Cardano.Api entirely.

CI fix after rerun

The first rerun of #5272 exposed a real Conway integration failure: sealed Conway transaction bytes could be decoded as Dijkstra by the new ledger-native default path, after which downstream code hit the still-incomplete Dijkstra transaction support.

This PR now keeps the public sealedTxFromBytes default capped at Conway, and the transaction layer re-decodes sealed bytes with the preferred latest era supplied by the caller before signing or decoding. That preserves the old "no later than current era" behavior without restoring the cardano-api bridge.

Remaining out of scope

  • Removing cardano-api from cardano-wallet-primitive.cabal; other primitive modules still import it.
  • Removing lib/cardano-api-extra/.
  • Full deletion of the old Shelley/Transaction.hs module.

Test plan

  • nix develop --command fourmolu --mode inplace ... on touched Haskell files
  • git diff --check
  • nix develop --command just build 'cardano-wallet-primitive cardano-wallet cardano-wallet-unit:unit'
  • nix develop --command cabal test cardano-wallet-unit:unit -O0 -v0 --test-options '--match="TransactionSpec"'
  • nix develop --command cabal test cardano-wallet-unit:unit -O0 -v0 --test-options '--match="SealedTx serialisation/deserialisation"'
  • nix develop --command cabal build cardano-wallet cardano-wallet-api cardano-wallet-unit:unit --enable-tests --enable-benchmarks --minimize-conflict-set -O0 -v0
  • rg 'Cardano\.Api|qualified Cardano.Api' lib/primitive/lib/Cardano/Wallet/Primitive/Types/Tx/SealedTx.hs returns no matches
  • CI green on the rebased tip

@paolino paolino self-assigned this Apr 24, 2026
@paolino paolino added the Refactoring No functional changes label Apr 24, 2026
@paolino paolino force-pushed the feat/drop-cardano-api-sealedtx branch from b530ab1 to 65a5529 Compare April 25, 2026 09:41
@paolino paolino force-pushed the feat/drop-cardano-api-sealedtx-remainder branch from 80ec3ce to dd6ae54 Compare April 25, 2026 09:42
@paolino paolino force-pushed the feat/drop-cardano-api-sealedtx branch from 65a5529 to c2ca434 Compare April 25, 2026 10:11
@paolino paolino force-pushed the feat/drop-cardano-api-sealedtx-remainder branch from dd6ae54 to 0b619dd Compare April 25, 2026 10:16
@paolino paolino requested review from paweljakubas and removed request for paweljakubas April 25, 2026 10:18
@paolino
Copy link
Copy Markdown
Collaborator Author

paolino commented Apr 27, 2026

@copilot, pls review

Copy link
Copy Markdown

Copilot AI commented Apr 27, 2026

@copilot

Hi @paolino! It looks like your comment is just a ping with no specific request. Could you let me know what you'd like me to do or look at? Happy to help!

@paolino
Copy link
Copy Markdown
Collaborator Author

paolino commented Apr 27, 2026

@copilot , pls review

@paolino
Copy link
Copy Markdown
Collaborator Author

paolino commented Apr 28, 2026

@copilot, pls review

Copy link
Copy Markdown

Copilot AI commented Apr 28, 2026

@copilot , pls review

Here's my review of the 7 commits in this PR:


Overall assessment ✅

The PR is clean, well-structured, and achieves its stated goal: SealedTx no longer imports Cardano.Api and all deprecated cardano-api-facing functions are removed. The commits are logically ordered and each has a clear, accurate commit message.


Commit-by-commit notes

1 – docs: plan (734f624) — planning doc in specs/ only; no code concerns.

2 – port metadata extraction in Shared/Transactions (80a3de4)
getMetadataFromTx via Meta.getEraMetadata is a direct, idiomatic replacement for the cardanoTxIdeallyNoLaterThan round-trip. No issues.

3 – drop dead code in addRequiredSigners (8faf76e)
Removing the commented-out cardano-api body is the right call. The updated TODO comment (reqSignerHashesTxBodyL, Alonzo-and-later) gives the next implementer clear guidance.

4 – buildAndSignTransactionPure (a929e8e)
Replacing toCardanoApiTx → inAnyCardanoEra → sealedTxFromCardano with sealWriteTx (Write.recentEra @era) is a straightforward improvement. The refreshed haddock on sealWriteTx is accurate.

5 – Server.hs bridge removal (e2e72af)
Both call sites (constructSharedTransaction, toApiSerialisedTransaction) now pass Write.recentEra explicitly, which is correct since the era is known at the call sites. No issues.

6 – Remove SealedTx cardano-api surface (0b619dd)

  • readTxFromBytes is cleanly ported from AnyCardanoEra to EraValue Read.Era.
  • withinEra uses a locally-defined numberEra ordering, which is clear and exhaustive.
  • Test helpers (sealedTxFromCardano, getSealedTxWitnesses, cardanoTx) are moved to the unit test files — appropriate, since they remain useful for roundtrip tests but don't belong in the primitive library.
  • cardanoTx in the test file has a partial match error for Dijkstra (error "cardanoTx: Dijkstra not yet supported"). This is intentional and reasonable for now — an explicit error is better than a silent failure — but it's worth tracking for when Dijkstra support lands.

7 – Keep decoding within supported era (b4da418)
sealedTxFromBytes defaulting to EraValue Conway prevents Conway-serialised bytes from being decoded as Dijkstra before Dijkstra transaction support is complete. The readSealedTxIdeallyNoLaterThan helper in Transaction.hs mirrors the old cardanoTxIdeallyNoLaterThan semantics: try preferredLatestEra first, fall back to the already-cached unsafeReadTx if re-decoding fails. The new prop_sealedTxDefaultRecentEraRoundtrip test directly covers this invariant. The _erapreferredLatestEra rename in addVkWitnesses is a good clarity improvement.


Minor observations

  • readSealedTxIdeallyNoLaterThan in Transaction.hs is private (not exported). If the "re-decode or fall back" logic ever needs to be used from another module, consider promoting it. For now, keeping it local is fine.
  • sealedTxFromBytes' haddock says -- ^ Most recent era for the first argument; "maximum era" or "upper-bound era" would be slightly more precise given that it acts as a cap, not the actual current protocol era. Very minor.
  • The {-# OPTIONS_GHC -Wno-unused-imports #-} pragma added in commit 8 to TransactionsNew.hs (carried over from before) — this is a pre-existing issue and not introduced here, so fine.

No blocking concerns. The changes are correct, the fallback logic in readSealedTxIdeallyNoLaterThan is sound, and the test coverage is appropriate for the scope of the change.

@copilot, pls review

Review posted above on the earlier comment thread — please see the detailed notes there.

Copy link
Copy Markdown
Collaborator

@paweljakubas paweljakubas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, nothing suspicious here, pretty straightforward

paolino added a commit that referenced this pull request May 8, 2026
…5236)

## Summary

Remaining call-site migration after #5270 merged. This branch is rebased
directly onto current `master` and contains the commits that switch
existing wallet call sites onto the ledger-native surfaces introduced by
#5270.

Base branch: `master`.
Follow-ups: #5271, then #5272.

## What is in this PR

| # | SHA | Purpose |
|---|---|---|
| 1 | `84bd668` | Use `constructUnsignedTxLedger` in `balanceTx` paths |
| 2 | `4c8b61b` | Replace `AnyCardanoEra` with `Read.EraValue` in
NetworkLayer |
| 3 | `55097e6` | Build delegation and voting certificates in
`balanceTx` paths |
| 4 | `f5b8ed2` | Replace `StakeAddress` with wallet-owned
`RewardAccount` |
| 5 | `26bad3e` | Migrate `TxMetadata` end-to-end to wallet-owned types
|
| 6 | `d543a6b` | Adapt singleton network conversions to #5270's
`sNetworkIdToLedger` helper |

The foundation work previously described here landed in #5270. In
particular, the wallet-owned `NetworkId`, `TxMetadata`, `SealedTx`,
ledger-native transaction builders, certificate helpers, and witness
helpers are now part of `master`.

## What this PR does not do

- It does not delete the remaining cardano-api bridges.
- It does not remove `cardano-api` from cabal files.
- It does not delete `lib/cardano-api-extra/`.
- It does not finish the `SealedTx` decommission; that remains split
across #5271 and #5272.

## Test plan

- [x] Rebased cleanly onto `origin/master` after #5270 merged
(2026-04-25).
- [x] `nix develop --command cabal build cardano-wallet
cardano-wallet-api cardano-wallet-unit:unit --enable-benchmarks
--enable-tests --minimize-conflict-set -O0 -v0`
- [ ] CI green on the rebased tip.

Note: `just build 'cardano-wallet cardano-wallet-unit:unit'` reaches
unrelated `cardano-wallet-read` deprecation/unused warnings promoted by
`-Werror` on this branch; the same target without `-Werror` passes.
@paolino paolino force-pushed the feat/drop-cardano-api-sealedtx branch from 18a23ac to fdfadf0 Compare May 8, 2026 10:21
paolino added a commit that referenced this pull request May 8, 2026
…5271)

## Summary

Progress toward removing `Cardano.Api*` from
`Cardano.Wallet.Primitive.Types.Tx.SealedTx`. Moves the first set of
production and test call sites off the deprecated surface. Follow-up
#5272 now finishes the remaining SealedTx cleanup.

Base branch: `001-drop-cardano-api` (#5236).
Rebased after #5270 merged and #5236 was rebased onto `master`.

## What this PR does

Eight bisect-safe commits:

| # | SHA | Effect |
|---|---|---|
| 1 | `04d65b2` | `docs`: plan document for the decommission series |
| 2 | `b4b87af` | Test helper `compareOnCBOR` -> raw `serialisedTx`
bytes, avoiding a cardano-api roundtrip |
| 3 | `282807d` | Submission path `_postSealedTx` -> `EraValue Read.Tx`
+ `consensusGenTxFromTxRecent`; deletes `unsealShelleyTx` /
`UnsealException` |
| 4 | `a6d0c9d` | `parsePartialTx` -> `deserializeTx :: Either
DecoderError (Read.Tx era)` per era |
| 5 | `f1ba908` | Delete `cardanoTxInExactEra` after the callers are
gone |
| 6 | `55f2bd9` | Add `sealedTxFromLedgerTx :: Read.IsEra era => Read.Tx
era -> SealedTx`; migrate `sealWriteTx` off
`toCardanoApiTx`/`sealedTxFromCardano'` |
| 7 | `e6feabc` | Add `sealedTxWitnessCount :: SealedTx -> Int` via
ledger witnesses; migrate Server.hs away from `getSealedTxWitnesses` |
| 8 | `c2ca434` | Integration test `getMetadataFromTx` ->
`Meta.getMetadata . Meta.getEraMetadata` on `EraValue Read.Tx` |

New ledger-native surface added to `SealedTx`:

- `sealedTxFromLedgerTx :: Read.IsEra era => Read.Tx era -> SealedTx`
- `sealedTxWitnessCount :: SealedTx -> Int`

Deleted:

- `cardanoTxInExactEra`
- `Cardano.Wallet.Primitive.Ledger.Shelley.unsealShelleyTx` /
`UnsealException`

## What this PR does not do

The deprecated SealedTx cardano-api exports are removed in #5272, not
here.

Out of scope:

- Removing `cardano-api` from `cardano-wallet-primitive.cabal`.
- Removing `lib/cardano-api-extra/`.

## Test plan

- [x] Rebased cleanly onto updated #5236 after #5270 merged
(2026-04-25).
- [ ] CI green on the rebased tip.
Base automatically changed from feat/drop-cardano-api-sealedtx to master May 8, 2026 11:00
paolino added 5 commits May 8, 2026 12:14
Enumerates every remaining caller of the deprecated surface after
#5271: production (Cardano.Wallet.hs, Server.hs), old tx builder
(Shelley/Transaction.hs, partially overlaps with #5243), integration
tests (Shared/Transactions.hs, addRequiredSigners in TransactionsNew)
and unit tests (TransactionSpec, TransactionLedgerSpec).

Organises the work into three phases:
  A. reachable without #5243 (4 sites)
  B. unit-test migration (~20 sites, two files)
  C. delete the bridge functions and the three Cardano.Api*
     imports in SealedTx.hs
…ative

Mirror the TransactionsNew.hs:getMetadataFromTx port: project metadata
directly from Read.Tx era via Meta.getMetadata . Meta.getEraMetadata
instead of round-tripping through cardano-api. Drops the two callers
of cardanoTxIdeallyNoLaterThan in this file and the Cardano.Api /
ApiEra qualified imports.
The function has been an ADP-3077 TODO stub (error "...") for a long
time; the commented-out cardano-api body still referenced
getSealedTxBody and sealedTxFromCardanoBody, which blocks their
removal. Drop the dead block and note that a future implementation
should operate on Read.Tx era via reqSignerHashesTxBodyL.
The signedTx wrapping in Cardano.Wallet went through
toCardanoApiTx -> inAnyCardanoEra -> sealedTxFromCardano purely to
smuggle the already-ledger tx back through cardano-api. Use the
existing ledger-native sealWriteTx from Transaction.Ledger instead
(now exported). Drops the two cardano-api bridges inAnyCardanoEra and
sealedTxFromCardano from this module.

Also refresh the sealWriteTx haddock (the function has been ledger-
native since the last revamp; the stale comment claimed otherwise).
Server.hs carried a local copy of sealWriteTx that re-entered
cardano-api (toCardanoApiTx + sealedTxFromCardano) to wrap an already
ledger-native balanceTx result. Import the ledger-native sealWriteTx
from Transaction.Ledger instead; the local helper goes away. Two call
sites now pass Write.recentEra explicitly.

Drops the cardano-api bridges cardanoEraFromRecentEra, toCardanoApiTx
and W.sealedTxFromCardano from this module's imports.
@paolino paolino force-pushed the feat/drop-cardano-api-sealedtx-remainder branch from b4da418 to eb80853 Compare May 8, 2026 11:18
@paolino paolino merged commit fbcb6ad into master May 8, 2026
112 of 113 checks passed
@paolino paolino deleted the feat/drop-cardano-api-sealedtx-remainder branch May 8, 2026 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Refactoring No functional changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants