Amendment: ExpandedSignerList

Co-authored-by: Scott Schurr <[email protected]>

Builds/CMake/RippledCore.cmake

@@ -82,6 +82,7 @@ target_sources (xrpl_core PRIVATE
   src/ripple/protocol/impl/PublicKey.cpp
   src/ripple/protocol/impl/Quality.cpp
   src/ripple/protocol/impl/Rate2.cpp
+  src/ripple/protocol/impl/Rules.cpp
   src/ripple/protocol/impl/SField.cpp
   src/ripple/protocol/impl/SOTemplate.cpp
   src/ripple/protocol/impl/STAccount.cpp
@@ -213,6 +214,7 @@ install (
     src/ripple/protocol/PublicKey.h
     src/ripple/protocol/Quality.h
     src/ripple/protocol/Rate.h
+    src/ripple/protocol/Rules.h
     src/ripple/protocol/SField.h
     src/ripple/protocol/SOTemplate.h
     src/ripple/protocol/STAccount.h

src/ripple/app/consensus/RCLConsensus.cpp

@@ -636,7 +636,7 @@ RCLConsensus::Adaptor::doAccept(
         auto const lastVal = ledgerMaster_.getValidatedLedger();
         std::optional<Rules> rules;
         if (lastVal)
-            rules.emplace(*lastVal, app_.config().features);
+            rules = makeRulesGivenLedger(*lastVal, app_.config().features);
         else
             rules.emplace(app_.config().features);
         app_.openLedger().accept(

src/ripple/app/ledger/Ledger.cpp

@@ -626,7 +626,7 @@ Ledger::setup(Config const& config)
 
     try
     {
-        rules_ = Rules(*this, config.features);
+        rules_ = makeRulesGivenLedger(*this, config.features);
     }
     catch (SHAMapMissingNode const&)
     {

src/ripple/app/misc/NetworkOPs.cpp

@@ -1744,7 +1744,7 @@ NetworkOPsImp::switchLastClosedLedger(
         auto const lastVal = app_.getLedgerMaster().getValidatedLedger();
         std::optional<Rules> rules;
         if (lastVal)
-            rules.emplace(*lastVal, app_.config().features);
+            rules = makeRulesGivenLedger(*lastVal, app_.config().features);
         else
             rules.emplace(app_.config().features);
         app_.openLedger().accept(

src/ripple/app/tx/impl/SetSignerList.cpp

@@ -83,6 +83,7 @@ SetSignerList::preflight(PreflightContext const& ctx)
         return ret;
 
     auto const result = determineOperation(ctx.tx, ctx.flags, ctx.j);
+
     if (std::get<0>(result) != tesSUCCESS)
         return std::get<0>(result);
 
@@ -99,7 +100,11 @@ SetSignerList::preflight(PreflightContext const& ctx)
         // Validate our settings.
         auto const account = ctx.tx.getAccountID(sfAccount);
         NotTEC const ter = validateQuorumAndSignerEntries(
-            std::get<1>(result), std::get<2>(result), account, ctx.j);
+            std::get<1>(result),
+            std::get<2>(result),
+            account,
+            ctx.j,
+            ctx.rules);
         if (ter != tesSUCCESS)
         {
             return ter;
@@ -150,7 +155,7 @@ SetSignerList::preCompute()
 // is valid until the featureMultiSignReserve amendment passes.  Once it
 // passes then just 1 OwnerCount is associated with a SignerList.
 static int
-signerCountBasedOwnerCountDelta(std::size_t entryCount)
+signerCountBasedOwnerCountDelta(std::size_t entryCount, Rules const& rules)
 {
     // We always compute the full change in OwnerCount, taking into account:
     //  o The fact that we're adding/removing a SignerList and
@@ -165,9 +170,10 @@ signerCountBasedOwnerCountDelta(std::size_t entryCount)
     // units.  A SignerList with 8 entries would cost 10 OwnerCount units.
     //
     // The static_cast should always be safe since entryCount should always
-    // be in the range from 1 to 8.  We've got a lot of room to grow.
+    // be in the range from 1 to 8 (or 32 if ExpandedSignerList is enabled).
+    // We've got a lot of room to grow.
     assert(entryCount >= STTx::minMultiSigners);
-    assert(entryCount <= STTx::maxMultiSigners);
+    assert(entryCount <= STTx::maxMultiSigners(&rules));
     return 2 + static_cast<int>(entryCount);
 }
 
@@ -196,7 +202,8 @@ removeSignersFromLedger(
     {
         STArray const& actualList = signers->getFieldArray(sfSignerEntries);
         removeFromOwnerCount =
-            signerCountBasedOwnerCountDelta(actualList.size()) * -1;
+            signerCountBasedOwnerCountDelta(actualList.size(), view.rules()) *
+            -1;
     }
 
     // Remove the node from the account directory.
@@ -239,13 +246,14 @@ SetSignerList::validateQuorumAndSignerEntries(
     std::uint32_t quorum,
     std::vector<SignerEntries::SignerEntry> const& signers,
     AccountID const& account,
-    beast::Journal j)
+    beast::Journal j,
+    Rules const& rules)
 {
     // Reject if there are too many or too few entries in the list.
     {
         std::size_t const signerCount = signers.size();
         if ((signerCount < STTx::minMultiSigners) ||
-            (signerCount > STTx::maxMultiSigners))
+            (signerCount > STTx::maxMultiSigners(&rules)))
         {
             JLOG(j.trace()) << "Too many or too few signers in signer list.";
             return temMALFORMED;
@@ -260,6 +268,9 @@ SetSignerList::validateQuorumAndSignerEntries(
         return temBAD_SIGNER;
     }
 
+    // Is the ExpandedSignerList amendment active?
+    bool const expandedSignerList = rules.enabled(featureExpandedSignerList);
+
     // Make sure no signers reference this account.  Also make sure the
     // quorum can be reached.
     std::uint64_t allSignersWeight(0);
@@ -280,6 +291,14 @@ SetSignerList::validateQuorumAndSignerEntries(
             return temBAD_SIGNER;
         }
 
+        if (signer.tag && !expandedSignerList)
+        {
+            JLOG(j.trace()) << "Malformed transaction: sfWalletLocator "
+                               "specified in SignerEntry "
+                            << "but featureExpandedSignerList is not enabled.";
+            return temMALFORMED;
+        }
+
         // Don't verify that the signer accounts exist.  Non-existent accounts
         // may be phantom accounts (which are permitted).
     }
@@ -322,7 +341,8 @@ SetSignerList::replaceSignerList()
     std::uint32_t flags{lsfOneOwnerCount};
     if (!ctx_.view().rules().enabled(featureMultiSignReserve))
     {
-        addedOwnerCount = signerCountBasedOwnerCountDelta(signers_.size());
+        addedOwnerCount = signerCountBasedOwnerCountDelta(
+            signers_.size(), ctx_.view().rules());
         flags = 0;
     }
 
@@ -390,6 +410,9 @@ SetSignerList::writeSignersToSLE(
     if (flags)  // Only set flags if they are non-default (default is zero).
         ledgerEntry->setFieldU32(sfFlags, flags);
 
+    bool const expandedSignerList =
+        ctx_.view().rules().enabled(featureExpandedSignerList);
+
     // Create the SignerListArray one SignerEntry at a time.
     STArray toLedger(signers_.size());
     for (auto const& entry : signers_)
@@ -399,6 +422,11 @@ SetSignerList::writeSignersToSLE(
         obj.reserve(2);
         obj.setAccountID(sfAccount, entry.account);
         obj.setFieldU16(sfSignerWeight, entry.weight);
+
+        // This is a defensive check to make absolutely sure we will never write
+        // a tag into the ledger while featureExpandedSignerList is not enabled
+        if (expandedSignerList && entry.tag)
+            obj.setFieldH256(sfWalletLocator, *(entry.tag));
     }
 
     // Assign the SignerEntries.

src/ripple/app/tx/impl/SetSignerList.h

@@ -25,6 +25,7 @@
 #include <ripple/app/tx/impl/Transactor.h>
 #include <ripple/basics/Log.h>
 #include <ripple/protocol/Indexes.h>
+#include <ripple/protocol/Rules.h>
 #include <ripple/protocol/STArray.h>
 #include <ripple/protocol/STObject.h>
 #include <ripple/protocol/STTx.h>
@@ -83,7 +84,8 @@ class SetSignerList : public Transactor
         std::uint32_t quorum,
         std::vector<SignerEntries::SignerEntry> const& signers,
         AccountID const& account,
-        beast::Journal j);
+        beast::Journal j,
+        Rules const&);
 
     TER
     replaceSignerList();

src/ripple/app/tx/impl/SignerEntries.cpp

@@ -22,6 +22,7 @@
 #include <ripple/protocol/STArray.h>
 #include <ripple/protocol/STObject.h>
 #include <cstdint>
+#include <optional>
 
 namespace ripple {
 
@@ -41,7 +42,7 @@ SignerEntries::deserialize(
     }
 
     std::vector<SignerEntry> accountVec;
-    accountVec.reserve(STTx::maxMultiSigners);
+    accountVec.reserve(STTx::maxMultiSigners());
 
     STArray const& sEntries(obj.getFieldArray(sfSignerEntries));
     for (STObject const& sEntry : sEntries)
@@ -57,7 +58,9 @@ SignerEntries::deserialize(
         // Extract SignerEntry fields.
         AccountID const account = sEntry.getAccountID(sfAccount);
         std::uint16_t const weight = sEntry.getFieldU16(sfSignerWeight);
-        accountVec.emplace_back(account, weight);
+        std::optional<uint256> const tag = sEntry.at(~sfWalletLocator);
+
+        accountVec.emplace_back(account, weight, tag);
     }
     return accountVec;
 }

src/ripple/app/tx/impl/SignerEntries.h

@@ -23,9 +23,11 @@
 #include <ripple/app/tx/impl/Transactor.h>  // NotTEC
 #include <ripple/basics/Expected.h>         //
 #include <ripple/beast/utility/Journal.h>   // beast::Journal
+#include <ripple/protocol/Rules.h>          // Rules
 #include <ripple/protocol/STTx.h>           // STTx::maxMultiSigners
 #include <ripple/protocol/TER.h>            // temMALFORMED
 #include <ripple/protocol/UintTypes.h>      // AccountID
+#include <optional>
 
 namespace ripple {
 
@@ -42,9 +44,13 @@ class SignerEntries
     {
         AccountID account;
         std::uint16_t weight;
+        std::optional<uint256> tag;
 
-        SignerEntry(AccountID const& inAccount, std::uint16_t inWeight)
-            : account(inAccount), weight(inWeight)
+        SignerEntry(
+            AccountID const& inAccount,
+            std::uint16_t inWeight,
+            std::optional<uint256> inTag)
+            : account(inAccount), weight(inWeight), tag(inTag)
         {
         }
 

src/ripple/app/tx/impl/apply.cpp

@@ -54,7 +54,7 @@ checkValidity(
             ? STTx::RequireFullyCanonicalSig::yes
             : STTx::RequireFullyCanonicalSig::no;
 
-        auto const sigVerify = tx.checkSign(requireCanonicalSig);
+        auto const sigVerify = tx.checkSign(requireCanonicalSig, rules);
         if (!sigVerify)
         {
             router.setFlags(id, SF_SIGBAD);

src/ripple/ledger/ReadView.h

@@ -29,6 +29,7 @@
 #include <ripple/ledger/detail/ReadViewFwdRange.h>
 #include <ripple/protocol/Indexes.h>
 #include <ripple/protocol/Protocol.h>
+#include <ripple/protocol/Rules.h>
 #include <ripple/protocol/STAmount.h>
 #include <ripple/protocol/STLedgerEntry.h>
 #include <ripple/protocol/STTx.h>
@@ -125,64 +126,6 @@ struct LedgerInfo
 
 //------------------------------------------------------------------------------
 
-class DigestAwareReadView;
-
-/** Rules controlling protocol behavior. */
-class Rules
-{
-private:
-    class Impl;
-
-    std::shared_ptr<Impl const> impl_;
-
-public:
-    Rules(Rules const&) = default;
-    Rules&
-    operator=(Rules const&) = default;
-
-    Rules() = delete;
-
-    /** Construct an empty rule set.
-
-        These are the rules reflected by
-        the genesis ledger.
-    */
-    explicit Rules(std::unordered_set<uint256, beast::uhash<>> const& presets);
-
-    /** Construct rules from a ledger.
-
-        The ledger contents are analyzed for rules
-        and amendments and extracted to the object.
-    */
-    explicit Rules(
-        DigestAwareReadView const& ledger,
-        std::unordered_set<uint256, beast::uhash<>> const& presets);
-
-    /** Returns `true` if a feature is enabled. */
-    bool
-    enabled(uint256 const& id) const;
-
-    /** Returns `true` if these rules don't match the ledger. */
-    bool
-    changed(DigestAwareReadView const& ledger) const;
-
-    /** Returns `true` if two rule sets are identical.
-
-        @note This is for diagnostics. To determine if new
-        rules should be constructed, call changed() first instead.
-    */
-    bool
-    operator==(Rules const&) const;
-
-    bool
-    operator!=(Rules const& other) const
-    {
-        return !(*this == other);
-    }
-};
-
-//------------------------------------------------------------------------------
-
 /** A view into a ledger.
 
     This interface provides read access to state
@@ -423,6 +366,11 @@ getCloseAgree(LedgerInfo const& info)
 void
 addRaw(LedgerInfo const&, Serializer&, bool includeHash = false);
 
+Rules
+makeRulesGivenLedger(
+    DigestAwareReadView const& ledger,
+    std::unordered_set<uint256, beast::uhash<>> const& presets);
+
 }  // namespace ripple
 
 #include <ripple/ledger/detail/ReadViewFwdRange.ipp>