more unit tests for hooks, still wip

src/ripple/app/hook/applyHook.h

@@ -92,7 +92,7 @@ namespace hook_api
     (((TERtoInt(x)) << 16)*-1)
 
 // for debugging if you want a lot of output change these to if (1)
-#define HOOK_DBG 1
+#define HOOK_DBG 0
 #define DBG_PRINTF if (HOOK_DBG) printf
 #define DBG_FPRINTF if (HOOK_DBG) fprintf
 

src/ripple/app/tx/impl/SetHook.cpp

@@ -388,7 +388,7 @@ SetHook::validateHookSetEntry(SetHookCtx& ctx, STObject const& hookSetObj)
                 return false;
             }
 
-            // validate api version, if provided
+            // ensure api version is provided
             if (!hookSetObj.isFieldPresent(sfHookApiVersion))
             {
                 JLOG(ctx.j.trace())
@@ -1093,7 +1093,6 @@ SetHook::setHook()
         if (hookSetObj)
             op = inferOperation(hookSetObj->get());
 
-
         // these flags are not able to be passed onto the ledger object
         int newFlags = 0;
         if (flags)
@@ -1106,15 +1105,6 @@ SetHook::setHook()
                 newFlags -= hsfNSDELETE;
         }
 
-
-        printf("HookSet operation %d: %s\n", hookSetNumber,
-                (op == hsoNSDELETE ? "hsoNSDELETE" :
-                (op == hsoDELETE ? "hsoDELETE" :
-                (op == hsoCREATE ? "hsoCREATE" :
-                (op == hsoINSTALL ? "hsoINSTALL" :
-                (op == hsoUPDATE ? "hsoUPDATE" :
-                (op == hsoNOOP ? "hsoNOOP" : "hsoINALID")))))));
-
         // if an existing hook exists at this position in the chain then extract the relevant fields
         if (oldHook && oldHook->get().isFieldPresent(sfHookHash))
         {

src/ripple/protocol/TER.h

@@ -341,9 +341,9 @@ TERtoInt(TECcodes v)
 template <template <typename> class Trait>
 class TERSubset
 {
+public:
     TERUnderlyingType code_;
 
-public:
     // Constructors
     constexpr TERSubset() : code_(tesSUCCESS)
     {

src/ripple/protocol/impl/TER.cpp

@@ -177,6 +177,7 @@ transResults()
         MAKE_ERROR(terOWNERS,                 "Non-zero owner count."),
         MAKE_ERROR(terQUEUED,                 "Held until escalated fee drops."),
         MAKE_ERROR(terPRE_TICKET,             "Ticket is not yet in ledger."),
+        MAKE_ERROR(terNO_HOOK,                "No hook with that hash exists on the ledger."),
 
         MAKE_ERROR(tesSUCCESS,                "The transaction was applied. Only final in a validated ledger."),
     };

src/test/app/SetHook_test.cpp

@@ -56,8 +56,6 @@ class SetHook_test : public beast::unit_test::suite
 {
 public:
 
-
-
     // This is a large fee, large enough that we can set most small test hooks without running into fee issues
     // we only want to test fee code specifically in fee unit tests, the rest of the time we want to ignore it.
     #define HSFEE fee(1'000'000)
@@ -78,7 +76,7 @@ public:
     }
 
     void
-    testMalformedTxStructure()
+    testTxStructure()
     {
         testcase("Checks malformed transactions");
         using namespace jtx;
@@ -126,114 +124,11 @@ public:
             env.close();
         }
 
-        {
-            Json::Value jv = 
-                ripple::test::jtx::hook(alice, {{hso(accept_wasm)}}, 0);
-            Json::Value iv = jv[jss::Hooks][0U];
-            iv[jss::Hook][jss::HookHash] = to_string(uint256{beast::zero});
-            jv[jss::Hooks][0U] = iv;
-            env(jv,
-                M("Cannot have both CreateCode and HookHash"),        
-                HSFEE, ter(temMALFORMED));
-            env.close();
-        }
 
-        env(ripple::test::jtx::hook(alice, {{hso(long_wasm)}}, 0),
-            M("If CreateCode is present, then it must be less than 64kib"),
-            HSFEE, ter(temMALFORMED));
-        env.close();
 
     }
 
-    void testMalformedDelete()
+    void testGrants()
-    {
-        testcase("Checks malformed delete operation");
-        using namespace jtx;
-        Env env{*this, supported_amendments()};
-
-        auto const alice = Account{"alice"};
-        env.fund(XRP(10000), alice);
-
-        Json::Value jv;
-        jv[jss::Account] = alice.human();
-        jv[jss::TransactionType] = jss::SetHook;
-        jv[jss::Flags] = 0;
-        jv[jss::Hooks] = Json::Value{Json::arrayValue};
-        
-        // flag required
-        {
-            Json::Value iv;
-            iv[jss::CreateCode] = "";
-            jv[jss::Hooks][0U][jss::Hook] = iv;
-            env(jv,
-                M("Hook DELETE operation must include hsfOVERRIDE flag"),        
-                HSFEE, ter(temMALFORMED));
-            env.close();
-        }
-
-        // invalid flags
-        {
-            Json::Value iv;
-            iv[jss::CreateCode] = "";
-            iv[jss::Flags] = "2147483648";
-            jv[jss::Hooks][0U][jss::Hook] = iv;
-            env(jv,
-                M("Hook DELETE operation must include hsfOVERRIDE flag"),        
-                HSFEE, ter(temMALFORMED));
-            env.close();
-        }
-
-        // grants, parameters, hookon, hookapiversion, hooknamespace keys must be absent
-        for (auto const& [key, value]: 
-            JSSMap {
-                {jss::HookGrants, Json::arrayValue},
-                {jss::HookParameters, Json::arrayValue},
-                {jss::HookOn, "0"},
-                {jss::HookApiVersion, "0"},
-                {jss::HookNamespace, to_string(uint256{beast::zero})}
-            })
-        {
-            Json::Value iv;
-            iv[jss::CreateCode] = "";
-            iv[key] = value;
-            jv[jss::Hooks][0U][jss::Hook] = iv;
-            env(jv,
-                M("Hook DELETE operation cannot include: grants, params, hookon, apiversion, namespace"), 
-                HSFEE, ter(temMALFORMED));
-            env.close();
-        } 
-    }
-
-    void testMalformedInstall()
-    {
-        testcase("Checks malformed install operation");
-        using namespace jtx;
-        Env env{*this, supported_amendments()};
-
-        auto const alice = Account{"alice"};
-        env.fund(XRP(10000), alice);
-
-        Json::Value jv;
-        jv[jss::Account] = alice.human();
-        jv[jss::TransactionType] = jss::SetHook;
-        jv[jss::Flags] = 0;
-        jv[jss::Hooks] = Json::Value{Json::arrayValue};
-
-        // trying to set api version
-        {
-            Json::Value iv;
-            iv[jss::HookHash] = to_string(uint256{beast::zero});
-            iv[jss::HookApiVersion] = 1U;
-            jv[jss::Hooks][0U][jss::Hook] = iv;
-            env(jv,
-                M("Hook Install operation cannot set apiversion"), 
-                HSFEE, ter(temMALFORMED));
-            env.close();
-        }
-
-    }
-    
-    void testMalformedGrants()
     {
         testcase("Checks malformed grants on install operation");
         using namespace jtx;
@@ -298,10 +193,9 @@ public:
                 HSFEE, ter(temMALFORMED));
             env.close();
         }
