Review fixes (#6512)

This commit is contained in:
Olek
2026-04-20 14:03:39 -04:00
committed by GitHub
parent 8cc2169939
commit ce2586c039
8 changed files with 111 additions and 87 deletions

View File

@@ -81,7 +81,7 @@ public:
checkSelf() const override
{
return !currentLedgerObj_ && !data_ &&
std::ranges::find_if(cache_, [](auto& p) { return !!p; }) == cache_.end();
std::ranges::none_of(cache_, [](auto const& p) { return !!p; });
}
std::optional<Bytes> const&

View File

@@ -48,7 +48,7 @@ struct WasmRuntimeWrapper
////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
enum WasmTypes { WT_I32, WT_I64, WT_U8V };
enum WasmTypes { WT_I32, WT_I64 };
struct WasmImportFunc
{
@@ -142,22 +142,15 @@ WasmImpFunc(
////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
struct WasmParamVec
{
std::uint8_t const* d = nullptr;
std::int32_t sz = 0;
};
struct WasmParam
{
// We are not supporting float/double
WasmTypes type = WT_I32;
union
{
std::int32_t i32;
std::int64_t i64 = 0;
float f32;
double f64;
WasmParamVec u8v;
} of;
};
@@ -177,24 +170,6 @@ wasmParamsHlp(std::vector<WasmParam>& v, std::int64_t p, Types&&... args)
wasmParamsHlp(v, std::forward<Types>(args)...);
}
// We are not supporting float/double for now
// Leaving this code here so that it is easier to add later if needed
// template <class... Types>
// inline void
// wasmParamsHlp(std::vector<WasmParam>& v, float p, Types&&... args)
// {
// v.push_back({.type = WT_F32, .of = {.f32 = p}});
// wasmParamsHlp(v, std::forward<Types>(args)...);
// }
// template <class... Types>
// inline void
// wasmParamsHlp(std::vector<WasmParam>& v, double p, Types&&... args)
// {
// v.push_back({.type = WT_F64, .of = {.f64 = p}});
// wasmParamsHlp(v, std::forward<Types>(args)...);
// }
inline void
wasmParamsHlp(std::vector<WasmParam>& v)
{

View File

@@ -6,19 +6,19 @@
namespace xrpl {
static std::string_view const W_ENV = "env";
static std::string_view const W_HOST_LIB = "host_lib";
static std::string_view const W_MEM = "memory";
static std::string_view const W_STORE = "store";
static std::string_view const W_LOAD = "load";
static std::string_view const W_SIZE = "size";
static std::string_view const W_ALLOC = "allocate";
static std::string_view const W_DEALLOC = "deallocate";
static std::string_view const W_PROC_EXIT = "proc_exit";
std::string_view inline constexpr W_ENV = "env";
std::string_view inline constexpr W_HOST_LIB = "host_lib";
std::string_view inline constexpr W_MEM = "memory";
std::string_view inline constexpr W_STORE = "store";
std::string_view inline constexpr W_LOAD = "load";
std::string_view inline constexpr W_SIZE = "size";
std::string_view inline constexpr W_ALLOC = "allocate";
std::string_view inline constexpr W_DEALLOC = "deallocate";
std::string_view inline constexpr W_PROC_EXIT = "proc_exit";
static std::string_view const ESCROW_FUNCTION_NAME = "finish";
std::string_view inline constexpr ESCROW_FUNCTION_NAME = "finish";
uint32_t const MAX_PAGES = 128; // 8MB = 64KB*128
uint32_t inline constexpr MAX_PAGES = 128; // 8MB = 64KB*128
class WasmiEngine;
@@ -36,8 +36,6 @@ class WasmEngine
operator=(WasmEngine&&) = delete;
public:
~WasmEngine() = default;
static WasmEngine&
instance();

View File

@@ -125,6 +125,7 @@ struct WasmiResult
{
}
WasmiResult() = delete;
~WasmiResult() = default;
WasmiResult(WasmiResult&& o) = default;
WasmiResult&
@@ -158,14 +159,17 @@ private:
public:
InstanceWrapper();
InstanceWrapper(InstanceWrapper const&) = delete;
InstanceWrapper(InstanceWrapper&& o);
InstanceWrapper(StorePtr& s, ModulePtr& m, WasmExternVec const& imports, beast::Journal j);
InstanceWrapper&
operator=(InstanceWrapper&& o);
InstanceWrapper(StorePtr& s, ModulePtr& m, WasmExternVec const& imports, beast::Journal j);
~InstanceWrapper() = default;
InstanceWrapper&
operator=(InstanceWrapper const&) = delete;
operator bool() const;

View File

@@ -6,19 +6,17 @@ namespace xrpl {
typedef std::variant<STBase const*, uint256 const*> FieldValue;
namespace detail {
template <class T>
Bytes
getIntBytes(STBase const* obj)
{
static_assert(std::is_integral<T>::value, "Only integral types");
static_assert(std::is_integral_v<T>, "Only integral types");
XRPL_ASSERT(obj, "getIntBytes null pointer");
auto const& num(static_cast<STInteger<T> const*>(obj)); // NOLINT
auto const* num(static_cast<STInteger<T> const*>(obj)); // NOLINT
T const data = adjustWasmEndianess(num->value());
auto const* b = reinterpret_cast<uint8_t const*>(&data);
auto const* e = reinterpret_cast<uint8_t const*>(&data + 1);
return Bytes{b, e};
uint8_t const* b = reinterpret_cast<uint8_t const*>(&data);
return Bytes{b, b + sizeof(T)};
}
static Expected<Bytes, HostFunctionError>
@@ -217,8 +215,6 @@ getArrayLen(FieldValue const& variantField)
return Unexpected(HostFunctionError::NO_ARRAY); // LCOV_EXCL_LINE
}
} // namespace detail
Expected<int32_t, HostFunctionError>
WasmHostFunctionsImpl::cacheLedgerObj(uint256 const& objId, int32_t cacheIdx)
{
@@ -253,7 +249,7 @@ WasmHostFunctionsImpl::cacheLedgerObj(uint256 const& objId, int32_t cacheIdx)
Expected<Bytes, HostFunctionError>
WasmHostFunctionsImpl::getTxField(SField const& fname) const
{
return detail::getAnyFieldData(ctx_.tx.peekAtPField(fname));
return getAnyFieldData(ctx_.tx.peekAtPField(fname));
}
Expected<Bytes, HostFunctionError>
@@ -262,7 +258,7 @@ WasmHostFunctionsImpl::getCurrentLedgerObjField(SField const& fname) const
auto const sle = getCurrentLedgerObj();
if (!sle.has_value())
return Unexpected(sle.error());
return detail::getAnyFieldData(sle.value()->peekAtPField(fname));
return getAnyFieldData(sle.value()->peekAtPField(fname));
}
Expected<Bytes, HostFunctionError>
@@ -271,7 +267,7 @@ WasmHostFunctionsImpl::getLedgerObjField(int32_t cacheIdx, SField const& fname)
auto const normalizedIdx = normalizeCacheIndex(cacheIdx);
if (!normalizedIdx.has_value())
return Unexpected(normalizedIdx.error());
return detail::getAnyFieldData(cache_[normalizedIdx.value()]->peekAtPField(fname));
return getAnyFieldData(cache_[normalizedIdx.value()]->peekAtPField(fname));
}
// Subsection: nested getters
@@ -279,11 +275,11 @@ WasmHostFunctionsImpl::getLedgerObjField(int32_t cacheIdx, SField const& fname)
Expected<Bytes, HostFunctionError>
WasmHostFunctionsImpl::getTxNestedField(Slice const& locator) const
{
auto const r = detail::locateField(ctx_.tx, locator);
auto const r = locateField(ctx_.tx, locator);
if (!r)
return Unexpected(r.error());
return detail::getAnyFieldData(r.value());
return getAnyFieldData(r.value());
}
Expected<Bytes, HostFunctionError>
@@ -293,11 +289,11 @@ WasmHostFunctionsImpl::getCurrentLedgerObjNestedField(Slice const& locator) cons
if (!sle.has_value())
return Unexpected(sle.error());
auto const r = detail::locateField(*sle.value(), locator);
auto const r = locateField(*sle.value(), locator);
if (!r)
return Unexpected(r.error());
return detail::getAnyFieldData(r.value());
return getAnyFieldData(r.value());
}
Expected<Bytes, HostFunctionError>
@@ -307,11 +303,11 @@ WasmHostFunctionsImpl::getLedgerObjNestedField(int32_t cacheIdx, Slice const& lo
if (!normalizedIdx.has_value())
return Unexpected(normalizedIdx.error());
auto const r = detail::locateField(*cache_[normalizedIdx.value()], locator);
auto const r = locateField(*cache_[normalizedIdx.value()], locator);
if (!r)
return Unexpected(r.error());
return detail::getAnyFieldData(r.value());
return getAnyFieldData(r.value());
}
// Subsection: array length getters
@@ -323,10 +319,10 @@ WasmHostFunctionsImpl::getTxArrayLen(SField const& fname) const
return Unexpected(HostFunctionError::NO_ARRAY);
auto const* field = ctx_.tx.peekAtPField(fname);
if (detail::noField(field))
if (noField(field))
return Unexpected(HostFunctionError::FIELD_NOT_FOUND);
return detail::getArrayLen(field);
return getArrayLen(field);
}
Expected<int32_t, HostFunctionError>
@@ -340,10 +336,10 @@ WasmHostFunctionsImpl::getCurrentLedgerObjArrayLen(SField const& fname) const
return Unexpected(sle.error());
auto const* field = sle.value()->peekAtPField(fname);
if (detail::noField(field))
if (noField(field))
return Unexpected(HostFunctionError::FIELD_NOT_FOUND);
return detail::getArrayLen(field);
return getArrayLen(field);
}
Expected<int32_t, HostFunctionError>
@@ -357,10 +353,10 @@ WasmHostFunctionsImpl::getLedgerObjArrayLen(int32_t cacheIdx, SField const& fnam
return Unexpected(normalizedIdx.error());
auto const* field = cache_[normalizedIdx.value()]->peekAtPField(fname);
if (detail::noField(field))
if (noField(field))
return Unexpected(HostFunctionError::FIELD_NOT_FOUND);
return detail::getArrayLen(field);
return getArrayLen(field);
}
// Subsection: nested array length getters
@@ -368,12 +364,12 @@ WasmHostFunctionsImpl::getLedgerObjArrayLen(int32_t cacheIdx, SField const& fnam
Expected<int32_t, HostFunctionError>
WasmHostFunctionsImpl::getTxNestedArrayLen(Slice const& locator) const
{
auto const r = detail::locateField(ctx_.tx, locator);
auto const r = locateField(ctx_.tx, locator);
if (!r)
return Unexpected(r.error());
auto const& field = r.value();
return detail::getArrayLen(field);
return getArrayLen(field);
}
Expected<int32_t, HostFunctionError>
@@ -382,12 +378,12 @@ WasmHostFunctionsImpl::getCurrentLedgerObjNestedArrayLen(Slice const& locator) c
auto const sle = getCurrentLedgerObj();
if (!sle.has_value())
return Unexpected(sle.error());
auto const r = detail::locateField(*sle.value(), locator);
auto const r = locateField(*sle.value(), locator);
if (!r)
return Unexpected(r.error());
auto const& field = r.value();
return detail::getArrayLen(field);
return getArrayLen(field);
}
Expected<int32_t, HostFunctionError>
@@ -397,12 +393,12 @@ WasmHostFunctionsImpl::getLedgerObjNestedArrayLen(int32_t cacheIdx, Slice const&
if (!normalizedIdx.has_value())
return Unexpected(normalizedIdx.error());
auto const r = detail::locateField(*cache_[normalizedIdx.value()], locator);
auto const r = locateField(*cache_[normalizedIdx.value()], locator);
if (!r)
return Unexpected(r.error());
auto const& field = r.value();
return detail::getArrayLen(field);
return getArrayLen(field);
}
} // namespace xrpl

View File

@@ -10,6 +10,8 @@
#include <xrpl/tx/wasm/HostFuncWrapper.h>
#include <xrpl/tx/wasm/WasmVM.h>
#include <utility>
namespace xrpl {
using SFieldCRef = std::reference_wrapper<SField const>;
@@ -486,10 +488,9 @@ isAmendmentEnabled_wrap(void* env, wasm_val_vec_t const* params, wasm_val_vec_t*
if (slice->size() == uint256::bytes)
{
if (auto ret = hf->isAmendmentEnabled(uint256::fromVoid(slice->data())); *ret == 1)
{
if (auto const ret = hf->isAmendmentEnabled(uint256::fromVoid(slice->data()));
ret && *ret == 1)
return returnResult(runtime, params, results, ret, index);
}
}
if (slice->size() > 64)
@@ -1435,7 +1436,13 @@ trace_wrap(void* env, wasm_val_vec_t const* params, wasm_val_vec_t* results)
auto* runtime = reinterpret_cast<WasmRuntimeWrapper*>(hf->getRT());
int index = 0;
if (params->data[1].of.i32 + params->data[3].of.i32 > maxWasmParamLength)
int64_t const len = (int64_t)params->data[1].of.i32 + params->data[3].of.i32;
if (len < 0)
{
return hfResult(results, HostFunctionError::INVALID_PARAMS);
}
if (std::cmp_greater(len, maxWasmParamLength))
{
return hfResult(results, HostFunctionError::DATA_FIELD_TOO_LARGE);
}
@@ -1457,6 +1464,7 @@ trace_wrap(void* env, wasm_val_vec_t const* params, wasm_val_vec_t* results)
{
return hfResult(results, asHex.error()); // LCOV_EXCL_LINE
}
if (*asHex != 0 && *asHex != 1)
{
return hfResult(results, HostFunctionError::INVALID_PARAMS);
@@ -1473,7 +1481,14 @@ traceNum_wrap(void* env, wasm_val_vec_t const* params, wasm_val_vec_t* results)
auto* hf = getHF(env);
auto* runtime = reinterpret_cast<WasmRuntimeWrapper*>(hf->getRT());
int index = 0;
if (params->data[1].of.i32 > maxWasmParamLength)
int32_t const len = params->data[1].of.i32;
if (len < 0)
{
return hfResult(results, HostFunctionError::INVALID_PARAMS);
}
if (std::cmp_greater(len, maxWasmParamLength))
{
return hfResult(results, HostFunctionError::DATA_FIELD_TOO_LARGE);
}
@@ -1501,17 +1516,29 @@ traceAccount_wrap(void* env, wasm_val_vec_t const* params, wasm_val_vec_t* resul
auto* hf = getHF(env);
auto* runtime = reinterpret_cast<WasmRuntimeWrapper*>(hf->getRT());
if (params->data[1].of.i32 > maxWasmParamLength)
int32_t const len = params->data[1].of.i32;
if (len < 0)
{
return hfResult(results, HostFunctionError::INVALID_PARAMS);
}
if (std::cmp_greater(len, maxWasmParamLength))
{
return hfResult(results, HostFunctionError::DATA_FIELD_TOO_LARGE);
}
int i = 0;
auto const msg = getDataString(runtime, params, i);
if (!msg)
{
return hfResult(results, msg.error());
}
auto const account = getDataAccountID(runtime, params, i);
if (!account)
{
return hfResult(results, account.error());
}
return returnResult(runtime, params, results, hf->traceAccount(*msg, *account), i);
}
@@ -1524,17 +1551,29 @@ traceFloat_wrap(void* env, wasm_val_vec_t const* params, wasm_val_vec_t* results
auto* hf = getHF(env);
auto* runtime = reinterpret_cast<WasmRuntimeWrapper*>(hf->getRT());
if (params->data[1].of.i32 > maxWasmParamLength)
int32_t const len = params->data[1].of.i32;
if (len < 0)
{
return hfResult(results, HostFunctionError::INVALID_PARAMS);
}
if (std::cmp_greater(len, maxWasmParamLength))
{
return hfResult(results, HostFunctionError::DATA_FIELD_TOO_LARGE);
}
int i = 0;
auto const msg = getDataString(runtime, params, i);
if (!msg)
{
return hfResult(results, msg.error());
}
auto const number = getDataSlice(runtime, params, i);
if (!number)
{
return hfResult(results, number.error());
}
return returnResult(runtime, params, results, hf->traceFloat(*msg, *number), i);
}
@@ -1547,17 +1586,29 @@ traceAmount_wrap(void* env, wasm_val_vec_t const* params, wasm_val_vec_t* result
auto* hf = getHF(env);
auto* runtime = reinterpret_cast<WasmRuntimeWrapper*>(hf->getRT());
if (params->data[1].of.i32 > maxWasmParamLength)
int32_t const len = params->data[1].of.i32;
if (len < 0)
{
return hfResult(results, HostFunctionError::INVALID_PARAMS);
}
if (std::cmp_greater(len, maxWasmParamLength))
{
return hfResult(results, HostFunctionError::DATA_FIELD_TOO_LARGE);
}
int i = 0;
auto const msg = getDataString(runtime, params, i);
if (!msg)
{
return hfResult(results, msg.error());
}
auto const amountSliceOpt = getDataSlice(runtime, params, i);
if (!amountSliceOpt)
{
return hfResult(results, amountSliceOpt.error());
}
auto const amountSlice = amountSliceOpt.value();
auto serialIter = SerialIter(amountSlice);
@@ -1571,8 +1622,11 @@ traceAmount_wrap(void* env, wasm_val_vec_t const* params, wasm_val_vec_t* result
{
amount = std::nullopt;
}
if (!amount)
{
return hfResult(results, HostFunctionError::INVALID_PARAMS);
}
return returnResult(runtime, params, results, hf->traceAmount(*msg, *amount), i);
}

View File

@@ -120,9 +120,6 @@ runEscrowWasm(
auto const ret =
vm.run(wasmCode, hfs, gasLimit, funcName, params, createWasmImport(hfs), hfs.getJournal());
// std::cout << "runEscrowWasm, mod size: " << wasmCode.size()
// << ", gasLimit: " << gasLimit << ", funcName: " << funcName;
if (!ret)
{
#ifdef DEBUG_OUTPUT

View File

@@ -895,7 +895,7 @@ WasmiEngine::checkHlp(
// Create and instantiate the module.
if (wasmCode.empty())
throw std::runtime_error("empty nodule");
throw std::runtime_error("empty module");
int const m = addModule(wasmCode, false, imports, -1);
if ((m < 0) || !moduleWrap_)