Update trace method

src/libxrpl/tx/wasm/HostContext.cpp

@@ -2,16 +2,27 @@
 
 #include <xrpl/basics/Slice.h>
 #include <xrpl/basics/base_uint.h>
+#include <xrpl/basics/strHex.h>
+#include <xrpl/protocol/AccountID.h>
+#include <xrpl/protocol/Protocol.h>
 #include <xrpl/protocol/SField.h>
+#include <xrpl/protocol/STAmount.h>
+#include <xrpl/protocol/Serializer.h>
 #include <xrpl/tx/wasm/HostFunc.h>
 #include <xrpl/tx/wasm/WasmCommon.h>
 
 #include <rust/cxx.h>
+// For `TraceDataType`, which the bridge declares and this header defines.
+#include <xrpl_wasm_vm_ffi_cxxbridge/lib.h>
 
 #include <cstddef>
 #include <cstdint>
 #include <cstring>
+#include <exception>
+#include <optional>
+#include <string>
 #include <string_view>
+#include <type_traits>
 
 namespace xrpl {
 
@@ -53,6 +64,74 @@ answerScalar(rust::Slice<std::uint8_t> out, T value)
     return answer(out, reinterpret_cast<std::uint8_t const*>(&wire), sizeof(wire));
 }
 
+// A traced integer, which the guest sends as bytes rather than as a wasm scalar so that one
+// import serves every type. `std::nullopt` if the buffer is not the width the type needs.
+//
+// `memcpy` regardless of alignment, and no `reinterpret_cast` fast path: a trace must cost
+// the same whatever address the guest chose for its buffer.
+template <class T>
+std::optional<T>
+traceInt(Slice const& data)
+{
+    static_assert(std::is_integral_v<T>);
+    if (data.size() != sizeof(T))
+        return std::nullopt;
+
+    T x;
+    std::memcpy(&x, data.data(), sizeof(T));
+    return adjustWasmEndianess(x);
+}
+
+// The guest's bytes as the text a log line carries, or `std::nullopt` when they do not hold
+// the type they claim.
+//
+// The engine refuses a code that names no type before it crosses, so `type` is always one of
+// the variants; the trailing `return` is what the `switch` owes a scoped enum, not a case
+// this can meet.
+//
+// May throw: `STAmount`'s deserializer rejects malformed input that way.
+std::optional<std::string>
+traceFormat(TraceDataType type, Slice const& data)
+{
+    switch (type)
+    {
+        case TraceDataType::Int64:
+            if (auto const x = traceInt<std::int64_t>(data))
+                return std::to_string(*x);
+            return std::nullopt;
+
+        case TraceDataType::Uint64:
+            if (auto const x = traceInt<std::uint64_t>(data))
+                return std::to_string(*x);
+            return std::nullopt;
+
+        case TraceDataType::Xfloat:
+            return wasm_float::floatToString(data);
+
+        case TraceDataType::Account:
+            if (data.size() != AccountID::size())
+                return std::nullopt;
+            return toBase58(AccountID::fromVoid(data.data()));
+
+        case TraceDataType::Amount: {
+            SerialIter iter(data);
+            STAmount const amount(iter, sfGeneric);
+            return amount.getFullText();
+        }
+
+        case TraceDataType::AsHex:
+            return strHex(data);
+
+        case TraceDataType::AsText:
+            // An empty Slice has a null data(), which std::string may not be handed.
+            if (data.empty())
+                return std::string();
+            return std::string(reinterpret_cast<char const*>(data.data()), data.size());
+    }
+
+    return std::nullopt;
+}
+
 }  // namespace
 
 HostContext::HostContext(HostFunctions& hostFunctions) : hostFunctions_(hostFunctions)
@@ -102,30 +181,43 @@ HostContext::sha512Half(rust::Slice<std::uint8_t const> data, rust::Slice<std::u
     });
 }
 
