Fix nPayFee calculation #3932

pull Gabrola wants to merge 1 commits into bitcoin:master from Gabrola:master changing 1 files +1 −1

Gabrola commented at 10:57 PM on March 21, 2014: none

nPayFee is more than intended if nBytes is exactly in multiples of 1000 bytes

Fix nPayFee

nPayFee is more than intended if nBytes is exactly in multiples of 1000
bytes

64cd4b34f3

ghost commented at 11:06 PM on March 21, 2014: none

Would changing this to /1023 instead of /1000 result in faster code as the compiler could reduce this to a series of bit shifting operations?
luke-jr commented at 11:15 PM on March 21, 2014: member

@Gabrola What makes you think it's more than intended? @NanoAkron It'd need to be /1024
Gabrola commented at 11:20 PM on March 21, 2014: none

@NanoAkron If you meant 1024, that still wouldn't work on really large transactions since the rule is 1,000 bytes not 1kB @luke-jr nPayFee would be nTransactionFee * 2 if nBytes was 1000 when it should be just nTransactionFee * 1
luke-jr commented at 11:25 PM on March 21, 2014: member

@Gabrola My question is why you think it should be * 1.
Gabrola commented at 11:33 PM on March 21, 2014: none

The transaction relay rule states: Base fee is: 10,000 satoshis per 1,000 bytes, transaction size rounded UP to the nearest 1,000 bytes (e.g. a 1,001 byte transaction requires a 20,000 satoshi fee). Can be changed with the -minrelaytxfee setting.

I don't think this implies that 1,000 bytes should round up to a 20,000 satoshi fee as well.
Gabrola renamed this:
~~Fix nPayFee~~
Fix nPayFee calculation
on Mar 22, 2014
BitcoinPullTester commented at 10:43 PM on March 22, 2014: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/6166d342ad6db2ffff101f70268d17e91de1d117 for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.
laanwj commented at 7:24 AM on March 31, 2014: member

Is this consistent with the transaction selection in the mining code?
Gabrola commented at 9:26 PM on March 31, 2014: none

I just checked and it's not consistent with GetMinFee() This means we'll need to change GetMinFee() first but that means we'll have to wait for the majority of miners to be using 0.9.99 before we change the client code. Should we change GetMinFee() for now or just ignore the problem considering the unlikeliness of transaction sizes being exactly divisible by 1000?
laanwj commented at 6:33 AM on April 1, 2014: member

That's not worth it IMO. The fees system is being reworked any way with floating fees.

As long as GetMinFee and the wallet are consistent, it's fine at it is.

Thanks for checking.
laanwj closed this on Apr 1, 2014
DrahtBot locked this on Sep 8, 2021

Contributors