-
     }
 
-    void testMalformedParams()
+    void testParams()
     {
         testcase("Checks malformed params on install operation");
         using namespace jtx;
@@ -409,9 +303,103 @@ public:
 
     }
 
-    void testMalformedCreate()
+    void testInstall()
     {
-        testcase("Checks malformed create operation");
+        testcase("Checks malformed install operation");
+        using namespace jtx;
+        Env env{*this, supported_amendments()};
+
+        auto const alice = Account{"alice"};
+        env.fund(XRP(10000), alice);
+
+        auto const bob = Account{"bob"};
+        env.fund(XRP(10000), bob);
+
+        // create a hook that we can then install
+        {
+            env(ripple::test::jtx::hook(bob, {{
+                hso(accept_wasm), 
+                hso(rollback_wasm)
+            }}, 0),
+                M("First set = tesSUCCESS"),
+                HSFEE, ter(tesSUCCESS));
+        }
+
+        std::string accept_hash = to_string(ripple::sha512Half_s(
+            ripple::Slice(accept_wasm.data(), accept_wasm.size())
+        ));
+        
+        std::string rollback_hash = to_string(ripple::sha512Half_s(
+            ripple::Slice(accept_wasm.data(), accept_wasm.size())
+        ));
+
+        Json::Value jv;
+        jv[jss::Account] = alice.human();
+        jv[jss::TransactionType] = jss::SetHook;
+        jv[jss::Flags] = 0;
+        jv[jss::Hooks] = Json::Value{Json::arrayValue};
+
+        // can't set api version
+        {
+            Json::Value iv;
+            iv[jss::HookHash] = accept_hash;
+            iv[jss::HookApiVersion] = 1U;
+            jv[jss::Hooks][0U][jss::Hook] = iv;
+            env(jv,
+                M("Hook Install operation cannot set apiversion"),
+                HSFEE, ter(temMALFORMED));
+            env.close();
+        }
+
+        // can't set non-existent hook
+        {
+            Json::Value iv;
+            iv[jss::HookHash] = "DEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEF";
+            jv[jss::Hooks][0U][jss::Hook] = iv;
+            env(jv,
+                M("Hook Install operation cannot set non existent hook hash"),
+                HSFEE, ter(terNO_HOOK));
+            env.close();
+        }
+
+        // can set extant hook
+        {
+            Json::Value iv;
+            iv[jss::HookHash] = accept_hash;
+            jv[jss::Hooks][0U][jss::Hook] = iv;
+            env(jv,
+                M("Hook Install operation can set extant hook hash"),
+                HSFEE, ter(tesSUCCESS));
+            env.close();
+        }
+
+        // can't set extant hook over other hook without override flag
+        {
+            Json::Value iv;
+            iv[jss::HookHash] = rollback_hash;
+            jv[jss::Hooks][0U][jss::Hook] = iv;
+            env(jv,
+                M("Hook Install operation can set extant hook hash"),
+                HSFEE, ter(tecREQUIRES_FLAG));
+            env.close();
+        }
+
+        // can set extant hook over other hook with override flag
+        {
+            Json::Value iv;
+            iv[jss::HookHash] = rollback_hash;
+            iv[jss::Flags] = hsfOVERRIDE;
+            jv[jss::Hooks][0U][jss::Hook] = iv;
+            env(jv,
+                M("Hook Install operation can set extant hook hash"),
+                HSFEE, ter(tesSUCCESS));
+            env.close();
+        }
+    }
+
+    void testDelete()
+    {
+        testcase("Checks malformed delete operation");
         using namespace jtx;
         Env env{*this, supported_amendments()};
 
@@ -423,11 +411,297 @@ public:
         jv[jss::TransactionType] = jss::SetHook;
         jv[jss::Flags] = 0;
         jv[jss::Hooks] = Json::Value{Json::arrayValue};
-        // RH UPTO
+
+        // flag required
+        {
+            Json::Value iv;
+            iv[jss::CreateCode] = "";
+            jv[jss::Hooks][0U][jss::Hook] = iv;
+            env(jv,
+                M("Hook DELETE operation must include hsfOVERRIDE flag"),
+                HSFEE, ter(temMALFORMED));
+            env.close();
+        }
+
+        // invalid flags
+        {
+            Json::Value iv;
+            iv[jss::CreateCode] = "";
+            iv[jss::Flags] = "2147483648";
+            jv[jss::Hooks][0U][jss::Hook] = iv;
+            env(jv,
+                M("Hook DELETE operation must include hsfOVERRIDE flag"),
+                HSFEE, ter(temMALFORMED));
+            env.close();
+        }
+
+        // grants, parameters, hookon, hookapiversion, hooknamespace keys must be absent
+        for (auto const& [key, value]:
+            JSSMap {
+                {jss::HookGrants, Json::arrayValue},
+                {jss::HookParameters, Json::arrayValue},
+                {jss::HookOn, "0"},
+                {jss::HookApiVersion, "0"},
+                {jss::HookNamespace, to_string(uint256{beast::zero})}
+            })
+        {
+            Json::Value iv;
+            iv[jss::CreateCode] = "";
+            iv[key] = value;
+            jv[jss::Hooks][0U][jss::Hook] = iv;
+            env(jv,
+                M("Hook DELETE operation cannot include: grants, params, hookon, apiversion, namespace"),
+                HSFEE, ter(temMALFORMED));
+            env.close();
+        }
     }
 
