Skip to content
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion bin/pre-commit/clang_tidy_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,19 @@ def main():
if not os.environ.get("TIDY"):
return 0

repo_root = Path(__file__).parent.parent
# Derive the repo root from git so this keeps working regardless of where
# under the tree this script lives. Fall back to the script location
# (bin/pre-commit/ is two levels below the repo root) if git is unavailable.
result = subprocess.run(
["git", "rev-parse", "--show-toplevel"],
capture_output=True,
text=True,
cwd=Path(__file__).parent,
)
if result.returncode == 0:
repo_root = Path(result.stdout.strip())
else:
repo_root = Path(__file__).parent.parent.parent
files = staged_files(repo_root)
if not files:
return 0
Expand Down
8 changes: 4 additions & 4 deletions include/xrpl/server/detail/Door.h
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,11 @@ class Door : public IOList::Work, public std::enable_shared_from_this<Door<Handl
acceptor_type acceptor_;
boost::asio::strand<boost::asio::io_context::executor_type> strand_;
bool ssl_{
port_.protocol.count("https") > 0 || port_.protocol.count("wss") > 0 ||
port_.protocol.count("wss2") > 0 || port_.protocol.count("peer") > 0};
port_.protocol.contains("https") || port_.protocol.contains("wss") ||
port_.protocol.contains("wss2") || port_.protocol.contains("peer")};
bool plain_{
port_.protocol.count("http") > 0 || port_.protocol.count("ws") > 0 ||
(port_.protocol.count("ws2") != 0u)};
port_.protocol.contains("http") || port_.protocol.contains("ws") ||
(port_.protocol.contains("ws2"))};
static constexpr std::chrono::milliseconds kInitialAcceptDelay{50};
static constexpr std::chrono::milliseconds kMaxAcceptDelay{2000};
std::chrono::milliseconds accept_delay_{kInitialAcceptDelay};
Expand Down
7 changes: 7 additions & 0 deletions include/xrpl/tx/wasm/HostFunc.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#include <xrpl/basics/Expected.h>
#include <xrpl/basics/Slice.h>
#include <xrpl/beast/insight/Event.h>
#include <xrpl/beast/utility/Journal.h>
#include <xrpl/protocol/AccountID.h>
#include <xrpl/protocol/Asset.h>
Expand Down Expand Up @@ -121,6 +122,12 @@ struct HostFunctions
return j;
}

[[nodiscard]] virtual beast::insight::Event
executionTimeEvent(std::string_view name) const
{
return {};
}

[[nodiscard]] virtual bool
checkSelf() const
{
Expand Down
10 changes: 10 additions & 0 deletions include/xrpl/tx/wasm/HostFuncImpl.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#pragma once

#include <xrpl/core/CollectorManager.h>
#include <xrpl/tx/ApplyContext.h>
#include <xrpl/tx/wasm/HostFunc.h>

Expand Down Expand Up @@ -77,6 +78,15 @@ class WasmHostFunctionsImpl : public HostFunctions
return rt_;
}

beast::insight::Event
executionTimeEvent(std::string_view name) const override
{
return ctx_.registry.get()
.getCollectorManager()
.group(std::string{name.data(), name.size()})
->makeEvent("finish_time");
}

