logos-co/logos-protocol
1
0
Fork 0
mirror of https://github.com/logos-co/logos-protocol.git synced 2026-08-27 12:01:15 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: uint64 survives the event path and the plain wire (#30)

Browse Source 
* fix(events): the event bridge converts through the canonical helper

setEventListenerStdBridge adapts the universal event callback (name + JSON
string) to the Qt EventCallback (name + QVariantList). It is the event-path
counterpart of callMethodStdBridge, but it did the conversion itself:

    callMethodStdBridge       -> logos::nlohmannToQVariant        (canonical)
    setEventListenerStdBridge -> QJsonDocument::fromJson
                                 + QJsonValue::toVariant          (Qt's parser)

Two consequences, both measured by the LIDL conformance matrix as M6:

  * a uint64 above int64max degraded to a double. Qt 6 backs QJsonValue with
    QCborValue, so integers up to int64 DID survive — only values with no
    integral representation there fell back to double. echoUint(2^64-1) was
    exact while uintEvent(2^64-1) arrived as 1.8446744073709552e+19: same
    value, same process, one hop later.

  * canonical tagged bytes {"_bytes": ...} were not decoded, arriving as a
    QVariantMap where the method path yields a QByteArray. This never showed up
    end-to-end because the undecoded map round-trips to JSON and the python
    client decodes the tag itself — but a C++ or QML event subscriber got a map.

Both now go through logos::nlohmannArgsToQVariantList, which the generated
cdylib emitTrampoline already used. Numbers and bytes no longer depend on
whether a value left the module as a return or as an event.

Not the residue of the codec convergence, despite how M6 was originally
registered. #29 converged six copies of the VALUE codec; this was a seventh
conversion inside an ADAPTER, which that scope never touched. It is also not on
the providers' own path — a Qt provider stores its callback verbatim and a
cdylib provider already converted correctly. The one live caller is the
logoscore daemon's CoreServiceImpl, which forwards every watched module event;
that is why C++ and Rust providers measured identically.

Why it survived: the bridge appeared in the test suite once, in
test_universal_provider_dispatch.cpp, purely to satisfy the pure virtual. No
test asserted anything about an event payload. The method path got 15 contract
tests in #29; the event path got none.

tests: 11 new cells pin the bridge directly — uint64 past int64max, 2^53+1,
int64::min, large integers nested in containers, tagged bytes at top level and
at depth, plus the shapes that already worked (multi-param order, double staying
double, null elements, empty payload, the non-array raw-string fallback) so a
future rewrite cannot quietly drop them. 210/210.

verified: logos-cpp-sdk, logos-qt-sdk, logos-liblogos and logos-logoscore-cli
all green against this build; the conformance matrix goes 156 -> 158 pass with
M6's two cells retired, and the ext table stays 40/40.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(events): pin the signedness rule the convergence brings with it

nlohmannArgsToQVariantList classifies every non-negative integer as unsigned, so
a LIDL `int` event argument now arrives as ULongLong where it used to be
LongLong. That matches what nlohmannToQVariant (the method path) and the cdylib
emitTrampoline already did — the surfaces now agree — but it is an observable
metatype change that nothing asserted.

Pinned in both directions (non-negative -> ULongLong, negative -> LongLong) so
it stays a decision rather than a side effect. Value-level reads are unaffected.

212/212.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(plain): RpcValue can represent a uint64 above int64max

The plain (tcp/tcp_ssl) wire squeezed every unsigned value through int64_t, so a
LIDL `uint` above int64max wrapped — independently in each direction:

    outbound  qvariant_rpc_value.cpp  QMetaType::ULongLong -> int64_t(...)
    inbound   json_mapping.cpp        is_number_unsigned   -> get<int64_t>()

Neither wraps loudly: .get<int64_t>() past int64max returns -1 with no
exception. Two peers both running this code agreed on -1, so nothing looked
broken from inside — and no plain-tier test used an integer outside int32 range.

Measured over real tcp before the fix:

    echoUint(2^63)   -> -9223372036854775808
    echoUint(2^64-1) -> -1

This was never a wire-format constraint. Both codecs carry uint64 natively (CBOR
emits major type 0, `1b ff..ff`) and the envelope's own `id` field already
crossed this wire as uint64_t. Only RpcValue *payloads* could not represent it.

RpcValue gains a uint64_t alternative, used through `makeInteger()` and ONLY for
values above int64max — the sole case where int64_t loses information. Anything
broader would change the representation of every non-negative integer already on
this wire, and since std::variant equality compares the alternative index it
would break comparisons against int64-built values, to fix nothing. Small
unsigned values keep crossing as signed, pinned by a test so the rule stays
visible.

Also fixes an off-by-one in the QJsonValue::Double -> int64 guard while here:
double(int64max) rounds UP to exactly 2^63, so `d <= double(int64max)` admitted
2^63 and then ran int64_t(d) out of range — undefined behaviour, saturating on
arm64 and INT64_MIN on x86-64. Now a strict `<` against 2^63.

tests: 14 new. Both codecs round-trip 2^64-1 flat and nested; negatives stay
signed; the Qt boundary is exact in both directions; the narrow representation
rule and the 2^63 guard are pinned. 226/226.

verified end-to-end, cross-process, with a negative control: the new 64-bit
boundary cases in logos-logoscore-py fail on the pinned protocol over tcp with
exactly the values above, and all 68 pass with this build — on local, tcp and
tcp_ssl alike.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Dario Lipicar
2026-07-29 00:26:32 -03:00
committed by GitHub
co-authored by Claude Opus 5
parent 362b03fb1e
commit 8b8a358c8b
7 changed files with 554 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files
cpp/implementations/plain/json_mapping.cpp
+7 -1
View File
@@ -36,6 +36,7 @@ json valueToJson(const RpcValue& v)
if (v.isNull()) return nullptr;
if (v.isBool()) return v.asBool();
if (v.isInt()) return v.asInt();
if (v.isUInt()) return v.asUInt(); // > int64max: nlohmann keeps it as number_unsigned
if (v.isDouble()) return v.asDouble();
if (v.isString()) return v.asString();
if (v.isBytes()) return logos::bytesToJson(v.asBytes().data);
@@ -56,7 +57,12 @@ RpcValue jsonToValue(const json& j)
{
if (j.is_null()) return RpcValue{std::monostate{}};
if (j.is_boolean()) return RpcValue{j.get<bool>()};
if (j.is_number_integer() || j.is_number_unsigned())
// is_number_unsigned() is checked FIRST and routed through makeInteger:
// .get<int64_t>() on a value above int64max wraps silently (2^64-1 -> -1)
// with no exception, so a correct peer's uint64 used to arrive as -1.
if (j.is_number_unsigned())
return RpcValue::makeInteger(j.get<uint64_t>());
if (j.is_number_integer())
return RpcValue{j.get<int64_t>()};
if (j.is_number_float()) return RpcValue{j.get<double>()};
if (j.is_string()) return RpcValue{j.get<std::string>()};
cpp/implementations/plain/qvariant_rpc_value.cpp
+17 -2
View File
@@ -22,9 +22,14 @@ RpcValue fromJsonValue(const QJsonValue& v)
case QJsonValue::Double: {
double d = v.toDouble();
double intPart = 0.0;
// Strict `<` on the upper bound: double(int64max) rounds UP to exactly
// 2^63, so `d <= double(int64max)` admitted d == 2^63, and int64_t(d) on
// an out-of-range double is undefined behaviour — saturating to int64max
// on arm64, INT64_MIN on x86-64. The lower bound needs no such care:
// double(int64min) is exactly -2^63 and representable.
if (std::modf(d, &intPart) == 0.0 &&
d >= double(std::numeric_limits<int64_t>::min()) &&
d <= double(std::numeric_limits<int64_t>::max()))
d < 9223372036854775808.0) // 2^63, i.e. int64max + 1
return RpcValue{int64_t(d)};
return RpcValue{d};
}
@@ -53,6 +58,12 @@ QJsonValue toJsonValue(const RpcValue& v)
if (v.isNull()) return QJsonValue(QJsonValue::Null);
if (v.isBool()) return QJsonValue(v.asBool());
if (v.isInt()) return QJsonValue(static_cast<double>(v.asInt()));
// QJsonValue has no unsigned primitive and its double cannot hold the band
// above int64max exactly. This path only carries method-introspection
// METADATA (parameter descriptors), never payload values, so the lossy cast
// is acceptable here — but without this branch a uint64 would fall through
// to Null, which is worse than imprecise.
if (v.isUInt()) return QJsonValue(static_cast<double>(v.asUInt()));
if (v.isDouble()) return QJsonValue(v.asDouble());
if (v.isString()) return QJsonValue(QString::fromStdString(v.asString()));
if (v.isBytes()) {
@@ -126,7 +137,10 @@ RpcValue qvariantToRpcValue(const QVariant& v)
case QMetaType::ULongLong:
case QMetaType::UShort:
case QMetaType::UChar:
return RpcValue{int64_t(v.toULongLong())};
// makeInteger, not int64_t(): a LIDL `uint` above int64max used to wrap
// to -1 here, silently and in every direction. Values that fit int64_t
// still take the int64_t alternative, so nothing else changes.
return RpcValue::makeInteger(v.toULongLong());
case QMetaType::Float:
case QMetaType::Double:
return RpcValue{v.toDouble()};
@@ -181,6 +195,7 @@ QVariant rpcValueToQVariant(const RpcValue& v)
if (v.isNull()) return QVariant();
if (v.isBool()) return QVariant(v.asBool());
if (v.isInt()) return QVariant(static_cast<qlonglong>(v.asInt()));
if (v.isUInt()) return QVariant(static_cast<qulonglong>(v.asUInt()));
if (v.isDouble()) return QVariant(v.asDouble());
if (v.isString()) return QVariant(QString::fromStdString(v.asString()));
if (v.isBytes()) {
cpp/implementations/plain/rpc_value.h
+25
View File
@@ -3,6 +3,7 @@
#include <algorithm>
#include <cstdint>
#include <limits>
#include <stdexcept>
#include <string>
#include <utility>
@@ -60,6 +61,7 @@ struct RpcValue {
std::monostate, // null
bool,
int64_t,
uint64_t, // ONLY for values above int64max — see makeInteger()
double,
std::string,
RpcBytes,
@@ -74,6 +76,7 @@ struct RpcValue {
RpcValue(bool b) : value(b) {}
RpcValue(int i) : value(static_cast<int64_t>(i)) {}
RpcValue(int64_t i) : value(i) {}
RpcValue(uint64_t u) : value(u) {}
RpcValue(double d) : value(d) {}
RpcValue(const char* s) : value(std::string(s)) {}
RpcValue(std::string s) : value(std::move(s)) {}
@@ -81,17 +84,39 @@ struct RpcValue {
RpcValue(RpcList l) : value(std::move(l)) {}
RpcValue(RpcMap m) : value(std::move(m)) {}
// Canonical way to build an integer from an unsigned source.
//
// The uint64_t alternative exists for exactly one reason: to carry values
// int64_t cannot. It is NOT used for every non-negative integer, and that is
// deliberate — std::variant equality compares the alternative index first,
// so representing 42 as uint64_t would make RpcValue{42} != decode("42") and
// silently change the metatype of every non-negative integer already
// crossing this wire, to fix nothing. Values that fit int64_t keep their
// existing representation; only the band above int64max is new.
static RpcValue makeInteger(uint64_t u) {
if (u <= static_cast<uint64_t>(std::numeric_limits<int64_t>::max()))
return RpcValue{static_cast<int64_t>(u)};
return RpcValue{u};
}
bool isNull() const { return std::holds_alternative<std::monostate>(value); }
bool isBool() const { return std::holds_alternative<bool>(value); }
bool isInt() const { return std::holds_alternative<int64_t>(value); }
bool isUInt() const { return std::holds_alternative<uint64_t>(value); }
bool isDouble() const { return std::holds_alternative<double>(value); }
bool isString() const { return std::holds_alternative<std::string>(value); }
bool isBytes() const { return std::holds_alternative<RpcBytes>(value); }
bool isList() const { return std::holds_alternative<RpcList>(value); }
bool isMap() const { return std::holds_alternative<RpcMap>(value); }
// True for either integer alternative — use this when you care about "is a
// whole number" rather than about signedness, so a uint64 above int64max is
// not mistaken for a non-integer.
bool isIntegral() const { return isInt() || isUInt(); }
bool asBool() const { return std::get<bool>(value); }
int64_t asInt() const { return std::get<int64_t>(value); }
uint64_t asUInt() const { return std::get<uint64_t>(value); }
double asDouble() const { return std::get<double>(value); }
const std::string& asString() const { return std::get<std::string>(value); }
const RpcBytes& asBytes() const { return std::get<RpcBytes>(value); }
cpp/logos_provider_interface.cpp
+19 -6
View File
@@ -1,7 +1,6 @@
#include "logos_provider_interface.h"
#include "logos_json_convert.h"
#include <QDebug>
#include <QJsonDocument>
// ---------------------------------------------------------------------------
// LogosProviderObject — universal virtual defaults
@@ -46,11 +45,25 @@ void LogosProviderObject::setEventListenerStdBridge(EventCallback callback)
setEventListenerStd([callback](const std::string& eventName, const std::string& data) {
if (!callback) return;
QVariantList qData;
QJsonDocument doc = QJsonDocument::fromJson(
QByteArray::fromStdString(data));
if (doc.isArray()) {
for (const QJsonValue& v : doc.array())
qData.append(v.toVariant());
// Parse with nlohmann and convert with the SAME helper the method path
// uses (callMethodStdBridge above), so a value does not depend on
// whether it left the module as a return or as an event.
//
// This used QJsonDocument::fromJson + QJsonValue::toVariant. Qt 6 backs
// QJsonValue with QCborValue, so integers up to int64 survived — but a
// uint64 above int64max has no integral representation there and fell
// back to double: 18446744073709551615 arrived as 1.8446744073709552e+19,
// exact on the method path and rounded one hop later. The Qt parser also
// has no notion of the canonical {"_bytes": ...} tag, so byte payloads
// arrived as a QVariantMap and only survived because that map round-trips
// to a consumer that decodes the tag itself.
//
// parse(..., nullptr, false) is the non-throwing form: malformed input
// yields a discarded value and takes the raw-string fallback below,
// which is the behaviour QJsonDocument gave for unparseable data.
const nlohmann::json payload = nlohmann::json::parse(data, nullptr, false);
if (!payload.is_discarded() && payload.is_array()) {
qData = logos::nlohmannArgsToQVariantList(payload);
} else {
qData.append(QString::fromStdString(data));
}