-std::int32_t
-HostContext::trace(rust::Str msg, rust::Slice<std::uint8_t const> data, bool asHex) const noexcept
+void
+HostContext::trace(rust::Str msg, rust::Slice<std::uint8_t const> data, TraceDataType dataType)
+    const noexcept
 {
-    return guarded(hostFunctions_.getJournal(), kHostInternal, [&] {
-        auto const status = hostFunctions_.trace(
-            std::string_view{msg.data(), msg.size()}, Slice{data.data(), data.size()}, asHex);
-        if (!status)
-            return hfErrorToInt(status.error());
+    auto const journal = hostFunctions_.getJournal();
 
-        return *status;
-    });
-}
+    // Not `guarded`: a buffer that does not hold what it claims is an ordinary contract
+    // mistake, so it belongs in the log the contract is writing to rather than in the error
+    // log as an internal failure - and it must not become one, since there is nothing to
+    // report it to.
+    try
+    {
+        if (msg.size() + data.size() > kMaxWasmDataLength)
+        {
+            JLOG(journal.trace()) << "WasmTrace: message and data too long";
+            return;
+        }
 
-std::int32_t
-HostContext::traceNum(rust::Str msg, std::int64_t number) const noexcept
-{
-    return guarded(hostFunctions_.getJournal(), kHostInternal, [&] {
-        auto const status =
-            hostFunctions_.traceNum(std::string_view{msg.data(), msg.size()}, number);
-        if (!status)
-            return hfErrorToInt(status.error());
+        // Rendered whatever the log level: the level decides what is written, never whether
+        // the host is called, so a run costs the same on every node.
+        auto const text = traceFormat(dataType, Slice{data.data(), data.size()});
+        if (!text)
+        {
+            JLOG(journal.trace()) << "WasmTrace: data does not hold the type it names";
+            return;
+        }
 
-        return *status;
-    });
+        hostFunctions_.trace(std::string_view{msg.data(), msg.size()}, *text);
+    }
+    catch (std::exception const& e)
+    {
+        JLOG(journal.trace()) << "WasmTrace: threw: " << e.what();
+    }
+    catch (...)
+    {
+        JLOG(journal.trace()) << "WasmTrace: threw";
+    }
 }
 
 }  // namespace xrpl

src/tests/libxrpl/tx/wasm/MockHostFunctions.h

@@ -46,16 +46,12 @@ struct MockHostFunctions : HostFunctions
         (Slice const& data),
         (const, override));
 
+    // Takes the rendered text, not the guest's buffer: rendering is `HostContext`'s, so what
+    // a test asserts here is the log line a node would write.
     MOCK_METHOD(
-        (std::expected<std::int32_t, HostFunctionError>),
+        void,
         trace,
-        (std::string_view const& msg, Slice const& data, bool asHex),
-        (const, override));
-
-    MOCK_METHOD(
-        (std::expected<std::int32_t, HostFunctionError>),
-        traceNum,
-        (std::string_view const& msg, std::int64_t number),
+        (std::string_view const& msg, std::string_view const& data),
         (const, override));
 };
 

src/tests/libxrpl/tx/wasm/host_calls/Trace.cpp

@@ -1,62 +1,220 @@
+#include <xrpl/basics/Number.h>
+#include <xrpl/protocol/AccountID.h>
+#include <xrpl/protocol/Protocol.h>
+#include <xrpl/protocol/STAmount.h>
+#include <xrpl/protocol/Serializer.h>
+#include <xrpl/tx/wasm/HostFunc.h>
 #include <xrpl/tx/wasm/WasmCommon.h>
 
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
 #include <tx/wasm/MockHostFunctions.h>
 #include <tx/wasm/WasmFixture.h>
+// For `TraceDataType`, which the bridge declares and this header defines.
+#include <xrpl_wasm_vm_ffi_cxxbridge/lib.h>
 
-#include <expected>
+#include <cstdint>
+#include <format>
 #include <string>
 #include <string_view>