-    void testMalformedUpdate()
+    void testNSDelete()
     {
+        testcase("Checks malformed nsdelete operation");
+        using namespace jtx;
+        Env env{*this, supported_amendments()};
+
+        auto const alice = Account{"alice"};
+        env.fund(XRP(10000), alice);
+
+        auto const bob = Account{"bob"};
+        env.fund(XRP(10000), bob);
+
+        Json::Value jv;
+        jv[jss::Account] = alice.human();
+        jv[jss::TransactionType] = jss::SetHook;
+        jv[jss::Flags] = 0;
+        jv[jss::Hooks] = Json::Value{Json::arrayValue};
+        
+        for (auto const& [key, value]:
+            JSSMap {
+                {jss::HookGrants, Json::arrayValue},
+                {jss::HookParameters, Json::arrayValue},
+                {jss::HookOn, "0"},
+                {jss::HookApiVersion, "0"},
+            })
+        {
+            Json::Value iv;
+            iv[key] = value;
+            iv[jss::Flags] = hsfNSDELETE;
+            iv[jss::HookNamespace] = to_string(uint256{beast::zero});
+            jv[jss::Hooks][0U][jss::Hook] = iv;
+            env(jv,
+                M("Hook NSDELETE operation cannot include: grants, params, hookon, apiversion"),
+                HSFEE, ter(temMALFORMED));
+            env.close();
+        }
+
+        // create a namespace
+        std::string ns = "CAFECAFECAFECAFECAFECAFECAFECAFECAFECAFECAFECAFECAFECAFECAFECAFE";
+        {
+            // create hook
+
+            Json::Value jv = 
+                ripple::test::jtx::hook(alice, {{hso(makestate_wasm)}}, 0);
+
+            jv[jss::Hooks][0U][jss::Hook][jss::HookNamespace] = ns;
+            env(jv, M("Create makestate hook"), HSFEE, ter(tesSUCCESS));
+
+            // run hook
+                 
+            env(pay(bob, alice, XRP(1)),
+                M("Run create state hook"),
+                fee(XRP(1)));
+            env.close();
+        }
+
+        // RH UPTO
+        // delete the namespace
+
+    }
+
+    void testCreate()
+    {
+        testcase("Checks malformed create operation");
+        using namespace jtx;
+        Env env{*this, supported_amendments()};
+
+        auto const bob = Account{"bob"};
+        env.fund(XRP(10000), bob);
+
+        auto const alice = Account{"alice"};
+        env.fund(XRP(10000), alice);
+
+
+        // test normal create and missing override flag
+        {
+            env(ripple::test::jtx::hook(bob, {{hso(accept_wasm)}}, 0),
+                    M("First set = tesSUCCESS"),
+                    HSFEE, ter(tesSUCCESS));
+
+            env(ripple::test::jtx::hook(bob, {{hso(accept_wasm)}}, 0),
+                    M("Second set = tecREQUIRES_FLAG"),
+                    HSFEE, ter(tecREQUIRES_FLAG));
+            env.close();
+        }
+
+        Json::Value jv;
+        jv[jss::Account] = alice.human();
+        jv[jss::TransactionType] = jss::SetHook;
+        jv[jss::Flags] = 0;
+        jv[jss::Hooks] = Json::Value{Json::arrayValue};
+
+        // payload too large
+        {
+            env(ripple::test::jtx::hook(alice, {{hso(long_wasm)}}, 0),
+                M("If CreateCode is present, then it must be less than 64kib"),
+                HSFEE, ter(temMALFORMED));
+            env.close();
+        }
+
+        // namespace missing
+        {
+            Json::Value iv;
+            iv[jss::CreateCode] = strHex(accept_wasm);
+            iv[jss::HookApiVersion] = 0U;
+            iv[jss::HookOn] = uint64_hex(0);
+            jv[jss::Hooks][0U] = Json::Value{};
+            jv[jss::Hooks][0U][jss::Hook] = iv;
+
+            env(jv,
+                M("HSO Create operation must contain namespace"),
+                HSFEE, ter(temMALFORMED));
+            env.close();
+        }
+
+        // api version missing
+        {
+            Json::Value iv;
+            iv[jss::CreateCode] = strHex(accept_wasm);
+            iv[jss::HookNamespace] = to_string(uint256{beast::zero});
+            iv[jss::HookOn] = uint64_hex(0);
+            jv[jss::Hooks][0U] = Json::Value{};
+            jv[jss::Hooks][0U][jss::Hook] = iv;
+
+            env(jv,
+                M("HSO Create operation must contain api version"),
+                HSFEE, ter(temMALFORMED));
+            env.close();
+        }
+
+        // api version wrong
+        {
+            Json::Value iv;
+            iv[jss::CreateCode] = strHex(accept_wasm);
+            iv[jss::HookNamespace] = to_string(uint256{beast::zero});
+            iv[jss::HookApiVersion] = 1U;
+            iv[jss::HookOn] = uint64_hex(0);
+            jv[jss::Hooks][0U] = Json::Value{};
+            jv[jss::Hooks][0U][jss::Hook] = iv;
+
+            env(jv,
+                M("HSO Create operation must contain valid api version"),
+                HSFEE, ter(temMALFORMED));
+            env.close();
+        }
+
+        // hookon missing
+        {
+            Json::Value iv;
+            iv[jss::CreateCode] = strHex(accept_wasm);
+            iv[jss::HookNamespace] = to_string(uint256{beast::zero});
+            iv[jss::HookApiVersion] = 0U;
+            jv[jss::Hooks][0U] = Json::Value{};
+            jv[jss::Hooks][0U][jss::Hook] = iv;
+
+            env(jv,
+                M("HSO Create operation must contain hookon"),
+                HSFEE, ter(temMALFORMED));
+            env.close();
+        }
+
+        // hook hash present
+        {
+            Json::Value jv =
+                ripple::test::jtx::hook(alice, {{hso(accept_wasm)}}, 0);
+            Json::Value iv = jv[jss::Hooks][0U];
+            iv[jss::Hook][jss::HookHash] = to_string(uint256{beast::zero});
+            jv[jss::Hooks][0U] = iv;
+            env(jv,
+                M("Cannot have both CreateCode and HookHash"),
+                HSFEE, ter(temMALFORMED));
+            env.close();
+        }
+    }
+
+    void testUpdate()
+    {
+        testcase("Checks malformed update operation");
+        using namespace jtx;
+        Env env{*this, supported_amendments()};
+
+        auto const alice = Account{"alice"};
+        env.fund(XRP(10000), alice);
+
+        Json::Value jv;
+        jv[jss::Account] = alice.human();
+        jv[jss::TransactionType] = jss::SetHook;
+        jv[jss::Flags] = 0;
+        jv[jss::Hooks] = Json::Value{Json::arrayValue};
+        
+        // first create the hook
+        {
+            Json::Value iv;
+            iv[jss::CreateCode] = strHex(accept_wasm);
+            iv[jss::HookNamespace] = to_string(uint256{beast::zero});
+            iv[jss::HookApiVersion] = 0U;
+            iv[jss::HookOn] = uint64_hex(0);
+            jv[jss::Hooks][0U] = Json::Value{};
+            jv[jss::Hooks][0U][jss::Hook] = iv;
+            env(jv,
+                M("Create accept"),
+                HSFEE, ter(tesSUCCESS));
+            env.close();
+        }
+    
+        // all alice operations below are then updates
+
+        // must not specify override flag
+        {
+            Json::Value iv;
+            iv[jss::Flags] = hsfOVERRIDE;
+            jv[jss::Hooks][0U] = Json::Value{};
+            jv[jss::Hooks][0U][jss::Hook] = iv;
+
+            env(jv,
+                M("Override flag not allowed on update"),
+                HSFEE, ter(temMALFORMED));
+            env.close();
+        }
+
+        // must not specify NSDELETE unless also Namespace
+        {
+            Json::Value iv;
+            iv[jss::Flags] = hsfNSDELETE;
+            jv[jss::Hooks][0U] = Json::Value{};
+            jv[jss::Hooks][0U][jss::Hook] = iv;
+
+            env(jv,
+                M("NSDELETE flag not allowed on update unless HookNamespace also present"),
+                HSFEE, ter(temMALFORMED));
+            env.close();
+        }
+
+
+        // api version not allowed in update
+        {
+            Json::Value iv;
+            iv[jss::HookApiVersion] = 0U;
+            jv[jss::Hooks][0U] = Json::Value{};
+            jv[jss::Hooks][0U][jss::Hook] = iv;
+
+            env(jv,
+                M("ApiVersion not allowed in update"),
+                HSFEE, ter(temMALFORMED));
+            env.close();
+        }
     }
 
 
