Skip to content

Export and reuse extractWitnessable* functions in tests#1125

Merged
Jimbo4350 merged 2 commits intomasterfrom
worktree-1083-replace-inline-witness-conversions
Mar 11, 2026
Merged

Export and reuse extractWitnessable* functions in tests#1125
Jimbo4350 merged 2 commits intomasterfrom
worktree-1083-replace-inline-witness-conversions

Conversation

@Jimbo4350
Copy link
Contributor

@Jimbo4350 Jimbo4350 commented Mar 10, 2026 •
edited
Loading

Changelog

- description: |
    Export `extractWitnessableTxIns`, `extractWitnessableMints`,
    `extractWitnessableCertificates`, `extractWitnessableWithdrawals`,
    `extractWitnessableVotes`, and `extractWitnessableProposals` from
    `Cardano.Api.Experimental.Tx.Internal.BodyContent.New` and
    `Cardano.Api.Experimental.Tx`, and reuse them in tests to eliminate
    duplicated inline conversion logic.
  type:
  - feature
  projects:
  - cardano-api

Context

Closes #1083

How to trust this PR

  • The exported functions are the same logic that was previously inlined as list
    comprehensions in prop_extractAllIndexedPlutusScriptWitnesses
  • Run cabal test cardano-api-test and confirm the property test still passes

Checklist

  • Commit sequence broadly makes sense and commits have useful messages
  • New tests are added if needed and existing tests are updated. See Running tests for more details
  • Self-reviewed the diff

@Jimbo4350
Export and reuse extractWitnessable* functions in tests
17264b1 
Replace six inline list comprehensions in prop_extractAllIndexedPlutusScriptWitnesses
with calls to the existing extractWitnessable* production helpers, which are now
exported from New.hs and re-exported from Tx.hs.
@Jimbo4350 Jimbo4350 force-pushed the worktree-1083-replace-inline-witness-conversions branch from c2aba71 to 17264b1 Compare March 10, 2026 13:40
@Jimbo4350 Jimbo4350 changed the base branch from main to master March 10, 2026 13:40
@Jimbo4350 Jimbo4350 marked this pull request as ready for review March 10, 2026 13:41
Copilot AI review requested due to automatic review settings March 10, 2026 13:41
Copilot AI reviewed Mar 10, 2026
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the prop_extractAllIndexedPlutusScriptWitnesses test by replacing inline list comprehensions with calls to the already-existing production extractWitnessable* functions, eliminating duplicated conversion logic between tests and production code.

Changes:

  • Exports 6 extractWitnessable* functions (TxIns, Mints, Certificates, Withdrawals, Votes, Proposals) from Cardano.Api.Experimental.Tx.Internal.BodyContent.New and Cardano.Api.Experimental.Tx
  • Replaces inline list comprehensions in the test with calls to the newly-exported production functions, removing the now-unused Data.Map.Ordered import

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
cardano-api/src/Cardano/Api/Experimental/Tx/Internal/BodyContent/New.hs Adds 6 extractWitnessable* functions to the module's export list under "Internal conversions"
cardano-api/src/Cardano/Api/Experimental/Tx.hs Re-exports the 6 extractWitnessable* functions under "Internal functions"
cardano-api/test/cardano-api-test/Test/Cardano/Api/Transaction/Body/Plutus/Scripts.hs Replaces 6 inline list comprehensions with calls to the production extractWitnessable* functions and removes the unused OMap import

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

github-advanced-security[bot]
github-advanced-security bot found potential problems Mar 10, 2026
View reviewed changes
cardano-api/test/cardano-api-test/Test/Cardano/Api/Transaction/Body/Plutus/Scripts.hs
| let Exp.TxProposalProcedures pMap = generatedTxProposalProcedures
, (p, wit) <- OMap.toAscList pMap
]
map (\(w, sw) -> (w, anyScriptWitnessToAnyWitness sw)) $

Check notice

Code scanning / HLint

Use second Note test

cardano-api/test/cardano-api-test/Test/Cardano/Api/Transaction/Body/Plutus/Scripts.hs:180:22-69: Suggestion: Use second
  
Found:
  \ (w, sw) -> (w, anyScriptWitnessToAnyWitness sw)
  
Perhaps:
  Data.Bifunctor.second anyScriptWitnessToAnyWitness
  
Note: increases laziness
Copy link
Contributor

@carbolymer carbolymer Mar 11, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

@copilot how come it did not show up in the hlint github action?

@Jimbo4350
Fix formatting and remove unused ordered-containers dependency
ee7d59f 
- Apply fourmolu formatting to Scripts.hs (break long lines)
- Remove ordered-containers from cardano-api-test build-depends
  (unused after replacing inline list comprehensions with direct calls
  to the exported extractWitnessable* functions)
@Jimbo4350 Jimbo4350 force-pushed the worktree-1083-replace-inline-witness-conversions branch from 18c3814 to ee7d59f Compare March 10, 2026 15:41
Copilot AI mentioned this pull request Mar 11, 2026
Draft
Copy link
Contributor

Copilot AI commented Mar 11, 2026

@carbolymer I've opened a new pull request, #1127, to work on those changes. Once the pull request is ready, I'll request review from you.

@Jimbo4350 Jimbo4350 added this pull request to the merge queue Mar 11, 2026
Merged via the queue into master with commit 897236c Mar 11, 2026
32 checks passed
@Jimbo4350 Jimbo4350 deleted the worktree-1083-replace-inline-witness-conversions branch March 11, 2026 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@carbolymer carbolymer carbolymer approved these changes

@dcoutts dcoutts Awaiting requested review from dcoutts dcoutts is a code owner

@erikd erikd Awaiting requested review from erikd erikd is a code owner

@newhoggy newhoggy Awaiting requested review from newhoggy newhoggy is a code owner

@palas palas Awaiting requested review from palas palas is a code owner

@kevinhammond kevinhammond Awaiting requested review from kevinhammond kevinhammond is a code owner

@CarlosLopezDeLara CarlosLopezDeLara Awaiting requested review from CarlosLopezDeLara CarlosLopezDeLara is a code owner

@disassembler disassembler Awaiting requested review from disassembler disassembler is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Refactor allGeneratedPlutusScriptWitnesses

4 participants

@Jimbo4350 @carbolymer