Interpret fee rates in reference fee unit (#21)

* Introduce CValue for distinguishing amounts from values * Remove random character * Define insertion operator for CValue * Move CValue to separate module * Remove consensus changes * Remove CValue usage for fee rates, restrict to mempool * More cleanup * Use CValue for descendant and ancestor fee updates * More mempool fixes * Remove unused include * Fix variable shadowing * More conversions * Factor in fee asset when validating package fees * More conversions * Convert dust relay fee from RFU * More cleanup * Cleanup * Don't factor in asset in when calculating dust threshold * Convert package fees * Return CValue from CalculateExchangeValue * Use CValue for reverse exchange rate conversion * Update method documentation * Rename currency conversion methods * Fix typo * Add justification for RFU to comments * Use accessor instead of public instance variable unpacking CValue * Use GetValue() in fee rate conversion * Only apply dust check to outputs denominated in same asset as fee * Interpret coin selection effective feerate in RFU * Change GetModifiedFee() method to use RFU for summing with ancestor and descendant transactions * Add functional test for "fee_rate" parameter in fundrawtransaction RPC and -mintxfee node configuration parameter * Normalize fees to RFU during fee estimation * Lint fixes * More linting fixes * More linting * Apply method renams to fee amount conversions * Add any asset fee rates test to test runner * More amount conversion fixes * Add compiler switch for currency constants * Add new configuration flag to documentation * Apply fixes to fee estimation functional test from ElementsProject/elements#1298 * Add tests for paytxfee parameter * Cleanup * Fix compiler flag * Add tests for blockmintxfee * Remove whitespace * Add comments clarifying what's being demonstrated * Remove nFeeValue, recompute as needed * Revert "Remove nFeeValue, recompute as needed" This reverts commit ab3a67b. * Move No Coin configuration documentation to separate line * Add constant for full name of currency atom * Fix constant reference * Fix typo
SequentiaSEQ · May 3, 2024 · 5f8dadf · 5f8dadf
1 parent 67c9bc6
commit 5f8dadf
Show file tree

Hide file tree

Showing 25 changed files with 384 additions and 86 deletions.
diff --git a/README.md b/README.md
@@ -36,6 +36,10 @@ To speed up the build if not necessary, disable bench and tests in configure:
 ```bash
 ./configure --without-gui --without-natpmp --without-miniupnpc --disable-bench --disable-tests
 ```
+To configure RPC documentation to denominate fee rates using RFU and rfa instead of BTC and sat:
+```bash
+./configure --enable-any-asset-fees
+```
 
 Modes
 -----
@@ -106,4 +110,3 @@ https://github.com/ElementsProject/elementsproject.github.io
 Secure Reporting
 ------------------
 See [our vulnerability reporting guide](SECURITY.md)
-
diff --git a/configure.ac b/configure.ac
@@ -267,13 +267,24 @@ AC_ARG_ENABLE([liquid],
   [liquid_build=yes],
   [liquid_build=no])
 
+AC_ARG_ENABLE([any_asset_fees],
+  [AS_HELP_STRING([--enable-any-asset-fees],
+  [Enable build that uses constants for any asset fees feature])],
+  [any_asset_fees=yes],
+  [any_asset_fees=no])
+
 if test "$use_asm" = "yes"; then
   AC_DEFINE([USE_ASM], [1], [Define this symbol to build in assembly routines])
 fi
 
 if test "$liquid_build" = "yes"; then
   AC_DEFINE(LIQUID, 1, [Define this symbol for Liquid builds])
 fi
+
+if test "$any_asset_fees" = "yes"; then
+  AC_DEFINE(ANY_ASSET_FEES, 1, [Define this symbol to modify constants for any asset fees feature])
+fi
+
 AC_ARG_ENABLE([zmq],
   [AS_HELP_STRING([--disable-zmq],
   [disable ZMQ notifications])],
@@ -1841,6 +1852,7 @@ AM_CONDITIONAL([WORDS_BIGENDIAN], [test "$ac_cv_c_bigendian" = "yes"])
 AM_CONDITIONAL([USE_NATPMP], [test "$use_natpmp" = "yes"])
 AM_CONDITIONAL([USE_UPNP], [test "$use_upnp" = "yes"])
 AM_CONDITIONAL([LIQUID], [test "$liquid_build" = "yes"])
+AM_CONDITIONAL([ANY_ASSET_FEES], [test "$any_asset_fees" = "yes"])
 
 dnl for minisketch
 AM_CONDITIONAL([ENABLE_CLMUL], [test "$enable_clmul" = "yes"])
@@ -1995,6 +2007,7 @@ echo "  with upnp       = $use_upnp"
 echo "  with natpmp     = $use_natpmp"
 echo "  use asm         = $use_asm"
 echo "  liquid_build    = $liquid_build"
+echo "  any_asset_fees  = $any_asset_fees"
 echo "  USDT tracing    = $use_usdt"
 echo "  sanitizers      = $use_sanitizers"
 echo "  debug enabled   = $enable_debug"

diff --git a/src/Makefile.am b/src/Makefile.am
@@ -213,6 +213,7 @@ BITCOIN_CORE_H = \
   policy/policy.h \
   policy/rbf.h \
   policy/settings.h \
+  policy/value.h \
   pow.h \
   primitives/pak.h \
   protocol.h \

diff --git a/src/exchangerates.cpp b/src/exchangerates.cpp
@@ -1,24 +1,40 @@
 #include <assetsdir.h>
 #include <exchangerates.h>
 #include <policy/policy.h>
+#include <policy/value.h>
 #include <util/settings.h>
 #include <util/system.h>
 #include <univalue.h>
 
 #include <fstream>
 
-CAmount ExchangeRateMap::CalculateExchangeValue(const CAmount& amount, const CAsset& asset) {
+CValue ExchangeRateMap::ConvertAmountToValue(const CAmount& amount, const CAsset& asset) {
+    int64_t int64_max = std::numeric_limits<int64_t>::max();
     auto it = this->find(asset);
     if (it == this->end()) {
-        return 0;
+        return CValue(0);
     }
     auto scaled_value = it->second.m_scaled_value;
-    __uint128_t value = ((__uint128_t)amount * (__uint128_t)scaled_value) / (__uint128_t)exchange_rate_scale;
+    __uint128_t result = ((__uint128_t)amount * (__uint128_t)scaled_value) / (__uint128_t)exchange_rate_scale;
+    if (result > int64_max) {
+        return CValue(int64_max);
+    } else {
+        return CValue((int64_t) result);
+    }
+}
+
+CAmount ExchangeRateMap::ConvertValueToAmount(const CValue& value, const CAsset& asset) {
     int64_t int64_max = std::numeric_limits<int64_t>::max();
-    if (value > int64_max) {
+    auto it = this->find(asset);
+    if (it == this->end()) {
+        return int64_max;
+    }
+    auto scaled_value = it->second.m_scaled_value;
+    __uint128_t result = ((__uint128_t)value.GetValue() * (__uint128_t)exchange_rate_scale) / (__uint128_t)scaled_value;
+    if (result > int64_max) {
         return int64_max;
     } else {
-        return (int64_t) value;
+        return (int64_t) result;
     }
 }
 

diff --git a/src/exchangerates.h b/src/exchangerates.h
@@ -3,9 +3,10 @@
 
 #include <fs.h>
 #include <policy/policy.h>
+#include <policy/value.h>
 #include <univalue.h>
 
-constexpr const CAmount exchange_rate_scale = COIN;
+constexpr const CAmount exchange_rate_scale = COIN; // 100,000,000
 const std::string exchange_rates_config_file = "exchangerates.json";
 
 class CAssetExchangeRate
@@ -33,15 +34,24 @@ class ExchangeRateMap : public std::map<CAsset, CAssetExchangeRate>
     }
 
     /**
-     * Calculate the exchange value
+     * Convert an amount denominated in some asset to the node's RFU (reference fee unit)
      *
-     * @param[in]   amount       Corresponds to CTxMemPoolEntry.nFeeAmount
+     * @param[in]   amount       Corresponds to CTxMemPoolEntry.nFee
      * @param[in]   asset        Corresponds to CTxMemPoolEntry.nFeeAsset
-     * @return the value at current exchange rate. Corresponds to CTxMemPoolEntry.nFee
+     * @return the value at current exchange rate. Corresponds to CTxMemPoolEntry.nFeeValue
      */
-    CAmount CalculateExchangeValue(const CAmount& amount, const CAsset& asset);
+    CValue ConvertAmountToValue(const CAmount& amount, const CAsset& asset);
 
-/**
+    /**
+     * Convert an amount denominated in the node's RFU (reference fee unit) into some asset
+     *
+     * @param[in]   value        Corresponds to CTxMemPoolEntry.nFeeValue
+     * @param[in]   asset        Corresponds to CTxMemPoolEntry.nFeeAsset
+     * @return the amount at current exchange rate. Corresponds to CTxMemPoolEntry.nFee
+     */
+    CAmount ConvertValueToAmount(const CValue& value, const CAsset& asset);
+
+    /**
      * Load the exchange rate map from the default JSON config file in <datadir>/exchangerates.json.
      *
      * @param[in]   errors        Vector for storing error messages, if there are any.

diff --git a/src/node/miner.cpp b/src/node/miner.cpp
@@ -452,15 +452,15 @@ void BlockAssembler::addPackageTxs(int& nPackagesSelected, int& nDescendantsUpda
         assert(!inBlock.count(iter));
 
         uint64_t packageSize = iter->GetSizeWithAncestors();
-        CAmount packageFees = iter->GetModFeesWithAncestors();
+        CValue packageFees = iter->GetModFeesWithAncestors();
         int64_t packageSigOpsCost = iter->GetSigOpCostWithAncestors();
         if (fUsingModified) {
             packageSize = modit->nSizeWithAncestors;
             packageFees = modit->nModFeesWithAncestors;
             packageSigOpsCost = modit->nSigOpCostWithAncestors;
         }
 
-        if (packageFees < blockMinFeeRate.GetFee(packageSize)) {
+        if (packageFees.GetValue() < blockMinFeeRate.GetFee(packageSize)) {
             // Everything else we might consider has a lower fee rate
             return;
         }

diff --git a/src/node/miner.h b/src/node/miner.h
@@ -47,13 +47,13 @@ struct CTxMemPoolModifiedEntry {
 
     int64_t GetModifiedFee() const { return iter->GetModifiedFee(); }
     uint64_t GetSizeWithAncestors() const { return nSizeWithAncestors; }
-    CAmount GetModFeesWithAncestors() const { return nModFeesWithAncestors; }
+    CValue GetModFeesWithAncestors() const { return nModFeesWithAncestors; }
     size_t GetTxSize() const { return iter->GetTxSize(); }
     const CTransaction& GetTx() const { return iter->GetTx(); }
 
     CTxMemPool::txiter iter;
     uint64_t nSizeWithAncestors;
-    CAmount nModFeesWithAncestors;
+    CValue nModFeesWithAncestors;
     int64_t nSigOpCostWithAncestors;
 };
 
@@ -116,7 +116,7 @@ struct update_for_parent_inclusion
 
     void operator() (CTxMemPoolModifiedEntry &e)
     {
-        e.nModFeesWithAncestors -= iter->GetFee();
+        e.nModFeesWithAncestors -= iter->GetFeeValue();
         e.nSizeWithAncestors -= iter->GetTxSize();
         e.nSigOpCostWithAncestors -= iter->GetSigOpCost();
     }

diff --git a/src/node/transaction.cpp b/src/node/transaction.cpp
@@ -76,8 +76,8 @@ TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef t
                 if (result.m_result_type != MempoolAcceptResult::ResultType::VALID) {
                     return HandleATMPError(result.m_state, err_string);
                 } else if (g_con_any_asset_fees) {
-                    CAmount mBaseFeesValue = ExchangeRateMap::GetInstance().CalculateExchangeValue(result.m_base_fees.value(), tx->GetFeeAsset(::policyAsset));
-                    if (mBaseFeesValue > max_tx_fee) {
+                    CValue mBaseFeesValue = ExchangeRateMap::GetInstance().ConvertAmountToValue(result.m_base_fees.value(), tx->GetFeeAsset(::policyAsset));
+                    if (mBaseFeesValue.GetValue() > max_tx_fee) {
                         return TransactionError::MAX_FEE_EXCEEDED;
                     }
                 } else if (result.m_base_fees.value() > max_tx_fee) {

diff --git a/src/policy/feerate.cpp b/src/policy/feerate.cpp
@@ -3,7 +3,11 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
+#include <asset.h>
+#include <exchangerates.h>
 #include <policy/feerate.h>
+#include <policy/value.h>
+#include <primitives/transaction.h>
 
 #include <tinyformat.h>
 
@@ -36,6 +40,15 @@ CAmount CFeeRate::GetFee(uint32_t num_bytes) const
     return nFee;
 }
 
+CAmount CFeeRate::GetFee(uint32_t num_bytes, const CAsset& asset) const
+{
+    CValue nFee = CValue(this->GetFee(num_bytes));
+    if (g_con_any_asset_fees) {
+        nFee = ExchangeRateMap::GetInstance().ConvertValueToAmount(nFee, asset); 
+    }
+    return nFee.GetValue();
+}
+
 std::string CFeeRate::ToString(const FeeEstimateMode& fee_estimate_mode) const
 {
     switch (fee_estimate_mode) {

diff --git a/src/policy/feerate.h b/src/policy/feerate.h
@@ -6,13 +6,21 @@
 #ifndef BITCOIN_POLICY_FEERATE_H
 #define BITCOIN_POLICY_FEERATE_H
 
+#include <asset.h>
 #include <consensus/amount.h>
 #include <serialize.h>
 
 #include <string>
 
+#ifdef ANY_ASSET_FEES
+const std::string CURRENCY_UNIT = "RFU"; // One formatted unit (reference fee unit)
+const std::string CURRENCY_ATOM = "rfa"; // One indivisible minimum value unit (reference fee atom)
+const std::string CURRENCY_ATOM_FULL = "reference fee atom";
+#else
 const std::string CURRENCY_UNIT = "BTC"; // One formatted unit
 const std::string CURRENCY_ATOM = "sat"; // One indivisible minimum value unit
+const std::string CURRENCY_ATOM_FULL = "satoshi";
+#endif
 
 /* Used to determine type of fee estimation requested */
 enum class FeeEstimateMode {
@@ -56,6 +64,12 @@ class CFeeRate
      */
     CAmount GetFee(uint32_t num_bytes) const;
 
+    /**
+     * Return the fee in denominations of the fee asset for the given
+     * vsize in vbytes.
+     */
+    CAmount GetFee(uint32_t num_bytes, const CAsset& asset) const;
+
     /**
      * Return the fee in satoshis for a vsize of 1000 vbytes
      */

diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp
@@ -569,7 +569,7 @@ void CBlockPolicyEstimator::processTransaction(const CTxMemPoolEntry& entry, boo
     trackedTxs++;
 
     // Feerates are stored and reported as BTC-per-kb:
-    CFeeRate feeRate(entry.GetFee(), entry.GetTxSize());
+    CFeeRate feeRate(entry.GetFeeValue().GetValue(), entry.GetTxSize());
 
     mapMemPoolTxs[hash].blockHeight = txHeight;
     unsigned int bucketIndex = feeStats->NewTx(txHeight, (double)feeRate.GetFeePerK());

diff --git a/src/policy/policy.cpp b/src/policy/policy.cpp
@@ -53,7 +53,7 @@ CAmount GetDustThreshold(const CTxOut& txout, const CFeeRate& dustRelayFeeIn)
         nSize += (32 + 4 + 1 + 107 + 4); // the 148 mentioned above
     }
 
-    return dustRelayFeeIn.GetFee(nSize);
+    return dustRelayFeeIn.GetFee(nSize, txout.nAsset.GetAsset());
 }
 
 bool IsDust(const CTxOut& txout, const CFeeRate& dustRelayFeeIn)
@@ -146,7 +146,7 @@ bool IsStandardTx(const CTransaction& tx, bool permit_bare_multisig, const CFeeR
         } else if ((whichType == TxoutType::MULTISIG) && (!permit_bare_multisig)) {
             reason = "bare-multisig";
             return false;
-        } else if ((txout.nAsset.IsExplicit() && txout.nAsset.GetAsset() == policyAsset) && IsDust(txout, dust_relay_fee)) {
+        } else if ((txout.nAsset.IsExplicit() && txout.nAsset.GetAsset() == tx.GetFeeAsset(::policyAsset)) && IsDust(txout, dust_relay_fee)) {
             reason = "dust";
             return false;
         }

diff --git a/src/policy/value.h b/src/policy/value.h
@@ -0,0 +1,65 @@
+// Copyright (c) 2009-2010 Satoshi Nakamoto
+// Copyright (c) 2009-2021 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#ifndef BITCOIN_POLICY_VALUE_H
+#define BITCOIN_POLICY_VALUE_H
+
+#include <cstdint>
+
+/** ELEMENTS: Amount denominated in the node's RFU (reference fee unit). Used only
+ *  when con_any_asset_fees is enabled in order to distinguish from amounts in an 
+ *  actual asset. RFU is needed to make amounts comparable when sorting transactions 
+ *  in the mempool, as well as for fee estimation and subsequent validation of those 
+ *  fees according to various limits (e.g., mintxfee, paytsxfee, blockmintxfee, 
+ *  incrementalrelaytxfee, etc.).
+ */
+struct CValue 
+{ 
+    private: 
+        int64_t value;
+
+    public:
+        CValue(): value(0) {}
+        CValue(const int64_t value): value(value) {}
+
+        int64_t GetValue() const
+        {
+            return value;
+        }
+
+        CValue operator -(const CValue& operand)
+        {
+            return CValue(value - operand.value);
+        }
+
+        CValue operator -=(const CValue& operand)
+        {
+            value -= operand.value;
+            return *this;
+        }
+
+        CValue operator +(const CValue& operand)
+        {
+            return CValue(value + operand.value);
+        }
+
+        CValue operator +=(const CValue& operand)
+        {
+            value += operand.value;
+            return *this;
+        }
+
+        bool operator ==(const CValue& operand)
+        {
+            return value == operand.value;
+        }
+
+        bool operator !=(const CValue& operand)
+        {
+            return value != operand.value;
+        }
+};
+
+#endif // BITCOIN_POLICY_VALUE_H