@@ -488,7 +762,7 @@ public:
     }
 
     void
-    testMalformedWasm()
+    testWasm()
     {
         testcase("Checks malformed hook binaries");
         using namespace jtx;
@@ -558,14 +832,20 @@ public:
     {
         //testTicketSetHook();  // RH TODO
         testHooksDisabled();
-        testMalformedTxStructure();
+        testTxStructure();
         testInferHookSetOperation();
-        testMalformedDelete();
+        testParams();
-        testMalformedInstall();
+        testGrants();
-        testMalformedParams();
-        testMalformedGrants();
 
-        testMalformedWasm();
+        testDelete();
+        testInstall();
+        testCreate();
+
+        testUpdate();
+
+        testNSDelete();
+
+        testWasm();
         testAccept();
         testRollback();
     }
@@ -679,6 +959,24 @@ private:
         )[test.hook]"
     ];
 
+    TestHook
+    makestate_wasm = // WASM: 5
+    wasm[
+        R"[test.hook](
+            #include <stdint.h>
+            extern int32_t _g           (uint32_t id, uint32_t maxiter);
+            extern int64_t accept       (uint32_t read_ptr, uint32_t read_len, int64_t error_code);
+            extern int64_t state_set    (uint32_t read_ptr, uint32_t read_len, uint32_t kread_ptr, uint32_t kread_len);
+            #define SBUF(x) x, sizeof(x)
+            int64_t hook(uint32_t reserved )
+            {
+                _g(1,1);
+                uint8_t test_key[] = "key";
+                uint8_t test_value[] = "value";
+                return accept(0,0, state_set(SBUF(test_value), SBUF(test_key)));
+            }
+        )[test.hook]"
+    ];
 
 };
 BEAST_DEFINE_TESTSUITE(SetHook, tx, ripple);