+#include <utility>
 
 namespace xrpl::test {
 
-using testing::Return;
+namespace {
 
-// trace — two byte inputs and a flag, no output.
+// Bytes as a WAT data segment's contents. Hex-escaped throughout, so a buffer needs no
+// thought about which of its bytes the text format would otherwise read.
+std::string
+watBytes(Bytes const& bytes)
+{
+    std::string escaped;
+    escaped.reserve(bytes.size() * 4);
+    for (auto const byte : bytes)
+        escaped += std::format("\\{:02x}", byte);
+    return escaped;
+}
+
+Bytes
+serialized(STAmount const& amount)
+{
+    Serializer s;
+    amount.add(s);
+    return s.getData();
+}
+
+}  // namespace
+
+// trace — a message, a data type, and a buffer holding what that type says. One import for
+// what were five, so what a test varies is the type rather than the function.
+//
+// The buffer arrives as bytes and leaves as text: `HostContext` renders it, and the host is
+// handed the finished line. So a test says which renderer the type selected.
 struct TraceCall : HostCallTest
 {
+    static constexpr std::int32_t kDataAt = 64;
+
+    // What the guest passes. `typeCode` rather than a `TraceDataType` so a test can send a
+    // code that names no type, which is the guest's to get wrong.
+    std::int32_t typeCode{static_cast<std::int32_t>(TraceDataType::AsText)};
+    Bytes data;
+
+    void
+    traces(TraceDataType type, Bytes bytes)
+    {
+        typeCode = static_cast<std::int32_t>(type);
+        data = std::move(bytes);
+    }
+
+    void
+    traces(TraceDataType type, std::string_view text)
+    {
+        traces(type, Bytes{text.begin(), text.end()});
+    }
+
     [[nodiscard]] std::string
     wat() const override
     {
-        return std::string{R"wat(
+        // {0} data offset, {1} the data itself, {2} the type under test, {3} its length,
+        // {4} a type the constant modules can name, {5} the data cap.
+        return std::format(
+            R"wat(
 (module
-  (import "host_lib" "trace" (func $trace (param i32 i32 i32 i32 i32) (result i32)))
+  (import "host_lib" "trace" (func $trace (param i32 i32 i32 i32 i32)))
   (memory (export "memory") 1)
   (data (i32.const 0) "note")
-  (data (i32.const 16) "\07\08")
+  (data (i32.const {0}) "{1}")
 
   (func (export "escrow_finish") (result i32)
-    (call $trace (i32.const 0) (i32.const 4) (i32.const 16) (i32.const 2) (i32.const 1)))
+    (call $trace (i32.const 0) (i32.const 4) (i32.const {2}) (i32.const {0}) (i32.const {3}))
+    (i32.const 1))
 
-  (func (export "not_as_hex") (result i32)
-    (call $trace (i32.const 0) (i32.const 4) (i32.const 16) (i32.const 2) (i32.const 0))))
-)wat"};
+  (func (export "unnamed_type") (result i32)
+    (call $trace (i32.const 0) (i32.const 4) (i32.const 0) (i32.const {0}) (i32.const 0))
+    (i32.const 1))
+
+  (func (export "past_memory") (result i32)
+    (call $trace (i32.const 0) (i32.const 4) (i32.const {4}) (i32.const 65536) (i32.const 1))
+    (i32.const 1))
+
+  (func (export "too_long") (result i32)
+    (call $trace (i32.const 0) (i32.const 4) (i32.const {4}) (i32.const {0}) (i32.const {5}))
+    (i32.const 1)))
+)wat",
+            kDataAt,
+            watBytes(data),
+            typeCode,
+            data.size(),
+            static_cast<std::int32_t>(TraceDataType::AsHex),
+            kMaxWasmDataLength);
+    }
+
+    // The line the host was handed, for a run that is expected to reach it.
+    void
+    expectTraced(std::string_view text)
+    {
+        EXPECT_CALL(host, trace(std::string_view("note"), text));
+
+        EXPECT_EQ(hostAnswer(), 1) << "the contract runs on past its trace";
     }
 };
 
-// Two borrowed regions in one call, which is the shape a single-input helper could not
-// express — so this pins that both arrive intact, and the flag with them.
-TEST_F(TraceCall, MessageDataAndFlagAllArrive)
+// The eight-byte types are the pair worth naming: the same bytes, and the type is the whole
+// difference between the two readings.
+TEST_F(TraceCall, Int64ReadsTheBufferSigned)
 {
-    EXPECT_CALL(host, trace(std::string_view("note"), BytesAre("\x07\x08"), true))
-        .WillOnce(Return(0));
+    traces(TraceDataType::Int64, Bytes(8, 0xff));
 
-    EXPECT_EQ(hostAnswer(), 0) << "a call with nothing to report answers 0";
+    expectTraced("-1");
 }
 
-TEST_F(TraceCall, HexFlagIsGuestsToChoose)
+TEST_F(TraceCall, Uint64ReadsTheSameBufferUnsigned)
 {
-    EXPECT_CALL(host, trace(testing::_, testing::_, false)).WillOnce(Return(0));
+    traces(TraceDataType::Uint64, Bytes(8, 0xff));
 
-    EXPECT_EQ(hostAnswer("not_as_hex"), 0);
+    expectTraced("18446744073709551615");
 }
 
