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>
2025-11-19 18:15:50 +00:00 · 2024-06-27 11:52:02 -07:00
parent 95e16b0eed
commit 463dd92c9e
12 changed files with 347 additions and 321 deletions
--- a/include/xrpl/protocol/Feature.h
+++ b/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 = 99;
+static constexpr std::size_t numFeatures = 100;

 /** Amendments that this server supports and the default voting behavior.
   Whether they are enabled depends on the Rules defined in the validated
@@ -393,6 +393,7 @@ extern uint256 const fixAMMv1_1;
 extern uint256 const featureNFTokenMintOffer;
 extern uint256 const fixReducedOffersV2;
 extern uint256 const fixEnforceNFTokenTrustline;
+extern uint256 const fixInnerObjTemplate2;

 }  // namespace ripple

--- a/include/xrpl/protocol/STObject.h
+++ b/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;
--- a/src/libxrpl/protocol/Feature.cpp
+++ b/src/libxrpl/protocol/Feature.cpp
@@ -519,6 +519,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.
--- a/src/libxrpl/protocol/STObject.cpp
+++ b/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))
--- a/src/libxrpl/protocol/XChainAttestations.cpp
+++ b/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};
--- a/src/test/rpc/LedgerData_test.cpp
+++ b/src/test/rpc/LedgerData_test.cpp
@@ -303,12 +303,18 @@ 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;

            std::vector<std::string> const keys = {
-            "ED74D4036C6591A4BDF9C54CEFA39B996A5DCE5F86D11FDA1874481CE9D5A1CDC"
-            "1"};
-        Env env{*this, network::makeNetworkVLConfig(21337, keys)};
+                "ED74D4036C6591A4BDF9C54CEFA39B996A5DCE5F86D11FDA1874481CE9D5A1"
+                "CDC1"};
+            Env env{*this, network::makeNetworkVLConfig(21337, keys), features};

            Account const alice{"alice"};
            Account const gw{"gateway"};
@@ -364,8 +370,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));

            {
@@ -375,7 +384,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);
@@ -383,18 +393,19 @@ public:

            {
                std::string const createCodeHex =
-                "0061736D01000000011B0460027F7F017F60047F7F7F7F017E60037F7F7E01"
-                "7E60017F017E02270303656E76025F67000003656E760973746174655F7365"
-                "74000103656E76066163636570740002030201030503010002062B077F0141"
-                "9088040B7F004180080B7F00418A080B7F004180080B7F00419088040B7F00"
-                "41000B7F0041010B07080104686F6F6B00030AE7800001E3800002017F017E"
-                "230041106B220124002001200036020C41012200200010001A200141800828"
-                "0000360208200141046A410022002F0088083B010020012000280084083602"
-                "004100200020014106200141086A4104100110022102200141106A24002002"
-                "0B0B1001004180080B096B65790076616C7565";
+                    "0061736D01000000011B0460027F7F017F60047F7F7F7F017E60037F7F"
+                    "7E017E60017F017E02270303656E76025F67000003656E760973746174"
+                    "655F736574000103656E76066163636570740002030201030503010002"
+                    "062B077F01419088040B7F004180080B7F00418A080B7F004180080B7F"
+                    "00419088040B7F0041000B7F0041010B07080104686F6F6B00030AE780"
+                    "0001E3800002017F017E230041106B220124002001200036020C410122"
+                    "00200010001A2001418008280000360208200141046A410022002F0088"
+                    "083B010020012000280084083602004100200020014106200141086A41"
+                    "04100110022102200141106A240020020B0B1001004180080B096B6579"
+                    "0076616C7565";
                std::string ns_str =
-                "CAFECAFECAFECAFECAFECAFECAFECAFECAFECAFECAFECAFECAFECAFECAFECA"
-                "FE";
+                    "CAFECAFECAFECAFECAFECAFECAFECAFECAFECAFECAFECAFECAFECAFECA"
+                    "FECAFE";
                Json::Value jv = ripple::test::jtx::hook(
                    Account{"bob3"}, {{hso(createCodeHex)}}, 0);
                jv[jss::Hooks][0U][jss::Hook][jss::HookNamespace] = ns_str;
@@ -418,8 +429,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[sfCancelAfter.fieldName] = NetClock::time_point{env.now() + 300s}
+                jv[sfPublicKey.fieldName] =
+                    strHex(Account{"bob6"}.pk().slice());
+                jv[sfCancelAfter.fieldName] =
+                    NetClock::time_point{env.now() + 300s}
                        .time_since_epoch()
                        .count();
                env(jv);
@@ -430,7 +443,8 @@ public:
                env(noop(master), fee(10'000'000'000), ter(tesSUCCESS));
                env.close();
                env(import::import(
-                    alice, import::loadXpop(test::ImportTCAccountSet::w_seed)),
+                        alice,
+                        import::loadXpop(test::ImportTCAccountSet::w_seed)),
                    fee(10 * 10),
                    ter(tesSUCCESS));
                env.close();
@@ -439,10 +453,10 @@ public:
            {
                // ADD UNL REPORT
                std::vector<std::string> const _ivlKeys = {
-                "ED74D4036C6591A4BDF9C54CEFA39B996A5DCE5F86D11FDA1874481CE9D5A1"
-                "CDC1",
-                "ED74D4036C6591A4BDF9C54CEFA39B996A5DCE5F86D11FDA1874481CE9D5A1"
-                "CDC2",
+                    "ED74D4036C6591A4BDF9C54CEFA39B996A5DCE5F86D11FDA1874481CE9"
+                    "D5A1CDC1",
+                    "ED74D4036C6591A4BDF9C54CEFA39B996A5DCE5F86D11FDA1874481CE9"
+                    "D5A1CDC2",
                };

                std::vector<PublicKey> ivlKeys;
@@ -453,10 +467,10 @@ public:
                }

                std::vector<std::string> const _vlKeys = {
-                "ED8E43A943A174190BA2FAE91F44AC6E2D1D8202EFDCC2EA3DBB39814576D6"
-                "90F7",
-                "ED45D1840EE724BE327ABE9146503D5848EFD5F38B6D5FEDE71E80ACCE5E6E"
-                "738B"};
+                    "ED8E43A943A174190BA2FAE91F44AC6E2D1D8202EFDCC2EA3DBB398145"
+                    "76D690F7",
+                    "ED45D1840EE724BE327ABE9146503D5848EFD5F38B6D5FEDE71E80ACCE"
+                    "5E6E738B"};

                std::vector<PublicKey> vlKeys;
                for (auto const& strPk : _vlKeys)
@@ -508,7 +522,6 @@ public:
            {  // jvParams[jss::type] = "unl_report";
                auto const jrr = makeRequest(jss::unl_report);
                BEAST_EXPECT(checkArraySize(jrr[jss::state], 1));
-
                for (auto const& j : jrr[jss::state])
                    BEAST_EXPECT(j["LedgerEntryType"] == jss::UNLReport);
            }
@@ -638,6 +651,7 @@ public:
                BEAST_EXPECT(jrr["error_message"] == "Invalid field 'type'.");
            }
        }
+    }

    void
    run() override
@@ -650,7 +664,7 @@ public:
        testLedgerHeader();
        testLedgerType();
    }
-};
+};  // namespace ripple

 BEAST_DEFINE_TESTSUITE_PRIO(LedgerData, app, ripple, 1);

--- a/src/xrpld/app/ledger/Ledger.cpp
+++ b/src/xrpld/app/ledger/Ledger.cpp
@@ -831,7 +831,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());
--- a/src/xrpld/app/misc/detail/AMMUtils.cpp
+++ b/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);
--- a/src/xrpld/app/tx/detail/AMMVote.cpp
+++ b/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(
--- a/src/xrpld/app/tx/detail/Change.cpp
+++ b/src/xrpld/app/tx/detail/Change.cpp
@@ -931,11 +931,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.emplace_back(STUInt32(
-            sfCloseTime, view().parentCloseTime().time_since_epoch().count()));
+        entry[sfAmendment] = amendment;
+        entry[sfCloseTime] =
+            view().parentCloseTime().time_since_epoch().count();

        if (!ctx_.app.getAmendmentTable().isSupported(amendment))
        {
--- a/src/xrpld/app/tx/detail/SetSignerList.cpp
+++ b/src/xrpld/app/tx/detail/SetSignerList.cpp
@@ -359,11 +359,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.setFieldU16(sfSignerWeight, entry.weight);
+        obj[sfAccount] = entry.account;
+        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
--- a/src/xrpld/rpc/detail/TransactionSign.cpp
+++ b/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(