From c8af20242584a865a8cc94d1e771c543590ab9d0 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Fri, 12 Apr 2024 14:51:13 +0100 Subject: [PATCH] respond to comments --- src/ripple/ledger/impl/ApplyStateTable.cpp | 3 +- src/ripple/protocol/STLedgerEntry.h | 2 +- src/ripple/protocol/impl/STLedgerEntry.cpp | 6 +- src/test/ledger/Directory_test.cpp | 82 ++++++++++++++-------- 4 files changed, 60 insertions(+), 33 deletions(-) diff --git a/src/ripple/ledger/impl/ApplyStateTable.cpp b/src/ripple/ledger/impl/ApplyStateTable.cpp index 161dc922271..ab2f0ca8cdb 100644 --- a/src/ripple/ledger/impl/ApplyStateTable.cpp +++ b/src/ripple/ledger/impl/ApplyStateTable.cpp @@ -21,10 +21,8 @@ #include #include #include -#include #include #include -#include namespace ripple { namespace detail { @@ -615,6 +613,7 @@ ApplyStateTable::threadTx( return; } // threadItem only applied to AccountRoot + assert(sle->isThreadedType(base.rules())); threadItem(meta, sle); } diff --git a/src/ripple/protocol/STLedgerEntry.h b/src/ripple/protocol/STLedgerEntry.h index abfef0c3a80..6fd50aa154e 100644 --- a/src/ripple/protocol/STLedgerEntry.h +++ b/src/ripple/protocol/STLedgerEntry.h @@ -21,11 +21,11 @@ #define RIPPLE_PROTOCOL_STLEDGERENTRY_H_INCLUDED #include -#include #include namespace ripple { +class Rules; class Invariants_test; class STLedgerEntry final : public STObject, public CountedObject diff --git a/src/ripple/protocol/impl/STLedgerEntry.cpp b/src/ripple/protocol/impl/STLedgerEntry.cpp index ec3febe4a52..10ec5627aa3 100644 --- a/src/ripple/protocol/impl/STLedgerEntry.cpp +++ b/src/ripple/protocol/impl/STLedgerEntry.cpp @@ -17,13 +17,15 @@ */ //============================================================================== +#include + #include #include #include #include #include #include -#include +#include #include #include #include @@ -138,7 +140,7 @@ STLedgerEntry::isThreadedType(Rules const& rules) const newPreviousTxnIDTypes.cbegin(), newPreviousTxnIDTypes.cend(), type_); - return getFieldIndex(sfPreviousTxnID) != -1 && !excludePrevTxnID; + return !excludePrevTxnID && getFieldIndex(sfPreviousTxnID) != -1; } bool diff --git a/src/test/ledger/Directory_test.cpp b/src/test/ledger/Directory_test.cpp index 7a0d6bc9d38..42dcdab9bd1 100644 --- a/src/test/ledger/Directory_test.cpp +++ b/src/test/ledger/Directory_test.cpp @@ -406,28 +406,30 @@ struct Directory_test : public beast::unit_test::suite auto const alice = Account{"alice"}; auto const USD = gw["USD"]; - auto ledger_data = [](Env& env) { + auto ledger_data = [this](Env& env) { Json::Value params; params[jss::type] = jss::directory; params[jss::ledger_index] = "validated"; - return env.rpc( - "json", "ledger_data", to_string(params))[jss::result]; + 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(); + // 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) { - auto const jrr = ledger_data(env); - BEAST_EXPECTS( - checkArraySize(jrr[jss::state], 2), jrr.toStyledString()); - auto const directory = jrr[jss::state][0u]; BEAST_EXPECT( directory["LedgerEntryType"] == jss::DirectoryNode); // sanity check @@ -436,26 +438,50 @@ struct Directory_test : public beast::unit_test::suite 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(); + // 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))); - 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) { - auto const jrr = ledger_data(env); - BEAST_EXPECTS( - checkArraySize(jrr[jss::state], 3), jrr.toStyledString()); - auto const directory = jrr[jss::state][0u]; BEAST_EXPECT( directory["LedgerEntryType"] == jss::DirectoryNode); // sanity check - BEAST_EXPECT(directory.isMember("PreviousTxnID")); - BEAST_EXPECT(directory.isMember("PreviousTxnLgrSeq")); + 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); + } } } }