fixInnerObjTemplate2 amendment (#5047)

* fixInnerObjTemplate2 amendment:

Apply inner object templates to all remaining (non-AMM)
inner objects.

Adds a unit test for applying the template to sfMajorities.
Other remaining inner objects showed no problems having
templates applied.

* Move CMake directory

* Rearrange sources

* Rewrite includes

* Recompute loops

---------

Co-authored-by: Pretty Printer <cpp@ripple.com>

include/xrpl/protocol/Feature.h

@@ -80,7 +80,7 @@ namespace detail {
 // Feature.cpp. Because it's only used to reserve storage, and determine how
 // large to make the FeatureBitset, it MAY be larger. It MUST NOT be less than
 // the actual number of amendments. A LogicError on startup will verify this.
-static constexpr std::size_t numFeatures = 76;
+static constexpr std::size_t numFeatures = 77;
 
 /** Amendments that this server supports and the default voting behavior.
    Whether they are enabled depends on the Rules defined in the validated
@@ -369,6 +369,7 @@ extern uint256 const fixAMMv1_1;
 extern uint256 const featureNFTokenMintOffer;
 extern uint256 const fixReducedOffersV2;
 extern uint256 const fixEnforceNFTokenTrustline;
+extern uint256 const fixInnerObjTemplate2;
 
 }  // namespace ripple
 

include/xrpl/protocol/STObject.h

@@ -44,7 +44,6 @@
 namespace ripple {
 
 class STArray;
-class Rules;
 
 inline void
 throwFieldNotFound(SField const& field)
@@ -105,7 +104,7 @@ public:
     explicit STObject(SField const& name);
 
     static STObject
-    makeInnerObject(SField const& name, Rules const& rules);
+    makeInnerObject(SField const& name);
 
     iterator
     begin() const;

src/libxrpl/protocol/Feature.cpp

@@ -496,6 +496,7 @@ REGISTER_FIX    (fixAMMv1_1,                    Supported::yes, VoteBehavior::De
 REGISTER_FEATURE(NFTokenMintOffer,              Supported::yes, VoteBehavior::DefaultNo);
 REGISTER_FIX    (fixReducedOffersV2,            Supported::yes, VoteBehavior::DefaultNo);
 REGISTER_FIX    (fixEnforceNFTokenTrustline,    Supported::yes, VoteBehavior::DefaultNo);
+REGISTER_FIX    (fixInnerObjTemplate2,          Supported::yes, VoteBehavior::DefaultNo);
 
 // The following amendments are obsolete, but must remain supported
 // because they could potentially get enabled.

src/libxrpl/protocol/STObject.cpp

@@ -61,10 +61,19 @@ STObject::STObject(SerialIter& sit, SField const& name, int depth) noexcept(
 }
 
 STObject
-STObject::makeInnerObject(SField const& name, Rules const& rules)
+STObject::makeInnerObject(SField const& name)
 {
     STObject obj{name};
-    if (rules.enabled(fixInnerObjTemplate))
+
+    // The if is complicated because inner object templates were added in
+    // two phases:
+    //  1. If there are no available Rules, then always apply the template.
+    //  2. fixInnerObjTemplate added templates to two AMM inner objects.
+    //  3. fixInnerObjTemplate2 added templates to all remaining inner objects.
+    std::optional<Rules> const& rules = getCurrentTransactionRules();
+    bool const isAMMObj = name == sfAuctionSlot || name == sfVoteEntry;
+    if (!rules || (rules->enabled(fixInnerObjTemplate) && isAMMObj) ||
+        (rules->enabled(fixInnerObjTemplate2) && !isAMMObj))
     {
         if (SOTemplate const* elements =
                 InnerObjectFormats::getInstance().findSOTemplateBySField(name))

src/libxrpl/protocol/XChainAttestations.cpp

@@ -203,7 +203,8 @@ AttestationClaim::AttestationClaim(Json::Value const& v)
 STObject
 AttestationClaim::toSTObject() const
 {
-    STObject o{sfXChainClaimAttestationCollectionElement};
+    STObject o =
+        STObject::makeInnerObject(sfXChainClaimAttestationCollectionElement);
     addHelper(o);
     o[sfXChainClaimID] = claimID;
     if (dst)
@@ -345,7 +346,8 @@ AttestationCreateAccount::AttestationCreateAccount(
 STObject
 AttestationCreateAccount::toSTObject() const
 {
-    STObject o{sfXChainCreateAccountAttestationCollectionElement};
+    STObject o = STObject::makeInnerObject(
+        sfXChainCreateAccountAttestationCollectionElement);
     addHelper(o);
 
     o[sfXChainAccountCreateCount] = createCount;
@@ -497,7 +499,7 @@ XChainClaimAttestation::XChainClaimAttestation(
 STObject
 XChainClaimAttestation::toSTObject() const
 {
-    STObject o{sfXChainClaimProofSig};
+    STObject o = STObject::makeInnerObject(sfXChainClaimProofSig);
     o[sfAttestationSignerAccount] =
         STAccount{sfAttestationSignerAccount, keyAccount};
     o[sfPublicKey] = publicKey;
@@ -609,7 +611,7 @@ XChainCreateAccountAttestation::XChainCreateAccountAttestation(
 STObject
 XChainCreateAccountAttestation::toSTObject() const
 {
-    STObject o{sfXChainCreateAccountProofSig};
+    STObject o = STObject::makeInnerObject(sfXChainCreateAccountProofSig);
 
     o[sfAttestationSignerAccount] =
         STAccount{sfAttestationSignerAccount, keyAccount};

src/test/rpc/LedgerData_test.cpp

@@ -300,8 +300,14 @@ public:
     {
         // Put a bunch of different LedgerEntryTypes into a ledger
         using namespace test::jtx;
+
+        // Make sure fixInnerObjTemplate2 doesn't break amendments.
+        for (FeatureBitset const& features :
+             {supported_amendments() - fixInnerObjTemplate2,
+              supported_amendments() | fixInnerObjTemplate2})
+        {
             using namespace std::chrono;
-        Env env{*this, envconfig(validator, "")};
+            Env env{*this, envconfig(validator, ""), features};
 
             Account const gw{"gateway"};
             auto const USD = gw["USD"];
@@ -352,8 +358,11 @@ public:
                 if (!majorities.empty())
                     break;
             }
+
             env(signers(
-            Account{"bob0"}, 1, {{Account{"bob1"}, 1}, {Account{"bob2"}, 1}}));
+                Account{"bob0"},
+                1,
+                {{Account{"bob1"}, 1}, {Account{"bob2"}, 1}}));
             env(ticket::create(env.master, 1));
 
             {
@@ -363,7 +372,8 @@ public:
                 jv[jss::Account] = Account{"bob5"}.human();
                 jv[jss::Destination] = Account{"bob6"}.human();
                 jv[jss::Amount] = XRP(50).value().getJson(JsonOptions::none);
-            jv[sfFinishAfter.fieldName] = NetClock::time_point{env.now() + 10s}
+                jv[sfFinishAfter.fieldName] =
+                    NetClock::time_point{env.now() + 10s}
                         .time_since_epoch()
                         .count();
                 env(jv);
@@ -377,8 +387,10 @@ public:
                 jv[jss::Destination] = Account{"bob7"}.human();
                 jv[jss::Amount] = XRP(100).value().getJson(JsonOptions::none);
                 jv[jss::SettleDelay] = NetClock::duration{10s}.count();
-            jv[sfPublicKey.fieldName] = strHex(Account{"bob6"}.pk().slice());
+                jv[sfPublicKey.fieldName] =
-            jv[sfCancelAfter.fieldName] = NetClock::time_point{env.now() + 300s}
+                    strHex(Account{"bob6"}.pk().slice());
+                jv[sfCancelAfter.fieldName] =
+                    NetClock::time_point{env.now() + 300s}
                         .time_since_epoch()
                         .count();
                 env(jv);
@@ -497,6 +509,7 @@ public:
                 BEAST_EXPECT(jrr["error_message"] == "Invalid field 'type'.");
             }
         }
+    }
 
     void
     run() override

src/xrpld/app/ledger/Ledger.cpp

@@ -793,7 +793,7 @@ Ledger::updateNegativeUNL()
 
     if (hasToDisable)
     {
-        newNUnl.emplace_back(sfDisabledValidator);
+        newNUnl.push_back(STObject::makeInnerObject(sfDisabledValidator));
         newNUnl.back().setFieldVL(
             sfPublicKey, sle->getFieldVL(sfValidatorToDisable));
         newNUnl.back().setFieldU32(sfFirstLedgerSequence, seq());

src/xrpld/app/misc/detail/AMMUtils.cpp

@@ -312,7 +312,7 @@ initializeFeeAuctionVote(
     auto const& rules = view.rules();
     // AMM creator gets the voting slot.
     STArray voteSlots;
-    STObject voteEntry = STObject::makeInnerObject(sfVoteEntry, rules);
+    STObject voteEntry = STObject::makeInnerObject(sfVoteEntry);
     if (tfee != 0)
         voteEntry.setFieldU16(sfTradingFee, tfee);
     voteEntry.setFieldU32(sfVoteWeight, VOTE_WEIGHT_SCALE_FACTOR);
@@ -325,7 +325,7 @@ initializeFeeAuctionVote(
     if (rules.enabled(fixInnerObjTemplate) &&
         !ammSle->isFieldPresent(sfAuctionSlot))
     {
-        STObject auctionSlot = STObject::makeInnerObject(sfAuctionSlot, rules);
+        STObject auctionSlot = STObject::makeInnerObject(sfAuctionSlot);
         ammSle->set(std::move(auctionSlot));
     }
     STObject& auctionSlot = ammSle->peekFieldObject(sfAuctionSlot);

src/xrpld/app/tx/detail/AMMVote.cpp

@@ -104,7 +104,7 @@ applyVote(
     Number den{0};
     // Account already has vote entry
     bool foundAccount = false;
-    auto const& rules = ctx_.view().rules();
+
     // Iterate over the current vote entries and update each entry
     // per current total tokens balance and each LP tokens balance.
     // Find the entry with the least tokens and whether the account
@@ -120,7 +120,7 @@ applyVote(
             continue;
         }
         auto feeVal = entry[sfTradingFee];
-        STObject newEntry = STObject::makeInnerObject(sfVoteEntry, rules);
+        STObject newEntry = STObject::makeInnerObject(sfVoteEntry);
         // The account already has the vote entry.
         if (account == account_)
         {
@@ -159,7 +159,7 @@ applyVote(
     {
         auto update = [&](std::optional<std::uint8_t> const& minPos =
                               std::nullopt) {
-            STObject newEntry = STObject::makeInnerObject(sfVoteEntry, rules);
+            STObject newEntry = STObject::makeInnerObject(sfVoteEntry);
             if (feeNew != 0)
                 newEntry.setFieldU16(sfTradingFee, feeNew);
             newEntry.setFieldU32(

src/xrpld/app/tx/detail/Change.cpp

@@ -300,11 +300,11 @@ Change::applyAmendment()
     if (gotMajority)
     {
         // This amendment now has a majority
-        newMajorities.push_back(STObject(sfMajority));
+        newMajorities.push_back(STObject::makeInnerObject(sfMajority));
         auto& entry = newMajorities.back();
-        entry.emplace_back(STUInt256(sfAmendment, amendment));
+        entry[sfAmendment] = amendment;
-        entry.emplace_back(STUInt32(
+        entry[sfCloseTime] =
-            sfCloseTime, view().parentCloseTime().time_since_epoch().count()));
+            view().parentCloseTime().time_since_epoch().count();
 
         if (!ctx_.app.getAmendmentTable().isSupported(amendment))
         {

src/xrpld/app/tx/detail/SetSignerList.cpp

@@ -416,11 +416,11 @@ SetSignerList::writeSignersToSLE(
     STArray toLedger(signers_.size());
     for (auto const& entry : signers_)
     {
-        toLedger.emplace_back(sfSignerEntry);
+        toLedger.push_back(STObject::makeInnerObject(sfSignerEntry));
         STObject& obj = toLedger.back();
         obj.reserve(2);
-        obj.setAccountID(sfAccount, entry.account);
+        obj[sfAccount] = entry.account;
-        obj.setFieldU16(sfSignerWeight, entry.weight);
+        obj[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

src/xrpld/rpc/detail/TransactionSign.cpp

@@ -1051,7 +1051,7 @@ transactionSignFor(
     auto& sttx = preprocResult.second;
     {
         // Make the signer object that we'll inject.
-        STObject signer(sfSigner);
+        STObject signer = STObject::makeInnerObject(sfSigner);
         signer[sfAccount] = *signerAccountID;
         signer.setFieldVL(sfTxnSignature, signForParams.getSignature());
         signer.setFieldVL(