From 3439888aefb642a915023fb62b734101070d8dd8 Mon Sep 17 00:00:00 2001 From: Richard Holland Date: Tue, 19 Apr 2022 12:04:27 +0000 Subject: [PATCH] tsh collect call, compiling not tested --- src/ripple/app/tx/applyHook.h | 20 +- src/ripple/app/tx/impl/SetAccount.cpp | 18 ++ src/ripple/app/tx/impl/SetHook.cpp | 231 +++++++++++------- src/ripple/app/tx/impl/Transactor.cpp | 127 ++++++++-- src/ripple/app/tx/impl/applyHook.cpp | 3 - src/ripple/protocol/LedgerFormats.h | 7 +- src/ripple/protocol/SField.h | 1 + src/ripple/protocol/TxFlags.h | 1 + .../protocol/impl/InnerObjectFormats.cpp | 2 +- src/ripple/protocol/impl/LedgerFormats.cpp | 3 +- src/ripple/protocol/impl/SField.cpp | 1 + 11 files changed, 275 insertions(+), 139 deletions(-) diff --git a/src/ripple/app/tx/applyHook.h b/src/ripple/app/tx/applyHook.h index 60680448a..d03102aeb 100644 --- a/src/ripple/app/tx/applyHook.h +++ b/src/ripple/app/tx/applyHook.h @@ -38,14 +38,18 @@ namespace hook struct HookResult; bool isEmittedTxn(ripple::STTx const& tx); - - // this map type acts as both a read and write cache for hook execution - using HookStateMap = std::map>>>; // the value + // This map type acts as both a read and write cache for hook execution + // and is preserved across the execution of the set of hook chains + // being executed in the current transaction. It is committed to lgr + // only upon tesSuccess for the otxn. + using HookStateMap = + std::map< + ripple::AccountID, // account that owns the state + std::map>>>; // the value enum TSHFlags : uint8_t { diff --git a/src/ripple/app/tx/impl/SetAccount.cpp b/src/ripple/app/tx/impl/SetAccount.cpp index c965457fd..48a2c5d23 100644 --- a/src/ripple/app/tx/impl/SetAccount.cpp +++ b/src/ripple/app/tx/impl/SetAccount.cpp @@ -403,6 +403,24 @@ SetAccount::doApply() } } + // + // TshCollect + // + if (view().rules().enabled(featureHooks)) + { + if (uSetFlag == asfTshCollect) + { + JLOG(j_.trace()) << "Set lsfTshCollect."; + uFlagsOut |= lsfTshCollect; + } + else if (uClearFlag == asfTshCollect) + { + JLOG(j_.trace()) << "Clear lsfTshCollect."; + uFlagsOut &= ~lsfTshCollect; + } + } + + // // EmailHash // diff --git a/src/ripple/app/tx/impl/SetHook.cpp b/src/ripple/app/tx/impl/SetHook.cpp index aed41dfc0..8fb98cbd9 100644 --- a/src/ripple/app/tx/impl/SetHook.cpp +++ b/src/ripple/app/tx/impl/SetHook.cpp @@ -42,6 +42,7 @@ #include #include #include +#include #define DEBUG_GUARD_CHECK 1 #define HS_ACC() ctx.tx.getAccountID(sfAccount) << "-" << ctx.tx.getTransactionID() @@ -87,15 +88,15 @@ parseLeb128(std::vector& buf, int start_offset, int* end_offset) JLOG(ctx.j.trace())\ << "HookSet(" << hook::log::SHORT_HOOK << ")[" << HS_ACC() << "]: "\ << "Malformed transaction: Hook truncated or otherwise invalid\n";\ - return {false, 0};\ + return {};\ }\ } // checks the WASM binary for the appropriate required _g guard calls and rejects it if they are not found // start_offset is where the codesection or expr under analysis begins and end_offset is where it ends -// returns {valid, worst case instruction count} +// returns {worst case instruction count} if valid or {} if invalid // may throw overflow_error -std::pair +std::optional check_guard( SetHookCtx& ctx, ripple::Blob& hook, int codesec, @@ -149,7 +150,7 @@ check_guard( << "HookSet(" << hook::log::CALL_ILLEGAL << ")[" << HS_ACC() << "]: GuardCheck " << "Hook calls a function outside of the whitelisted imports " << "codesec: " << codesec << " hook byte offset: " << i; - return {false, 0}; + return {}; } if (callee_idx == guard_func_idx) @@ -164,7 +165,7 @@ check_guard( << "HookSet(" << hook::log::GUARD_PARAMETERS << ")[" << HS_ACC() << "]: GuardCheck " << "_g() called but could not detect constant parameters " << "codesec: " << codesec << " hook byte offset: " << i << "\n"; - return {false, 0}; + return {}; } uint64_t a = stack.top(); @@ -182,7 +183,7 @@ check_guard( << "[" << HS_ACC() << "]: GuardCheck " << "_g() called but could not detect constant parameters " << "codesec: " << codesec << " hook byte offset: " << i << "\n"; - return {false, 0}; + return {}; } // update the instruction count for this block depth to the largest possible guard @@ -213,7 +214,7 @@ check_guard( JLOG(ctx.j.trace()) << "HookSet(" << hook::log::CALL_INDIRECT << ")[" << HS_ACC() << "]: GuardCheck " << "Call indirect detected and is disallowed in hooks " << "codesec: " << codesec << " hook byte offset: " << i; - return {false, 0}; + return {}; /* if (DEBUG_GUARD_CHECK) printf("%d - call_indirect instruction at %d\n", mode, i); @@ -240,7 +241,7 @@ check_guard( << "[" << HS_ACC() << "]: GuardCheck " << "_g() did not occur at start of function or loop statement " << "codesec: " << codesec << " hook byte offset: " << i; - return {false, 0}; + return {}; } // execution to here means we are in 'search mode' for loop instructions @@ -384,7 +385,7 @@ check_guard( << "HookSet(" << hook::log::MEMORY_GROW << ")[" << HS_ACC() << "]: GuardCheck " << "Memory.grow instruction not allowed at " << "codesec: " << codesec << " hook byte offset: " << i << "\n"; - return {false, 0}; + return {}; } continue; } @@ -454,7 +455,7 @@ check_guard( << "HookSet(" << hook::log::BLOCK_ILLEGAL << ")[" << HS_ACC() << "]: GuardCheck " << "Unexpected 0x0B instruction, malformed" << "codesec: " << codesec << " hook byte offset: " << i; - return {false, 0}; + return {}; } // perform the instruction count * guard accounting @@ -475,19 +476,18 @@ check_guard( << "HookSet(" << hook::log::INSTRUCTION_EXCESS << ")[" << HS_ACC() << "]: GuardCheck " << "Maximum possible instructions exceed 1048575, please make your hook smaller " << "or check your guards!"; - return {false, 0}; + return {}; } // if we reach the end of the code looking for another trigger the guards are installed correctly if (mode == 1) - return {true, instruction_count[0].second}; + return instruction_count[0].second; JLOG(ctx.j.trace()) << "HookSet(" << hook::log::GUARD_MISSING << ")[" << HS_ACC() << "]: GuardCheck " << "Guard did not occur before end of loop / function. " << "Codesec: " << codesec; - return {false, 0}; - + return {}; } bool @@ -611,14 +611,20 @@ HookSetOperation inferOperation(STObject const& hookSetObj) // may throw overflow_error -std::pair +std::optional< // unpopulated means invalid +std::pair< + uint64_t, // max instruction count for hook() + uint64_t // max instruction count for cbak() +>> validateCreateCode(SetHookCtx& ctx, STObject const& hookSetObj) { if (!hookSetObj.isFieldPresent(sfCreateCode)) - return { false, 0 }; + return {}; + + uint64_t maxInstrCountHook = 0; + uint64_t maxInstrCountCbak = 0; - uint64_t maxInstrCount = 0; Blob hook = hookSetObj.getFieldVL(sfCreateCode); uint64_t byteCount = hook.size(); @@ -628,7 +634,7 @@ validateCreateCode(SetHookCtx& ctx, STObject const& hookSetObj) JLOG(ctx.j.trace()) << "HookSet(" << hook::log::WASM_TOO_SMALL << ")[" << HS_ACC() << "]: " << "Malformed transaction: Hook was not valid webassembly binary. Too small."; - return {false, 0}; + return {}; } // check header, magic number @@ -641,7 +647,7 @@ validateCreateCode(SetHookCtx& ctx, STObject const& hookSetObj) << "HookSet(" << hook::log::WASM_BAD_MAGIC << ")[" << HS_ACC() << "]: " << "Malformed transaction: Hook was not valid webassembly binary. " << "Missing magic number or version."; - return {false, 0}; + return {}; } } @@ -669,7 +675,7 @@ validateCreateCode(SetHookCtx& ctx, STObject const& hookSetObj) JLOG(ctx.j.trace()) << "HookSet(" << hook::log::WASM_PARSE_LOOP << ")[" << HS_ACC() << "]: Malformed transaction: Hook is invalid WASM binary."; - return {false, 0}; + return {}; } j = i; @@ -695,7 +701,7 @@ validateCreateCode(SetHookCtx& ctx, STObject const& hookSetObj) << "HookSet(" << hook::log::IMPORTS_MISSING << ")[" << HS_ACC() << "]: Malformed transaction. " << "Hook did not import any functions... " << "required at least guard(uint32_t, uint32_t) and accept or rollback"; - return {false, 0}; + return {}; } // process each import one by one @@ -709,7 +715,7 @@ validateCreateCode(SetHookCtx& ctx, STObject const& hookSetObj) JLOG(ctx.j.trace()) << "HookSet(" << hook::log::IMPORT_MODULE_BAD << ")[" << HS_ACC() << "]: Malformed transaction. " << "Hook attempted to specify nil or invalid import module"; - return {false, 0}; + return {}; } if (std::string_view( (const char*)(hook.data() + i), (size_t)mod_length ) != "env") @@ -717,7 +723,7 @@ validateCreateCode(SetHookCtx& ctx, STObject const& hookSetObj) JLOG(ctx.j.trace()) << "HookSet(" << hook::log::IMPORT_MODULE_ENV << ")[" << HS_ACC() << "]: Malformed transaction. " << "Hook attempted to specify import module other than 'env'"; - return {false, 0}; + return {}; } i += mod_length; CHECK_SHORT_HOOK(); @@ -730,7 +736,7 @@ validateCreateCode(SetHookCtx& ctx, STObject const& hookSetObj) << "HookSet(" << hook::log::IMPORT_NAME_BAD << ")[" << HS_ACC() << "]: Malformed transaction. " << "Hook attempted to specify nil or invalid import name"; - return {false, 0}; + return {}; } std::string import_name { (const char*)(hook.data() + i), (size_t)name_length }; @@ -763,7 +769,7 @@ validateCreateCode(SetHookCtx& ctx, STObject const& hookSetObj) << HS_ACC() << "]: Malformed transaction. " << "Hook attempted to import a function that does not " << "appear in the hook_api function set: `" << import_name << "`"; - return {false, 0}; + return {}; } func_upto++; } @@ -773,7 +779,7 @@ validateCreateCode(SetHookCtx& ctx, STObject const& hookSetObj) JLOG(ctx.j.trace()) << "HookSet(" << hook::log::GUARD_IMPORT << ")[" << HS_ACC() << "]: Malformed transaction. " << "Hook did not import _g (guard) function"; - return {false, 0}; + return {}; } last_import_number = func_upto - 1; @@ -796,7 +802,7 @@ validateCreateCode(SetHookCtx& ctx, STObject const& hookSetObj) << HS_ACC() << "]: Malformed transaction. " << "Hook did not export any functions... " << "required hook(int64_t), callback(int64_t)."; - return {false, 0}; + return {}; } for (int j = 0; j < export_count; ++j) @@ -814,7 +820,7 @@ validateCreateCode(SetHookCtx& ctx, STObject const& hookSetObj) << "HookSet(" << hook::log::EXPORT_HOOK_FUNC << ")[" << HS_ACC() << "]: Malformed transaction. " << "Hook did not export: A valid int64_t hook(uint32_t)"; - return {false, 0}; + return {}; } i++; CHECK_SHORT_HOOK(); @@ -831,7 +837,7 @@ validateCreateCode(SetHookCtx& ctx, STObject const& hookSetObj) << "HookSet(" << hook::log::EXPORT_CBAK_FUNC << ")[" << HS_ACC() << "]: Malformed transaction. " << "Hook did not export: A valid int64_t cbak(uint32_t)"; - return {false, 0}; + return {}; } i++; CHECK_SHORT_HOOK(); cbak_func_idx = parseLeb128(hook, i, &i); CHECK_SHORT_HOOK(); @@ -844,15 +850,14 @@ validateCreateCode(SetHookCtx& ctx, STObject const& hookSetObj) } // execution to here means export section was parsed - if (!(hook_func_idx && cbak_func_idx)) + if (!hook_func_idx) { JLOG(ctx.j.trace()) << "HookSet(" << hook::log::EXPORT_MISSING << ")[" << HS_ACC() << "]: Malformed transaction. " - << "Hook did not export: " << - ( !hook_func_idx ? "int64_t hook(uint32_t); " : "" ) << - ( !cbak_func_idx ? "int64_t cbak(uint32_t);" : "" ); - return {false, 0}; + << "Hook did not export: " + << ( !hook_func_idx ? "int64_t hook(uint32_t); " : "" ); + return {}; } } else if (section_type == 3) // function section @@ -865,7 +870,7 @@ validateCreateCode(SetHookCtx& ctx, STObject const& hookSetObj) << ")[" << HS_ACC() << "]: Malformed transaction. " << "Hook did not establish any functions... " << "required hook(int64_t), callback(int64_t)."; - return {false, 0}; + return {}; } for (int j = 0; j < function_count; ++j) @@ -884,20 +889,26 @@ validateCreateCode(SetHookCtx& ctx, STObject const& hookSetObj) // look them up in the functions section. this is a rule of the webassembly spec // note that at this point in execution we are guarenteed these are populated *hook_func_idx -= import_count; - *cbak_func_idx -= import_count; + + if (cbak_func_idx) + *cbak_func_idx -= import_count; if (func_type_map.find(*hook_func_idx) == func_type_map.end() || - func_type_map.find(*cbak_func_idx) == func_type_map.end()) + (cbak_func_idx && func_type_map.find(*cbak_func_idx) == func_type_map.end())) { JLOG(ctx.j.trace()) << "HookSet(" << hook::log::FUNC_TYPELESS << ")[" << HS_ACC() << "]: Malformed transaction. " << "hook or cbak functions did not have a corresponding type in WASM binary."; - return {false, 0}; + return {}; } int hook_type_idx = func_type_map[*hook_func_idx]; - int cbak_type_idx = func_type_map[*cbak_func_idx]; + + // cbak function is optional so if it exists it has a type otherwise it is skipped in checks + std::optional cbak_type_idx; + if (cbak_func_idx) + cbak_type_idx = func_type_map[*cbak_func_idx]; // second pass... where we check all the guard function calls follow the guard rules // minimal other validation in this pass because first pass caught most of it @@ -923,7 +934,7 @@ validateCreateCode(SetHookCtx& ctx, STObject const& hookSetObj) << "Codesec: " << section_type << " " << "Local: " << j << " " << "Offset: " << i; - return {false, 0}; + return {}; } CHECK_SHORT_HOOK(); @@ -944,7 +955,7 @@ validateCreateCode(SetHookCtx& ctx, STObject const& hookSetObj) << "Codesec: " << section_type << " " << "Local: " << j << " " << "Offset: " << i; - return {false, 0}; + return {}; } printf("Function type idx: %d, hook_func_idx: %d, cbak_func_idx: %d " @@ -952,14 +963,14 @@ validateCreateCode(SetHookCtx& ctx, STObject const& hookSetObj) j, *hook_func_idx, *cbak_func_idx, param_count, param_type); // hook and cbak parameter check here - if ((j == hook_type_idx || j == cbak_type_idx) && + if ((j == hook_type_idx || (cbak_type_idx && j == cbak_type_idx)) && (param_count != 1 || param_type != 0x7F /* i32 */ )) { JLOG(ctx.j.trace()) << "HookSet(" << hook::log::PARAM_HOOK_CBAK << ")[" << HS_ACC() << "]: Malformed transaction. " << "hook and cbak function definition must have exactly one uint32_t parameter."; - return {false, 0}; + return {}; } } @@ -973,7 +984,7 @@ validateCreateCode(SetHookCtx& ctx, STObject const& hookSetObj) << "HookSet(" << hook::log::FUNC_RETURN_COUNT << ")[" << HS_ACC() << "]: Malformed transaction. " << "Hook declares a function type that returns fewer or more than one value."; - return {false, 0}; + return {}; } // this can only ever be 1 in production, but in testing it may also be 0 or >1 @@ -994,7 +1005,7 @@ validateCreateCode(SetHookCtx& ctx, STObject const& hookSetObj) << "Codesec: " << section_type << " " << "Local: " << j << " " << "Offset: " << i; - return {false, 0}; + return {}; } printf("Function type idx: %d, hook_func_idx: %d, cbak_func_idx: %d " @@ -1002,7 +1013,7 @@ validateCreateCode(SetHookCtx& ctx, STObject const& hookSetObj) j, *hook_func_idx, *cbak_func_idx, result_count, result_type); // hook and cbak return type check here - if ((j == hook_type_idx || j == cbak_type_idx) && + if ((j == hook_type_idx || (cbak_type_idx && j == cbak_type_idx)) && (result_count != 1 || result_type != 0x7E /* i64 */ )) { JLOG(ctx.j.trace()) @@ -1012,7 +1023,7 @@ validateCreateCode(SetHookCtx& ctx, STObject const& hookSetObj) << " function definition must have exactly one int64_t return type. " << "resultcount=" << result_count << ", resulttype=" << result_type << ", " << "paramcount=" << param_count; - return {false, 0}; + return {}; } } } @@ -1042,7 +1053,7 @@ validateCreateCode(SetHookCtx& ctx, STObject const& hookSetObj) << "Codesec: " << j << " " << "Local: " << k << " " << "Offset: " << i; - return {false, 0}; + return {}; } i++; CHECK_SHORT_HOOK(); } @@ -1052,15 +1063,18 @@ validateCreateCode(SetHookCtx& ctx, STObject const& hookSetObj) // execution to here means we are up to the actual expr for the codesec/function - auto [valid, instruction_count] = + auto valid = check_guard(ctx, hook, j, i, code_end, guard_import_number, last_import_number); if (!valid) - return {false, 0}; + return {}; - // the worst case execution is the fee, this includes the worst case between cbak and hook - if (instruction_count > maxInstrCount) - maxInstrCount = instruction_count; + if (hook_func_idx && *hook_func_idx == j) + maxInstrCountHook = *valid; + else if (cbak_func_idx && *cbak_func_idx == j) + maxInstrCountCbak = *valid; + else + assert(false); i = code_end; @@ -1072,7 +1086,9 @@ validateCreateCode(SetHookCtx& ctx, STObject const& hookSetObj) // execution to here means guards are installed correctly JLOG(ctx.j.trace()) - << "HookSet(" << hook::log::WASM_SMOKE_TEST << ")[" << HS_ACC() << "]: Trying to wasm instantiate proposed hook " + << "HookSet(" + << hook::log::WASM_SMOKE_TEST + << ")[" << HS_ACC() << "]: Trying to wasm instantiate proposed hook " << "size = " << hook.size(); std::optional result = @@ -1083,19 +1099,24 @@ validateCreateCode(SetHookCtx& ctx, STObject const& hookSetObj) JLOG(ctx.j.trace()) << "HookSet(" << hook::log::WASM_TEST_FAILURE << ")[" << HS_ACC() << "]: " << "Tried to set a hook with invalid code. VM error: " << *result; - return {false, 0}; + return {}; } - return {true, maxInstrCount}; + return std::pair{maxInstrCountHook, maxInstrCountCbak}; } // This is a context-free validation, it does not take into account the current state of the ledger // returns < valid, instruction count > // may throw overflow_error -std::pair +std::variant< + bool, // true = valid + std::pair< // if set implicitly valid, and return instruction counts (hsoCREATE only) + uint64_t, // max instruction count for hook + uint64_t // max instruction count for cbak + > +> validateHookSetEntry(SetHookCtx& ctx, STObject const& hookSetObj) { - uint32_t flags = hookSetObj.isFieldPresent(sfFlags) ? hookSetObj.getFieldU32(sfFlags) : 0; @@ -1103,7 +1124,7 @@ validateHookSetEntry(SetHookCtx& ctx, STObject const& hookSetObj) { case hsoNOOP: { - return {true, 0}; + return true; } case hsoNSDELETE: @@ -1120,7 +1141,7 @@ validateHookSetEntry(SetHookCtx& ctx, STObject const& hookSetObj) << "HookSet(" << hook::log::NSDELETE_FIELD << ")[" << HS_ACC() << "]: Malformed transaction: SetHook nsdelete operation should contain only " << "sfHookNamespace & sfFlags"; - return {false, 0}; + return false; } if (flags != hsfNSDELETE) @@ -1128,10 +1149,10 @@ validateHookSetEntry(SetHookCtx& ctx, STObject const& hookSetObj) JLOG(ctx.j.trace()) << "HookSet(" << hook::log::NSDELETE_FLAGS << ")[" << HS_ACC() << "]: Malformed transaction: SetHook nsdelete operation should only specify hsfNSDELETE"; - return {false, 0}; + return false; } - return {true, 0}; + return true; } case hsoDELETE: @@ -1146,7 +1167,7 @@ validateHookSetEntry(SetHookCtx& ctx, STObject const& hookSetObj) JLOG(ctx.j.trace()) << "HookSet(" << hook::log::DELETE_FIELD << ")[" << HS_ACC() << "]: Malformed transaction: SetHook delete operation should contain only sfCreateCode & sfFlags"; - return {false, 0}; + return false; } if (!(flags & hsfOVERRIDE)) @@ -1154,7 +1175,7 @@ validateHookSetEntry(SetHookCtx& ctx, STObject const& hookSetObj) JLOG(ctx.j.trace()) << "HookSet(" << hook::log::OVERRIDE_MISSING << ")[" << HS_ACC() << "]: Malformed transaction: SetHook delete operation was missing the hsfOVERRIDE flag"; - return {false, 0}; + return false; } @@ -1163,10 +1184,10 @@ validateHookSetEntry(SetHookCtx& ctx, STObject const& hookSetObj) JLOG(ctx.j.trace()) << "HookSet(" << hook::log::FLAGS_INVALID << ")[" << HS_ACC() << "]: Malformed transaction: SetHook delete operation specified invalid flags"; - return {false, 0}; + return false; } - return {true, 0}; + return true; } case hsoINSTALL: @@ -1174,12 +1195,12 @@ validateHookSetEntry(SetHookCtx& ctx, STObject const& hookSetObj) // validate hook params structure, if any if (hookSetObj.isFieldPresent(sfHookParameters) && !validateHookParams(ctx, hookSetObj.getFieldArray(sfHookParameters))) - return {false, 0}; + return false; // validate hook grants structure, if any if (hookSetObj.isFieldPresent(sfHookGrants) && !validateHookGrants(ctx, hookSetObj.getFieldArray(sfHookGrants))) - return {false, 0}; + return false; // api version not allowed in update if (hookSetObj.isFieldPresent(sfHookApiVersion)) @@ -1187,14 +1208,14 @@ validateHookSetEntry(SetHookCtx& ctx, STObject const& hookSetObj) JLOG(ctx.j.trace()) << "HookSet(" << hook::log::API_ILLEGAL << ")[" << HS_ACC() << "]: Malformed transaction: SetHook install operation sfHookApiVersion must not be included."; - return {false, 0}; + return false; } // namespace may be valid, if the user so chooses // hookon may be present if the user so chooses // flags may be present if the user so chooses - return {true, 0}; + return true; } case hsoUPDATE: @@ -1207,18 +1228,18 @@ validateHookSetEntry(SetHookCtx& ctx, STObject const& hookSetObj) << "HookSet(" << hook::log::FLAGS_INVALID << ")[" << HS_ACC() << "]: Malformed transaction: SetHook update operation only hsfNSDELETE may be specified and " << "only if a new HookNamespace is also specified."; - return {false, 0}; + return false; } // validate hook params structure if (hookSetObj.isFieldPresent(sfHookParameters) && !validateHookParams(ctx, hookSetObj.getFieldArray(sfHookParameters))) - return {false, 0}; + return false; // validate hook grants structure if (hookSetObj.isFieldPresent(sfHookGrants) && !validateHookGrants(ctx, hookSetObj.getFieldArray(sfHookGrants))) - return {false, 0}; + return false; // api version not allowed in update if (hookSetObj.isFieldPresent(sfHookApiVersion)) @@ -1226,14 +1247,14 @@ validateHookSetEntry(SetHookCtx& ctx, STObject const& hookSetObj) JLOG(ctx.j.trace()) << "HookSet(" << hook::log::API_ILLEGAL << ")[" << HS_ACC() << "]: Malformed transaction: SetHook update operation sfHookApiVersion must not be included."; - return {false, 0}; + return false; } // namespace may be valid, if the user so chooses // hookon may be present if the user so chooses // flags may be present if the user so chooses - return {true, 0}; + return true; } case hsoCREATE: @@ -1241,12 +1262,12 @@ validateHookSetEntry(SetHookCtx& ctx, STObject const& hookSetObj) // validate hook params structure if (hookSetObj.isFieldPresent(sfHookParameters) && !validateHookParams(ctx, hookSetObj.getFieldArray(sfHookParameters))) - return {false, 0}; + return false; // validate hook grants structure if (hookSetObj.isFieldPresent(sfHookGrants) && !validateHookGrants(ctx, hookSetObj.getFieldArray(sfHookGrants))) - return {false, 0}; + return false; // ensure hooknamespace is present @@ -1255,7 +1276,7 @@ validateHookSetEntry(SetHookCtx& ctx, STObject const& hookSetObj) JLOG(ctx.j.trace()) << "HookSet(" << hook::log::NAMESPACE_MISSING << ")[" << HS_ACC() << "]: Malformed transaction: SetHook sfHookDefinition must contain sfHookNamespace."; - return {false, 0}; + return false; } // validate api version, if provided @@ -1264,7 +1285,7 @@ validateHookSetEntry(SetHookCtx& ctx, STObject const& hookSetObj) JLOG(ctx.j.trace()) << "HookSet(" << hook::log::API_MISSING << ")[" << HS_ACC() << "]: Malformed transaction: SetHook sfHookApiVersion must be included."; - return {false, 0}; + return false; } auto version = hookSetObj.getFieldU16(sfHookApiVersion); @@ -1274,7 +1295,7 @@ validateHookSetEntry(SetHookCtx& ctx, STObject const& hookSetObj) JLOG(ctx.j.trace()) << "HookSet(" << hook::log::API_INVALID << ")[" << HS_ACC() << "]: Malformed transaction: SetHook sfHook->sfHookApiVersion invalid. (Try 0)."; - return {false, 0}; + return false; } // validate sfHookOn @@ -1283,11 +1304,16 @@ validateHookSetEntry(SetHookCtx& ctx, STObject const& hookSetObj) JLOG(ctx.j.trace()) << "HookSet(" << hook::log::HOOKON_MISSING << ")[" << HS_ACC() << "]: Malformed transaction: SetHook must include sfHookOn when creating a new hook."; - return {false, 0}; + return false; } // finally validate web assembly byte code - return validateCreateCode(ctx, hookSetObj); + { + auto result = validateCreateCode(ctx, hookSetObj); + if (!result) + return false; + return *result; + } } case hsoINVALID: @@ -1296,7 +1322,7 @@ validateHookSetEntry(SetHookCtx& ctx, STObject const& hookSetObj) JLOG(ctx.j.trace()) << "HookSet(" << hook::log::HASH_OR_CODE << ")[" << HS_ACC() << "]: Malformed transaction: SetHook must provide only one of sfCreateCode or sfHookHash."; - return {false, 0}; + return false; } } } @@ -1447,12 +1473,12 @@ SetHook::preflight(PreflightContext const& ctx) { // may throw if leb128 overflow is detected - [[maybe_unused]] - auto [valid, _] = + auto valid = validateHookSetEntry(shCtx, *hookSetObj); - if (!valid) - return temMALFORMED; + if (std::holds_alternative(valid) && !std::get(valid)) + return temMALFORMED; + } catch (std::exception& e) { @@ -2036,23 +2062,34 @@ SetHook::setHook() } else { - uint64_t maxInstrCount = 0; + uint64_t maxInstrCountHook = 0; + uint64_t maxInstrCountCbak = 0; bool valid = false; // create hook definition SLE try { - std::tie(valid, maxInstrCount) = + auto valid = validateHookSetEntry(ctx, hookSetObj->get()); - if (!valid) + // if invalid return an error + if (std::holds_alternative(valid)) { - JLOG(ctx.j.warn()) - << "HookSet(" << hook::log::WASM_INVALID << ")[" << HS_ACC() - << "]: Malformed transaction: SetHook operation would create invalid hook wasm"; - return tecINTERNAL; + if (!std::get(valid)) + { + JLOG(ctx.j.warn()) + << "HookSet(" << hook::log::WASM_INVALID << ")[" << HS_ACC() + << "]: Malformed transaction: SetHook operation would create invalid hook wasm"; + return tecINTERNAL; + } + else + assert(false); // should never happen } + + // otherwise assign instruction counts + std::tie(maxInstrCountHook, maxInstrCountCbak) = + std::get>(valid); } catch (std::exception& e) { @@ -2084,7 +2121,11 @@ SetHook::setHook() newHookDef->setFieldVL( sfCreateCode, wasmBytes); newHookDef->setFieldH256( sfHookSetTxnID, ctx.tx.getTransactionID()); newHookDef->setFieldU64( sfReferenceCount, 1); - newHookDef->setFieldAmount(sfFee, XRPAmount {hook::computeExecutionFee(maxInstrCount)} ); + newHookDef->setFieldAmount(sfFee, + XRPAmount {hook::computeExecutionFee(maxInstrCountHook)}); + if (maxInstrCountCbak > 0) + newHookDef->setFieldAmount(sfHookCallbackFee, + XRPAmount {hook::computeExecutionFee(maxInstrCountCbak)}); view().insert(newHookDef); newHook.setFieldH256(sfHookHash, *createHookHash); newHooks.push_back(std::move(newHook)); diff --git a/src/ripple/app/tx/impl/Transactor.cpp b/src/ripple/app/tx/impl/Transactor.cpp index d2bdbfc89..c4c0d5f0a 100644 --- a/src/ripple/app/tx/impl/Transactor.cpp +++ b/src/ripple/app/tx/impl/Transactor.cpp @@ -228,16 +228,16 @@ Transactor::calculateBaseFee(ReadView const& view, STTx const& tx) // if the txn is NOT an emitted txn then we process the sending account's hook chain if (tx.isFieldPresent(sfEmitDetails)) { - STObject const& emitDetails = const_cast(tx).getField(sfEmitDetails).downcast(); - + uint256 const& callbackHookHash = emitDetails.getFieldH256(sfEmitHookHash); std::shared_ptr hookDef = view.read(keylet::hookDefinition(callbackHookHash)); - - if (hookDef) - hookExecutionFee += FeeUnit64{(uint32_t)(hookDef->getFieldAmount(sfFee).xrp().drops())}; + + if (hookDef && hookDef->isFieldPresent(sfHookCallbackFee)) + hookExecutionFee += + FeeUnit64{(uint32_t)(hookDef->getFieldAmount(sfHookCallbackFee).xrp().drops())}; } else hookExecutionFee += @@ -896,7 +896,7 @@ static __inline__ unsigned long long rdtsc(void) return ( (unsigned long long)lo)|( ((unsigned long long)hi)<<32 ); } -bool /* error = true */ +bool /* retval of true means an error */ gatherHookParameters( std::shared_ptr const& hookDef, ripple::STObject const* hookObj, @@ -933,7 +933,7 @@ gatherHookParameters( return false; } -bool /* error = true */ +bool /* retval of true means an error */ executeHookChain( std::shared_ptr const& hookSLE, std::shared_ptr& stateMap, @@ -1095,12 +1095,12 @@ Transactor::operator()() // and any associated chains which are executed during this transaction also std::shared_ptr stateMap = std::make_shared(); - auto const& ledger = ctx_.view(); + auto& view = ctx_.view(); auto const& accountID = ctx_.tx.getAccountID(sfAccount); std::vector sendResults; std::vector recvResults; - auto const& hooksSending = ledger.read(keylet::hook(accountID)); + auto const& hooksSending = view.read(keylet::hook(accountID)); // First check if the Sending account has any hooks that can be fired if (hooksSending && hooksSending->isFieldPresent(sfHooks) && !ctx_.emitted()) @@ -1111,23 +1111,91 @@ Transactor::operator()() if (!rollback) { std::vector> tsh = - hook::getTransactionalStakeHolders(ctx_.tx, ledger); + hook::getTransactionalStakeHolders(ctx_.tx, view); for (auto& [tshAccountID, canRollback] : tsh) { - auto const& hooksReceiving = ledger.read(keylet::hook(tshAccountID)); - if (hooksReceiving && hooksReceiving->isFieldPresent(sfHooks)) + auto klTshHook = keylet::hook(tshAccountID); + + auto tshHook = view.read(klTshHook); + if (!(tshHook && tshHook->isFieldPresent(sfHooks))) + continue; + + // scoping here allows tshAcc to leave scope before + // hook execution, which is probably safer { - bool tshRollback = - executeHookChain(hooksReceiving, stateMap, - recvResults, executedHookCount, tshAccountID, ctx_, j_, result); - if (canRollback && tshRollback) + // check if the TSH exists and/or has any hooks + auto tshAcc = view.peek(keylet::account(tshAccountID)); + if (!tshAcc) + continue; + + + // compute and deduct fees for the TSH if applicable + FeeUnit64 tshFee = + calculateHookChainFee(view, ctx_.tx, klTshHook); + + // no hooks to execute, skip tsh + if (tshFee == 0) + continue; + + XRPAmount tshFeeDrops = view.fees().toDrops(tshFee); + assert(tshFeeDrops >= beast::zero); + + STAmount priorBalance = tshAcc->getFieldAmount(sfBalance); + + if (canRollback) { - rollback = true; - break; + // this is not a collect call so we will force the tsh's fee to 0 + // the otxn paid the fee for this tsh chain execution already. + tshFeeDrops = 0; + } + else + { + // this is a collect call so first check if the tsh can accept + uint32_t tshFlags = tshAcc->getFieldU32(sfFlags); + if (!canRollback && !(tshFlags & lsfTshCollect)) + { + // this TSH doesn't allow collect calls, skip + JLOG(j_.trace()) + << "HookInfo[" << accountID << "]: TSH acc " << tshAccountID << " " + << "hook chain execution skipped due to lack of lsfTshCollect flag."; + continue; + } + + // now check if they can afford this collect call + auto const uOwnerCount = tshAcc->getFieldU32(sfOwnerCount); + auto const reserve = view.fees().accountReserve(uOwnerCount); + + if (tshFeeDrops + reserve > priorBalance) + { + JLOG(j_.trace()) + << "HookInfo[" << accountID << "]: TSH acc " << tshAccountID << " " + << "hook chain execution skipped due to lack of TSH acc funds."; + continue; + } } - //RH TODO: charge non-rollback tsh executions a fee here + if (tshFeeDrops > beast::zero) + { + STAmount finalBalance = priorBalance -= tshFeeDrops; + assert(finalBalance >= beast::zero); + assert(finalBalance < priorBalance); + + tshAcc->setFieldAmount(sfBalance, finalBalance); + + view.update(tshAcc); + } + } + + // execution to here means we can run the TSH's hook chain + bool tshRollback = + executeHookChain(tshHook, stateMap, + recvResults, executedHookCount, tshAccountID, ctx_, j_, result); + + if (canRollback && tshRollback) + { + rollback = true; + break; } } } @@ -1153,19 +1221,16 @@ Transactor::operator()() auto const& emitDetails = const_cast(ctx_.tx).getField(sfEmitDetails).downcast(); + // callbacks are optional so if there isn't a callback then skip if (!emitDetails.isFieldPresent(sfEmitCallback)) - { - JLOG(j_.fatal()) - << "HookError[]: Callback Processing: Failure: sfEmitCallback missing"; break; - } AccountID const& callbackAccountID = emitDetails.getAccountID(sfEmitCallback); uint256 const& callbackHookHash = emitDetails.getFieldH256(sfEmitHookHash); - auto const& hooksCallback = view().peek(keylet::hook(callbackAccountID)); - auto const& hookDef = view().peek(keylet::hookDefinition(callbackHookHash)); + auto const& hooksCallback = view.peek(keylet::hook(callbackAccountID)); + auto const& hookDef = view.peek(keylet::hookDefinition(callbackHookHash)); if (!hookDef) { JLOG(j_.warn()) @@ -1173,6 +1238,14 @@ Transactor::operator()() break; } + if (!hookDef->isFieldPresent(sfHookCallbackFee)) + { + JLOG(j_.trace()) + << "HookInfo[" << callbackAccountID << "]: Callback specified by emitted txn " + << "but hook lacks a cbak function, skipping."; + break; + } + if (!hooksCallback) { JLOG(j_.warn()) @@ -1232,7 +1305,6 @@ Transactor::operator()() callbackHookHash, ns, hookDef->getFieldVL(sfCreateCode), - // params parameters, {}, stateMap, @@ -1273,7 +1345,8 @@ Transactor::operator()() if (!found) { JLOG(j_.warn()) - << "HookError[]: Hookhash not found on callback account"; + << "HookError[" << callbackAccountID << "]: Hookhash " + << callbackHookHash << " not found on callback account"; } } } diff --git a/src/ripple/app/tx/impl/applyHook.cpp b/src/ripple/app/tx/impl/applyHook.cpp index be448f08e..57e112fe8 100644 --- a/src/ripple/app/tx/impl/applyHook.cpp +++ b/src/ripple/app/tx/impl/applyHook.cpp @@ -77,9 +77,6 @@ namespace hook case ttTICKET_CREATE: case ttHOOK_SET: case ttOFFER_CREATE: // this is handled seperately - //case ttCONTRACT: // not used - //case ttSPINAL_TAP: // not used - //case ttNICKNAME_SET: // not used { break; } diff --git a/src/ripple/protocol/LedgerFormats.h b/src/ripple/protocol/LedgerFormats.h index 1f2012ab0..68bab4aaa 100644 --- a/src/ripple/protocol/LedgerFormats.h +++ b/src/ripple/protocol/LedgerFormats.h @@ -242,10 +242,9 @@ enum LedgerSpecificFlags { lsfNoFreeze = 0x00200000, // True, cannot freeze ripple states lsfGlobalFreeze = 0x00400000, // True, all assets frozen lsfDefaultRipple = - 0x00800000, // True, trust lines allow rippling by default - lsfDepositAuth = 0x01000000, // True, all deposits require authorization - - lsfHookEnabled = 0x02000000, // True, all in and out tx will fire hook code + 0x00800000, // True, trust lines allow rippling by default + lsfDepositAuth = 0x01000000, // True, all deposits require authorization + lsfTshCollect = 0x02000000, // True, allow TSH collect-calls to acc hooks // ltOFFER lsfPassive = 0x00010000, diff --git a/src/ripple/protocol/SField.h b/src/ripple/protocol/SField.h index 0e38e5b94..4028d6876 100644 --- a/src/ripple/protocol/SField.h +++ b/src/ripple/protocol/SField.h @@ -479,6 +479,7 @@ extern SF_AMOUNT const sfMinimumOffer; extern SF_AMOUNT const sfRippleEscrow; extern SF_AMOUNT const sfDeliveredAmount; extern SF_AMOUNT const sfBrokerFee; +extern SF_AMOUNT const sfHookCallbackFee; // variable length (common) extern SF_VL const sfPublicKey; diff --git a/src/ripple/protocol/TxFlags.h b/src/ripple/protocol/TxFlags.h index 27d1ff3b3..09b5c858a 100644 --- a/src/ripple/protocol/TxFlags.h +++ b/src/ripple/protocol/TxFlags.h @@ -71,6 +71,7 @@ const std::uint32_t asfNoFreeze = 6; const std::uint32_t asfGlobalFreeze = 7; const std::uint32_t asfDefaultRipple = 8; const std::uint32_t asfDepositAuth = 9; +const std::uint32_t asfTshCollect = 11; // OfferCreate flags: const std::uint32_t tfPassive = 0x00010000; diff --git a/src/ripple/protocol/impl/InnerObjectFormats.cpp b/src/ripple/protocol/impl/InnerObjectFormats.cpp index ab5a06ce4..688165b3b 100644 --- a/src/ripple/protocol/impl/InnerObjectFormats.cpp +++ b/src/ripple/protocol/impl/InnerObjectFormats.cpp @@ -30,7 +30,7 @@ InnerObjectFormats::InnerObjectFormats() {sfEmitBurden, soeREQUIRED}, {sfEmitParentTxnID, soeREQUIRED}, {sfEmitNonce, soeREQUIRED}, - {sfEmitCallback, soeREQUIRED}, + {sfEmitCallback, soeOPTIONAL}, {sfEmitHookHash, soeREQUIRED} }); diff --git a/src/ripple/protocol/impl/LedgerFormats.cpp b/src/ripple/protocol/impl/LedgerFormats.cpp index 9824a5264..700a4f156 100644 --- a/src/ripple/protocol/impl/LedgerFormats.cpp +++ b/src/ripple/protocol/impl/LedgerFormats.cpp @@ -203,7 +203,8 @@ LedgerFormats::LedgerFormats() {sfCreateCode, soeREQUIRED}, {sfHookSetTxnID, soeREQUIRED}, {sfReferenceCount, soeREQUIRED}, - {sfFee, soeREQUIRED} + {sfFee, soeREQUIRED}, + {sfHookCallbackFee, soeOPTIONAL} }, commonFields); diff --git a/src/ripple/protocol/impl/SField.cpp b/src/ripple/protocol/impl/SField.cpp index d300db4c4..a4857d875 100644 --- a/src/ripple/protocol/impl/SField.cpp +++ b/src/ripple/protocol/impl/SField.cpp @@ -231,6 +231,7 @@ CONSTRUCT_TYPED_SFIELD(sfMinimumOffer, "MinimumOffer", AMOUNT, CONSTRUCT_TYPED_SFIELD(sfRippleEscrow, "RippleEscrow", AMOUNT, 17); CONSTRUCT_TYPED_SFIELD(sfDeliveredAmount, "DeliveredAmount", AMOUNT, 18); CONSTRUCT_TYPED_SFIELD(sfBrokerFee, "BrokerFee", AMOUNT, 19); +CONSTRUCT_TYPED_SFIELD(sfHookCallbackFee, "HookCallbackFee", AMOUNT, 20); // variable length (common) CONSTRUCT_TYPED_SFIELD(sfPublicKey, "PublicKey", VL, 1);