Drop aligned proxy for byte buffers; normalize recover_key contract; signature-registry canary#314
Merged
Merged
Conversation
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.
fed7592 to
249ef52
Compare
Background ---------- The forthcoming legacy_span / legacy_ptr -> span cleanup touches ~80 host-intrinsic signatures in libraries/chain/include/sysio/chain/webassembly/interface.hpp. Any accidental drift during the cleanup -- a constness flip on a sha256 hash out-param, a legacy_span<account_name> quietly losing its Align=8, a kv_set parameter reorder, an early-return int64_t flipped to int32_t -- would be observable only as a behavioral regression in tests or, worse, in production. Separately, AntelopeIO/leap#621 documents a long-standing trap in the sys-vm-oc runtime: adding a REGISTER_*_HOST_FUNCTION(foo) without also adding "env.foo" to get_intrinsic_table() in sys-vm-oc/intrinsic_mapping.hpp builds clean and only fails at runtime (or worse, silently maps to a bogus jump-table ordinal), because find_intrinsic_index() returns SIZE_MAX for unknown names and that flows through integral_constant<size_t, SIZE_MAX> as a legal constant expression. Leap was blocked on c++20 constexpr std::string::c_str(); wire is on c++23 (CMakeLists.txt:10) so the port is unblocked. Both are compile-time signals for consensus-critical drift. Both are pure-additive. Landing them together so the checkpoint gives a single coherent "what guards are in place before the cleanup touches anything" answer. Part A -- host-intrinsic signature registry -------------------------------------------- New header: libraries/chain/include/sysio/chain/webassembly/intrinsic_signature_registry.hpp -- one static_assert(std::is_same_v<decltype(&interface::fn), ExpectedSig>) per host intrinsic the cleanup will touch, plus representative non-cleanup signatures for completeness. 110 intrinsics pinned, covering: - All legacy_span<[const] char> readers / writers: sha family, ripemd160, assert_*, recover_key, check_*_authorization, set_proposed_producers{,_ex}, set_blockchain_parameters_packed, get_context_free_data, read_action_data, get_action, read_transaction, send_inline, send_context_free_inline, printhex, sysio_assert_message, prints_l, all 22 kv_* intrinsics. - All legacy_ptr<[const] fc::sha*> / legacy_ptr<[const] digest_type> / legacy_ptr<int64_t, 8> / legacy_ptr<[const] __int128> / legacy_ptr<[const] float128_t> cases: preactivate_feature, is_feature_activated, get_resource_limits, all assert_sha / sha / recover_key / assert_recover_key digest params, all compiler_builtins 128-bit integer + float128 intrinsics (__multi3, __divti3, __ashlti3, __addtf3, __multf3, __fixtfti, ..., 38 total). - null_terminated_ptr cases: prints, sysio_assert. - The span<[const] char> cohort that is meant to stay span<> through the cleanup: alt_bn128_*, mod_exp, blake2_f, blake2b_256, sha3, k1_recover, all bls_*, set_finalizers, set_action_return_value, get_parameters_packed, set_parameters_packed, get_wasm_parameters_packed, set_wasm_parameters_packed, get_permission_lower_bound. - memcpy/memmove/memcmp/memset (struct-by-value memcpy_params / memcmp_params / memset_params params) since their from_wasm unpack logic is consensus-critical. sys-vm.cpp's include list gets one new line so the static_asserts compile exactly once per build rather than per consuming TU. softfloat _sysio_f32_* / _sysio_f64_* intrinsics are intentionally NOT pinned -- they are plain float / double / bool args with no proxy types, the cleanup does not touch them, and their signatures are boring enough that a canary adds only noise. How a registry entry fires: when the cleanup PR flips (for example) legacy_span<const char> to span<const char> in kv_set's signature, the registry's corresponding SYS_PIN_INTRINSIC( kv_set, ...) fails at compile time with kv_set host-intrinsic signature drifted -- update intrinsic_signature_registry.hpp, bump protocol feature, regen reference data, sync wire-cdt Resolving that error is the engineer's checklist for doing the signature change correctly: update the registry, bump the protocol feature, regen deep-mind / snapshot / consensus-blockchain reference data per CLAUDE.md, sync wire-cdt's corresponding declarations. All four steps are implied by the diagnostic message. This commit itself caught one signature discrepancy during authorship: get_sender's pinned shape was initially uint64_t (interface::*)() const; interface.hpp actually returns name. The static_assert fired during the first compile, the registry was corrected, and the build is green. That anecdotal catch is the kind of drift the canary is built for. Part B -- OC intrinsic-mapping canary (port of AntelopeIO/leap#621) ------------------------------------------------------------------ New template in sys-vm-oc/intrinsic_mapping.hpp: template<std::size_t Index> struct require_intrinsic_index { static_assert(Index != std::numeric_limits<std::size_t>::max(), "OC intrinsic mapping missing -- the host function is " "REGISTER_*_HOST_FUNCTION'd but its \"env.<name>\" string " "is not in get_intrinsic_table() above. Add it to the " "table so sys-vm-oc can resolve the intrinsic at its " "fixed jump-table offset."); static constexpr std::size_t value = Index; }; Six call sites updated to route find_intrinsic_index through require_intrinsic_index: - sys-vm-oc.hpp:343 (depth_assertion_intrinsic_offset) - sys-vm-oc.hpp:396 (register_sysvm_oc template body) - executor.cpp:78 (grow_memory_intrinsic) - executor.cpp:87 (sysio_exit_intrinsic) - executor.cpp:100 (DEFINE_SYSVMOC_TRAP_INTRINSIC macro, covers 5 trap intrinsics) - executor.cpp:138 (check_memcpy_params_intrinsic) Canary verified by temporarily renaming one call site to find_intrinsic_index("sysvmoc_internal.grow_memory_TYPO") and rebuilding. Clang fires error: static assertion failed due to requirement '18446744073709551615UL != std::numeric_limits<unsigned long>::max()': OC intrinsic mapping missing -- the host function is REGISTER_*_HOST_FUNCTION'd but its "env.<name>" string is not in get_intrinsic_table() above. note: in instantiation of template class 'sysio::chain::sysvmoc::require_intrinsic_index<18446744073709551615>' requested here 78 | require_intrinsic_index<find_intrinsic_index("sysvmoc_internal.grow_memory_TYPO")>::value with the offending call site named in the note. Typo reverted. Deferred -------- Defensive guards on REGISTER_HOST_FUNCTION itself -- "return type must be void/primitive/name", "no parameter may be a raw T*" -- were scoped into Tier 0 of the cleanup plan but carved out here. They protect against a different class of drift (a NEW intrinsic being added with a wrong-shaped signature) which is not what this cleanup touches. Bundle them into a follow-on if the review feedback wants tighter guards, otherwise leave for future work. Scope / risk ------------ No runtime change, no CDT change, no protocol feature, no reference-data regen. chain lib compiles, unit_test passes, intrinsic_probe_tests runs green under sys-vm-oc (exercising the OC path). The change fits in one new header + one registry-template addition to an existing header + seven call-site one-liners. Refs ---- - AntelopeIO/leap#621 -- the OC intrinsic-table check, c++20-blocked there, unblocked for wire on c++23. - Companion to PR #310 (feature/intrinsic-adversarial-probes) which added the runtime probes for the same cleanup.
…sics)
Background
----------
argument_proxy<span<T>, Align=alignof(T)> with T = char / const char has Align=1; its ptr % 1 == 0 alignment check is trivially always true, so the legacy-proxy copy path is never taken for byte spans. What legacy_span<[const] char> actually costs at every call site is the proxy wrapper itself: a bigger struct (void* original_ptr + unique_ptr<char[]> copy), an extra branch in the constructor, a non-trivial destructor that runs an always-empty memcpy-back check. Plain vm::span<[const] char> is just {ptr, size} passed by value -- no proxy, no branch, no destructor work.
At the wasm ABI level the two are identical: both unpack from (i32 ptr, i32 size). No wasm-visible shape changes, no CDT-side change required (CDT still declares the import as (const char*, uint32_t) and contracts are unaffected). This commit is therefore a pure host-side struct-overhead reduction on the hot intrinsic path.
EOSIO precedent: when the original kv_* intrinsics were introduced in EOSIO/eos#8223 (2020) they already used plain span<const char>; the non-legacy span variants were the intended shape all along. Wire inherited the legacy_span spelling for byte buffers as a copy-paste artifact, not a deliberate ABI choice. AntelopeIO/eos#8996 explicitly documents that "only span<char>/span<const char> is whitelisted for in-place zero-copy because char has alignment 1."
Scope
-----
41 signatures in libraries/chain/include/sysio/chain/webassembly/interface.hpp converted:
- legacy_span<char> -> span<char> (9 uses -- output byte buffers: get_blockchain_parameters_packed, read_action_data, read_transaction, get_action, get_context_free_data, recover_key pub, the 4 kv_it/kv_idx key/value out-params and recover_key out-pub)
- legacy_span<const char> -> span<const char> (32 uses -- input byte buffers: sha*/assert_sha*/ripemd160 data, recover_key/assert_recover_key sig+pub, check_*_authorization payloads, sysio_assert_message, prints_l, printhex, send_inline, send_context_free_inline, set_proposed_producers{,_ex}, set_blockchain_parameters_packed, all 16 kv_* byte-span params)
Implementation files (.cpp) in libraries/chain/webassembly/ updated in lockstep: action, crypto, cf_transaction, permission, transaction, console, kv_database, cf_system, privileged, context_free. Function bodies required no edits -- argument_proxy<span<T>, A> inherits from span<T>, so .data() / .size() / iteration are identical after the type change.
intrinsic_signature_registry.hpp (Tier 0 canary) updated to match; its 48 pinned signatures move in lockstep with interface.hpp so drift detection stays intact.
preconditions.hpp gets one new specialization: is_whitelisted_legacy_type<vm::span<T>> with the same char-only rule as is_whitelisted_type<vm::span<T>>. This lets REGISTER_LEGACY_*_HOST_FUNCTION-registered intrinsics carry mixed args -- some legacy_ptr, some plain span -- through legacy_static_check_wl_args. Needed because many Tier-1 intrinsics are hybrids (e.g. sha256(span<const char> data, legacy_ptr<fc::sha256> hash)) and will stay hybrid until Tier 2 redesigns the hash-API output param too.
Explicitly NOT touched
----------------------
- legacy_span<account_name> (get_active_producers, 1 use) -- account_name is uint64_t, not char, so the whitelist does not accept span<account_name>. Moving it cleanly requires either extending the whitelist with alignment validation in sys-vm's from_wasm OR reshaping the signature to span<char> + reinterpret. Deferred to Tier 2.
- All legacy_ptr uses -- 46 of them across fc::sha* / digest_type / int64_t/128_t / float128_t. Tier 3 and the hash-API redesign (Tier 2) own those.
- argument_proxy / null_terminated_ptr machinery itself -- still in common.hpp; only the call-site uses for byte buffers are swapped.
- Softfloat _sysio_f32/f64_* intrinsics -- plain float/double args, no proxy types, untouched.
- wire-cdt -- no change required. The wasm ABI for (ptr, u32) is identical for legacy_span<char> and span<char>, so CDT's __attribute__((sysio_wasm_import)) declarations remain valid.
Verification
------------
- Chain lib: 20 TUs compile clean under c++23. No REGISTER_*_HOST_FUNCTION changes needed; preconditions specialization keeps the precondition happy for hybrid intrinsics.
- intrinsic_probe_tests (97 cases) green on all three runtimes: sys-vm, sys-vm-jit, sys-vm-oc.
- crypto_golden_tests, kv_api_tests, kv_tests all green.
- Tier 0 signature registry: no drift; the registry updates were the expected mirror of interface.hpp.
Risk
----
No wasm ABI change, no CDT change, no contract rebuild, no protocol feature, no reference-data regen. The pinned signatures in the registry mirror the new shape so a future accidental revert to legacy_span would fire the canary.
Background ---------- After Tier 1 removed the argument_proxy wrapper for byte-buffer intrinsics (legacy_span<[const] char> -> span<[const] char>), the remaining legacy_span / legacy_ptr uses are all cases where the argument_proxy IS doing real work: alignment normalization between wasm (typically 4-byte aligned pointers for aligned 8/16-byte types) and host (alignof(__int128)=16, alignof(fc::sha256)=8, etc.). Those proxies stay; the name was always inaccurate. "legacy" suggests deprecated / back-compat-only, but these are load-bearing alignment machinery on a pre-launch chain that has never had a "non-legacy" era to be legacy relative to. Rename to "aligned" -- honest about what it does. Scope ----- Pure source rename across the chain library + tests. Zero wasm-ABI change, zero CDT impact, zero contract rebuild, zero reference-data regen, zero protocol feature. Identifiers renamed (word-boundary replacements): - legacy_ptr -> aligned_ptr (46 uses -- host intrinsic params for fc::sha256 / fc::sha1 / fc::sha512 / fc::ripemd160 / const digest_type / int64_t[,8] / uint32_t / __int128 / uint128_t / float128_t) - legacy_span -> aligned_span (1 use -- get_active_producers with account_name; Tier 2 reshape if ever done would replace this) - is_legacy_ptr -> is_aligned_ptr (preconditions helper) - is_legacy_span -> is_aligned_span (preconditions helper) - is_whitelisted_legacy_type -> is_whitelisted_aligned_type (preconditions) - is_whitelisted_legacy_type_v -> is_whitelisted_aligned_type_v (inline alias) - are_whitelisted_legacy_types_v -> are_whitelisted_aligned_types_v - legacy_static_check_wl_args -> aligned_static_check_wl_args (sys-vm precondition) - REGISTER_LEGACY_HOST_FUNCTION -> REGISTER_ALIGNED_HOST_FUNCTION (79 macro invocations) - REGISTER_LEGACY_CF_HOST_FUNCTION -> REGISTER_ALIGNED_CF_HOST_FUNCTION - REGISTER_LEGACY_CF_ONLY_HOST_FUNCTION -> REGISTER_ALIGNED_CF_ONLY_HOST_FUNCTION null_terminated_ptr is kept (accurate name; not "legacy"). argument_proxy<T*, Align> in the sysio/vm library is untouched -- it's the underlying machinery, lives in vcpkg, and the rename is above it.
…re variants Background ---------- The probe-PR commit message (cc6a62f) flagged a documented API inconsistency in recover_key: for fixed-size key types (k1/r1/em) a pub buffer smaller than the required 34 bytes FC_ASSERT'd through fc::datastream, while variable-size keys (wa/ed) silently truncated via memcpy and returned the full required size. Same intrinsic, two contracts depending on which variant the caller happened to encounter. The mixed behavior is fixable host-side without any wasm ABI or CDT change, so landing it here as part of the same cleanup branch. Unified contract ---------------- recover_key now obeys one rule regardless of signature variant: - Always returns the full packed-pubkey size (so callers can use "query with buffer=0, allocate, call again" or CDT's "pre-allocate optimistic, check return, retry if larger"). - When the caller's pub buffer fits, packs in place via fc::datastream -- unchanged fast path, no heap allocation. This is the overwhelmingly common case (CDT's recover_key wrapper pre-allocates 256 bytes optimistically; K1 34-byte keys always fit). - When the caller's pub buffer does NOT fit AND has non-zero size, allocates a temporary via fc::raw::pack and memcpy's min(pub.size, needed) bytes into the caller's window -- the partial-write behavior the wa/ed path already shipped with. Heap allocation is paid only on this adversarial / under-sized-buffer path. - When the caller passes a zero-size pub buffer (size-query pattern), returns required size without touching the buffer (no heap allocation). - Never throws on a small buffer. Previous k1/r1/em throw path is gone. Tests ----- - unittests/test-contracts/intrinsic_probe/intrinsic_probe.cpp recsmpub: rewritten. Previously pinned the throw behavior; now pins the unified truncate+return-required-size contract. A canary region around the 4-byte pub window catches any host write outside the window, and an explicit byte-zero check catches a future regression back to "no write on small buffer" (the K1 variant tag is index 0, so small_pub[0] becomes 0 on partial write -- distinguishable from the 0xaa canary fill). - unittests/intrinsic_probe_tests.cpp recover_key_small_pub: updated from BOOST_CHECK_THROW(fc::exception) to BOOST_CHECK_NO_THROW. The probe's own sysio_assert_message_exception path surfaces host misbehavior as specific test failures rather than a generic throw. - api_part2_tests/crypto_tests/test_recover_key_partial (existing test exercising wa mock-webauthn half-size buffer): unchanged behavior -- was already on the truncate path, still passes. - intrinsic_probe_tests (97 cases) green across sys-vm, sys-vm-jit, sys-vm-oc. - crypto_golden_tests, signature_recovery_tests: green. Scope / risk ------------ Host-only change. Zero wasm ABI change, zero CDT change, zero contract rebuild, no reference-data regen, no protocol feature. Behavior change is observable only in the pathological caller-passes-undersized-buffer case, which no in-tree test relied on for k1/r1/em (all existing callers pass correctly-sized buffers via CDT wrappers). wa/ed callers see identical behavior.
…st coverage Background ---------- Before this commit recover_key threw on six distinct failure modes. Five of those are contract-observable input issues (empty / truncated / bad-variant-tag / bad-recovery-byte / off-curve r,s), and converting them to rc = -1 lets a contract accept untrusted signature data without the transaction aborting. The sixth case -- the speculative-block variable-size subjective guard against WebAuthn DoS -- must keep throwing, for reasons documented inline. This gives a clean split across the two sig-recovery intrinsics: - recover_key : returns rc on every contract-observable outcome, including failure (rc = -1); throws only on the one subjective-resource path - assert_recover_key: throws on any failure (contract chose the strict path) Which was the original design intent behind the two-function split; it just wasn't wired that way for recover_key. Host change ----------- libraries/chain/webassembly/crypto.cpp::recover_key: - fc::raw::unpack(sig) wrapped in try/catch -> rc = -1 on empty / truncated / bad-variant-tag input. - SYS_ASSERT for unactivated / unknown variant replaced with a plain if-return -> rc = -1. - fc::crypto::public_key::recover(s, *digest) wrapped in try/catch -> rc = -1 on bad recovery byte, off-curve r/s, or ed25519 verify failure. - Speculative-block variable-size SYS_ASSERT KEPT, with a DEFERRED comment explaining why: the cap is a per-node-configurable subjective value, and converting the throw to a return would expose that subjective value to the contract and enable a consensus divergence whenever two producers run with different configured limits. The comment records both paths to eventually removing the throw (promote to objective hardcoded cap, OR promote to chain_config for governance tunability) and notes that the original design was deliberately subjective per the webauthn-support PR dabc794 (May 2019), still the shape on AntelopeIO/spring main today. assert_recover_key is untouched -- it keeps its "throw on everything" contract. Test coverage ------------- Complete matrix for recover_key (99 probe cases total on this branch, up from 97): recok golden / valid sig / sufficient pub buffer no throw, rc == pk_len, pub populated recsmpub small pub buffer (partial-write + canary + rc == pk_len) no throw recqsize zero-size pub buffer (size-query pattern) NEW no throw, rc == pk_len, no write recuald unaligned digest pointer (copy-in path) no throw, rc == pk_len recempsig empty sig no throw, rc == -1 (was throw) recshort 1-byte sig no throw, rc == -1 (was throw) recbadvar variant tag = 0x7F no throw, rc == -1 (was throw) recbadrec K1 recovery byte out of [27, 35) no throw, rc == -1 (was throw) recbadrs 1-bit flip in r -- math may succeed with different pub no throw, rc == -1 OR rc == pk_len (!= original) OR math may reject the curve point recbigwa WebAuthn sig with auth_data + client_json = 17 KiB NEW throws sig_variable_size_limit_exception The two new probes (recqsize, recbigwa) fill the only coverage gaps left: the size-query contract and the one remaining throw. recbigwa's WA-sig blob is fabricated in the driver -- the bytes don't need to be a cryptographically valid WA sig, they just need to unpack successfully so variable_size() exceeds the cap and the speculative-size guard fires before any recovery math. Driver-side test cases updated in lockstep -- the five BOOST_CHECK_THROW(fc::exception) cases for recover_key become BOOST_CHECK_NO_THROW. The probe bodies assert rc == -1 in-contract, so any regression back to throwing surfaces as a driver failure and a regression to "silently returns positive rc for bad input" surfaces as a probe-side sysio_assert_message_exception with a specific diagnostic string. The recover_key_wa_oversized_variable_size case pins the one remaining throw via BOOST_CHECK_THROW(sig_variable_size_limit_exception). Probe contract adds a file-scope 32 KiB static big_wa_buf because CDT wasm stack is not configured for per-frame allocations that large.
The hardened form rewrote arithmetic right shift via unsigned operations plus a sign-extension mask to avoid "implementation-defined behavior of signed >> on negative values". That was true pre-C++20 only; C++20 [expr.shift]/3 defines signed `>>` as arithmetic shift (rounded toward negative infinity), well-defined for both signs. Only shift counts >= the type width remain UB. Delegate to the language guarantee and saturate the out-of-range case to the sign bit (0 for non-negative, -1 for negative). Bit-identical output to the prior form for every input.
81beb12 to
84273f3
Compare
249ef52 to
c85e1a8
Compare
…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.
huangminghuang
approved these changes
May 16, 2026
heifner
added a commit
that referenced
this pull request
May 16, 2026
…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).
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
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
legacy_span/legacy_ptr->spancleanup, a small-buffer-contract normalization forrecover_key, and a never-throw rewrite ofrecover_keyfor contract-observable failures. All commits on this branch are wasm-ABI-stable: no wire-cdt change.Stacked on #310 (feature/intrinsic-adversarial-probes) -- which provides the test-only probe infrastructure these commits rely on for regression coverage. Merge #310 first, then this PR rebases automatically onto
feature/kv-iterator-pool-split.Commits (five on this branch, top to bottom of the diff)
ca77414424-- 110-entry host-intrinsic signature registry + OC intrinsic-table compile-time canary (port of possible to register a host function and neglect to map it to an OC call index w/o compile error AntelopeIO/leap#621, c++23 unblocks it). Pure-additive. A singlestatic_assertfires if any future cleanup accidentally flips a signature; the OC canary fires if anyone adds aREGISTER_*_HOST_FUNCTIONwithout the matching"env.<name>"entry inget_intrinsic_table(). Verified by temporarily mutating both call sites during authorship.4131bb0024-- 41 byte-buffer intrinsics flip fromlegacy_span<[const] char>tospan<[const] char>. argument_proxy wrapper hasAlign=alignof(char)=1, so its alignment checkptr % 1 == 0is trivially always true and the copy path is never taken -- removing the wrapper for byte spans is pure per-call overhead reduction. Matches EOSIO's original kv-intrinsics shape (#8223, 2020) and wire-cdt's existingconst char*, uint32_tdeclarations, so CDT is unaffected.4f6288c57c-- Tier 3: word-boundary rename across 18 files.legacy_ptr->aligned_ptr(46 uses),legacy_span->aligned_span(1 use, get_active_producers),REGISTER_LEGACY_*_HOST_FUNCTION->REGISTER_ALIGNED_*_HOST_FUNCTION(79 call sites + 3 macro defs), and the matching helpers inpreconditions.hpp. The alignment proxies are load-bearing for__int128/float128_t/fc::sha*wasm-to-host alignment normalization; renaming says what they actually do. Nolegacy_*tokens remain in the repo after this commit.b00728dd03--recover_keysmall-buffer contract unified across all signature variants:memcpy(min(pub.size, needed))and always return the full required size. Pre-change k1/r1/em asserted throughfc::datastreamon small buffers while wa/ed silently truncated -- a documented footgun. Fast path keeps the in-place datastream pack for the common case where the buffer fits; the heap-allocating truncate path runs only on under-sized buffers.942d812eb3--recover_keyreturns-1on every contract-observable failure (empty / truncated / bad-variant-tag / bad-recovery-byte / off-curve r,s). Contracts can now accept untrusted user-submitted signature data without the transaction aborting.assert_recover_keyis untouched -- it retains the throw-on-everything contract. Onerecover_keythrow remains: the speculative-block WebAuthn variable-size subjective DoS guard.Refs
memory/project_recover_key_objective_cap.md-- deferred follow-on for the single remainingrecover_keythrow.-