From d58cddf554cecfbb407808736af6e5ece39c6462 Mon Sep 17 00:00:00 2001 From: Richard Holland Date: Fri, 12 Jun 2026 16:20:47 +1000 Subject: [PATCH] test cases for hookmap fix --- src/test/app/HookAPI_test.cpp | 58 ++++++++++++++++++++++++++- src/xrpld/app/hook/detail/HookAPI.cpp | 11 +++++ 2 files changed, 68 insertions(+), 1 deletion(-) diff --git a/src/test/app/HookAPI_test.cpp b/src/test/app/HookAPI_test.cpp index 6a3546016..b6047dcc8 100644 --- a/src/test/app/HookAPI_test.cpp +++ b/src/test/app/HookAPI_test.cpp @@ -3632,8 +3632,9 @@ public: auto const alice = Account{"alice"}; auto const bob = Account{"bob"}; + auto const claire = Account{"claire"}; Env env{*this, features}; - env.fund(XRP(100000), alice, bob); + env.fund(XRP(100000), alice, bob, claire); env.close(); // Compute hook hash for the accept hook @@ -3773,6 +3774,60 @@ public: BEAST_EXPECT(result2.has_value()); BEAST_EXPECT(result2.value() == newData.size()); } + + { + // fixHookMap: foreign state set without grant after state previous + // modified + + HookStateMap stateMap; + auto hookCtx = makeStubHookContext( + applyCtx, alice.id(), bob.id(), {}, stateMap); + + AccountID const aliceid = alice.id(); + + // Pre-populate stateMap + stateMap[alice.id()] = { + 100, // availableForReserves + 1, // namespaceCount + 1, // hookStateScale + {}}; + + auto& api = hookCtx.api(); + + // setup a hook on alice, and on claire, no grants + env(hook(alice, {{hso(genesis::AcceptHook)}}, 0), fee(XRP(1))); + env(hook(claire, {{hso(genesis::AcceptHook)}}, 0), fee(XRP(1))); + env.close(); + + // First modification + auto result1 = + api.state_foreign_set(testKey, testNs, aliceid, testData); + BEAST_EXPECT(result1.has_value()); + + // Second modification this time using bob as hookacc (should hit + // cache) + auto hookCtx2 = makeStubHookContext( + applyCtx, claire.id(), bob.id(), {}, stateMap); + auto& api2 = hookCtx2.api(); + Bytes newData{0x04, 0x05}; + auto result2 = + api2.state_foreign_set(testKey, testNs, aliceid, newData); + + if (features[fixHookMap]) + { + // new behaviour: grant is missing, cannot write + BEAST_EXPECT(!result2.has_value()); + BEAST_EXPECT(result2.error() == NOT_AUTHORIZED); + BEAST_EXPECT(hookCtx2.result.foreignStateSetDisabled); + } + else + { + // old behaviour: allow this illegal write due to the entry + // being modified previously in the map + BEAST_EXPECT(result2.has_value()); + BEAST_EXPECT(result2.value() == newData.size()); + } + } } void @@ -4832,6 +4887,7 @@ public: test_state(features); test_state_foreign(features); + test_state_foreign_set(features - fixHookMap); test_state_foreign_set(features); test_state_foreign_set_max(features); test_state_set(features); diff --git a/src/xrpld/app/hook/detail/HookAPI.cpp b/src/xrpld/app/hook/detail/HookAPI.cpp index 23b4d9faa..dec51f781 100644 --- a/src/xrpld/app/hook/detail/HookAPI.cpp +++ b/src/xrpld/app/hook/detail/HookAPI.cpp @@ -1945,8 +1945,19 @@ HookAPI::state_foreign_set( { auto const sle = hookCtx.applyCtx.view().read(ripple::keylet::hook(account)); + if (!sle) + { + if (hasFix) + { + hookCtx.result.foreignStateSetDisabled = true; + return Unexpected(NOT_AUTHORIZED); + } + return Unexpected(INTERNAL_ERROR); + } + + // RH TODO: test this code path more completely bool found_auth = false;