test cases for hookmap fix

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);

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;