src/test/jtx/hook.h

@@ -34,6 +34,8 @@ hook(Account const& account, std::optional<std::vector<Json::Value>> hooks, std:
 Json::Value
 hso(std::vector<uint8_t> const& wasmBytes, uint64_t hookOn = 0, uint256 ns = beast::zero, uint8_t apiversion = 0);
 
+std::string uint64_hex(uint64_t x);
+
 }  // namespace jtx
 }  // namespace test
 }  // namespace ripple

src/test/jtx/impl/Env.cpp

@@ -349,7 +349,7 @@ Env::postconditions(JTx const& jt, TER ter, bool didApply)
         !test.expect(
             ter == *jt.ter,
             "apply: Got " + transToken(ter) + " (" + transHuman(ter) +
-                "); Expected " + transToken(*jt.ter) + " (" +
+                ") [=" + std::to_string(ter.code_) + "]; Expected " + transToken(*jt.ter) + " (" +
                 transHuman(*jt.ter) + ")"))
     {
         test.log << pretty(jt.jv) << std::endl;

src/test/jtx/impl/hook.cpp

@@ -45,7 +45,7 @@ hook(Account const& account, std::optional<std::vector<Json::Value>> hooks, std:
 }
 
 
-inline std::string uint64_hex(uint64_t x)
+std::string uint64_hex(uint64_t x)
 {
     auto const nibble = [](uint64_t n) -> char
     {