From 7f72ef014be8a4eff833791f4f2f84444fdb966d Mon Sep 17 00:00:00 2001 From: Richard Holland Date: Tue, 29 Nov 2022 11:25:28 +0000 Subject: [PATCH] slot_set bug fixes, slot_set test --- src/ripple/app/hook/applyHook.h | 12 +-- src/ripple/app/hook/impl/applyHook.cpp | 24 +++--- src/test/app/SetHook_test.cpp | 105 +++++++++++++++++++++++++ 3 files changed, 123 insertions(+), 18 deletions(-) diff --git a/src/ripple/app/hook/applyHook.h b/src/ripple/app/hook/applyHook.h index 97b1bc5a7..ac64ce8c8 100644 --- a/src/ripple/app/hook/applyHook.h +++ b/src/ripple/app/hook/applyHook.h @@ -167,7 +167,7 @@ namespace hook_api DECLARE_HOOK_FUNCTION(int64_t, slot, uint32_t write_ptr, uint32_t write_len, uint32_t slot ); DECLARE_HOOK_FUNCTION(int64_t, slot_clear, uint32_t slot ); DECLARE_HOOK_FUNCTION(int64_t, slot_count, uint32_t slot ); - DECLARE_HOOK_FUNCTION(int64_t, slot_set, uint32_t read_ptr, uint32_t read_len, int32_t slot ); + DECLARE_HOOK_FUNCTION(int64_t, slot_set, uint32_t read_ptr, uint32_t read_len, uint32_t slot ); DECLARE_HOOK_FUNCTION(int64_t, slot_size, uint32_t slot ); DECLARE_HOOK_FUNCTION(int64_t, slot_subarray, uint32_t parent_slot, uint32_t array_id, uint32_t new_slot ); DECLARE_HOOK_FUNCTION(int64_t, slot_subfield, uint32_t parent_slot, uint32_t field_id, uint32_t new_slot ); @@ -323,12 +323,12 @@ namespace hook // the map stores pairs consisting of a memory view and whatever shared or unique ptr is required to // keep the underlying object alive for the duration of the hook's execution // slot number -> { keylet or hash, { pointer to current object, storage for that object } } - std::map slot {}; - uint8_t slot_counter { 1 }; - std::queue slot_free {}; + std::map slot {}; + std::queue slot_free {}; + uint32_t slot_counter { 0 }; // uint16 to avoid accidental overflow and to allow more slots in future + uint16_t emit_nonce_counter { 0 }; // incremented whenever nonce is called to ensure unique nonces + uint16_t ledger_nonce_counter { 0 }; int64_t expected_etxn_count { -1 }; // make this a 64bit int so the uint32 from the hookapi cant overflow it - uint8_t emit_nonce_counter { 0 }; // incremented whenever nonce is called to ensure unique nonces - uint8_t ledger_nonce_counter { 0 }; std::map nonce_used {}; uint32_t generation = 0; // used for caching, only generated when txn_generation is called uint64_t burden = 0; // used for caching, only generated when txn_burden is called diff --git a/src/ripple/app/hook/impl/applyHook.cpp b/src/ripple/app/hook/impl/applyHook.cpp index a30bab241..8dcb00a76 100644 --- a/src/ripple/app/hook/impl/applyHook.cpp +++ b/src/ripple/app/hook/impl/applyHook.cpp @@ -542,7 +542,7 @@ get_free_slot(hook::HookContext& hookCtx) // no slots were available in the queue so increment slot counter if (slot_into == 0) - slot_into = hookCtx.slot_counter++; + slot_into = ++hookCtx.slot_counter; return slot_into; } @@ -1800,7 +1800,7 @@ DEFINE_HOOK_FUNCTION( ? applyCtx.tx.getFieldH256(sfTransactionHash) : applyCtx.tx.getTransactionID(); - hookCtx.slot.emplace( std::pair { slot_into, hook::SlotEntry { + hookCtx.slot.emplace( std::pair { slot_into, hook::SlotEntry { .storage = st_tx, .entry = 0 }}); @@ -2088,13 +2088,13 @@ DEFINE_HOOK_FUNCTION( int64_t, slot_set, uint32_t read_ptr, uint32_t read_len, // readptr is a keylet - int32_t slot_into /* providing 0 allocates a slot to you */ ) + uint32_t slot_into /* providing 0 allocates a slot to you */ ) { HOOK_SETUP(); // populates memory_ctx, memory, memory_length, applyCtx, hookCtx on current stack if (NOT_IN_BOUNDS(read_ptr, read_len, memory_length)) return OUT_OF_BOUNDS; - if ((read_len != 32 && read_len != 34) || slot_into < 0 || slot_into > hook_api::max_slots) + if ((read_len != 32 && read_len != 34) || slot_into > hook_api::max_slots) return INVALID_ARGUMENT; // check if we can emplace the object to a slot @@ -2110,7 +2110,10 @@ DEFINE_HOOK_FUNCTION( if (!kl) return DOESNT_EXIST; - auto sle = applyCtx.view().peek(*kl); + if (kl->key == beast::zero) + return DOESNT_EXIST; + + auto const sle = applyCtx.view().read(*kl); if (!sle) return DOESNT_EXIST; @@ -2119,9 +2122,7 @@ DEFINE_HOOK_FUNCTION( else if (read_len == 32) { - uint256 hash; - if (!hash.parseHex((const char*)(memory + read_ptr))) - return INVALID_ARGUMENT; + uint256 hash = ripple::base_uint<256>::fromVoid(memory + read_ptr); ripple::error_code_i ec { ripple::error_code_i::rpcUNKNOWN }; @@ -2142,11 +2143,10 @@ DEFINE_HOOK_FUNCTION( if (slot_into == 0) slot_into = get_free_slot(hookCtx); - - hookCtx.slot.emplace( std::pair { slot_into, hook::SlotEntry { + hookCtx.slot[slot_into] = hook::SlotEntry { .storage = *slot_value, .entry = 0 - }}); + }; hookCtx.slot[slot_into].entry = &(*hookCtx.slot[slot_into].storage); return slot_into; @@ -4920,7 +4920,7 @@ DEFINE_HOOK_FUNCTION( if (slot_into == 0) slot_into = get_free_slot(hookCtx); - hookCtx.slot.emplace( std::pair { slot_into, hook::SlotEntry { + hookCtx.slot.emplace( std::pair { slot_into, hook::SlotEntry { .storage = hookCtx.result.provisionalMeta, .entry = 0 }}); diff --git a/src/test/app/SetHook_test.cpp b/src/test/app/SetHook_test.cpp index 43584ceb0..73e442ee0 100644 --- a/src/test/app/SetHook_test.cpp +++ b/src/test/app/SetHook_test.cpp @@ -4370,6 +4370,111 @@ public: void test_slot_set() { + testcase("Test slot_set"); + using namespace jtx; + Env env{*this, supported_amendments()}; + + Account const alice{"alice"}; + Account const bob{"bob"}; + env.fund(XRP(10000), alice); + env.fund(XRP(10000), bob); + + TestHook hook = wasm[R"[test.hook]( + #include + extern int32_t _g (uint32_t id, uint32_t maxiter); + #define GUARD(maxiter) _g((1ULL << 31U) + __LINE__, (maxiter)+1) + extern int64_t accept (uint32_t read_ptr, uint32_t read_len, int64_t error_code); + extern int64_t rollback (uint32_t read_ptr, uint32_t read_len, int64_t error_code); + extern int64_t otxn_id(uint32_t, uint32_t, uint32_t); + extern int64_t slot_set(uint32_t, uint32_t, uint32_t); + extern int64_t slot_size(uint32_t); + #define sfBalance ((6U << 16U) + 2U) + #define sfFlags ((2U << 16U) + 2U) + #define DOESNT_EXIST -5 + #define OUT_OF_BOUNDS -1 + #define INVALID_ARGUMENT -7 + #define NO_FREE_SLOTS -6 + #define ASSERT(x)\ + if (!(x))\ + rollback((uint32_t)#x, sizeof(#x), __LINE__); + #define SBUF(x) (uint32_t)(x), sizeof(x) + + // skip keylet + uint8_t kl_sk[] = + { + 0x00U, 0x68U, + 0xB4U,0x97U,0x9AU,0x36U,0xCDU,0xC7U,0xF3U,0xD3U,0xD5U,0xC3U, + 0x1AU,0x4EU,0xAEU,0x2AU,0xC7U,0xD7U,0x20U,0x9DU,0xDAU,0x87U, + 0x75U,0x88U,0xB9U,0xAFU,0xC6U,0x67U,0x99U,0x69U,0x2AU,0xB0U, + 0xD6U,0x6BU + }; + + int64_t hook(uint32_t reserved ) + { + _g(1,1); + + // bounds check + ASSERT(slot_set(1, 1000000, 0) == OUT_OF_BOUNDS); + ASSERT(slot_set(1000000, 1024, 0) == OUT_OF_BOUNDS); + + // read len is only allowed to be 32 (txn id) or 34 (keylet) + uint8_t kl_zero[34]; + ASSERT(slot_set((uint32_t)kl_zero, 0, 0) == INVALID_ARGUMENT); + ASSERT(slot_set((uint32_t)kl_zero, 31, 0) == INVALID_ARGUMENT); + ASSERT(slot_set((uint32_t)kl_zero, 33, 0) == INVALID_ARGUMENT); + ASSERT(slot_set((uint32_t)kl_zero, 35, 0) == INVALID_ARGUMENT); + ASSERT(slot_set((uint32_t)kl_zero, 34, 256) == INVALID_ARGUMENT); + + // request an invalid keylet + ASSERT(slot_set(SBUF(kl_zero), 0) == DOESNT_EXIST); + kl_zero[0] = 1; + ASSERT(slot_set(SBUF(kl_zero), 0) == DOESNT_EXIST); + + ASSERT(slot_size(1) == DOESNT_EXIST); + // request a valid keylet + ASSERT(slot_set(SBUF(kl_sk), 0) > 0); + ASSERT(slot_size(1) > 0); + + // fill up all slots + for (uint32_t i = 1; GUARD(257), i < 256; ++i) + ASSERT(slot_set(SBUF(kl_sk), 0) > 0); + + // request a final slot that should fail + ASSERT(slot_set(SBUF(kl_sk), 0)); + + // overwrite an existing slot, should work + ASSERT(slot_set(SBUF(kl_sk), 10) == 10); + + // check all the slots contain an object, the same object + uint32_t s = slot_size(1); + + for (uint32_t i = 2; GUARD(257), i < 256; ++i) + ASSERT(s == slot_size(i)); + + // slot a txn + + uint8_t txn[32]; + ASSERT(otxn_id(SBUF(txn), 0) == 32); + ASSERT(slot_set(SBUF(txn), 1) == 1); + + uint32_t s2 = slot_size(1); + + // ensure it's not the same object that was there before + ASSERT(s != s2 && s2 > 0); + + // done! + accept(0,0,0); + } + )[test.hook]"]; + + // install the hook on alice + env(ripple::test::jtx::hook(alice, {{hso(hook, overrideFlag)}}, 0), + M("set slot_set"), + HSFEE); + env.close(); + + // invoke the hook + env(pay(bob, alice, XRP(1)), M("test slot_set"), fee(XRP(1))); } void