Merge pull request #2441 from rnicoll/1.14.4-fees-disable-rounding

1.14.4 fees disable rounding

10 files changed, 111 insertions, 11 deletions

diff --git a/qa/pull-tester/rpc-tests.py b/qa/pull-tester/rpc-tests.py
index 71c7b5f20..c5528118e 100755
--- a/qa/pull-tester/rpc-tests.py
+++ b/qa/pull-tester/rpc-tests.py
@@ -156,6 +156,7 @@ testScripts = [
     'import-rescan.py',
     'harddustlimit.py',
     'paytxfee.py',
+    'feelimit.py',
     # While fee bumping should work in Doge, these tests depend on free transactions, which we don't support.
     # Disable until we can do a full rewrite of the tests (possibly upstream), or revise fee schedule, or something
     'bumpfee.py',
diff --git a/qa/rpc-tests/feelimit.py b/qa/rpc-tests/feelimit.py
new file mode 100644
index 000000000..aef63564c
--- /dev/null
+++ b/qa/rpc-tests/feelimit.py
@@ -0,0 +1,77 @@
+#!/usr/bin/env python3
+# Copyright (c) 2021 The Dogecoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+"""Fee limit QA test.
+# Tests nodes under fee limit, verifies fee rounding
+"""
+
+from test_framework.test_framework import BitcoinTestFramework
+from test_framework.util import *
+from decimal import Decimal
+
+class FeeLimitTest(BitcoinTestFramework):
+
+    def __init__(self):
+        super().__init__()
+        self.setup_clean_chain = True
+        self.num_nodes = 2
+
+    def setup_network(self, split=False):
+        self.nodes = []
+        self.nodes.append(start_node(0, self.options.tmpdir, []))
+        self.nodes.append(start_node(1, self.options.tmpdir, []))
+
+        connect_nodes_bi(self.nodes,0,1)
+
+        self.is_network_split=False
+        self.sync_all()
+
+    def run_test(self):
+        # mine some blocks
+        self.nodes[0].generate(101)
+        self.sync_all()
+
+        # Output address
+        addr_to = self.nodes[1].getnewaddress()
+        lower_fee_per_kb = Decimal("0.001")
+
+        # Force generating a transaction with 1.14.5-like fees by manually building the tx
+        utx = self.nodes[0].listunspent()[0]
+        inputs  = [ {'txid' : utx['txid'], 'vout' : utx['vout']}]
+        outputs = { addr_to : utx['amount'] - Decimal("1.0") }
+        rawtx   = self.nodes[0].createrawtransaction(inputs, outputs)
+
+        # Work out how big this transaction would be, then change the output to match
+        tx_size = count_bytes(self.nodes[0].signrawtransaction(rawtx)['hex'])
+        fee = lower_fee_per_kb * (tx_size + 1) / Decimal("1000")
+        outputs = { addr_to : utx['amount'] - fee }
+        rawtx   = self.nodes[0].createrawtransaction(inputs, outputs)
+        signedtx = self.nodes[0].signrawtransaction(rawtx)
+        self.nodes[0].sendrawtransaction(signedtx["hex"])
+
+        # Sync all of the nodes
+        self.sync_all()
+
+        # Check if the TX made it to node 2
+        assert_equal(self.nodes[1].getmempoolinfo()['size'], 1)
+        assert_equal(self.nodes[0].getmempoolinfo()['size'], 1)
+
+        # Force generating a transaction with fees which are too low to relay
+        too_low_fee = lower_fee_per_kb * (tx_size - 4) / Decimal("1000")
+        utx = self.nodes[0].listunspent()[0]
+        inputs  = [ {'txid' : utx['txid'], 'vout' : utx['vout']}]
+        outputs = { self.nodes[1].getnewaddress() : utx['amount'] - too_low_fee }
+        rawtx   = self.nodes[0].createrawtransaction(inputs, outputs)
+        signedtx = self.nodes[0].signrawtransaction(rawtx)
+        self.nodes[0].sendrawtransaction(signedtx["hex"])
+
+        # wait 10 seconds to sync mempools
+        time.sleep(10)
+
+        # Check the TX did not relay, so is only on node 0
+        assert_equal(self.nodes[1].getmempoolinfo()['size'], 1)
+        assert_equal(self.nodes[0].getmempoolinfo()['size'], 2)
+
+if __name__ == '__main__':
+    FeeLimitTest().main()
diff --git a/qa/rpc-tests/test_framework/util.py b/qa/rpc-tests/test_framework/util.py
index 4e4011b3a..a9723df07 100644
--- a/qa/rpc-tests/test_framework/util.py
+++ b/qa/rpc-tests/test_framework/util.py
@@ -509,7 +509,7 @@ def random_transaction(nodes, amount, min_fee, fee_increment, fee_variants):
 
 def assert_fee_amount(fee, tx_size, fee_per_kB):
     """Assert the fee was in range"""
-    target_fee = round_tx_size(tx_size) * fee_per_kB / 1000
+    target_fee = fee_per_kB / 1000
     if fee < target_fee:
         raise AssertionError("Fee of %s BTC too low! (Should be %s BTC)"%(str(fee), str(target_fee)))
     # allow the wallet's estimation to be at most 2 bytes off
diff --git a/src/amount.cpp b/src/amount.cpp
index 176c0be85..aa6bf943e 100644
--- a/src/amount.cpp
+++ b/src/amount.cpp
@@ -30,6 +30,16 @@ CAmount CFeeRate::GetFee(size_t nBytes_) const
         nSize = nSize + 1000 - (nSize % 1000);
     }
 
+    return GetRelayFee(nSize);
+}
+
+// Dogecoin: Specifically for 1.14.4 we lower accepted relay fees by removing rounding,
+// in 1.14.5 we should unify the GetFee() functions again.
+CAmount CFeeRate::GetRelayFee(size_t nBytes_) const
+{
+    assert(nBytes_ <= uint64_t(std::numeric_limits<int64_t>::max()));
+    int64_t nSize = int64_t(nBytes_);
+
     CAmount nFee = nSatoshisPerK * nSize / 1000;
 
     if (nFee == 0 && nSize != 0) {
diff --git a/src/amount.h b/src/amount.h
index e6e503905..02d61fc9b 100644
--- a/src/amount.h
+++ b/src/amount.h
@@ -46,10 +46,14 @@ public:
     CFeeRate(const CAmount& nFeePaid, size_t nBytes);
     CFeeRate(const CFeeRate& other) { nSatoshisPerK = other.nSatoshisPerK; }
     /**
-     * Return the fee in satoshis for the given size in bytes.
+     * Return the wallet fee in koinus for the given size in bytes.
      */
     CAmount GetFee(size_t nBytes) const;
     /**
+     * Return the relay fee in koinus for the given size in bytes.
+     */
+    CAmount GetRelayFee(size_t nBytes) const;
+    /**
      * Return the fee in satoshis for a size of 1000 bytes
      */
     CAmount GetFeePerK() const { return GetFee(1000); }
diff --git a/src/dogecoin-fees.cpp b/src/dogecoin-fees.cpp
index c0542eb3c..890908ad6 100644
--- a/src/dogecoin-fees.cpp
+++ b/src/dogecoin-fees.cpp
@@ -46,7 +46,7 @@ CAmount GetDogecoinMinRelayFee(const CTransaction& tx, unsigned int nBytes, bool
             return 0;
     }
 
-    CAmount nMinFee = ::minRelayTxFeeRate.GetFee(nBytes);
+    CAmount nMinFee = ::minRelayTxFeeRate.GetRelayFee(nBytes);
     nMinFee += GetDogecoinDustFee(tx.vout, ::minRelayTxFeeRate);
 
     if (fAllowFree)
diff --git a/src/miner.cpp b/src/miner.cpp
index 55a90b86f..c4ec82d6a 100644
--- a/src/miner.cpp
+++ b/src/miner.cpp
@@ -490,7 +490,7 @@ void BlockAssembler::addPackageTxs(int &nPackagesSelected, int &nDescendantsUpda
             packageSigOpsCost = modit->nSigOpCostWithAncestors;
         }
 
-        if (packageFees < blockMinFeeRate.GetFee(packageSize)) {
+        if (packageFees < blockMinFeeRate.GetRelayFee(packageSize)) {
             // Everything else we might consider has a lower fee rate
             return;
         }
diff --git a/src/test/amount_tests.cpp b/src/test/amount_tests.cpp
index 47e73ed38..80a6c3a6f 100644
--- a/src/test/amount_tests.cpp
+++ b/src/test/amount_tests.cpp
@@ -19,7 +19,7 @@ BOOST_AUTO_TEST_CASE(GetFeeTest)
     BOOST_CHECK_EQUAL(feeRate.GetFee(1e5), 0);
 
     feeRate = CFeeRate(1000);
-    // Must always just return the arg
+    // Wallet fees are rounded up
     BOOST_CHECK_EQUAL(feeRate.GetFee(0), 0);
     BOOST_CHECK_EQUAL(feeRate.GetFee(1), 1000);
     BOOST_CHECK_EQUAL(feeRate.GetFee(121), 1000);
@@ -27,6 +27,14 @@ BOOST_AUTO_TEST_CASE(GetFeeTest)
     BOOST_CHECK_EQUAL(feeRate.GetFee(1e3), 1000);
     BOOST_CHECK_EQUAL(feeRate.GetFee(9e3), 9000);
 
+    // Relay fees must always just return the arg
+    BOOST_CHECK_EQUAL(feeRate.GetRelayFee(0), 0);
+    BOOST_CHECK_EQUAL(feeRate.GetRelayFee(1), 1);
+    BOOST_CHECK_EQUAL(feeRate.GetRelayFee(121), 121);
+    BOOST_CHECK_EQUAL(feeRate.GetRelayFee(999), 999);
+    BOOST_CHECK_EQUAL(feeRate.GetRelayFee(1e3), 1e3);
+    BOOST_CHECK_EQUAL(feeRate.GetRelayFee(9e3), 9e3);
+
     feeRate = CFeeRate(-1000);
     // Must always just return -1 * arg
     BOOST_CHECK_EQUAL(feeRate.GetFee(0), 0);
diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp
index ba06a2ab7..3995d3096 100644
--- a/src/test/miner_tests.cpp
+++ b/src/test/miner_tests.cpp
@@ -121,7 +121,7 @@ void TestPackageSelection(const CChainParams& chainparams, CScript scriptPubKey,
 
     // Calculate a fee on child transaction that will put the package just
     // below the block min tx fee (assuming 1 child tx of the same size).
-    CAmount feeToUse = blockMinFeeRate.GetFee(2*freeTxSize) - 1;
+    CAmount feeToUse = blockMinFeeRate.GetRelayFee(2*freeTxSize) - 1;
 
     tx.vin[0].prevout.hash = hashFreeTx;
     tx.vout[0].nValue = 5000000000LL - 1000 - 50000 - feeToUse;
@@ -158,7 +158,7 @@ void TestPackageSelection(const CChainParams& chainparams, CScript scriptPubKey,
     // This tx can't be mined by itself
     tx.vin[0].prevout.hash = hashFreeTx2;
     tx.vout.resize(1);
-    feeToUse = blockMinFeeRate.GetFee(freeTxSize);
+    feeToUse = blockMinFeeRate.GetRelayFee(freeTxSize);
     tx.vout[0].nValue = 5000000000LL - 100000000 - feeToUse;
     uint256 hashLowFeeTx2 = tx.GetHash();
     mempool.addUnchecked(hashLowFeeTx2, entry.Fee(feeToUse).SpendsCoinbase(false).FromTx(tx));
diff --git a/src/validation.cpp b/src/validation.cpp
index 8fc4ef6c7..9a307c9d0 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -786,10 +786,10 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const C
             return state.DoS(0, false, REJECT_NONSTANDARD, "bad-txns-too-many-sigops", false,
                 strprintf("%d", nSigOpsCost));
 
-        CAmount mempoolRejectFee = pool.GetMinFee(GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000).GetFee(nSize);
+        CAmount mempoolRejectFee = pool.GetMinFee(GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000).GetRelayFee(nSize);
         if (mempoolRejectFee > 0 && nModifiedFees < mempoolRejectFee) {
             return state.DoS(0, false, REJECT_INSUFFICIENTFEE, "mempool min fee not met", false, strprintf("%d < %d", nFees, mempoolRejectFee));
-        } else if (GetBoolArg("-relaypriority", DEFAULT_RELAYPRIORITY) && nModifiedFees < ::minRelayTxFeeRate.GetFee(nSize) && !AllowFree(entry.GetPriority(chainActive.Height() + 1))) {
+        } else if (GetBoolArg("-relaypriority", DEFAULT_RELAYPRIORITY) && nModifiedFees < ::minRelayTxFeeRate.GetRelayFee(nSize) && !AllowFree(entry.GetPriority(chainActive.Height() + 1))) {
             // Require that free transactions have sufficient priority to be mined in the next block.
             return state.DoS(0, false, REJECT_INSUFFICIENTFEE, "insufficient priority");
         }
@@ -966,14 +966,14 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const C
             // Finally in addition to paying more fees than the conflicts the
             // new transaction must pay for its own bandwidth.
             CAmount nDeltaFees = nModifiedFees - nConflictingFees;
-            if (nDeltaFees < ::incrementalRelayFee.GetFee(nSize))
+            if (nDeltaFees < ::incrementalRelayFee.GetRelayFee(nSize))
             {
                 return state.DoS(0, false,
                         REJECT_INSUFFICIENTFEE, "insufficient fee", false,
                         strprintf("rejecting replacement %s, not enough additional fees to relay; %s < %s",
                               hash.ToString(),
                               FormatMoney(nDeltaFees),
-                              FormatMoney(::incrementalRelayFee.GetFee(nSize))));
+                              FormatMoney(::incrementalRelayFee.GetRelayFee(nSize))));
             }
         }