bool
checkSelf() const override
{
Expand Down
17 changes: 16 additions & 1 deletion src/libxrpl/tx/wasm/WasmVM.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include <xrpl/tx/wasm/HostFuncWrapper.h> // IWYU pragma: keep
#include <xrpl/tx/wasm/ParamsHelper.h>

#include <chrono>
#include <cstdint>
#include <string>
#include <vector>
Expand Down Expand Up @@ -126,9 +127,16 @@ runEscrowWasm(
auto& vm = WasmEngine::instance();
// vm.initMaxPages(MAX_PAGES);

auto const start = std::chrono::steady_clock::now();

auto const ret =
vm.run(wasmCode, hfs, gasLimit, funcName, params, createWasmImport(hfs), hfs.getJournal());

hfs.executionTimeEvent("runEscrowWasm")
.notify(
std::chrono::duration_cast<std::chrono::milliseconds>(
std::chrono::steady_clock::now() - start));

if (!ret)
{
#ifdef DEBUG_OUTPUT
Expand All @@ -140,7 +148,7 @@ runEscrowWasm(
#ifdef DEBUG_OUTPUT
std::cout << ", ret: " << ret->result << ", gas spent: " << ret->cost << std::endl;
#endif
return EscrowResult{ret->result, ret->cost};
return EscrowResult{.result = ret->result, .cost = ret->cost};
}

NotTEC
Expand All @@ -154,9 +162,16 @@ preflightEscrowWasm(
auto& vm = WasmEngine::instance();
// vm.initMaxPages(MAX_PAGES);

auto const start = std::chrono::steady_clock::now();

auto const ret =
vm.check(wasmCode, hfs, funcName, params, createWasmImport(hfs), hfs.getJournal());

hfs.executionTimeEvent("preflightEscrowWasm")
.notify(
std::chrono::duration_cast<std::chrono::milliseconds>(
std::chrono::steady_clock::now() - start));

return ret;
}

Expand Down
8 changes: 4 additions & 4 deletions src/libxrpl/tx/wasm/WasmiVM.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ InstanceWrapper::setGas(std::int64_t gas) const
ModulePtr
ModuleWrapper::init(StorePtr& s, Bytes const& wasmBin, beast::Journal j)
{
wasm_byte_vec_t const code{wasmBin.size(), (char*)(wasmBin.data())};
wasm_byte_vec_t const code{.size = wasmBin.size(), .data = (char*)(wasmBin.data())};
ModulePtr m = ModulePtr(wasm_module_new(s.get(), &code), &wasm_module_delete);
if (!m)
throw std::runtime_error("can't create module");
Expand Down Expand Up @@ -711,8 +711,8 @@ WasmiResult
WasmiEngine::call(FuncInfo const& f, std::vector<wasm_val_t>& in)
{
WasmiResult ret(NR);
wasm_val_vec_t const inv =
in.empty() ? wasm_val_vec_t WASM_EMPTY_VEC : wasm_val_vec_t{in.size(), in.data()};
wasm_val_vec_t const inv = in.empty() ? wasm_val_vec_t WASM_EMPTY_VEC
: wasm_val_vec_t{.size = in.size(), .data = in.data()};

#ifdef SHOW_CALL_TIME
auto const start = usecs();
Expand Down Expand Up @@ -960,7 +960,7 @@ wasm_trap_t*
WasmiEngine::newTrap(std::string const& txt)
{
static char empty[1] = {0};
wasm_message_t msg = {1, empty};
wasm_message_t msg = {.size = 1, .data = empty};

if (!txt.empty())
wasm_name_new(&msg, txt.size() + 1, txt.c_str()); // include 0
Expand Down
29 changes: 29 additions & 0 deletions src/test/app/TestHostFunctions.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
#include <test/jtx/Env.h>
#include <test/unit_test/SuiteJournal.h>

#include <xrpl/beast/insight/Event.h>
#include <xrpl/beast/insight/EventImpl.h>
#include <xrpl/ledger/AmendmentTable.h>
#include <xrpl/ledger/detail/ApplyViewBase.h>
#include <xrpl/ledger/helpers/NFTokenHelpers.h>
Expand All @@ -18,6 +20,23 @@

namespace xrpl::test {

/**
* Lets a test assert that the WASM execution-timing path fired (and inspect
* the recorded duration) without needing a StatsD sink or a full Collector.
*/
struct RecordingEventImpl : public beast::insight::EventImpl
{
std::size_t count = 0;
value_type last{};

void
notify(value_type const& value) override
{
++count;
last = value;
}
};

struct TestLedgerDataProvider : public HostFunctions
{
jtx::Env& env;
Expand Down Expand Up @@ -54,6 +73,7 @@ struct TestHostFunctions : public HostFunctions
Bytes data;
int clock_drift = 0;
void* rt = nullptr;
std::shared_ptr<RecordingEventImpl> execTimeEvent = std::make_shared<RecordingEventImpl>();

public:
TestHostFunctions(test::jtx::Env& env, int cd = 0)
Expand All @@ -76,6 +96,15 @@ struct TestHostFunctions : public HostFunctions
return rt;
}

// Return an Event backed by our recording impl so a test can assert that
// the WASM execution was timed. The name is ignored -- every call records
// into the same impl.
[[nodiscard]] beast::insight::Event
executionTimeEvent(std::string_view name) const override
{
return beast::insight::Event(execTimeEvent);
}

Expected<std::uint32_t, HostFunctionError>
getLedgerSqn() const override
{
Expand Down
50 changes: 50 additions & 0 deletions src/test/app/Wasm_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1544,11 +1544,61 @@ struct Wasm_test : public beast::unit_test::Suite
}
}

template <typename Functor>
void
perf(TestHostFunctions& hfs, size_t runs, Functor&& f)
{
for (auto i = size_t{}; i < runs; ++i)
{
auto result = f();
BEAST_EXPECT(result);
}
BEAST_EXPECT(hfs.execTimeEvent->count == runs);
}

void
perfEscrowFinish()
{
testcase("perf escrow finish");
using namespace test::jtx;

static constexpr auto kRuns = 1000;

auto const wasm = hexToBytes(kAllHostFunctionsWasmHex);

Env env{*this};
auto hfns = TestHostFunctions{env, 0};

perf(hfns, kRuns, [&] {
return runEscrowWasm(wasm, hfns, 1'000'000, escrowFunctionName, {}).has_value();
});
}

void
perfEscrowCreate()
{
testcase("perf escrow create");
using namespace test::jtx;

static constexpr auto kRuns = 1000;

auto const wasm = hexToBytes(kAllHostFunctionsWasmHex);

Env env{*this};
auto mock = HostFunctions{env.journal};
Comment thread
TimothyBanks marked this conversation as resolved.
Outdated
auto hfns = TestHostFunctions{env, 0};

perf(hfns, kRuns, [&] { return !preflightEscrowWasm(wasm, mock, escrowFunctionName); });

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mock is passed to preflightEscrowWasm but perf() checks hfns.execTimeEvent->count — assertion always fails. Pass hfns instead:

Suggested change
perf(hfns, kRuns, [&] { return !preflightEscrowWasm(wasm, mock, escrowFunctionName); });
perf(hfns, kRuns, [&] { return !preflightEscrowWasm(wasm, hfns, escrowFunctionName); });

}

void
run() override
{
using namespace test::jtx;

perfEscrowFinish();
perfEscrowCreate();

testGetDataHelperFunctions();
testWasmLib();
testBadWasm();
Expand Down
2 changes: 1 addition & 1 deletion src/xrpld/app/main/Application.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
#include <xrpld/app/ledger/OrderBookDBImpl.h>
#include <xrpld/app/ledger/TransactionMaster.h>
#include <xrpld/app/main/BasicApp.h>
#include <xrpld/app/main/CollectorManager.h>
#include <xrpld/app/main/GRPCServer.h>
#include <xrpld/app/main/LoadManager.h>
#include <xrpld/app/main/NodeIdentity.h>
Expand Down Expand Up @@ -57,6 +56,7 @@
#include <xrpl/beast/utility/PropertyStream.h>
#include <xrpl/beast/utility/instrumentation.h>
#include <xrpl/core/ClosureCounter.h>
#include <xrpl/core/CollectorManager.h>
#include <xrpl/core/HashRouter.h>
#include <xrpl/core/Job.h>
#include <xrpl/core/NetworkIDService.h>
Expand Down
3 changes: 2 additions & 1 deletion src/xrpld/app/main/CollectorManager.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#include <xrpld/app/main/CollectorManager.h>

#include <xrpl/core/CollectorManager.h>

#include <xrpl/basics/BasicConfig.h>
#include <xrpl/beast/insight/Collector.h>
Expand Down
2 changes: 1 addition & 1 deletion src/xrpld/rpc/ServerHandler.h
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
#pragma once

#include <xrpld/app/main/Application.h>
#include <xrpld/app/main/CollectorManager.h>
#include <xrpld/rpc/detail/WSInfoSub.h>

#include <xrpl/core/CollectorManager.h>
#include <xrpl/core/JobQueue.h>
#include <xrpl/json/Output.h>
#include <xrpl/server/Server.h>
Expand Down
18 changes: 9 additions & 9 deletions src/xrpld/rpc/detail/ServerHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -165,10 +165,10 @@ ServerHandler::setup(Setup const& setup, beast::Journal journal)
port.port = endpointPort;

if ((setup_.client.port == 0u) &&
(port.protocol.count("http") > 0 || port.protocol.count("https") > 0))
(port.protocol.contains("http") || port.protocol.contains("https")))
setup_.client.port = endpointPort;

if ((setup_.overlay.port() == 0u) && (port.protocol.count("peer") > 0))
if ((setup_.overlay.port() == 0u) && (port.protocol.contains("peer")))
setup_.overlay.port(endpointPort);
}
}
Expand Down Expand Up @@ -217,7 +217,7 @@ ServerHandler::onHandoff(
using namespace boost::beast;
auto const& p{session.port().protocol};
bool const isWs{
p.count("ws") > 0 || p.count("ws2") > 0 || p.count("wss") > 0 || p.count("wss2") > 0};
p.contains("ws") || p.contains("ws2") || p.contains("wss") || p.contains("wss2")};

if (websocket::is_upgrade(request))
{
Expand Down Expand Up @@ -251,7 +251,7 @@ ServerHandler::onHandoff(
return handoff;
}

if (bundle && p.count("peer") > 0)
if (bundle && p.contains("peer"))
return app_.getOverlay().onHandoff(std::move(bundle), std::move(request), remoteAddress);

if (isWs && isStatusRequest(request))
Expand Down Expand Up @@ -301,7 +301,7 @@ void
ServerHandler::onRequest(Session& session)
{
// Make sure RPC is enabled on the port
if (session.port().protocol.count("http") == 0 && session.port().protocol.count("https") == 0)
if (!session.port().protocol.contains("http") && !session.port().protocol.contains("https"))
{
httpReply(403, "Forbidden", makeOutput(session), app_.getJournal("RPC"));
session.close(true);
Expand Down Expand Up @@ -1180,7 +1180,7 @@ parsePorts(Config const& config, std::ostream& log)
else
{
auto const count = std::count_if(result.cbegin(), result.cend(), [](Port const& p) {
return p.protocol.count("peer") != 0;
return p.protocol.contains("peer");
});

if (count > 1)
Expand All @@ -1203,12 +1203,12 @@ setupClient(ServerHandler::Setup& setup)
decltype(setup.ports)::const_iterator iter;
for (iter = setup.ports.cbegin(); iter != setup.ports.cend(); ++iter)
{
if (iter->protocol.count("http") > 0 || iter->protocol.count("https") > 0)
if (iter->protocol.contains("http") || iter->protocol.contains("https"))
break;
}
if (iter == setup.ports.cend())
return;
setup.client.secure = iter->protocol.count("https") > 0;
setup.client.secure = iter->protocol.contains("https");
if (beast::IP::isUnspecified(iter->ip))
{
// VFALCO HACK! to make localhost work
Expand All @@ -1230,7 +1230,7 @@ static void
setupOverlay(ServerHandler::Setup& setup)
{
auto const iter = std::ranges::find_if(
setup.ports, [](Port const& port) { return port.protocol.count("peer") != 0; });
setup.ports, [](Port const& port) { return port.protocol.contains("peer"); });
if (iter == setup.ports.cend())
{
setup.overlay = {};
Expand Down
Loading
Loading