Skip to content

Conversation

@errfrom
Copy link
Collaborator

@errfrom errfrom commented Apr 8, 2025

Closes Move balancer to a separate package with public API #1621

Extracted the default CTL transaction balancer into its own package: CTL Tx Balancing & Coin Selection #2

Pre-review checklist

  • All code has been formatted using our config (make format)
  • Any new API features or modification of existing behavior are covered with tests
  • The template (templates/ctl-scaffold) has been updated
  • The changelog has been updated under the ## Unreleased header, using the appropriate sub-headings (### Added, ### Changed, ### Removed, ### Fixed), and the links to the appropriate issues/PRs have been included

@errfrom errfrom self-assigned this Apr 8, 2025
Copy link
Collaborator

@marcusbfs marcusbfs left a comment

Choose a reason for hiding this comment

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

Looking great. Two things:

  • the test Evaluation with additional UTxOs with native scripts is failing
  • A new version of the packaget-set containing the balancer package must be released. Then, update CTL (and the scaffold template) to use it

@errfrom errfrom force-pushed the dshuiski/marcusbfs/1621 branch from 3f6f0cb to e35b239 Compare April 10, 2025 16:10
@errfrom errfrom force-pushed the dshuiski/marcusbfs/1621 branch from e35b239 to 4b41b25 Compare April 10, 2025 16:14
@errfrom errfrom marked this pull request as ready for review April 10, 2025 16:29
@errfrom errfrom requested a review from marcusbfs April 10, 2025 16:29
@errfrom errfrom mentioned this pull request Apr 10, 2025
4 tasks
Copy link
Collaborator

@marcusbfs marcusbfs left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@KristianBalaj KristianBalaj left a comment

Choose a reason for hiding this comment

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

The default balancer is in the Cardano.Transaction.Balancer AFAIU. But, it sounds too general namespace for me. Instead, it should be something more specific IMO, that describes it better.

@errfrom errfrom force-pushed the dshuiski/marcusbfs/1621 branch from c14fbc3 to 848a692 Compare April 24, 2025 11:09
@errfrom errfrom requested a review from KristianBalaj April 24, 2025 12:12
@errfrom errfrom merged commit a9b3a70 into develop Apr 25, 2025
3 checks passed
@errfrom errfrom deleted the dshuiski/marcusbfs/1621 branch April 25, 2025 11:35
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.

Move balancer to a separate package with public API

4 participants