Refactor fee initialization and configuration (XRPLF#4319):

* Create the FeeSettings object in genesis ledger. * Initialize with default values from the config. Removes the need to pass a Config down into the Ledger initialization functions, including setup(). * Drop the undocumented fee config settings in favor of the [voting] section. * Because old mainnet ledgers (prior to 562177 - yes, I looked it up), don't have FeeSettings, some of the other ctors will default them to the config values before setup() tries to load the object. * Update default Config fee values to match mainnet. * Fix unit tests: * Updated fees: Some tests are converted to use computed values of fee object, but the default Env config was also updated to fix the rest. * Unit tests that check the structure of the ledger have updated hashes and counts.
ximinez · Mar 27, 2023 · 42c0ab0 · 42c0ab0
1 parent 9fcb28a
commit 42c0ab0
Show file tree

Hide file tree

Showing 34 changed files with 289 additions and 199 deletions.
diff --git a/cfg/rippled-example.cfg b/cfg/rippled-example.cfg
@@ -1441,7 +1441,7 @@
 #       default. Don't change this without understanding the consequences.
 #
 #       Example:
-#           account_reserve = 20000000   # 20 XRP
+#           account_reserve = 10000000   # 10 XRP
 #
 #   owner_reserve = <drops>
 #
@@ -1453,7 +1453,7 @@
 #       default. Don't change this without understanding the consequences.
 #
 #       Example:
-#           owner_reserve = 5000000      # 5 XRP
+#           owner_reserve = 2000000      # 2 XRP
 #
 #-------------------------------------------------------------------------------
 #

diff --git a/cfg/rippled-reporting.cfg b/cfg/rippled-reporting.cfg
@@ -1401,7 +1401,7 @@
 #       default. Don't change this without understanding the consequences.
 #
 #       Example:
-#           account_reserve = 20000000   # 20 XRP
+#           account_reserve = 10000000   # 10 XRP
 #
 #   owner_reserve = <drops>
 #
@@ -1413,7 +1413,7 @@
 #       default. Don't change this without understanding the consequences.
 #
 #       Example:
-#           owner_reserve = 5000000      # 5 XRP
+#           owner_reserve = 2000000      # 2 XRP
 #
 #-------------------------------------------------------------------------------
 #

diff --git a/src/ripple/app/ledger/Ledger.cpp b/src/ripple/app/ledger/Ledger.cpp
@@ -209,8 +209,34 @@ Ledger::Ledger(
         rawInsert(sle);
     }
 
+    {
+        auto sle = std::make_shared<SLE>(keylet::fees());
+        // Whether featureXRPFees is supported will depend on startup options.
+        if (std::find(amendments.begin(), amendments.end(), featureXRPFees) !=
+            amendments.end())
+        {
+            sle->at(sfBaseFeeDrops) = config.FEES.reference_fee;
+            sle->at(sfReserveBaseDrops) = config.FEES.account_reserve;
+            sle->at(sfReserveIncrementDrops) = config.FEES.owner_reserve;
+        }
+        else
+        {
+            if (auto const f =
+                    config.FEES.reference_fee.dropsAs<std::uint64_t>())
+                sle->at(sfBaseFee) = *f;
+            if (auto const f =
+                    config.FEES.account_reserve.dropsAs<std::uint32_t>())
+                sle->at(sfReserveBase) = *f;
+            if (auto const f =
+                    config.FEES.owner_reserve.dropsAs<std::uint32_t>())
+                sle->at(sfReserveIncrement) = *f;
+            sle->at(sfReferenceFeeUnits) = Config::FEE_UNITS_DEPRECATED;
+        }
+        rawInsert(sle);
+    }
+
     stateMap_->flushDirty(hotACCOUNT_NODE);
-    setImmutable(config);
+    setImmutable();
 }
 
 Ledger::Ledger(
@@ -259,7 +285,8 @@ Ledger::Ledger(
     txMap_->setImmutable();
     stateMap_->setImmutable();
 
-    if (!setup(config))
+    defaultFees(config);
+    if (!setup())
         loaded = false;
 
     if (!loaded)
@@ -329,11 +356,12 @@ Ledger::Ledger(
     info_.seq = ledgerSeq;
     info_.closeTime = closeTime;
     info_.closeTimeResolution = ledgerDefaultTimeResolution;
-    setup(config);
+    defaultFees(config);
+    setup();
 }
 
 void
-Ledger::setImmutable(Config const& config, bool rehash)
+Ledger::setImmutable(bool rehash)
 {
     // Force update, since this is the only
     // place the hash transitions to valid
@@ -349,23 +377,22 @@ Ledger::setImmutable(Config const& config, bool rehash)
     mImmutable = true;
     txMap_->setImmutable();
     stateMap_->setImmutable();
-    setup(config);
+    setup();
 }
 
 void
 Ledger::setAccepted(
     NetClock::time_point closeTime,
     NetClock::duration closeResolution,
-    bool correctCloseTime,
-    Config const& config)
+    bool correctCloseTime)
 {
     // Used when we witnessed the consensus.
     assert(!open());
 
     info_.closeTime = closeTime;
     info_.closeTimeResolution = closeResolution;
     info_.closeFlags = correctCloseTime ? 0 : sLCF_NoConsensusTime;
-    setImmutable(config);
+    setImmutable();
 }
 
 bool
@@ -587,13 +614,13 @@ Ledger::rawTxInsertWithHash(
 }
 
 bool
-Ledger::setup(Config const& config)
+Ledger::setup()
 {
     bool ret = true;
 
     try
     {
-        rules_ = makeRulesGivenLedger(*this, config.features);
+        rules_ = makeRulesGivenLedger(*this, rules_);
     }
     catch (SHAMapMissingNode const&)
     {
@@ -604,10 +631,6 @@ Ledger::setup(Config const& config)
         Rethrow();
     }
 
-    fees_.base = config.FEE_DEFAULT;
-    fees_.reserve = config.FEE_ACCOUNT_RESERVE;
-    fees_.increment = config.FEE_OWNER_RESERVE;
-
     try
     {
         if (auto const sle = read(keylet::fees()))
@@ -667,6 +690,18 @@ Ledger::setup(Config const& config)
     return ret;
 }
 
+void
+Ledger::defaultFees(Config const& config)
+{
+    assert(fees_.base == 0 && fees_.reserve == 0 && fees_.increment == 0);
+    if (fees_.base == 0)
+        fees_.base = config.FEES.reference_fee;
+    if (fees_.reserve == 0)
+        fees_.reserve = config.FEES.account_reserve;
+    if (fees_.increment == 0)
+        fees_.increment = config.FEES.owner_reserve;
+}
+
 std::shared_ptr<SLE>
 Ledger::peek(Keylet const& k) const
 {
@@ -1071,7 +1106,10 @@ finishLoadByIndexOrHash(
     if (!ledger)
         return;
 
-    ledger->setImmutable(config);
+    assert(
+        ledger->info().seq < XRP_LEDGER_EARLIEST_FEES ||
+        ledger->read(keylet::fees()));
+    ledger->setImmutable();
 
     JLOG(j.trace()) << "Loaded ledger: " << to_string(ledger->info().hash);
 

diff --git a/src/ripple/app/ledger/Ledger.h b/src/ripple/app/ledger/Ledger.h
@@ -266,11 +266,10 @@ class Ledger final : public std::enable_shared_from_this<Ledger>,
     setAccepted(
         NetClock::time_point closeTime,
         NetClock::duration closeResolution,
-        bool correctCloseTime,
-        Config const& config);
+        bool correctCloseTime);
 
     void
-    setImmutable(Config const& config, bool rehash = true);
+    setImmutable(bool rehash = true);
 
     bool
     isImmutable() const
@@ -395,7 +394,10 @@ class Ledger final : public std::enable_shared_from_this<Ledger>,
     class txs_iter_impl;
 
     bool
-    setup(Config const& config);
+    setup();
+
+    void
+    defaultFees(Config const& config);
 
     bool mImmutable;
 

diff --git a/src/ripple/app/ledger/impl/BuildLedger.cpp b/src/ripple/app/ledger/impl/BuildLedger.cpp
@@ -75,8 +75,10 @@ buildLedgerImpl(
     built->unshare();
 
     // Accept ledger
-    built->setAccepted(
-        closeTime, closeResolution, closeTimeCorrect, app.config());
+    assert(
+        built->info().seq < XRP_LEDGER_EARLIEST_FEES ||
+        built->read(keylet::fees()));
+    built->setAccepted(closeTime, closeResolution, closeTimeCorrect);
 
     return built;
 }

diff --git a/src/ripple/app/ledger/impl/InboundLedger.cpp b/src/ripple/app/ledger/impl/InboundLedger.cpp
@@ -155,7 +155,10 @@ InboundLedger::init(ScopedLockType& collectionLock)
 
     JLOG(journal_.debug()) << "Acquiring ledger we already have in "
                            << " local store. " << hash_;
-    mLedger->setImmutable(app_.config());
+    assert(
+        mLedger->info().seq < XRP_LEDGER_EARLIEST_FEES ||
+        mLedger->read(keylet::fees()));
+    mLedger->setImmutable();
 
     if (mReason == Reason::HISTORY || mReason == Reason::SHARD)
         return;
@@ -416,7 +419,10 @@ InboundLedger::tryDB(NodeStore::Database& srcDB)
     {
         JLOG(journal_.debug()) << "Had everything locally";
         complete_ = true;
-        mLedger->setImmutable(app_.config());
+        assert(
+            mLedger->info().seq < XRP_LEDGER_EARLIEST_FEES ||
+            mLedger->read(keylet::fees()));
+        mLedger->setImmutable();
     }
 }
 
@@ -513,7 +519,10 @@ InboundLedger::done()
 
     if (complete_ && !failed_ && mLedger)
     {
-        mLedger->setImmutable(app_.config());
+        assert(
+            mLedger->info().seq < XRP_LEDGER_EARLIEST_FEES ||
+            mLedger->read(keylet::fees()));
+        mLedger->setImmutable();
         switch (mReason)
         {
             case Reason::SHARD:

diff --git a/src/ripple/app/main/Application.cpp b/src/ripple/app/main/Application.cpp
@@ -1699,7 +1699,7 @@ ApplicationImp::fdRequired() const
 void
 ApplicationImp::startGenesisLedger()
 {
-    std::vector<uint256> initialAmendments =
+    std::vector<uint256> const initialAmendments =
         (config_->START_UP == Config::FRESH) ? m_amendmentTable->getDesired()
                                              : std::vector<uint256>{};
 
@@ -1710,7 +1710,10 @@ ApplicationImp::startGenesisLedger()
     auto const next =
         std::make_shared<Ledger>(*genesis, timeKeeper().closeTime());
     next->updateSkipList();
-    next->setImmutable(*config_);
+    assert(
+        next->info().seq < XRP_LEDGER_EARLIEST_FEES ||
+        next->read(keylet::fees()));
+    next->setImmutable();
     openLedger_.emplace(next, cachedSLEs_, logs_->journal("OpenLedger"));
     m_ledgerMaster->storeLedger(next);
     m_ledgerMaster->switchLCL(next);
@@ -1728,7 +1731,10 @@ ApplicationImp::getLastFullLedger()
         if (!ledger)
             return ledger;
 
-        ledger->setImmutable(*config_);
+        assert(
+            ledger->info().seq < XRP_LEDGER_EARLIEST_FEES ||
+            ledger->read(keylet::fees()));
+        ledger->setImmutable();
 
         if (getLedgerMaster().haveLedger(seq))
             ledger->setValidated();
@@ -1879,8 +1885,11 @@ ApplicationImp::loadLedgerFromFile(std::string const& name)
 
         loadLedger->stateMap().flushDirty(hotACCOUNT_NODE);
 
+        assert(
+            loadLedger->info().seq < XRP_LEDGER_EARLIEST_FEES ||
+            loadLedger->read(keylet::fees()));
         loadLedger->setAccepted(
-            closeTime, closeTimeResolution, !closeTimeEstimated, *config_);
+            closeTime, closeTimeResolution, !closeTimeEstimated);
 
         return loadLedger;
     }

diff --git a/src/ripple/app/misc/FeeVote.h b/src/ripple/app/misc/FeeVote.h
@@ -32,23 +32,6 @@ namespace ripple {
 class FeeVote
 {
 public:
-    /** Fee schedule to vote for.
-        During voting ledgers, the FeeVote logic will try to move towards
-        these values when injecting fee-setting transactions.
-        A default-constructed Setup contains recommended values.
-    */
-    struct Setup
-    {
-        /** The cost of a reference transaction in drops. */
-        XRPAmount reference_fee{10};
-
-        /** The account reserve requirement in drops. */
-        XRPAmount account_reserve{10 * DROPS_PER_XRP};
-
-        /** The per-owned item reserve requirement in drops. */
-        XRPAmount owner_reserve{2 * DROPS_PER_XRP};
-    };
-
     virtual ~FeeVote() = default;
 
     /** Add local fee preference to validation.
@@ -74,16 +57,13 @@ class FeeVote
         std::shared_ptr<SHAMap> const& initialPosition) = 0;
 };
 
-/** Build FeeVote::Setup from a config section. */
-FeeVote::Setup
-setup_FeeVote(Section const& section);
-
+struct FeeSetup;
 /** Create an instance of the FeeVote logic.
     @param setup The fee schedule to vote for.
     @param journal Where to log.
 */
 std::unique_ptr<FeeVote>
-make_FeeVote(FeeVote::Setup const& setup, beast::Journal journal);
+make_FeeVote(FeeSetup const& setup, beast::Journal journal);
 
 }  // namespace ripple
 

diff --git a/src/ripple/app/misc/FeeVoteImpl.cpp b/src/ripple/app/misc/FeeVoteImpl.cpp
@@ -93,11 +93,11 @@ VotableValue::getVotes() const -> std::pair<value_type, bool>
 class FeeVoteImpl : public FeeVote
 {
 private:
-    Setup target_;
+    FeeSetup target_;
     beast::Journal const journal_;
 
 public:
-    FeeVoteImpl(Setup const& setup, beast::Journal journal);
+    FeeVoteImpl(FeeSetup const& setup, beast::Journal journal);
 
     void
     doValidation(Fees const& lastFees, Rules const& rules, STValidation& val)
@@ -112,7 +112,7 @@ class FeeVoteImpl : public FeeVote
 
 //--------------------------------------------------------------------------
 
-FeeVoteImpl::FeeVoteImpl(Setup const& setup, beast::Journal journal)
+FeeVoteImpl::FeeVoteImpl(FeeSetup const& setup, beast::Journal journal)
     : target_(setup), journal_(journal)
 {
 }
@@ -335,28 +335,8 @@ FeeVoteImpl::doVoting(
 
 //------------------------------------------------------------------------------
 
-FeeVote::Setup
-setup_FeeVote(Section const& section)
-{
-    FeeVote::Setup setup;
-    {
-        std::uint64_t temp;
-        if (set(temp, "reference_fee", section) &&
-            temp <= std::numeric_limits<XRPAmount::value_type>::max())
-            setup.reference_fee = temp;
-    }
-    {
-        std::uint32_t temp;
-        if (set(temp, "account_reserve", section))
-            setup.account_reserve = temp;
-        if (set(temp, "owner_reserve", section))
-            setup.owner_reserve = temp;
-    }
-    return setup;
-}
-
 std::unique_ptr<FeeVote>
-make_FeeVote(FeeVote::Setup const& setup, beast::Journal journal)
+make_FeeVote(FeeSetup const& setup, beast::Journal journal)
 {
     return std::make_unique<FeeVoteImpl>(setup, journal);
 }