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.

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

Fix edge case for IS (skip inputs that are too large) #1695

Merged
merged 2 commits into from
Oct 25, 2017

Conversation

UdjinM6
Copy link

@UdjinM6 UdjinM6 commented Oct 23, 2017

Selecting huge inputs will make IS fail due to violation of spork threshold, we must exclude them from our set of available inputs for IS txes.

@UdjinM6 UdjinM6 added the bug label Oct 23, 2017
@UdjinM6 UdjinM6 added this to the 12.2 milestone Oct 23, 2017
@@ -2451,6 +2451,11 @@ static void ApproximateBestSubset(vector<pair<CAmount, pair<const CWalletTx*,uns
vfIncluded[i] = true;
if (nTotal >= nTargetValue)
{
if (fUseInstantSend && nTotal > sporkManager.GetSporkValue(SPORK_5_INSTANTSEND_MAX_VALUE)) {
Copy link

@codablock codablock Oct 23, 2017

Choose a reason for hiding this comment

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

It took me some time to understand what this actually does. As I understand it, it skips considering this input as candidate for the "best subset". If this is the intent, wouldn't it be better to just add if (nTotal + vValue[i].first > max) continue at the beginning of the inner loop (after line 2441)?

EDIT: Also, all the randomization stuff is unimportant for too large IX inputs, because it's nevertheless invalid for IX. So it's more clear when it's outside of these ifs.

@codablock
Copy link

utACK

1 similar comment
@gladcow
Copy link

gladcow commented Oct 25, 2017

utACK

@UdjinM6 UdjinM6 merged commit 788ae63 into dashpay:v0.12.2.x Oct 25, 2017
UdjinM6 added a commit to UdjinM6/dash that referenced this pull request Nov 3, 2017
UdjinM6 added a commit that referenced this pull request Nov 3, 2017
codablock added a commit to codablock/dash that referenced this pull request Dec 27, 2017
dashpay#1695 introduced a fix for a instant send related edge case. Somehow the
parameters got mixed up and fUseInstantSend was passed as "iterations".
UdjinM6 pushed a commit that referenced this pull request Dec 28, 2017
…et (#1819)

#1695 introduced a fix for a instant send related edge case. Somehow the
parameters got mixed up and fUseInstantSend was passed as "iterations".
UdjinM6 pushed a commit to UdjinM6/dash that referenced this pull request Jan 5, 2018
…et (dashpay#1819)

dashpay#1695 introduced a fix for a instant send related edge case. Somehow the
parameters got mixed up and fUseInstantSend was passed as "iterations".
andvgal pushed a commit to energicryptocurrency/gen2-energi that referenced this pull request Dec 26, 2018
…et (dashpay#1819)

dashpay#1695 introduced a fix for a instant send related edge case. Somehow the
parameters got mixed up and fUseInstantSend was passed as "iterations".
@UdjinM6 UdjinM6 deleted the fixisedgecase branch January 31, 2019 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants