diff --git a/src/ripple/ledger/impl/ApplyStateTable.cpp b/src/ripple/ledger/impl/ApplyStateTable.cpp index ae70c71918..ab2f0ca8cd 100644 --- a/src/ripple/ledger/impl/ApplyStateTable.cpp +++ b/src/ripple/ledger/impl/ApplyStateTable.cpp @@ -189,8 +189,9 @@ ApplyStateTable::apply( { assert(curNode && origNode); - if (curNode->isThreadedType()) // thread transaction to node - // item modified + if (curNode->isThreadedType( + to.rules())) // thread transaction to node + // item modified threadItem(meta, curNode); STObject prevs(sfPreviousFields); @@ -224,7 +225,8 @@ ApplyStateTable::apply( assert(curNode && !origNode); threadOwners(to, meta, curNode, newMod, j); - if (curNode->isThreadedType()) // always thread to self + if (curNode->isThreadedType( + to.rules())) // always thread to self threadItem(meta, curNode); STObject news(sfNewFields); @@ -610,6 +612,8 @@ ApplyStateTable::threadTx( JLOG(j.warn()) << "Threading to non-existent account: " << toBase58(to); return; } + // threadItem only applied to AccountRoot + assert(sle->isThreadedType(base.rules())); threadItem(meta, sle); } diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index d00dd1555f..fb6b390773 100644 --- a/src/ripple/protocol/Feature.h +++ b/src/ripple/protocol/Feature.h @@ -74,7 +74,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 = 71; +static constexpr std::size_t numFeatures = 72; /** Amendments that this server supports and the default voting behavior. Whether they are enabled depends on the Rules defined in the validated @@ -358,6 +358,7 @@ extern uint256 const fixAMMOverflowOffer; extern uint256 const featurePriceOracle; extern uint256 const fixEmptyDID; extern uint256 const fixXChainRewardRounding; +extern uint256 const fixPreviousTxnID; } // namespace ripple diff --git a/src/ripple/protocol/STLedgerEntry.h b/src/ripple/protocol/STLedgerEntry.h index 14732ff5c2..6fd50aa154 100644 --- a/src/ripple/protocol/STLedgerEntry.h +++ b/src/ripple/protocol/STLedgerEntry.h @@ -25,6 +25,7 @@ namespace ripple { +class Rules; class Invariants_test; class STLedgerEntry final : public STObject, public CountedObject @@ -67,7 +68,7 @@ public: // is this a ledger entry that can be threaded bool - isThreadedType() const; + isThreadedType(Rules const& rules) const; bool thread( diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index 8bd4b7aea2..a81edad336 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/src/ripple/protocol/impl/Feature.cpp @@ -465,6 +465,7 @@ REGISTER_FIX (fixAMMOverflowOffer, Supported::yes, VoteBehavior::De REGISTER_FEATURE(PriceOracle, Supported::yes, VoteBehavior::DefaultNo); REGISTER_FIX (fixEmptyDID, Supported::yes, VoteBehavior::DefaultNo); REGISTER_FIX (fixXChainRewardRounding, Supported::yes, VoteBehavior::DefaultNo); +REGISTER_FIX (fixPreviousTxnID, Supported::yes, VoteBehavior::DefaultNo); // The following amendments are obsolete, but must remain supported // because they could potentially get enabled. diff --git a/src/ripple/protocol/impl/LedgerFormats.cpp b/src/ripple/protocol/impl/LedgerFormats.cpp index 26cd7ea69b..4f117a5d60 100644 --- a/src/ripple/protocol/impl/LedgerFormats.cpp +++ b/src/ripple/protocol/impl/LedgerFormats.cpp @@ -74,6 +74,8 @@ LedgerFormats::LedgerFormats() {sfIndexNext, soeOPTIONAL}, {sfIndexPrevious, soeOPTIONAL}, {sfNFTokenID, soeOPTIONAL}, + {sfPreviousTxnID, soeOPTIONAL}, + {sfPreviousTxnLgrSeq, soeOPTIONAL}, }, commonFields); @@ -142,6 +144,8 @@ LedgerFormats::LedgerFormats() { {sfAmendments, soeOPTIONAL}, // Enabled {sfMajorities, soeOPTIONAL}, + {sfPreviousTxnID, soeOPTIONAL}, + {sfPreviousTxnLgrSeq, soeOPTIONAL}, }, commonFields); @@ -149,14 +153,16 @@ LedgerFormats::LedgerFormats() ltFEE_SETTINGS, { // Old version uses raw numbers - {sfBaseFee, soeOPTIONAL}, - {sfReferenceFeeUnits, soeOPTIONAL}, - {sfReserveBase, soeOPTIONAL}, - {sfReserveIncrement, soeOPTIONAL}, + {sfBaseFee, soeOPTIONAL}, + {sfReferenceFeeUnits, soeOPTIONAL}, + {sfReserveBase, soeOPTIONAL}, + {sfReserveIncrement, soeOPTIONAL}, // New version uses Amounts {sfBaseFeeDrops, soeOPTIONAL}, {sfReserveBaseDrops, soeOPTIONAL}, {sfReserveIncrementDrops, soeOPTIONAL}, + {sfPreviousTxnID, soeOPTIONAL}, + {sfPreviousTxnLgrSeq, soeOPTIONAL}, }, commonFields); @@ -240,6 +246,8 @@ LedgerFormats::LedgerFormats() {sfDisabledValidators, soeOPTIONAL}, {sfValidatorToDisable, soeOPTIONAL}, {sfValidatorToReEnable, soeOPTIONAL}, + {sfPreviousTxnID, soeOPTIONAL}, + {sfPreviousTxnLgrSeq, soeOPTIONAL}, }, commonFields); @@ -272,14 +280,16 @@ LedgerFormats::LedgerFormats() add(jss::AMM, ltAMM, { - {sfAccount, soeREQUIRED}, - {sfTradingFee, soeDEFAULT}, - {sfVoteSlots, soeOPTIONAL}, - {sfAuctionSlot, soeOPTIONAL}, - {sfLPTokenBalance, soeREQUIRED}, - {sfAsset, soeREQUIRED}, - {sfAsset2, soeREQUIRED}, - {sfOwnerNode, soeREQUIRED}, + {sfAccount, soeREQUIRED}, + {sfTradingFee, soeDEFAULT}, + {sfVoteSlots, soeOPTIONAL}, + {sfAuctionSlot, soeOPTIONAL}, + {sfLPTokenBalance, soeREQUIRED}, + {sfAsset, soeREQUIRED}, + {sfAsset2, soeREQUIRED}, + {sfOwnerNode, soeREQUIRED}, + {sfPreviousTxnID, soeOPTIONAL}, + {sfPreviousTxnLgrSeq, soeOPTIONAL}, }, commonFields); diff --git a/src/ripple/protocol/impl/STLedgerEntry.cpp b/src/ripple/protocol/impl/STLedgerEntry.cpp index 8ad5e1fb5b..10ec5627aa 100644 --- a/src/ripple/protocol/impl/STLedgerEntry.cpp +++ b/src/ripple/protocol/impl/STLedgerEntry.cpp @@ -17,14 +17,19 @@ */ //============================================================================== +#include + #include #include #include #include +#include #include -#include +#include #include #include +#include +#include #include namespace ripple { @@ -124,9 +129,18 @@ STLedgerEntry::getJson(JsonOptions options) const } bool -STLedgerEntry::isThreadedType() const +STLedgerEntry::isThreadedType(Rules const& rules) const { - return getFieldIndex(sfPreviousTxnID) != -1; + static constexpr std::array newPreviousTxnIDTypes = { + ltDIR_NODE, ltAMENDMENTS, ltFEE_SETTINGS, ltNEGATIVE_UNL, ltAMM}; + // Exclude PrevTxnID/PrevTxnLgrSeq if the fixPreviousTxnID amendment is not + // enabled and the ledger object type is in the above set + bool const excludePrevTxnID = !rules.enabled(fixPreviousTxnID) && + std::count( + newPreviousTxnIDTypes.cbegin(), + newPreviousTxnIDTypes.cend(), + type_); + return !excludePrevTxnID && getFieldIndex(sfPreviousTxnID) != -1; } bool diff --git a/src/ripple/protocol/jss.h b/src/ripple/protocol/jss.h index 2655e73ef5..b5f3d1fcb5 100644 --- a/src/ripple/protocol/jss.h +++ b/src/ripple/protocol/jss.h @@ -524,6 +524,7 @@ JSS(node_writes_duration_us); // out: GetCounts JSS(node_write_retries); // out: GetCounts JSS(node_writes_delayed); // out::GetCounts JSS(nth); // out: RPC server_definitions +JSS(nunl); // in: AccountObjects JSS(obligations); // out: GatewayBalances JSS(offer); // in: LedgerEntry JSS(offers); // out: NetworkOPs, AccountOffers, Subscribe diff --git a/src/ripple/rpc/impl/RPCHelpers.cpp b/src/ripple/rpc/impl/RPCHelpers.cpp index 58503b0bf4..a6179b04c8 100644 --- a/src/ripple/rpc/impl/RPCHelpers.cpp +++ b/src/ripple/rpc/impl/RPCHelpers.cpp @@ -934,30 +934,31 @@ chooseLedgerEntryType(Json::Value const& params) std::pair result{RPC::Status::OK, ltANY}; if (params.isMember(jss::type)) { - static constexpr std::array, 21> + static constexpr std::array, 22> types{ {{jss::account, ltACCOUNT_ROOT}, {jss::amendments, ltAMENDMENTS}, + {jss::amm, ltAMM}, + {jss::bridge, ltBRIDGE}, {jss::check, ltCHECK}, {jss::deposit_preauth, ltDEPOSIT_PREAUTH}, + {jss::did, ltDID}, {jss::directory, ltDIR_NODE}, {jss::escrow, ltESCROW}, {jss::fee, ltFEE_SETTINGS}, {jss::hashes, ltLEDGER_HASHES}, + {jss::nunl, ltNEGATIVE_UNL}, + {jss::oracle, ltORACLE}, + {jss::nft_offer, ltNFTOKEN_OFFER}, + {jss::nft_page, ltNFTOKEN_PAGE}, {jss::offer, ltOFFER}, {jss::payment_channel, ltPAYCHAN}, {jss::signer_list, ltSIGNER_LIST}, {jss::state, ltRIPPLE_STATE}, {jss::ticket, ltTICKET}, - {jss::nft_offer, ltNFTOKEN_OFFER}, - {jss::nft_page, ltNFTOKEN_PAGE}, - {jss::amm, ltAMM}, - {jss::bridge, ltBRIDGE}, {jss::xchain_owned_claim_id, ltXCHAIN_OWNED_CLAIM_ID}, {jss::xchain_owned_create_account_claim_id, - ltXCHAIN_OWNED_CREATE_ACCOUNT_CLAIM_ID}, - {jss::did, ltDID}, - {jss::oracle, ltORACLE}}}; + ltXCHAIN_OWNED_CREATE_ACCOUNT_CLAIM_ID}}}; auto const& p = params[jss::type]; if (!p.isString()) diff --git a/src/test/app/AMMExtended_test.cpp b/src/test/app/AMMExtended_test.cpp index 8c7cbce92f..91e8ffa53d 100644 --- a/src/test/app/AMMExtended_test.cpp +++ b/src/test/app/AMMExtended_test.cpp @@ -3271,7 +3271,7 @@ private: if (!BEAST_EXPECT(checkArraySize(affected, 4u))) return; auto ff = - affected[2u][sfModifiedNode.fieldName][sfFinalFields.fieldName]; + affected[1u][sfModifiedNode.fieldName][sfFinalFields.fieldName]; BEAST_EXPECT( ff[sfHighLimit.fieldName] == bob["USD"](100).value().getJson(JsonOptions::none)); diff --git a/src/test/app/TxQ_test.cpp b/src/test/app/TxQ_test.cpp index 31e0b7b812..4ddb15a145 100644 --- a/src/test/app/TxQ_test.cpp +++ b/src/test/app/TxQ_test.cpp @@ -2806,6 +2806,12 @@ public: { // This test focuses on which gaps in queued transactions are // allowed to be filled even when the account's queue is full. + + // NOTE: This test is fragile and dependent on ordering of + // transactions, which is affected by the closed/validated + // ledger hash. This test may need to be edited if changes + // are made that impact the ledger hash. + // TODO: future-proof this test. using namespace jtx; testcase("full queue gap handling"); @@ -2946,9 +2952,9 @@ public: // may not reduce to 8. env.close(); checkMetrics(__LINE__, env, 9, 50, 6, 5, 256); - BEAST_EXPECT(env.seq(alice) == aliceSeq + 13); + BEAST_EXPECT(env.seq(alice) == aliceSeq + 15); - // Close ledger 7. That should remove 7 more of alice's transactions. + // Close ledger 7. That should remove 4 more of alice's transactions. env.close(); checkMetrics(__LINE__, env, 2, 60, 7, 6, 256); BEAST_EXPECT(env.seq(alice) == aliceSeq + 19); diff --git a/src/test/ledger/Directory_test.cpp b/src/test/ledger/Directory_test.cpp index 8274cecd23..42dcdab9bd 100644 --- a/src/test/ledger/Directory_test.cpp +++ b/src/test/ledger/Directory_test.cpp @@ -396,6 +396,96 @@ struct Directory_test : public beast::unit_test::suite } } + void + testPreviousTxnID() + { + testcase("fixPreviousTxnID"); + using namespace jtx; + + auto const gw = Account{"gateway"}; + auto const alice = Account{"alice"}; + auto const USD = gw["USD"]; + + auto ledger_data = [this](Env& env) { + Json::Value params; + params[jss::type] = jss::directory; + params[jss::ledger_index] = "validated"; + auto const result = + env.rpc("json", "ledger_data", to_string(params))[jss::result]; + BEAST_EXPECT(!result.isMember(jss::marker)); + return result; + }; + + // fixPreviousTxnID is disabled. + Env env(*this, supported_amendments() - fixPreviousTxnID); + env.fund(XRP(10000), alice, gw); + env.close(); + env.trust(USD(1000), alice); + env(pay(gw, alice, USD(1000))); + env.close(); + + { + auto const jrr = ledger_data(env); + auto const& jstate = jrr[jss::state]; + BEAST_EXPECTS(checkArraySize(jstate, 2), jrr.toStyledString()); + for (auto const& directory : jstate) + { + BEAST_EXPECT( + directory["LedgerEntryType"] == + jss::DirectoryNode); // sanity check + // The PreviousTxnID and PreviousTxnLgrSeq fields should not be + // on the DirectoryNode object when the amendment is disabled + BEAST_EXPECT(!directory.isMember("PreviousTxnID")); + BEAST_EXPECT(!directory.isMember("PreviousTxnLgrSeq")); + } + } + + // Now enable the amendment so the directory node is updated. + env.enableFeature(fixPreviousTxnID); + env.close(); + + // Make sure the `PreviousTxnID` and `PreviousTxnLgrSeq` fields now + // exist + env(offer(alice, XRP(1), USD(1))); + auto const txID = to_string(env.tx()->getTransactionID()); + auto const ledgerSeq = env.current()->info().seq; + env.close(); + // Make sure the fields only exist if the object is touched + env(noop(gw)); + env.close(); + + { + auto const jrr = ledger_data(env); + auto const& jstate = jrr[jss::state]; + BEAST_EXPECTS(checkArraySize(jstate, 3), jrr.toStyledString()); + for (auto const& directory : jstate) + { + BEAST_EXPECT( + directory["LedgerEntryType"] == + jss::DirectoryNode); // sanity check + if (directory[jss::Owner] == gw.human()) + { + // gw's directory did not get touched, so it + // should not have those fields populated + BEAST_EXPECT(!directory.isMember("PreviousTxnID")); + BEAST_EXPECT(!directory.isMember("PreviousTxnLgrSeq")); + } + else + { + // All of the other directories, including the order + // book, did get touched, so they should have those + // fields + BEAST_EXPECT( + directory.isMember("PreviousTxnID") && + directory["PreviousTxnID"].asString() == txID); + BEAST_EXPECT( + directory.isMember("PreviousTxnLgrSeq") && + directory["PreviousTxnLgrSeq"].asUInt() == ledgerSeq); + } + } + } + } + void run() override { @@ -403,6 +493,7 @@ struct Directory_test : public beast::unit_test::suite testDirIsEmpty(); testRipd1353(); testEmptyChain(); + testPreviousTxnID(); } }; diff --git a/src/test/rpc/TransactionEntry_test.cpp b/src/test/rpc/TransactionEntry_test.cpp index 5eddb640cb..431c812d62 100644 --- a/src/test/rpc/TransactionEntry_test.cpp +++ b/src/test/rpc/TransactionEntry_test.cpp @@ -328,7 +328,7 @@ class TransactionEntry_test : public beast::unit_test::suite "TransactionType" : "Payment", "TxnSignature" : "3044022033D9EBF7F02950AF2F6B13C07AEE641C8FEBDD540A338FCB9027A965A4AED35B02206E4E227DCC226A3456C0FEF953449D21645A24EB63CA0BB7C5B62470147FD1D1", })", - "39AA166131D56622EFD96CB4B2BD58003ACD37091C90977FF6B81419DB451775", + "3A6E375BFDFF029A571AFBB3BC46C4F52963FAF043B406D0E59A7194C1A8F98E", "2000-01-01T00:00:20Z"); check_tx( @@ -350,7 +350,7 @@ class TransactionEntry_test : public beast::unit_test::suite "TransactionType" : "Payment", "TxnSignature" : "30450221008A722B7F16EDB2348886E88ED4EC682AE9973CC1EE0FF37C93BB2CEC821D3EDF022059E464472031BA5E0D88A93E944B6A8B8DB3E1D5E5D1399A805F615789DB0BED", })", - "39AA166131D56622EFD96CB4B2BD58003ACD37091C90977FF6B81419DB451775", + "3A6E375BFDFF029A571AFBB3BC46C4F52963FAF043B406D0E59A7194C1A8F98E", "2000-01-01T00:00:20Z"); env(offer(A2, XRP(100), A2["USD"](1))); @@ -379,7 +379,7 @@ class TransactionEntry_test : public beast::unit_test::suite "TransactionType" : "OfferCreate", "TxnSignature" : "304502210093FC93ACB77B4E3DE3315441BD010096734859080C1797AB735EB47EBD541BD102205020BB1A7C3B4141279EE4C287C13671E2450EA78914EFD0C6DB2A18344CD4F2", })", - "0589B876DF5AFE335781E8FC12C2EC62A80151DF13BBAFE9EB2DA62E798ED434", + "73D6C8E66E0DC22F3E6F7D39BF795A6831BEB412823A986C7CC19470C93557C0", "2000-01-01T00:00:30Z"); }