From 902ed9b4920beee3b7dfc24d91d8f24b7d9adbcb Mon Sep 17 00:00:00 2001 From: tequ Date: Mon, 21 Sep 2026 20:12:15 +0900 Subject: [PATCH 1/5] 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 2/5] 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 3/5] 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 4/5] 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 5/5] 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