diff --git a/src/ripple/app/tx/applyHookMacro.h b/src/ripple/app/tx/applyHookMacro.h index 97f5b1b1f..04c01f64d 100644 --- a/src/ripple/app/tx/applyHookMacro.h +++ b/src/ripple/app/tx/applyHookMacro.h @@ -210,8 +210,8 @@ } // ptr = pointer inside the wasm memory space #define NOT_IN_BOUNDS(ptr, len, memory_length)\ - (ptr > memory_length || \ - static_cast(ptr) + static_cast(len) > static_cast(memory_length)) + ((static_cast(ptr) > static_cast(memory_length)) || \ + ((static_cast(ptr) + static_cast(len)) > static_cast(memory_length))) #define HOOK_EXIT(read_ptr, read_len, error_code, exit_type)\ {\ diff --git a/src/ripple/app/tx/impl/SetHook.cpp b/src/ripple/app/tx/impl/SetHook.cpp index 6f4542c7d..78edc2d44 100644 --- a/src/ripple/app/tx/impl/SetHook.cpp +++ b/src/ripple/app/tx/impl/SetHook.cpp @@ -1505,7 +1505,8 @@ updateHookParameters( newParameters.push_back(std::move(param)); } - newHook.setFieldArray(sfHookParameters, std::move(newParameters)); + if (newParameters.size() > 0) + newHook.setFieldArray(sfHookParameters, std::move(newParameters)); return tesSUCCESS; } @@ -1604,6 +1605,9 @@ SetHook::setHook() uint32_t flags = hookSetObj->isFieldPresent(sfFlags) ? hookSetObj->getFieldU32(sfFlags) : 0; + HookSetOperation op = inferOperation(*hookSetObj); + + // if an existing hook exists at this position in the chain then extract the relevant fields if (oldHook && oldHook->get().isFieldPresent(sfHookHash)) { @@ -1628,7 +1632,6 @@ SetHook::setHook() oldHookOn = *defHookOn; } - // in preparation for three way merge populate fields if they are present on the HookSetObj if (hookSetObj->isFieldPresent(sfHookHash)) { @@ -1645,7 +1648,6 @@ SetHook::setHook() newDirKeylet = keylet::hookStateDir(account_, *newNamespace); } - HookSetOperation op = inferOperation(*hookSetObj); // users may destroy a namespace in any operation except NOOP and INVALID if (op != hsoNOOP && op != hsoINVALID && (flags & hsoNSDELETE) && oldDirSLE) @@ -1702,8 +1704,13 @@ SetHook::setHook() } // decrement the hook definition and mark it for deletion if appropriate - if (oldDefSLE && reduceReferenceCount(oldDefSLE)) - defsToDestroy.emplace(*oldDefKeylet); + if (oldDefSLE) + { + if (reduceReferenceCount(oldDefSLE)) + defsToDestroy.emplace(*oldDefKeylet); + + view().update(oldDefSLE); + } continue; } @@ -1743,6 +1750,7 @@ SetHook::setHook() << "]: SetHook create operation would override but hsfOVERRIDE flag wasn't specified"; return tecREQUIRES_FLAG; } + ripple::Blob wasmBytes = hookSetObj->getFieldVL(sfCreateCode); @@ -1766,6 +1774,7 @@ SetHook::setHook() // update reference count newDefSLE = view().peek(keylet); newDefKeylet = keylet; + // this falls through to hsoINSTALL } else @@ -1795,6 +1804,15 @@ SetHook::setHook() << "]: Malformed transaction: SetHook operation would create invalid hook wasm"; return tecINTERNAL; } + + // decrement the hook definition and mark it for deletion if appropriate + if (oldDefSLE) + { + if (reduceReferenceCount(oldDefSLE)) + defsToDestroy.emplace(*oldDefKeylet); + + view().update(oldDefSLE); + } auto newHookDef = std::make_shared( keylet ); newHookDef->setFieldH256(sfHookHash, *createHookHash); @@ -1838,10 +1856,15 @@ SetHook::setHook() return tecNO_ENTRY; } - // decrement references of old hook definition - if (oldHook && reduceReferenceCount(oldDefSLE)) + // decrement the hook definition and mark it for deletion if appropriate + if (oldDefSLE) + { + if (reduceReferenceCount(oldDefSLE)) defsToDestroy.emplace(*oldDefKeylet); + view().update(oldDefSLE); + } + // set the hookhash on the new hook, and allow for a fall through event from hsoCREATE if (!createHookHash) createHookHash = hookSetObj->getFieldH256(sfHookHash); @@ -1851,6 +1874,10 @@ SetHook::setHook() // increment reference count of target HookDefintion incrementReferenceCount(newDefSLE); + // change which definition we're using to the new target + defNamespace = newDefSLE->getFieldH256(sfHookNamespace); + defHookOn = newDefSLE->getFieldU64(sfHookOn); + // set the namespace if it differs from the definition namespace if (newNamespace && *defNamespace != *newNamespace) newHook.setFieldH256(sfHookNamespace, *newNamespace); @@ -1861,7 +1888,7 @@ SetHook::setHook() // parameters TER result = - updateHookParameters(ctx, *hookSetObj, oldDefSLE, newHook); + updateHookParameters(ctx, *hookSetObj, newDefSLE, newHook); if (result != tesSUCCESS) return result; @@ -1871,6 +1898,9 @@ SetHook::setHook() newHook.setFieldArray(sfHookGrants, hookSetObj->getFieldArray(sfHookGrants)); newHooks.push_back(std::move(newHook)); + + view().update(newDefSLE); + continue; } diff --git a/src/ripple/app/tx/impl/applyHook.cpp b/src/ripple/app/tx/impl/applyHook.cpp index 2931f4767..69c867677 100644 --- a/src/ripple/app/tx/impl/applyHook.cpp +++ b/src/ripple/app/tx/impl/applyHook.cpp @@ -327,12 +327,14 @@ hook::setHookState( auto& view = applyCtx.view(); auto j = applyCtx.app.journal("View"); - auto const sle = + auto const sleAccount = ( acc == hookResult.account ? view.peek(hookResult.accountKeylet) : view.peek(ripple::keylet::account(acc))); - if (!sle) + JLOG(j.trace()) << "HookState[" << acc << "]: hookResult.account = " << hookResult.account; + + if (!sleAccount) return tefINTERNAL; // if the blob is too large don't set it @@ -342,10 +344,12 @@ hook::setHookState( auto hookStateKeylet = ripple::keylet::hookState(acc, key, ns); auto hookStateDirKeylet = ripple::keylet::hookStateDir(acc, ns); - uint32_t stateCount = sle->getFieldU32(sfHookStateCount); + uint32_t stateCount = sleAccount->getFieldU32(sfHookStateCount); uint32_t oldStateReserve = COMPUTE_HOOK_DATA_OWNER_COUNT(stateCount); - auto const oldHookState = view.peek(hookStateKeylet); + auto hookState = view.peek(hookStateKeylet); + + bool createNew = !hookState; // if the blob is nil then delete the entry if it exists if (data.size() == 0) @@ -354,14 +358,17 @@ hook::setHookState( if (!view.peek(hookStateKeylet)) return tesSUCCESS; // a request to remove a non-existent entry is defined as success - auto const hint = (*oldHookState)[sfOwnerNode]; + if (!view.peek(hookStateDirKeylet)) + return tefBAD_LEDGER; + + auto const hint = (*hookState)[sfOwnerNode]; // Remove the node from the namespace directory if (!view.dirRemove(hookStateDirKeylet, hint, hookStateKeylet.key, false)) return tefBAD_LEDGER; // remove the actual hook state obj - view.erase(oldHookState); + view.erase(hookState); // adjust state object count if (stateCount > 0) @@ -369,23 +376,23 @@ hook::setHookState( // if removing this state entry would destroy the allotment then reduce the owner count if (COMPUTE_HOOK_DATA_OWNER_COUNT(stateCount) < oldStateReserve) - adjustOwnerCount(view, sle, -1, j); + adjustOwnerCount(view, sleAccount, -1, j); - sle->setFieldU32(sfHookStateCount, stateCount); - view.update(sle); + sleAccount->setFieldU32(sfHookStateCount, stateCount); + view.update(sleAccount); return tesSUCCESS; } + + std::uint32_t ownerCount{(*sleAccount)[sfOwnerCount]}; - std::uint32_t ownerCount{(*sle)[sfOwnerCount]}; - - if (oldHookState) { - view.erase(oldHookState); - } else { + if (createNew) + { ++stateCount; - if (COMPUTE_HOOK_DATA_OWNER_COUNT(stateCount) > oldStateReserve) { + if (COMPUTE_HOOK_DATA_OWNER_COUNT(stateCount) > oldStateReserve) + { // the hook used its allocated allotment of state entries for its previous ownercount // increment ownercount and give it another allotment @@ -393,43 +400,45 @@ hook::setHookState( XRPAmount const newReserve{ view.fees().accountReserve(ownerCount)}; - if (STAmount((*sle)[sfBalance]).xrp() < newReserve) + if (STAmount((*sleAccount)[sfBalance]).xrp() < newReserve) return tecINSUFFICIENT_RESERVE; - adjustOwnerCount(view, sle, 1, j); + adjustOwnerCount(view, sleAccount, 1, j); } // update state count - sle->setFieldU32(sfHookStateCount, stateCount); - view.update(sle); + sleAccount->setFieldU32(sfHookStateCount, stateCount); + view.update(sleAccount); + + // create an entry + hookState = std::make_shared(hookStateKeylet); + } + - // add new data to ledger - auto newHookState = std::make_shared(hookStateKeylet); - view.insert(newHookState); - newHookState->setFieldVL(sfHookStateData, data); - newHookState->setFieldH256(sfHookStateKey, key); - newHookState->setAccountID(sfAccount, acc); // RH TODO remove these - newHookState->setFieldH256(sfHookNamespace, ns); // in prod + hookState->setFieldVL(sfHookStateData, data); + hookState->setFieldH256(sfHookStateKey, key); - if (!oldHookState) { + if (createNew) + { // Add the hook to the account's directory if it wasn't there already auto const page = view.dirInsert( hookStateDirKeylet, hookStateKeylet.key, describeOwnerDir(acc)); - JLOG(j.trace()) << "HookInfo[" << HR_ACC() << "]: " - << "Create/update hook state: " - << (page ? "success" : "failure"); - if (!page) return tecDIR_FULL; - - newHookState->setFieldU64(sfOwnerNode, *page); + + hookState->setFieldU64(sfOwnerNode, *page); + + // add new data to ledger + view.insert(hookState); } + else + view.update(hookState); return tesSUCCESS; } @@ -751,9 +760,9 @@ DEFINE_HOOK_FUNCTION( if (nread_ptr == 0 && nread_len == 0 && !(aread_ptr == 0 && aread_len == 0)) return INVALID_ARGUMENT; - if ((nread_len && !NOT_IN_BOUNDS(nread_ptr, nread_len, memory_length)) || - (kread_len && !NOT_IN_BOUNDS(kread_ptr, kread_len, memory_length)) || - (aread_len && !NOT_IN_BOUNDS(aread_ptr, aread_len, memory_length))) + if ((nread_len && NOT_IN_BOUNDS(nread_ptr, nread_len, memory_length)) || + (kread_len && NOT_IN_BOUNDS(kread_ptr, kread_len, memory_length)) || + (aread_len && NOT_IN_BOUNDS(aread_ptr, aread_len, memory_length))) return OUT_OF_BOUNDS; uint32_t maxSize = hook::maxHookStateDataSize(); @@ -766,7 +775,7 @@ DEFINE_HOOK_FUNCTION( : ripple::base_uint<256>::fromVoid(memory + nread_ptr); ripple::AccountID acc = - aread_len == 0 + aread_len == 20 ? AccountID::fromVoid(memory + aread_ptr) : hookCtx.result.account; @@ -905,7 +914,17 @@ void hook::commitChangesToLedger( // this entry isn't just cached, it was actually modified auto slice = Slice(blob.data(), blob.size()); - setHookState(hookResult, applyCtx, acc, ns, key, slice); + TER result = + setHookState(hookResult, applyCtx, acc, ns, key, slice); + + if (result != tesSUCCESS) + { + JLOG(j.warn()) + << "HookError[" << HR_ACC() << "]: SetHookState failed: " << result + << " Key: " << key + << " Value: " << slice; + + } // ^ should not fail... checks were done before map insert } } @@ -3239,7 +3258,7 @@ DEFINE_HOOK_FUNCTION( { JLOG(j.trace()) << "HookInfo[" << HC_ACC() << "]: Guard violation. " - << "Src line: " << id + << "Src line: " << id << " " << "Iterations: " << hookCtx.guard_map[id]; } hookCtx.result.exitType = hook_api::ExitType::ROLLBACK; diff --git a/src/ripple/protocol/impl/LedgerFormats.cpp b/src/ripple/protocol/impl/LedgerFormats.cpp index 45f4ab1d3..d4bd2ea56 100644 --- a/src/ripple/protocol/impl/LedgerFormats.cpp +++ b/src/ripple/protocol/impl/LedgerFormats.cpp @@ -67,8 +67,6 @@ LedgerFormats::LedgerFormats() {sfTakerGetsCurrency, soeOPTIONAL}, // for order book directories {sfTakerGetsIssuer, soeOPTIONAL}, // for order book directories {sfExchangeRate, soeOPTIONAL}, // for order book directories - {sfHookNamespace, soeOPTIONAL}, // for hook state directories - {sfOwnerNode, soeOPTIONAL}, // for hook state directories {sfReferenceCount, soeOPTIONAL}, // for hook state directories {sfIndexes, soeREQUIRED}, {sfRootIndex, soeREQUIRED}, @@ -210,11 +208,9 @@ LedgerFormats::LedgerFormats() add(jss::HookState, ltHOOK_STATE, { - {sfAccount, soeOPTIONAL}, //RH TODO: this can be removed for production {sfOwnerNode, soeREQUIRED}, {sfHookStateKey, soeREQUIRED}, {sfHookStateData, soeREQUIRED}, - {sfHookNamespace, soeOPTIONAL} // as can this }, commonFields);