From 65ff61d51892c55961d9ab71bd68a646481389b6 Mon Sep 17 00:00:00 2001 From: Niq Dudfield Date: Tue, 8 Sep 2026 10:39:45 +0700 Subject: [PATCH 01/11] fix: fail fast when amendment blocked instead of degraded state (#707) --- src/test/rpc/AmendmentBlocked_test.cpp | 543 +++++++++++++++++++ src/xrpld/app/ledger/detail/LedgerMaster.cpp | 40 +- src/xrpld/app/main/Application.cpp | 60 ++ src/xrpld/app/main/Application.h | 26 + src/xrpld/app/main/Main.cpp | 25 + src/xrpld/app/misc/AmendmentTable.h | 18 + src/xrpld/app/misc/NetworkOPs.cpp | 111 +++- src/xrpld/app/misc/detail/AmendmentTable.cpp | 40 ++ 8 files changed, 856 insertions(+), 7 deletions(-) diff --git a/src/test/rpc/AmendmentBlocked_test.cpp b/src/test/rpc/AmendmentBlocked_test.cpp index 196ce0e463..54086d7716 100644 --- a/src/test/rpc/AmendmentBlocked_test.cpp +++ b/src/test/rpc/AmendmentBlocked_test.cpp @@ -19,15 +19,548 @@ #include #include +#include +#include +#include +#include +#include +#include #include +#include #include +#include +#include +#include +#include #include +#include +#include #include +#include +#include +#include +#include +#include +#include +#include +#include namespace ripple { class AmendmentBlocked_test : public beast::unit_test::suite { + // An amendment id this binary will never support. + static uint256 + unsupportedAmendmentId() + { + std::string const in = "AmendmentBlocked_test.unsupported"; + sha256_hasher h; + using beast::hash_append; + hash_append(h, in); + auto const d = static_cast(h); + uint256 result; + std::memcpy(result.data(), d.data(), d.size()); + return result; + } + + // The majority period only sets how far out activation is expected, so + // shorten it from two weeks. That keeps the ledger close-time jumps in + // these tests down to minutes, and makes the relationship to the + // five-minute shutdown lead time in LedgerMaster obvious. + static std::unique_ptr + shortMajorityConfig() + { + using namespace std::chrono_literals; + auto cfg = test::jtx::envconfig(); + cfg->AMENDMENT_MAJORITY_TIME = 15min; + return cfg; + } + + // Give the amendment table a majority, as of now, for an amendment we do + // not support; activation is then expected one majority period out. + // Bypasses the ReadView overload (and therefore needValidatedLedger) on + // purpose: because lastUpdateSeq_ is set to the current sequence, later + // closes within the same 256-ledger block will not recompute -- and so + // will not clear -- what we inject here. + void + injectUnsupportedMajority(test::jtx::Env& env) + { + auto const seq = env.closed()->info().seq; + majorityAmendments_t majority; + majority[unsupportedAmendmentId()] = env.now(); + env.app().getAmendmentTable().doValidatedLedger(seq, {}, majority); + + auto const first = + env.app().getAmendmentTable().firstUnsupportedExpected(); + BEAST_EXPECT( + first && + *first == env.now() + env.app().config().AMENDMENT_MAJORITY_TIME); + } + + void + testReceiptFileHelpers() + { + testcase("amendment blocked receipt helpers"); + + auto const id = unsupportedAmendmentId(); + + { + beast::temp_dir td; + Config cfg; + cfg.CONFIG_DIR = td.path(); + + auto const path = amendmentBlockedFilePath(cfg); + BEAST_EXPECT(path.filename() == "README_AMENDMENT_BLOCKED"); + BEAST_EXPECT( + path.parent_path() == boost::filesystem::path{td.path()}); + BEAST_EXPECT(!boost::filesystem::exists(path)); + + // Nothing to remove yet, and that is not an error. + boost::system::error_code ec; + BEAST_EXPECT(!removeAmendmentBlockedFile(cfg, ec)); + BEAST_EXPECT(!ec); + + BEAST_EXPECT(!writeAmendmentBlockedFile( + cfg, {to_string(id) + " (already active)"})); + BEAST_EXPECT(boost::filesystem::exists(path)); + + auto const contents = getFileContents(ec, path); + BEAST_EXPECT(!ec); + BEAST_EXPECT( + contents.find("XAHAUD STOPPED: UPGRADE REQUIRED") == 0); + // When it stopped, what was running, and what it choked on. + BEAST_EXPECT(contents.find("Stopped at:") != std::string::npos); + BEAST_EXPECT( + contents.find(BuildInfo::getVersionString()) != + std::string::npos); + BEAST_EXPECT(contents.find(to_string(id)) != std::string::npos); + BEAST_EXPECT(contents.find("already active") != std::string::npos); + BEAST_EXPECT(contents.find("Upgrade xahaud") != std::string::npos); + // The receipt is self-clearing, so it must not tell the operator + // to delete anything. + BEAST_EXPECT( + contents.find("removed automatically") != std::string::npos); + + // The timestamp is rendered, not a placeholder: to_string_iso + // gives YYYY-MM-DDTHH:MM:SSZ. + auto const stampAt = contents.find("Stopped at:"); + if (BEAST_EXPECT(stampAt != std::string::npos)) + { + auto const eol = contents.find('\n', stampAt); + auto const line = contents.substr(stampAt, eol - stampAt); + BEAST_EXPECT(line.find("20") != std::string::npos); + BEAST_EXPECT(line.find('T') != std::string::npos); + BEAST_EXPECT(line.find('Z') != std::string::npos); + } + + // Now it can be removed, and removal is reported. + ec.clear(); + BEAST_EXPECT(removeAmendmentBlockedFile(cfg, ec)); + BEAST_EXPECT(!ec); + BEAST_EXPECT(!boost::filesystem::exists(path)); + + // With no amendments to name the receipt still says something + // useful. + BEAST_EXPECT(!writeAmendmentBlockedFile(cfg, {})); + ec.clear(); + auto const bare = getFileContents(ec, path); + BEAST_EXPECT(!ec); + BEAST_EXPECT( + bare.find("not supported by this build") != std::string::npos); + } + + // A directory we cannot write to must surface an error rather than + // throw. The shutdown continues either way; only the receipt is lost, + // which is what happens on installs where CONFIG_DIR is read-only for + // the account xahaud runs as. + { + Config cfg; + cfg.CONFIG_DIR = + boost::filesystem::path{"/"} / "no" / "such" / "directory"; + BEAST_EXPECT(!!writeAmendmentBlockedFile(cfg, {})); + + boost::system::error_code ec; + BEAST_EXPECT(!removeAmendmentBlockedFile(cfg, ec)); + } + } + + void + testStandaloneDoesNotStop() + { + testcase("standalone does not stop or leave a receipt"); + using namespace test::jtx; + + beast::temp_dir td; + Env env{*this, envconfig([&](std::unique_ptr cfg) { + cfg->CONFIG_DIR = td.path(); + return cfg; + })}; + BEAST_EXPECT(env.app().config().standalone()); + + auto const path = amendmentBlockedFilePath(env.app().config()); + env.app().getOPs().setAmendmentBlocked(); + + BEAST_EXPECT(env.app().getOPs().isAmendmentBlocked()); + BEAST_EXPECT(!env.app().getOPs().isAmendmentWarned()); + BEAST_EXPECT(!env.app().isStopping()); + BEAST_EXPECT(!boost::filesystem::exists(path)); + } + + void + testShutdownOnBlock() + { + testcase("amendment blocked stops the server"); + using namespace test::jtx; + + // A non-standalone Env is needed to exercise shutdown rather than + // only setting the blocked flag. Consequences when editing: + // setup() arms the state timer, run() arms the deadlock detector, and + // signalStop() below releases run() to tear the application down + // concurrently with the rest of this function. Do all the assertions + // straight away and let the Env go out of scope promptly. + beast::temp_dir td; + Env env{*this, envconfig([&](std::unique_ptr config) { + config->NODE_SIZE = 0; + config->setupControl(true, true, false); + // setupControl picks a production node size for + // non-standalone; put it back to "tiny" for the test. + config->NODE_SIZE = 0; + config->CONFIG_DIR = td.path(); + config->legacy("database_path", td.path()); + return config; + })}; + BEAST_EXPECT(!env.app().config().standalone()); + + auto const path = amendmentBlockedFilePath(env.app().config()); + BEAST_EXPECT(path.filename() == "README_AMENDMENT_BLOCKED"); + BEAST_EXPECT(!boost::filesystem::exists(path)); + BEAST_EXPECT(!env.app().isStopping()); + + // Make the table report an unsupported amendment so the receipt has + // something concrete to name. + auto const id = unsupportedAmendmentId(); + env.app().getAmendmentTable().enable(id); + BEAST_EXPECT(env.app().getAmendmentTable().hasUnsupportedEnabled()); + + env.app().getOPs().setAmendmentBlocked(); + BEAST_EXPECT(env.app().getOPs().isAmendmentBlocked()); + BEAST_EXPECT(env.app().isStopping()); + BEAST_EXPECT(boost::filesystem::exists(path)); + + boost::system::error_code readError; + auto const contents = getFileContents(readError, path); + BEAST_EXPECT(!readError); + BEAST_EXPECT( + contents.find("XAHAUD STOPPED: UPGRADE REQUIRED") != + std::string::npos); + BEAST_EXPECT(contents.find("Upgrade xahaud") != std::string::npos); + BEAST_EXPECT(contents.find(to_string(id)) != std::string::npos); + BEAST_EXPECT(contents.find("already active") != std::string::npos); + + // Repeat calls are a no-op: Change::applyAmendment reaches this from + // the transaction apply path without an isBlocked() guard, so it must + // not rewrite the receipt or re-log once per ledger. + BEAST_EXPECT(boost::filesystem::remove(path)); + env.app().getOPs().setAmendmentBlocked(); + BEAST_EXPECT(!boost::filesystem::exists(path)); + } + + void + checkJump(bool unsupported) + { + using namespace test::jtx; + + beast::temp_dir td; + Env env{*this, envconfig([&](std::unique_ptr cfg) { + cfg->setupControl(true, true, false); + cfg->NODE_SIZE = 0; + cfg->CONFIG_DIR = td.path(); + cfg->legacy("database_path", td.path()); + return cfg; + })}; + auto& app = env.app(); + auto& ops = app.getOPs(); + auto& master = app.getLedgerMaster(); + std::lock_guard lock(app.getMasterMutex()); + auto const before = master.getClosedLedger(); + + // A direct child is not preferred over our LCL: consensus may be + // about to build it. Two ledgers ahead forces the JUMP path. + auto parent = + std::make_shared(*before, app.timeKeeper().closeTime()); + parent->updateSkipList(); + parent->setImmutable(); + auto candidate = + std::make_shared(*parent, app.timeKeeper().closeTime()); + candidate->updateSkipList(); + + if (unsupported) + { + auto const key = keylet::amendments(); + auto const existing = candidate->read(key); + auto sle = existing ? std::make_shared(*existing) + : std::make_shared(key); + STVector256 amendments; + if (sle->isFieldPresent(sfAmendments)) + amendments = sle->getFieldV256(sfAmendments); + amendments.push_back(unsupportedAmendmentId()); + sle->setFieldV256(sfAmendments, amendments); + if (existing) + candidate->rawReplace(sle); + else + candidate->rawInsert(sle); + } + + // A serialized uint32 with an unknown field number. Insert raw bytes + // so the real transaction parser, reached through TxQ, must throw. + BEAST_EXPECT(SField::getField(STI_UINT32, 255).isInvalid()); + auto tx = std::make_shared(); + tx->add8(0x20); + tx->add8(255); + while (tx->getDataLength() < txMinSizeBytes) + tx->add8(0); + auto meta = std::make_shared(); + meta->add8(0xE1); + auto const txID = sha512Half(tx->slice()); + candidate->rawTxInsert(txID, tx, meta); + candidate->setImmutable(); + + bool unknownField = false; + try + { + candidate->txRead(txID); + } + catch (std::runtime_error const& e) + { + unknownField = std::string(e.what()).starts_with("Unknown field"); + } + if (!BEAST_EXPECT(unknownField)) + return; + + master.storeLedger(candidate); + auto const keys = randomKeyPair(KeyType::secp256k1); + auto const nodeID = calcNodeID(keys.first); + auto validation = std::make_shared( + app.timeKeeper().closeTime(), + keys.first, + keys.second, + nodeID, + [&](STValidation& v) { + v.setFieldH256(sfLedgerHash, candidate->info().hash); + v.setFieldU32(sfLedgerSequence, candidate->seq()); + }); + // Add directly so ledger acceptance does not discover the amendment + // before JUMP has a chance to inspect the candidate itself. + BEAST_EXPECT( + app.getValidations().add(nodeID, RCLValidation{validation}) == + ValStatus::current); + if (!BEAST_EXPECT( + app.getValidations().getPreferredLCL( + RCLValidatedLedger{before, env.journal}, + master.getValidLedgerIndex(), + {}) == candidate->info().hash)) + return; + + BEAST_EXPECT(!app.getAmendmentTable().hasUnsupportedEnabled()); + BEAST_EXPECT(!app.getAmendmentTable().firstUnsupportedExpected()); + BEAST_EXPECT(!ops.isAmendmentBlocked()); + BEAST_EXPECT(!app.isStopping()); + auto const receipt = amendmentBlockedFilePath(app.config()); + BEAST_EXPECT(!boost::filesystem::exists(receipt)); + + bool threw = false; + try + { + ops.endConsensus({}); + } + catch (std::runtime_error const& e) + { + threw = true; + BEAST_EXPECT(std::string(e.what()).starts_with("Unknown field")); + } + BEAST_EXPECT(threw == !unsupported); + BEAST_EXPECT(ops.isAmendmentBlocked() == unsupported); + BEAST_EXPECT(app.isStopping() == unsupported); + BEAST_EXPECT(boost::filesystem::exists(receipt) == unsupported); + BEAST_EXPECT( + master.getClosedLedger()->info().hash == before->info().hash); + } + + void + testJumpStopsForUnsupportedAmendment() + { + testcase("JUMP to an unsupported ledger stops the server"); + checkJump(/*unsupported=*/true); + } + + void + testJumpRethrowsParsingError() + { + testcase("JUMP parsing errors without unsupported amendments escape"); + checkJump(/*unsupported=*/false); + } + + void + testWarnsOutsideShutdownWindow() + { + testcase("unsupported majority warns while activation is distant"); + using namespace test::jtx; + + Env env{*this, shortMajorityConfig()}; + BEAST_EXPECT(!env.app().getOPs().isBlocked()); + + BEAST_EXPECT(env.close()); + injectUnsupportedMajority(env); + BEAST_EXPECT(env.close()); + + // A majority period out: warn, keep running. + BEAST_EXPECT(env.app().getOPs().isAmendmentWarned()); + BEAST_EXPECT(!env.app().getOPs().isAmendmentBlocked()); + + // The table can name the amendment, and reports it as pending rather + // than active. This is what ends up in the receipt. + auto const unsupported = + env.app().getAmendmentTable().unsupportedAmendments(); + BEAST_EXPECT(unsupported.size() == 1); + if (unsupported.size() == 1) + { + BEAST_EXPECT(unsupported[0].id == unsupportedAmendmentId()); + BEAST_EXPECT( + unsupported[0].expected == + env.app().getAmendmentTable().firstUnsupportedExpected()); + } + + // Because firstUnsupportedExpected() is set, the warning carries the + // expected activation date. + auto const si = env.rpc("server_info")[jss::result]; + BEAST_EXPECT(si.isMember(jss::info)); + auto const& warnings = si[jss::info][jss::warnings]; + BEAST_EXPECT(warnings.isArray() && warnings.size() == 1); + if (warnings.isArray() && warnings.size() == 1) + { + BEAST_EXPECT( + warnings[0u][jss::id].asInt() == warnRPC_UNSUPPORTED_MAJORITY); + auto const& details = warnings[0u][jss::details]; + BEAST_EXPECT(details.isMember(jss::expected_date)); + BEAST_EXPECT(details.isMember(jss::expected_date_UTC)); + } + } + + void + testUnsupportedAmendmentReporting() + { + testcase("unsupported amendments are reported for the receipt"); + using namespace test::jtx; + + Env env{*this, shortMajorityConfig()}; + auto& table = env.app().getAmendmentTable(); + BEAST_EXPECT(table.unsupportedAmendments().empty()); + + // Majority, not yet active -> reported with an expected time. + BEAST_EXPECT(env.close()); + injectUnsupportedMajority(env); + auto pending = table.unsupportedAmendments(); + BEAST_EXPECT(pending.size() == 1); + if (pending.size() == 1) + BEAST_EXPECT(pending[0].expected.has_value()); + + // Majority lost -> nothing to report. The list is recomputed from the + // ledger each time, so it must not accumulate. + auto const seq = env.closed()->info().seq; + table.doValidatedLedger(seq, {}, {}); + BEAST_EXPECT(table.unsupportedAmendments().empty()); + BEAST_EXPECT(!table.firstUnsupportedExpected()); + + // Active -> reported with no expected time. + BEAST_EXPECT(table.enable(unsupportedAmendmentId())); + BEAST_EXPECT(table.hasUnsupportedEnabled()); + auto const active = table.unsupportedAmendments(); + BEAST_EXPECT(active.size() == 1); + if (active.size() == 1) + { + BEAST_EXPECT(active[0].id == unsupportedAmendmentId()); + BEAST_EXPECT(!active[0].expected); + } + } + + void + testWarningVisibleWithoutAdmin() + { + testcase("unsupported majority warning is not admin only"); + using namespace test::jtx; + + // No closes here: ledger_accept is Role::ADMIN, server_info is + // Role::USER, which is the whole point of the test. + Env env{*this, envconfig(no_admin)}; + env.app().getOPs().setAmendmentWarned(); + BEAST_EXPECT(env.app().getOPs().isAmendmentWarned()); + + auto const si = env.rpc("server_info")[jss::result]; + BEAST_EXPECT(si.isMember(jss::info)); + auto const& warnings = si[jss::info][jss::warnings]; + BEAST_EXPECT(warnings.isArray() && warnings.size() == 1); + if (warnings.isArray() && warnings.size() == 1) + { + BEAST_EXPECT( + warnings[0u][jss::id].asInt() == warnRPC_UNSUPPORTED_MAJORITY); + } + } + + void + testShutsDownInsideShutdownWindow() + { + testcase("unsupported majority stops the server before activation"); + using namespace test::jtx; + + Env env{*this, shortMajorityConfig()}; + BEAST_EXPECT(!env.app().getOPs().isBlocked()); + + BEAST_EXPECT(env.close()); + injectUnsupportedMajority(env); + + // First close only warns: activation is still a majority period away. + BEAST_EXPECT(env.close()); + BEAST_EXPECT(env.app().getOPs().isAmendmentWarned()); + BEAST_EXPECT(!env.app().getOPs().isAmendmentBlocked()); + + auto const expected = + *env.app().getAmendmentTable().firstUnsupportedExpected(); + + // Close two minutes short of the expected activation time. This is not + // a flag ledger and the warning has already been issued, so the check + // has to run on every validated ledger to see this at all. + BEAST_EXPECT(env.close(expected - NetClock::duration{120})); + BEAST_EXPECT(env.app().getOPs().isAmendmentBlocked()); + BEAST_EXPECT(!env.app().getOPs().isAmendmentWarned()); + + // Standalone, so the flag is set but the server keeps running. + BEAST_EXPECT(!env.app().isStopping()); + } + + void + testShutsDownWhenActivationOverdue() + { + testcase("unsupported majority stops the server when overdue"); + using namespace test::jtx; + + Env env{*this, shortMajorityConfig()}; + BEAST_EXPECT(!env.app().getOPs().isBlocked()); + + BEAST_EXPECT(env.close()); + injectUnsupportedMajority(env); + + auto const expected = + *env.app().getAmendmentTable().firstUnsupportedExpected(); + + // firstUnsupportedExpected() is a lower bound: the amendment actually + // activates at the first flag ledger at or after it, so the server can + // legitimately still be running an hour past it. That is the most + // dangerous state, not the safest, and must stop the server. + BEAST_EXPECT(env.close(expected + NetClock::duration{3600})); + BEAST_EXPECT(env.app().getOPs().isAmendmentBlocked()); + BEAST_EXPECT(!env.app().getOPs().isAmendmentWarned()); + } + void testBlockedMethods() { @@ -250,6 +783,16 @@ public: void run() override { + testReceiptFileHelpers(); + testStandaloneDoesNotStop(); + testShutdownOnBlock(); + testJumpRethrowsParsingError(); + testJumpStopsForUnsupportedAmendment(); + testWarnsOutsideShutdownWindow(); + testUnsupportedAmendmentReporting(); + testWarningVisibleWithoutAdmin(); + testShutsDownInsideShutdownWindow(); + testShutsDownWhenActivationOverdue(); testBlockedMethods(); } }; diff --git a/src/xrpld/app/ledger/detail/LedgerMaster.cpp b/src/xrpld/app/ledger/detail/LedgerMaster.cpp index 5b3275c041..e76970c63c 100644 --- a/src/xrpld/app/ledger/detail/LedgerMaster.cpp +++ b/src/xrpld/app/ledger/detail/LedgerMaster.cpp @@ -289,14 +289,49 @@ LedgerMaster::setValidLedger(std::shared_ptr const& l) app_.getSHAMapStore().onLedgerClosed(getValidatedLedger()); mLedgerHistory.validatedLedger(l, consensusHash); app_.getAmendmentTable().doValidatedLedger(l); + + using namespace std::chrono_literals; + + // How far ahead of the expected activation time to stop the server. + // firstUnsupportedExpected() is only a lower bound on activation: the + // amendment goes live at the first flag ledger at or after that time, so + // stopping early is harmless while stopping late risks being handed a + // ledger we cannot deserialize. + constexpr auto amendmentShutdownLeadTime = 5min; + if (!app_.getOPs().isBlocked()) { + auto const firstUnsupported = + app_.getAmendmentTable().firstUnsupportedExpected(); + if (app_.getAmendmentTable().hasUnsupportedEnabled()) { JLOG(m_journal.error()) << "One or more unsupported amendments " "activated: server blocked."; app_.getOPs().setAmendmentBlocked(); } + else if ( + firstUnsupported && + app_.timeKeeper().closeTime() + amendmentShutdownLeadTime >= + *firstUnsupported) + { + // Activation is imminent, or the expected time has already passed + // and we are only waiting on the next flag ledger. Shut down now, + // while we can still deserialize the ledgers we are handed. + // + // This is deliberately checked on every validated ledger rather + // than only on flag ledgers: the lead time above is much shorter + // than the flag ledger interval, so a flag-ledger-only check would + // usually skip straight over the window. + // + // The comparison must not be written as (*first - now), because + // NetClock::rep is unsigned and wraps once the expected time is in + // the past -- which is the most dangerous case, not the safest. + JLOG(m_journal.error()) + << "Unsupported amendment expected to activate at " + << to_string(*firstUnsupported) << ". Shutting down."; + app_.getOPs().setAmendmentBlocked(); + } else if (!app_.getOPs().isAmendmentWarned() || l->isFlagLedger()) { // Amendments can lose majority, so re-check periodically (every @@ -308,12 +343,11 @@ LedgerMaster::setValidLedger(std::shared_ptr const& l) // this message may be logged more than once per session, because // the node will otherwise function normally, and this gives // operators an opportunity to see and resolve the warning. - if (auto const first = - app_.getAmendmentTable().firstUnsupportedExpected()) + if (firstUnsupported) { JLOG(m_journal.error()) << "One or more unsupported amendments " "reached majority. Upgrade before " - << to_string(*first) + << to_string(*firstUnsupported) << " to prevent your server from " "becoming amendment blocked."; app_.getOPs().setAmendmentWarned(); diff --git a/src/xrpld/app/main/Application.cpp b/src/xrpld/app/main/Application.cpp index bfbfe660d5..fb9006fe37 100644 --- a/src/xrpld/app/main/Application.cpp +++ b/src/xrpld/app/main/Application.cpp @@ -59,6 +59,7 @@ #include #include #include +#include #include #include #include @@ -2372,6 +2373,65 @@ Application::Application() : beast::PropertyStream::Source("app") //------------------------------------------------------------------------------ +boost::filesystem::path +amendmentBlockedFilePath(Config const& config) +{ + return config.CONFIG_DIR / "README_AMENDMENT_BLOCKED"; +} + +boost::system::error_code +writeAmendmentBlockedFile( + Config const& config, + std::vector const& amendments) +{ + using namespace std::chrono; + + std::ostringstream ss; + ss << "XAHAUD STOPPED: UPGRADE REQUIRED\n" + << "\n" + << "Stopped at: " + << to_string_iso(time_point_cast(system_clock::now())) << "\n" + << "This build: " << BuildInfo::getVersionString() << "\n" + << "\n"; + + if (amendments.empty()) + { + ss << "One or more network amendments are not supported by this " + "build.\n"; + } + else + { + ss << "Amendments this build does not support:\n"; + for (auto const& amendment : amendments) + ss << " " << amendment << "\n"; + } + + ss << "\n" + << "The network has moved to rules this build does not implement, so " + "the\n" + << "server stopped rather than keep serving ledgers it cannot read.\n" + << "\n" + << "To get back in sync:\n" + << "1. Upgrade xahaud to a version that supports the amendments " + "above.\n" + << "2. Start xahaud again. This file is removed automatically on " + "start.\n" + << "\n" + << "Starting this build again without upgrading will stop the server " + "again\n" + << "and rewrite this file. Nothing needs to be deleted by hand.\n"; + + boost::system::error_code ec; + writeFileContents(ec, amendmentBlockedFilePath(config), ss.str()); + return ec; +} + +bool +removeAmendmentBlockedFile(Config const& config, boost::system::error_code& ec) +{ + return boost::filesystem::remove(amendmentBlockedFilePath(config), ec); +} + std::unique_ptr make_Application( std::unique_ptr config, diff --git a/src/xrpld/app/main/Application.h b/src/xrpld/app/main/Application.h index 52460a570d..cecfed989c 100644 --- a/src/xrpld/app/main/Application.h +++ b/src/xrpld/app/main/Application.h @@ -27,9 +27,14 @@ #include #include #include +#include #include +#include #include #include +#include +#include +#include namespace ripple { @@ -279,6 +284,27 @@ make_Application( std::unique_ptr logs, std::unique_ptr timeKeeper); +/** Location of the receipt left behind when the server stops because it does + not support a network amendment. */ +boost::filesystem::path +amendmentBlockedFilePath(Config const& config); + +/** Write the amendment-blocked receipt: a record for the operator of when the + server stopped and which amendments it could not support. `amendments` + holds one already-rendered line per unsupported amendment, and may be + empty. Best effort -- any error is returned rather than thrown, and the + caller is expected to continue shutting down either way. */ +boost::system::error_code +writeAmendmentBlockedFile( + Config const& config, + std::vector const& amendments); + +/** Remove any amendment-blocked receipt left behind by a previous run. + Returns true if a receipt was present and has been removed; sets `ec` if + removal was attempted and failed. */ +bool +removeAmendmentBlockedFile(Config const& config, boost::system::error_code& ec); + } // namespace ripple #endif diff --git a/src/xrpld/app/main/Main.cpp b/src/xrpld/app/main/Main.cpp index d7b935d751..f85ece9c1d 100644 --- a/src/xrpld/app/main/Main.cpp +++ b/src/xrpld/app/main/Main.cpp @@ -809,6 +809,31 @@ run(int argc, char** argv) // No arguments. Run server. if (!vm.count("parameters")) { + // Clear any receipt left by a previous amendment-blocked shutdown. It + // records why the server stopped; it is not a lock. If this build + // still does not support the amendment it will stop again and write a + // fresh one, so there is nothing for the operator to delete by hand + // and no way for a stale receipt to keep a working build down. + // Standalone does not write receipts, so it does not clear them + // either, leaving the file readable for diagnosis. + if (!config->standalone()) + { + boost::system::error_code ec; + auto const blockedFile = amendmentBlockedFilePath(*config); + if (removeAmendmentBlockedFile(*config, ec)) + { + JLOG(logs->journal("Application").warn()) + << "Removed amendment-blocked receipt " << blockedFile + << " left by a previous run."; + } + else if (ec) + { + JLOG(logs->journal("Application").warn()) + << "Could not remove amendment-blocked receipt " + << blockedFile << ": " << ec.message(); + } + } + // TODO: this comment can be removed in a future release - // say 1.7 or higher if (config->had_trailing_comments()) diff --git a/src/xrpld/app/misc/AmendmentTable.h b/src/xrpld/app/misc/AmendmentTable.h index d6193adca2..07a0dad246 100644 --- a/src/xrpld/app/misc/AmendmentTable.h +++ b/src/xrpld/app/misc/AmendmentTable.h @@ -27,6 +27,7 @@ #include #include +#include namespace ripple { @@ -50,6 +51,17 @@ public: VoteBehavior const vote; }; + /** An amendment seen on the network that this server has no code + support for. */ + struct UnsupportedAmendment + { + uint256 id; + + /** The time the amendment is expected to activate. Unset if it is + already active. */ + std::optional expected; + }; + virtual ~AmendmentTable() = default; virtual uint256 @@ -80,6 +92,12 @@ public: virtual std::optional firstUnsupportedExpected() const = 0; + /** Amendments this server does not support that are already enabled, or + that have reached majority and are expected to activate. Ordered by + amendment id. Intended for operator-facing diagnostics. */ + virtual std::vector + unsupportedAmendments() const = 0; + virtual Json::Value getJson(bool isAdmin) const = 0; diff --git a/src/xrpld/app/misc/NetworkOPs.cpp b/src/xrpld/app/misc/NetworkOPs.cpp index a2c80eb6b0..75525d276d 100644 --- a/src/xrpld/app/misc/NetworkOPs.cpp +++ b/src/xrpld/app/misc/NetworkOPs.cpp @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -1701,11 +1702,68 @@ NetworkOPsImp::isAmendmentBlocked() return amendmentBlocked_; } +// Render the unsupported amendments for the operator-facing receipt. This is +// the only identification available: an amendment this build does not support +// has no name here, so the id is what the operator matches against the +// release notes. +static std::vector +describeUnsupportedAmendments(AmendmentTable const& table) +{ + std::vector lines; + + for (auto const& amendment : table.unsupportedAmendments()) + { + std::ostringstream ss; + ss << to_string(amendment.id); + if (amendment.expected) + ss << " (expected to activate " + << to_string_iso(*amendment.expected) << ")"; + else + ss << " (already active)"; + lines.push_back(ss.str()); + } + + return lines; +} + void NetworkOPsImp::setAmendmentBlocked() { - amendmentBlocked_ = true; + // Idempotent: this is reached from Change::applyAmendment (i.e. from the + // transaction application path, which is not guarded by isBlocked()) as + // well as from LedgerMaster::setValidLedger. Writing the receipt and + // logging once per process is enough, and it keeps the synchronous file + // write out of any subsequent ledger apply. + if (amendmentBlocked_.exchange(true)) + return; + setMode(OperatingMode::CONNECTED); + if (!app_.config().standalone()) + { + auto const blockedFile = amendmentBlockedFilePath(app_.config()); + if (auto const ec = writeAmendmentBlockedFile( + app_.config(), + describeUnsupportedAmendments(app_.getAmendmentTable()))) + { + JLOG(m_journal.fatal()) + << "Could not write amendment-blocked receipt " << blockedFile + << ": " << ec.message(); + } + else + { + JLOG(m_journal.fatal()) + << "Amendment-blocked receipt written to " << blockedFile; + } + JLOG(m_journal.fatal()) + << "This version of xahaud does not support a network amendment. " + "The amendment will activate soon or is already active. " + "The server will stop. Upgrade xahaud before you restart the " + "server."; + app_.signalStop( + "Unsupported network amendment. Upgrade xahaud before you restart " + "the server. Details: " + + blockedFile.string()); + } } inline bool @@ -1851,6 +1909,27 @@ NetworkOPsImp::checkLastClosedLedger( return true; } +// True if `view` enables an amendment this binary does not implement. Reads +// only the amendments ledger entry, so it is safe to call once transaction +// deserialization is already known to be failing. +static bool +ledgerHasUnsupportedAmendments( + AmendmentTable const& table, + ReadView const& view) +{ + auto const sle = view.read(keylet::amendments()); + if (!sle || !sle->isFieldPresent(sfAmendments)) + return false; + + for (auto const& amendment : sle->getFieldV256(sfAmendments)) + { + if (!table.isSupported(amendment)) + return true; + } + + return false; +} + void NetworkOPsImp::switchLastClosedLedger( std::shared_ptr const& newLCL) @@ -1861,8 +1940,32 @@ NetworkOPsImp::switchLastClosedLedger( clearNeedNetworkLedger(); - // Update fee computations. - app_.getTxQ().processClosedLedger(app_, *newLCL, true); + // Update fee computations. May throw if the ledger contains + // transactions with fields unknown to this binary (e.g. after an + // unsupported amendment activates). Catch to allow graceful shutdown. + try + { + app_.getTxQ().processClosedLedger(app_, *newLCL, true); + } + catch (std::runtime_error const& e) + { + // Do not decide this on amendmentBlocked_ alone. A JUMP can happen + // before any validated ledger has been processed -- e.g. immediately + // after a restart, which is precisely the case that crashed -- so the + // flag may still be clear here. Ask the ledger itself instead. + // Anything else is a real bug and must propagate. + if (!amendmentBlocked_ && + !ledgerHasUnsupportedAmendments(app_.getAmendmentTable(), *newLCL)) + throw; + + JLOG(m_journal.error()) << "Failed to process closed ledger " + << newLCL->info().seq << ": " << e.what(); + + // No-op if we are already blocked; otherwise this starts the + // shutdown that should have been started before activation. + setAmendmentBlocked(); + return; + } // Caller must own master lock { @@ -2542,7 +2645,7 @@ NetworkOPsImp::getServerInfo(bool human, bool admin, bool counters) "may be incorrectly configured or some [validator_list_sites] " "may be unreachable."; } - if (admin && isAmendmentWarned()) + if (isAmendmentWarned()) { Json::Value& w = warnings.append(Json::objectValue); w[jss::id] = warnRPC_UNSUPPORTED_MAJORITY; diff --git a/src/xrpld/app/misc/detail/AmendmentTable.cpp b/src/xrpld/app/misc/detail/AmendmentTable.cpp index d7a5ae8247..bc8229f2a9 100644 --- a/src/xrpld/app/misc/detail/AmendmentTable.cpp +++ b/src/xrpld/app/misc/detail/AmendmentTable.cpp @@ -432,6 +432,11 @@ private: // will be enabled. std::optional firstUnsupportedExpected_; + // Unsupported amendments that have reached majority, and the time each + // is expected to activate. Recomputed alongside + // firstUnsupportedExpected_, so it clears when majority is lost. + std::vector> unsupportedMajority_; + beast::Journal const j_; // Database which persists veto/unveto vote @@ -495,6 +500,9 @@ public: std::optional firstUnsupportedExpected() const override; + std::vector + unsupportedAmendments() const override; + Json::Value getJson(bool isAdmin) const override; Json::Value @@ -807,6 +815,36 @@ AmendmentTableImpl::firstUnsupportedExpected() const return firstUnsupportedExpected_; } +std::vector +AmendmentTableImpl::unsupportedAmendments() const +{ + std::lock_guard lock(mutex_); + + std::vector result; + + // Already active. + for (auto const& [id, state] : amendmentMap_) + { + if (state.enabled && !state.supported) + result.push_back({id, std::nullopt}); + } + + // Reached majority, not yet active. + for (auto const& entry : unsupportedMajority_) + { + if (std::none_of(result.begin(), result.end(), [&entry](auto const& u) { + return u.id == entry.first; + })) + result.push_back({entry.first, entry.second}); + } + + std::sort(result.begin(), result.end(), [](auto const& a, auto const& b) { + return a.id < b.id; + }); + + return result; +} + std::vector AmendmentTableImpl::doValidation(std::set const& enabled) const { @@ -967,6 +1005,7 @@ AmendmentTableImpl::doValidatedLedger( // if it's currently set. If it's not set when the loop is done, then any // prior unknown amendments have lost majority. firstUnsupportedExpected_.reset(); + unsupportedMajority_.clear(); for (auto const& [hash, time] : majority) { AmendmentState& s = add(hash, lock); @@ -978,6 +1017,7 @@ AmendmentTableImpl::doValidatedLedger( { JLOG(j_.info()) << "Unsupported amendment " << hash << " reached majority at " << to_string(time); + unsupportedMajority_.emplace_back(hash, time + majorityTime_); if (!firstUnsupportedExpected_ || firstUnsupportedExpected_ > time) firstUnsupportedExpected_ = time; } From ddbaeedbfea1374b911c34a7fb062520a80f8326 Mon Sep 17 00:00:00 2001 From: tequ Date: Tue, 8 Sep 2026 20:41:54 +0900 Subject: [PATCH 02/11] fix guard_checker.cpp to return cost instead fee --- include/xrpl/hook/guard_checker.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/xrpl/hook/guard_checker.cpp b/include/xrpl/hook/guard_checker.cpp index 815c5f493e..001034c7eb 100644 --- a/include/xrpl/hook/guard_checker.cpp +++ b/include/xrpl/hook/guard_checker.cpp @@ -88,7 +88,7 @@ main(int argc, char** argv) hook, std::cout, "", - false, + true, hook_api::getImportWhitelist(rules), hook_api::getGuardRulesVersion(rules)); From e93c46b3f53a2e0302c7c4dd595c0d01e71ada8d Mon Sep 17 00:00:00 2001 From: tequ Date: Tue, 8 Sep 2026 21:16:40 +0900 Subject: [PATCH 03/11] fix guard-checker log --- include/xrpl/hook/Guard.h | 36 ++++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/include/xrpl/hook/Guard.h b/include/xrpl/hook/Guard.h index 255b97c026..73ce07c644 100644 --- a/include/xrpl/hook/Guard.h +++ b/include/xrpl/hook/Guard.h @@ -280,8 +280,7 @@ compute_wce( // expr under analysis begins and end_offset is where it ends returns {worst // case instruction count} if valid or {} if invalid may throw overflow_error, // length_error -inline std::optional< - std::pair> // {instruction count, execution cost} +inline std::optional // count or cost depending on returnCost check_guard( std::vector const& wasm, int codesec, @@ -291,6 +290,7 @@ check_guard( int last_import_idx, GuardLog guardLog, std::string guardLogAccStr, + bool returnCost, /* RH NOTE: * rules version is a bit field, so rule update 1 is 0x01, update 2 is 0x02 * and update 3 is 0x04 ideally at rule version 3 all bits so far are set @@ -825,9 +825,19 @@ check_guard( return {}; } - GUARDLOG(hook::log::INSTRUCTION_COUNT) - << "GuardCheck " - << "Total worse-case execution count: " << instruction_count << "\n"; + if (returnCost) + { + GUARDLOG(hook::log::INSTRUCTION_COUNT) + << "GuardCheck " + << "Total worse-case execution cost: " << execution_cost << "\n"; + } + else + { + GUARDLOG(hook::log::INSTRUCTION_COUNT) + << "GuardCheck " + << "Total worse-case execution count: " << instruction_count + << "\n"; + } if (instruction_count >= 0xFFFFU) { @@ -839,7 +849,10 @@ check_guard( << "\n"; return {}; } - return std::pair{instruction_count, execution_cost}; + if (returnCost) + return execution_cost; + else + return instruction_count; } // RH TODO: reprogram this function to use REQUIRE/ADVANCE @@ -1520,21 +1533,16 @@ validateGuards( last_import_number, guardLog, guardLogAccStr, + returnCost, rulesVersion); if (!valid) return {}; if (hook_func_idx && *hook_func_idx == j) - if (!returnCost) - maxInstrCountHook = valid->first; - else - maxInstrCountHook = valid->second; + maxInstrCountHook = *valid; else if (cbak_func_idx && *cbak_func_idx == j) - if (!returnCost) - maxInstrCountCbak = valid->first; - else - maxInstrCountCbak = valid->second; + maxInstrCountCbak = *valid; else { if (DEBUG_GUARD) From 8b7bdd6303942c1960a1321daad0eea6cc905f9a Mon Sep 17 00:00:00 2001 From: tequ Date: Tue, 8 Sep 2026 22:02:16 +0900 Subject: [PATCH 04/11] Change log message output to distinguish between hook and cbak costs --- include/xrpl/hook/Guard.h | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/include/xrpl/hook/Guard.h b/include/xrpl/hook/Guard.h index 73ce07c644..060fc85f4b 100644 --- a/include/xrpl/hook/Guard.h +++ b/include/xrpl/hook/Guard.h @@ -825,20 +825,6 @@ check_guard( return {}; } - if (returnCost) - { - GUARDLOG(hook::log::INSTRUCTION_COUNT) - << "GuardCheck " - << "Total worse-case execution cost: " << execution_cost << "\n"; - } - else - { - GUARDLOG(hook::log::INSTRUCTION_COUNT) - << "GuardCheck " - << "Total worse-case execution count: " << instruction_count - << "\n"; - } - if (instruction_count >= 0xFFFFU) { GUARDLOG(hook::log::INSTRUCTION_EXCESS) @@ -1540,9 +1526,23 @@ validateGuards( return {}; if (hook_func_idx && *hook_func_idx == j) + { + GUARDLOG(hook::log::INSTRUCTION_COUNT) + << "GuardCheck " + << "Total hook worse-case execution " + << (returnCost ? "cost: " : "count: ") << *valid + << "\n"; maxInstrCountHook = *valid; + } else if (cbak_func_idx && *cbak_func_idx == j) + { + GUARDLOG(hook::log::INSTRUCTION_COUNT) + << "GuardCheck " + << "Total cbak worse-case execution " + << (returnCost ? "cost: " : "count: ") << *valid + << "\n"; maxInstrCountCbak = *valid; + } else { if (DEBUG_GUARD) From 401a35957ddc4d336e2388815cdfb34812652bfe Mon Sep 17 00:00:00 2001 From: tequ Date: Wed, 9 Sep 2026 12:00:24 +0900 Subject: [PATCH 05/11] retire fixXahauV1, fixXahauV2 Amendments (#722) --- cfg/xahaud-standalone.cfg | 2 - include/xrpl/protocol/detail/features.macro | 4 +- src/test/app/BaseFee_test.cpp | 111 +--- src/test/app/Escrow_test.cpp | 318 ++++----- src/test/app/GenesisMint_test.cpp | 46 +- src/test/app/Import_test.cpp | 30 +- src/test/app/Offer_test.cpp | 54 +- src/test/app/SetHookTSH_test.cpp | 237 ++----- src/test/app/SetHook_test.cpp | 68 +- src/test/app/URIToken_test.cpp | 52 +- src/test/app/XahauGenesis_test.cpp | 31 +- src/xrpld/app/hook/detail/HookAPI.cpp | 3 +- src/xrpld/app/hook/detail/applyHook.cpp | 148 ++--- src/xrpld/app/tx/detail/CancelOffer.cpp | 29 +- src/xrpld/app/tx/detail/Escrow.cpp | 52 +- src/xrpld/app/tx/detail/GenesisMint.cpp | 106 +-- src/xrpld/app/tx/detail/Import.cpp | 14 +- src/xrpld/app/tx/detail/Invoke.cpp | 21 - src/xrpld/app/tx/detail/SetHook.cpp | 5 +- src/xrpld/app/tx/detail/Transactor.cpp | 5 +- src/xrpld/app/tx/detail/URIToken.cpp | 703 ++++---------------- src/xrpld/ledger/detail/ApplyStateTable.cpp | 5 +- 22 files changed, 521 insertions(+), 1523 deletions(-) diff --git a/cfg/xahaud-standalone.cfg b/cfg/xahaud-standalone.cfg index 3933b4ba83..4a90ec6bfe 100644 --- a/cfg/xahaud-standalone.cfg +++ b/cfg/xahaud-standalone.cfg @@ -146,8 +146,6 @@ D686F2538F410C9D0D856788E98E3579595DAF7B38D38887F81ECAC934B06040 HooksUpdate1 3C43D9A973AA4443EF3FC38E42DD306160FBFFDAB901CD8BAA15D09F2597EB87 NonFungibleTokensV1 0285B7E5E08E1A8E4C15636F0591D87F73CB6A7B6452A932AD72BBC8E5D1CBE3 fixNFTokenDirV1 36799EA497B1369B170805C078AEFE6188345F9B3E324C21E9CA3FF574E3C3D6 fixNFTokenNegOffer -4C499D17719BB365B69010A436B64FD1A82AAB199FC1CEB06962EBD01059FB09 fixXahauV1 -215181D23BF5C173314B5FDB9C872C92DE6CC918483727DE037C0C13E7E6EE9D fixXahauV2 0D8BF22FF7570D58598D1EF19EBB6E142AD46E59A223FD3816262FBB69345BEA Remit 7CA0426E7F411D39BB014E57CD9E08F61DE1750F0D41FCD428D9FB80BB7596B0 ZeroB2M 4B8466415FAB32FFA89D9DCBE166A42340115771DF611A7160F8D7439C87ECD8 fixNSDelete diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index f47b2bd37b..b4fdf46729 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -89,8 +89,6 @@ XRPL_FIX (240819, Supported::yes, VoteBehavior::DefaultYe XRPL_FIX (NSDelete, Supported::yes, VoteBehavior::DefaultNo) XRPL_FEATURE(ZeroB2M, Supported::yes, VoteBehavior::DefaultNo) XRPL_FEATURE(Remit, Supported::yes, VoteBehavior::DefaultNo) -XRPL_FIX (XahauV2, Supported::yes, VoteBehavior::DefaultNo) -XRPL_FIX (XahauV1, Supported::yes, VoteBehavior::DefaultNo) XRPL_FEATURE(HooksUpdate1, Supported::yes, VoteBehavior::DefaultYes) XRPL_FEATURE(XahauGenesis, Supported::yes, VoteBehavior::DefaultYes) XRPL_FEATURE(Import, Supported::yes, VoteBehavior::DefaultYes) @@ -161,6 +159,8 @@ XRPL_FEATURE(CryptoConditionsSuite, Supported::yes, VoteBehavior::Obsolete) // pre-amendment code has been removed and the identifiers are deprecated. // All known amendments and amendments that may appear in a validated // ledger must be registered either here or above with the "active" amendments +XRPL_RETIRE(fixXahauV2) +XRPL_RETIRE(fixXahauV1) XRPL_RETIRE(MultiSign) XRPL_RETIRE(TrustSetAuth) XRPL_RETIRE(FeeEscalation) diff --git a/src/test/app/BaseFee_test.cpp b/src/test/app/BaseFee_test.cpp index 97de936315..e8c3dac522 100644 --- a/src/test/app/BaseFee_test.cpp +++ b/src/test/app/BaseFee_test.cpp @@ -88,9 +88,7 @@ class BaseFee_test : public beast::unit_test::suite auto tx = fset(account, asfTshCollect); // verify hooks fee - std::string const feeResult = - env.current()->rules().enabled(fixXahauV1) ? "16" : "10"; - testRPCCall(env, tx, feeResult); + testRPCCall(env, tx, "16"); } void @@ -112,9 +110,7 @@ class BaseFee_test : public beast::unit_test::suite auto tx = acctdelete(account, bene); // verify hooks fee - std::string const feeResult = - env.current()->rules().enabled(fixXahauV1) ? "200000" : "200000"; - testRPCCall(env, tx, feeResult); + testRPCCall(env, tx, "200000"); } static uint256 @@ -142,9 +138,7 @@ class BaseFee_test : public beast::unit_test::suite auto tx = check::cancel(account, checkId); // verify hooks fee - std::string const feeResult = - env.current()->rules().enabled(fixXahauV1) ? "16" : "10"; - testRPCCall(env, tx, feeResult); + testRPCCall(env, tx, "16"); } void @@ -167,9 +161,7 @@ class BaseFee_test : public beast::unit_test::suite auto tx = check::cash(dest, checkId, XRP(100)); // verify hooks fee - std::string const feeResult = - env.current()->rules().enabled(fixXahauV1) ? "16" : "10"; - testRPCCall(env, tx, feeResult); + testRPCCall(env, tx, "16"); } void @@ -191,9 +183,7 @@ class BaseFee_test : public beast::unit_test::suite auto tx = check::create(account, dest, XRP(100)); // verify hooks fee - std::string const feeResult = - env.current()->rules().enabled(fixXahauV1) ? "16" : "10"; - testRPCCall(env, tx, feeResult); + testRPCCall(env, tx, "16"); } void @@ -214,9 +204,7 @@ class BaseFee_test : public beast::unit_test::suite auto tx = reward::claim(account); // verify hooks fee - std::string const feeResult = - env.current()->rules().enabled(fixXahauV1) ? "16" : "10"; - testRPCCall(env, tx, feeResult); + testRPCCall(env, tx, "16"); } void @@ -238,9 +226,7 @@ class BaseFee_test : public beast::unit_test::suite auto tx = deposit::auth(account, authed); // verify hooks fee - std::string const feeResult = - env.current()->rules().enabled(fixXahauV1) ? "16" : "10"; - testRPCCall(env, tx, feeResult); + testRPCCall(env, tx, "16"); } void @@ -262,9 +248,7 @@ class BaseFee_test : public beast::unit_test::suite auto tx = cancel(account, account, seq1); // verify hooks fee - std::string const feeResult = - env.current()->rules().enabled(fixXahauV1) ? "16" : "10"; - testRPCCall(env, tx, feeResult); + testRPCCall(env, tx, "16"); } void @@ -286,8 +270,7 @@ class BaseFee_test : public beast::unit_test::suite auto tx = escrow(account, dest, XRP(10)); // verify hooks fee - std::string const feeResult = - env.current()->rules().enabled(fixXahauV1) ? "16" : "10"; + std::string const feeResult = "16"; testRPCCall(env, tx, feeResult); } @@ -310,9 +293,7 @@ class BaseFee_test : public beast::unit_test::suite auto tx = finish(account, account, seq1); // verify hooks fee - std::string const feeResult = - env.current()->rules().enabled(fixXahauV1) ? "16" : "10"; - testRPCCall(env, tx, feeResult); + testRPCCall(env, tx, "16"); } void @@ -334,9 +315,7 @@ class BaseFee_test : public beast::unit_test::suite account, import::loadXpop(ImportTCAccountSet::w_seed)); // verify hooks fee - std::string const feeResult = - env.current()->rules().enabled(fixXahauV1) ? "106" : "100"; - testRPCCall(env, tx, feeResult); + testRPCCall(env, tx, "106"); } void @@ -357,9 +336,7 @@ class BaseFee_test : public beast::unit_test::suite auto tx = invoke::invoke(account); // verify hooks fee - std::string const feeResult = - env.current()->rules().enabled(fixXahauV1) ? "16" : "16"; - testRPCCall(env, tx, feeResult); + testRPCCall(env, tx, "16"); } void @@ -381,9 +358,7 @@ class BaseFee_test : public beast::unit_test::suite auto tx = offer_cancel(account, offerSeq); // verify hooks fee - std::string const feeResult = - env.current()->rules().enabled(fixXahauV1) ? "16" : "10"; - testRPCCall(env, tx, feeResult); + testRPCCall(env, tx, "16"); } void @@ -406,9 +381,7 @@ class BaseFee_test : public beast::unit_test::suite auto tx = offer(account, USD(1000), XRP(1000)); // verify hooks fee - std::string const feeResult = - env.current()->rules().enabled(fixXahauV1) ? "16" : "10"; - testRPCCall(env, tx, feeResult); + testRPCCall(env, tx, "16"); } void @@ -430,9 +403,7 @@ class BaseFee_test : public beast::unit_test::suite auto tx = pay(account, dest, XRP(1)); // verify hooks fee - std::string const feeResult = - env.current()->rules().enabled(fixXahauV1) ? "16" : "10"; - testRPCCall(env, tx, feeResult); + testRPCCall(env, tx, "16"); } static uint256 @@ -468,9 +439,7 @@ class BaseFee_test : public beast::unit_test::suite auto tx = paychan::claim(account, chan, reqBal, authAmt); // verify hooks fee - std::string const feeResult = - env.current()->rules().enabled(fixXahauV1) ? "16" : "10"; - testRPCCall(env, tx, feeResult); + testRPCCall(env, tx, "16"); } void @@ -494,9 +463,7 @@ class BaseFee_test : public beast::unit_test::suite auto tx = paychan::create(account, dest, XRP(10), settleDelay, pk); // verify hooks fee - std::string const feeResult = - env.current()->rules().enabled(fixXahauV1) ? "16" : "10"; - testRPCCall(env, tx, feeResult); + testRPCCall(env, tx, "16"); } void @@ -519,9 +486,7 @@ class BaseFee_test : public beast::unit_test::suite auto tx = paychan::fund(account, chan, XRP(1)); // verify hooks fee - std::string const feeResult = - env.current()->rules().enabled(fixXahauV1) ? "16" : "10"; - testRPCCall(env, tx, feeResult); + testRPCCall(env, tx, "16"); } void @@ -552,9 +517,7 @@ class BaseFee_test : public beast::unit_test::suite hookParams[jss::HookParameter][jss::HookParameterValue] = "DEADBEEF"; // verify hooks fee - std::string const feeResult = - env.current()->rules().enabled(fixXahauV1) ? "73022" : "73016"; - testRPCCall(env, tx, feeResult); + testRPCCall(env, tx, "73022"); } void @@ -576,9 +539,7 @@ class BaseFee_test : public beast::unit_test::suite auto tx = regkey(account, dest); // verify hooks fee - std::string const feeResult = - env.current()->rules().enabled(fixXahauV1) ? "0" : "0"; - testRPCCall(env, tx, feeResult); + testRPCCall(env, tx, "0"); } void @@ -601,9 +562,7 @@ class BaseFee_test : public beast::unit_test::suite auto tx = signers(account, 2, {{signer1, 1}, {signer2, 1}}); // verify hooks fee - std::string const feeResult = - env.current()->rules().enabled(fixXahauV1) ? "16" : "10"; - testRPCCall(env, tx, feeResult); + testRPCCall(env, tx, "16"); } void @@ -624,9 +583,7 @@ class BaseFee_test : public beast::unit_test::suite auto tx = ticket::create(account, 2); // verify hooks fee - std::string const feeResult = - env.current()->rules().enabled(fixXahauV1) ? "16" : "10"; - testRPCCall(env, tx, feeResult); + testRPCCall(env, tx, "16"); } void @@ -649,9 +606,7 @@ class BaseFee_test : public beast::unit_test::suite auto tx = trust(account, USD(1000)); // verify hooks fee - std::string const feeResult = - env.current()->rules().enabled(fixXahauV1) ? "16" : "10"; - testRPCCall(env, tx, feeResult); + testRPCCall(env, tx, "16"); } void @@ -675,9 +630,7 @@ class BaseFee_test : public beast::unit_test::suite auto tx = uritoken::burn(issuer, hexid); // verify hooks fee - std::string const feeResult = - env.current()->rules().enabled(fixXahauV1) ? "16" : "10"; - testRPCCall(env, tx, feeResult); + testRPCCall(env, tx, "16"); } void @@ -702,9 +655,7 @@ class BaseFee_test : public beast::unit_test::suite tx[jss::Amount] = "1000000"; // verify hooks fee - std::string const feeResult = - env.current()->rules().enabled(fixXahauV1) ? "16" : "10"; - testRPCCall(env, tx, feeResult); + testRPCCall(env, tx, "16"); } void @@ -728,9 +679,7 @@ class BaseFee_test : public beast::unit_test::suite auto tx = uritoken::cancel(issuer, hexid); // verify hooks fee - std::string const feeResult = - env.current()->rules().enabled(fixXahauV1) ? "16" : "10"; - testRPCCall(env, tx, feeResult); + testRPCCall(env, tx, "16"); } void @@ -757,8 +706,7 @@ class BaseFee_test : public beast::unit_test::suite tx[jss::Amount] = "1000000"; // verify hooks fee - std::string const feeResult = - env.current()->rules().enabled(fixXahauV1) ? "16" : "10"; + std::string const feeResult = "16"; testRPCCall(env, tx, feeResult); } @@ -781,9 +729,7 @@ class BaseFee_test : public beast::unit_test::suite auto tx = uritoken::mint(account, uri); // verify hooks fee - std::string const feeResult = - env.current()->rules().enabled(fixXahauV1) ? "16" : "10"; - testRPCCall(env, tx, feeResult); + testRPCCall(env, tx, "16"); } void @@ -827,7 +773,6 @@ public: using namespace test::jtx; auto const sa = supported_amendments(); testWithFeats(sa); - testWithFeats(sa - fixXahauV1); } }; diff --git a/src/test/app/Escrow_test.cpp b/src/test/app/Escrow_test.cpp index bc3bf161c0..ebd9ed138d 100644 --- a/src/test/app/Escrow_test.cpp +++ b/src/test/app/Escrow_test.cpp @@ -4422,216 +4422,160 @@ struct Escrow_test : public beast::unit_test::suite auto const gw = Account{"gateway"}; auto const USD = gw["USD"]; - for (bool const withXahauV1 : {true, false}) + Env env{*this, features}; + env.fund(XRP(10000), alice, bob, gw); + env.close(); + env.trust(USD(1000000), alice); + env.trust(USD(1000000), bob); + env.close(); + env(pay(gw, alice, USD(10000))); + env(pay(gw, bob, USD(10000))); + env.close(); + + // EscrowCancel - EscrowID { - auto const amend = withXahauV1 ? features : features - fixXahauV1; - Env env{*this, amend}; - env.fund(XRP(10000), alice, bob, gw); - env.close(); - env.trust(USD(1000000), alice); - env.trust(USD(1000000), bob); - env.close(); - env(pay(gw, alice, USD(10000))); - env(pay(gw, bob, USD(10000))); + uint256 const escrowId{getEscrowIndex(alice, env.seq(alice))}; + env(escrow(alice, bob, USD(1000)), + finish_time(env.now() + 1s), + cancel_time(env.now() + 2s), + fee(1500)); env.close(); - // EscrowCancel - EscrowID - { - uint256 const escrowId{getEscrowIndex(alice, env.seq(alice))}; - env(escrow(alice, bob, USD(1000)), - finish_time(env.now() + 1s), - cancel_time(env.now() + 2s), - fee(1500)); - env.close(); + // withXahauV1 - no OfferSequence + env(cancel(bob, alice), + escrow_id(escrowId), + fee(1500), + ter(tesSUCCESS)); + env.close(); - if (withXahauV1) - { - // withXahauV1 - no OfferSequence - env(cancel(bob, alice), - escrow_id(escrowId), - fee(1500), - ter(tesSUCCESS)); - env.close(); - } - else - { - // !withXahauV1 - OfferSequence == 0 - env(cancel(bob, alice, 0), - escrow_id(escrowId), - fee(1500), - ter(tesSUCCESS)); - env.close(); - } + auto const escrowLE = env.le(keylet::unchecked(escrowId)); + BEAST_EXPECT(!escrowLE); + } - auto const escrowLE = env.le(keylet::unchecked(escrowId)); - BEAST_EXPECT(!escrowLE); - } + // EscrowCancel - no EscrowID or OfferSequence + { + uint256 const escrowId{getEscrowIndex(alice, env.seq(alice))}; + env(escrow(alice, bob, USD(1000)), + finish_time(env.now() + 1s), + cancel_time(env.now() + 2s), + fee(1500)); + env.close(); - // EscrowCancel - no EscrowID or OfferSequence - { - uint256 const escrowId{getEscrowIndex(alice, env.seq(alice))}; - env(escrow(alice, bob, USD(1000)), - finish_time(env.now() + 1s), - cancel_time(env.now() + 2s), - fee(1500)); - env.close(); + env(cancel(bob, alice), fee(1500), ter(temMALFORMED)); + env.close(); - env(cancel(bob, alice), fee(1500), ter(temMALFORMED)); - env.close(); + auto const escrowLE = env.le(keylet::unchecked(escrowId)); + BEAST_EXPECT(escrowLE); + } - auto const escrowLE = env.le(keylet::unchecked(escrowId)); - BEAST_EXPECT(escrowLE); - } + // EscrowCancel - EscrowID & OfferSequence + { + uint256 const escrowId{getEscrowIndex(alice, env.seq(alice))}; + auto const seq = env.seq(alice); + env(escrow(alice, bob, USD(1000)), + finish_time(env.now() + 1s), + cancel_time(env.now() + 2s), + fee(1500)); + env.close(); - // EscrowCancel - EscrowID & OfferSequence - { - uint256 const escrowId{getEscrowIndex(alice, env.seq(alice))}; - auto const seq = env.seq(alice); - env(escrow(alice, bob, USD(1000)), - finish_time(env.now() + 1s), - cancel_time(env.now() + 2s), - fee(1500)); - env.close(); + env(cancel(bob, alice, seq), + escrow_id(escrowId), + fee(1500), + ter(temMALFORMED)); + env.close(); - env(cancel(bob, alice, seq), - escrow_id(escrowId), - fee(1500), - ter(temMALFORMED)); - env.close(); + auto const escrowLE = env.le(keylet::unchecked(escrowId)); + BEAST_EXPECT(escrowLE); + } - auto const escrowLE = env.le(keylet::unchecked(escrowId)); - BEAST_EXPECT(escrowLE); - } + // EscrowCancel - EscrowID & OfferSequence 0 + { + uint256 const escrowId{getEscrowIndex(alice, env.seq(alice))}; + env(escrow(alice, bob, USD(1000)), + finish_time(env.now() + 1s), + cancel_time(env.now() + 2s), + fee(1500)); + env.close(); - // EscrowCancel - EscrowID & OfferSequence 0 - { - uint256 const escrowId{getEscrowIndex(alice, env.seq(alice))}; - env(escrow(alice, bob, USD(1000)), - finish_time(env.now() + 1s), - cancel_time(env.now() + 2s), - fee(1500)); - env.close(); + // withXahauV1 - OfferSequence 0 == temMALFORMED + env(cancel(bob, alice, 0), + escrow_id(escrowId), + fee(1500), + ter(temMALFORMED)); + env.close(); + auto const escrowLE = env.le(keylet::unchecked(escrowId)); + BEAST_EXPECT(escrowLE); + } - if (withXahauV1) - { - // withXahauV1 - OfferSequence 0 == temMALFORMED - env(cancel(bob, alice, 0), - escrow_id(escrowId), - fee(1500), - ter(temMALFORMED)); - env.close(); - auto const escrowLE = env.le(keylet::unchecked(escrowId)); - BEAST_EXPECT(escrowLE); - } - else - { - // withXahauV1 - OfferSequence 0 == tesSUCCESS - env(cancel(bob, alice, 0), - escrow_id(escrowId), - fee(1500), - ter(tesSUCCESS)); - env.close(); - auto const escrowLE = env.le(keylet::unchecked(escrowId)); - BEAST_EXPECT(!escrowLE); - } - } + // EscrowFinish - EscrowID + { + uint256 const escrowId{getEscrowIndex(alice, env.seq(alice))}; + env(escrow(alice, bob, USD(1000)), + finish_time(env.now() + 1s), + fee(1500)); + env.close(5s); - // EscrowFinish - EscrowID - { - uint256 const escrowId{getEscrowIndex(alice, env.seq(alice))}; - env(escrow(alice, bob, USD(1000)), - finish_time(env.now() + 1s), - fee(1500)); - env.close(5s); + // withXahauV1 - no OfferSequence + env(finish(bob, alice), + escrow_id(escrowId), + fee(1500), + ter(tesSUCCESS)); + env.close(); - if (withXahauV1) - { - // withXahauV1 - no OfferSequence - env(finish(bob, alice), - escrow_id(escrowId), - fee(1500), - ter(tesSUCCESS)); - env.close(); - } - else - { - // !withXahauV1 - OfferSequence == 0 - env(finish(bob, alice, 0), - escrow_id(escrowId), - fee(1500), - ter(tesSUCCESS)); - env.close(); - } + auto const escrowLE = env.le(keylet::unchecked(escrowId)); + BEAST_EXPECT(!escrowLE); + } - auto const escrowLE = env.le(keylet::unchecked(escrowId)); - BEAST_EXPECT(!escrowLE); - } + // EscrowFinish - no EscrowID or OfferSequence + { + uint256 const escrowId{getEscrowIndex(alice, env.seq(alice))}; + env(escrow(alice, bob, USD(1000)), + finish_time(env.now() + 1s), + fee(1500)); + env.close(); - // EscrowFinish - no EscrowID or OfferSequence - { - uint256 const escrowId{getEscrowIndex(alice, env.seq(alice))}; - env(escrow(alice, bob, USD(1000)), - finish_time(env.now() + 1s), - fee(1500)); - env.close(); + env(finish(bob, alice), fee(1500), ter(temMALFORMED)); + env.close(); - env(finish(bob, alice), fee(1500), ter(temMALFORMED)); - env.close(); + auto const escrowLE = env.le(keylet::unchecked(escrowId)); + BEAST_EXPECT(escrowLE); + } - auto const escrowLE = env.le(keylet::unchecked(escrowId)); - BEAST_EXPECT(escrowLE); - } + // EscrowFinish- EscrowID & OfferSequence + { + uint256 const escrowId{getEscrowIndex(alice, env.seq(alice))}; + auto const seq = env.seq(alice); + env(escrow(alice, bob, USD(1000)), + finish_time(env.now() + 1s), + fee(1500)); + env.close(5s); - // EscrowFinish- EscrowID & OfferSequence - { - uint256 const escrowId{getEscrowIndex(alice, env.seq(alice))}; - auto const seq = env.seq(alice); - env(escrow(alice, bob, USD(1000)), - finish_time(env.now() + 1s), - fee(1500)); - env.close(5s); + env(finish(bob, alice, seq), + escrow_id(escrowId), + fee(1500), + ter(temMALFORMED)); + env.close(); - env(finish(bob, alice, seq), - escrow_id(escrowId), - fee(1500), - ter(temMALFORMED)); - env.close(); + auto const escrowLE = env.le(keylet::unchecked(escrowId)); + BEAST_EXPECT(escrowLE); + } - auto const escrowLE = env.le(keylet::unchecked(escrowId)); - BEAST_EXPECT(escrowLE); - } + // EscrowFinish- EscrowID & OfferSequence 0 + { + uint256 const escrowId{getEscrowIndex(alice, env.seq(alice))}; + env(escrow(alice, bob, USD(1000)), + finish_time(env.now() + 1s), + fee(1500)); + env.close(5s); - // EscrowFinish- EscrowID & OfferSequence 0 - { - uint256 const escrowId{getEscrowIndex(alice, env.seq(alice))}; - env(escrow(alice, bob, USD(1000)), - finish_time(env.now() + 1s), - fee(1500)); - env.close(5s); - - if (withXahauV1) - { - // withXahauV1 - OfferSequence 0 == temMALFORMED - env(finish(bob, alice, 0), - escrow_id(escrowId), - fee(1500), - ter(temMALFORMED)); - env.close(); - auto const escrowLE = env.le(keylet::unchecked(escrowId)); - BEAST_EXPECT(escrowLE); - } - else - { - // !withXahauV1 - OfferSequence 0 == tesSUCCESS - env(finish(bob, alice, 0), - escrow_id(escrowId), - fee(1500), - ter(tesSUCCESS)); - env.close(); - auto const escrowLE = env.le(keylet::unchecked(escrowId)); - BEAST_EXPECT(!escrowLE); - } - } + // withXahauV1 - OfferSequence 0 == temMALFORMED + env(finish(bob, alice, 0), + escrow_id(escrowId), + fee(1500), + ter(temMALFORMED)); + env.close(); + auto const escrowLE = env.le(keylet::unchecked(escrowId)); + BEAST_EXPECT(escrowLE); } } diff --git a/src/test/app/GenesisMint_test.cpp b/src/test/app/GenesisMint_test.cpp index 9ae21ca3e1..ab7fe38c1d 100644 --- a/src/test/app/GenesisMint_test.cpp +++ b/src/test/app/GenesisMint_test.cpp @@ -336,10 +336,7 @@ struct GenesisMint_test : public beast::unit_test::suite env.close(); // validate emitted txn - auto const txResult = env.current()->rules().enabled(fixXahauV1) - ? "tesSUCCESS" - : "tecINTERNAL"; - validateEmittedTxn(env, txResult, __LINE__); + validateEmittedTxn(env, "tesSUCCESS", __LINE__); } // missing an amount @@ -569,39 +566,25 @@ struct GenesisMint_test : public beast::unit_test::suite validateEmittedTxn(env, "tesSUCCESS", __LINE__); } - auto const amtResult = env.current()->rules().enabled(fixXahauV1) - ? 30000000ULL - : 10000000ULL; // try to include the same destination twice - { - auto const txResult = env.current()->rules().enabled(fixXahauV1) - ? ter(tesSUCCESS) - : ter(tecHOOK_REJECTED); - env(invoke::invoke( - invoker, - env.master, - genesis::makeBlob({ - {greg.id(), - XRP(10).value(), - std::nullopt, - std::nullopt}, - {greg.id(), - XRP(10).value(), - std::nullopt, - std::nullopt}, - })), - fee(XRP(1)), - txResult); - env.close(); - env.close(); - } + env(invoke::invoke( + invoker, + env.master, + genesis::makeBlob({ + {greg.id(), XRP(10).value(), std::nullopt, std::nullopt}, + {greg.id(), XRP(10).value(), std::nullopt, std::nullopt}, + })), + fee(XRP(1)), + ter(tesSUCCESS)); + env.close(); + env.close(); // check { auto const le = env.le(keylet::account(greg.id())); BEAST_EXPECT( !!le && - le->getFieldAmount(sfBalance).xrp().drops() == amtResult); + le->getFieldAmount(sfBalance).xrp().drops() == 30000000ULL); } // trip the supply cap invariant @@ -626,7 +609,7 @@ struct GenesisMint_test : public beast::unit_test::suite auto const le = env.le(keylet::account(greg.id())); BEAST_EXPECT( !!le && - le->getFieldAmount(sfBalance).xrp().drops() == amtResult); + le->getFieldAmount(sfBalance).xrp().drops() == 30000000ULL); auto const postCoins = env.current()->info().drops; auto const txnFee = @@ -698,7 +681,6 @@ public: using namespace test::jtx; auto const sa = supported_amendments(); testWithFeats(sa); - testWithFeats(sa - fixXahauV1); testWithFeats(sa - fixHookAPI20251128); } }; diff --git a/src/test/app/Import_test.cpp b/src/test/app/Import_test.cpp index bc146cc03a..49379fec83 100644 --- a/src/test/app/Import_test.cpp +++ b/src/test/app/Import_test.cpp @@ -1663,11 +1663,9 @@ class Import_test : public beast::unit_test::suite // temMALFORMED - Import: xpop did not contain an 80% quorum for the txn // it purports to prove. - for (bool const withXahauV1 : {true, false}) { - auto const amend = withXahauV1 ? features : features - fixXahauV1; test::jtx::Env env{ - *this, network::makeNetworkVLConfig(21337, keys), amend}; + *this, network::makeNetworkVLConfig(21337, keys), features}; auto const alice = Account("alice"); env.fund(XRP(1000), alice); @@ -1679,16 +1677,10 @@ class Import_test : public beast::unit_test::suite ""; valData["n9MTTavZe6EPqqyQ27pJbNWpfHw8ZNspVgznrwx5HWm7cdTjKQie"] = ""; - if (withXahauV1) - { - valData - ["n94J5LCRu9bBWrJiwRWujvuVECVPWbXFcQ2VN38qLD378F5pDSDM"] = - ""; - } + valData["n94J5LCRu9bBWrJiwRWujvuVECVPWbXFcQ2VN38qLD378F5pDSDM"] = + ""; tmpXpop[jss::validation][jss::data] = valData; - auto const txResult = - withXahauV1 ? ter(temMALFORMED) : ter(temMALFORMED); - env(import::import(alice, tmpXpop), txResult); + env(import::import(alice, tmpXpop), ter(temMALFORMED)); } test::jtx::Env env{*this, network::makeNetworkVLConfig(21337, keys)}; @@ -5238,7 +5230,6 @@ class Import_test : public beast::unit_test::suite *this, network::makeNetworkVLConfig(21337, keys)}; auto const feeDrops = env.current()->fees().base; - bool const fixV2 = env.current()->rules().enabled(fixXahauV2); // confirm total coins header auto const initCoins = env.current()->info().drops; @@ -5288,18 +5279,17 @@ class Import_test : public beast::unit_test::suite auto const preAlice = env.balance(alice); auto const preCoins = env.current()->info().drops; - auto const result = fixV2 ? ter(tesSUCCESS) : ter(tecHOOK_REJECTED); env(import::import( alice, import::loadXpop(ImportTCAccountSet::w_seed)), import::issuer(issuer), fee(1'000'000), - result); + ter(tesSUCCESS)); env.close(); // fixXahauV2 bool const zeroBurn = env.current()->rules().enabled(featureZeroB2M); - auto const mintXAH = fixV2 ? XRP(zeroBurn ? 0 : 1000) : XRP(0); + auto const mintXAH = XRP(zeroBurn ? 0 : 1000); // confirm fee was burned mint / no mint auto const postAlice = env.balance(alice); BEAST_EXPECT(postAlice == preAlice - XRP(1) + mintXAH); @@ -5309,17 +5299,14 @@ class Import_test : public beast::unit_test::suite BEAST_EXPECT(postCoins == preCoins - XRP(1) + mintXAH); // resubmit import without issuer - auto const result2 = - fixV2 ? ter(tefPAST_IMPORT_SEQ) : ter(tesSUCCESS); env(import::import( alice, import::loadXpop(ImportTCAccountSet::w_seed)), fee(feeDrops * 10), - result2); + ter(tefPAST_IMPORT_SEQ)); env.close(); // total burn - auto const totalBurn = - fixV2 ? XRP(0) : XRP(zeroBurn ? 0 : 1000) - (feeDrops * 10); + auto const totalBurn = XRP(0); // confirm fee was minted / not minted auto const postAlice2 = env.balance(alice); @@ -6276,7 +6263,6 @@ public: { using namespace test::jtx; FeatureBitset const all{supported_amendments()}; - testWithFeats(all - fixXahauV2); testWithFeats(all - featureZeroB2M); testWithFeats(all); } diff --git a/src/test/app/Offer_test.cpp b/src/test/app/Offer_test.cpp index d8d9ce1ccb..aa53c61733 100644 --- a/src/test/app/Offer_test.cpp +++ b/src/test/app/Offer_test.cpp @@ -5105,10 +5105,8 @@ public: using namespace jtx; // OfferCreate - for (bool const withXahauV1 : {true, false}) { - auto const amend = withXahauV1 ? features : features - fixXahauV1; - Env env{*this, amend}; + Env env{*this, features}; auto const gw = Account{"gateway"}; auto const alice = Account{"alice"}; auto const USD = gw["USD"]; @@ -5192,10 +5190,8 @@ public: } // OfferCancel - for (bool const withXahauV1 : {true, false}) { - auto const amend = withXahauV1 ? features : features - fixXahauV1; - Env env{*this, amend}; + Env env{*this, features}; auto const gw = Account{"gateway"}; auto const alice = Account{"alice"}; auto const USD = gw["USD"]; @@ -5227,24 +5223,10 @@ public: env(offer(alice, XRP(50), USD(50))); env.close(); - if (withXahauV1) - { - env(offer_cancel(alice), - offer_id(offerId), - ter(tesSUCCESS)); - env.close(); - auto const offerLE = env.le(keylet::unchecked(offerId)); - BEAST_EXPECT(!offerLE); - } - else - { - env(offer_cancel(alice), - offer_id(offerId), - ter(temBAD_SEQUENCE)); - env.close(); - auto const offerLE = env.le(keylet::unchecked(offerId)); - BEAST_EXPECT(offerLE); - } + env(offer_cancel(alice), offer_id(offerId), ter(tesSUCCESS)); + env.close(); + auto const offerLE = env.le(keylet::unchecked(offerId)); + BEAST_EXPECT(!offerLE); } // no offer id or offer sequence @@ -5266,24 +5248,12 @@ public: env(offer(alice, XRP(50), USD(50))); env.close(); - if (withXahauV1) - { - env(offer_cancel(alice, offerSeqId), - offer_id(offerId), - ter(temBAD_SEQUENCE)); - env.close(); - auto const offerLE = env.le(keylet::unchecked(offerId)); - BEAST_EXPECT(offerLE); - } - else - { - env(offer_cancel(alice, offerSeqId), - offer_id(offerId), - ter(tesSUCCESS)); - env.close(); - auto const offerLE = env.le(keylet::unchecked(offerId)); - BEAST_EXPECT(!offerLE); - } + env(offer_cancel(alice, offerSeqId), + offer_id(offerId), + ter(temBAD_SEQUENCE)); + env.close(); + auto const offerLE = env.le(keylet::unchecked(offerId)); + BEAST_EXPECT(offerLE); } // both offer id and offer sequence 0 diff --git a/src/test/app/SetHookTSH_test.cpp b/src/test/app/SetHookTSH_test.cpp index af422f79e3..218c0ffab2 100644 --- a/src/test/app/SetHookTSH_test.cpp +++ b/src/test/app/SetHookTSH_test.cpp @@ -740,15 +740,7 @@ private: { auto const executions = meta[sfHookExecutions.jsonName]; auto const execution = executions[0u][sfHookExecution.jsonName]; - bool const fixV2 = env.current()->rules().enabled(fixXahauV2); - if (fixV2) - { - BEAST_EXPECT(execution[sfFlags.jsonName] == expected); - } - else - { - BEAST_REQUIRE(!execution[sfFlags.jsonName]); - } + BEAST_EXPECT(execution[sfFlags.jsonName] == expected); } void @@ -2269,16 +2261,10 @@ private: setTSHHook(env, account, testStrong); // cancel escrow - Json::Value tx; - if (!env.current()->rules().enabled(fixXahauV1)) - { - tx = cancel(account, account, 0); - } - else - { - tx = cancel(account, account); - } - env(tx, escrow_id(escrowId), fee(XRP(1)), ter(tesSUCCESS)); + env(cancel(account, account), + escrow_id(escrowId), + fee(XRP(1)), + ter(tesSUCCESS)); env.close(); // verify tsh hook triggered @@ -2323,16 +2309,10 @@ private: setTSHHook(env, dest, testStrong); // cancel escrow - Json::Value tx; - if (!env.current()->rules().enabled(fixXahauV1)) - { - tx = cancel(account, account, 0); - } - else - { - tx = cancel(account, account); - } - env(tx, escrow_id(escrowId), fee(XRP(1)), ter(tesSUCCESS)); + env(cancel(account, account), + escrow_id(escrowId), + fee(XRP(1)), + ter(tesSUCCESS)); env.close(); // verify tsh hook triggered @@ -2381,16 +2361,10 @@ private: setTSHHook(env, dest, testStrong); // cancel escrow - Json::Value tx; - if (!env.current()->rules().enabled(fixXahauV1)) - { - tx = cancel(dest, account, 0); - } - else - { - tx = cancel(dest, account); - } - env(tx, escrow_id(escrowId), fee(XRP(1)), ter(tesSUCCESS)); + env(cancel(dest, account), + escrow_id(escrowId), + fee(XRP(1)), + ter(tesSUCCESS)); env.close(); // verify tsh hook triggered @@ -2432,23 +2406,14 @@ private: setTSHHook(env, account, testStrong); // cancel escrow - bool const fixV1 = env.current()->rules().enabled(fixXahauV1); - Json::Value tx; - if (!fixV1) - { - tx = cancel(dest, account, 0); - } - else - { - tx = cancel(dest, account); - } - env(tx, escrow_id(escrowId), fee(XRP(1)), ter(tesSUCCESS)); + env(cancel(dest, account), + escrow_id(escrowId), + fee(XRP(1)), + ter(tesSUCCESS)); env.close(); // verify tsh hook triggered - auto const expected = - (fixV1 ? (testStrong ? tshSTRONG : tshSTRONG) - : (testStrong ? tshNONE : tshNONE)); + auto const expected = testStrong ? tshSTRONG : tshSTRONG; testTSHStrongWeak(env, expected, __LINE__); } @@ -2497,16 +2462,10 @@ private: setTSHHook(env, gw, testStrong); // cancel escrow - Json::Value tx; - if (!env.current()->rules().enabled(fixXahauV1)) - { - tx = cancel(account, account, 0); - } - else - { - tx = cancel(account, account); - } - env(tx, escrow_id(escrowId), fee(XRP(1)), ter(tesSUCCESS)); + env(cancel(account, account), + escrow_id(escrowId), + fee(XRP(1)), + ter(tesSUCCESS)); env.close(); // verify tsh hook triggered @@ -2922,16 +2881,10 @@ private: setTSHHook(env, account, testStrong); // finish escrow - Json::Value tx; - if (!env.current()->rules().enabled(fixXahauV1)) - { - tx = finish(account, account, 0); - } - else - { - tx = finish(account, account); - } - env(tx, escrow_id(escrowId), fee(XRP(1)), ter(tesSUCCESS)); + env(finish(account, account), + escrow_id(escrowId), + fee(XRP(1)), + ter(tesSUCCESS)); env.close(); // verify tsh hook triggered @@ -2970,23 +2923,14 @@ private: setTSHHook(env, dest, testStrong); // finish escrow - bool const fixV1 = env.current()->rules().enabled(fixXahauV1); - Json::Value tx; - if (!fixV1) - { - tx = finish(account, account, 0); - } - else - { - tx = finish(account, account); - } - env(tx, escrow_id(escrowId), fee(XRP(1)), ter(tesSUCCESS)); + env(finish(account, account), + escrow_id(escrowId), + fee(XRP(1)), + ter(tesSUCCESS)); env.close(); // verify tsh hook triggered - auto const expected = - (fixV1 ? (testStrong ? tshSTRONG : tshSTRONG) - : (testStrong ? tshNONE : tshNONE)); + auto const expected = testStrong ? tshSTRONG : tshSTRONG; testTSHStrongWeak(env, expected, __LINE__); } @@ -3022,16 +2966,10 @@ private: setTSHHook(env, dest, testStrong); // finish escrow - Json::Value tx; - if (!env.current()->rules().enabled(fixXahauV1)) - { - tx = finish(dest, account, 0); - } - else - { - tx = finish(dest, account); - } - env(tx, escrow_id(escrowId), fee(XRP(1)), ter(tesSUCCESS)); + env(finish(dest, account), + escrow_id(escrowId), + fee(XRP(1)), + ter(tesSUCCESS)); env.close(); // verify tsh hook triggered @@ -3070,23 +3008,14 @@ private: setTSHHook(env, account, testStrong); // finish escrow - bool const fixV1 = env.current()->rules().enabled(fixXahauV1); - Json::Value tx; - if (!fixV1) - { - tx = finish(dest, account, 0); - } - else - { - tx = finish(dest, account); - } - env(tx, escrow_id(escrowId), fee(XRP(1)), ter(tesSUCCESS)); + env(finish(dest, account), + escrow_id(escrowId), + fee(XRP(1)), + ter(tesSUCCESS)); env.close(); // verify tsh hook triggered - auto const expected = - (fixV1 ? (testStrong ? tshSTRONG : tshSTRONG) - : (testStrong ? tshNONE : tshNONE)); + auto const expected = testStrong ? tshSTRONG : tshSTRONG; testTSHStrongWeak(env, expected, __LINE__); } @@ -3132,17 +3061,10 @@ private: setTSHHook(env, gw, testStrong); // finish escrow - bool const fixV1 = env.current()->rules().enabled(fixXahauV1); - Json::Value tx; - if (!fixV1) - { - tx = finish(dest, account, 0); - } - else - { - tx = finish(dest, account); - } - env(tx, escrow_id(escrowId), fee(XRP(1)), ter(tesSUCCESS)); + env(finish(dest, account), + escrow_id(escrowId), + fee(XRP(1)), + ter(tesSUCCESS)); env.close(); // verify tsh hook triggered @@ -3448,10 +3370,7 @@ private: env.close(); // verify tsh hook triggered - bool const fixV2 = env.current()->rules().enabled(fixXahauV2); - auto const expected = - (fixV2 ? (testStrong ? tshNONE : tshWEAK) - : (testStrong ? tshSTRONG : tshSTRONG)); + auto const expected = testStrong ? tshNONE : tshWEAK; testTSHStrongWeak(env, expected, __LINE__); } } @@ -6310,10 +6229,7 @@ private: env.close(); // verify tsh hook triggered - bool const fixV2 = env.current()->rules().enabled(fixXahauV2); - auto const expected = - (fixV2 ? (testStrong ? tshSTRONG : tshSTRONG) - : (testStrong ? tshNONE : tshNONE)); + auto const expected = testStrong ? tshSTRONG : tshSTRONG; testTSHStrongWeak(env, expected, __LINE__); } @@ -6394,10 +6310,7 @@ private: env.close(); // verify tsh hook triggered - bool const fixV2 = env.current()->rules().enabled(fixXahauV2); - auto const expected = - (fixV2 ? (testStrong ? tshSTRONG : tshSTRONG) - : (testStrong ? tshNONE : tshNONE)); + auto const expected = testStrong ? tshSTRONG : tshSTRONG; testTSHStrongWeak(env, expected, __LINE__); } } @@ -6503,15 +6416,12 @@ private: env.close(); // verify tsh hook triggered - bool const fixV1 = env.current()->rules().enabled(fixXahauV1); bool const withIOUIssuerWeakTSH = env.current()->rules().enabled(featureIOUIssuerWeakTSH); - auto const expected = - (fixV1 - ? (testStrong ? tshNONE - : (withIOUIssuerWeakTSH ? tshWEAK : tshNONE)) - : (testStrong ? tshSTRONG : tshSTRONG)); + auto const expected = testStrong + ? tshNONE + : (withIOUIssuerWeakTSH ? tshWEAK : tshNONE); testTSHStrongWeak(env, expected, __LINE__); } @@ -6610,14 +6520,11 @@ private: env.close(); // verify tsh hook triggered - bool const fixV1 = env.current()->rules().enabled(fixXahauV1); bool const withIOUIssuerWeakTSH = env.current()->rules().enabled(featureIOUIssuerWeakTSH); - auto const expected = - (fixV1 - ? (testStrong ? tshNONE - : (withIOUIssuerWeakTSH ? tshWEAK : tshNONE)) - : (testStrong ? tshSTRONG : tshSTRONG)); + auto const expected = testStrong + ? tshNONE + : (withIOUIssuerWeakTSH ? tshWEAK : tshNONE); testTSHStrongWeak(env, expected, __LINE__); } @@ -6667,15 +6574,12 @@ private: env.close(); // verify tsh hook triggered - bool const fixV1 = env.current()->rules().enabled(fixXahauV1); bool const withIOUIssuerWeakTSH = env.current()->rules().enabled(featureIOUIssuerWeakTSH); - auto const expected = - (fixV1 - ? (testStrong ? tshNONE - : (withIOUIssuerWeakTSH ? tshWEAK : tshNONE)) - : (testStrong ? tshSTRONG : tshSTRONG)); + auto const expected = testStrong + ? tshNONE + : (withIOUIssuerWeakTSH ? tshWEAK : tshNONE); testTSHStrongWeak(env, expected, __LINE__); } @@ -7735,7 +7639,6 @@ private: env.close(); auto const preDest = env.balance(dest); - bool const withFix = env.current()->rules().enabled(fixXahauV2); env.app().openLedger().modify([&](OpenView& view, beast::Journal j) { auto const tx = @@ -7743,24 +7646,15 @@ private: auto result = ripple::apply(env.app(), view, *tx, tapNONE, env.journal); - bool const applyResult = withFix ? false : true; - if (withFix) - { - BEAST_EXPECT(result.ter == tefNONDIR_EMIT); - } - else - { - BEAST_EXPECT(result.ter == tesSUCCESS); - } - BEAST_EXPECT(result.applied == applyResult); + BEAST_EXPECT(result.ter == tefNONDIR_EMIT); + BEAST_EXPECT(!result.applied); return result.applied; }); env.close(); auto const postDest = env.balance(dest); - auto const postValue = withFix ? XRP(0) : XRP(1); - BEAST_EXPECT(postDest == preDest + postValue); + BEAST_EXPECT(postDest == preDest); for (size_t i = 0; i < 4; i++) { @@ -7771,8 +7665,7 @@ private: } auto const postDest1 = env.balance(dest); - auto const postValue1 = withFix ? XRP(0) : XRP(2); - BEAST_EXPECT(postDest1 == postDest + postValue1); + BEAST_EXPECT(postDest1 == postDest); } void @@ -8621,10 +8514,8 @@ public: using namespace test::jtx; static FeatureBitset const all{supported_amendments()}; - static std::array const feats{ + static std::array const feats{ all, - all - fixXahauV1 - fixXahauV2 - featureIOUIssuerWeakTSH, - all - fixXahauV2 - featureIOUIssuerWeakTSH, all - featureIOUIssuerWeakTSH, }; @@ -8654,14 +8545,10 @@ public: } \ }; -SETHOOKTSH_TEST(1, false) -SETHOOKTSH_TEST(2, false) -SETHOOKTSH_TEST(3, true) +SETHOOKTSH_TEST(1, true) BEAST_DEFINE_TESTSUITE_PRIO(SetHookTSH0, app, ripple, 2); BEAST_DEFINE_TESTSUITE_PRIO(SetHookTSH1, app, ripple, 2); -BEAST_DEFINE_TESTSUITE_PRIO(SetHookTSH2, app, ripple, 2); -BEAST_DEFINE_TESTSUITE_PRIO(SetHookTSH3, app, ripple, 2); } // namespace test } // namespace ripple diff --git a/src/test/app/SetHook_test.cpp b/src/test/app/SetHook_test.cpp index 8c60f38624..bf09631c54 100644 --- a/src/test/app/SetHook_test.cpp +++ b/src/test/app/SetHook_test.cpp @@ -294,9 +294,7 @@ public: iv[jss::CreateCode] = ""; jv[jss::Hooks][0U][jss::Hook] = iv; - bool const fixV1 = env.current()->rules().enabled(fixXahauV1); - auto const txResult = fixV1 ? ter(tesSUCCESS) : ter(tefBAD_LEDGER); - env(jv, HSFEE, txResult); + env(jv, HSFEE, ter(tesSUCCESS)); env.close(); } @@ -3703,8 +3701,6 @@ public: env(invoke, M("test emit"), fee(XRP(1))); - bool const fixV2 = env.current()->rules().enabled(fixXahauV2); - std::optional emithash; { auto meta = env.meta(); // meta can close @@ -3714,9 +3710,7 @@ public: BEAST_REQUIRE(meta->isFieldPresent(sfHookExecutions)); auto const hookEmissions = meta->getFieldArray(sfHookEmissions); - BEAST_EXPECT( - hookEmissions[0u].isFieldPresent(sfEmitNonce) == fixV2 ? true - : false); + BEAST_EXPECT(hookEmissions[0u].isFieldPresent(sfEmitNonce)); BEAST_EXPECT( hookEmissions[0u].getAccountID(sfHookAccount) == alice.id()); @@ -3811,8 +3805,7 @@ public: BEAST_EXPECT(hookExecutions[0].getFieldU8(sfHookResult) == 3); BEAST_EXPECT( hookExecutions[0].getFieldU16(sfHookEmitCount) == 2); - if (fixV2) - BEAST_EXPECT(hookExecutions[0].getFieldU32(sfFlags) == 2); + BEAST_EXPECT(hookExecutions[0].getFieldU32(sfFlags) == 2); } env.close(); burden_expected *= 2U; @@ -3836,8 +3829,7 @@ public: BEAST_EXPECT( hookExecutions[0].getFieldU64(sfHookReturnCode) == 283); // emission failure on first emit - if (fixV2) - BEAST_EXPECT(hookExecutions[0].getFieldU32(sfFlags) == 2); + BEAST_EXPECT(hookExecutions[0].getFieldU32(sfFlags) == 2); } BEAST_EXPECT(txcount == 256); } @@ -4094,8 +4086,6 @@ public: env(invoke, M("test emit"), fee(XRP(1))); - bool const fixV2 = env.current()->rules().enabled(fixXahauV2); - std::optional emithash; { auto meta = env.meta(); // meta can close @@ -4105,9 +4095,7 @@ public: BEAST_REQUIRE(meta->isFieldPresent(sfHookExecutions)); auto const hookEmissions = meta->getFieldArray(sfHookEmissions); - BEAST_EXPECT( - hookEmissions[0u].isFieldPresent(sfEmitNonce) == fixV2 ? true - : false); + BEAST_EXPECT(hookEmissions[0u].isFieldPresent(sfEmitNonce)); BEAST_EXPECT( hookEmissions[0u].getAccountID(sfHookAccount) == alice.id()); @@ -4202,8 +4190,7 @@ public: BEAST_EXPECT(hookExecutions[0].getFieldU8(sfHookResult) == 3); BEAST_EXPECT( hookExecutions[0].getFieldU16(sfHookEmitCount) == 2); - if (fixV2) - BEAST_EXPECT(hookExecutions[0].getFieldU32(sfFlags) == 2); + BEAST_EXPECT(hookExecutions[0].getFieldU32(sfFlags) == 2); } env.close(); burden_expected *= 2U; @@ -4227,8 +4214,7 @@ public: BEAST_EXPECT( hookExecutions[0].getFieldU64(sfHookReturnCode) == 172); // emission failure on first emit - if (fixV2) - BEAST_EXPECT(hookExecutions[0].getFieldU32(sfFlags) == 2); + BEAST_EXPECT(hookExecutions[0].getFieldU32(sfFlags) == 2); } BEAST_EXPECT(txcount == 256); } @@ -6965,12 +6951,8 @@ public: BEAST_REQUIRE(hookExecutions.size() == 2); // get the data in the return code of the execution - bool const fixV2 = env.current()->rules().enabled(fixXahauV2); - if (fixV2) - { - BEAST_EXPECT(hookExecutions[0].getFieldU32(sfFlags) == 5); - BEAST_EXPECT(hookExecutions[1].getFieldU32(sfFlags) == 0); - } + BEAST_EXPECT(hookExecutions[0].getFieldU32(sfFlags) == 5); + BEAST_EXPECT(hookExecutions[1].getFieldU32(sfFlags) == 0); BEAST_EXPECT(hookExecutions[0].getFieldU64(sfHookReturnCode) == 0); BEAST_EXPECT(hookExecutions[1].getFieldU64(sfHookReturnCode) == 1); @@ -10043,19 +10025,13 @@ public: fee(XRP(1))); } - // fixXahauV1 - bool const fixV1 = env.current()->rules().enabled(fixXahauV1); - auto const txResult = fixV1 ? ter(tecHOOK_REJECTED) : ter(tesSUCCESS); env(pay(bob, alice, XRP(1)), M("test state_foreign_set_max"), fee(XRP(1)), - ter(txResult)); + ter(tecHOOK_REJECTED)); env.close(); // verify hook result - // TOO_MANY_NAMESPACES / -45 - std::string const hookResult = fixV1 ? "800000000000002d" : "9"; - Json::Value params; params[jss::transaction] = env.tx()->getJson(JsonOptions::none)[jss::hash]; @@ -10063,7 +10039,9 @@ public: auto const meta = jrr[jss::result][jss::meta]; auto const executions = meta[sfHookExecutions.jsonName]; auto const execution = executions[0u][sfHookExecution.jsonName]; - BEAST_EXPECT(execution[sfHookReturnCode.jsonName] == hookResult); + // TOO_MANY_NAMESPACES / -45 + BEAST_EXPECT( + execution[sfHookReturnCode.jsonName] == "800000000000002d"); } void @@ -15202,16 +15180,12 @@ public: using namespace test::jtx; static FeatureBitset const all{supported_amendments()}; - static std::array const feats{ + static std::array const feats{ all, - all - fixXahauV2, - all - fixXahauV1 - fixXahauV2, - all - fixXahauV1 - fixXahauV2 - fixNSDelete, - all - fixXahauV1 - fixXahauV2 - fixNSDelete - fixPageCap, - all - fixXahauV1 - fixXahauV2 - fixNSDelete - fixPageCap - - featureHookCanEmit, - all - fixXahauV1 - fixXahauV2 - fixNSDelete - fixPageCap - - featureExtendedHookState, + all - fixNSDelete, + all - fixNSDelete - fixPageCap, + all - fixNSDelete - fixPageCap - featureHookCanEmit, + all - fixNSDelete - fixPageCap - featureExtendedHookState, all - featureNamedHooks, }; @@ -15472,9 +15446,7 @@ SETHOOK_TEST(1, false) SETHOOK_TEST(2, false) SETHOOK_TEST(3, false) SETHOOK_TEST(4, false) -SETHOOK_TEST(5, false) -SETHOOK_TEST(6, false) -SETHOOK_TEST(7, true) +SETHOOK_TEST(5, true) BEAST_DEFINE_TESTSUITE_PRIO(SetHook0, app, ripple, 2); BEAST_DEFINE_TESTSUITE_PRIO(SetHook1, app, ripple, 2); @@ -15482,8 +15454,6 @@ BEAST_DEFINE_TESTSUITE_PRIO(SetHook2, app, ripple, 2); BEAST_DEFINE_TESTSUITE_PRIO(SetHook3, app, ripple, 2); BEAST_DEFINE_TESTSUITE_PRIO(SetHook4, app, ripple, 2); BEAST_DEFINE_TESTSUITE_PRIO(SetHook5, app, ripple, 2); -BEAST_DEFINE_TESTSUITE_PRIO(SetHook6, app, ripple, 2); -BEAST_DEFINE_TESTSUITE_PRIO(SetHook7, app, ripple, 2); } // namespace test } // namespace ripple #undef M diff --git a/src/test/app/URIToken_test.cpp b/src/test/app/URIToken_test.cpp index a4c84138f6..b04e08508c 100644 --- a/src/test/app/URIToken_test.cpp +++ b/src/test/app/URIToken_test.cpp @@ -191,7 +191,6 @@ struct URIToken_test : public beast::unit_test::suite using namespace jtx; using namespace std::literals::chrono_literals; - // fixXahauV1 { Env env{*this, features}; auto const alice = Account("alice"); @@ -204,11 +203,9 @@ struct URIToken_test : public beast::unit_test::suite std::string const hexid{strHex(tid)}; // temMALFORMED - cannot include sfDestination without sfAmount - bool const withFixXahauV1 = - env.current()->rules().enabled(fixXahauV1); - auto const txResult = - withFixXahauV1 ? ter(temMALFORMED) : ter(tefINTERNAL); - env(uritoken::mint(alice, uri), uritoken::dest(bob), txResult); + env(uritoken::mint(alice, uri), + uritoken::dest(bob), + ter(temMALFORMED)); env.close(); } @@ -517,12 +514,10 @@ struct URIToken_test : public beast::unit_test::suite env.close(); // tecINSUFFICIENT_FUNDS - insufficient xrp - fees - // fixXahauV1 - fix checking wrong account for insufficient xrp env(pay(env.master, alice, XRP(10000))); - auto const txResult = env.current()->rules().enabled(fixXahauV1) - ? ter(tecINSUFFICIENT_FUNDS) - : ter(tecINTERNAL); - env(uritoken::buy(bob, hexid), uritoken::amt(XRP(10000)), txResult); + env(uritoken::buy(bob, hexid), + uritoken::amt(XRP(10000)), + ter(tecINSUFFICIENT_FUNDS)); env.close(); // clear sell and reset new sell @@ -579,11 +574,9 @@ struct URIToken_test : public beast::unit_test::suite env.close(); // tecINSUFFICIENT_FUNDS - insufficient xrp - fees - // fixXahauV1 - fix checking wrong account for insufficient xrp - auto const txResult1 = env.current()->rules().enabled(fixXahauV1) - ? ter(tecINSUFFICIENT_FUNDS) - : ter(tecINTERNAL); - env(uritoken::buy(bob, hexid), uritoken::amt(XRP(1000)), txResult1); + env(uritoken::buy(bob, hexid), + uritoken::amt(XRP(1000)), + ter(tecINSUFFICIENT_FUNDS)); env.close(); // clear sell and set usd sell @@ -623,12 +616,10 @@ struct URIToken_test : public beast::unit_test::suite env.close(); // tecNO_LINE_INSUF_RESERVE - insufficient xrp to create line - auto const txResult = env.current()->rules().enabled(fixXahauV1) - ? ter(tecINSUF_RESERVE_SELLER) - : ter(tecNO_LINE_INSUF_RESERVE); - env(noop(echo), fee(XRP(50)), ter(tesSUCCESS)); - env(uritoken::buy(dave, hexid), uritoken::amt(USD(1)), txResult); + env(uritoken::buy(dave, hexid), + uritoken::amt(USD(1)), + ter(tecINSUF_RESERVE_SELLER)); env.close(); } @@ -2121,14 +2112,7 @@ struct URIToken_test : public beast::unit_test::suite env(uritoken::buy(bob, id), uritoken::amt(delta)); env.close(); auto const postAlice = env.balance(alice, USD.issue()); - if (!env.current()->rules().enabled(fixXahauV1)) - { - BEAST_EXPECT(to_string(postAlice.value()) == tc.multiply); - } - else - { - BEAST_EXPECT(to_string(postAlice.value()) == tc.divide); - } + BEAST_EXPECT(to_string(postAlice.value()) == tc.divide); BEAST_EXPECT(env.balance(bob, USD.issue()) == preBob - delta); } @@ -2167,14 +2151,7 @@ struct URIToken_test : public beast::unit_test::suite env.close(); auto const postAlice = env.balance(alice, USD.issue()); - if (!env.current()->rules().enabled(fixXahauV1)) - { - BEAST_EXPECT(postAlice.value() == preAlice); - } - else - { - BEAST_EXPECT(postAlice.value() == preAlice); - } + BEAST_EXPECT(postAlice.value() == preAlice); BEAST_EXPECT(env.balance(bob, USD.issue()) == preBob - USD(0)); } @@ -2682,7 +2659,6 @@ public: using namespace test::jtx; auto const sa = supported_amendments(); testWithFeats(sa); - testWithFeats(sa - fixXahauV1); } }; diff --git a/src/test/app/XahauGenesis_test.cpp b/src/test/app/XahauGenesis_test.cpp index b3b5e7e951..7128142315 100644 --- a/src/test/app/XahauGenesis_test.cpp +++ b/src/test/app/XahauGenesis_test.cpp @@ -4354,11 +4354,11 @@ struct XahauGenesis_test : public beast::unit_test::suite using namespace std::chrono_literals; testcase("test claim reward valid for L1 with unl report"); - for (bool const withXahauV1 : {true, false}) { - FeatureBitset _features = features - featureXahauGenesis; - auto const amend = withXahauV1 ? _features : _features - fixXahauV1; - Env env{*this, makeNetworkConfig(21337), amend}; + Env env{ + *this, + makeNetworkConfig(21337), + features - featureXahauGenesis}; double const rateDrops = 0.00333333333 * 1'000'000; STAmount const feesXRP = XRP(1); @@ -4453,9 +4453,7 @@ struct XahauGenesis_test : public beast::unit_test::suite accountKeyAndSle(*env.current(), alice); // claim reward - auto const txResult = - withXahauV1 ? ter(tesSUCCESS) : ter(tecHOOK_REJECTED); - env(claimReward(alice, env.master), fee(feesXRP), txResult); + env(claimReward(alice, env.master), fee(feesXRP), ter(tesSUCCESS)); env.close(); // trigger emitted txn @@ -4464,8 +4462,7 @@ struct XahauGenesis_test : public beast::unit_test::suite // calculate rewards STAmount const netReward = rewardUserAmount(*acctSle, preLedger, rateDrops); - STAmount const l1Reward = - withXahauV1 ? rewardL1Amount(netReward, 20) : STAmount(0); + STAmount const l1Reward = rewardL1Amount(netReward, 20); // validate govern rewards BEAST_EXPECT(env.balance(bob) == preBob + l1Reward); @@ -4475,16 +4472,14 @@ struct XahauGenesis_test : public beast::unit_test::suite // validate account fields STAmount const postAlice = preAlice + netReward + l1Reward; - bool const boolResult = withXahauV1 ? true : false; bool const has240819 = env.current()->rules().enabled(fix240819); - BEAST_EXPECT( - expectAccountFields( - env, - alice, - preLedger, - preLedger + 1, - has240819 ? (preAlice - feesXRP) : postAlice, - preTime) == boolResult); + BEAST_EXPECT(expectAccountFields( + env, + alice, + preLedger, + preLedger + 1, + has240819 ? (preAlice - feesXRP) : postAlice, + preTime)); } } diff --git a/src/xrpld/app/hook/detail/HookAPI.cpp b/src/xrpld/app/hook/detail/HookAPI.cpp index dec51f781f..ca9d309764 100644 --- a/src/xrpld/app/hook/detail/HookAPI.cpp +++ b/src/xrpld/app/hook/detail/HookAPI.cpp @@ -2743,8 +2743,7 @@ HookAPI::set_state_cache( if (modified && stateMap.modified_entry_count >= max_state_modifications) return Unexpected(TOO_MANY_STATE_MODIFICATIONS); - bool const createNamespace = view.rules().enabled(fixXahauV1) && - !view.exists(keylet::hookStateDir(acc, ns)); + bool const createNamespace = !view.exists(keylet::hookStateDir(acc, ns)); if (stateMap.find(acc) == stateMap.end()) { diff --git a/src/xrpld/app/hook/detail/applyHook.cpp b/src/xrpld/app/hook/detail/applyHook.cpp index f967e9ae68..270df4cc45 100644 --- a/src/xrpld/app/hook/detail/applyHook.cpp +++ b/src/xrpld/app/hook/detail/applyHook.cpp @@ -70,9 +70,6 @@ getTransactionalStakeHolders(STTx const& tx, ReadView const& rv) return rv.read(keylet::nftoffer(*id)); }; - bool const fixV1 = rv.rules().enabled(fixXahauV1); - bool const fixV2 = rv.rules().enabled(fixXahauV2); - switch (tt) { case ttCRON: { @@ -121,7 +118,7 @@ getTransactionalStakeHolders(STTx const& tx, ReadView const& rv) case ttIMPORT: { if (tx.isFieldPresent(sfIssuer)) - ADD_TSH(tx.getAccountID(sfIssuer), fixV2 ? tshWEAK : tshSTRONG); + ADD_TSH(tx.getAccountID(sfIssuer), tshWEAK); break; } @@ -146,34 +143,13 @@ getTransactionalStakeHolders(STTx const& tx, ReadView const& rv) break; // pass, already a TSH - // new logic - if (fixV1) - { - // the owner burns their token, and the issuer is a weak TSH - if (*otxnAcc == owner && rv.exists(keylet::account(issuer))) - ADD_TSH(issuer, tshWEAK); - // the issuer burns the owner's token, and the owner is a weak - // TSH - else if (rv.exists(keylet::account(owner))) - ADD_TSH(owner, tshWEAK); - - break; - } - - // old logic - { - if (*otxnAcc == owner) - { - // the owner burns their token, and the issuer is a weak TSH - ADD_TSH(issuer, tshSTRONG); - } - else - { - // the issuer burns the owner's token, and the owner is a - // weak TSH - ADD_TSH(owner, tshSTRONG); - } - } + // the owner burns their token, and the issuer is a weak TSH + if (*otxnAcc == owner && rv.exists(keylet::account(issuer))) + ADD_TSH(issuer, tshWEAK); + // the issuer burns the owner's token, and the owner is a weak + // TSH + else if (rv.exists(keylet::account(owner))) + ADD_TSH(owner, tshWEAK); break; } @@ -207,15 +183,12 @@ getTransactionalStakeHolders(STTx const& tx, ReadView const& rv) case ttURITOKEN_MINT: { // destination is a strong tsh - if (fixV2 && tx.isFieldPresent(sfDestination)) + if (tx.isFieldPresent(sfDestination)) ADD_TSH(tx.getAccountID(sfDestination), tshSTRONG); break; } case ttURITOKEN_CANCEL_SELL_OFFER: { - if (!fixV2) - break; - Keylet const id{ltURI_TOKEN, tx.getFieldH256(sfURITokenID)}; if (!rv.exists(id)) return {}; @@ -386,61 +359,38 @@ getTransactionalStakeHolders(STTx const& tx, ReadView const& rv) case ttESCROW_CANCEL: case ttESCROW_FINISH: { - // new logic - if (fixV1) - { - if (!tx.isFieldPresent(sfOwner)) - return {}; + if (!tx.isFieldPresent(sfOwner)) + return {}; - AccountID const owner = tx.getAccountID(sfOwner); + AccountID const owner = tx.getAccountID(sfOwner); - bool const hasSeq = tx.isFieldPresent(sfOfferSequence); - bool const hasID = tx.isFieldPresent(sfEscrowID); - if (!hasSeq && !hasID) - return {}; + bool const hasSeq = tx.isFieldPresent(sfOfferSequence); + bool const hasID = tx.isFieldPresent(sfEscrowID); + if (!hasSeq && !hasID) + return {}; - Keylet kl = hasSeq - ? keylet::escrow(owner, tx.getFieldU32(sfOfferSequence)) - : Keylet(ltESCROW, tx.getFieldH256(sfEscrowID)); + Keylet kl = hasSeq + ? keylet::escrow(owner, tx.getFieldU32(sfOfferSequence)) + : Keylet(ltESCROW, tx.getFieldH256(sfEscrowID)); - auto escrow = rv.read(kl); + auto escrow = rv.read(kl); - if (!escrow || - escrow->getFieldU16(sfLedgerEntryType) != ltESCROW) - return {}; + if (!escrow || escrow->getFieldU16(sfLedgerEntryType) != ltESCROW) + return {}; - // this should always be the same as owner, but defensively... - AccountID const src = escrow->getAccountID(sfAccount); - AccountID const dst = escrow->getAccountID(sfDestination); + // this should always be the same as owner, but defensively... + AccountID const src = escrow->getAccountID(sfAccount); + AccountID const dst = escrow->getAccountID(sfDestination); - // the source account is a strong transacitonal stakeholder for - // fin and can - ADD_TSH(src, tshSTRONG); + // the source account is a strong transacitonal stakeholder for + // fin and can + ADD_TSH(src, tshSTRONG); - // the dest acc is a strong tsh for fin and weak for can - if (src != dst) - ADD_TSH(dst, tt == ttESCROW_FINISH ? tshSTRONG : tshWEAK); + // the dest acc is a strong tsh for fin and weak for can + if (src != dst) + ADD_TSH(dst, tt == ttESCROW_FINISH ? tshSTRONG : tshWEAK); - break; - } - // old logic - { - if (!tx.isFieldPresent(sfOwner) || - !tx.isFieldPresent(sfOfferSequence)) - return {}; - - auto escrow = rv.read(keylet::escrow( - tx.getAccountID(sfOwner), tx.getFieldU32(sfOfferSequence))); - - if (!escrow) - return {}; - - ADD_TSH(escrow->getAccountID(sfAccount), tshSTRONG); - ADD_TSH( - escrow->getAccountID(sfDestination), - tt == ttESCROW_FINISH ? tshSTRONG : tshWEAK); - break; - } + break; } case ttPAYCHAN_FUND: @@ -1366,13 +1316,9 @@ DEFINE_HOOK_FUNCTION( auto const key = make_state_key( std::string_view{(const char*)(memory + kread_ptr), (size_t)kread_len}); - if (view.rules().enabled(fixXahauV1)) - { - auto const sleAccount = view.peek(hookCtx.result.accountKeylet); - if (!sleAccount) - // should return hook_api::hook_return_code - return static_cast(tefINTERNAL); - } + if (!view.exists(hookCtx.result.accountKeylet)) + // should return hook_api::hook_return_code + return static_cast(tefINTERNAL); if (!key) return INTERNAL_ERROR; @@ -1582,7 +1528,6 @@ hook::finalizeHookResult( } } - bool const fixV2 = applyCtx.view().rules().enabled(fixXahauV2); // add a metadata entry for this hook execution result { STObject meta{sfHookExecution}; @@ -1611,18 +1556,14 @@ hook::finalizeHookResult( meta.setFieldU16(sfHookStateChangeCount, hookResult.changedStateCount); meta.setFieldH256(sfHookHash, hookResult.hookHash); - // add informational flags in fix2 - if (fixV2) - { - uint32_t flags = 0; - if (hookResult.isStrong) - flags |= hefSTRONG; - if (hookResult.isCallback) - flags |= hefCALLBACK; - if (hookResult.executeAgainAsWeak) - flags |= hefDOAAW; - meta.setFieldU32(sfFlags, flags); - } + uint32_t flags = 0; + if (hookResult.isStrong) + flags |= hefSTRONG; + if (hookResult.isCallback) + flags |= hefCALLBACK; + if (hookResult.executeAgainAsWeak) + flags |= hefDOAAW; + meta.setFieldU32(sfFlags, flags); avi.addHookExecutionMetaData(std::move(meta)); } @@ -1636,8 +1577,7 @@ hook::finalizeHookResult( meta.setFieldH256(sfHookHash, hookResult.hookHash); meta.setAccountID(sfHookAccount, hookResult.account); meta.setFieldH256(sfEmittedTxnID, etxnid); - if (fixV2) - meta.setFieldH256(sfEmitNonce, enonce); + meta.setFieldH256(sfEmitNonce, enonce); avi.addHookEmissionMetaData(std::move(meta)); } } diff --git a/src/xrpld/app/tx/detail/CancelOffer.cpp b/src/xrpld/app/tx/detail/CancelOffer.cpp index c91ca9bd44..1b7e7fb6e0 100644 --- a/src/xrpld/app/tx/detail/CancelOffer.cpp +++ b/src/xrpld/app/tx/detail/CancelOffer.cpp @@ -39,26 +39,14 @@ CancelOffer::preflight(PreflightContext const& ctx) return temINVALID_FLAG; } - if (ctx.rules.enabled(fixXahauV1)) + if ((!ctx.tx.isFieldPresent(sfOfferSequence) && + !ctx.tx.isFieldPresent(sfOfferID)) || + (ctx.tx.isFieldPresent(sfOfferSequence) && + ctx.tx.isFieldPresent(sfOfferID))) { - if ((!ctx.tx.isFieldPresent(sfOfferSequence) && - !ctx.tx.isFieldPresent(sfOfferID)) || - (ctx.tx.isFieldPresent(sfOfferSequence) && - ctx.tx.isFieldPresent(sfOfferID))) - { - JLOG(ctx.j.trace()) - << "CancelOffer::preflight: invalid sequence or offer id"; - return temBAD_SEQUENCE; - } - } - else - { - if (!ctx.tx.isFieldPresent(sfOfferSequence) || - ctx.tx[sfOfferSequence] == 0) - { - JLOG(ctx.j.trace()) << "CancelOffer::preflight: missing sequence"; - return temBAD_SEQUENCE; - } + JLOG(ctx.j.trace()) + << "CancelOffer::preflight: invalid sequence or offer id"; + return temBAD_SEQUENCE; } return preflight2(ctx); @@ -112,8 +100,7 @@ CancelOffer::doApply() else JLOG(j_.debug()) << "Trying to cancel offer #" << *offerSequence; - bool const fixV1 = view().rules().enabled(fixXahauV1); - if (fixV1 && sleOffer->getFieldU16(sfLedgerEntryType) != ltOFFER) + if (sleOffer->getFieldU16(sfLedgerEntryType) != ltOFFER) { JLOG(j_.debug()) << "OfferCancel specified non-offer ledger object"; return tecINTERNAL; diff --git a/src/xrpld/app/tx/detail/Escrow.cpp b/src/xrpld/app/tx/detail/Escrow.cpp index 9d93142417..fd2f26c735 100644 --- a/src/xrpld/app/tx/detail/Escrow.cpp +++ b/src/xrpld/app/tx/detail/Escrow.cpp @@ -454,23 +454,11 @@ EscrowFinish::preflight(PreflightContext const& ctx) // sfOfferSequence was changed to optional, so ensure the behaviour is the // same until amendment passes - if (!ctx.rules.enabled(fixXahauV1)) - { - if (!ctx.tx.isFieldPresent(sfOfferSequence)) - return temMALFORMED; - - if (ctx.tx.isFieldPresent(sfEscrowID) && - ctx.tx.getFieldU32(sfOfferSequence) != 0) - return temMALFORMED; - } - else - { - if ((!ctx.tx.isFieldPresent(sfEscrowID) && - !ctx.tx.isFieldPresent(sfOfferSequence)) || - (ctx.tx.isFieldPresent(sfEscrowID) && - ctx.tx.isFieldPresent(sfOfferSequence))) - return temMALFORMED; - } + if ((!ctx.tx.isFieldPresent(sfEscrowID) && + !ctx.tx.isFieldPresent(sfOfferSequence)) || + (ctx.tx.isFieldPresent(sfEscrowID) && + ctx.tx.isFieldPresent(sfOfferSequence))) + return temMALFORMED; return tesSUCCESS; } @@ -512,8 +500,6 @@ EscrowFinish::doApply() std::optional escrowID = ctx_.tx[~sfEscrowID]; std::optional offerSequence = ctx_.tx[~sfOfferSequence]; - bool const fixV1 = view().rules().enabled(fixXahauV1); - Keylet k = escrowID ? Keylet(ltESCROW, *escrowID) : keylet::escrow(ctx_.tx[sfOwner], *offerSequence); @@ -521,7 +507,7 @@ EscrowFinish::doApply() if (!slep) return tecNO_TARGET; - if (fixV1 && slep->getFieldU16(sfLedgerEntryType) != ltESCROW) + if (slep->getFieldU16(sfLedgerEntryType) != ltESCROW) return tecINTERNAL; AccountID const account = (*slep)[sfAccount]; @@ -740,23 +726,11 @@ EscrowCancel::preflight(PreflightContext const& ctx) // sfOfferSequence was changed to optional, so ensure the behaviour is the // same until amendment passes - if (!ctx.rules.enabled(fixXahauV1)) - { - if (!ctx.tx.isFieldPresent(sfOfferSequence)) - return temMALFORMED; - - if (ctx.tx.isFieldPresent(sfEscrowID) && - ctx.tx.getFieldU32(sfOfferSequence) != 0) - return temMALFORMED; - } - else - { - if ((!ctx.tx.isFieldPresent(sfEscrowID) && - !ctx.tx.isFieldPresent(sfOfferSequence)) || - (ctx.tx.isFieldPresent(sfEscrowID) && - ctx.tx.isFieldPresent(sfOfferSequence))) - return temMALFORMED; - } + if ((!ctx.tx.isFieldPresent(sfEscrowID) && + !ctx.tx.isFieldPresent(sfOfferSequence)) || + (ctx.tx.isFieldPresent(sfEscrowID) && + ctx.tx.isFieldPresent(sfOfferSequence))) + return temMALFORMED; return preflight2(ctx); } @@ -772,8 +746,6 @@ EscrowCancel::doApply() std::optional escrowID = ctx_.tx[~sfEscrowID]; std::optional offerSequence = ctx_.tx[~sfOfferSequence]; - bool const fixV1 = view().rules().enabled(fixXahauV1); - Keylet k = escrowID ? Keylet(ltESCROW, *escrowID) : keylet::escrow(ctx_.tx[sfOwner], *offerSequence); @@ -781,7 +753,7 @@ EscrowCancel::doApply() if (!slep) return tecNO_TARGET; - if (fixV1 && slep->getFieldU16(sfLedgerEntryType) != ltESCROW) + if (slep->getFieldU16(sfLedgerEntryType) != ltESCROW) return tecINTERNAL; if (ctx_.view().rules().enabled(fix1571)) diff --git a/src/xrpld/app/tx/detail/GenesisMint.cpp b/src/xrpld/app/tx/detail/GenesisMint.cpp index f467bf1e63..18e92c26a3 100644 --- a/src/xrpld/app/tx/detail/GenesisMint.cpp +++ b/src/xrpld/app/tx/detail/GenesisMint.cpp @@ -75,8 +75,6 @@ GenesisMint::preflight(PreflightContext const& ctx) return temMALFORMED; } - bool const allowDuplicates = ctx.rules.enabled(fixXahauV1); - std::unordered_set alreadySeen; for (auto const& dest : dests) { @@ -139,17 +137,6 @@ GenesisMint::preflight(PreflightContext const& ctx) "disallowed account zero or one."; return temMALFORMED; } - - if (allowDuplicates) - continue; - - if (alreadySeen.find(accid) != alreadySeen.end()) - { - JLOG(ctx.j.warn()) << "GenesisMint: duplicate in destinations."; - return temMALFORMED; - } - - alreadySeen.emplace(accid); } return preflight2(ctx); @@ -177,99 +164,8 @@ GenesisMint::doApply() { auto const& dests = ctx_.tx.getFieldArray(sfGenesisMints); - if (!view().rules().enabled(fixXahauV1)) - { - STAmount dropsAdded{0}; - for (auto const& dest : dests) - { - auto const amt = dest[~sfAmount]; - - if (amt && !isXRP(*amt)) - { - JLOG(ctx_.journal.warn()) << "GenesisMint: Non-xrp amount."; - return tecINTERNAL; - } - - auto const flags = dest[~sfGovernanceFlags]; - auto const marks = dest[~sfGovernanceMarks]; - - auto const id = dest.getAccountID(sfDestination); - auto const k = keylet::account(id); - auto sle = view().peek(k); - - bool const created = !sle; - - if (created) - { - // Create the account. - std::uint32_t const seqno{ - view().info().parentCloseTime.time_since_epoch().count()}; - - sle = std::make_shared(k); - sle->setAccountID(sfAccount, id); - - sle->setFieldU32(sfSequence, seqno); - - if (amt) - { - sle->setFieldAmount(sfBalance, *amt); - dropsAdded += *amt; - } - else // give them 2 XRP if the account didn't exist, same as - // ttIMPORT - { - XRPAmount const initialXrp = - Import::computeStartingBonus(ctx_.view()); - sle->setFieldAmount(sfBalance, initialXrp); - dropsAdded += initialXrp; - } - } - else if (amt) - { - // Credit the account - STAmount startBal = sle->getFieldAmount(sfBalance); - STAmount finalBal = startBal + *amt; - if (finalBal <= startBal) - { - JLOG(ctx_.journal.warn()) - << "GenesisMint: cannot credit " << dest - << " due to balance overflow"; - return tecINTERNAL; - } - - sle->setFieldAmount(sfBalance, finalBal); - dropsAdded += *amt; - } - - // set flags and marks as applicable - if (flags) - sle->setFieldH256(sfGovernanceFlags, *flags); - - if (marks) - sle->setFieldH256(sfGovernanceMarks, *marks); - - if (created) - view().insert(sle); - else - view().update(sle); - } - - // update ledger header - if (dropsAdded < beast::zero || - dropsAdded.xrp() + view().info().drops < view().info().drops) - { - JLOG(ctx_.journal.warn()) << "GenesisMint: dropsAdded overflowed\n"; - return tecINTERNAL; - } - - if (dropsAdded > beast::zero) - ctx_.rawView().rawDestroyXRP(-dropsAdded.xrp()); - - return tesSUCCESS; - } - // RH NOTE: - // As of fixXahauV1, duplicate accounts are allowed + // duplicate accounts are allowed // so we first do a summation loop then an actioning loop // if amendment is not active then there's exactly one entry per diff --git a/src/xrpld/app/tx/detail/Import.cpp b/src/xrpld/app/tx/detail/Import.cpp index 5bfac2dafd..c7a95024af 100644 --- a/src/xrpld/app/tx/detail/Import.cpp +++ b/src/xrpld/app/tx/detail/Import.cpp @@ -821,17 +821,9 @@ Import::preflight(PreflightContext const& ctx) << " validation count: " << validationCount; // check if the validation count is adequate - auto hasInsufficientQuorum = - [&ctx](uint64_t quorum, uint64_t validationCount) { - if (ctx.rules.enabled(fixXahauV1)) - { - return quorum > validationCount; - } - else - { - return quorum >= validationCount; - } - }; + auto hasInsufficientQuorum = [](uint64_t quorum, uint64_t validationCount) { + return quorum > validationCount; + }; if (hasInsufficientQuorum(quorum, validationCount)) { JLOG(ctx.j.warn()) << "Import: xpop did not contain an 80% quorum for " diff --git a/src/xrpld/app/tx/detail/Invoke.cpp b/src/xrpld/app/tx/detail/Invoke.cpp index baae9a5549..a3e1306f57 100644 --- a/src/xrpld/app/tx/detail/Invoke.cpp +++ b/src/xrpld/app/tx/detail/Invoke.cpp @@ -92,27 +92,6 @@ Invoke::calculateBaseFee(ReadView const& view, STTx const& tx) extraFee += XRPAmount{static_cast(tx.getFieldVL(sfBlob).size())}; - // old code (prior to fixXahauV1) - if (!view.rules().enabled(fixXahauV1)) - { - if (tx.isFieldPresent(sfHookParameters)) - { - uint64_t paramBytes = 0; - auto const& params = tx.getFieldArray(sfHookParameters); - for (auto const& param : params) - { - paramBytes += - (param.isFieldPresent(sfHookParameterName) - ? param.getFieldVL(sfHookParameterName).size() - : 0) + - (param.isFieldPresent(sfHookParameterValue) - ? param.getFieldVL(sfHookParameterValue).size() - : 0); - } - extraFee += XRPAmount{static_cast(paramBytes)}; - } - } - return Transactor::calculateBaseFee(view, tx) + extraFee; } diff --git a/src/xrpld/app/tx/detail/SetHook.cpp b/src/xrpld/app/tx/detail/SetHook.cpp index 53b3b6b7e1..967e17e7ca 100644 --- a/src/xrpld/app/tx/detail/SetHook.cpp +++ b/src/xrpld/app/tx/detail/SetHook.cpp @@ -2173,10 +2173,7 @@ SetHook::setHook() else if (oldHookSLE && !newHooksEmpty) { // UPDATE ltHOOK - if (view().rules().enabled(fixXahauV1)) - { - (*newHookSLE)[sfOwnerNode] = (*oldHookSLE)[sfOwnerNode]; - } + (*newHookSLE)[sfOwnerNode] = (*oldHookSLE)[sfOwnerNode]; view().erase(oldHookSLE); view().insert(newHookSLE); } diff --git a/src/xrpld/app/tx/detail/Transactor.cpp b/src/xrpld/app/tx/detail/Transactor.cpp index 7b7c2402c5..6b2e63c59f 100644 --- a/src/xrpld/app/tx/detail/Transactor.cpp +++ b/src/xrpld/app/tx/detail/Transactor.cpp @@ -408,7 +408,7 @@ Transactor::calculateBaseFee(ReadView const& view, STTx const& tx) XRPAmount accumulator = baseFee; if (view.rules().enabled(featureHooks) && - view.rules().enabled(fixXahauV1) && tx.isFieldPresent(sfHookParameters)) + tx.isFieldPresent(sfHookParameters)) { uint64_t paramBytes = 0; auto const& params = tx.getFieldArray(sfHookParameters); @@ -712,8 +712,7 @@ Transactor::checkPriorTxAndLastLedger(PreclaimContext const& ctx) if (ctx.view.txExists(ctx.tx.getTransactionID())) return tefALREADY; - if (hook::isEmittedTxn(ctx.tx) && ctx.view.rules().enabled(featureHooks) && - ctx.view.rules().enabled(fixXahauV2)) + if (hook::isEmittedTxn(ctx.tx) && ctx.view.rules().enabled(featureHooks)) { // check if the emitted txn exists on ledger and is in the emission // directory if not that's a re-apply so discard diff --git a/src/xrpld/app/tx/detail/URIToken.cpp b/src/xrpld/app/tx/detail/URIToken.cpp index 01cbbc635e..f5ed4ab243 100644 --- a/src/xrpld/app/tx/detail/URIToken.cpp +++ b/src/xrpld/app/tx/detail/URIToken.cpp @@ -78,16 +78,11 @@ URIToken::preflight(PreflightContext const& ctx) } } - // fix amendment to return temMALFORMED if sfDestination field is present + // return temMALFORMED if sfDestination field is present // and sfAmount field is not present - if (ctx.rules.enabled(fixXahauV1)) - { - if (ctx.tx.isFieldPresent(sfDestination) && - !ctx.tx.isFieldPresent(sfAmount)) - { - return temMALFORMED; - } - } + if (ctx.tx.isFieldPresent(sfDestination) && + !ctx.tx.isFieldPresent(sfAmount)) + return temMALFORMED; // the validation for the URI field is also the same regardless of the txn // type @@ -144,8 +139,6 @@ URIToken::preflight(PreflightContext const& ctx) TER URIToken::preclaim(PreclaimContext const& ctx) { - bool const fixV1 = ctx.view.rules().enabled(fixXahauV1); - std::shared_ptr sleU; uint32_t leFlags; std::optional issuer; @@ -235,74 +228,42 @@ URIToken::preclaim(PreclaimContext const& ctx) if (purchaseAmount < saleAmount) return tecINSUFFICIENT_PAYMENT; - if (fixV1) + if (purchaseAmount.native() && saleAmount->native()) { - if (purchaseAmount.native() && saleAmount->native()) - { - // native transfer + // native transfer - STAmount needed{ctx.view.fees().accountReserve( - sle->getFieldU32(sfOwnerCount) + 1)}; + STAmount needed{ctx.view.fees().accountReserve( + sle->getFieldU32(sfOwnerCount) + 1)}; - STAmount const fee = ctx.tx.getFieldAmount(sfFee).xrp(); + STAmount const fee = ctx.tx.getFieldAmount(sfFee).xrp(); - if (needed + fee < needed) - return tecINTERNAL; - - needed += fee; - - if (needed + purchaseAmount < needed) - return tecINTERNAL; - - needed += purchaseAmount; - - if (needed > sle->getFieldAmount(sfBalance)) - return tecINSUFFICIENT_FUNDS; - } - else if (purchaseAmount.native() || saleAmount->native()) - { - // should not be able to happen + if (needed + fee < needed) return tecINTERNAL; - } - else - { - // iou transfer - STAmount availableFunds{accountFunds( - ctx.view, - acc, - purchaseAmount, - fhZERO_IF_FROZEN, - ctx.j)}; + needed += fee; - if (purchaseAmount > availableFunds) - return tecINSUFFICIENT_FUNDS; - } + if (needed + purchaseAmount < needed) + return tecINTERNAL; + + needed += purchaseAmount; + + if (needed > sle->getFieldAmount(sfBalance)) + return tecINSUFFICIENT_FUNDS; + } + else if (purchaseAmount.native() || saleAmount->native()) + { + // should not be able to happen + return tecINTERNAL; } else { - // old logic + // iou transfer - if (purchaseAmount.native() && saleAmount->native()) - { - // if it's an xrp sale/purchase then no trustline needed - if (purchaseAmount > - (sleOwner->getFieldAmount(sfBalance) - ctx.tx[sfFee])) - return tecINSUFFICIENT_FUNDS; - } - else - { - // iou - STAmount availableFunds{accountFunds( - ctx.view, - acc, - purchaseAmount, - fhZERO_IF_FROZEN, - ctx.j)}; + STAmount availableFunds{accountFunds( + ctx.view, acc, purchaseAmount, fhZERO_IF_FROZEN, ctx.j)}; - if (purchaseAmount > availableFunds) - return tecINSUFFICIENT_FUNDS; - } + if (purchaseAmount > availableFunds) + return tecINSUFFICIENT_FUNDS; } return tesSUCCESS; } @@ -343,8 +304,6 @@ URIToken::doApply() Sandbox sb(&ctx_.view()); - bool const fixV1 = sb.rules().enabled(fixXahauV1); - auto const sle = sb.peek(keylet::account(account_)); if (!sle) return tefINTERNAL; @@ -483,504 +442,134 @@ URIToken::doApply() if (purchaseAmount.issue() != saleAmount->issue()) return temBAD_CURRENCY; - if (fixV1) + if (purchaseAmount < saleAmount) + return tecINSUFFICIENT_PAYMENT; + + // if it's an xrp sale/purchase then no trustline needed + if (purchaseAmount.native()) { - // this is the reworked version of the buy routine + STAmount needed{sb.fees().accountReserve( + sle->getFieldU32(sfOwnerCount) + 1)}; - if (purchaseAmount < saleAmount) - return tecINSUFFICIENT_PAYMENT; + STAmount const fee = ctx_.tx.getFieldAmount(sfFee).xrp(); - // if it's an xrp sale/purchase then no trustline needed - if (purchaseAmount.native()) - { - STAmount needed{sb.fees().accountReserve( - sle->getFieldU32(sfOwnerCount) + 1)}; + if (needed + fee < needed) + return tecINTERNAL; - STAmount const fee = ctx_.tx.getFieldAmount(sfFee).xrp(); + needed += fee; - if (needed + fee < needed) - return tecINTERNAL; + if (needed + purchaseAmount < needed) + return tecINTERNAL; - needed += fee; + needed += purchaseAmount; - if (needed + purchaseAmount < needed) - return tecINTERNAL; - - needed += purchaseAmount; - - if (needed > mPriorBalance) - return tecINSUFFICIENT_FUNDS; - } - else - { - // IOU sale - if (TER result = trustTransferAllowed( - sb, {account_, *owner}, purchaseAmount.issue(), j); - !isTesSuccess(result)) - { - JLOG(j.trace()) - << "URIToken::doApply trustTransferAllowed result=" - << result; - - return result; - } - - if (STAmount availableFunds{accountFunds( - sb, account_, purchaseAmount, fhZERO_IF_FROZEN, j)}; - purchaseAmount > availableFunds) - return tecINSUFFICIENT_FUNDS; - } - - // execute the funds transfer, we'll check reserves last - if (TER result = accountSend( - sb, - account_, - *owner, - purchaseAmount, - j, - WaiveTransferFee::No, - false); + if (needed > mPriorBalance) + return tecINSUFFICIENT_FUNDS; + } + else + { + // IOU sale + if (TER result = trustTransferAllowed( + sb, {account_, *owner}, purchaseAmount.issue(), j); !isTesSuccess(result)) + { + JLOG(j.trace()) + << "URIToken::doApply trustTransferAllowed result=" + << result; + return result; - - // add token to new owner dir - auto const newPage = sb.dirInsert( - keylet::ownerDir(account_), - *kl, - describeOwnerDir(account_)); - - JLOG(j_.trace()) << "Adding URIToken to owner directory " - << to_string(kl->key) << ": " - << (newPage ? "success" : "failure"); - - if (!newPage) - return tecDIR_FULL; - - // remove from current owner directory - if (!sb.dirRemove( - keylet::ownerDir(*owner), - sleU->getFieldU64(sfOwnerNode), - kl->key, - true)) - { - JLOG(j.fatal()) - << "Could not remove URIToken from owner directory"; - - return tefBAD_LEDGER; } - // adjust owner counts - adjustOwnerCount(sb, sleOwner, -1, j); - adjustOwnerCount(sb, sle, 1, j); - - // clean the offer off the object - sleU->makeFieldAbsent(sfAmount); - if (sleU->isFieldPresent(sfDestination)) - sleU->makeFieldAbsent(sfDestination); - - // set the new owner of the object - sleU->setAccountID(sfOwner, account_); - - // tell the ledger where to find it - sleU->setFieldU64(sfOwnerNode, *newPage); - - // check each side has sufficient balance remaining to cover the - // updated ownercounts - auto hasSufficientReserve = - [&](std::shared_ptr const& sle) -> bool { - std::uint32_t const uOwnerCount = - sle->getFieldU32(sfOwnerCount); - return sle->getFieldAmount(sfBalance) >= - sb.fees().accountReserve(uOwnerCount); - }; - - if (!hasSufficientReserve(sle)) - { - JLOG(j.trace()) << "URIToken: buyer " << account_ - << " has insufficient reserve to buy"; - return tecINSUFFICIENT_RESERVE; - } - - // This should only happen if the owner burned their reserves - // below the needed amount via another transactor. If this - // happens they should top up their account before selling! - if (!hasSufficientReserve(sleOwner)) - { - JLOG(j.warn()) - << "URIToken: seller " << *owner - << " has insufficient reserve to allow purchase!"; - return tecINSUF_RESERVE_SELLER; - } - - sb.update(sle); - sb.update(sleU); - sb.update(sleOwner); - sb.apply(ctx_.rawView()); - return tesSUCCESS; - } - - // old logic - { - STAmount const purchaseAmount = - ctx_.tx.getFieldAmount(sfAmount); - - bool const sellerLow = purchaseAmount.getIssuer() > *owner; - bool const buyerLow = purchaseAmount.getIssuer() > account_; - bool sellerIssuer = purchaseAmount.getIssuer() == *owner; - bool buyerIssuer = purchaseAmount.getIssuer() == account_; - - // check if the seller has listed it at all - if (!saleAmount) - return tecNO_PERMISSION; - - // check if the seller has listed it for sale to a specific - // account - if (dest && *dest != account_) - return tecNO_PERMISSION; - - if (purchaseAmount.issue() != saleAmount->issue()) - return temBAD_CURRENCY; - - std::optional initBuyerBal; - std::optional initSellerBal; - std::optional finBuyerBal; - std::optional finSellerBal; - std::optional dstAmt; - std::optional tlSeller; - std::shared_ptr sleDstLine; - std::shared_ptr sleSrcLine; - - // if it's an xrp sale/purchase then no trustline needed - if (purchaseAmount.native()) - { - if (purchaseAmount < saleAmount) - return tecINSUFFICIENT_PAYMENT; - - if (purchaseAmount > - ((*sleOwner)[sfBalance] - ctx_.tx[sfFee])) - return tecINSUFFICIENT_FUNDS; - - dstAmt = purchaseAmount; - - initSellerBal = (*sleOwner)[sfBalance]; - initBuyerBal = (*sle)[sfBalance]; - - finSellerBal = *initSellerBal + purchaseAmount; - finBuyerBal = *initBuyerBal - purchaseAmount; - } - else - { - // IOU sale - STAmount availableFunds{accountFunds( + if (STAmount availableFunds{accountFunds( sb, account_, purchaseAmount, fhZERO_IF_FROZEN, j)}; - - // check for any possible bars to a buy transaction - // between these accounts for this asset - - if (buyerIssuer) - { - // pass: issuer does not create own trustline - } - else - { - TER result = trustTransferAllowed( - sb, {account_, *owner}, purchaseAmount.issue(), j); - JLOG(j.trace()) - << "URIToken::doApply trustTransferAllowed result=" - << result; - - if (!isTesSuccess(result)) - return result; - } - - if (purchaseAmount > availableFunds) - return tecINSUFFICIENT_FUNDS; - - // check if the seller has a line - tlSeller = keylet::line( - *owner, - purchaseAmount.getIssuer(), - purchaseAmount.getCurrency()); - Keylet tlBuyer = keylet::line( - account_, - purchaseAmount.getIssuer(), - purchaseAmount.getCurrency()); - - sleDstLine = sb.peek(*tlSeller); - sleSrcLine = sb.peek(tlBuyer); - - if (sellerIssuer) - { - // pass: issuer does not create own trustline - } - else if (!sleDstLine) - { - // they do not, so we can create one if they have - // sufficient reserve - - if (std::uint32_t const ownerCount = {sleOwner->at( - sfOwnerCount)}; - (*sleOwner)[sfBalance] < - sb.fees().accountReserve(ownerCount + 1)) - { - JLOG(j_.trace()) - << "Trust line does not exist. " - "Insufficent reserve to create line."; - - return tecNO_LINE_INSUF_RESERVE; - } - } - if (buyerIssuer) - { - // pass: issuer does not adjust own trustline - initBuyerBal = purchaseAmount.zeroed(); - finBuyerBal = purchaseAmount.zeroed(); - } - else - { - // remove from buyer - initBuyerBal = buyerLow ? ((*sleSrcLine)[sfBalance]) - : -((*sleSrcLine)[sfBalance]); - finBuyerBal = *initBuyerBal - purchaseAmount; - } - - dstAmt = purchaseAmount; - static Rate const parityRate(QUALITY_ONE); - auto xferRate = transferRate(sb, saleAmount->getIssuer()); - if (!sellerIssuer && !buyerIssuer && xferRate != parityRate) - { - dstAmt = multiplyRound( - purchaseAmount, - xferRate, - purchaseAmount.issue(), - true); - } - - initSellerBal = !sleDstLine ? purchaseAmount.zeroed() - : sellerLow ? ((*sleDstLine)[sfBalance]) - : -((*sleDstLine)[sfBalance]); - - finSellerBal = *initSellerBal + *dstAmt; - } - - // sanity check balance mutations (xrp or iou, both are checked - // the same way now) - if (*finSellerBal < *initSellerBal) - { - JLOG(j.warn()) - << "URIToken txid=" << ctx_.tx.getTransactionID() << " " - << "finSellerBal < initSellerBal"; - return tecINTERNAL; - } - - if (*finBuyerBal > *initBuyerBal) - { - JLOG(j.warn()) - << "URIToken txid=" << ctx_.tx.getTransactionID() << " " - << "finBuyerBal > initBuyerBal"; - return tecINTERNAL; - } - - if (*finBuyerBal < beast::zero) - { - JLOG(j.warn()) - << "URIToken txid=" << ctx_.tx.getTransactionID() << " " - << "finBuyerBal < 0"; - return tecINTERNAL; - } - - if (*finSellerBal < beast::zero) - { - JLOG(j.warn()) - << "URIToken txid=" << ctx_.tx.getTransactionID() << " " - << "finSellerBal < 0"; - return tecINTERNAL; - } - - // to this point no ledger changes have been made - // make them in a sensible order such that failure doesn't - // require cleanup - - // add to new owner's directory first, this can fail if they - // have too many objects - auto const newPage = sb.dirInsert( - keylet::ownerDir(account_), - *kl, - describeOwnerDir(account_)); - - JLOG(j_.trace()) << "Adding URIToken to owner directory " - << to_string(kl->key) << ": " - << (newPage ? "success" : "failure"); - - if (!newPage) - { - // nothing has happened at all and there is nothing to clean - // up we can just leave with DIR_FULL - return tecDIR_FULL; - } - - // Next create destination trustline where applicable. This - // could fail for a variety of reasons. If it does fail we need - // to remove the dir entry we just added to the buyer before we - // leave. - bool lineCreated = false; - if (!isXRP(purchaseAmount) && !sleDstLine && !sellerIssuer) - { - // clang-format off - if (TER const ter = trustCreate( - sb, // payment sandbox - sellerLow, // is dest low? - purchaseAmount.getIssuer(), // source - *owner, // destination - tlSeller->key, // ledger index - sleOwner, // Account to add to - false, // authorize account - (sleOwner->getFlags() & lsfDefaultRipple) == 0, - false, // freeze trust line - false, // deepfreeze trust line - *dstAmt, // initial balance zero - Issue( - purchaseAmount.getCurrency(), - *owner), // limit of zero - 0, // quality in - 0, // quality out - j); // journal - !isTesSuccess(ter)) - { - // remove the newly inserted directory entry before we leave - // - if (!sb.dirRemove(keylet::ownerDir(account_), *newPage, kl->key, true)) - { - JLOG(j.fatal()) - << "Could not remove URIToken from owner directory"; - - return tefBAD_LEDGER; - } - - // leave - return ter; - } - // clang-format on - - // add their trustline to their ownercount - lineCreated = true; - } - - // execution to here means we added the URIToken to the buyer's - // directory and we definitely have a way to send the funds to - // the seller. - - // remove from current owner directory - if (!sb.dirRemove( - keylet::ownerDir(*owner), - sleU->getFieldU64(sfOwnerNode), - kl->key, - true)) - { - JLOG(j.fatal()) - << "Could not remove URIToken from owner directory"; - - // remove the newly inserted directory entry before we leave - if (!sb.dirRemove( - keylet::ownerDir(account_), - *newPage, - kl->key, - true)) - { - JLOG(j.fatal()) << "Could not remove URIToken from " - "owner directory (2)"; - } - - // clean up any trustline we might have made - if (lineCreated) - { - auto line = sb.peek(*tlSeller); - if (line) - sb.erase(line); - } - - return tefBAD_LEDGER; - } - - // above is all the things that could fail. we now have swapped - // the ownership as far as the ownerdirs are concerned, and we - // have a place to pay to and from. - - // if a trustline was created then the ownercount stays the same - // on the seller +1 TL -1 URIToken - if (!lineCreated && !isXRP(purchaseAmount)) - adjustOwnerCount(sb, sleOwner, -1, j); - - // the buyer gets a new object - adjustOwnerCount(sb, sle, 1, j); - - // clean the offer off the object - sleU->makeFieldAbsent(sfAmount); - if (sleU->isFieldPresent(sfDestination)) - sleU->makeFieldAbsent(sfDestination); - - // set the new owner of the object - sleU->setAccountID(sfOwner, account_); - - // tell the ledger where to find it - sleU->setFieldU64(sfOwnerNode, *newPage); - - // update the buyer's balance - if (isXRP(purchaseAmount)) - { - // the sale is for xrp, so set the balance - sle->setFieldAmount(sfBalance, *finBuyerBal); - } - else if (sleSrcLine) - { - // update the buyer's line to reflect the reduction of the - // purchase price - sleSrcLine->setFieldAmount( - sfBalance, buyerLow ? *finBuyerBal : -(*finBuyerBal)); - } - else if (buyerIssuer) - { - // pass: buyer is issuer, no update required. - } - else - return tecINTERNAL; - - // update the seller's balance - if (isXRP(purchaseAmount)) - { - // the sale is for xrp, so set the balance - sleOwner->setFieldAmount(sfBalance, *finSellerBal); - } - else if (sleDstLine) - { - // the line already existed on the seller side so update it - sleDstLine->setFieldAmount( - sfBalance, - sellerLow ? *finSellerBal : -(*finSellerBal)); - } - else if (lineCreated) - { - // pass, the TL already has this balance set on it at - // creation - } - else if (sellerIssuer) - { - // pass: seller is issuer, no update required. - } - else - return tecINTERNAL; - - if (sleSrcLine) - sb.update(sleSrcLine); - if (sleDstLine) - sb.update(sleDstLine); - - sb.update(sle); - sb.update(sleU); - sb.update(sleOwner); - sb.apply(ctx_.rawView()); - return tesSUCCESS; + purchaseAmount > availableFunds) + return tecINSUFFICIENT_FUNDS; } + + // execute the funds transfer, we'll check reserves last + if (TER result = accountSend( + sb, + account_, + *owner, + purchaseAmount, + j, + WaiveTransferFee::No, + false); + !isTesSuccess(result)) + return result; + + // add token to new owner dir + auto const newPage = sb.dirInsert( + keylet::ownerDir(account_), *kl, describeOwnerDir(account_)); + + JLOG(j_.trace()) + << "Adding URIToken to owner directory " << to_string(kl->key) + << ": " << (newPage ? "success" : "failure"); + + if (!newPage) + return tecDIR_FULL; + + // remove from current owner directory + if (!sb.dirRemove( + keylet::ownerDir(*owner), + sleU->getFieldU64(sfOwnerNode), + kl->key, + true)) + { + JLOG(j.fatal()) + << "Could not remove URIToken from owner directory"; + + return tefBAD_LEDGER; + } + + // adjust owner counts + adjustOwnerCount(sb, sleOwner, -1, j); + adjustOwnerCount(sb, sle, 1, j); + + // clean the offer off the object + sleU->makeFieldAbsent(sfAmount); + if (sleU->isFieldPresent(sfDestination)) + sleU->makeFieldAbsent(sfDestination); + + // set the new owner of the object + sleU->setAccountID(sfOwner, account_); + + // tell the ledger where to find it + sleU->setFieldU64(sfOwnerNode, *newPage); + + // check each side has sufficient balance remaining to cover the + // updated ownercounts + auto hasSufficientReserve = + [&](std::shared_ptr const& sle) -> bool { + std::uint32_t const uOwnerCount = + sle->getFieldU32(sfOwnerCount); + return sle->getFieldAmount(sfBalance) >= + sb.fees().accountReserve(uOwnerCount); + }; + + if (!hasSufficientReserve(sle)) + { + JLOG(j.trace()) << "URIToken: buyer " << account_ + << " has insufficient reserve to buy"; + return tecINSUFFICIENT_RESERVE; + } + + // This should only happen if the owner burned their reserves + // below the needed amount via another transactor. If this + // happens they should top up their account before selling! + if (!hasSufficientReserve(sleOwner)) + { + JLOG(j.warn()) + << "URIToken: seller " << *owner + << " has insufficient reserve to allow purchase!"; + return tecINSUF_RESERVE_SELLER; + } + + sb.update(sle); + sb.update(sleU); + sb.update(sleOwner); + sb.apply(ctx_.rawView()); + return tesSUCCESS; } case ttURITOKEN_BURN: { @@ -1010,10 +599,8 @@ URIToken::doApply() sb.erase(sleU); - auto& sleAcc = fixV1 ? sleOwner : sle; - - adjustOwnerCount(sb, sleAcc, -1, j); - sb.update(sleAcc); + adjustOwnerCount(sb, sleOwner, -1, j); + sb.update(sleOwner); sb.apply(ctx_.rawView()); return tesSUCCESS; } diff --git a/src/xrpld/ledger/detail/ApplyStateTable.cpp b/src/xrpld/ledger/detail/ApplyStateTable.cpp index ff308f59b4..eb785cca36 100644 --- a/src/xrpld/ledger/detail/ApplyStateTable.cpp +++ b/src/xrpld/ledger/detail/ApplyStateTable.cpp @@ -131,8 +131,6 @@ ApplyStateTable::generateTxMeta( if (!hookEmission.empty()) meta.setHookEmissions(STArray{hookEmission, sfHookEmissions}); - bool const recordDefaultAmounts = to.rules().enabled(fixXahauV1); - Mods newMod; for (auto& item : items_) { @@ -245,8 +243,7 @@ ApplyStateTable::generateTxMeta( for (auto const& obj : *curNode) { bool const shouldRecord = - (obj.getSType() == STI_AMOUNT && recordDefaultAmounts) || - !obj.isDefault(); + (obj.getSType() == STI_AMOUNT) || !obj.isDefault(); // save non-default values if (shouldRecord && From c208a40ce82f12b2bad5260b374d39a95695e06c Mon Sep 17 00:00:00 2001 From: tequ Date: Wed, 9 Sep 2026 14:31:11 +0900 Subject: [PATCH 06/11] remove WASM: $COUNTER from SetHook_wasm.h (#804) --- src/test/app/SetHook_wasm.h | 101 ------------------------------- src/test/app/build_test_hooks.sh | 1 - 2 files changed, 102 deletions(-) diff --git a/src/test/app/SetHook_wasm.h b/src/test/app/SetHook_wasm.h index 5a8d43cf34..aecbc58a39 100644 --- a/src/test/app/SetHook_wasm.h +++ b/src/test/app/SetHook_wasm.h @@ -9,7 +9,6 @@ namespace ripple { namespace test { std::map> wasm = { - /* ==== WASM: 0 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -190,7 +189,6 @@ std::map> wasm = { 0x3DU, 0x20U, 0x54U, 0x4FU, 0x4FU, 0x5FU, 0x42U, 0x49U, 0x47U, 0x00U, }}, - /* ==== WASM: 1 ==== */ {R"[test.hook]( (module (type (;0;) (func (param i32 i32) (result i64))) @@ -240,7 +238,6 @@ std::map> wasm = { 0x0BU, }}, - /* ==== WASM: 2 ==== */ {R"[test.hook]( (module (type (;0;) (func (param i32 i32) (result i64))) @@ -294,7 +291,6 @@ std::map> wasm = { 0x14U, 0x10U, 0x00U, 0x1AU, 0x0CU, 0x00U, 0x0BU, 0x0BU, }}, - /* ==== WASM: 3 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -361,7 +357,6 @@ std::map> wasm = { 0x80U, 0x80U, 0x00U, 0x20U, 0x02U, 0x0BU, }}, - /* ==== WASM: 4 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -467,7 +462,6 @@ std::map> wasm = { 0x6AU, 0x24U, 0x80U, 0x80U, 0x80U, 0x80U, 0x00U, 0x20U, 0x03U, 0x0BU, }}, - /* ==== WASM: 5 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -542,7 +536,6 @@ std::map> wasm = { 0x6AU, 0x24U, 0x80U, 0x80U, 0x80U, 0x80U, 0x00U, 0x20U, 0x05U, 0x0BU, }}, - /* ==== WASM: 6 ==== */ {R"[test.hook]( #include extern int32_t _g(uint32_t, uint32_t); @@ -1159,7 +1152,6 @@ std::map> wasm = { 0x78U, 0x29U, 0x29U, 0x20U, 0x3DU, 0x3DU, 0x20U, 0x33U, 0x32U, 0x00U, }}, - /* ==== WASM: 7 ==== */ {R"[test.hook]( #include extern int32_t _g(uint32_t, uint32_t); @@ -1588,7 +1580,6 @@ std::map> wasm = { 0x00U, }}, - /* ==== WASM: 8 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -1794,7 +1785,6 @@ std::map> wasm = { 0x3DU, 0x20U, 0x30U, 0x78U, 0x45U, 0x31U, 0x00U, }}, - /* ==== WASM: 9 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -1916,7 +1906,6 @@ std::map> wasm = { 0x58U, 0x4EU, 0x00U, }}, - /* ==== WASM: 10 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -2049,7 +2038,6 @@ std::map> wasm = { 0x4FU, 0x4EU, 0x43U, 0x45U, 0x53U, 0x00U, }}, - /* ==== WASM: 11 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -2129,7 +2117,6 @@ std::map> wasm = { 0x54U, 0x00U, }}, - /* ==== WASM: 12 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -2174,7 +2161,6 @@ std::map> wasm = { 0x30U, 0x00U, }}, - /* ==== WASM: 13 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -2493,7 +2479,6 @@ std::map> wasm = { 0x80U, 0x80U, 0x80U, 0x00U, 0x0BU, }}, - /* ==== WASM: 14 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -3279,7 +3264,6 @@ std::map> wasm = { 0x37U, 0x36U, 0x33U, 0x4CU, 0x4CU, 0x20U, 0x29U, 0x00U, }}, - /* ==== WASM: 15 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -3669,7 +3653,6 @@ std::map> wasm = { 0x80U, 0x80U, 0x80U, 0x00U, 0x0BU, }}, - /* ==== WASM: 16 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -3850,7 +3833,6 @@ std::map> wasm = { 0x38U, 0x4CU, 0x4CU, 0x29U, 0x00U, }}, - /* ==== WASM: 17 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -4039,7 +4021,6 @@ std::map> wasm = { 0x29U, 0x00U, }}, - /* ==== WASM: 18 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -4278,7 +4259,6 @@ std::map> wasm = { 0x00U, 0x42U, 0x00U, 0x10U, 0x85U, 0x80U, 0x80U, 0x80U, 0x00U, 0x0BU, }}, - /* ==== WASM: 19 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -4980,7 +4960,6 @@ std::map> wasm = { 0x38U, 0x35U, 0x35U, 0x32U, 0x55U, 0x29U, 0x00U, }}, - /* ==== WASM: 20 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -6325,7 +6304,6 @@ std::map> wasm = { 0x20U, 0x29U, 0x00U, }}, - /* ==== WASM: 21 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -6427,7 +6405,6 @@ std::map> wasm = { 0x84U, 0x80U, 0x80U, 0x80U, 0x00U, 0x0BU, }}, - /* ==== WASM: 22 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -6470,7 +6447,6 @@ std::map> wasm = { 0x00U, 0x0BU, }}, - /* ==== WASM: 23 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -6618,7 +6594,6 @@ std::map> wasm = { 0x34U, 0x34U, 0x4CU, 0x4CU, 0x2CU, 0x20U, 0x33U, 0x29U, 0x00U, }}, - /* ==== WASM: 24 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -6946,7 +6921,6 @@ std::map> wasm = { 0x38U, 0x34U, 0x39U, 0x30U, 0x4CU, 0x4CU, 0x00U, }}, - /* ==== WASM: 25 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -7151,7 +7125,6 @@ std::map> wasm = { 0x10U, 0x85U, 0x80U, 0x80U, 0x80U, 0x00U, 0x0BU, }}, - /* ==== WASM: 26 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -7835,7 +7808,6 @@ std::map> wasm = { 0x20U, 0x3DU, 0x3DU, 0x20U, 0x30U, 0x00U, }}, - /* ==== WASM: 27 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -8130,7 +8102,6 @@ std::map> wasm = { 0x32U, 0x34U, 0x31U, 0x36U, 0x55U, 0x4CU, 0x4CU, 0x00U, }}, - /* ==== WASM: 28 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -8843,7 +8814,6 @@ std::map> wasm = { 0x31U, 0x33U, 0x33U, 0x38U, 0x20U, 0x29U, 0x00U, }}, - /* ==== WASM: 29 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -8928,7 +8898,6 @@ std::map> wasm = { 0x30U, 0x29U, 0x20U, 0x3DU, 0x3DU, 0x20U, 0x32U, 0x30U, 0x00U, }}, - /* ==== WASM: 30 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -8990,7 +8959,6 @@ std::map> wasm = { 0x80U, 0x80U, 0x00U, 0x0BU, }}, - /* ==== WASM: 31 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -9067,7 +9035,6 @@ std::map> wasm = { 0x31U, 0x29U, 0x20U, 0x3DU, 0x3DU, 0x20U, 0x33U, 0x32U, 0x00U, }}, - /* ==== WASM: 32 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -9144,7 +9111,6 @@ std::map> wasm = { 0x31U, 0x29U, 0x20U, 0x3DU, 0x3DU, 0x20U, 0x33U, 0x32U, 0x00U, }}, - /* ==== WASM: 33 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -9401,7 +9367,6 @@ std::map> wasm = { 0x00U, 0x00U, }}, - /* ==== WASM: 34 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -9559,7 +9524,6 @@ std::map> wasm = { 0x04U, 0x00U, 0x00U, }}, - /* ==== WASM: 35 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -9912,7 +9876,6 @@ std::map> wasm = { 0x00U, 0x2AU, 0x04U, 0x00U, 0x00U, 0x31U, 0x04U, 0x00U, 0x00U, }}, - /* ==== WASM: 36 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -9945,7 +9908,6 @@ std::map> wasm = { 0x82U, 0x80U, 0x80U, 0x80U, 0x00U, 0x1AU, 0x20U, 0x01U, 0x0BU, }}, - /* ==== WASM: 37 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -10172,7 +10134,6 @@ std::map> wasm = { 0x00U, }}, - /* ==== WASM: 38 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -10203,7 +10164,6 @@ std::map> wasm = { 0x80U, 0x80U, 0x00U, 0x1AU, 0x20U, 0x01U, 0x0BU, }}, - /* ==== WASM: 39 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -10514,7 +10474,6 @@ std::map> wasm = { 0x00U, }}, - /* ==== WASM: 40 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -10591,7 +10550,6 @@ std::map> wasm = { 0x29U, 0x20U, 0x3DU, 0x3DU, 0x20U, 0x33U, 0x32U, 0x00U, }}, - /* ==== WASM: 41 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -10625,7 +10583,6 @@ std::map> wasm = { 0x80U, 0x80U, 0x00U, 0x1AU, 0x20U, 0x01U, 0x0BU, }}, - /* ==== WASM: 42 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -10712,7 +10669,6 @@ std::map> wasm = { 0x32U, 0x00U, }}, - /* ==== WASM: 43 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -10746,7 +10702,6 @@ std::map> wasm = { 0x0BU, }}, - /* ==== WASM: 44 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -10886,7 +10841,6 @@ std::map> wasm = { 0x54U, 0x5FU, 0x4DU, 0x45U, 0x54U, 0x00U, }}, - /* ==== WASM: 45 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -11076,7 +11030,6 @@ std::map> wasm = { 0x31U, 0x34U, 0x00U, }}, - /* ==== WASM: 46 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -11230,7 +11183,6 @@ std::map> wasm = { 0x00U, }}, - /* ==== WASM: 47 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -11391,7 +11343,6 @@ std::map> wasm = { 0x00U, }}, - /* ==== WASM: 48 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -11548,7 +11499,6 @@ std::map> wasm = { 0x53U, 0x4CU, 0x4FU, 0x54U, 0x53U, 0x00U, }}, - /* ==== WASM: 49 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -11633,7 +11583,6 @@ std::map> wasm = { 0x74U, 0x79U, 0x70U, 0x65U, 0x28U, 0x29U, 0x00U, }}, - /* ==== WASM: 50 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -11890,7 +11839,6 @@ std::map> wasm = { 0x00U, 0x00U, }}, - /* ==== WASM: 51 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -12128,7 +12076,6 @@ std::map> wasm = { 0x3EU, 0x20U, 0x30U, 0x00U, }}, - /* ==== WASM: 52 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -12224,7 +12171,6 @@ std::map> wasm = { 0x53U, 0x54U, 0x00U, }}, - /* ==== WASM: 53 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -12334,7 +12280,6 @@ std::map> wasm = { 0x20U, 0x31U, 0x00U, }}, - /* ==== WASM: 54 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -12464,7 +12409,6 @@ std::map> wasm = { 0x30U, 0x30U, 0x30U, 0x4CU, 0x4CU, 0x00U, }}, - /* ==== WASM: 55 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -12738,7 +12682,6 @@ std::map> wasm = { 0x30U, 0x00U, }}, - /* ==== WASM: 56 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -12875,7 +12818,6 @@ std::map> wasm = { 0x2CU, 0x20U, 0x73U, 0x29U, 0x20U, 0x3DU, 0x3DU, 0x20U, 0x31U, 0x00U, }}, - /* ==== WASM: 57 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -13169,7 +13111,6 @@ std::map> wasm = { 0x5FU, 0x53U, 0x4CU, 0x4FU, 0x54U, 0x53U, 0x00U, }}, - /* ==== WASM: 58 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -13403,7 +13344,6 @@ std::map> wasm = { 0x5FU, 0x53U, 0x4CU, 0x4FU, 0x54U, 0x53U, 0x00U, }}, - /* ==== WASM: 59 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -13699,7 +13639,6 @@ std::map> wasm = { 0x2CU, 0x20U, 0x31U, 0x29U, 0x20U, 0x3DU, 0x3DU, 0x20U, 0x30U, 0x00U, }}, - /* ==== WASM: 60 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -13903,7 +13842,6 @@ std::map> wasm = { 0x6EU, 0x74U, 0x32U, 0x22U, 0x20U, 0x2BU, 0x20U, 0x69U, 0x29U, 0x00U, }}, - /* ==== WASM: 61 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -14020,7 +13958,6 @@ std::map> wasm = { 0x69U, 0x29U, 0x00U, }}, - /* ==== WASM: 62 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -14129,7 +14066,6 @@ std::map> wasm = { 0x6EU, 0x74U, 0x65U, 0x6EU, 0x74U, 0x32U, 0x22U, 0x29U, 0x00U, }}, - /* ==== WASM: 63 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -14402,7 +14338,6 @@ std::map> wasm = { 0x00U, }}, - /* ==== WASM: 64 ==== */ {R"[test.hook]( #include #define sfInvoiceID ((5U << 16U) + 17U) @@ -14621,7 +14556,6 @@ std::map> wasm = { 0x30U, 0x29U, 0x20U, 0x3DU, 0x3DU, 0x20U, 0x33U, 0x32U, 0x00U, }}, - /* ==== WASM: 65 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -14733,7 +14667,6 @@ std::map> wasm = { 0x58U, 0x49U, 0x53U, 0x54U, 0x00U, }}, - /* ==== WASM: 66 ==== */ {R"[test.hook]( #include #define sfInvoiceID ((5U << 16U) + 17U) @@ -14869,7 +14802,6 @@ std::map> wasm = { 0x3DU, 0x20U, 0x33U, 0x32U, 0x00U, }}, - /* ==== WASM: 67 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -15005,7 +14937,6 @@ std::map> wasm = { 0x49U, 0x47U, 0x00U, }}, - /* ==== WASM: 68 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -15144,7 +15075,6 @@ std::map> wasm = { 0x66U, 0x28U, 0x64U, 0x61U, 0x74U, 0x61U, 0x32U, 0x29U, 0x00U, }}, - /* ==== WASM: 69 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -15256,7 +15186,6 @@ std::map> wasm = { 0x29U, 0x20U, 0x3DU, 0x3DU, 0x20U, 0x30U, 0x00U, }}, - /* ==== WASM: 70 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -15350,7 +15279,6 @@ std::map> wasm = { 0x61U, 0x64U, 0x5BU, 0x69U, 0x5DU, 0x00U, }}, - /* ==== WASM: 71 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -15483,7 +15411,6 @@ std::map> wasm = { 0x64U, 0x5BU, 0x69U, 0x5DU, 0x00U, }}, - /* ==== WASM: 72 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -15571,7 +15498,6 @@ std::map> wasm = { 0x61U, 0x74U, 0x61U, 0x29U, 0x00U, }}, - /* ==== WASM: 73 ==== */ {R"[test.hook]( #include #define sfInvoiceID ((5U << 16U) + 17U) @@ -15682,7 +15608,6 @@ std::map> wasm = { 0x20U, 0x33U, 0x32U, 0x00U, }}, - /* ==== WASM: 74 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -15893,7 +15818,6 @@ std::map> wasm = { 0x00U, }}, - /* ==== WASM: 75 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -16001,7 +15925,6 @@ std::map> wasm = { 0x20U, 0x22U, 0x32U, 0x22U, 0x2CU, 0x20U, 0x31U, 0x29U, 0x00U, }}, - /* ==== WASM: 76 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -16613,7 +16536,6 @@ std::map> wasm = { 0x00U, }}, - /* ==== WASM: 77 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -16800,7 +16722,6 @@ std::map> wasm = { 0x63U, 0x65U, 0x29U, 0x20U, 0x3EU, 0x20U, 0x30U, 0x00U, }}, - /* ==== WASM: 78 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -17150,7 +17071,6 @@ std::map> wasm = { 0x20U, 0x30U, 0x00U, }}, - /* ==== WASM: 79 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -17286,7 +17206,6 @@ std::map> wasm = { 0x00U, }}, - /* ==== WASM: 80 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -17345,7 +17264,6 @@ std::map> wasm = { 0x64U, 0xE1U, 0xF1U, }}, - /* ==== WASM: 81 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -17518,7 +17436,6 @@ std::map> wasm = { 0x54U, 0x5FU, 0x45U, 0x58U, 0x49U, 0x53U, 0x54U, 0x00U, }}, - /* ==== WASM: 82 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -17666,7 +17583,6 @@ std::map> wasm = { 0x30U, 0x00U, 0x22U, 0x00U, 0x00U, 0x00U, 0x00U, }}, - /* ==== WASM: 83 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -17820,7 +17736,6 @@ std::map> wasm = { 0x0FU, 0x0BU, 0x02U, 0x56U, 0x00U, }}, - /* ==== WASM: 84 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -17917,7 +17832,6 @@ std::map> wasm = { 0x3DU, 0x20U, 0x30U, 0x00U, }}, - /* ==== WASM: 85 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -17976,7 +17890,6 @@ std::map> wasm = { 0x4FU, 0x46U, 0x5FU, 0x42U, 0x4FU, 0x55U, 0x4EU, 0x44U, 0x53U, 0x00U, }}, - /* ==== WASM: 86 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -18035,7 +17948,6 @@ std::map> wasm = { 0x4EU, 0x44U, 0x53U, 0x00U, }}, - /* ==== WASM: 87 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -19864,7 +19776,6 @@ std::map> wasm = { 0x53U, 0x4DU, 0x41U, 0x4CU, 0x4CU, 0x00U, }}, - /* ==== WASM: 88 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -21483,7 +21394,6 @@ std::map> wasm = { 0x29U, 0x29U, 0x00U, }}, - /* ==== WASM: 89 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -24416,7 +24326,6 @@ std::map> wasm = { 0x4FU, 0x4FU, 0x5FU, 0x53U, 0x4DU, 0x41U, 0x4CU, 0x4CU, 0x00U, }}, - /* ==== WASM: 90 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -26381,7 +26290,6 @@ std::map> wasm = { 0x54U, 0x4FU, 0x4FU, 0x5FU, 0x53U, 0x4DU, 0x41U, 0x4CU, 0x4CU, 0x00U, }}, - /* ==== WASM: 91 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -26666,7 +26574,6 @@ std::map> wasm = { 0x29U, 0x20U, 0x3DU, 0x3DU, 0x20U, 0x30U, 0x00U, }}, - /* ==== WASM: 92 ==== */ {R"[test.hook]( #include extern int32_t _g(uint32_t, uint32_t); @@ -27253,7 +27160,6 @@ std::map> wasm = { 0x4EU, 0x5FU, 0x46U, 0x41U, 0x49U, 0x4CU, 0x55U, 0x52U, 0x45U, 0x00U, }}, - /* ==== WASM: 93 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -27282,7 +27188,6 @@ std::map> wasm = { 0x0BU, }}, - /* ==== WASM: 94 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -27314,7 +27219,6 @@ std::map> wasm = { 0x20U, 0x52U, 0x65U, 0x6AU, 0x65U, 0x63U, 0x74U, 0x65U, 0x64U, 0x00U, }}, - /* ==== WASM: 95 ==== */ {R"[test.hook]( (module (type (;0;) (func (param i32 i32 i64) (result i64))) @@ -27341,7 +27245,6 @@ std::map> wasm = { 0x41U, 0x00U, 0x41U, 0x00U, 0x42U, 0x00U, 0x10U, 0x00U, 0x0BU, }}, - /* ==== WASM: 96 ==== */ {R"[test.hook]( (module (type (;0;) (func (param i32 i32) (result i32))) @@ -27394,7 +27297,6 @@ std::map> wasm = { 0x00U, 0x1AU, 0x0BU, }}, - /* ==== WASM: 97 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -34037,7 +33939,6 @@ std::map> wasm = { 0x39U, 0x30U, 0x31U, 0x32U, 0x33U, 0x00U, }}, - /* ==== WASM: 98 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -34083,7 +33984,6 @@ std::map> wasm = { 0x0BU, 0x06U, 0x76U, 0x61U, 0x6CU, 0x75U, 0x65U, 0x00U, }}, - /* ==== WASM: 99 ==== */ {R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); @@ -34112,7 +34012,6 @@ std::map> wasm = { 0x0BU, }}, - /* ==== WASM: 100 ==== */ {R"[test.hook]( #include extern int64_t accept (uint32_t read_ptr, uint32_t read_len, int64_t error_code); diff --git a/src/test/app/build_test_hooks.sh b/src/test/app/build_test_hooks.sh index 4e57701a88..eb66dbd508 100755 --- a/src/test/app/build_test_hooks.sh +++ b/src/test/app/build_test_hooks.sh @@ -48,7 +48,6 @@ cat $INPUT_FILE | tr '\n' '\f' | sed -E 's/\)\[test\.hook\]"[\f \t]*/\/*end*\//g' | while read -r line do - echo "/* ==== WASM: $COUNTER ==== */" >> $OUTPUT_FILE echo -n '{ R"[test.hook](' >> $OUTPUT_FILE cat <<< "$line" | sed -E 's/.{7}$//g' | tr -d '\n' | tr '\f' '\n' >> $OUTPUT_FILE echo ')[test.hook]",' >> $OUTPUT_FILE From 902ed9b4920beee3b7dfc24d91d8f24b7d9adbcb Mon Sep 17 00:00:00 2001 From: tequ Date: Mon, 21 Sep 2026 20:12:15 +0900 Subject: [PATCH 07/11] Use normal consequences factory for standard transactions (#774) Replace redundant custom consequence factories that returned normal consequences with the built-in Normal factory. --- src/xrpld/app/tx/detail/ClaimReward.cpp | 6 ------ src/xrpld/app/tx/detail/ClaimReward.h | 5 +---- src/xrpld/app/tx/detail/Cron.cpp | 6 ------ src/xrpld/app/tx/detail/Cron.h | 5 +---- src/xrpld/app/tx/detail/CronSet.cpp | 6 ------ src/xrpld/app/tx/detail/CronSet.h | 5 +---- src/xrpld/app/tx/detail/Invoke.cpp | 6 ------ src/xrpld/app/tx/detail/Invoke.h | 5 +---- src/xrpld/app/tx/detail/SetRemarks.cpp | 6 ------ src/xrpld/app/tx/detail/SetRemarks.h | 5 +---- 10 files changed, 5 insertions(+), 50 deletions(-) diff --git a/src/xrpld/app/tx/detail/ClaimReward.cpp b/src/xrpld/app/tx/detail/ClaimReward.cpp index e298a40155..8651cced05 100644 --- a/src/xrpld/app/tx/detail/ClaimReward.cpp +++ b/src/xrpld/app/tx/detail/ClaimReward.cpp @@ -30,12 +30,6 @@ namespace ripple { -TxConsequences -ClaimReward::makeTxConsequences(PreflightContext const& ctx) -{ - return TxConsequences{ctx.tx, TxConsequences::normal}; -} - NotTEC ClaimReward::preflight(PreflightContext const& ctx) { diff --git a/src/xrpld/app/tx/detail/ClaimReward.h b/src/xrpld/app/tx/detail/ClaimReward.h index 9c46678019..d93bd5bb36 100644 --- a/src/xrpld/app/tx/detail/ClaimReward.h +++ b/src/xrpld/app/tx/detail/ClaimReward.h @@ -32,15 +32,12 @@ namespace ripple { class ClaimReward : public Transactor { public: - static constexpr ConsequencesFactoryType ConsequencesFactory{Custom}; + static constexpr ConsequencesFactoryType ConsequencesFactory{Normal}; explicit ClaimReward(ApplyContext& ctx) : Transactor(ctx) { } - static TxConsequences - makeTxConsequences(PreflightContext const& ctx); - static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/Cron.cpp b/src/xrpld/app/tx/detail/Cron.cpp index 60440d6af1..96a4bdc7f8 100644 --- a/src/xrpld/app/tx/detail/Cron.cpp +++ b/src/xrpld/app/tx/detail/Cron.cpp @@ -30,12 +30,6 @@ namespace ripple { -TxConsequences -Cron::makeTxConsequences(PreflightContext const& ctx) -{ - return TxConsequences{ctx.tx, TxConsequences::normal}; -} - NotTEC Cron::preflight(PreflightContext const& ctx) { diff --git a/src/xrpld/app/tx/detail/Cron.h b/src/xrpld/app/tx/detail/Cron.h index cab51b2917..dbf6c49ddb 100644 --- a/src/xrpld/app/tx/detail/Cron.h +++ b/src/xrpld/app/tx/detail/Cron.h @@ -30,7 +30,7 @@ namespace ripple { class Cron : public Transactor { public: - static constexpr ConsequencesFactoryType ConsequencesFactory{Custom}; + static constexpr ConsequencesFactoryType ConsequencesFactory{Normal}; explicit Cron(ApplyContext& ctx) : Transactor(ctx) { @@ -39,9 +39,6 @@ public: static XRPAmount calculateBaseFee(ReadView const& view, STTx const& tx); - static TxConsequences - makeTxConsequences(PreflightContext const& ctx); - static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/CronSet.cpp b/src/xrpld/app/tx/detail/CronSet.cpp index 9c7e057ed0..d4525c3eb1 100644 --- a/src/xrpld/app/tx/detail/CronSet.cpp +++ b/src/xrpld/app/tx/detail/CronSet.cpp @@ -27,12 +27,6 @@ namespace ripple { -TxConsequences -CronSet::makeTxConsequences(PreflightContext const& ctx) -{ - return TxConsequences{ctx.tx, TxConsequences::normal}; -} - NotTEC CronSet::preflight(PreflightContext const& ctx) { diff --git a/src/xrpld/app/tx/detail/CronSet.h b/src/xrpld/app/tx/detail/CronSet.h index 9952ab7b56..775a0a3768 100644 --- a/src/xrpld/app/tx/detail/CronSet.h +++ b/src/xrpld/app/tx/detail/CronSet.h @@ -29,7 +29,7 @@ namespace ripple { class CronSet : public Transactor { public: - static constexpr ConsequencesFactoryType ConsequencesFactory{Custom}; + static constexpr ConsequencesFactoryType ConsequencesFactory{Normal}; explicit CronSet(ApplyContext& ctx) : Transactor(ctx) { @@ -38,9 +38,6 @@ public: static XRPAmount calculateBaseFee(ReadView const& view, STTx const& tx); - static TxConsequences - makeTxConsequences(PreflightContext const& ctx); - static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/Invoke.cpp b/src/xrpld/app/tx/detail/Invoke.cpp index a3e1306f57..9302396bbf 100644 --- a/src/xrpld/app/tx/detail/Invoke.cpp +++ b/src/xrpld/app/tx/detail/Invoke.cpp @@ -26,12 +26,6 @@ namespace ripple { -TxConsequences -Invoke::makeTxConsequences(PreflightContext const& ctx) -{ - return TxConsequences{ctx.tx, TxConsequences::normal}; -} - NotTEC Invoke::preflight(PreflightContext const& ctx) { diff --git a/src/xrpld/app/tx/detail/Invoke.h b/src/xrpld/app/tx/detail/Invoke.h index 3daec09e7a..4e11213f52 100644 --- a/src/xrpld/app/tx/detail/Invoke.h +++ b/src/xrpld/app/tx/detail/Invoke.h @@ -30,7 +30,7 @@ namespace ripple { class Invoke : public Transactor { public: - static constexpr ConsequencesFactoryType ConsequencesFactory{Custom}; + static constexpr ConsequencesFactoryType ConsequencesFactory{Normal}; explicit Invoke(ApplyContext& ctx) : Transactor(ctx) { @@ -39,9 +39,6 @@ public: static XRPAmount calculateBaseFee(ReadView const& view, STTx const& tx); - static TxConsequences - makeTxConsequences(PreflightContext const& ctx); - static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/SetRemarks.cpp b/src/xrpld/app/tx/detail/SetRemarks.cpp index 9111f5f590..88cbfe1bed 100644 --- a/src/xrpld/app/tx/detail/SetRemarks.cpp +++ b/src/xrpld/app/tx/detail/SetRemarks.cpp @@ -30,12 +30,6 @@ namespace ripple { -TxConsequences -SetRemarks::makeTxConsequences(PreflightContext const& ctx) -{ - return TxConsequences{ctx.tx, TxConsequences::normal}; -} - NotTEC SetRemarks::validateRemarks(STArray const& remarks, beast::Journal const& j) { diff --git a/src/xrpld/app/tx/detail/SetRemarks.h b/src/xrpld/app/tx/detail/SetRemarks.h index 21d2a01c94..412004aebe 100644 --- a/src/xrpld/app/tx/detail/SetRemarks.h +++ b/src/xrpld/app/tx/detail/SetRemarks.h @@ -30,7 +30,7 @@ namespace ripple { class SetRemarks : public Transactor { public: - static constexpr ConsequencesFactoryType ConsequencesFactory{Custom}; + static constexpr ConsequencesFactoryType ConsequencesFactory{Normal}; explicit SetRemarks(ApplyContext& ctx) : Transactor(ctx) { @@ -39,9 +39,6 @@ public: static XRPAmount calculateBaseFee(ReadView const& view, STTx const& tx); - static TxConsequences - makeTxConsequences(PreflightContext const& ctx); - static NotTEC preflight(PreflightContext const& ctx); From 2e32b5c6bbd35ae1f6f6ffd1cc865115b87b732f Mon Sep 17 00:00:00 2001 From: Niq Dudfield Date: Wed, 23 Sep 2026 12:33:43 +0700 Subject: [PATCH 08/11] feat(server_definitions): distinguish config-forced from ledger-enabled amendments (#765) --- include/xrpl/protocol/jss.h | 4 +- src/test/rpc/ServerDefinitions_test.cpp | 136 +++++++++++++++++-- src/xrpld/rpc/handlers/ServerDefinitions.cpp | 20 +++ 3 files changed, 151 insertions(+), 9 deletions(-) diff --git a/include/xrpl/protocol/jss.h b/include/xrpl/protocol/jss.h index 5df507af4a..27c4f400a7 100644 --- a/include/xrpl/protocol/jss.h +++ b/include/xrpl/protocol/jss.h @@ -292,7 +292,9 @@ JSS(duration_us); // out: NetworkOPs JSS(effective); // out: ValidatorList // in: UNL JSS(elapsed_seconds); -JSS(enabled); // out: AmendmentTable +JSS(enabled); // out: AmendmentTable (on-ledger); + // ServerDefinitions (this server) +JSS(ledger_enabled); // out: ServerDefinitions (on-ledger) JSS(engine_result); // out: NetworkOPs, TransactionSign, Submit JSS(engine_result_code); // out: NetworkOPs, TransactionSign, Submit JSS(engine_result_message); // out: NetworkOPs, TransactionSign, Submit diff --git a/src/test/rpc/ServerDefinitions_test.cpp b/src/test/rpc/ServerDefinitions_test.cpp index 81fb4c8e3b..e6af9a6a65 100644 --- a/src/test/rpc/ServerDefinitions_test.cpp +++ b/src/test/rpc/ServerDefinitions_test.cpp @@ -163,7 +163,7 @@ public: void testNoParams(FeatureBitset features) { - testcase("No Params, None Enabled"); + testcase("Default Env: config-forced, none on-ledger"); using namespace test::jtx; Env env{*this}; @@ -178,8 +178,6 @@ public: { if (!BEAST_EXPECT(feature.isMember(jss::name))) return; - // default config - so all should be disabled, and - // supported. Some may be vetoed. bool expectVeto = (votes.at(feature[jss::name].asString()) == VoteBehavior::DefaultNo); @@ -188,8 +186,12 @@ public: VoteBehavior::Obsolete); BEAST_EXPECTS( feature.isMember(jss::enabled) && - !feature[jss::enabled].asBool(), + feature[jss::enabled].asBool(), feature[jss::name].asString() + " enabled"); + BEAST_EXPECTS( + feature.isMember(jss::ledger_enabled) && + !feature[jss::ledger_enabled].asBool(), + feature[jss::name].asString() + " ledger_enabled"); BEAST_EXPECTS( feature.isMember(jss::vetoed) && feature[jss::vetoed].isBool() == !expectObsolete && @@ -208,7 +210,7 @@ public: void testSomeEnabled(FeatureBitset features) { - testcase("No Params, Some Enabled"); + testcase("Two config-forced, none on-ledger"); using namespace test::jtx; Env env{ @@ -228,7 +230,10 @@ public: (void)id.parseHex(it.key().asString().c_str()); if (!BEAST_EXPECT((*it).isMember(jss::name))) return; - bool expectEnabled = env.app().getAmendmentTable().isEnabled(id); + bool const expectOnLedger = + env.app().getAmendmentTable().isEnabled(id); + bool const expectForced = + id == featureDepositAuth || id == featureDepositPreauth; bool expectSupported = env.app().getAmendmentTable().isSupported(id); bool expectVeto = @@ -239,9 +244,14 @@ public: VoteBehavior::Obsolete); BEAST_EXPECTS( (*it).isMember(jss::enabled) && - (*it)[jss::enabled].asBool() == expectEnabled, + (*it)[jss::enabled].asBool() == + (expectOnLedger || expectForced), (*it)[jss::name].asString() + " enabled"); - if (expectEnabled) + BEAST_EXPECTS( + (*it).isMember(jss::ledger_enabled) && + (*it)[jss::ledger_enabled].asBool() == expectOnLedger, + (*it)[jss::name].asString() + " ledger_enabled"); + if (expectOnLedger) BEAST_EXPECTS( !(*it).isMember(jss::vetoed), (*it)[jss::name].asString() + " vetoed"); @@ -360,12 +370,122 @@ public: } } + void + testConfigForced(FeatureBitset features) + { + testcase("Config-forced features ([features] stanza)"); + + using namespace test::jtx; + + auto const forced = featurePriceOracle; + auto const forcedHex = to_string(forced); + + Env env{*this, FeatureBitset(forced)}; + + auto jrr = env.rpc("server_definitions")[jss::result]; + if (!BEAST_EXPECT(jrr.isMember(jss::features))) + return; + + bool sawForced = false; + for (auto it = jrr[jss::features].begin(); + it != jrr[jss::features].end(); + ++it) + { + auto const& f = *it; + auto const name = f[jss::name].asString(); + + if (!BEAST_EXPECTS( + f.isMember(jss::enabled) && f.isMember(jss::ledger_enabled), + name + " enabled/ledger_enabled")) + return; + + BEAST_EXPECTS( + !f[jss::ledger_enabled].asBool(), name + " ledger_enabled"); + + if (it.key().asString() == forcedHex) + { + sawForced = true; + BEAST_EXPECTS(f[jss::enabled].asBool(), name + " enabled"); + } + else + { + BEAST_EXPECTS(!f[jss::enabled].asBool(), name + " enabled"); + } + } + BEAST_EXPECT(sawForced); + } + + void + testOnLedger(FeatureBitset features) + { + testcase("On-ledger plus one config-forced"); + + using namespace test::jtx; + + // Veto XahauGenesis: FRESH would otherwise enable it via pseudo-tx. + auto const forced = featurePriceOracle; + auto const forcedHex = to_string(forced); + + Env env{ + *this, + envconfig([](std::unique_ptr cfg) { + cfg->START_UP = Config::FRESH; + cfg->section(SECTION_VETO_AMENDMENTS) + .append(to_string(featureXahauGenesis) + " XahauGenesis"); + return cfg; + }), + FeatureBitset(forced)}; + + auto jrr = env.rpc("server_definitions")[jss::result]; + if (!BEAST_EXPECT(jrr.isMember(jss::features))) + return; + + bool sawOnLedger = false; + bool sawForced = false; + for (auto it = jrr[jss::features].begin(); + it != jrr[jss::features].end(); + ++it) + { + uint256 id; + (void)id.parseHex(it.key().asString().c_str()); + auto const& f = *it; + auto const name = f[jss::name].asString(); + + if (!BEAST_EXPECTS( + f.isMember(jss::enabled) && f.isMember(jss::ledger_enabled), + name + " enabled/ledger_enabled")) + return; + + bool const onLedger = env.app().getAmendmentTable().isEnabled(id); + bool const isForced = it.key().asString() == forcedHex; + + BEAST_EXPECTS( + f[jss::ledger_enabled].asBool() == onLedger, + name + " ledger_enabled"); + BEAST_EXPECTS( + f[jss::enabled].asBool() == (onLedger || isForced), + name + " enabled"); + + if (onLedger) + sawOnLedger = true; + if (isForced) + { + sawForced = true; + BEAST_EXPECTS(!onLedger, name + " forced not on-ledger"); + } + } + BEAST_EXPECT(sawOnLedger); + BEAST_EXPECT(sawForced); + } + void testServerFeatures(FeatureBitset features) { testNoParams(features); testSomeEnabled(features); testWithMajorities(features); + testConfigForced(features); + testOnLedger(features); } void diff --git a/src/xrpld/rpc/handlers/ServerDefinitions.cpp b/src/xrpld/rpc/handlers/ServerDefinitions.cpp index 8c99a8de23..8b1728b887 100644 --- a/src/xrpld/rpc/handlers/ServerDefinitions.cpp +++ b/src/xrpld/rpc/handlers/ServerDefinitions.cpp @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -545,6 +546,25 @@ doServerDefinitions(RPC::JsonContext& context) features[to_string(h)][jss::majority] = t.time_since_epoch().count(); + // getJson's enabled is on-ledger; [features] also apply here. + for (auto const& name : features.getMemberNames()) + { + Json::Value& entry = features[name]; + entry[jss::ledger_enabled] = entry[jss::enabled].asBool(); + } + for (auto const& h : context.app.config().features) + { + Json::Value& entry = features[to_string(h)]; + if (!entry.isMember(jss::name)) + { + if (auto const fname = featureToName(h); !fname.empty()) + entry[jss::name] = fname; + } + if (!entry.isMember(jss::ledger_enabled)) + entry[jss::ledger_enabled] = false; + entry[jss::enabled] = true; + } + lastFeatures = features; { const std::string out = Json::FastWriter().write(features); From e87eabd8beb2a464b90afdd26d1aba3380dd9d5c Mon Sep 17 00:00:00 2001 From: tequ Date: Fri, 25 Sep 2026 10:17:12 +0900 Subject: [PATCH 09/11] HookOnV2_1 Amendment (#766) --- include/xrpl/protocol/detail/features.macro | 4 +- src/test/app/SetHook_test.cpp | 363 ++++++++++++++++++-- src/xrpld/app/tx/detail/SetHook.cpp | 169 ++++++--- src/xrpld/app/tx/detail/SetHook.h | 2 +- 4 files changed, 455 insertions(+), 83 deletions(-) diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index b4fdf46729..5dcc329c18 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -34,6 +34,7 @@ // If you add an amendment here, then do not forget to increment `numFeatures` // in include/xrpl/protocol/Feature.h. +XRPL_FEATURE(HookOnV2_1, Supported::yes, VoteBehavior::DefaultNo) XRPL_FEATURE(OnChainManifests, Supported::yes, VoteBehavior::DefaultNo) XRPL_FIX (HookMap, Supported::yes, VoteBehavior::DefaultYes) XRPL_FIX (GuardDepth32, Supported::yes, VoteBehavior::DefaultNo) @@ -65,7 +66,8 @@ XRPL_FEATURE(XChainBridge, Supported::no, VoteBehavior::DefaultNo XRPL_FEATURE(AMM, Supported::no, VoteBehavior::DefaultNo) XRPL_FIX (ReducedOffersV1, Supported::yes, VoteBehavior::DefaultYes) XRPL_FEATURE(HooksUpdate2, Supported::yes, VoteBehavior::DefaultNo) -XRPL_FEATURE(HookOnV2, Supported::yes, VoteBehavior::DefaultNo) +// replaced to HookOnV2_1 +XRPL_FEATURE(HookOnV2, Supported::no, VoteBehavior::DefaultNo) XRPL_FIX (HookAPI20251128, Supported::yes, VoteBehavior::DefaultYes) XRPL_FIX (CronStacking, Supported::yes, VoteBehavior::DefaultYes) XRPL_FEATURE(ExtendedHookState, Supported::yes, VoteBehavior::DefaultNo) diff --git a/src/test/app/SetHook_test.cpp b/src/test/app/SetHook_test.cpp index bf09631c54..0c1263454b 100644 --- a/src/test/app/SetHook_test.cpp +++ b/src/test/app/SetHook_test.cpp @@ -1355,11 +1355,16 @@ public: auto const baseFee = drops[jss::base_fee_no_hooks]; BEAST_EXPECT(baseFee == to_string(feeDrops)); auto const openLedgerFee = drops[jss::open_ledger_fee]; - BEAST_EXPECT(openLedgerFee == expected); + BEAST_EXPECTS( + openLedgerFee == expected, + "openLedgerFee: " + openLedgerFee.asString() + + ", expected: " + expected); // verify hooks fee auto const hooksFee = jrr[jss::result][jss::fee_hooks_feeunits]; - BEAST_EXPECT(hooksFee == expected); + BEAST_EXPECTS( + hooksFee == expected, + "hooksFee: " + hooksFee.asString() + ", expected: " + expected); } void @@ -1369,7 +1374,8 @@ public: using namespace jtx; Env env{*this, features}; - bool const hookOnV2 = env.current()->rules().enabled(featureHookOnV2); + bool const hookOnV2 = env.current()->rules().enabled(featureHookOnV2) || + env.current()->rules().enabled(featureHookOnV2_1); auto const alice = Account{"alice"}; auto const bob = Account{"bob"}; @@ -1532,12 +1538,10 @@ public: jv.removeMember(jss::HookOn); jv[jss::HookOnIncoming] = "0000000000000000000000000000000000000000000000000000000000" - "0000" - "00"; + "000000"; jv[jss::HookOnOutgoing] = "0000000000000000000000000000000000000000000000000000000000" - "0000" - "01"; + "000001"; env(ripple::test::jtx::hook(alice, {{jv}}, 0), M("Execution: Install"), HSFEE); @@ -1602,12 +1606,10 @@ public: jv.removeMember(jss::HookOn); jv[jss::HookOnIncoming] = "fffffffffffffffffffffffffffffffffffffff7ffffffffffffffffff" - "bfff" - "ff"; // Invoke high + "bfffff"; // Invoke high jv[jss::HookOnOutgoing] = "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffff" - "bfff" - "fe"; // Payment high + "bffffe"; // Payment high env(ripple::test::jtx::hook(alice, {{jv}}, 0), M("Execution: Install"), HSFEE); @@ -1618,8 +1620,7 @@ public: jv[jss::Flags] = hsfOVERRIDE; jv[jss::HookOn] = "0000000000000000000000000000000000000000000000000000000000" - "0000" - "00"; + "000000"; env(ripple::test::jtx::hook(alice, {{jv}}, 0), M("Execution: Install"), HSFEE); @@ -1653,18 +1654,285 @@ public: deleteHook(alice); } + for (auto const& withFix : {false, true}) + { + // test featureHookOnV2_1 preflight + auto f = features - featureHookOnV2 - featureHookOnV2_1; + if (withFix) + f = f | featureHookOnV2_1; + else + f = f | featureHookOnV2; + Env env{*this, f}; + + env.fund(XRP(10000), alice, bob); + env.close(); + + TER const expected = + env.current()->rules().enabled(featureHookOnV2_1) + ? TER(temMALFORMED) + : TER(tesSUCCESS); + + // 0. only one direction HookOn is not allowed + for (auto const& key : {jss::HookOnIncoming, jss::HookOnOutgoing}) + { + auto noopJv = Json::Value{}; + noopJv[key] = + "fffffffffffffffffffffffffffffffffffffff7ffffffffffffffffff" + "bfffff"; + + env(ripple::test::jtx::hook(alice, {{noopJv}}, 0), + M("Lone direction HookOn NOOP"), + HSFEE, + ter(expected)); + env.close(); + + if (!withFix) + BEAST_EXPECT(!env.le(keylet::hook(alice))); + } + + // 1. Create Hook with HookOnIncoming/Outgoing to alice + auto jv = hso(accept_wasm); + jv.removeMember(jss::HookOn); + jv[jss::HookOnIncoming] = + "fffffffffffffffffffffffffffffffffffffff7ffffffffffffffffff" + "bfffff"; // Invoke high + jv[jss::HookOnOutgoing] = + "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffff" + "bffffe"; // Payment high + env(ripple::test::jtx::hook(alice, {{jv}}, 0), HSFEE); + env.close(); + + // 2. Install Hook with HookOn, Incoming/Outgoing to bob + jv = Json::Value{}; + jv[jss::HookHash] = accept_hash_str; + jv[jss::Flags] = hsfOVERRIDE; + jv[jss::HookOn] = + "0000000000000000000000000000000000000000000000000000000000" + "000000"; + jv[jss::HookOnIncoming] = + "fffffffffffffffffffffffffffffffffffffff7ffffffffffffffffff" + "bfffff"; // Invoke high + jv[jss::HookOnOutgoing] = + "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffff" + "bffffe"; // Payment high + env(ripple::test::jtx::hook(bob, {{jv}}, 0), HSFEE, ter(expected)); + env.close(); + + // 3. Update Hook with HookOn, Incoming/Outgoing to alice + jv = Json::Value{}; + jv[jss::HookOn] = + "0000000000000000000000000000000000000000000000000000000000" + "000000"; + jv[jss::HookOnIncoming] = + "fffffffffffffffffffffffffffffffffffffff7ffffffffffffffffff" + "bfffff"; // Invoke high + jv[jss::HookOnOutgoing] = + "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffff" + "bffffe"; // Payment high + env(ripple::test::jtx::hook(alice, {{jv}}, 0), + HSFEE, + ter(expected)); + env.close(); + } + + for (auto const& withFix : {false, true}) + { + // test featureHookOnV2_1 install/update + auto f = features - featureHookOnV2 - featureHookOnV2_1; + if (withFix) + f = f | featureHookOnV2_1; + else + f = f | featureHookOnV2; + + { + // install:: HookDefinition: HookOn -> Hook:Incoming/Outgoing + // update:: Hook: HookOn -> Hook:Incoming/Outgoing + Env env{*this, f}; + + env.fund(XRP(10000), alice, bob); + env.close(); + + // Create Hook with HookOn to alice + auto jv = hso(accept_wasm); + jv.removeMember(jss::HookOn); + jv[jss::HookOn] = + "0000000000000000000000000000000000000000000000000000000000" + "000000"; + env(ripple::test::jtx::hook(alice, {{jv}}, 0), HSFEE); + env.close(); + + // Install Hook with Incoming/Outgoing to bob + jv = Json::Value{}; + jv[jss::HookHash] = accept_hash_str; + jv[jss::Flags] = hsfOVERRIDE; + jv[jss::HookOnIncoming] = + "fffffffffffffffffffffffffffffffffffffff7ffffffffffffffffff" + "bfffff"; // Invoke high + jv[jss::HookOnOutgoing] = + "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffff" + "bffffe"; // Payment high + env(ripple::test::jtx::hook(bob, {{jv}}, 0), HSFEE); + env.close(); + { + auto const hooksObj = env.le(keylet::hook(bob)); + BEAST_EXPECT(hooksObj && hooksObj->isFieldPresent(sfHooks)); + auto const& hooks = hooksObj->getFieldArray(sfHooks); + BEAST_EXPECT(hooks.size() == 1); + auto const& h = hooks[0]; + BEAST_EXPECT(!h.isFieldPresent(sfHookOn)); + BEAST_EXPECT(h.isFieldPresent(sfHookOnOutgoing)); + BEAST_EXPECT(h.isFieldPresent(sfHookOnIncoming)); + } + + // Update Hook with HookOn, Incoming/Outgoing to alice + jv = Json::Value{}; + jv[jss::HookOnIncoming] = + "fffffffffffffffffffffffffffffffffffffff7ffffffffffffffffff" + "bfffff"; // Invoke high + jv[jss::HookOnOutgoing] = + "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffff" + "bffffe"; // Payment high + env(ripple::test::jtx::hook(alice, {{jv}}, 0), HSFEE); + env.close(); + { + auto const hooksObj = env.le(keylet::hook(alice)); + BEAST_EXPECT(hooksObj && hooksObj->isFieldPresent(sfHooks)); + auto const& hooks = hooksObj->getFieldArray(sfHooks); + BEAST_EXPECT(hooks.size() == 1); + auto const& h = hooks[0]; + BEAST_EXPECT(!h.isFieldPresent(sfHookOn)); + BEAST_EXPECT(h.isFieldPresent(sfHookOnOutgoing)); + BEAST_EXPECT(h.isFieldPresent(sfHookOnIncoming)); + } + + // Re-update Hook with HookOn, Incoming/Outgoing to alice + jv = Json::Value{}; + jv[jss::HookOn] = + "fffffffffffffffffffffffffffffffffffffff7ffffffffffffffffff" + "bfffff"; + env(ripple::test::jtx::hook(alice, {{jv}}, 0), HSFEE); + env.close(); + { + auto const hooksObj = env.le(keylet::hook(alice)); + BEAST_EXPECT(hooksObj && hooksObj->isFieldPresent(sfHooks)); + auto const& hooks = hooksObj->getFieldArray(sfHooks); + BEAST_EXPECT(hooks.size() == 1); + auto const& h = hooks[0]; + BEAST_EXPECT(h.isFieldPresent(sfHookOn)); + if (withFix) + { + BEAST_EXPECT(!h.isFieldPresent(sfHookOnOutgoing)); + BEAST_EXPECT(!h.isFieldPresent(sfHookOnIncoming)); + } + else + { + BEAST_EXPECT(h.isFieldPresent(sfHookOnOutgoing)); + BEAST_EXPECT(h.isFieldPresent(sfHookOnIncoming)); + } + } + } + + { + // install:: HookDefinition: Incoming/Outgoing -> Hook:HookOn + // update:: Hook: Incoming/Outgoing -> Hook:HookOn + Env env{*this, f}; + + env.fund(XRP(10000), alice, bob); + env.close(); + + // Create Hook with HookOn to alice + auto jv = hso(accept_wasm); + jv.removeMember(jss::HookOn); + jv[jss::HookOnIncoming] = + "fffffffffffffffffffffffffffffffffffffff7ffffffffffffffffff" + "bfffff"; // Invoke high + jv[jss::HookOnOutgoing] = + "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffff" + "bffffe"; // Payment high + env(ripple::test::jtx::hook(alice, {{jv}}, 0), HSFEE); + env.close(); + + // Install Hook with Incoming/Outgoing to bob + jv = Json::Value{}; + jv[jss::HookHash] = accept_hash_str; + jv[jss::Flags] = hsfOVERRIDE; + jv[jss::HookOn] = + "0000000000000000000000000000000000000000000000000000000000" + "000000"; + env(ripple::test::jtx::hook(bob, {{jv}}, 0), HSFEE); + env.close(); + { + auto const hooksObj = env.le(keylet::hook(bob)); + BEAST_EXPECT(hooksObj && hooksObj->isFieldPresent(sfHooks)); + auto const& hooks = hooksObj->getFieldArray(sfHooks); + BEAST_EXPECT(hooks.size() == 1); + auto const& h = hooks[0]; + BEAST_EXPECT(h.isFieldPresent(sfHookOn)); + BEAST_EXPECT(!h.isFieldPresent(sfHookOnOutgoing)); + BEAST_EXPECT(!h.isFieldPresent(sfHookOnIncoming)); + } + + // Update Hook with HookOn, Incoming/Outgoing to alice + jv = Json::Value{}; + jv[jss::HookOn] = + "0000000000000000000000000000000000000000000000000000000000" + "000000"; + env(ripple::test::jtx::hook(alice, {{jv}}, 0), HSFEE); + env.close(); + { + auto const hooksObj = env.le(keylet::hook(alice)); + BEAST_EXPECT(hooksObj && hooksObj->isFieldPresent(sfHooks)); + auto const& hooks = hooksObj->getFieldArray(sfHooks); + BEAST_EXPECT(hooks.size() == 1); + auto const& h = hooks[0]; + if (withFix) + BEAST_EXPECT(h.isFieldPresent(sfHookOn)); + // Cause different results between gcc and clang due to + // undefined behavior so we don't test it + // else + // BEAST_EXPECT(!h.isFieldPresent(sfHookOn)); + BEAST_EXPECT(!h.isFieldPresent(sfHookOnOutgoing)); + BEAST_EXPECT(!h.isFieldPresent(sfHookOnIncoming)); + } + + // Re-update Hook with HookOn, Incoming/Outgoing to alice + jv = Json::Value{}; + jv[jss::HookOnIncoming] = + "fffffffffffffffffffffffffffffffffffffff7ffffffffffffffffff" + "bffff0"; // Invoke high + jv[jss::HookOnOutgoing] = + "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffff" + "bffff1"; // Payment high + env(ripple::test::jtx::hook(alice, {{jv}}, 0), HSFEE); + env.close(); + { + auto const hooksObj = env.le(keylet::hook(alice)); + BEAST_EXPECT(hooksObj && hooksObj->isFieldPresent(sfHooks)); + auto const& hooks = hooksObj->getFieldArray(sfHooks); + BEAST_EXPECT(hooks.size() == 1); + auto const& h = hooks[0]; + if (withFix) + BEAST_EXPECT(!h.isFieldPresent(sfHookOn)); + // Cause different results between gcc and clang due to + // undefined behavior so we don't test it + // else + // BEAST_EXPECT(h.isFieldPresent(sfHookOn)); + BEAST_EXPECT(h.isFieldPresent(sfHookOnOutgoing)); + BEAST_EXPECT(h.isFieldPresent(sfHookOnIncoming)); + } + } + } + // Fee RPC { auto jv = hso(accept_wasm); jv.removeMember(jss::HookOn); jv[jss::HookOnIncoming] = "fffffffffffffffffffffffffffffffffffffff7ffffffffffffffffff" - "bfff" - "ff"; // Invoke high + "bfffff"; // Invoke high jv[jss::HookOnOutgoing] = "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffff" - "bfff" - "fe"; // Payment high + "bffffe"; // Payment high env(ripple::test::jtx::hook(alice, {{jv}}, 0), HSFEE); env.close(); @@ -3003,25 +3271,37 @@ public: testInferHookSetOperation() { testcase("Test operation inference"); + using namespace jtx; + + auto const alice = Account{"alice"}; + Env env{*this}; + env.fund(XRP(10000), alice); + env(noop(alice)); + + SetHookCtx shCtx{ + .j = env.app().journal("SetHook"), + .tx = *(env.tx()), + .app = env.app(), + .rules = env.current()->rules()}; // hsoNOOP { STObject hso{sfHook}; - BEAST_EXPECT(SetHook::inferOperation(hso) == hsoNOOP); + BEAST_EXPECT(SetHook::inferOperation(shCtx, hso) == hsoNOOP); } // hsoCREATE { STObject hso{sfHook}; hso.setFieldVL(sfCreateCode, {1}); // non-empty create code - BEAST_EXPECT(SetHook::inferOperation(hso) == hsoCREATE); + BEAST_EXPECT(SetHook::inferOperation(shCtx, hso) == hsoCREATE); } // hsoDELETE { STObject hso{sfHook}; hso.setFieldVL(sfCreateCode, ripple::Blob{}); // empty create code - BEAST_EXPECT(SetHook::inferOperation(hso) == hsoDELETE); + BEAST_EXPECT(SetHook::inferOperation(shCtx, hso) == hsoDELETE); } // hsoINSTALL @@ -3029,7 +3309,7 @@ public: STObject hso{sfHook}; hso.setFieldH256( sfHookHash, uint256{beast::zero}); // all zeros hook hash - BEAST_EXPECT(SetHook::inferOperation(hso) == hsoINSTALL); + BEAST_EXPECT(SetHook::inferOperation(shCtx, hso) == hsoINSTALL); } // hsoNSDELETE @@ -3038,14 +3318,14 @@ public: hso.setFieldH256( sfHookNamespace, uint256{beast::zero}); // all zeros hook hash hso.setFieldU32(sfFlags, hsfNSDELETE); - BEAST_EXPECT(SetHook::inferOperation(hso) == hsoNSDELETE); + BEAST_EXPECT(SetHook::inferOperation(shCtx, hso) == hsoNSDELETE); } // hsoUPDATE { STObject hso{sfHook}; hso.setFieldH256(sfHookOn, UINT256_BIT[0]); - BEAST_EXPECT(SetHook::inferOperation(hso) == hsoUPDATE); + BEAST_EXPECT(SetHook::inferOperation(shCtx, hso) == hsoUPDATE); } // hsoINVALID @@ -3054,7 +3334,39 @@ public: hso.setFieldVL(sfCreateCode, {1}); // non-empty create code hso.setFieldH256( sfHookHash, uint256{beast::zero}); // all zeros hook hash - BEAST_EXPECT(SetHook::inferOperation(hso) == hsoINVALID); + BEAST_EXPECT(SetHook::inferOperation(shCtx, hso) == hsoINVALID); + + for (auto fix : {true, false}) + { + auto feature = supported_amendments() - featureHookOnV2 - + featureHookOnV2_1; + if (fix) + feature = feature | featureHookOnV2_1; + else + feature = feature | featureHookOnV2; + Env env{*this, feature}; + SetHookCtx shCtx{ + .j = env.app().journal("SetHook"), + .tx = *env.tx(), + .app = env.app(), + .rules = env.current()->rules()}; + + STObject hso2{sfHook}; + hso2.setFieldH256( + sfHookOnOutgoing, + uint256{beast::zero}); // all zeros hook on outgoing + BEAST_EXPECT( + SetHook::inferOperation(shCtx, hso2) == + (fix ? hsoINVALID : hsoNOOP)); + + STObject hso3{sfHook}; + hso3.setFieldH256( + sfHookOnIncoming, + uint256{beast::zero}); // all zeros hook on incoming + BEAST_EXPECT( + SetHook::inferOperation(shCtx, hso3) == + (fix ? hsoINVALID : hsoNOOP)); + } } } @@ -15075,7 +15387,8 @@ public: testNSDeletePartial(features); testPageCap(features); - testHookOnV2(features); + testHookOnV2((features | featureHookOnV2) - featureHookOnV2_1); + testHookOnV2((features | featureHookOnV2_1) - featureHookOnV2); testHookName(features); testFillCopy(features); diff --git a/src/xrpld/app/tx/detail/SetHook.cpp b/src/xrpld/app/tx/detail/SetHook.cpp index 967e17e7ca..5a870ab12b 100644 --- a/src/xrpld/app/tx/detail/SetHook.cpp +++ b/src/xrpld/app/tx/detail/SetHook.cpp @@ -207,7 +207,7 @@ validateHookParams(SetHookCtx& ctx, STArray const& hookParams) // infer which operation the user is attempting to execute from the present and // absent fields HookSetOperation -SetHook::inferOperation(STObject const& hookSetObj) +SetHook::inferOperation(SetHookCtx& ctx, STObject const& hookSetObj) { uint64_t wasmByteCount = hookSetObj.isFieldPresent(sfCreateCode) ? hookSetObj.getFieldVL(sfCreateCode).size() @@ -216,7 +216,12 @@ SetHook::inferOperation(STObject const& hookSetObj) bool hasHash = hookSetObj.isFieldPresent(sfHookHash); bool hasCode = hookSetObj.isFieldPresent(sfCreateCode); - if (hasHash && hasCode) // Both HookHash and CreateCode: invalid + bool invalidHookOn = ctx.rules.enabled(featureHookOnV2_1) && + hookSetObj.isFieldPresent(sfHookOnOutgoing) != + hookSetObj.isFieldPresent(sfHookOnIncoming); + + if ((hasHash && hasCode) || invalidHookOn) // Both HookHash and CreateCode + // or invalid HookOn: invalid return hsoINVALID; else if (hasHash) // Hookhash only: install return hsoINSTALL; @@ -244,6 +249,62 @@ SetHook::inferOperation(STObject const& hookSetObj) : hsoUPDATE; } +bool +validateHookOn(SetHookCtx& ctx, STObject const& hookSetObj) +{ + if (!hookSetObj.isFieldPresent(sfHookOn)) + { + if (!ctx.rules.enabled(featureHookOnV2) && + !ctx.rules.enabled(featureHookOnV2_1)) + { + JLOG(ctx.j.trace()) + << "HookSet(" << hook::log::HOOKON_MISSING << ")[" << HS_ACC() + << "]: Malformed transaction: SetHook must include " + "sfHookOn before featureHookOnV2_1 is enabled."; + return false; + } + + if (!hookSetObj.isFieldPresent(sfHookOnOutgoing) || + !hookSetObj.isFieldPresent(sfHookOnIncoming)) + { + JLOG(ctx.j.trace()) + << "HookSet(" << hook::log::HOOKON_MISSING << ")[" << HS_ACC() + << "]: Malformed transaction: SetHook must include " + "sfHookOnOutgoing and sfHookOnIncoming " + "when creating a new hook without sfHookOn."; + return false; + } + + auto const outgoing = hookSetObj.getFieldH256(sfHookOnOutgoing); + auto const incoming = hookSetObj.getFieldH256(sfHookOnIncoming); + if (outgoing == incoming) + { + JLOG(ctx.j.trace()) + << "HookSet(" << hook::log::HOOKON_MISSING << ")[" << HS_ACC() + << "]: Malformed transaction: SetHook outgoing and " + "incoming hookon must be different."; + return false; + } + + return true; + } + else + { + if (hookSetObj.isFieldPresent(sfHookOnOutgoing) || + hookSetObj.isFieldPresent(sfHookOnIncoming)) + { + JLOG(ctx.j.trace()) + << "HookSet(" << hook::log::HOOKON_MISSING << ")[" << HS_ACC() + << "]: Malformed transaction: SetHook must no" + "include sfHookOnOutgoing and sfHookOnIncoming " + "when creating a new hook with sfHookOn."; + return false; + } + } + + return true; +} + // This is a context-free validation, it does not take into account the current // state of the ledger returns < valid, instruction count > may throw // overflow_error @@ -254,7 +315,7 @@ SetHook::validateHookSetEntry(SetHookCtx& ctx, STObject const& hookSetObj) ? hookSetObj.getFieldU32(sfFlags) : 0; - switch (inferOperation(hookSetObj)) + switch (inferOperation(ctx, hookSetObj)) { case hsoNOOP: { return true; @@ -363,6 +424,13 @@ SetHook::validateHookSetEntry(SetHookCtx& ctx, STObject const& hookSetObj) // hookon may be present if the user so chooses // flags may be present if the user so chooses + if (ctx.rules.enabled(featureHookOnV2_1) && + (hookSetObj.isFieldPresent(sfHookOn) || + hookSetObj.isFieldPresent(sfHookOnOutgoing) || + hookSetObj.isFieldPresent(sfHookOnIncoming)) && + !validateHookOn(ctx, hookSetObj)) + return false; + return true; } @@ -407,6 +475,13 @@ SetHook::validateHookSetEntry(SetHookCtx& ctx, STObject const& hookSetObj) // hookon may be present if the user so chooses // flags may be present if the user so chooses + if (ctx.rules.enabled(featureHookOnV2_1) && + (hookSetObj.isFieldPresent(sfHookOn) || + hookSetObj.isFieldPresent(sfHookOnOutgoing) || + hookSetObj.isFieldPresent(sfHookOnIncoming)) && + !validateHookOn(ctx, hookSetObj)) + return false; + return true; } @@ -456,56 +531,8 @@ SetHook::validateHookSetEntry(SetHookCtx& ctx, STObject const& hookSetObj) } // validate sfHookOn - if (!hookSetObj.isFieldPresent(sfHookOn)) - { - if (!ctx.rules.enabled(featureHookOnV2)) - { - JLOG(ctx.j.trace()) - << "HookSet(" << hook::log::HOOKON_MISSING << ")[" - << HS_ACC() - << "]: Malformed transaction: SetHook must include " - "sfHookOn before featureHookOnV2 is enabled."; - return false; - } - - if (!hookSetObj.isFieldPresent(sfHookOnOutgoing) || - !hookSetObj.isFieldPresent(sfHookOnIncoming)) - { - JLOG(ctx.j.trace()) - << "HookSet(" << hook::log::HOOKON_MISSING << ")[" - << HS_ACC() - << "]: Malformed transaction: SetHook must include " - "sfHookOnOutgoing and sfHookOnIncoming " - "when creating a new hook without sfHookOn."; - return false; - } - - auto const outgoing = hookSetObj.getFieldH256(sfHookOnOutgoing); - auto const incoming = hookSetObj.getFieldH256(sfHookOnIncoming); - if (outgoing == incoming) - { - JLOG(ctx.j.trace()) - << "HookSet(" << hook::log::HOOKON_MISSING << ")[" - << HS_ACC() - << "]: Malformed transaction: SetHook outgoing and " - "incoming hookon must be different."; - return false; - } - } - else - { - if (hookSetObj.isFieldPresent(sfHookOnOutgoing) || - hookSetObj.isFieldPresent(sfHookOnIncoming)) - { - JLOG(ctx.j.trace()) - << "HookSet(" << hook::log::HOOKON_MISSING << ")[" - << HS_ACC() - << "]: Malformed transaction: SetHook must no" - "include sfHookOnOutgoing and sfHookOnIncoming " - "when creating a new hook with sfHookOn."; - return false; - } - } + if (!validateHookOn(ctx, hookSetObj)) + return false; // validate sfHookCanEmit // HookCanEmit field is an optional field for backward compatibility @@ -1405,7 +1432,7 @@ SetHook::setHook() HookSetOperation op = hsoNOOP; if (hookSetObj) - op = inferOperation(hookSetObj->get()); + op = inferOperation(ctx, hookSetObj->get()); // these flags are not able to be passed onto the ledger object int newFlags = 0; @@ -1627,10 +1654,23 @@ SetHook::setHook() newHook.setFieldH256(sfHookNamespace, *newNamespace); } + if (ctx.rules.enabled(featureHookOnV2_1)) + { + // sanity check + if (newHookOn && (newHookOnOutgoing || newHookOnIncoming)) + return tecINTERNAL; // LCOV_EXCL_LINE + + if ((!newHookOnOutgoing && newHookOnIncoming) || + (newHookOnOutgoing && !newHookOnIncoming)) + return tecINTERNAL; // LCOV_EXCL_LINE + } + // set the hookon field if it differs from definition if (newHookOn) { - if (*defHookOn == *newHookOn) + if ((!view().rules().enabled(featureHookOnV2_1) || + defHookOn.has_value()) && + *defHookOn == *newHookOn) { if (newHook.isFieldPresent(sfHookOn)) newHook.makeFieldAbsent(sfHookOn); @@ -1665,6 +1705,22 @@ SetHook::setHook() sfHookOnIncoming, *newHookOnIncoming); } + if (ctx.rules.enabled(featureHookOnV2_1)) + { + if (newHookOn) + { + if (newHook.isFieldPresent(sfHookOnIncoming)) + newHook.makeFieldAbsent(sfHookOnIncoming); + if (newHook.isFieldPresent(sfHookOnOutgoing)) + newHook.makeFieldAbsent(sfHookOnOutgoing); + } + if (newHookOnOutgoing || newHookOnIncoming) + { + if (newHook.isFieldPresent(sfHookOn)) + newHook.makeFieldAbsent(sfHookOn); + } + } + // set the hookcanemit field if it differs from definition if (newHookCanEmit) { @@ -1843,7 +1899,8 @@ SetHook::setHook() newHookDef->setFieldH256(sfHookHash, *createHookHash); // only HookOn or (HookOnOutgoing and HookOnIncoming) - if (!view().rules().enabled(featureHookOnV2) || + if ((!view().rules().enabled(featureHookOnV2) && + !view().rules().enabled(featureHookOnV2_1)) || (!newHookOnOutgoing && !newHookOnIncoming)) newHookDef->setFieldH256(sfHookOn, *newHookOn); else diff --git a/src/xrpld/app/tx/detail/SetHook.h b/src/xrpld/app/tx/detail/SetHook.h index db67715e42..06b6ff9efb 100644 --- a/src/xrpld/app/tx/detail/SetHook.h +++ b/src/xrpld/app/tx/detail/SetHook.h @@ -86,7 +86,7 @@ public: calculateBaseFee(ReadView const& view, STTx const& tx); static HookSetOperation - inferOperation(STObject const& hookSetObj); + inferOperation(SetHookCtx& ctx, STObject const& hookSetObj); static HookSetValidation validateHookSetEntry(SetHookCtx& ctx, STObject const& hookSetObj); From ba130f8363b671878c6be340cd6bcb4958f4cdcc Mon Sep 17 00:00:00 2001 From: tequ Date: Fri, 25 Sep 2026 11:36:31 +0900 Subject: [PATCH 10/11] AMMClawback supported::no (#827) Co-authored-by: Richard Holland --- include/xrpl/protocol/detail/features.macro | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index 5dcc329c18..23a36947ba 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -46,7 +46,7 @@ XRPL_FEATURE(HookAPISerializedType240, Supported::yes, VoteBehavior::DefaultNo XRPL_FEATURE(PermissionedDomains, Supported::no, VoteBehavior::DefaultNo) XRPL_FEATURE(DynamicNFT, Supported::no, VoteBehavior::DefaultNo) XRPL_FEATURE(Credentials, Supported::no, VoteBehavior::DefaultNo) -XRPL_FEATURE(AMMClawback, Supported::yes, VoteBehavior::DefaultNo) +XRPL_FEATURE(AMMClawback, Supported::no, VoteBehavior::DefaultNo) XRPL_FEATURE(MPTokensV1, Supported::no, VoteBehavior::DefaultNo) // InvariantsV1_1 will be changes to Supported::yes when all the // invariants expected to be included under it are complete. From c2e253b365541c30edc03ab3fae686664e6faf2f Mon Sep 17 00:00:00 2001 From: tequ Date: Fri, 25 Sep 2026 12:10:18 +0900 Subject: [PATCH 11/11] refactor new account seqno (#772) When creating accounts in GenesisLedger, we previously set the account Sequence to 0. However, since this conflicts with the PseudoAccount requirements, we are changing it to 1. There will be no impact on networks that are already running, and for future networks, there won't be any impact unless you create accounts via GenesisLedger. This specifically addresses an issue that comes up during unittests. --- src/test/app/AMMExtended_test.cpp | 3 ++ src/test/app/LedgerMaster_test.cpp | 8 +-- src/test/app/MPToken_test.cpp | 10 ++-- src/test/app/SetHookTSH_test.cpp | 20 +++---- src/test/app/SetHook_test.cpp | 8 +-- src/test/app/SetHook_wasm.h | 16 +++--- src/test/app/URIToken_test.cpp | 1 + src/test/rpc/AccountObjects_test.cpp | 62 +++++++++++----------- src/test/rpc/AccountOffers_test.cpp | 12 ++--- src/xrpld/app/tx/detail/AMMCreate.cpp | 7 +-- src/xrpld/app/tx/detail/Change.cpp | 3 +- src/xrpld/app/tx/detail/GenesisMint.cpp | 3 +- src/xrpld/app/tx/detail/Import.cpp | 7 +-- src/xrpld/app/tx/detail/InvariantCheck.cpp | 6 +-- src/xrpld/app/tx/detail/Payment.cpp | 7 +-- src/xrpld/app/tx/detail/Remit.cpp | 6 +-- src/xrpld/app/tx/detail/XChainBridge.cpp | 6 +-- src/xrpld/ledger/View.h | 18 +++++++ 18 files changed, 98 insertions(+), 105 deletions(-) diff --git a/src/test/app/AMMExtended_test.cpp b/src/test/app/AMMExtended_test.cpp index 5475b4a71a..08a98c6c7f 100644 --- a/src/test/app/AMMExtended_test.cpp +++ b/src/test/app/AMMExtended_test.cpp @@ -150,12 +150,15 @@ private: env.fund(XRP(20'000), alice, bob, carol, gw1, gw2); env.fund(XRP(20'000), dan); + env.close(); env.trust(USD1(20'000), alice, bob, carol, dan); env.trust(USD2(1'000), alice, bob, carol, dan); + env.close(); env(pay(gw1, dan, USD1(10'050))); env(pay(gw1, bob, USD1(50))); env(pay(gw2, bob, USD2(50))); + env.close(); AMM ammDan(env, dan, XRP(10'000), USD1(10'050)); diff --git a/src/test/app/LedgerMaster_test.cpp b/src/test/app/LedgerMaster_test.cpp index eaa2100c0c..09f3d81e72 100644 --- a/src/test/app/LedgerMaster_test.cpp +++ b/src/test/app/LedgerMaster_test.cpp @@ -104,8 +104,8 @@ class LedgerMaster_test : public beast::unit_test::suite startLegSeq, txnIndex); BEAST_EXPECT( *result == - uint256("0CC11AD1AD89661689F6B6148D82CB6A7101DA4D66EC843670262D" - "0B618F5745")); + uint256("DCAECE52F028B9D0F7CA43CBE15AB4EF7B84A8EF5D455238992E1E" + "DF2BDA1637")); } // success (second tx) { @@ -114,8 +114,8 @@ class LedgerMaster_test : public beast::unit_test::suite startLegSeq + 1, txnIndex); BEAST_EXPECT( *result == - uint256("DCAECE52F028B9D0F7CA43CBE15AB4EF7B84A8EF5D455238992E1E" - "DF2BDA1637")); + uint256("722BD5EC9E4AE36E724B6223F5A5887927AADFD0B18A3053AAE57F" + "434F8C9FE8")); } } diff --git a/src/test/app/MPToken_test.cpp b/src/test/app/MPToken_test.cpp index 0d89008ae7..505c756c34 100644 --- a/src/test/app/MPToken_test.cpp +++ b/src/test/app/MPToken_test.cpp @@ -1174,11 +1174,11 @@ class MPToken_test : public beast::unit_test::suite JsonOptions::none)[sfAffectedNodes.fieldName]; // Issuer got 10 in the transfer fees BEAST_EXPECT( - meta[2u][sfModifiedNode.fieldName][sfFinalFields.fieldName] + meta[0u][sfModifiedNode.fieldName][sfFinalFields.fieldName] [sfOutstandingAmount.fieldName] == "9990"); // Destination account got 9'990 BEAST_EXPECT( - meta[0u][sfModifiedNode.fieldName][sfFinalFields.fieldName] + meta[3u][sfModifiedNode.fieldName][sfFinalFields.fieldName] [sfMPTAmount.fieldName] == "9990"); // Source account spent 10'000 BEAST_EXPECT( @@ -1910,8 +1910,8 @@ class MPToken_test : public beast::unit_test::suite env.tx()->getJson(JsonOptions::none)[jss::hash].asString()}; BEAST_EXPECTS( txHash == - "42175D43E3D236A86B0F1B07182A1B2C78AD382372CFA09A48239C395FFFEE" - "30", + "61041108F0DAD50BAFD2F7B1102AC70B283EBDCED194CEBAF04D0184E4A70B" + "02", txHash); Json::Value const meta = env.rpc("tx", txHash)[jss::result][jss::meta]; auto const id = meta[jss::mpt_issuance_id].asString(); @@ -1919,7 +1919,7 @@ class MPToken_test : public beast::unit_test::suite BEAST_EXPECT(meta.isMember(jss::mpt_issuance_id)); BEAST_EXPECT(id == to_string(mptAlice.issuanceID())); BEAST_EXPECTS( - id == "00000001AE123A8556F3CF91154711376AFB0F894F832B3D", id); + id == "00000002AE123A8556F3CF91154711376AFB0F894F832B3D", id); } void diff --git a/src/test/app/SetHookTSH_test.cpp b/src/test/app/SetHookTSH_test.cpp index 218c0ffab2..2884a998ff 100644 --- a/src/test/app/SetHookTSH_test.cpp +++ b/src/test/app/SetHookTSH_test.cpp @@ -8279,16 +8279,16 @@ private: // validate the emitted txn ids std::vector const txIds = { - "9610F73CDD6590EB6B3C82E5EC55D4B4C80CD7128B98AA556F7EC9DD96AE7056", - "2F4582A29272390C0C25A80D4A3BCE5A14ACE6D86D8D0CB2C57719EB6FA881AE", - "89A301CFEF0DD781AB9032A6A2DCE0937BC0119D2CDD06033B8B2FD80968E519", - "DD8721B59024E168480B4DF8F8E93778601F0BD2E77FC991F3DA1182F5AD8B1E", - "5D735C2EE3CB8289F8E11621FDC9565F9D6D67F3AE59D65332EACE591D67945F", - "F02470E01731C968881AF4CBDEC90BB9E1F7AB0BE1CC22AF15451FB6D191096D", - "8AD65E541DECD49B1693F8C17DFD8A2B906F49C673C4FD2034FF772E2BE50C30", - "9F225229059CCC6257814D03C107884CF588C1C246A89ADFC16E50DF671B834C", - "13C2A54A14BADF3648CED05175E1CCAD713F7E5EA56D9735CF8813CD5551F281", - "87C60F41A96554587CED289F83F52DEE3CF670EEB189B067E6066B9A06056ADF", + "2FBEF981BAC322225D13C274D0118FECC1B93A89EDBC0AB3CBD8B30A1591024E", + "3C0A12F6A322486ABDA4180DC2B424BD252FD2FA6A33553B0B5D1526C020C475", + "AFFD55D757136FC0BE665F16E0D8CFBDC0D37CF85B5D3329BE1DBD3A44BB281C", + "BBE43C01068CE511DBF24F84A928E481451FF448AF2493E1A9BE099628D1F523", + "45970753F7B52448C23D66E5E19DBBC65F5BE6055C04AC3BA9896D7D67891DB1", + "97F4E09B2AEF4D798D378B1390B0624A68F304247B358334167E93217D502A16", + "770D3B786C8E3BEF560D9589B02557B0BDFC1C7FD15C6CF42E86068A6A411EFC", + "239DFA86EE8391418170D7FDF5AA8B8EEE0BBEF5E9F928BC29C23EEF6F1858AD", + "9FAA93AC6340C87B5C722FF0E8D4D3F39B9A0CA5E9799EC4CE316BFD5E379540", + "8672B2D8AA5E9EF86929B8662FE436541B2F739052732020B874BC304601C4DB", }; Json::Value params; params[jss::transaction] = diff --git a/src/test/app/SetHook_test.cpp b/src/test/app/SetHook_test.cpp index 0c1263454b..4a5ff684e1 100644 --- a/src/test/app/SetHook_test.cpp +++ b/src/test/app/SetHook_test.cpp @@ -4589,10 +4589,10 @@ public: uint8_t expected1[49] = { 0xEDU, 0x20U, 0x2EU, 0x00U, 0x00U, 0x00U, 0x01U, 0x3DU, 0x00U, 0x00U, - 0x00U, 0x00U, 0x00U, 0x00U, 0x00U, 0x01U, 0x5BU, 0xB8U, 0x05U, 0xD6U, - 0xC3U, 0x52U, 0xDFU, 0x7AU, 0x27U, 0x76U, 0x6DU, 0xC0U, 0x20U, 0x47U, - 0xB7U, 0x64U, 0x22U, 0x5AU, 0xB7U, 0x5DU, 0xF3U, 0xFAU, 0x0DU, 0xE3U, - 0xBDU, 0xC6U, 0x40U, 0xBAU, 0xD0U, 0x0AU, 0x66U, 0xEBU, 0x68U, + 0x00U, 0x00U, 0x00U, 0x00U, 0x00U, 0x01U, 0x5BU, 0xBCU, 0x09U, 0x95U, + 0xFDU, 0x03U, 0x0FU, 0xF1U, 0x85U, 0xF4U, 0xECU, 0xACU, 0x94U, 0xDBU, + 0x02U, 0x19U, 0x26U, 0xFEU, 0x58U, 0xD4U, 0x8DU, 0x48U, 0x21U, 0xCCU, + 0x82U, 0x2EU, 0x4AU, 0x6AU, 0xC9U, 0xC8U, 0xC6U, 0x7CU, 0x88U }; // 0x5CU // EmitNonce 32bytes diff --git a/src/test/app/SetHook_wasm.h b/src/test/app/SetHook_wasm.h index aecbc58a39..f21f620432 100644 --- a/src/test/app/SetHook_wasm.h +++ b/src/test/app/SetHook_wasm.h @@ -1613,10 +1613,10 @@ std::map> wasm = { uint8_t expected1[49] = { 0xEDU, 0x20U, 0x2EU, 0x00U, 0x00U, 0x00U, 0x01U, 0x3DU, 0x00U, 0x00U, - 0x00U, 0x00U, 0x00U, 0x00U, 0x00U, 0x01U, 0x5BU, 0xB8U, 0x05U, 0xD6U, - 0xC3U, 0x52U, 0xDFU, 0x7AU, 0x27U, 0x76U, 0x6DU, 0xC0U, 0x20U, 0x47U, - 0xB7U, 0x64U, 0x22U, 0x5AU, 0xB7U, 0x5DU, 0xF3U, 0xFAU, 0x0DU, 0xE3U, - 0xBDU, 0xC6U, 0x40U, 0xBAU, 0xD0U, 0x0AU, 0x66U, 0xEBU, 0x68U, + 0x00U, 0x00U, 0x00U, 0x00U, 0x00U, 0x01U, 0x5BU, 0xBCU, 0x09U, 0x95U, + 0xFDU, 0x03U, 0x0FU, 0xF1U, 0x85U, 0xF4U, 0xECU, 0xACU, 0x94U, 0xDBU, + 0x02U, 0x19U, 0x26U, 0xFEU, 0x58U, 0xD4U, 0x8DU, 0x48U, 0x21U, 0xCCU, + 0x82U, 0x2EU, 0x4AU, 0x6AU, 0xC9U, 0xC8U, 0xC6U, 0x7CU, 0x88U }; // 0x5CU // EmitNonce 32bytes @@ -1763,10 +1763,10 @@ std::map> wasm = { 0x36U, 0x00U, 0x00U, 0x00U, 0x00U, 0x00U, 0x00U, 0x00U, 0x00U, 0x00U, 0x00U, 0x00U, 0x00U, 0xEDU, 0x20U, 0x2EU, 0x00U, 0x00U, 0x00U, 0x01U, 0x3DU, 0x00U, 0x00U, 0x00U, 0x00U, 0x00U, 0x00U, 0x00U, 0x01U, 0x5BU, - 0xB8U, 0x05U, 0xD6U, 0xC3U, 0x52U, 0xDFU, 0x7AU, 0x27U, 0x76U, 0x6DU, - 0xC0U, 0x20U, 0x47U, 0xB7U, 0x64U, 0x22U, 0x5AU, 0xB7U, 0x5DU, 0xF3U, - 0xFAU, 0x0DU, 0xE3U, 0xBDU, 0xC6U, 0x40U, 0xBAU, 0xD0U, 0x0AU, 0x66U, - 0xEBU, 0x68U, 0x68U, 0x6FU, 0x6FU, 0x6BU, 0x5FU, 0x68U, 0x61U, 0x73U, + 0xBCU, 0x09U, 0x95U, 0xFDU, 0x03U, 0x0FU, 0xF1U, 0x85U, 0xF4U, 0xECU, + 0xACU, 0x94U, 0xDBU, 0x02U, 0x19U, 0x26U, 0xFEU, 0x58U, 0xD4U, 0x8DU, + 0x48U, 0x21U, 0xCCU, 0x82U, 0x2EU, 0x4AU, 0x6AU, 0xC9U, 0xC8U, 0xC6U, + 0x7CU, 0x88U, 0x68U, 0x6FU, 0x6FU, 0x6BU, 0x5FU, 0x68U, 0x61U, 0x73U, 0x68U, 0x28U, 0x28U, 0x75U, 0x69U, 0x6EU, 0x74U, 0x33U, 0x32U, 0x5FU, 0x74U, 0x29U, 0x65U, 0x78U, 0x70U, 0x65U, 0x63U, 0x74U, 0x65U, 0x64U, 0x5FU, 0x68U, 0x6FU, 0x6FU, 0x6BU, 0x5FU, 0x68U, 0x61U, 0x73U, 0x68U, diff --git a/src/test/app/URIToken_test.cpp b/src/test/app/URIToken_test.cpp index b04e08508c..1a6436e421 100644 --- a/src/test/app/URIToken_test.cpp +++ b/src/test/app/URIToken_test.cpp @@ -980,6 +980,7 @@ struct URIToken_test : public beast::unit_test::suite // setup env env.fund(XRP(1000), alice, bob, gw); + env.close(); env.trust(USD(100000), alice, bob); env.close(); env(pay(gw, alice, USD(1000))); diff --git a/src/test/rpc/AccountObjects_test.cpp b/src/test/rpc/AccountObjects_test.cpp index 73090e1a2d..a46b8a194a 100644 --- a/src/test/rpc/AccountObjects_test.cpp +++ b/src/test/rpc/AccountObjects_test.cpp @@ -37,20 +37,36 @@ namespace test { static char const* bobs_account_objects[] = { R"json({ - "Account" : "rPMh7Pi9ct699iZUTWaytJUoHcJ7cgyziK", - "BookDirectory" : "B025997A323F5C3E03DDF1334471F5984ABDE31C59D463525D038D7EA4C68000", - "BookNode" : "0", - "Flags" : 65536, - "LedgerEntryType" : "Offer", - "OwnerNode" : "0", - "Sequence" : 4, - "TakerGets" : { - "currency" : "USD", - "issuer" : "r32rQHyesiTtdWFU7UJVtff4nCR5SHCbJW", - "value" : "1" - }, - "TakerPays" : "100000000", - "index" : "A984D036A0E562433A8377CA57D1A1E056E58C0D04818F8DFD3A1AA3F217DD82" + "Account" : "rPMh7Pi9ct699iZUTWaytJUoHcJ7cgyziK", + "BookDirectory" : "50AD0A9E54D2B381288D535EB724E4275FFBF41580D28A925D038D7EA4C68000", + "BookNode" : "0", + "Flags" : 65536, + "LedgerEntryType" : "Offer", + "OwnerNode" : "0", + "Sequence" : 4, + "TakerGets" : { + "currency" : "USD", + "issuer" : "rPMh7Pi9ct699iZUTWaytJUoHcJ7cgyziK", + "value" : "1" + }, + "TakerPays" : "100000000", + "index" : "A984D036A0E562433A8377CA57D1A1E056E58C0D04818F8DFD3A1AA3F217DD82" +})json", + R"json({ + "Account" : "rPMh7Pi9ct699iZUTWaytJUoHcJ7cgyziK", + "BookDirectory" : "B025997A323F5C3E03DDF1334471F5984ABDE31C59D463525D038D7EA4C68000", + "BookNode" : "0", + "Flags" : 65536, + "LedgerEntryType" : "Offer", + "OwnerNode" : "0", + "Sequence" : 5, + "TakerGets" : { + "currency" : "USD", + "issuer" : "r32rQHyesiTtdWFU7UJVtff4nCR5SHCbJW", + "value" : "1" + }, + "TakerPays" : "100000000", + "index" : "CAFE32332D752387B01083B60CC63069BA4A969C9730836929F841450F6A718E" })json", R"json({ "Balance" : { @@ -95,22 +111,6 @@ static char const* bobs_account_objects[] = { }, "LowNode" : "0", "index" : "D89BC239086183EB9458C396E643795C1134963E6550E682A190A5F021766D43" -})json", - R"json({ - "Account" : "rPMh7Pi9ct699iZUTWaytJUoHcJ7cgyziK", - "BookDirectory" : "50AD0A9E54D2B381288D535EB724E4275FFBF41580D28A925D038D7EA4C68000", - "BookNode" : "0", - "Flags" : 65536, - "LedgerEntryType" : "Offer", - "OwnerNode" : "0", - "Sequence" : 3, - "TakerGets" : { - "currency" : "USD", - "issuer" : "rPMh7Pi9ct699iZUTWaytJUoHcJ7cgyziK", - "value" : "1" - }, - "TakerPays" : "100000000", - "index" : "E11029302EE744401427793A4F37BCB18F698D55C96851BEC5ABBD6242CF03D7" })json"}; class AccountObjects_test : public beast::unit_test::suite @@ -332,7 +332,7 @@ public: auto& aobj = resp[jss::result][jss::account_objects][i]; aobj.removeMember("PreviousTxnID"); aobj.removeMember("PreviousTxnLgrSeq"); - BEAST_EXPECT(aobj == bobj[i + 1]); + BEAST_EXPECT(aobj == bobj[i + 2]); } } // test stepped one-at-a-time with limit=1, resume from prev marker diff --git a/src/test/rpc/AccountOffers_test.cpp b/src/test/rpc/AccountOffers_test.cpp index 64113915e4..a3955c96a6 100644 --- a/src/test/rpc/AccountOffers_test.cpp +++ b/src/test/rpc/AccountOffers_test.cpp @@ -117,9 +117,9 @@ public: BEAST_EXPECT(jroOuter[0u][jss::quality] == "100000000"); BEAST_EXPECT(jroOuter[0u][jss::taker_gets][jss::currency] == "USD"); BEAST_EXPECT( - jroOuter[0u][jss::taker_gets][jss::issuer] == bob.human()); - BEAST_EXPECT(jroOuter[0u][jss::taker_gets][jss::value] == "1"); - BEAST_EXPECT(jroOuter[0u][jss::taker_pays] == "100000000"); + jroOuter[0u][jss::taker_gets][jss::issuer] == gw.human()); + BEAST_EXPECT(jroOuter[0u][jss::taker_gets][jss::value] == "2"); + BEAST_EXPECT(jroOuter[0u][jss::taker_pays] == "200000000"); BEAST_EXPECT(jroOuter[1u][jss::quality] == "5000000"); BEAST_EXPECT(jroOuter[1u][jss::taker_gets][jss::currency] == "USD"); @@ -131,9 +131,9 @@ public: BEAST_EXPECT(jroOuter[2u][jss::quality] == "100000000"); BEAST_EXPECT(jroOuter[2u][jss::taker_gets][jss::currency] == "USD"); BEAST_EXPECT( - jroOuter[2u][jss::taker_gets][jss::issuer] == gw.human()); - BEAST_EXPECT(jroOuter[2u][jss::taker_gets][jss::value] == "2"); - BEAST_EXPECT(jroOuter[2u][jss::taker_pays] == "200000000"); + jroOuter[2u][jss::taker_gets][jss::issuer] == bob.human()); + BEAST_EXPECT(jroOuter[2u][jss::taker_gets][jss::value] == "1"); + BEAST_EXPECT(jroOuter[2u][jss::taker_pays] == "100000000"); } { diff --git a/src/xrpld/app/tx/detail/AMMCreate.cpp b/src/xrpld/app/tx/detail/AMMCreate.cpp index 6133caa8d3..392978ace4 100644 --- a/src/xrpld/app/tx/detail/AMMCreate.cpp +++ b/src/xrpld/app/tx/detail/AMMCreate.cpp @@ -252,12 +252,7 @@ applyCreate( auto sleAMMRoot = std::make_shared(keylet::account(*ammAccount)); sleAMMRoot->setAccountID(sfAccount, *ammAccount); sleAMMRoot->setFieldAmount(sfBalance, STAmount{}); - std::uint32_t const seqno{ - ctx_.view().rules().enabled(featureXahauGenesis) - ? ctx_.view().info().parentCloseTime.time_since_epoch().count() - : ctx_.view().rules().enabled(featureDeletableAccounts) - ? ctx_.view().seq() - : 1}; + std::uint32_t const seqno = newAccountSeqNo(ctx_.view()); sleAMMRoot->setFieldU32(sfSequence, seqno); // Ignore reserves requirement, disable the master key, allow default // rippling (AMM LPToken can be used in payments and offer crossing but diff --git a/src/xrpld/app/tx/detail/Change.cpp b/src/xrpld/app/tx/detail/Change.cpp index 8b437dda32..b25a7916f6 100644 --- a/src/xrpld/app/tx/detail/Change.cpp +++ b/src/xrpld/app/tx/detail/Change.cpp @@ -554,8 +554,7 @@ Change::activateXahauGenesis() { sle = std::make_shared(kl); sle->setAccountID(sfAccount, accid); - std::uint32_t const seqno{ - sb.info().parentCloseTime.time_since_epoch().count()}; + std::uint32_t const seqno = newAccountSeqNo(sb); sle->setFieldU32(sfSequence, seqno); } diff --git a/src/xrpld/app/tx/detail/GenesisMint.cpp b/src/xrpld/app/tx/detail/GenesisMint.cpp index 18e92c26a3..c9e1e99276 100644 --- a/src/xrpld/app/tx/detail/GenesisMint.cpp +++ b/src/xrpld/app/tx/detail/GenesisMint.cpp @@ -243,8 +243,7 @@ GenesisMint::doApply() if (created) { // Create the account. - std::uint32_t const seqno{ - view().info().parentCloseTime.time_since_epoch().count()}; + std::uint32_t const seqno = newAccountSeqNo(view()); sle = std::make_shared(k); sle->setAccountID(sfAccount, id); diff --git a/src/xrpld/app/tx/detail/Import.cpp b/src/xrpld/app/tx/detail/Import.cpp index c7a95024af..7647e9b727 100644 --- a/src/xrpld/app/tx/detail/Import.cpp +++ b/src/xrpld/app/tx/detail/Import.cpp @@ -1307,12 +1307,7 @@ Import::doApply() if (create) { // Create the account. - std::uint32_t const seqno{ - view().rules().enabled(featureXahauGenesis) - ? view().info().parentCloseTime.time_since_epoch().count() - : view().rules().enabled(featureDeletableAccounts) - ? view().seq() - : 1}; + std::uint32_t const seqno = newAccountSeqNo(view()); sle = std::make_shared(keylet::account(id)); sle->setAccountID(sfAccount, id); diff --git a/src/xrpld/app/tx/detail/InvariantCheck.cpp b/src/xrpld/app/tx/detail/InvariantCheck.cpp index 0d3d88c00c..2136a8b7ed 100644 --- a/src/xrpld/app/tx/detail/InvariantCheck.cpp +++ b/src/xrpld/app/tx/detail/InvariantCheck.cpp @@ -1041,11 +1041,7 @@ ValidNewAccountRoot::finalize( tt == ttXCHAIN_ADD_ACCOUNT_CREATE_ATTESTATION) && isTesSuccess(result)) { - std::uint32_t const startingSeq{ - view.rules().enabled(featureXahauGenesis) - ? view.info().parentCloseTime.time_since_epoch().count() - : view.rules().enabled(featureDeletableAccounts) ? view.seq() - : 1}; + std::uint32_t const startingSeq = newAccountSeqNo(view); if (accountSeq_ != startingSeq) { diff --git a/src/xrpld/app/tx/detail/Payment.cpp b/src/xrpld/app/tx/detail/Payment.cpp index 316a309b90..f24e94647b 100644 --- a/src/xrpld/app/tx/detail/Payment.cpp +++ b/src/xrpld/app/tx/detail/Payment.cpp @@ -354,12 +354,7 @@ Payment::doApply() if (!sleDst) { - std::uint32_t const seqno{ - view().rules().enabled(featureXahauGenesis) - ? view().info().parentCloseTime.time_since_epoch().count() - : view().rules().enabled(featureDeletableAccounts) - ? view().seq() - : 1}; + std::uint32_t const seqno = newAccountSeqNo(view()); // Create the account. sleDst = std::make_shared(k); diff --git a/src/xrpld/app/tx/detail/Remit.cpp b/src/xrpld/app/tx/detail/Remit.cpp index 36a4a7ad2d..e962ea3ba4 100644 --- a/src/xrpld/app/tx/detail/Remit.cpp +++ b/src/xrpld/app/tx/detail/Remit.cpp @@ -328,11 +328,7 @@ Remit::doApply() nativeRemit += accountReserve; // Create the account. - std::uint32_t const seqno{ - sb.rules().enabled(featureXahauGenesis) - ? sb.info().parentCloseTime.time_since_epoch().count() - : sb.rules().enabled(featureDeletableAccounts) ? sb.seq() - : 1}; + std::uint32_t const seqno = newAccountSeqNo(sb); sleDstAcc = std::make_shared(keylet::account(dstAccID)); sleDstAcc->setAccountID(sfAccount, dstAccID); diff --git a/src/xrpld/app/tx/detail/XChainBridge.cpp b/src/xrpld/app/tx/detail/XChainBridge.cpp index 589ae10846..5aaa223252 100644 --- a/src/xrpld/app/tx/detail/XChainBridge.cpp +++ b/src/xrpld/app/tx/detail/XChainBridge.cpp @@ -481,11 +481,7 @@ transferHelper( } // Create the account. - std::uint32_t const seqno{ - psb.rules().enabled(featureXahauGenesis) - ? psb.info().parentCloseTime.time_since_epoch().count() - : psb.rules().enabled(featureDeletableAccounts) ? psb.seq() - : 1}; + std::uint32_t const seqno = newAccountSeqNo(psb); sleDst = std::make_shared(dstK); sleDst->setAccountID(sfAccount, dst); diff --git a/src/xrpld/ledger/View.h b/src/xrpld/ledger/View.h index b38bd7ecbd..7b6d1f29ac 100644 --- a/src/xrpld/ledger/View.h +++ b/src/xrpld/ledger/View.h @@ -1266,6 +1266,24 @@ deleteAMMTrustLine( std::optional const& ammAccountID, beast::Journal j); +[[nodiscard]] inline std::uint32_t +newAccountSeqNo(ReadView const& view) +{ + auto const time = view.info().parentCloseTime.time_since_epoch().count(); + return view.rules().enabled(featureXahauGenesis) + // TEQU: + // When creating accounts in GenesisLedger, we previously set + // the account Sequence to 0. However, since this conflicts with + // the PseudoAccount requirements, we are changing it to 1. + // There will be no impact on networks that are already running, + // and for future networks, there won't be any impact unless you + // create accounts via GenesisLedger. + // This specifically addresses an issue that comes up during + // unittests. + ? (time == 0 ? 1 : time) + : view.rules().enabled(featureDeletableAccounts) ? view.seq() : 1; +} + } // namespace ripple #endif