-TEST_F(TraceCall, HostErrorBecomesContractReturnValue)
+TEST_F(TraceCall, AsTextTakesTheBufferVerbatim)
 {
-    EXPECT_CALL(host, trace).WillOnce(Return(std::unexpected(HostFunctionError::InvalidParams)));
+    traces(TraceDataType::AsText, "hello");
 
-    EXPECT_EQ(hostAnswer(), hfErrorToInt(HostFunctionError::InvalidParams));
+    expectTraced("hello");
+}
+
+TEST_F(TraceCall, AsHexEncodesTheBuffer)
+{
+    traces(TraceDataType::AsHex, Bytes{0x07, 0x08, 0xff});
+
+    expectTraced("0708FF");
+}
+
+// The zero account, so the expectation is the well-known base58 rather than a rendering of
+// whatever the renderer happened to do.
+TEST_F(TraceCall, AccountIsBase58)
+{
+    traces(TraceDataType::Account, Bytes(AccountID::size(), 0));
+
+    expectTraced("rrrrrrrrrrrrrrrrrrrrrhoLvTp");
+}
+
+TEST_F(TraceCall, AmountCarriesItsAssetIntoTheText)
+{
+    traces(TraceDataType::Amount, serialized(STAmount{XRPAmount{1000}}));
+
+    expectTraced("1000/XRP");
+}
+
+TEST_F(TraceCall, XfloatIsDecodedToItsValue)
+{
+    auto const encoded = wasm_float::floatFromIntImpl(
+        42, static_cast<std::int32_t>(Number::RoundingMode::ToNearest));
+    ASSERT_TRUE(encoded.has_value());
+    traces(TraceDataType::Xfloat, *encoded);
+
+    expectTraced("42");
+}
+
+// The width is part of the type, and a buffer that is not it holds no value to print. The
+// contract is not told: a trace answers nothing at all.
+TEST_F(TraceCall, ABufferOfTheWrongWidthIsDropped)
+{
+    traces(TraceDataType::Int64, Bytes(4, 0xff));
+
+    EXPECT_CALL(host, trace).Times(0);
+    EXPECT_EQ(hostAnswer(), 1);
+}
+
+// `STAmount`'s deserializer rejects this by throwing, which must not escape into the run.
+TEST_F(TraceCall, AMalformedAmountIsDroppedRatherThanThrown)
+{
+    traces(TraceDataType::Amount, Bytes(3, 0xff));
+
+    EXPECT_CALL(host, trace).Times(0);
+    EXPECT_EQ(hostAnswer(), 1);
+}
+
+// Zero is the code a guest sends by omission, which is why no type carries it.
+TEST_F(TraceCall, ACodeThatNamesNoTypeIsDropped)
+{
+    EXPECT_CALL(host, trace).Times(0);
+
+    EXPECT_EQ(hostAnswer("unnamed_type"), 1);
+}
+
+// The memory policy every input region is held to, on the one call that cannot report it.
+TEST_F(TraceCall, ARegionPastMemoryIsDropped)
+{
+    EXPECT_CALL(host, trace).Times(0);
+
+    EXPECT_EQ(hostAnswer("past_memory"), 1);
+}
+
+TEST_F(TraceCall, AMessageAndBufferPastTheDataCapAreDropped)
+{
+    EXPECT_CALL(host, trace).Times(0);
+
+    EXPECT_EQ(hostAnswer("too_long"), 1);
 }
 
 }  // namespace xrpl::test

src/tests/libxrpl/tx/wasm/host_calls/TraceNum.cpp

@@ -1,53 +0,0 @@
-#include <xrpl/tx/wasm/WasmCommon.h>
-
-#include <gmock/gmock.h>
-#include <gtest/gtest.h>
-#include <tx/wasm/WasmFixture.h>
-
-#include <cstdint>
-#include <expected>
-#include <limits>
-#include <string>
-#include <string_view>
-
-namespace xrpl::test {
-
-using testing::Return;
-
-// trace_num — a string and an i64, the ABI's only 64-bit parameter.
-struct TraceNumCall : HostCallTest
-{
-    [[nodiscard]] std::string
-    wat() const override
-    {
-        return std::string{R"wat(
-(module
-  (import "host_lib" "trace_num" (func $trace_num (param i32 i32 i64) (result i32)))
-  (memory (export "memory") 1)
-  (data (i32.const 0) "count")
-
-  (func (export "escrow_finish") (result i32)
-    (call $trace_num (i32.const 0) (i32.const 5) (i64.const -9223372036854775808))))
-)wat"};
-    }
-};
-
-// The extreme value on purpose: an `i64` that a truncating or sign-losing conversion anywhere
-// on the wire would visibly mangle.
-TEST_F(TraceNumCall, I64ArrivesWholeIncludingMostNegativeValue)
-{
-    EXPECT_CALL(host, traceNum(std::string_view("count"), std::numeric_limits<std::int64_t>::min()))
-        .WillOnce(Return(0));
-
-    EXPECT_EQ(hostAnswer(), 0);
-}
-
-TEST_F(TraceNumCall, HostErrorBecomesContractReturnValue)
-{
-    EXPECT_CALL(host, traceNum)
-        .WillOnce(Return(std::unexpected(HostFunctionError::IndexOutOfBounds)));
-
-    EXPECT_EQ(hostAnswer(), hfErrorToInt(HostFunctionError::IndexOutOfBounds));
-}
-
-}  // namespace xrpl::test