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

JSplit fee estimation and calculation optimized #974

Merged
merged 2 commits into from
Jan 30, 2021
Merged

JSplit fee estimation and calculation optimized #974

merged 2 commits into from
Jan 30, 2021

Conversation

levonpetrosyan93
Copy link
Contributor

@levonpetrosyan93 levonpetrosyan93 commented Jan 18, 2021 •
edited
Loading

  • In Lelantus coincontrol fee estimation was not set to be calculated for joinsplit, added the check if transaction is Lelantus joinsplit, it uses EstimateJoinSplitFee() function for calculation,
  • During joinsplit creation it was picking minimum fee, and if SubtractFeeFromAmount was mot set, it was doing one unneeded iteration, so before starting to create transaction, fee estimation is called and returned value is being chosen as transaction fee.
  • In fee estimation GetAvailableCoins() and balance calculation for sigma was called in each iteration, it is moved out of loop,

@lgtm-com
Copy link

lgtm-com bot commented Jan 18, 2021

This pull request introduces 1 alert and fixes 1 when merging 6bc85e8 into 738c8e1 - view on LGTM.com

new alerts:

  • 1 for Catching by value

fixed alerts:

  • 1 for Catching by value

@reubenyap
Copy link
Member

@levonpetrosyan93 needs to be updated

@lgtm-com
Copy link

lgtm-com bot commented Jan 25, 2021

This pull request introduces 3 alerts when merging 798914d into 5525561 - view on LGTM.com

new alerts:

  • 2 for Unused static function
  • 1 for Catching by value

@levonpetrosyan93 levonpetrosyan93 merged commit 7753738 into master Jan 30, 2021
@levonpetrosyan93 levonpetrosyan93 deleted the fee_calc branch January 30, 2021 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@psolstice psolstice psolstice approved these changes

@a-bezrukov a-bezrukov a-bezrukov approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@levonpetrosyan93 @reubenyap @psolstice @a-bezrukov