Merge master into opp-part3 (master-sync)#345
Open
heifner wants to merge 72 commits into
Open
Conversation
undo_index::post_modify handles AVL tree rebalancing when composite index key fields change. This avoids node deallocation, fresh node allocation, and reinsertion into all 3 AVL trees per secondary index update. Added test proving modify correctly rekeys across indices.
Wire's per-row KV value limit is 256 KiB and a direct setcode trx can carry ~512 KiB, so post-#291 sysio.msig setcode was capped well below what a non-msig setcode could deploy. This restores parity by chunking the serialized inner trx across rows of a new `propchunks` table when it exceeds an internal threshold (200 KiB), reassembling on `exec`, and exposing a read-only `getproposal` action that returns the assembled struct via /v1/chain/send_read_only_transaction. Storage layout - `proposal` row gains three appended `binary_extension` fields: `chunk_count`, `total_size`, `trx_hash`. Small proposals keep the legacy shape (full blob in `packed_transaction`, chunk_count = 0) so external tooling that reads the row directly via get_table_rows still works for the small case. - New `propchunks` `kv::scoped_table` keyed by (proposal_name, chunk_index) holds the bytes when a proposal is chunked; the parent row's `packed_transaction` is empty in that case. propose / approve / unapprove / cancel / exec - `propose` hashes the inner trx once via `sysio::sha256(trx_pos, size)` and stores it in `trx_hash`. Inner trxs at or below `proposal_chunk_size` go into `packed_transaction` as before; larger ones are split into ceil(size / proposal_chunk_size) `propchunks` rows. - Helpers `is_chunked`, `assemble_packed_trx`, `read_trx_header`, and `erase_proposal_chunks` keep the chunk-aware logic isolated. The header read pulls from chunk 0 alone for chunked proposals so approve doesn't pay reassembly cost just to inspect expiration / delay_sec. - `approve --proposal-hash` keeps the legacy `assert_sha256` path for inline proposals and uses the precomputed `trx_hash` for chunked proposals, since chunked rows have no inline blob to re-hash. - `exec` reassembles, deserializes, dispatches each action via `act.send()` exactly as before. Each individual dispatched action is still bounded by `max_inline_action_size`, so no chain-config change is required. - `cancel` and `exec` both call `erase_proposal_chunks` so chunk rows never outlive their parent proposal — no orphaned RAM. getproposal (read-only) - New `[[sysio::action("getproposal"), sysio::read_only]] proposal get_proposal(proposer, proposal_name)` returns the assembled proposal struct with `packed_transaction` always populated. CDT's codegen auto-emits `set_action_return_value` for non-void actions, and the chain skips `max_action_return_value_size` in read-only context, so the full ~512 KiB blob comes back via the trace's `action_traces[0].return_value`. clio multisig review now uses getproposal - Switched away from /v1/chain/get_table_rows on the `proposal` row. `multisig review` builds a one-action getproposal trx, posts it to /v1/chain/send_read_only_transaction, and pulls `processed.action_traces[0].return_value_data` (ABI-decoded by chain_plugin via the new action_results entry CDT generates from the return type). Storage-layout-agnostic — if sysio.msig refactors its tables again, clio keeps working as long as the getproposal signature is preserved. - This is the design principle for new clio commands going forward: read-only actions are the stable contract surface; get_table_rows reaches into table internals and is brittle (the same brittleness that bit us in #291 with ABI field-name matching). clio multisig propose_trx accepts structured action.data - Mirrors the try/catch fallback that `clio push transaction` already has at ~line 3145: try the literal `as<transaction>()` cast first, fall back to `abi_serializer::from_variant(trx_var, trx, abi_serializer_resolver, ...)` on the catch. Strict superset of the old behavior — old hex-data callers keep working unchanged, new callers can pass the natural JSON shape with structured `action.data` objects and clio will recursively encode via each contract's ABI. - Generic clio improvement that benefits anyone using propose_trx, not just the new msig integration test. Tests - contracts/tests/sysio.msig_tests.cpp adds three cases: - `big_transaction_chunked`: builds a >200 KiB inner trx (two setcode actions of sysio.system.wasm), proposes, exercises the chunked-path `--proposal-hash` check (correct hash succeeds, wrong hash rejected against the stored trx_hash), approves, and execs end-to-end. Default chain config — no setparams bumps. - `getproposal_read_only_returns_assembled`: pushes a read-only trx invoking getproposal, ABI-decodes the return value, asserts the reassembled `packed_transaction` byte-equals the original, and that `chunk_count > 0` and `total_size` matches. - `cancel_chunked_proposal_cleans_chunks`: proposes, cancels, re-proposes under the same name, approves, execs — would fail with "key already exists" on the second propose if cancel did not erase chunk rows. - New tests/multisig_review_test.py Python integration test launches a producer + non-producer API node cluster (with `--read-only-threads 1` on the API node), deploys the new sysio.msig wasm, submits a chunked propose via clio, runs `clio multisig review` against the API node so it actually goes through the read-only-trx getproposal path, and validates the JSON output: chunk_count > 0, total_size > 200 KiB, trx_hash populated, both setcode actions present, packed_transaction non-empty (proving the review fell through getproposal rather than echoing the empty inline row). Also runs `--show-approvals` which exercises the approvals2 / approvals / invals lookup paths. - All 117 contracts_unit_test cases, 14 read_only_trx_tests, and the full plugin_test suite pass.
…ling, extra chunked tests Addresses a pre-PR review pass over fd91d9b. No behavior changes in the happy path; perf, error UX, and test coverage improvements. Contract: assemble_packed_trx consumes proposal&& to avoid inline copy - `assemble_packed_trx` now takes `multisig::proposal&&` and, for the inline path, returns `std::move(prop.packed_transaction)` so the (~up to 200 KiB) vector is moved out instead of copied. NRVO doesn't apply to returning a const-lvalue member, so the previous `const proposal&` version paid a full heap copy on every inline approve/unapprove/exec auth recheck. - New small helper `read_proposal_chunks(const proposal&, ...)` holds the chunk-reading logic. Shared by `assemble_packed_trx`'s chunked branch and directly by `get_proposal`, which preserves `prop` for return — avoiding the moved-from-then-write awkwardness of routing `get_proposal` through the consume-prop helper. - `erase_proposal_chunks` now takes `(uint32_t chunk_count, name proposal_name, name self, name proposer)` instead of a `proposal&`. Lets `exec` and `cancel` call it AFTER the proposal has been moved into `assemble_packed_trx`. Both callers snapshot `chunk_count` (and `exec` snapshots `earliest_exec_time`) into locals before the std::move. - `get_proposal` rewritten to call `read_proposal_chunks` directly on a preserved `prop` local, then return `prop` via NRVO. Zero redundant copies for both inline and chunked paths. Contract: defense-in-depth static_assert on proposal_chunk_size - `read_trx_header`'s chunk-0 fast path assumes chunk 0 holds at least a full serialized transaction_header (max ~21 bytes). Added a `static_assert(proposal_chunk_size >= 128)` next to the constant so anyone reducing it has to confront the invariant at compile time before silently breaking the fast path. Contract: minor cleanups - Dropped the redundant `(char*)` casts on `memcpy(..., pkd_trans.data(), ...)` and the chunked equivalent — `std::vector<char>::data()` already returns `char*` for non-const vectors. - Replaced the ternary `(size - off < proposal_chunk_size) ? (size - off) : proposal_chunk_size` with `std::min(size - off, proposal_chunk_size)`. - Moved the `get_proposal` action declaration and `getproposal_action` wrapper from between `propchunks` and `old_approval_key` down to after `invalidations`, so all tables are grouped above the single read-only action at the bottom. Doesn't change the ABI (no action/table set changes) — confirmed by a clean `git status` on sysio.msig.abi after rebuild. clio: tighter exception handling in multisig review - Replaced `catch(...) { std::cerr << "Proposal not found"; }` around the send_read_only_transaction call with `catch(const fc::exception& e)`. The old catch-all swallowed network errors, parse failures, and server errors as "Proposal not found"; the new version only friendly-prints the literal contract assertion "proposal not found" and rethrows everything else so the real failure reaches clio's main handler. - Improved the missing_chain_api_plugin_exception message to explicitly mention `--read-only-threads N` and the producer-node restriction, so operators hitting this for the first time don't have to grep nodeop's source. - Wrapped the JSON envelope walk (result1 -> processed -> action_traces -> first_trace -> return_value_data) in a try/catch that catches the `fc::exception` base (covers both bad_cast_exception for wrong types and key_not_found_exception for missing fields). On failure the error message includes both the fc exception detail AND the raw response JSON so the user can diagnose what the node actually returned. The `proposal_object` is carried out of the try via a `const fc::variant_object*` pointer so the ~150 lines of downstream code don't need to move into the try block and no ~200 KiB copy of the packed_transaction hex is made. Tests: three additional chunked-path tests - `getproposal_read_only_inline` — sibling of the existing chunked test that exercises the inline branch of get_proposal (proposal_object returned via read_only trx for a small reqauth-based proposal). Uses BOOST_REQUIRE_EQUAL_COLLECTIONS for the byte-level packed_transaction comparison so failures report the first mismatching index instead of a bare assertion-failed message. - `unapprove_chunked_past_threshold` — exercises the chunked-trx-recheck branch in unapprove (the `assemble_packed_trx(std::move(prop), ...)` call at sysio.msig.cpp). Propose chunked -> approve(alice) -> approve (bob, sets earliest_exec_time inner optional) -> unapprove(bob, triggers the chunked reassembly and clears earliest_exec_time) -> exec (must reject because bob's approval was removed) -> re-approve (bob) + exec (must succeed, proves the unapprove path didn't corrupt anything). - `cancel_chunked_by_non_proposer_past_expiration` — exercises the chunk-0 read_trx_header fast path from `cancel`'s non-proposer branch. Computes a near-future expiration relative to `control->head().block_time()`, proposes chunked, tries non-proposer cancel before expiration (must fail), advances 20 blocks past expiration, cancels as non-proposer (must succeed AND clean up chunks), then re-proposes under the same name (would fail with "key already exists" if any chunk rows were orphaned). - The existing `getproposal_read_only_returns_assembled` test's byte comparison also switched to BOOST_REQUIRE_EQUAL_COLLECTIONS for the same diagnostic benefit. - `build_chunking_trx` takes an optional `expiration_iso` parameter (default: the existing far-future value) so the cancel-expiration test can inject a near-future value without disturbing the other chunked tests. Pre-commit sweep: contracts_unit_test 120/120, plugin_test clean, unit_test read_only_trx_tests 14/14, multisig_review_test.py end-to-end integration test passes.
operator+/-/*// for fc::variant were declared and defined but never called from anywhere in the tree (verified by grep + a clean full build of unit_test, plugin_test, contracts_unit_test, nodeop, and test_fc with the definitions removed and `= delete` declarations in their place). operator- additionally contained an unreachable bug: the array-walk loop counter decremented (`--i`) instead of incrementing, mirroring the otherwise-identical operator+. Rather than fix dead code, the four operators are marked `= delete` so any future caller fails at compile time with a clear message instead of silently invoking surprising multi-type coercion.
Adds libraries/libfc/benchmark/variant_bench.cpp, modeled on libraries/chain/benchmark/abi_serializer_bench.cpp -- plain chrono timing, no external benchmark dependency, EXCLUDE_FROM_ALL standalone target run manually. Each scenario warms up, then takes the median of 10 runs of N iterations and prints median/min/max ns/op. Scenario coverage targets the paths the fc::variant performance follow-on series will touch: ctor / copy / find / as_* / json round- trip on small and 50-key workloads. The 50-key shape mirrors an ABI-decoded get_table_rows row so deltas reflect a realistic caller. BASELINES.md documents how to build (Release is required) and which scenarios watch which catalogued perf items (lazy variant_object alloc, find_or, from_chars, hash side-table). The baseline-numbers row is a placeholder; the next commit captures it from a Release build. Build / run: ninja -C cmake-build-release -j8 variant_bench ./cmake-build-release/libraries/libfc/benchmark/variant_bench
variant_bench numbers from cmake-build-relwithdebinfo (-O2 -g -DNDEBUG) on 12th Gen Intel Core i9-12900K, clang 18.1.8. Headline observations that motivate Phase A: - as_enum_string_invalid: ~4 us per call. All of it is stoll throwing and the catch(...) unwinding. Replacing with from_chars (Phase A item 3) should drop this by 1-2 orders of magnitude. - find_hit_50key_last: 51 ns vs find_hit_50key_first 2.9 ns -- a 17x ratio. That's the linear scan over 50 entries. Phase B item 4 watches this; find_hit_4key at 4.1 ns is the regression watch (a hash index that hurts small objects is not a win). - contains_then_op_50key: 36.6 ns -- two scans of the same vector. Phase A item 2 (find_or) collapses this to one. - ctor_empty_mvo / ctor_empty_vo: 7.5 / 8.6 ns -- the make_shared<vector<entry>> allocation cost on every default ctor. Phase A item 1 (lazy-allocate) targets this. The series is pinned to RelWithDebInfo so deltas remain comparable and the binary stays debuggable; a re-baseline at -O3 can happen once at the end if absolute numbers need to leave the project.
Adds seven new test files under libraries/libfc/test/variant/ to lock
down current behaviour before the perf series starts touching it.
102 new test cases across:
- test_variant_ctor.cpp: every ctor (primitives, int128/256, char*,
wchar_t*, string, blob, variant_object,
mutable_variant_object, variants, copy,
move, optional<T>) plus deep-copy and
move-leaves-source-null invariants.
- test_variant_assign.cpp: self-assign (copy + move), cross-type
and same-type assignment, template
assignment.
- test_variant_as.cpp: conversion matrix for as_int64 /
as_uint64 / as_int128 / as_uint128 /
as_int256 / as_uint256 / as_double /
as_bool / as_string / as_blob, including
bad-cast paths.
- test_variant_enum.cpp: as_enum_value<E> for int / uint / bool /
double / valid string / invalid string /
object / array / blob sources. Watches
the upcoming stoll -> from_chars switch
(Phase A item 3) for behaviour parity.
- test_variant_visitor.cpp: visit() dispatch coverage of all 13 type
tags. Documents that int128 / uint128 /
int256 / uint256 dispatch through
handle(string), not the typed handle()
overloads -- those are unreachable in
the current visit() implementation.
- test_variant_object_misc.cpp: insertion-order preservation,
contains/find/operator[] consistency,
mvo set vs operator() (variant overload
appends, T&& template overload dedups
via set), erase, merge, op[] insert-
default-on-miss.
- test_variant_operators.cpp: ==, !=, <, > for primitives + arrays;
cross-type string coercion behaviour;
operator! truthiness; documents that
arithmetic operators are = delete.
Suite names use distinct prefixes (variant_ctor_suite,
variant_assign_suite, ...) to coexist with the pre-existing
variant_test_suite / variant_estimated_size_suite /
json_variant_test_suite without filter ambiguity.
The string-source path of variant::as_enum_value<E> previously used
std::stoll wrapped in `try { ... } catch (...) {}`. When the input
was non-numeric, stoll threw std::invalid_argument; the inner catch
swallowed it and the function then threw std::runtime_error to the
caller. Two stack unwinds per bad input.
std::from_chars is non-throwing and parses the same numeric grammar
(leading minus accepted, leading whitespace and leading '+' rejected,
suffix garbage silently ignored), so the swap is observably
equivalent for the cases the ABI serializer can produce -- the
existing test_variant_enum.cpp suite (added in the prior commit)
covers all of them.
Bench delta (libraries/libfc/benchmark/variant_bench, RelWithDebInfo,
12th Gen i9-12900K, ns/op median):
as_enum_string_valid 11.6 -> 4.6 (-60%, 2.5x)
as_enum_string_invalid 3976.4 -> 2965.0 (-25%, the outer
throw still dominates
the bad-input path)
as_enum_int 1.8 -> 1.5 (noise)
Other scenarios are within run-to-run noise (no shared code path).
Default-constructed variant_object and mutable_variant_object no
longer call make_shared<vector<entry>> / make_unique<...> at
construction time. _key_value stays null until the first mutating
operation (op[] / op() / set / reserve, or assignment from a
populated source). Read paths (begin/end/find/contains/op[]/size/
estimated_size) route through a non-const empty-vector singleton in
the null case so iterators are well-formed and comparable.
The mutable singleton is never written to: every write path
allocates a fresh vector first. As a side effect this fixes a
latent aliasing bug in `variant_object::operator=(const
mutable_variant_object&)` where the old `*_key_value = *obj._key_value`
write-through path would mutate any sibling variant_object that had
been copy-shared from the assignee. The new path always
make_shared<vector<entry>>(*obj._key_value), detaching cleanly.
find() in both vo and mvo bypasses the public begin()/end() to keep
the hot loop free of per-iteration null branches: one upfront
null-check, then iterate directly on the vector.
Bench delta (libraries/libfc/benchmark/variant_bench, RelWithDebInfo,
12th Gen i9-12900K, ns/op median):
ctor_empty_mvo 7.5 -> 1.4 (-81%, 5.4x)
ctor_empty_vo 8.6 -> 1.2 (-86%, 7.2x)
json_parse_50key 9760.6 -> ~9500 (~3%, low signal)
find_hit_50key_last 51.0 -> 55.1 (+8%, within run-to-run noise
band of +/-10% measured
across 3 runs)
walk_50key_by_name 997.4 -> 972.2 (within noise)
Adds variant_object::find_or(key, default_value) -> const variant& (plus a string-key overload). Returns the value for `key` if present, otherwise a reference to `default_value`. Caller is responsible for the default's lifetime. Replaces the common `obj.contains(k) ? obj[k] : default_v` pattern, which scans the entry vector twice on hit and throws+catches a key_not_found_exception on miss when the alternate branch is taken via op[]. Bench delta (libraries/libfc/benchmark/variant_bench, RelWithDebInfo, 12th Gen i9-12900K, ns/op median): contains_then_op_50key 38.9 ns (existing double-scan) find_or_50key_hit 19.9 ns (49% faster, single scan) find_or_50key_miss 15.1 ns (no throw on miss) Tests in test_variant_object_misc.cpp cover the hit / miss / empty- object / string-key-overload paths and assert that the returned reference is to the matching entry on hit and to the supplied default on miss.
…allers Step 1 of small-string-optimisation prep: change the public string accessor so it can return a view of inline bytes without requiring a heap std::string object. Pure API + caller migration in this commit; no SSO storage yet, all variant strings still go through `new std::string`. Migrated APIs (signatures changed; existing callers either keep working via implicit conversion or were updated): - fc::variant::get_string() now returns std::string_view (was const std::string&). as_string() (returns std::string by value) is unchanged for callers that need an owning copy. - fc::variant gains a std::string_view ctor. - fc::throw_bad_enum_cast(k, e) takes std::string_view k. - fc::reflector<E>::from_string collapsed into one std::string_view overload (was const char* + const std::string&). FC_REFLECT_ENUM_FROM_STRING uses string_view operator==; the WITH_STRIP variant replaces the `str = b + s` allocation with a starts_with + substr check on the elem name (allocation only happens when strip_base_enum=true and only for the prefix `b`). - fc::from_hex(const std::string&, ...) takes std::string_view (the body only walked iterators). - sysio::chain::asset::from_string and symbol::from_string take std::string_view; both also use std::string_view internally end- to-end via boost::algorithm::trim_copy(string_view) which returns string_view. to_int64 / sysio::chain::to_string still need owning strings, materialised at the call site. - sysio::chain::symbol's `string_to_symbol` / `string_to_symbol_c` helpers and the `symbol(uint8_t, ...)` ctor take std::string_view. - chain_plugin's `string_to_symbol` callers drop now-redundant `.c_str()` calls. Caller migrations: - fc::reflect/variant.hpp's enum from_variant adapter passes v.get_string() through to fc::reflector<T>::from_string without materialising. - bitset's from_variant passes v.get_string() into fc::bitset (which already had a string_view ctor). - variant.cpp's as_blob / from_variant<vector<char>> use string_view locally; base64_decode still gets a materialised std::string because its signature is out of scope. - sysio::chain::symbol_code's from_variant adapter constructs the symbol directly from v.get_string(). The next commit adds the actual SSO storage and reaps the construction-cost win on short strings.
Strings up to 14 bytes are now stored inline in the variant's 16-byte buffer instead of going through `new std::string`. Layout: bytes 0..13 : string content byte 14 : length (0..14) byte 15 : type tag = string_sso_type (13) Heap-allocated strings keep the existing layout (pointer in bytes 0..7, type tag string_type in byte 15) for any input longer than 14 bytes; the storage choice is made by every string-constructing ctor (const char*, char*, wchar_t*, const wchar_t*, std::string, std::string_view) based on the source length. Affected paths: - variant ctors for string-shaped sources route through a single make_string_inline_or_heap helper. std::string ctor short-circuits the move when the source fits inline, so the source's heap buffer (if any) drops on the way out. - clear() / copy ctor / copy assignment / move-related paths handle the new tag: SSO bytes are byte-copied via the existing _data array assignment with no heap involvement. - get_string() returns a view of inline bytes when the tag is string_sso_type, of the heap-resident string when string_type. - as_string() materialises a fresh std::string from inline bytes for SSO; existing heap path unchanged. - as_int64 / as_uint64 / as_int128 / as_uint128 / as_int256 / as_uint256 / as_double / as_bool / as_blob each gained an SSO branch parallel to the existing string_type branch. fc::to_int64 / to_uint64 / to_double now take std::string_view so the SSO branch can pass the view through without materialising. - is_string() returns true for both encodings. - visitor::handle(const std::string&) is now handle(std::string_view) -- visit() dispatches with a view of either inline or heap bytes for string_type / string_sso_type, and the visitor's similarly- unreachable handle(string_view) path is also used for the int128 / uint128 / int256 / uint256 cases (they have always packed a decimal-string view, not the typed handler). raw::variant_packer is the only production override; it now writes the bytes directly rather than calling raw::pack(string_view) (overload resolution picks the generic pack<Stream, T> over a string_view-typed pack via partial-ordering rules; inlining sidesteps that). - raw::pack(variant) translates string_sso_type to string_type at the wire boundary so existing peers and chainbase-resident buffers deserialize unchanged. Payload bytes are identical. - estimated_size() routes string_sso_type through the same formula as string_type (allowed to over-report; keeps tests stable). - json::to_stream handles the new tag alongside string_type. The single non-libfc caller checking `get_type() == string_type` (tests/trx_generator/trx_generator.cpp) is migrated to is_string(). Bench delta (libraries/libfc/benchmark/variant_bench, RelWithDebInfo, 12th Gen i9-12900K, ns/op median): ctor_short_string 14.2 -> 3.6 (-75%, 3.9x) ctor_sso_boundary_14 -- -> 3.4 (new row, max inline) ctor_just_over_sso_15 -- -> 14.3 (new row, just heap) ctor_long_string 19.6 -> 19.7 (unchanged, heap path) copy_short_string 11.0 -> 2.2 (-80%, 5.0x) copy_long_string 16.5 -> 18.8 (unchanged within noise) Workload-shaped scenarios (json_parse / walk) are within run-to-run noise: short-string allocation savings exist but are diluted by non-string parser cost in the 50-key fixture. New tests in test_variant_ctor.cpp cover the SSO/heap boundary, both encoding paths, copy independence, move-leaves-source-null for SSO, and SSO/heap equality.
variant::operator=(const variant&) previously did clear() (which delete's the existing heap object for heap-backed variants) followed by a fresh `new` of the rhs's underlying type. When the lhs already holds a heap object of the same type as the rhs, the dealloc+alloc pair is wasted -- the existing object can absorb the rhs's value directly via its own assignment operator. This commit checks for the same-type case at the top of op=(const&) and routes through the existing heap object for object_type, array_type, blob_type, string_type, and the std::string-backed multi-precision integer encodings (int128/uint128/int256/uint256). Cross-type assignment, and assignment between inline encodings (null/int/uint/double/bool/string_sso), continue to use the existing clear+memcpy path. Bench delta (libraries/libfc/benchmark/variant_bench, RelWithDebInfo, 12th Gen i9-12900K, ns/op median; pre/post captured by stashing the variant.cpp change and rerunning): assign_long_string_to_long 17.0 -> 3.3 (-81%, 5.2x) assign_object_to_object 10.3 -> 2.0 (-81%, 5.1x) assign_array_to_array 32.9 -> 12.1 (-63%, 2.7x) The non-assign rows in the log are unchanged within run-to-run noise (B5 only affects the same-type op=(const&) path).
The per-commit log in BASELINES.md tracks RelWithDebInfo (-O2 -g) so commits stay directly comparable. This commit captures the same post-B5 scenarios at Release (-O3 -DNDEBUG) on the same host so an external reader can see the absolute numbers Wire-Sysio hits in a deployment build. -O3 is 2-15% faster than -O2 on the inlinable paths (find loops, array copies, find_or, contains_then_op). A handful of rows (copy_short_string, copy_object_50key, as_enum_string_invalid) are slower at -O3, all within run-to-run variance for those particular scenarios. The relative ordering across scenarios is preserved, so the per-commit deltas remain meaningful. No code change in this commit -- BASELINES.md only.
Avoid materialising a std::string in variant::as_blob's base64 fallback path, where the source is already a string_view from variant::get_string(). The string_view overload is added alongside the existing const std::string& one - keeping both is necessary because the template fc::base64_decode is also visible at call sites and would silently win for std::string callers (returning std::string instead of std::vector<char>) if only string_view were left. detail::decode's remove_linebreaks branch is also routed through std::string regardless of the input String type - it needs erase(), which std::string_view does not provide.
- get_string() doc: add explicit lifetime contract - find_or() doc: add on-hit reference invalidation note - mutable_variant_object(variant) / (T&&): drop wasted entry-vector alloc (operator= replaces _key_value, so the initializer was overwritten) - mutable_variant_object: add const contains() overloads to keep the empty-state lookup allocation-free for default-constructed mvos - estimated_size: use [] instead of at() inside size-bounded loop - write_sso: static_assert(sso_max_length < 128) so a future bump can't silently corrupt the signed-byte length round-trip - test: variant_sso_wire_roundtrip pins the pack/unpack normalisation invariant (string_sso_type tag 13 must wire as legacy string_type 9) - test: self_assign_aliased_subvariant_same_type locks the spicy `v = v.get_object()["k"]` path under the same-type op= heap reuse
… assignment ASAN heap-use-after-free in self_assign_aliased_subvariant_same_type: when v aliases storage owned by *this (e.g. v = v.get_object()["k"] where lhs is object_type and rhs is string_type), the different-type path called clear() before reading from v in the new switch -- and clear() destroys the heap object that v references. Deep-copy v into a temporary BEFORE clear(), then take the temp's data and disarm its destructor. Same observable effect on lhs as the inline new-per-type switch the path used to do (the copy ctor does the same per-type allocation), just safe under aliasing. This is a latent pre-existing bug surfaced by the new test, not a regression introduced by the same-type op= heap reuse refactor.
… storage The int128/uint128/int256/uint256 variant types store their value via new std::string(...) (variant.cpp:134-155), but neither clear() nor the copy ctor handled these type tags: - clear(): leaks the heap std::string for these types on every destructor / assignment that should free them. - copy ctor: falls through to the inline _data byte-copy default arm, which copies the std::string* pointer rather than deep-copying. Two variants then share the same pointer; whichever clear() runs first (after the matched fix below) double-frees. Fix both atomically: - clear() adds the four type tags alongside string_type, all of which use the same std::string* storage shape. - copy ctor adds the same fan-out, deep-copying via new std::string(*v.string_ptr). These are pre-existing latent bugs (the leak was silent, the share was masked by the missing clear() handler so no double-free fired). Surfaced while reviewing the same-type op= refactor.
…t256 The destructor / copy-ctor / aliased-self-assign tests added with the clear() and copy ctor fixes only exercised int128_type / uint128_type. int256_type / uint256_type are guarded by the same fix in the same switch arms but had no test coverage. Add analog cases for both, mirroring the int128/uint128 tests one for one (destructor leak, copy ctor double-free / shallow-share, aliased self-assign through op=).
db.modify preserves the kv_index_object's chainbase id, so a live secondary iterator that cached the id would advance from the post-modify position and silently skip entries when sec_key changed. The prior remove+create path worked because the new object had a fresh id, so the cached id no longer resolved and iteration fell back to the slow re-seek using stored key bytes. Add kv_iterator_pool::invalidate_secondary_cache that clears cached_id only on matching secondary slots, leaving stored key bytes and status intact so the next op resumes from the old position. kv_idx_update calls it before db.modify.
…ssert Aliased self-assignment (rhs refers to storage owned by lhs) was UB in the previous fc::variant via the clear()-then-new pattern. Earlier in this branch e6e198a added a `variant tmp(v); take(tmp)` deep-copy in the different-type path that turned the UB into well-defined behaviour, but only there -- the same-type fast path added in 3e4e69a still UAFs on aliased rhs (vector and variant_object copy assignments read element-by-element from rhs while writing through lhs). Restore the previous UB contract symmetrically: - different-type path: switch back to clear()-then-new, removing the variant tmp(v) deep-copy from e6e198a. - same-type fast path: unchanged (the optimisation is preserved). - add a debug-only direct-aliasing detector (rhs_not_aliased) called from assert() so the body and call site compile out under NDEBUG. Catches v = v.get_array()[i] v = v.get_object()["k"] Deeper nesting (rhs reachable through inner array/object owned by lhs) remains UB and may slip past undetected. Drop the two tests that exercised aliased self-assign (a test cannot pin UB); keep coverage of the std::string-backed multi-precision op= paths via a new non-aliased int128_op_assign_same_and_cross_type test.
4252bc3 (this branch) replaced std::stoll with std::from_chars to avoid the exception round-trip. The two parsers agree on the inputs the ABI serializer actually emits but disagree on rejected forms: - " 1" (leading whitespace): stoll accepted, from_chars rejects. - "+1" (leading plus): stoll accepted, from_chars rejects. - "-1" / "1abc": both accept identically. The earlier comment claimed the behaviour matched stoll which was inaccurate. Update the comment to spell out the differences and pin them with new tests (string_source_with_leading_whitespace_throws, string_source_with_leading_plus_throws). ABI-emitted enum strings never contain whitespace or sign prefixes so the stricter contract surfaces malformed input as an exception rather than silently parsing it.
…ptimizations Resolve conflict in libraries/chain/apply_context.cpp at the kv_idx_update db.modify lambda. Master's remove+create added o.table_id = table_id; the merged db.modify path drops the assignment since idx.find already constrains table_id on the located row. Adapt HEAD's invalidate_secondary_cache (added in 1146c38) to master's uint16_t table_id kv API: - kv_iterator_slot/invalidate_secondary_cache use (code, table_id) instead of (code, table, index_id) - kv_idx_update call site updated accordingly - kv_tests kv_index_modify_rekeys_correctly and kv_iterator_pool_invalidate_secondary_cache rewritten against the new schema; the by_code_table_idx_prikey lookup is replaced with a 4-tuple find on by_code_table_id_seckey since master has only the seckey index.
…for unit testing The < / >= comparisons in the aliasing detector between &v and dst_vec's [begin, end) range are unspecified per [expr.rel] when &v does not point into the same array as begin. std::less<T*> is required by [comparisons]/2 to provide a strict total order over all object pointers, so it gives a defined result regardless of whether &v aliases lhs's storage. Rename the inline helper to a static member variant::_rhs_not_aliased and declare it in variant.hpp so test_variant_assign can verify the detector directly: the helper only reads pointer addresses (no deref, no write), so calling it on intentionally-aliased pointers is well-defined. The UB lives in the full operator= flow that follows the assert, which the tests do not invoke. Pointed out by huangminghuang on PR #315.
The merge from master in cd02d6d brought commit 30b1697, which collapsed kv_index_object's (table, index_id) into a single uint16_t table_id and reduced kv_iterator_pool::{allocate_secondary, invalidate_secondary_cache} to 2/3 args. apply_context.cpp::kv_idx_update merged correctly; the two test cases added in 4fe6dfd and 1146c38 still referenced the old fields and signatures. - kv_index_modify_rekeys_correctly: rewritten against table_id and by_code_table_id_seckey. Replaces the by_code_table_idx_prikey lookup (no longer a separate index) with a composite find on (sec_key + pri_key) plus an explicit "old key no longer resolves" check. - kv_iterator_pool_invalidate_secondary_cache: 2-arg allocate_secondary, 3-arg invalidate_secondary_cache. Old (different table) vs (different index) slots collapse to "different table_id"; kept as two distinct table_ids so the loop preserves more than one non-matching slot.
…ions Optimize kv_idx_update: use modify instead of remove+create
…d names Wire CDT emits `first`/`second` for `std::pair` and `std::map` fields in generated ABIs (see tests/toolchain/abigen-pass/nested_container.abi in wire-cdt). cc1dde4 previously swapped in a Leap-derived ABI using `key`/`value` as a workaround, but it regresses every time the WASM is regenerated and diverges from Wire CDT's own toolchain tests. Remove the workaround. The contract ABI now uses `first`/`second` matching CDT output, and the Python test inputs and assertions are updated to match. The `pvo` case (`pair<uint16_t, vec_op_uint16>`) was already on `first`/`second` and is unchanged. `transaction_json[...]['value']` row accessors are untouched - that is the clio RPC envelope field, not a pair field name. WASM is unchanged: kv_multi_index binary serialization is positional, so the field rename is purely a JSON-side concern.
…st-cdt-abi-names Test: align nested_container_multi_index with CDT pair/map field names
…ools
Pre-split, apply_context kept a single 1024-slot kv_iterator_pool with
each slot a union of every field used by either iterator kind (four
std::vector<char> buffers + an is_primary flag). Hot paths
(kv_it_next, kv_idx_next) loaded cache lines holding data the
operation never touched, and a contract that iterated both kinds
concurrently competed for the same 1024-slot budget.
Pool split
----------
* kv_iterator_slot_common holds the fields every iterator needs
(in_use, status, code, table_id, cached_id).
* kv_primary_slot adds primary-only byte buffers (prefix,
current_key).
* kv_secondary_slot adds secondary-only fields (current_sec_key,
current_pri_key).
* kv_primary_iterator_pool and kv_secondary_iterator_pool each
expose config::max_kv_iterators (1024) slots independently. A
contract may now hold up to 1024 primary and 1024 secondary
iterators simultaneously.
* Both pools lazily resize on first allocate(). Actions that never
touch KV iterators (e.g. sysio.token transfer, which routes
through kv_get/kv_set only) pay zero heap for the pools. First
allocate pays the same ~82 KB this code paid up front before;
no realloc, no reference invalidation, identical get() path.
Handle encoding (CONSENSUS-OBSERVABLE)
--------------------------------------
Handle values are contract-visible -- they are the return value of
kv_it_create / kv_idx_find_secondary / kv_idx_lower_bound, and
contracts may read, store, and branch on them. The encoding is
part of the consensus surface.
[ 0.. 9] slot index (covers max_kv_iterators = 1024)
[10..15] RESERVED -- must be zero
[16 ] secondary-pool tag (1 = secondary, 0 = primary)
[17..30] RESERVED -- must be zero
[31 ] always zero -- keeps handles positive when read as
int32_t (negative is reserved for "not found")
Bit 16 was chosen over a high bit so handle values stay small and
readable in logs/error messages (secondary handle 0x00010005 vs
0x40000005). Reserved bits give future protocol features room to
encode e.g. iterator generation counters or additional pool
discriminators without disturbing deployed contract code. Any
change to this layout is a protocol change.
Reserved-bit guard
------------------
kv_handle_check_reserved_zero() is called at every host-intrinsic
entry point that consumes a handle. A contract that fabricates a
handle by setting reserved bits fails with a clean
kv_invalid_iterator exception instead of silently aliasing a real
slot through the truncated slot-index mask.
Prefix-size bound on kv_it_create
---------------------------------
Cap prefix_size at config::max_kv_key_size_limit (constexpr absolute
ceiling, 1024 B). The prefix bytes are memcpy'd into the iterator
slot's std::vector<char>, so an unbounded prefix_size would let a
contract allocate arbitrary host-side storage per iterator (up to
max_kv_iterators slots per action). The absolute ceiling is used
instead of the dynamic max_kv_key_size because the cap exists to
limit host-side slot memory, not to match the current on-chain
stored-key limit, and the constexpr compile-time compare has zero
runtime cost. Legitimate CDT usage passes 0 or kv_scope_size
(8 bytes), far below the cap.
Capacity change
---------------
No contract that worked pre-split gets a new error. Some that
exhausted the shared 1024-slot pool mid-mixed use now succeed
(primary+secondary sum up to 2048).
Host ABI / chainbase
--------------------
No change. Host intrinsic signatures, kv_object/kv_index_object
layout, and serialization are all untouched.
…re tests
- kv_context.hpp: static_assert that config::max_kv_iterators fits in
kv_handle_slot_index_mask + 1 (catches mask-narrowing footgun if the
iterator budget grows).
- kv_context.hpp: validate_primary_handle / validate_secondary_handle
and kv_check_prefix_size moved out of apply_context.cpp into the
header as inline helpers. Lets the destroy paths share the same
validation as the other entry points and makes the checks unit-
testable without spinning up an apply_context.
- apply_context.cpp: kv_it_destroy / kv_idx_destroy now go through
validate_primary_handle / validate_secondary_handle, matching the
pattern of every other kv_it_* / kv_idx_* entry. kv_it_create's
prefix-size cap moves to kv_check_prefix_size.
- kv_tests.cpp:
* kv_validate_handle_dispatch: end-to-end coverage of the
validators (well-formed handle, wrong-pool tag, each
reserved-bit class).
* kv_check_prefix_size_bounds: at-limit OK, over-limit throws.
* kv_iterator_pool_basic: drop the implicit assumption that
primary handles equal slot indices -- compare against
kv_handle_slot_index(h1) so future encoding changes do not
break the test.
Remove the 43 host implementations of compiler-rt intrinsics: 9 int128 ops (__multi3, __divti3/__udivti3, __modti3/__umodti3, __ashlti3/ __ashrti3/__lshlti3/__lshrti3) and 34 softfloat-backed ops (__addtf3 family, comparisons, all int<->f128 / sf<->ti / df<->ti conversions). Contracts now resolve these locally from CDT's librt (default-linked) and libsf (opt-in via --use-rt) -- the host no longer needs to provide them, eliminating 43 entries from the consensus audit list. - intrinsic_signature_registry.hpp / sys-vm.cpp / sys-vm-oc/ intrinsic_mapping.hpp: drop 43 SYS_PIN_INTRINSIC + 43 REGISTER_*_HOST_FUNCTION + 43 PIC entries. - libraries/chain/webassembly/compiler_builtins.cpp: delete. - libraries/builtins/: delete -- the in-tree wasm builtins library was consumed only by the deleted host file. - libraries/CMakeLists.txt / libraries/chain/CMakeLists.txt: drop source / link references. - interface.hpp: drop the 43 method declarations. - sys-vm-oc/gs_seg_helpers.h: SYS_VM_OC_MEMORY_STRIDE drops by one 4KB page (the prologue shrinks past a rounding boundary). The static_assert in memory.hpp is the deliberate tripwire that flagged the drift; pre-launch the renumber is fine. - wasm_interface.cpp: drop the unused #include <compiler_builtins.hpp>. Pairs with wire-cdt cleanup/drop-builtin-host-fallback (librt rewrite, default-link, cdt.imports prune).
…r-builtin host removal Companion change for the host-side removal of 128-bit compiler builtins (this branch) and wire-cdt PR #51, which rewrites the builtins as standalone librt implementations and stops emitting env-imports for them. After PR #51, contracts compiled with the updated CDT link the builtins locally instead of importing env.__ashlti3, __ashrti3, __lshrti3, etc. Pre-populating the source tree with WASMs built against PR #51 means every replay path already references contracts the new host can accept. Rebuilt the 12 system contracts under contracts/sysio.* and the 30 test contracts under unittests/test-contracts/ with the updated CDT.
…build Catch the savanna/ibc test contract that was missed by the unittests/test-contracts/*/ glob in the previous sync commit (savanna/ibc lives one level deeper). A fresh contracts_project rebuild against wire-cdt PR #51 produced a smaller sysio.system WASM (131375 -> 122636 bytes) and a reordered ABI; sync those too.
Fc: variant performance improvements
…swap Flip WIRE snapshot magic for hex-dump readability
…plit Kv: split iterator slot pool into independent primary and secondary pools
PR #308 was originally stacked on the unmerged primary_id work. After rebase onto master, probe contract had to be retargeted to master's legacy_span pri_key host signatures. Dropped: 5 primary_id-only probes (negative/missing pid, cross-contract pid). Added: oversize pri_key probe, kv_it_value/kv_it_status on secondary handle rejection probes. Rewrote: prmera/danglng (master stores pri_key inline, no IT_ERASED on primary erase); lbpast (use kv_idx_next for end-state check since kv_it_status rejects sec handles); zval (kv_set returns RAM billable, not a stable id).
…ck_report The 1ms slack on |captured - baseline| total_elapsed_time_us was too tight for CI jitter (observed 3026us delta on a clean run). net/cpu/elapsed all aggregate at the same controller site under one condition, so they leak together or not at all. net_usage_us == 0 and cpu_usage_us == baseline are deterministic and already catch the same bug; the wall-clock check only added jitter noise. Failing CI: https://github.com/Wire-Network/wire-sysio/actions/runs/25808578465/job/75826113588
…tests Test: add adversarial kv_* host intrinsic probes
Background ---------- PR #308 (feature/kv-intrinsic-probe-tests) added adversarial probes for the 22 kv_* host intrinsics by driving the raw host ABI with inputs CDT's kv_multi_index / kv::table wrappers would never emit. This PR is the companion suite covering the remaining non-kv host intrinsics: the crypto family (sha1/sha256/sha512/ripemd160 + assert_*, recover_key/assert_recover_key), the 128-bit integer and float128 compiler_builtins, preactivate_feature, the privileged_check-gated resource / blockchain-parameters ops, and the console / action-data legacy_span readers. Motivation is the forthcoming legacy_span/legacy_ptr -> span cleanup. That cleanup touches every intrinsic in the list above, so the invariants the current implementation relies on -- alignment-proxy copy-in/out paths for legacy_ptr<fc::sha*> / legacy_ptr<int64_t, 8> / legacy_ptr<__int128>, zero-length-span acceptance, "query required size with buffer=0" semantics, SYS_ASSERT-throws-vs-returns-sentinel contracts -- need to be pinned before the refactor lands. Layout ------ unittests/test-contracts/intrinsic_probe/intrinsic_probe.cpp declares every host intrinsic it probes via extern "C" + __attribute__((sysio_wasm_import)). The few that conflict with sysio::internal_use_do_not_use wrappers CDT already declares (prints, sysio_assert, send_inline, get_action, read_transaction, etc.) are reached through a raw:: alias. One [[sysio::action]] per probe -- 97 probes total across sha family (16), assert_sha family (16), recover_key family (13), preactivate_feature (3), compiler_builtins int128 (12), compiler_builtins float128 (9), resource/auth/producer/blockchain (11), console/IO (17). unittests/intrinsic_probe_tests.cpp deploys the contract to both a non-privileged account (intprobe) and a privileged account (intprobe2); each BOOST_FIXTURE_TEST_CASE routes to the appropriate account based on whether the intrinsic under test is privileged_check-gated. Shared validating_tester singleton avoids re-paying bios setup per case. Setup policy is preactivate_feature_and_new_bios so set_privileged() can route through sysio.bios::setpriv and reserved_first_protocol_feature remains unactivated for the preactok probe. Coverage -------- Crypto -- four probes per hash family (golden, empty input, 1KB input, unaligned out-ptr forcing argument_proxy<fc::sha*, 8> copy-out path), four per assert_* family (correct match, zero-hash mismatch rejection, empty input with empty hash golden, unaligned const-hash ptr forcing copy-in path). recover_key / assert_recover_key -- each of the distinct failure paths is isolated: empty sig (fc::raw::unpack runs dry), short sig (truncation mid-shim), bad variant tag (fc::raw variant unpack out-of-range), bad recovery byte (elliptic_secp256k1.cpp FC_THROW_EXCEPTION "unable to reconstruct public key"), bad r/s (math either succeeds with a different pub or fails -- both acceptable, probe and driver handle both), K1 small-pub buffer (fc::datastream FC_ASSERT for fixed-size key types), unaligned digest (argument_proxy copy-in path). assert_recover_key adds the type-mismatch and recovered-pub-mismatch SYS_ASSERT paths separately. compiler_builtins -- each intrinsic probed with golden, unaligned out-ptr (argument_proxy<__int128*, 16> / argument_proxy<float128_t*, 16> copy-out path), and edge values (U64 carry in __multi3, INT128 shift >= 128 in __ashlti3, divide-by-zero in __div[u]ti3, NaN in __multf3, 1.0/0.0 in __divtf3, 2^127 overflow in __fixtfti). Privileged gates -- three probes per priv-gated family: the accepted priv path, the non-priv rejection (unaccessible_api), and either a body-level rejection (preactivate_feature with bogus digest) or an idempotent in-priv probe (setreslim reads current limits, bumps, restores so shared-tester state is unaffected). P2 -- get_resource_limits with aligned and unaligned int64_t out-ptr trios (the aligned-proxy copy-back path is the part most likely to regress under the cleanup), get_blockchain_parameters_packed with size=0 and with a buffer sized to the size query, get_active_producers with a sufficient buffer and with size=0, set_* probes for each family's non-priv rejection. P3 -- prints (null_terminated_ptr) and prints_l (legacy_span) both covered; sysio_assert vs sysio_assert_message both covered in firing and non-firing variants; sysio_assert_message with empty span pinned (not crashed); read_action_data with zero size pinned (returns 0, doesn't overflow); read_transaction and get_blockchain_parameters_packed both pin the "size=0 returns required bytes" contract; get_action with invalid type pinned to throw action_not_found_exception rather than return -1 (its return-(-1) path is only for valid-type / out-of-range-index); send_inline with an empty span pinned to throw during action unpack (no silent no-op). Host behaviors pinned for regression ------------------------------------ - assert_sha family throws crypto_api_exception "hash mismatch" via SYS_ASSERT; not a generic fc::exception. The separate exception-cleanup follow-on noted below can tighten the other recover_key paths toward the same shape; this pin gives that refactor a baseline. - recover_key for K1 with a pub buffer smaller than 34 bytes throws (fc::datastream fixed-size pack FC_ASSERT). Variable-size keys (ed, wa) silently truncate via memcpy -- an API inconsistency worth flagging to the follow-on cleanup but NOT a regression surface today (not reached with the K1 test vector this suite uses). - apply_context::get_action with type not in {0, 1} throws action_not_found_exception. The -1 sentinel applies only to valid-type / out-of-range-index. - __divti3 / __udivti3 throw arithmetic_exception on rhs==0 (SYS_ASSERT in compiler_builtins.cpp). - __ashlti3 with shift >= 128 returns 0 -- well-defined saturation, not UB. - __divtf3(1.0, 0.0) returns +Inf, not a throw -- IEEE 754 semantics preserved through softfloat. - privileged_check fires BEFORE the host body, so non-priv-rejection probes never reach their digest / buffer-content validation -- pinned uniformly across preactivate_feature / set_resource_limits / set_proposed_producers / set_blockchain_parameters_packed. - sysio_assert and sysio_assert_message both throw sysio_assert_message_exception on test==0, NOT sysio_assert_code_exception (those are distinct intrinsics with distinct exception types). Exception-cleanup follow-on (not in this PR) -------------------------------------------- recover_key / assert_recover_key currently surface three categories of failure with three different exception types: structural unpack errors leak as fc::exception, secp256k1 math errors leak as fc::assert_exception, and SYS_ASSERT-gated errors are the only ones that produce a typed crypto_api_exception. A follow-on should wrap fc::raw::unpack and public_key::recover calls in try/catch and rethrow as crypto_api_exception with a descriptive message so contracts and oncall can catch a single typed exception for "signature recovery failed". The small-pub-buffer inconsistency (fixed-size keys throw, variable-size keys truncate silently) should be normalized in the same PR. This probe suite pins current behavior; when the follow-on lands, several BOOST_CHECK_THROW types here tighten from fc::exception to crypto_api_exception. Verification ------------ 97 test cases x 3 WASM runtimes (sys-vm, sys-vm-jit, sys-vm-oc) = 291 invocations, all green. Refs ---- Companion to PR #308 (feature/kv-intrinsic-probe-tests). Same layout: dedicated test-contract directory, extern "C" raw imports at the call site for ABI explicitness, one [[sysio::action]] per probe, shared-tester singleton for cost amortization, pre-built .wasm/.abi committed alongside source.
… boundary The compiler-rt fallbacks ___fixtfti / ___fixdfti / ___fixsfti (float128 / double / float to int128) performed the final left shift in signed __int128. Any input with magnitude exactly 2^127 -- i.e. ldexp(1.0, 127) in any of the three precisions -- produces the bit pattern 2^127 during the shift, which overflows INT128_MAX and is signed-shift UB in C. UBSan with -fno-sanitize-recover=all flags this as: libraries/builtins/fixtfti.c:36:46: runtime error: left shift of 0x00010000000000000000000000000000 by 15 places cannot be represented in type '__int128' Without a sanitizer the result is a platform-dependent bit pattern -- INT128_MIN on the positive side (silent wrong answer), correct INT128_MIN on the negative side -- which violates the consensus-determinism contract for on-chain code. Fix: perform the left shift in unsigned __int128 and apply the sign with explicit saturation: - positive: mag > INT128_MAX -> INT128_MAX, else (__int128)mag - negative: mag >= 2^127 -> INT128_MIN, else -(__int128)mag The right-shift branch is unchanged; significand fits in rep_t with room to spare so the signed shift is defined there. The existing f128_fix_overflow probe only asserted "result != 0", which was already satisfied by the UB bit pattern. Tightened to the exact INT128_MAX saturation value so the fix is pinned normatively. Added matching fxdfovfl / fxsfovfl actions and f64_fix_overflow / f32_fix_overflow test cases so the double and float host paths (___fixdfti / ___fixsfti) are exercised at the same saturation boundary. Contract WASM + ABI regenerated via cdt-cpp.
…drs probe The recover_key_corrupt_rs case caught fc::exception broadly, which also swallows sysio_assert_message_exception -- the very type the contract's `check(recovered != original)` security assertion throws. Narrow the catch ladder so the contract assertion path re-surfaces as a BOOST_FAIL while the host secp256k1 math-failure path stays acceptable.
…rage boundary Resolves the reviewer finding that several non-kv host intrinsics were declared in the probe contract but never exercised. get_context_free_data: it is registered REGISTER_LEGACY_CF_ONLY_HOST_FUNCTION, so from a regular action its context_free_check precondition throws unaccessible_api before the body runs. Add a gcfdcf probe + driver case pinning that CF-only rejection, mirroring the existing privileged_check rejection probes (preactnp / setresnp). compiler_builtins (__modti3 / __umodti3 / __ashrti3 / __lshlti3 / __lshrti3 / __subtf3): not probed by design. The stacked host-compiler-builtin removal deletes these host implementations outright and relocates equivalent coverage (including INT128_MIN/-1 and shift>=128 edges) into the contract-side librt suite. Adding host-ABI probes here would pin behavior that change deletes. Documented as an explicit coverage boundary at the declaration block instead.
Pin vcpkg registry references
…al-probes Test: add adversarial non-kv host intrinsic probes
…pan-cleanup Resolve 4 add/add conflicts in the intrinsic_probe test contract + driver, all from the adversarial probes independently landing on master via #308/#310. Took the PR-side (never-throw recover_key, matching the merged crypto.cpp) and unioned in master's one orthogonal new probe (gcfdcf / get_context_free_data_non_cf). Regenerated wasm/abi from the merged contract source.
Test: drop wall-clock elapsed slack from failed_trx_excluded_from_block_report
Drop aligned proxy for byte buffers; normalize recover_key contract; signature-registry canary
…n-host-intrinsics All 9 conflicts were regenerated reference data / generated artifacts (no source conflicts -- compiler-builtin removal + test retargeting auto-merged with master's #314 content). Resolved by regenerating from the merged code compiled with the wire-cdt PR #51 toolchain (contract-side librt, no env compiler-builtin fallback): - contracts: rebuilt with PR-#51 CDT (0 env builtin imports); only sysio.system + intrinsic_probe differ vs the branch's checked-in artifacts. - deep-mind.log: regenerated (unchanged -- scenario unaffected). - snapshots (blocks.*, snap_v1.*): regenerated; head_block_id in sysio_util_snapshot_info_test.py set to the regenerated value. - consensus_blockchain/snapshot: regenerated. Pairs with wire-cdt PR #51 (cleanup/drop-builtin-host-fallback).
The int128/softfloat compiler-builtins no longer have interface declarations, signature pins, or sys-vm registrations, so sys_vm_host_functions_t::resolve() can never satisfy a module that imports them. Seeding them into the genesis intrinsic whitelist only advertised non-resolvable intrinsics in new-chain / snapshot state. Remove all 43. Regenerated snap_v1 and consensus_blockchain reference data for the shrunk whitelist table; chain_id, head_block_id, blocks.log and the deep-mind log are unaffected.
This PR deletes libraries/builtins and stops building/installing the archive, but SysioTester.cmake.in still resolved it via find_sysio_library and linked it into the SysioChain interface target. Downstream find_package(sysio) / include(SysioTester) would fail at the unresolved find_sysio_library(libbuiltins ...). Remove both references to match the in-tree target removal.
…ntrinsics Drop host compiler-builtin intrinsics; retarget tests at contract-side librt
…d-storage Sole conflict was the compiled contracts/sysio.msig/sysio.msig.wasm. Resolved by rebuilding sysio.msig from the merged source (the chunked-storage feature plus master) with the current CDT (wire-cdt master, post-#51 contract-side librt); sysio.msig.abi regenerated to match. The rebuilt contract has no env compiler-builtin imports, and reference data is unaffected (sysio.msig is not deployed in the snapshot / deep-mind / consensus scenarios).
sysio.msig: chunked storage for large proposals + read-only getproposal
…aster-sync # Conflicts: # contracts/sysio.authex/sysio.authex.wasm # contracts/sysio.chalg/sysio.chalg.wasm # contracts/sysio.epoch/sysio.epoch.wasm # contracts/sysio.msgch/sysio.msgch.wasm # contracts/sysio.opreg/sysio.opreg.wasm # contracts/sysio.roa/sysio.roa.wasm # contracts/sysio.uwrit/sysio.uwrit.wasm # libraries/chain/webassembly/crypto.cpp # libraries/chain/webassembly/runtimes/sys-vm.cpp
…ement' into feature/opp-part3-master-sync # Conflicts: # contracts/sysio.uwrit/src/sysio.uwrit.cpp # contracts/tests/sysio.uwrit_tests.cpp # libraries/chain/include/sysio/chain/webassembly/interface.hpp # libraries/chain/webassembly/crypto.cpp # libraries/chain/webassembly/runtimes/sys-vm.cpp
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Merges
origin/master(tipb11958e26e, incl. msig-chunked-storage PR #292) into theopp-part3line. Brings the opp underwriter/operator work up to date with master, including the newsysio::try_recover_keyCDT API.Merge commit:
9f4591323d(2-parent:22afa4dbe6opp +b11958e26emaster).Conflict resolution
sysio.uwritmigratedrecover_key_nothrow→sysio::try_recover_key(matches the new wire-cdt API; ABI cross-checked identical to a fresh rebuild)..wasmconflicts resolved deterministically by a cleancontracts_projectrebuild againstwire-cdt-masterCDT (which hastry_recover_key), then copying the regenerated artifacts in..abiverifiedsemantically unchanged.
Validation
sysio_uwrit_tests— validates therecover_key_nothrow → try_recover_keyresolution.sysio_msig_tests— the merge's source feature (PR sysio.msig: chunked storage for large proposals + read-only getproposal #292).Known PRE-EXISTING failures — not introduced by this merge
Proven by running the failing suites against the committed opp
.wasm(pre-mergeHEAD, old CDT): they fail identically (dispatch fails more: 3 vs 1). Zero sourcefiles under these contracts /libraries/oppchanged in the merge. Tracked for triage, not merge blockers:sysio_epoch_flushwtdw_tests(7) — fixture doesn't deploysysio.uwrit, butepoch::advanceunconditionally inline-callssysio.uwrit::chklocks(sysio.epoch.cpp:350-355, opp commit93862b4548). Test-side fix.sysio_reserve_tests(11) —pack_exception:setreservetest helper(
sysio.reserv_tests.cpp:72-76) omits theoutpost_kindABI field. Test-side fix.sysio_dispatch_tests(1) — logic gap: deliveredWITHDRAW_REQUESTdoesn't create theopreg::wtdwqueuerow (sysio.dispatch_tests.cpp:519). The CDT rebuild fixed 2 of 3 sibling cases; this one needs a dispatch→opreg source investigation.