integer division instead of implicit double conversion #14227

pull arvidn wants to merge 1 commits into bitcoin:master from arvidn:amount-integer-division changing 1 files +2 −2

arvidn commented at 7:49 PM on September 15, 2018: none

use integer division instead of double conversion and multiplication for computing amounts. This will most likely generate identical code.

My main argument in favour of this change is one of purity, that we should not rely on implicit conversion from CAmount -> double and back again. Today this implicit conversion can happen because CAmount is just a typedef to int64_t. However, I envision a future where CAmount is a proper type that does not allow suspicious implicit conversions like these.
use integer division instead of double conversion and multiplication for computing amounts be54f42e5f

in src/validation.h:58 in be54f42e5f

  52 | @@ -53,9 +53,9 @@ static const bool DEFAULT_WHITELISTFORCERELAY = true;
  53 |  /** Default for -minrelaytxfee, minimum relay fee for transactions */
  54 |  static const unsigned int DEFAULT_MIN_RELAY_TX_FEE = 1000;
  55 |  //! -maxtxfee default
  56 | -static const CAmount DEFAULT_TRANSACTION_MAXFEE = 0.1 * COIN;
  57 | +static const CAmount DEFAULT_TRANSACTION_MAXFEE = COIN / 10;
  58 |  //! Discourage users to set fees higher than this amount (in satoshis) per kB
  59 | -static const CAmount HIGH_TX_FEE_PER_KB = 0.01 * COIN;
  60 | +static const CAmount HIGH_TX_FEE_PER_KB = COIN / 100;

arvidn commented at 7:50 PM on September 15, 2018:

I imagine this could just be CENT as well

laanwj commented at 2:19 PM on September 17, 2018:

neh, please don't re-use CENT, better to leave this as a separate constant

fanquake added the label Refactoring on Sep 16, 2018
fanquake added the label Validation on Sep 16, 2018
sipa commented at 1:37 AM on September 16, 2018: member

utACK
practicalswift commented at 6:34 AM on September 16, 2018: contributor

utACK be54f42e5f309ff332d74828ae294636d77fb8ea
donaloconnor commented at 2:18 PM on September 16, 2018: contributor

utACK https://github.com/bitcoin/bitcoin/commit/be54f42e5f309ff332d74828ae294636d77fb8ea
gmaxwell commented at 3:40 PM on September 16, 2018: contributor

utACK.
promag commented at 9:14 PM on September 16, 2018: member

utACK be54f42.
Empact commented at 11:34 PM on September 16, 2018: member

utACK be54f42
laanwj merged this on Sep 17, 2018
laanwj closed this on Sep 17, 2018
laanwj referenced this in commit 72e358dca7 on Sep 17, 2018
MarcoFalke commented at 11:03 PM on September 17, 2018: member

utACK be54f42, but I'd prefer a type safe CAmount that also provides constexpr compatible constructors similar to python's Decimal('0.01').
arvidn commented at 12:11 AM on September 18, 2018: none

utACK be54f42, but I'd prefer a type safe CAmount

I'm working on it. that's how this came up
Munkybooty referenced this in commit a50eb0d587 on Jul 8, 2021
Munkybooty referenced this in commit 03333bbf45 on Jul 9, 2021
Munkybooty referenced this in commit 6f600051ae on Jul 11, 2021
Munkybooty referenced this in commit 40961394c8 on Jul 12, 2021
Munkybooty referenced this in commit 0cd799addf on Jul 12, 2021
DrahtBot locked this on Sep 8, 2021

Contributors

Labels

Refactoring Validation