Skip to content

Conversation

@errfrom
Copy link
Member

@errfrom errfrom commented Apr 8, 2025

No description provided.

@errfrom errfrom self-assigned this Apr 8, 2025
@errfrom errfrom changed the base branch from marcusbfs/initial-balancer to master April 9, 2025 11:49
txHasPlutusV1 :: Boolean
txHasPlutusV1 =
case p.transaction ^. _witnessSet <<< _plutusScripts of
case (unwrap (unwrap unbalancedTx).witnessSet).plutusScripts of
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is the change mainly for readability, or does it have performance implications as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure about performance, but I think we should avoid using lenses for such simple expressions to not overcomplicate the code

@marcusbfs
Copy link
Collaborator

@errfrom also, would it be worth introducing a newtype BalancedTransaction = BalancedTransaction Transaction for extra type safety? That way functions like runBalancerAffAff would return something like Aff (Either BalanceTxError BalancedTransaction).

@errfrom
Copy link
Member Author

errfrom commented Apr 10, 2025

@marcusbfs We've actually had this wrapper before, and it always came down to balancing type safety with ease of use. I believe we chose to prioritise the latter, so I'd leave it as is for now.

@errfrom errfrom marked this pull request as ready for review April 10, 2025 13:34
@errfrom errfrom merged commit b7cbf8e into master Apr 10, 2025
2 checks passed
@errfrom errfrom mentioned this pull request Apr 10, 2025
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

Successfully merging this pull request may close these issues.

3 participants