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.

Sign up for GitHub

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

Jump to bottom

make feeOpt dependent on minimum utxo #1947

Merged
merged 4 commits into from
Jul 25, 2020
Merged

make feeOpt dependent on minimum utxo #1947

merged 4 commits into from
Jul 25, 2020

Conversation

paweljakubas
Copy link
Contributor

Issue Number

Overview

  • I have make feeOpts dependent on minUtxoval
  • I have made fee balancing algorithm be aware of dust threshold

Comments

@paweljakubas paweljakubas self-assigned this Jul 24, 2020
@KtorZ KtorZ force-pushed the paweljakubas/proper-treatment-of-dusting-threshold branch from 88a59d6 to a92397b Compare July 24, 2020 18:57
KtorZ
KtorZ approved these changes Jul 24, 2020
View reviewed changes
Copy link
Member

@KtorZ KtorZ left a comment

Choose a reason for hiding this comment

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

Added refactor of the fee balancing function on top to get the dustThreshold acknowledged there too.

@KtorZ
Copy link
Member

KtorZ commented Jul 24, 2020

bors try

iohk-bors bot added a commit that referenced this pull request Jul 24, 2020
@iohk-bors
Try #1947:
56ccfbc
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jul 24, 2020

try

Build failed

@KtorZ KtorZ force-pushed the paweljakubas/proper-treatment-of-dusting-threshold branch from a92397b to b5150ef Compare July 25, 2020 10:02
@KtorZ KtorZ changed the base branch from master to rvl/1870/bump-deps July 25, 2020 10:03
@KtorZ KtorZ force-pushed the rvl/1870/bump-deps branch from a05514b to 11f35b4 Compare July 25, 2020 10:11
paweljakubas and others added 3 commits July 25, 2020 12:13
@paweljakubas @KtorZ
make feeOpt dependent on minimum utxo
7ab39b1
@KtorZ
add non-null dust & property reflecting it for 'rebalanceSelection'
dc9e571 
  This fails now, since 'rebalanceSelection' completely disregard any
  form of dust threshold.
@KtorZ
acknowledge dust threshold in fee balancing
ec9ea5d 
  I greatly simplified the function as well, it probably handles slightly worse some edge-cases but it's probably for the best in terms of maintenance burden.
@KtorZ KtorZ force-pushed the paweljakubas/proper-treatment-of-dusting-threshold branch from b5150ef to ec9ea5d Compare July 25, 2020 10:13
@KtorZ
Copy link
Member

KtorZ commented Jul 25, 2020

bors try

iohk-bors bot added a commit that referenced this pull request Jul 25, 2020
@iohk-bors
Try #1947:
9cafba2
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jul 25, 2020

try

Build failed

@KtorZ KtorZ mentioned this pull request Jul 25, 2020
Closed
@KtorZ
revert refactor on 'rebalanceSelection' and coalesce dust AFTER rebal…
70235f0 
…ancing

  What we loose from this might be exact balancing in some cases (when the transaction is exactly balanced but coalescing dust may remove one or more change output and change the fee).
  So we might hit problems with some edge-cases on Jörmungandr but things should be fine on cardano-node where we might occasionally pay a little bit more than necessary...
@KtorZ KtorZ force-pushed the paweljakubas/proper-treatment-of-dusting-threshold branch from 2e5ac55 to 70235f0 Compare July 25, 2020 14:19
@KtorZ KtorZ merged commit c4e978a into rvl/1870/bump-deps Jul 25, 2020
@KtorZ KtorZ deleted the paweljakubas/proper-treatment-of-dusting-threshold branch July 25, 2020 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KtorZ KtorZ KtorZ approved these changes

Assignees

@paweljakubas paweljakubas

Labels
None yet
Projects
None yet
Milestone
(ADP-329) minimal UTxO value
Development

Successfully merging this pull request may close these issues.
2 participants
@paweljakubas @KtorZ