Skip to content

TX fees and policy: fix relaypriority calculation error Issues #8334 #8336

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
Closed
maiiz wants to merge 3 commits into from
Closed

TX fees and policy: fix relaypriority calculation error Issues #8334 #8336

maiiz wants to merge 3 commits into from

Conversation

@maiiz
Copy link
Contributor

@maiiz maiiz commented Jul 13, 2016 •
edited
Loading

CCoinsViewCache::GetPriority has an overflow bug that affects the relaypriority calculation.
The way the arithmetic works in that function is

dResult += coins->vout[txin.prevout.n].nValue * (nHeight-coins->nHeight); 
the nValue variable type is int64_t, and the int64_t max value is 9223372036854775807

for example

nValue value  is 40000000000000
(nHeight->coins - nHeight )value is 300000 
40000000000000 * 300000 > 9223372036854775807

so the dResult will be negative number!
and the AllowFree function will return False, and the high priority transaction with low fee will be reject by node with error insufficient priority!

@maiiz
Issues #8334
e290b2a 
fix bug Issues #8334
@maflcko
Copy link
Member

maflcko commented Jul 13, 2016 •
edited
Loading

Mind to amend the commit subject to be more verbose?

@maiiz maiiz changed the title Issues #8334 TX fees and policy: fix relaypriority calculation error Issues #8334 Jul 13, 2016
@jonasschnelli
Copy link
Contributor

jonasschnelli commented Jul 13, 2016

utACK e290b2a.
I think a short unit or RPC test would be nice.

@paveljanik
Copy link
Contributor

paveljanik commented Jul 13, 2016

I think that in this case, it is also worth to mention the reason to cast the value to double in the source code!

@maflcko
Copy link
Member

maflcko commented Jul 13, 2016

I think that in this case, it is also worth to mention the reason to cast the value to double in the source code!

Should be enough to mention it in the commit subject line, so when in doubt, you can always do git blame.

@maflcko maflcko mentioned this pull request Jul 17, 2016
Merged
@maflcko
Copy link
Member

maflcko commented Jul 17, 2016

@maiiz You can change the commit subject line with git commit --amend, if you didn't know. Make sure to force push afterward.

@maiiz maiiz closed this Jul 18, 2016
@maiiz maiiz deleted the issues-8334 branch July 18, 2016 03:19
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

Wallet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@maiiz @maflcko @jonasschnelli @paveljanik