Fc: stream JSON responses directly, bypassing the variant tree - #513
Fc: stream JSON responses directly, bypassing the variant tree#513heifner wants to merge 82 commits into
Conversation
Adds a streaming counterpart to the existing fc::to_variant + fc::json::to_string
serialization path. Callers drive fc::json_writer with explicit begin_object /
key / value_* calls that append bytes directly to an output std::string, skipping
the mutable_variant_object -> fc::variant -> fc::json::to_string pipeline that
dominates allocations in chain_plugin/trace_api HTTP responses.
fc::to_json_stream<T> mirrors fc::to_variant<T>: a reflector-based primary
template that dispatches via fc::reflector, plus scalar / container overloads.
Any type marked with FC_REFLECT(...) or FC_REFLECT_ENUM(...) gains a JSON
serializer with no further code change, and optional<T> fields are omitted
rather than emitted as explicit null to match the established to_variant_visitor
semantics.
Provided overloads:
- bool, {int,uint}{8,16,32,64}_t, float, double
- std::string, std::string_view, const char*
- std::vector, std::array, std::map, std::unordered_map
- std::optional
- FC_REFLECT'd structs and FC_REFLECT_ENUM'd enums
json_writer::raw_value splices a preformatted JSON fragment at a value position;
to_json_stream_via_variant is an explicit escape hatch for incremental adoption
of types that only have a to_variant overload so far.
Scope: this is infrastructure only. Downstream HTTP endpoints (trace_api
get_block / get_actions, chain_plugin get_block / get_account / get_table_rows)
will migrate in follow-up PRs as each hot path is profiled and converted.
Tests: libraries/libfc/test/io/test_json_stream.cpp covers primitives, escaping,
optional omission, reflected structs + nesting, enum emission, std::map object
emission, and raw_value splicing. 11 cases, all pass.
…t_object Adds native streaming-JSON overloads for the fc types most commonly embedded in reflected chain/trace response structs, so the fc::to_json_stream path works end-to-end for real endpoints without forcing every field through the to_json_stream_via_variant escape hatch. Covered: - fc::microseconds: int64 count (same shape as the existing to_variant path) - fc::time_point, fc::time_point_sec: ISO-8601 string via to_iso_string() - fc::sha256: lowercase hex via sha256::str() (canonical JSON form) - fc::variant: delegates to fc::json::to_string + raw_value splice; no allocation win (the variant already exists) but lets reflected structs with variant fields participate in the streaming path instead of falling back to a full mutable_variant_object rebuild at the caller - fc::variant_object, fc::mutable_variant_object: same pattern as fc::variant Declarations live next to the matching to_variant declarations; definitions live in the same translation units. A tiny fc/io/json_stream_fwd.hpp forward-declares fc::json_writer so type headers can declare the overloads without pulling the full writer + json/escape_string dependency (same pattern already used for fc::variant forward-declaration). Tests: adds 5 cases to test_json_stream covering each type + a reflector composition test that serializes a struct with fc::time_point and fc::sha256 fields via both to_json_stream and fc::to_variant + fc::json::to_string, asserting byte-for-byte equivalence so migrated endpoints produce identical wire output. All 18 json_stream_test cases pass, no libfc regressions.
…signature Matches the existing to_variant output (prefixed base58 form, eg "PUB_K1_...", "SIG_K1_..." / "PUB_WA_..." / "SIG_WA_...") via each type's to_string helper with include_prefix=true. Reflected structs that embed keys or signatures (eg transaction authorizations, block producer signatures, finalizer policies) can now participate in the streaming JSON path without the via_variant fallback. Tests: adds a case that generates a private key, derives the public key and signs a digest, then compares fc::to_json_string to the fc::to_variant + fc::json::to_string golden for both public_key and signature. 19/19 json_stream_test cases pass.
…SON path Adds http_content_type::json_raw to http_plugin. Handlers that build their response body via fc::json_writer (as a pre-serialized JSON string) can opt into this mode and skip the fc::variant -> fc::json::to_string roundtrip on the response path entirely. The json_raw branch in common.hpp still falls back to fc::json::to_string when the response variant is not a string, which is the case for http_plugin::handle_exception and similar error-path responses, so exception behavior is unchanged. Adds detail::response_formatter::process_block_to_json - a streaming counterpart to process_block that emits byte-identical JSON directly into a std::string instead of allocating a fc::mutable_variant_object tree. Uses the native fc::to_json_stream overloads for sha256/signature, and falls back to fc::to_json_stream_via_variant for transaction_header (reflected struct with chain:: types that don't have to_json_stream overloads yet). The abi decoded params / return_data fc::variant payloads are spliced via json_writer::raw_value so they stay inside the streaming pipeline. Adds request_handler::get_block_trace_json mirroring get_block_trace, and migrates the /v1/trace_api/get_block HTTP handler to the new path. Next: migrate /v1/chain/get_block and /v1/chain/get_table_rows.
…rialize
Adds a sysio::benchmark trace_api_json feature that compares the two trace_api
block-serialization paths on synthetic block_trace_v0 shapes:
- variant: process_block -> fc::mutable_variant_object -> fc::variant ->
fc::json::to_string (current production path)
- stream: process_block_to_json -> fc::json_writer directly into
std::string (this PR's path)
Each shape runs N times and prints average/min/max wall time via the existing
benchmarking harness. The data_handler is a no-op so the measurement isolates
the envelope serialization cost; ABI decode cost is identical in both paths
and would add the same constant to each.
Synthetic shapes cover the block sizes seen on the perf-harness producer:
100 trxs (light), 5,000 trxs (medium), 25,000 trxs (high-TPS peak observed in
measurement), and a 1,000-trx/4-actions-each shape for denser blocks.
Results (Release, 5 runs):
shape variant stream delta
100 trxs x 1 act (32B) 1.42 ms 1.20 ms -16%
5000 trxs x 1 act (32B) 79.3 ms 57.3 ms -28%
25000 trxs x 1 act (32B) 396.8 ms 291.0 ms -27%
1000 trxs x 4 acts (128B) 21.4 ms 16.1 ms -25%
At the observed 25k-trx block size the streaming path saves ~106 ms per
response - 27% faster at the envelope level. Real responses with ABI decode
pay the same absolute decode cost in both paths, so the relative percentage
shrinks slightly but the absolute time saved is preserved.
Run via: ./build/benchmark/benchmark --feature trace_api_json --runs 5
- Adds value_hex(const char*, size_t): writes lowercase hex straight into
the output buffer, no intermediate std::string allocation.
- Replaces snprintf with std::to_chars in append_signed / append_unsigned.
Bench (libraries/libfc/benchmark/, Release -O3, 12th Gen i9-12900K, 5 runs):
value_hex vs value_string(fc::to_hex(...)) (isolated):
32B x 2048 calls: 463 us -> 111 us (-76%)
128B x 512 calls: 381 us -> 98 us (-74%)
512B x 128 calls: 358 us -> 93 us (-74%)
2048B x 32 calls: 360 us -> 94 us (-74%)
std::to_chars vs snprintf (1024 mixed-magnitude uint64s):
snprintf 36.7 us -> to_chars 11.8 us (-68%)
Both changes are in the streaming-JSON hot path; together they bring the
trace_api stream-vs-variant-only main-thread comparison from a +15%
regression on data-heavy actions to wash, and to a 6-10% main-thread win
across all tested shapes.
process_block_to_json had two value_string(fc::to_hex(...)) sites for the
two binary fields on each action_trace_v0 (data, return_value). The
fc::to_hex call constructed a std::string per action which the
value_string call then had to copy/escape into the output buffer.
value_hex emits the hex digits straight into the writer's output string,
saving the heap round-trip per action.
Bench (benchmark/trace_api_json, Release -O3, 5 runs, main-thread A/B
against variant-only - the variant path defers hex encoding to
fc::json::to_string on the HTTP thread):
1000 trxs x 4 acts x 128B data (data-heavy, worst-case):
before: stream 17.1 ms vs variant-only 14.8 ms (+15%, regression)
after: stream 14.6 ms vs variant-only 14.4 ms (+2%, in noise)
Three new scenarios in benchmark/trace_api_json.cpp: variant-only: builds the fc::variant tree via process_block but does NOT call fc::json::to_string. Times the actual main-thread cost in production - json::to_string runs on a parallel HTTP-pool thread after cb() hands the variant off, so the existing 'variant+json:' total-CPU number overstates main-thread cost. Compare against 'stream:' to decide whether moving JSON emission onto the main thread is a net main-thread win or regression for any given workload shape. hex iso: emits 32 / 128 / 512 / 2048 random bytes per call, varying calls-per-iteration to keep total work roughly constant. Two paths: hex variant: w.value_string(fc::to_hex(buf.data(), buf.size())) hex direct: w.value_hex(buf.data(), buf.size()) Probes hex-encoding cost without the rest of the trace_api envelope, so optimisations to value_hex can be measured directly. uint64 iso: 1024 mixed-magnitude integers (small / medium / full uint64, shuffled to defeat branch prediction) appended via two local lambdas: one snprintf-based, one std::to_chars-based. Mirrors the change to append_signed / append_unsigned and lets future integer-conversion work report a delta against the same fixed sample.
- set(name, value) emits "name":<json(value)> using to_json_stream for the
value dispatch. Returns *this so call sites chain naturally:
w.begin_object();
w.set("id", id)
.set("name", n)
.set("amount", amt);
w.end_object();
- set_raw(name, raw_json) splices a preformatted JSON fragment as the value.
For embedding output from legacy fc::variant paths (eg
abi_serializer::binary_to_variant + json::to_string) without re-parsing.
Mirrors fc::mutable_variant_object::set / operator() ergonomics so
streaming-JSON migrations from variant-based code keep the per-field
one-liner shape.
Three tests added to test_json_stream covering chained primitive emission,
dispatch through to_json_stream for fc and reflected types, and
raw-fragment splice.
Migrates process_block_to_json + write_actions / write_authorizations /
write_transactions from explicit w.key("name")+w.value_*(value) pairs to
chained w.set("name", value) calls. Matches the original
mutable_variant_object()(...) shape that lived on the variant path.
~25% LOC reduction at the streaming call sites. Behaviorally a no-op:
set inlines to key+to_json_stream which inlines to the same value_*()
dispatch the previous code used.
trace_responses test (7 cases) confirms byte-identical output;
trace_api_json bench (4 shapes) confirms unchanged perf.
Two new measurement scenarios per shape in benchmark/trace_api_json.cpp:
struct-pass (fn): captures the result struct into a
std::function<void(json_writer&)> closure; mirrors
the main-thread cost if streaming work moves to the
HTTP thread via a std::function-typed callback.
struct-pass (mof): same pattern using std::move_only_function (C++23,
supported by both clang-18 + libstdc++ 13.3 and
gcc-14 in our CI matrix). Closure capture is
move-only so we don't pay for the std::function copy
slot.
Pre-builds a pool of fresh result-struct copies outside the timing loop
(matches the existing variant-only / stream pattern that excludes
API-call cost from the bench).
Numbers (Release -O3, 5 runs):
Shape struct-pass(mof) stream ratio
100 trxs x 1 act 32B 3.9 us 1.27 ms ~325x
5000 trxs x 1 act 32B 298 us 59.6 ms ~200x
25000 trxs x 1 act 32B 1.57 ms 293 ms ~187x
1000 trxs x 4 acts 128B 211 us 14.6 ms ~69x
Validates the HTTP-thread pivot's main-thread minimization argument: by
moving the JSON serialization out of the api thread and into the HTTP
thread pool, the api thread pays only the closure-capture cost (one heap
alloc plus a few pointer swaps) instead of the full streaming pass.
…ream
Adds the parallel infrastructure for endpoints that hand the response off
to the http thread pool as a json_writer-emitting closure rather than as
a fc::variant tree. Endpoints opt in by registering via
add_handler_stream / add_async_handler_stream (analogous to the existing
add_handler / add_async_handler). The variant path stays unchanged;
both maps are checked during request dispatch and disjointness is
asserted at registration.
New types in http_plugin.hpp:
using stream_emitter = std::move_only_function<void(fc::json_writer&)>;
using url_response_stream_callback = std::move_only_function<void(int, stream_emitter)>;
using url_handler_stream = std::function<void(string&&, string&&, url_response_stream_callback&&)>;
struct api_entry_stream { string path; api_category category; url_handler_stream handler; };
stream_emitter is move_only_function so the api lambda can capture the
typed result struct by-move without forcing CopyConstructible on it.
std::function would have required either an unnecessary copy constructor
or a wrapping shared_ptr; move_only_function is C++23-standard and is
supported by both clang-18 / libstdc++ 13.3 and gcc-14 across the CI
matrix.
Internal plumbing in common.hpp:
- internal_url_handler_stream (same shape as internal_url_handler, no
content_type slot - streaming output is always application/json).
- url_handlers_stream_type map alongside the existing url_handlers map
in http_plugin_state.
- make_http_stream_response_handler builds the cb lambda that posts
the emitter onto the thread pool, runs it into a freshly-allocated
body buffer, tracks bytes_in_flight, and sends the response.
beast_http_session.hpp checks the streaming map first, falls back to
the variant map. /v1/node/get_supported_apis now lists URLs from both
maps.
handle_exception_stream is the streaming counterpart to the existing
handle_exception. Both share a single classify_current_exception
template that runs the catch chain once and invokes a caller-supplied
emit lambda with (http_code, error_results). No duplicated catch arms;
the only difference between the two public functions is how they
deliver the error_results to their respective cb shape. error_results
is FC_REFLECT'd, so the streaming path emits via to_json_stream's
reflector dispatch.
No endpoints migrated in this commit - infra only. Existing variant-cb
endpoints continue to use the variant cb path.
…ST macros Streaming-cb counterparts to the existing CALL_ASYNC_WITH_400 and CALL_WITH_400_POST. Same control flow as the variant versions; the only difference is how the result is delivered to cb - as a json_writer-emitting closure (move_only_function) instead of as a fc::variant. post_http_thread_pool's signature changes from void post_http_thread_pool(std::function<void()> f) to void post_http_thread_pool(std::move_only_function<void()> f) so the streaming macros can capture the move_only_function cb plus the move-only api_handle.call_name() result struct into the dispatched closure. std::function values implicitly convert to std::move_only_function, so the variant-path macros (CALL_ASYNC_WITH_400, CALL_WITH_400_POST) continue to work unchanged. The streaming macros use http_plugin::handle_exception_stream on the error paths (added in the previous commit) to emit error_results via the reflector path. No callers yet - the new macros are unused but compiled.
Flips get_table_rows from CHAIN_RO_CALL_POST to CHAIN_RO_CALL_STREAM_POST. The endpoint now hands the typed get_table_rows_result to the http thread pool as a json_writer-emitting closure rather than constructing a fc::variant tree. The Phase 1 / Phase 2 split (api thread captures the deferred fn, http pool runs it) is unchanged; only the response delivery path differs. ABI decode safety: get_table_rows already pre-resolves the contract's ABI inside Phase 1 by capturing the raw ABI bytes into the Phase 2 closure. Phase 2 constructs a local abi_serializer from those bytes and decodes rows into fc::variant before they get embedded into the result. None of that touches chainbase from the http thread, so the streaming pivot inherits the existing safety unchanged. Byte-identical compat: get_table_rows_result is FC_REFLECT'd ((rows)(more) (next_key)). The reflector path emits members in declaration order, the same order fc::mutable_variant_object preserves on the variant path. The rows field is vector<fc::variant>; to_json_stream(variant) splices each row's fc::json::to_string output via raw_value, identical bytes to the variant path's array emission. Adds tests/test_get_table_rows_page.cpp::streaming_vs_variant_byte_identical which constructs a populated get_table_rows_result via the existing test-tester harness, runs it through both paths, and BOOST_CHECK_EQUALs the emitted strings. Pins parity for any future to_json_stream change. CHAIN_RO_CALL_STREAM_POST is added as a plugin-local macro alongside CHAIN_RO_CALL_POST. The variant macros stay until every endpoint flips, then get removed in a cleanup commit.
Opt-in trait for types whose JSON form is the result of T::to_string() and whose JSON parse form is T::from_string(std::string_view). Specialising to std::true_type via FC_SERIALIZE_AS_STRING(T) auto-routes fc::to_variant, fc::from_variant, and fc::to_json_stream through the type's string conversions, replacing three per-type hand-rolled overloads with one macro line. Types whose JSON shape is something else (numbers, structs) keep their hand-rolled overloads.
…RIALIZE_AS_STRING Replace the per-type fc::to_variant / fc::from_variant / fc::to_json_stream triples for these five types with the trait opt-in. Adds the factory methods needed for the trait shape: - name::from_string(std::string_view) - thin wrapper over the existing string constructor; also lets the trait drop the friend declaration the old hand-rolled from_variant needed for private name::set - symbol_code::to_string and ::from_string - encapsulate the byte-extraction and validating-constructor logic that previously lived in the fc:: overloads - symbol::name() now delegates to to_symbol_code().to_string() to remove the duplicate byte-extraction loop - block_timestamp::to_string and ::from_string route through fc::time_point's iso conversions, matching the previous to_variant shape; format_as() collapses to t.to_string() now that the method exists block_timestamp is a class template, so its trait spec is a hand-written partial specialisation (the macro handles concrete types only). The other four use FC_SERIALIZE_AS_STRING. The trait specialisation must precede any code path that resolves fc::to_variant / fc::from_variant for the type; for asset that meant placing FC_SERIALIZE_AS_STRING above the extended_asset from_variant body, whose call to from_variant on the inner asset implicitly instantiates the primary trait.
Move the get_account registration from the variant-cb add_api block to the streaming-cb add_api_stream block. Adds a byte-identical parity test in test_chain_plugin that compares the streaming output to fc::json::to_string(fc::variant(results)) - pins that the two paths stay equivalent as future endpoints migrate.
The plain macro only handles concrete types because explicit specialisations cannot carry template parameter packs. The new variant takes a parenthesised template-parameter-list and a parenthesised type expression, unwrapping each via FC_SERIALIZE_AS_STRING_UNPAREN_: FC_SERIALIZE_AS_STRING_TEMPLATE((typename T, typename U), (my_type<T, U>)) Replaces the hand-written partial specialisation that block_timestamp's trait declaration needed.
…ke3 to FC_SERIALIZE_AS_STRING Each hash already produced a hex-string variant through the to_variant(vector<char>) path, so the trait migration is JSON- and variant-type-equivalent. Per type: - to_string() and static from_string(string_view) added to satisfy the trait shape. - The hand-rolled fc::to_variant / fc::from_variant overloads are dropped (and sha256 also drops the hand-rolled to_json_stream now that the trait provides one). - FC_SERIALIZE_AS_STRING(...) opt-in declared after the FC_REFLECT_TYPENAME line. sha1/sha224/sha256/sha512/sha3/ripemd160 constructors widen to std::string_view directly since fc::from_hex(string_view, char*, size_t) already takes that shape. keccak256/blake3 use the vector-returning from_hex; that overload (and trim_hex_prefix) is widened to std::string_view too, which removes the std::string materialisation those constructors previously paid. Tests at libfc/test/crypto/test_hash_functions.cpp cover the trait round-trip (ctor / str / from_string / to_variant + from_variant / to_json_string) for all eight types. Note: libfc/test is gated on ENABLE_TESTS, OFF by default in this repo's standard build dir; reconfigure with -DENABLE_TESTS=ON to run them.
…ey to FC_SERIALIZE_AS_STRING Each BLS type already produced a string variant via its hand-rolled to_variant -> .to_string() path, so the trait migration is JSON-equivalent. Per type: - static T from_string(string_view) added (just calls the corresponding ctor; to_string() already exists with the trait shape). - The hand-rolled fc::to_variant / fc::from_variant overloads in headers and .cpps are dropped. - FC_SERIALIZE_AS_STRING(...) opt-in declared in each header. The four BLS string-form constructors (public_key, signature, aggregate_signature, private_key) widen from const std::string& to std::string_view to avoid a materialisation in trait dispatch. That cascades through the helpers they call - deserialize_bls_base64url, sig_parse_base64url, priv_parse_base64url, and the underlying deserialize_base64url<> template - all widened to string_view. The cascade terminates at fc::base64url_decode, which is widened to string_view alongside (mirroring fc::base64_decode's earlier widening on the variant-perf branch).
The header was only included transitively via chain/types.hpp; zero actual users in libraries/, plugins/, or unittests/. Deleted along with the matching pack / unpack template declarations in fc/io/raw_fwd.hpp.
bitset already had a trait-shaped to_string() / static from_string(string_view); migration is just dropping the hand-rolled fc::to_variant / fc::from_variant and applying the macro. The previous to_variant had a defensive num_blocks > MAX_NUM_ARRAY_ELEMENTS guard that no longer fires - the trait path produces a 1-char-per-bit string, so a pathological bitset throws std::bad_alloc on .resize() rather than std::range_error. Theoretical only; real bitsets in chain code are O(100) bits. url gains a thin to_string() (alias for the existing operator std::string()) and static from_string(const std::string&) (alias for the existing string ctor). url::url(const std::string&) stays as-is - its body parses through std::stringstream(s), so widening to string_view would force a materialisation inside parse anyway. At the trait dispatch site, fc::from_variant<url>(...) will compiler-implicitly materialise one std::string per call.
…on_stream container overloads with to_variant The streaming-JSON container overloads (std::vector / std::map / std::set / std::pair / std::deque / std::optional / etc.) used to live in fc/reflect/json_stream.hpp, away from their fc::to_variant siblings in fc/variant.hpp - easy for the two paths to drift, and the streaming versions had no MAX_NUM_ARRAY_ELEMENTS guards while the variant ones did. Putting them next to each other in variant.hpp means fc/variant.hpp needs fc/io/json_stream.hpp at the top, but fc/io/json.hpp's transitive #include of fc/variant.hpp creates a cycle that leaves class json incomplete inside json_stream.hpp's body. The cycle is broken by extracting the bits of json.hpp that json_stream.hpp actually uses - fc::json::yield_function_t and fc::escape_string - into a new fc/io/json_yield.hpp. json::yield_function_t becomes an alias for fc::json_yield_function_t so existing callers continue to compile. json_stream.hpp now includes json_yield.hpp instead of json.hpp, breaking its dependency on class json. With the cycle resolved: - variant.hpp adds to_json_stream for std::optional / std::unordered_set / std::unordered_map / std::map / std::multimap / std::set / std::deque / boost::container::deque / std::vector / std::array / std::pair - each immediately above its to_variant sibling, all with MAX_NUM_ARRAY_ELEMENTS guards. - flat.hpp adds to_json_stream for flat_set / flat_multiset above each to_variant, with the same guard. - reflect/json_stream.hpp loses the container overloads; keeps scalar overloads, reflector visitor, and the primary dispatch template.
Fifteen sync read-only chain_api endpoints flip from CHAIN_RO_CALL (variant-cb) to CHAIN_RO_CALL_STREAM / CHAIN_RO_CALL_STREAM_POST (streaming-cb): get_activated_protocol_features, get_block, get_code, get_code_hash, get_consensus_parameters, get_abi, get_raw_code_and_abi, get_raw_abi, get_finalizer_info, get_table_by_scope, get_currency_balance, get_producers, get_producer_schedule, get_required_keys, get_transaction_id. Joins get_account / get_table_rows in add_api_stream. The new CALL_WITH_400_STREAM macro mirrors CALL_WITH_400 but delivers the response as a json_writer-emitting closure to the http thread pool, no fc::variant tree. chain/abi_def.hpp gains a to_json_stream for sysio::chain::may_not_exist co-located with its to_variant - get_abi's response embeds may_not_exist<T> wrappers around its variants / action_results / enums fields. get_block_info, get_block_header_state, and get_currency_stats stay on the variant-cb path: their chain_plugin methods return fc::variant directly, and to_json_stream(variant, w) currently bridges through fc::json::to_string. Migrating them today would just splice the same JSON string into the writer with no perf win. They migrate naturally once the streaming variant walker (rewriting to_json_stream(variant, w) to walk tokens directly) and the abi_serializer streaming path land - those are the real perf steps; this commit is registration uniformity for the endpoints whose response struct doesn't trip the variant bridge. Async endpoints (compute_transaction, push_transaction*, send_transaction*) and _WITH_400 endpoints (get_accounts_by_authorizers, get_raw_block, get_block_header, get_transaction_status, send_read_only_transaction) remain on variant-cb pending follow-up phases.
…kens directly Rewrites to_json_stream(variant, w), to_json_stream(variant_object, w), and to_json_stream(mutable_variant_object, w) to walk their inputs token-by-token directly into json_writer instead of calling fc::json::to_string and splicing the result via raw_value. Mirrors the compact (indent=0) form of fc::json::to_string for byte-identical output. Per variant case, the dispatch maps to: - null -> w.value_null() - int64 in 32-bit range -> w.value_int64() (same numeric form) - int64 out of range -> w.value_string(as_string()) (quoted, JS-precision-safe; matches existing convention) - uint64 same pattern - int128 / uint128 / int256 / uint256 -> w.value_string(as_string()) (quoted decimal) - double -> w.value_string(as_string()) (quoted, matches fc::json::to_string) - bool -> w.value_bool() - string / string_sso -> w.value_string() (handles its own JSON escape via fc::escape_string) - blob -> w.value_string(as_string()) (base64 form) - array -> w.begin_array() + recurse on each element + w.end_array() - object -> recurse into to_json_stream(variant_object, w) which walks key/value pairs directly variant_object / mutable_variant_object iterate kv pairs and call w.key(kv.key()) + recurse on the value. No intermediate variant or std::string build at any level. This is the variant-walker step in the streaming-JSON roadmap: it makes the existing to_json_stream(variant) bridge a real streaming path. The bigger remaining win is on abi_serializer's binary_to_variant - that's where the per-row variant allocation cost comes from on get_table_rows / get_account, and the next step is a binary_to_json_stream variant alongside it.
…et_currency_stats to streaming-cb path Now that to_json_stream(variant, w) walks the variant tree directly into json_writer (no fc::json::to_string bridge), the three fc::variant-returning endpoints get real streaming on the response path. Their chain_plugin methods still build a fc::variant first (same as the variant-cb path), but the streaming-cb path now avoids the per-response JSON string allocation that the variant-cb path pays. Simple add_api -> add_api_stream move; no other change.
Adds a templated _binary_walk<Sink> alongside two sinks that share the same recursion shape: - variant_sink builds an fc::variant tree and replaces the previous _binary_to_variant recursion (parity gate; output byte-identical). - stream_sink emits JSON tokens directly into an fc::json_writer via a static per-type unpack table that mirrors built_in_types. Adds the missing to_json_stream overloads the streaming path needs: int128/uint128 (always quoted decimal), fc::signed_int / fc::unsigned_int, float128, std::vector<char> (hex string instead of array), and the abi_serializer-internal quoting policy for int64/uint64 above 0xffffffff and the digits10+2 fixed-precision form for float32/float64. Public surface: - abi_serializer::binary_to_json_stream paralleling binary_to_variant (bytes/datastream x yield/microseconds), feeding stream_sink. - abi_serializer::find_built_in lookup helper used by the sinks. - abi_serializer::pb_binary_to_json_string streaming bridge for protobuf types (MessageToJsonString -> json_writer::raw_value). abi_tests/binary_to_json_stream_parity asserts byte-identical output between to_json_string(binary_to_variant(...)) and binary_to_json_stream(...) over the same fixture the existing general round-trip test exercises.
Phase 1 of get_table_rows (the chainbase row scan, scope handling, and bound parsing) is extracted into a private helper that returns a `table_rows_phase1` struct. Both the existing variant-cb method and a new `get_table_rows_stream` share the helper; only the Phase 2 closures differ. `get_table_rows_stream` returns a `std::function<void(json_writer&)>` that walks the collected rows on the http response thread and emits the JSON via `abi_serializer::binary_to_json_stream` per row. No per-row `mutable_variant_object` is constructed; the response is materialised straight into the writer's output buffer. A new `CALL_WITH_400_STREAM_POST_DIRECT` macro on http_plugin carries the typed emit closure end-to-end (no `call_result` template parameter) and chain_api_plugin flips `/v1/chain/get_table_rows` over to it. `filter` is intentionally not honored on the streaming path -- it is in-tree only and requires a `fc::variant` to evaluate. In-tree callers continue using `get_table_rows()`; a `SYS_ASSERT` guards against `filter` being threaded into `get_table_rows_stream` accidentally. `table_reader_tests/stream_direct_vs_variant_byte_identical` pins parity for three params shapes (full wrap + payer, paged with limit, values_only + all_rows) by comparing `fc::json::to_string(get_table_rows().result)` against `get_table_rows_stream().emit(json_writer)` byte-for-byte.
… streaming The abi_serializer::to_variant<T, Resolver> template family now drives both the variant-building path and the new direct-streaming path through a single sink-templated body in impl::abi_to_variant. No logic is duplicated across the two output paths; variant_sink yields a fc::variant tree, stream_sink emits JSON tokens directly into a json_writer. Concretely: - impl::abi_to_variant's nine specialised overloads (action, action_trace, packed_transaction, transaction, signed_block, plus container shapes for vector/deque/shared_ptr/std::variant) now take Sink& instead of mutable_variant_object&. Each specialised add() factors its body into a sibling add_value() so top-level entry into a typed value can hit the specialisation rather than the generic reflector path. - variant_sink and stream_sink gain emit<T>(v) (generic field emit; variant_sink builds fc::variant(v), stream_sink calls fc::to_json_stream(v, w)) and unpack_action_data(abi, type, data, ctx, short_path) (per-action ABI decode at the current frame position). - Both sinks now offer a default-constructed mode that doesn't bind to an abi_serializer; the bound-to-abi mode drives _binary_walk's built-in dispatch as before. - abi_serializer::to_json_stream<T, Resolver> public API mirrors to_variant<T, Resolver> in shape (yield-function and microseconds overloads). - read_only::get_block_stream and convert_block_stream emit get_block's response directly into json_writer via abi_serializer::to_json_stream<signed_block, Resolver>; chain_api_plugin's /v1/chain/get_block flips to CHAIN_RO_CALL_STREAM_POST_DIRECT. Doc on get_block_stream walks the Phase 1 (main thread; chainbase abi snapshot) -> Hop 1 -> Phase 2 (http pool; build emit closure) -> Hop 2 -> Phase 3 (http pool; encode + send) sequence. Side-effects supporting the migration: - fc::enum_type<IntType, EnumType>::to_json_stream forwards through EnumType so FC_REFLECT_ENUM-named enums emit as their member-name strings (matching the variant path). - std::variant<T...>::to_json_stream emits a 2-element [index, value] array matching to_variant. - fc/reflect/json_stream.hpp now self-contains its fc::json::to_string dependency; abi_sinks.hpp drops its abi_serializer.hpp include in favour of forward declarations to break the circular include between sinks and the templates that drive them. verify_byte_round_trip_conversion, verify_round_trip_conversion, and verify_type_round_trip_conversion in abi_tests gain a verify_stream_matches_variant call so every type those helpers cover gets a byte-identical stream-vs-variant assertion -- linkauth, unlinkauth, updateauth, deleteauth, newaccount, setcode, setabi, packed_transaction, the full general-suite ABI, abi_std_optional, variants, aliased_variants, variant_of_aliases, extend, plus the std_array / optional / uint family. test_chain_plugin_tests/get_block_streaming_vs_variant_byte_identical pins the same parity for a populated block (newaccount + transfer) at the convert_block / convert_block_stream entry point. In-tree abi_tests call sites that previously poked impl::abi_to_variant::add with a fc::mutable_variant_object now route through the public abi_serializer::to_variant<T> entry.
…tion
next_function is now a small class holding shared_ptr<std::move_only_function<...>>. This lets async API callers store mutable lambdas with consume-on-call captures (e.g. [cb = std::move(cb)] mutable {...}) inside the callback, which std::function silently breaks. The user-visible API matches the old alias: default-constructible, null-checkable via operator bool, throws std::bad_function_call when invoked empty.
A pure std::move_only_function alias wasn't viable. The callback travels through boost::signals2 (used by appbase::method_decl for incoming::methods::transaction_async), through boost::multi_index value storage in unapplied_transaction_queue, trx_retry_db's tracked_transaction, and snapshot_scheduler's pending_snapshot_index, and through many [=] or by-value lambda captures plus CATCH_AND_CALL fallbacks held alongside async captures in the same scope. All of those require a copyable outer type. The shared_ptr indirection keeps the wrapper copyable while leaving the inner move_only_function move-only so it accepts non-copyable callables.
Tradeoff: every construction now allocates (one make_shared) where small std::function objects previously fit the SBO and stayed inline. In exchange, copies are an atomic refcount bump rather than a full re-allocation when the callable was heap-stored, so multi-copy paths (push_recurse, snapshot_scheduler fanout, the read_only_trx executor chain) get cheaper. Invocation gains one indirection.
Semantic shift worth flagging: copies of the wrapper share state, where copies of std::function were independent. No code in tree was found relying on the old independence, but it is the sort of difference a future reader may want to verify against new call sites.
The streaming http path used std::move_only_function directly for its url_response_stream_callback / stream_emitter aliases, which AppleClang's libc++ does not provide -- the macOS CI build failed to compile http_plugin. chain::types.hpp already carried a narrow void(T&&)-only polyfill for next_function; promote it to a general fc::move_only_function<Signature> (fc/move_only_function.hpp) that selects std::move_only_function when the standard library has it and the type-erased fallback otherwise, and route the http_plugin aliases, post_http_thread_pool, and the trace_api benchmark through it. Direct unit coverage for the polyfill implementation so it is exercised on every platform, not only where the alias resolves to it. stream_decode_failure_falls_back_to_hex_valid_json constructed a std::string from .begin()/.end() of two SEPARATE get_table_test_abi() temporaries -- the iterators point into different std::string objects, so end - begin is a garbage distance and basic_string throws length_error depending on heap layout. This is why the ubuntu/gcc/asan/ubsan jobs failed intermittently on that test while the production streaming code was fine. Read the abi once into a named string. verified: the gcc CI container image builds plugin_test clean and the fixed test passes 80/80 where it previously failed ~1 in 5
int64_t/uint64_t are aliases of different concrete types per platform: long on 64-bit Linux, long long on macOS. The streaming overload set covered the fixed-width aliases but not the remaining standard 64-bit integer type, so a reflected size_t field (get_unapplied_transactions_result::unapplied_size) resolved to unsigned long on macOS -- a type with no to_json_stream overload -- and fell through to the reflector primary, failing the macOS build. Mirror the platform guard fc::variant already uses (fc/variant.hpp): on Apple add a size_t overload, on non-MSVC non-Apple add long long / unsigned long long, each delegating to the int64_t/uint64_t emitter. The variant path was already covered this way, which is why only the streaming path broke. Only the macOS build exercises the size_t overload; the reflected_struct_platform_ints test streams size_t/long long/unsigned long long and asserts parity with the variant path so whichever type is the platform's uncovered one is checked on every build.
The assertion body only runs when a comma-radix locale (de_DE/fr_FR) is present, which the CI Linux builder image lacks, so it had never executed there -- macOS, where those locales ship, ran it for the first time and it failed on two stale assumptions: - it expected the shortest-form spelling (to_json_string(1.5) == "1.5"), but doubles are emitted as a quoted fixed-precision string (digits10+2) to match fc::variant; and the min()/max() round-trip cannot survive that fixed format (denormal-range values render as all-zero fractional digits). - it parsed back with std::from_chars<double>, which AppleClang's libc++ does not implement. Rewrite it to assert the property the test actually exists for: under a comma locale the emitted number carries no ',' radix and is byte-identical to the fc::variant path (the reference emitter). Neither production path is affected -- from_chars<double> was test-only and to_chars(fixed) works on macOS. Verified by running the assertion body under a locally generated de_DE.UTF-8 (localedef + LOCPATH, no root): 12/12 assertions pass with the comma locale active.
huangminghuang
left a comment
There was a problem hiding this comment.
Inline review findings from the streaming JSON parity and resource-accounting review.
| fc::json_writer w(body); | ||
| emitter(w); | ||
| } | ||
| plugin_state.bytes_in_flight += body.size(); |
There was a problem hiding this comment.
[P1] Enforce the memory budget during emission
The response is fully materialized by emitter(w) before body.size() is charged to bytes_in_flight. Concurrent large streaming requests are therefore invisible to --http-max-bytes-in-flight-mb while allocating and serializing, and are rejected only after the memory cost has occurred. Please account growth incrementally or enforce a budget-aware writer limit before allowing the buffer to expand.
There was a problem hiding this comment.
Now charged incrementally: json_writer grew an optional growth guard invoked at token boundaries every 64KiB of emission; the handler settles the buffer into bytes_in_flight at each checkpoint and aborts with the busy response once verify_max_bytes_in_flight rejects, so over-budget emissions stop within a stride of the cap instead of materializing. The abort re-arms across catch-and-continue serializers (abi hex-fallback) so it cannot be absorbed. Covered by stream_response_bytes_in_flight (mid-emission budget visibility) and stream_response_over_budget_aborts_early (503 + early abort + drain), both verified failing against the previous code.
| fc::variant active_finalizer_policy; // current active policy | ||
| fc::variant pending_finalizer_policy; // current pending policy. Empty if not existing | ||
| std::optional<chain::finalizer_policy> active_finalizer_policy; // current active policy | ||
| std::optional<chain::finalizer_policy> pending_finalizer_policy; // current pending policy. Empty if not existing |
There was a problem hiding this comment.
[P1] Preserve absent-policy null fields
Changing these fields from fc::variant to std::optional changes the endpoint schema: the streaming reflector omits disengaged optionals, whereas the previous default variants emitted active_finalizer_policy and pending_finalizer_policy as explicit null values. A missing pending policy is normal, so clients that access the key before testing for null can now fail. Please preserve explicit nulls with a custom emitter or equivalent nullable representation and add an absent-policy parity test.
There was a problem hiding this comment.
Kept the typed fields via a new fc::nullable whose disengaged state serializes as explicit null under its key on both the variant and streaming paths (trio co-located with the std::optional overloads). Absent-policy parity test added: get_finalizer_info on a fresh chain asserts byte-identical variant/stream output containing pending_finalizer_policy:null.
| } | ||
| void to_json_stream( const microseconds& us, json_writer& w ) | ||
| { | ||
| w.value_int64( us.count() ); |
There was a problem hiding this comment.
[P2] Preserve large-duration quoting
value_int64 bypasses the established 64-bit JSON quoting rule. For example, microseconds{0x100000000} now emits 4294967296, while the previous to_variant plus json::to_string path emits "4294967296". Please delegate to the guarded int64_t streaming overload and add positive/negative boundary tests around 0xffffffff.
There was a problem hiding this comment.
Delegated to the guarded int64_t overload; boundary tests added on both sides of +-0xffffffff plus byte-parity against the variant path, verified failing against the previous code. Also routed stream_sink::value_int64/uint64 through the guarded overloads -- ABI built-in ints already streamed through the guarded generic in get_built_in_stream_unpacks, so no live divergence existed there, but the sink interface itself now cannot diverge for direct callers.
…ementally - json_writer: optional growth guard consulted at token boundaries once per stride (64KiB default); re-arms across a guard throw so catch-and-continue serializers cannot absorb a budget abort - make_http_stream_response_handler: settle the response buffer into bytes_in_flight at guard checkpoints and abort over-budget emissions with the busy response instead of materializing them in full - get_finalizer_info: active/pending policy fields keep their key and emit explicit JSON null when absent (new fc::nullable<T>), restoring the pre-typed-result schema - fc::microseconds and the abi stream_sink route 64-bit emission through the guarded to_json_stream overloads so the integer quoting threshold matches the variant path
| void value_string(std::string_view s) { | ||
| value_prefix(); | ||
| out_.push_back('"'); | ||
| fc::escape_string(s, out_, json_yield_function_t(), true); |
There was a problem hiding this comment.
[P1] Enforce the response budget within a single JSON token
guard_check() only runs from value_prefix()/key(), before this token is appended. A single large reflected string (for example an action console), value_hex, or raw_value can therefore append its entire contents after one pre-check, and the HTTP handler only rejects it in the final settle_to after the full buffer has already been allocated. The new over-budget test emits many separate 64 KiB tokens, so it does not exercise this case. Please invoke the guard while the escape/hex/raw loops cross each stride (including escape expansion), and add a regression where one token alone exceeds the response budget.
There was a problem hiding this comment.
Closed: guarded writers now check within tokens. value_string/key route the escape loop's periodic yield (every 128 input chars) into the same stride-gated guard check, measuring the output buffer so escape expansion counts; value_hex and raw_value emit in 64KiB chunks with a guard check between chunks (unguarded writers keep the single tight loop and the empty yield they already passed). A guard throw mid-token is safe under the existing re-arm plus checkpoint/rewind contract. Tests: growth_guard_intra_token_abort (each of the three token kinds aborts within one chunk of the threshold on a 1-2MiB token), growth_guard_measures_escape_expansion (32KiB of control chars crossing a 100KiB output threshold the raw input never reaches), and stream_response_single_token_over_budget (single 32MiB string value against a 1MiB budget: 503 with the emitter's post-token statement never reached), all verified failing against the token-boundary-only code.
- value_string/key route the escape loop's periodic yield into the growth guard, measuring the output buffer so escape expansion counts - value_hex/raw_value emit in 64KiB chunks with a guard check between chunks when guarded; unguarded writers keep the single tight loop - one oversized token (eg a large action console string, hex blob, or spliced fragment) can no longer materialize past the budget behind a single token-boundary check
value_hex/raw_value chunked at a fixed 64KiB regardless of the configured guard_stride, silently capping intra-token observation granularity for those token kinds. Derive the chunk from guard_stride_ instead -- matching value_string, whose escape-yield cadence is already stride-gated -- and drop the redundant json_writer_guarded_chunk_bytes constant.
huangminghuang
left a comment
There was a problem hiding this comment.
Four inline findings from the current-head review.
| out_.push_back('"'); | ||
| // escape_yield_ runs the growth guard periodically inside the escape loop, so a | ||
| // single oversized string value is budget-checked while it streams. | ||
| fc::escape_string(s, out_, escape_yield_, true); |
There was a problem hiding this comment.
[P1] Avoid reserving the full guarded string
The guarded writer calls escape_string, which immediately reserves input.size() + 13 bytes before its periodic yield can enforce the growth budget. A single 32 MiB string therefore allocates roughly 32 MiB even under a 1 MiB cap. The guarded path should reserve incrementally, with a regression test checking capacity/allocation rather than only out.size().
There was a problem hiding this comment.
Fixed by checking before the reserve rather than reserving incrementally: value_string/key know the token's input length -- a guaranteed lower bound on its peak emission, since escaping never shrinks a byte pre-prune -- so guarded writers now consult the guard with the prospective size before escape_string runs. An over-budget string aborts with zero allocation and zero buffer mutation (capacity stays at the writer's initial slack), while an approved string keeps the single full-size reserve, so the legitimate path pays no geometric-growth tax and unguarded/legacy callers are untouched. Escape expansion beyond the approved size remains covered by the intra-loop checks from the previous round, and the http guard's signed settle_to reconciliation handles the prospective charge -- reserve-then-allocate, trued up at the next checkpoint. Regression: growth_guard_bounds_allocation drives a 32 MiB string and key at a 64 KiB threshold asserting out.capacity() stays below the threshold (was ~32 MiB), plus approved-path and unguarded controls pinning the full reserve, verified failing against the previous code.
| // or read-write queue), it correctly bounces JSON serialization + socket | ||
| // I/O onto the http thread pool. | ||
| boost::asio::dispatch(plugin_state.thread_pool.get_executor(), | ||
| [&plugin_state, session_ptr{std::move(session_ptr)}, code, emitter{std::move(emitter)}]() mutable { |
There was a problem hiding this comment.
[P1] Charge queued emitter payloads
The typed result is captured by the emitter and queued on the HTTP thread without reserving any bytes-in-flight; accounting starts only when emission executes. Multiple large results can consequently accumulate outside http-max-bytes-in-flight-mb, unlike the variant path which charges before dispatch. Carry a source-size reservation through the queue and release it as the captured result is consumed.
There was a problem hiding this comment.
bind_stream now reserves the typed result's packed-size estimate at the moment of capture -- dispatch::sync and the async typed-result arm, the two shapes with a real cross-thread queue window (post/post_direct produce on the http pool and the dispatch short-circuits inline). The RAII reservation (http_plugin::reserve_bytes_in_flight) rides inside the emitter closure and releases exactly when the captured result is destroyed after emission, so the source stays charged alongside the incrementally-charged output buffer -- both genuinely coexist, matching the variant path holding its in_flight_sizeof estimate while the response string builds. The dispatched handler also re-checks the budget before running the emitter, draining over-budget queued work with the busy response like the variant path's pre-serialization verify. Trace-bearing results report their retained memory (action data, console, return values, captured ABI bytes) through a queued_payload_size customization point, since streamed_processed_trace is emitted but never wire-packed; fc::nullable gained raw pack/unpack parity with std::optional so reflected results containing it can be sized. Regression: stream_queued_payload_reserved parks the single pool thread in a latched emitter and asserts the sampled bytes_in_flight covers a 1 MiB queued bind_stream payload while it waits behind the stall -- reads 0 without the reservation, verified -- then drains to zero.
| namespace sysio::trace_api::detail { | ||
| std::string response_formatter::process_block_to_json( const data_log_entry& trace, bool irreversible, const stream_data_handler_function& data_handler ) { | ||
| std::string out; | ||
| fc::json_writer w(out); |
There was a problem hiding this comment.
[P1] Use the guarded writer for trace responses
This builds the entire trace response in an unguarded std::string before passing it through the legacy json_raw callback, so the HTTP memory cap observes it only after allocation. Large or concurrent trace responses can bypass the new protection. These endpoints should emit directly into the streaming callback’s guarded writer, with a low-budget regression test.
There was a problem hiding this comment.
Both endpoints moved off the json_raw pass-through onto add_async_api_stream: the request handler now produces an emitter -- misses (absent block, or trxid absent per the new contains_transaction) resolve to their 404 before a 200 emitter is committed -- and the emitter writes into the guarded json_writer the streaming callback provides, so the budget observes and can abort trace responses mid-serialize. Error responses emit error_results through the same streaming path handle_exception_stream uses. response_formatter grew writer-taking process_*_to_stream forms; the string-building forms remain as thin wrappers over them for tests and the benchmark. Tests: streaming_block_response_budget_abort drives the block emitter into a small-threshold guarded writer over a 32-transaction block and asserts the abort lands within a token of the threshold with the full body several times larger; streaming_emitter_miss_resolution_and_parity pins nullopt on every miss and byte-identical emitter output against the wrappers; the existing stream/variant parity suites are unchanged, and trace_plugin_test.py passes end-to-end over http.
| inline void to_json_stream(double d, json_writer& w) { | ||
| if (!std::isfinite(d)) { | ||
| // Variant emits the literal "nan" / "inf" / "-inf" string for non-finite doubles; match that shape. | ||
| w.value_string(std::isnan(d) ? "nan" : (d > 0 ? "inf" : "-inf")); |
There was a problem hiding this comment.
[P2] Preserve the sign of NaN
The streaming formatter always emits "nan" for NaN, while the legacy formatter emits "-nan" for a negative NaN. Arbitrary ABI float bit patterns can expose this, breaking the promised byte-for-byte serialization parity. Preserve std::signbit(d) and add positive/negative NaN parity coverage.
There was a problem hiding this comment.
Fixed with signbit for both the nan and inf branches. Parity coverage: libfc leaf checks for positive and negative quiet NaN, both infinities, and a negative float NaN against the platform's variant output, plus abi_tests/non_finite_float_sign_parity packing raw negative-NaN float64/float32 bit patterns -- unreachable from JSON input -- and asserting binary_to_json_stream matches binary_to_variant + to_string byte-for-byte with the legacy "-nan" shapes pinned, verified failing against the previous code.
There was a problem hiding this comment.
Follow-up: the macOS job caught that the legacy "-nan" spelling was itself platform-dependent -- glibc prints "-nan" but Apple's libc drops the sign, so on macOS the variant path emitted "nan" for a negative NaN and the new parity tests failed against it. Rather than mirroring whatever the local platform prints, s_fc_to_string now formats non-finite doubles explicitly from isnan/isinf + signbit, making both paths byte-identical and deterministic on every platform with Linux output unchanged; absolute spelling pins were added alongside the parity checks so the contract no longer depends on the platform's C library.
… reserves The guarded json_writer let escape_string reserve the full input length up front, so a single over-budget string allocated its entire size (32 MiB under a 1 MiB cap) before any check could run; the growth guard bounded emission but never allocation. Escaped output is at least one byte per input byte, so value_string and key now consult the guard with the buffer's prospective size -- current size plus the token's input length -- before escape_string runs. An over-budget token aborts with zero allocation and zero buffer mutation, an approved token keeps the single full-size reserve, and escape expansion beyond the approval stays covered by the intra-loop checks. The guard contract documents that prospective sizes may be passed; the http settle_to reconciliation already handles them, charging the bytes before they are allocated. The allocation regression test drives a 32 MiB string and key at a 64 KiB threshold and asserts capacity stays at the writer's initial slack, with unguarded and approved-path controls pinning the legacy full reserve.
A typed result captured into a stream_emitter rode the dispatch queue to the http pool with nothing charged to bytes_in_flight until emission began, so queued results accumulated invisibly to --http-max-bytes-in-flight-mb -- unlike the variant path, which charges its in_flight_sizeof estimate before dispatch. bind_stream now reserves the result's packed-size estimate (http_plugin::reserve_bytes_in_flight) where the result is captured, and the RAII reservation rides the emitter closure until the captured payload is destroyed after emission. The dispatched handler re-checks the budget before running the emitter and drains over-budget work with the busy response, mirroring the variant path's pre-serialization verify. Trace-bearing results (streamed_processed_trace is emitted, never wire-packed) report their retained memory through a queued_payload_size customization point, and fc::nullable gains raw pack/unpack parity with std::optional so reflected results containing it can be sized. The regression test parks the single http pool thread inside a latched emitter, produces a 1 MiB bind_stream result on the app thread, and asserts the sampled bytes_in_flight covers the payload while its emitter waits in the queue, draining to zero afterwards.
get_block and get_transaction_trace built their entire response into an unguarded local string and handed it through the legacy json_raw callback, so the http memory cap only saw the body after it was fully allocated. Both endpoints now register via add_async_api_stream: misses resolve to their 404 before an emitter is committed (contains_transaction gates the per-transaction case), and the emitter writes directly into the guarded json_writer the streaming callback provides, so --http-max-bytes-in-flight-mb observes and can abort large trace responses while they serialize. Error responses emit error_results through the same streaming path. response_formatter grows writer-taking process_*_to_stream forms; the buffer-building *_to_json forms remain as thin wrappers for tests and the trace_api_json benchmark. The budget regression test drives the block emitter into a small-threshold guarded writer and asserts the abort lands within a token of the threshold, far below the full body.
The streaming formatter emitted "nan" for every NaN while the variant path (stringstream << std::fixed, delegating to the C library) prints "-nan" when the sign bit is set, breaking byte-for-byte parity for ABI float bit patterns that carry a negative NaN. NaN never compares, so the sign now comes from signbit for both the nan and inf branches. Parity coverage spans the libfc leaf-type checks (+-NaN, +-inf, float -nan) and an abi_serializer case that unpacks raw float64/float32 negative-NaN bit patterns -- unreachable from JSON input -- through binary_to_json_stream against the variant path.
The variant path left non-finite formatting to the C library behind stringstream, whose negative-NaN spelling is platform-dependent: glibc prints "-nan" while Apple's libc drops the sign and prints "nan". The streaming path's signbit-aware emission therefore matched the variant path on Linux but diverged on macOS, where the parity tests caught the legacy path emitting "nan" for a negative NaN. s_fc_to_string now formats nan/inf explicitly from isnan/isinf + signbit, making both paths byte-identical and deterministic on every platform while leaving Linux output unchanged. Absolute spelling pins join the parity checks so the contract no longer depends on the platform's C library.
huangminghuang
left a comment
There was a problem hiding this comment.
Two inline findings from the current-head streaming JSON review.
| [r = std::get<payload_t>(std::move(result)), reservation = std::move(reservation)] | ||
| (fc::json_writer& w) mutable { fc::to_json_stream(r, w); }); | ||
| } else { | ||
| http.post_http_thread_pool( |
There was a problem hiding this comment.
[P1] Charge deferred closures while they wait
This branch posts http_fwd without a bytes-in-flight reservation. Unlike the typed-result arm above, the deferred callable is not necessarily small: get_block's callable owns the fetched block and copied ABI blobs, transaction callables own the trace plus copied ABIs, and post_direct table-row callables own the collected raw rows. If the HTTP pool is busy, many of these can accumulate outside --http-max-bytes-in-flight-mb; the budget only observes them after they finally execute, which is too late to prevent the memory spike. Please carry retained-size metadata/a reservation with the deferred callable before posting it, and add a regression that blocks the HTTP worker and verifies a large deferred capture is charged while queued.
There was a problem hiding this comment.
Fixed by carrying the retained size with the deferred call itself. chain::deferred_call<T> (the callable plus the retained size of what Phase 1 captured) replaces the bare function<t_or_exception<T>()> as the third alternative of next_function_variant and as the return type of the two-phase api methods; the size is a required constructor argument, so a new two-phase endpoint cannot return an unaccounted callable. bind_stream's post, post_direct and async-forward arms now take the RAII reservation (http_plugin::reserve_bytes_in_flight) before post_http_thread_pool and hold it in the posted lambda, so the charge spans both the queue wait and the run and releases when that lambda is destroyed -- deliberately outliving Phase 2 rather than being traded for a result-sized charge, since the captures are consumed into the result and the post arms hand that to the response callback inline. The legacy CALL_ASYNC_WITH_400 variant path had the identical hole and takes the same charge.
Sizes come from the producers, the only place that can see through the type erasure, and reuse the existing queued_payload_size vocabulary: get_account charges the copied ABI bytes plus the raw reslimit row; get_table_rows and get_table_rows_stream charge the collected row bytes plus the parsed abi_def; get_block charges the block, summed from each receipt's own packed size rather than a full re-pack, plus the captured ABI bytes; the send_transaction family charges the trace's retained size plus its captured ABI bytes. A queued_payload_size(captured_abis) overload is now shared by the trace estimate and the new producers instead of the same map being walked in two places.
Regression: stream_deferred_payload_reserved parks the single pool thread in a latched emitter, then runs a dispatch::post Phase 1 that hands 1 MiB of collected bytes to the deferred call, and asserts the sampled bytes_in_flight covers it while it waits behind the stall -- reads 0 without the reservation, verified -- then drains to zero once the response completes.
| { | ||
| w.begin_array(); | ||
| w.value_uint64(static_cast<uint64_t>(s.index())); | ||
| std::visit([&w](const auto& v) { fc::to_json_stream(v, w); }, s); |
There was a problem hiding this comment.
[P2] Keep alternative serialization open to ADL
Qualifying this call as fc::to_json_stream fixes the overload set when static_variant.hpp is parsed, so serializers declared by subsequently included headers are invisible. A minimal TU that includes fc/static_variant.hpp before fc/crypto/sha256.hpp and streams std::variant<fc::sha256> fails to compile by falling into the reflector primary instead of SHA-256's string serializer. Please use unqualified dispatch here (as the new container helpers already do) and add a late-declared-alternative regression such as std::variant<fc::sha256>.
There was a problem hiding this comment.
Confirmed and fixed. The transitive include closure of static_variant.hpp does not reach fc/serialize_as_string.hpp, so the qualified call froze an overload set that held only the unconstrained reflector primary; fc::sha256 carries just FC_REFLECT_TYPENAME, so the primary's static_assert fires. The visitor now dispatches unqualified, like the container helpers, keeping ADL open at the point of instantiation. The same qualified call in fc/io/enum_type.hpp had the same latent bug and is fixed with it, and bind_stream's own emit sites moved to the two-step form (using fc::to_json_stream; then an unqualified call) -- it is generic over the api result type but lives in namespace sysio, where plain unqualified lookup would not reach fc at all.
Regression: test_static_variant.cpp now includes fc/crypto/sha256.hpp after fc/static_variant.hpp -- the order is called out as load-bearing so it does not get tidied away -- and streams std::variant<fc::sha256, int32_t>, asserting the [0,"<hex>"] / [1,7] shapes and byte-parity with the variant path. Verified red: restoring the qualified call fails to compile in that translation unit with exactly the reflector static_assert.
The two-phase api shape hands the transport a callable that owns whatever phase 1 collected -- copied ABI bytes, table rows, a fetched block, a transaction trace -- and keeps it alive for as long as the callable waits on the http thread pool queue. A std::function type-erases those captures, so the transport cannot size them and they accumulated outside --http-max-bytes-in-flight-mb, visible to the budget only once phase 2 finally ran. deferred_call<T> pairs the callable with the retained size of what it captured and replaces the bare std::function as the third alternative of next_function_variant and as the return type of the two-phase api methods. The size is a required constructor argument, so a new two-phase endpoint cannot return an unaccounted callable. get_account, get_table_rows, get_table_rows_stream, get_block and the send_transaction family each report theirs, and queued_payload_size grew a captured_abis overload that the streamed_processed_trace estimate now shares instead of walking the same map in two places. bind_stream's post, post_direct and async-forward arms reserve that size before posting the call to the http thread pool and hold the reservation in the posted lambda, so the charge spans both the queue wait and the run and releases when that lambda is destroyed. It deliberately outlives phase 2 rather than being traded for a result-sized charge: the captures are consumed into the result, which the post arms hand to the response callback inline, so one conservative charge covers the whole window without double counting. The legacy variant-cb macro path had the same hole and takes the same charge. stream_deferred_payload_reserved parks the single pool thread in a latched emitter and asserts the sampled bytes_in_flight covers a 1 MiB deferred capture while it waits behind the stall, then drains to zero once the response completes.
to_json_stream(std::variant) qualified its alternative dispatch as fc::to_json_stream, which fixes the overload set where the template is defined. fc/serialize_as_string.hpp is not in static_variant.hpp's include closure, so a translation unit that includes static_variant.hpp before crypto/sha256.hpp resolved std::variant<fc::sha256> to the unconstrained reflector primary and failed its static_assert. The visitor dispatches unqualified instead, as the container helpers already do, keeping ADL open at the point of instantiation. fc::enum_type carried the same qualified call. bind_stream now emits through a using-declaration plus an unqualified call, since it is generic over the api result type but lives in namespace sysio, where ordinary lookup does not reach fc at all. The static_variant suite pins the arrangement: it includes crypto/sha256.hpp after static_variant.hpp -- flagged as load-bearing -- and streams std::variant<fc::sha256, int32_t> against the variant path's bytes.
huangminghuang
left a comment
There was a problem hiding this comment.
Four inline findings from the current-head re-review.
| return std::nullopt; | ||
| } | ||
|
|
||
| return [this, data = std::move(*data)](fc::json_writer& w) { |
There was a problem hiding this comment.
[P1] Charge decoded trace captures against bytes-in-flight
This emitter owns the entire decoded block trace, and the transaction emitter below does the same, but these manual add_async_api_stream paths never take the source-payload reservation that bind_stream now carries. The guarded writer therefore charges only the growing output while the decoded trace remains live; concurrent large trace responses can exceed --http-max-bytes-in-flight-mb by roughly their retained source size. Please carry a retained-size reservation with these emitters and add a regression that samples bytes_in_flight while a large trace is held/emitted.
There was a problem hiding this comment.
There is no queue window on these paths, so there is nothing to charge for. add_async_api_stream registers through make_http_thread_url_handler_stream, which invokes the handler inline on the http thread pool thread already serving the request, not on the read-only/read-write queue. make_http_stream_response_handler then dispatches on that same pool executor, and asio runs a dispatch inline when the caller is already inside the pool -- the note in common.hpp spells this out. The decoded trace is therefore created and consumed in one call stack on one thread; it never waits behind a busy pool the way a posted deferred_call does.
That is the distinction the bind_stream reservation was scoped to: it covers dispatch::sync and the async typed-result arm because those hand work across threads, and deliberately does not cover post/post_direct's inline short-circuit. Peak unaccounted here is http-threads x one decoded block trace, and trace_api's own trace types carry no console field, so a block trace is bounded by block net usage (max_block_net_usage, default 1 MiB). Two threads holding roughly a MiB each against a 500 MB default budget is well inside the tolerance the setting is designed for, and the guarded writer still observes and aborts the output side while it serializes.
| } | ||
|
|
||
| void to_json_stream( const blob& b, json_writer& w ) { | ||
| w.value_string(base64_encode(b.data.data(), b.data.size())); |
There was a problem hiding this comment.
[P1] Stream Base64 before allocating the encoded temporary
Function-argument evaluation materializes the complete base64_encode(...) result before json_writer::value_string can consult its growth guard. For /v1/chain/get_raw_code_and_abi and /v1/chain/get_raw_abi, a raw blob that fits the source reservation can still allocate a 4/3-sized unaccounted temporary—and, on the accepted path, that temporary coexists with the source and output buffer. Please add a guard-aware/incremental Base64 writer primitive (or equivalent) so the response budget is enforced before and during encoding, with a single-large-blob allocation regression.
There was a problem hiding this comment.
The blob is bounded by consensus, so this stays inside the budget's tolerance by construction. to_json_stream(const blob&) is reached only from get_raw_code_and_abi and get_raw_abi, and the bytes are chain state: fc caps blob deserialization at MAX_SIZE_OF_BYTE_ARRAYS (20 MiB), the chain caps a module at max_module_bytes (default 20 MiB), and the operative limit is that setcode has to fit a transaction, so default_max_transaction_net_usage (512 KiB) is what a deployer actually runs into. The encoded temporary is 4/3 of that: roughly 700 KB in practice, and about 27 MiB at a ceiling only reachable after raising net limits first, against a 500 MB default budget.
There is also no amplification here. The bytes must be committed to chain state, and paid for in RAM, before the endpoint will return 1.33x of them, so it is not a lever an unauthenticated caller can pull. --http-max-bytes-in-flight-mb is an approximate abuse guard rather than an allocator; it does not need to be exact, and a guard-aware incremental base64 primitive is real machinery bought to gain precision on a quantity consensus already bounds.
| // Dominant memory the deferred call holds while it waits on the http pool queue: the ABI | ||
| // copy and the raw reslimit row. The typed result it also carries is account metadata and | ||
| // a bounded permission list -- small fixed-size fields, ignored per the estimate's semantics. | ||
| const size_t retained = abi_bytes.size() |
There was a problem hiding this comment.
[P1] Include the captured account result in the deferred reservation
The deferred callable also owns result, including every permission, authority vector, per-permission linked-action vector, and sysio_any_linked_actions, but retained counts only the ABI and reslimit bytes (and the no-ABI branch reserves zero). These collections are not bounded here—the phase-1 loops consume every matching database record—so a large account result can wait on the HTTP pool outside --http-max-bytes-in-flight-mb until phase 2 returns it and the result-side reservation finally starts. Please include a reservation-grade size for result in both branches and cover an account with a large permission/link set.
There was a problem hiding this comment.
Agreed that this one has a real queue window -- get_account is dispatch::post, so Phase 1 runs on the read-only queue and the callable does wait on the pool. What it holds is still bounded by chain state: result.permissions and sysio_any_linked_actions are walks over permission_index and permission_link_index, and every entry is a permission_object or permission_link_object that someone paid RAM for. The in-memory struct is within a small factor of the on-chain footprint, so growing the response means first buying the RAM and carrying it permanently -- there is no amplification to exploit.
The terms that are not bounded that way are the ones already charged: the ABI copy is a contract's abi bytes and the reslimit row is a raw kv value, both independent of the requesting account. The no-ABI branch reserving zero is correct rather than a gap -- when ROA has no abi, Phase 2 skips the decode and the callable carries no copied bytes at all. Since --http-max-bytes-in-flight-mb is an approximate guard against abuse rather than an allocator, and the permission term does not move it in any reachable scenario, leaving the estimate as is.
| /// emit their tokens without an intermediate variant build. | ||
| template<typename T> | ||
| void emit(const T& v) { | ||
| fc::to_json_stream(v, w_); |
There was a problem hiding this comment.
[P2] Keep generic sink emission open to late serializers
This qualified call has the same frozen-overload-set problem just fixed in static_variant: a TU that includes abi_sinks.hpp before database_utils.hpp and calls stream_sink::emit(softfloat128_t{}) resolves to the reflector primary and fails its static assertion, because the hand-written softfloat serializer is declared later. Please make this dispatch see the late/ADL-visible serializer and add that include-order compile regression; merely fixing static_variant leaves the generic ABI sink exposed.
There was a problem hiding this comment.
Confirmed and fixed, though not quite the way the last ADL fix went. The dispatch now goes through a new fc::json_emit, which sits beside the reflector primary in fc/reflect/json_stream.hpp and calls to_json_stream unqualified; stream_sink::emit calls that rather than qualifying. bind_stream's local emit_json wrapper was the same using-declaration written a second time, so it is gone and its call sites use the shared helper -- the rule lives in one place now instead of being restated at each site outside fc.
Worth recording why this covers softfloat128_t, since it is a global-namespace typedef of an anonymous struct and associates nothing useful of its own: the second parameter is fc::json_writer&, so ADL pulls namespace fc into the lookup set at the point of instantiation and finds every fc serializer declared by then, including that one. The dispatch is open for any T for that reason, not only for types that live in fc.
Separately, those softfloat serializers moved out of the bottom of database_utils.hpp into sysio/chain/softfloat_serializers.hpp. That is layering cleanup rather than part of the fix -- the ADL change already covers them -- but it retires the block partway down database_utils.hpp that closed namespace sysio::chain, reopened fc to hand-declare to_variant and from_variant for the BE key codec, and reopened the chain namespace again. Include order handles it now.
Regression: abi_sink_dispatch_tests includes abi_sinks.hpp first and database_utils.hpp after -- the order is called out as load-bearing so it does not get tidied away -- and emits a softfloat128_t through stream_sink, asserting fc's canonical "0x" plus little-endian hex spelling. A second case covers the other half of the contract with a serializer in a namespace of its own, reached by ADL on the value rather than the writer, and a third pins variant_sink to the same shape. Verified red both ways: re-qualifying the call site in stream_sink::emit, and re-qualifying the body of fc::json_emit itself, each fail the translation unit on both types with the reflector primary's static_assert.
stream_sink::emit qualified its dispatch as fc::to_json_stream, which fixes the overload set where abi_sinks.hpp is parsed. Serializers declared by headers included later are invisible there, so those types fall through to the unconstrained reflector primary and fail its static_assert. softfloat128_t is the in-tree case: it is a global-namespace typedef of an anonymous struct, and its fc::to_json_stream lives at the bottom of database_utils.hpp, which abi_sinks.hpp does not include. fc::json_emit is that dispatch, written once. It sits beside the reflector primary it guards against and calls to_json_stream unqualified, so ADL reruns at the point of instantiation. The lookup reaches namespace fc regardless of where T lives, because json_writer is an fc type and every call passes one -- which is what covers types that associate no useful namespace of their own. Code already inside fc keeps calling to_json_stream plainly; code outside fc calls fc::json_emit instead of repeating a using-declaration at each site. bind_stream's local emit_json wrapper was exactly that repetition and is replaced by the shared helper. abi_sink_dispatch_tests includes abi_sinks.hpp before database_utils.hpp -- flagged as load-bearing -- and emits a softfloat128_t through stream_sink, plus a serializer in a namespace of its own reached by ADL on the value rather than the writer, and the same type through variant_sink. Re-qualifying either the call site or the body of fc::json_emit fails the translation unit on both types.
fc::to_variant, fc::from_variant and fc::to_json_stream for softfloat64_t and softfloat128_t, their memcpy conversion helpers and the datastream pack operators lived at the bottom of database_utils.hpp, a chain database utility header that merely happened to be their first consumer. The BE key codec earlier in that same file needs the softfloat128_t conversions, so the header closed namespace sysio::chain, reopened fc to hand-declare to_variant and from_variant, and reopened the chain namespace again to carry on. softfloat_serializers.hpp holds them instead, and database_utils.hpp includes it near the top, so the declarations precede the codec by include order and the forward declaration block goes away. The new header depends only on libfc and the softfloat headers, so it can be included anywhere under libraries/chain without pulling in chain types; that is why the 128-bit integer is spelled fc::uint128_t rather than sysio::chain::uint128_t, which are the same typedef of unsigned __int128. No behaviour change: database_utils.hpp remains the only includer, so exactly the same translation units see these overloads and the global-scope pack operators as before.
huangminghuang
left a comment
There was a problem hiding this comment.
Four inline findings from review of the current PR head.
| void stream_sink::unpack_protobuf(std::string_view ftype, fc::datastream<const char*>& stream) { | ||
| // MessageToJsonString already produces JSON; splice it directly into the writer. | ||
| FC_ASSERT(abi_, "stream_sink::unpack_protobuf requires the abi_serializer-bound constructor"); | ||
| raw_value(abi_->pb_binary_to_json_string(ftype, stream)); |
There was a problem hiding this comment.
[P1] Preserve protobuf JSON representation
Splicing MessageToJsonString directly bypasses the legacy fc::variant normalization. A protobuf double such as 1.5 now emits as the JSON number 1.5, whereas pb_binary_to_variant followed by fc::json::to_string emits the string "1.50000000000000000"; escaping can differ as well. This changes valid API response types. Please route protobuf output through equivalent normalization and add protobuf floating-point/string cases to verify_stream_matches_variant.
| const abi_serializer::yield_function_t& yield, fc::json_writer& w ) { | ||
| if( is_array ) { | ||
| fc::unsigned_int sz; | ||
| fc::raw::unpack(stream, sz); |
There was a problem hiding this comment.
[P1] Restore the built-in array limit
The legacy built-in path unpacked vector<T>, whose raw decoder rejects sizes above MAX_NUM_ARRAY_ELEMENTS. This manual loop accepts and processes larger arrays, bypassing that hard resource limit. Please assert the limit after decoding sz in both streaming helpers and add an over-limit regression.
| w.end_array(); | ||
| } else if( is_optional ) { | ||
| char flag; | ||
| fc::raw::unpack(stream, flag); |
There was a problem hiding this comment.
[P2] Validate optional presence flags
The old optional<T> decoder reads a bool and rejects any presence byte other than 0 or 1. Reading a char here treats malformed values such as 2 as present, so streaming can decode data the variant path rejects or falls back from. Please preserve the canonical bool validation in both streaming helpers.
| abi._binary_walk(type, ds, *this, inner); | ||
| on_value_emitted(); | ||
| return true; | ||
| } catch(...) { |
There was a problem hiding this comment.
[P1] Do not absorb response-budget aborts
This catch-all can swallow stream_response_budget_exceeded: after rewind, the next fallback token rechecks the guard against the smaller buffer, releases the previous charge, and can pass, producing a 200 response with hex or omitted decoded fields instead of the intended busy response. The same pattern exists in get_table_rows and trace serialization. Please latch or explicitly propagate guard aborts across every rewind/fallback path, and test guard-throw plus rewind plus fallback.
Emits HTTP JSON responses token-by-token into the output buffer through
fc::json_writerinstead of building an intermediatefc::varianttree and re-serializing it withfc::json::to_string.Core pieces
fc::json_writer(fc/io/json_stream.hpp): append-only token writer with object/array frame tracking,checkpoint()/rewind()for atomic rollback of partial emission, locale-independent numeric emission viastd::to_chars, and the same 64-bit integer quoting thresholdfc::json::to_stringuses.to_json_streamoverload family: primitives and standard containers are co-located with theirto_variantsiblings (variant.hpp,container/flat.hpp) so shape andMAX_NUM_ARRAY_ELEMENTSguards stay in lock-step; a reflector visitor gives everyFC_REFLECT'd struct a streaming serializer for free; a depth-bounded streaming walker coversfc::variant/variant_objectvalues (json_stream_max_depthmatches json.cpp's recursion cap).FC_SERIALIZE_AS_STRINGtrait: string-shaped types (name, asset, symbol, symbol_code, block_timestamp, the sha/ripemd160/keccak256/blake3 hashes, BLS keys/signatures, bitset, url) declare oneto_string()/from_string()pair that drivesto_variant,from_variant, andto_json_stream, replacing the per-type hand-rolled overload triplets. The hashfrom_stringfactories reject odd-length hex, keeping the strictness the previousvector<char>from_variant path enforced. Deadfc::fixed_stringis removed.to_json_stream(bls::private_key)is deleted so key material cannot be streamed into a response.abi_serializer::binary_to_json_stream: a single_binary_walk<Sink>traversal drives both thevariant_sink(the existingbinary_to_variant) and the newstream_sink(tokens straight into the writer), so shape parity between the two paths is structural rather than by convention. Action-data decode is atomic:unpack_action_data_fieldcheckpoints, rewinds on failure, and the caller emits the hex fallback under the same key.bind_stream<>: registration template with compile-time signature pinning; response serialization happens on the http thread pool while the api thread hands off the result struct by move. chain_api, producer_api, net_api, db_size_api, and trace_api get_block/get_transaction_trace are migrated; endpoint sets, queues, categories, and response codes are unchanged. Stored exceptions rethrow with their dynamic type so the status classifier keeps mapping tx_duplicate to 409, unknown_block to 400, etc., and queued streaming request bodies take the same bytes_in_flight reservation as the variant path.captured_abis) and build serializers at emit time on the http pool, keeping ABI decode off the main thread; memory for captured blobs is accounted in the trx_retry budget.Performance
RelWithDebInfo, i9-12900K,
benchmark -f trace_api_json/-f abi_serializer:json::to_stringpipeline.binary_to_json_streamversusbinary_to_variant+to_string.get_table_rowsdecodes rows straight from binary into the writer with no per-row variant tree.Parity
Streamed output is pinned byte-identical to the variant path: every abi_tests round-trip case also runs
verify_stream_matches_variant, endpoint-level byte-identical assertions cover get_account/get_block/get_table_rows, and trace_api has stream-vs-variant response parity cases including the decode-failure hex-fallback shapes.