fix(kv): reject duplicate primary keys in multi_index::emplace by heifner · Pull Request #113 · Wire-Network/wire-cdt

heifner · 2026-08-31T16:09:03Z

Summary

sysio::multi_index::emplace could leave a secondary index pointing at a row that no longer matches the key, and all three mutators could be driven through a handle opened on another account. Surfaced while reviewing #111; verified against the real chain runtime before fixing.

The defect

emplace called kv_set — an upsert — then store_secondaries, an unconditional kv_idx_store, with no check that the primary key was new:

::kv_set(_table_id, payer.value, key.data, key_size, value.data(), value.size());
store_secondaries(payer.value, obj);

Emplacing over an existing key overwrote the row and added a second index mapping, leaving the previous (sec_key → pri_key) entry behind. A later get_index<>().find(old_sec) resolves through that stale entry and returns a row whose secondary value has since changed.

The host is not at fault. Both intrinsics do what they document — kv_set is an upsert (apply_context.cpp:634) and kv_idx_store is a pure insert (apply_context.cpp:985: "kv_idx_store always creates a new entry"). On Antelope the duplicate was rejected at the chain layer by db_store_i64; that guard was lost with the legacy DB and the shim never picked it up.

kv::table::emplace already pays this check. The backward-compatibility wrapper was the one missing what the perf-first wrapper already had.

The second defect: writes ignore the handle's code

Reads take a code argument and honour it; kv_set, kv_erase and kv_idx_store have none and always land on the receiver. Because table_id derives from the table name alone, mutating through a foreign-code handle probes their table and writes your own row of the same name. Upstream multi_index rejects this in the wrapper — the host cannot, since there is no code argument on the write to check. Wire had dropped all three guards.

Demonstrated, not inferred

A throwaway action emplaced pk=1, sec="aaa", emplaced pk=1, sec="bbb", then looked up "aaa". Against integration_tests on the real runtime:

# before
assertion failure with message: ORPHAN: stale secondary A still resolves after duplicate emplace

# after
assertion failure with message: object with the same primary key already exists

The fix

Four changes to libraries/sysiolib/contracts/sysio/kv_multi_index.hpp:

A duplicate-key check in emplace, matching kv::table::emplace.
A receiver guard on emplace, modify and erase, with upstream's exact messages (a ported contract may assert on them). Three sites cover all seven entry points, since the iterator/index-level forms delegate.
receiving_account() — reads the sysio_contract_name global, which the generated dispatcher sets to the receiver as the first statement of apply(), and falls back to the current_receiver intrinsic where it is 0. SYSIO_DISPATCH and the native dispatch never set it; no path sets it to anything but the receiver, so 0 is a safe "unset" sentinel. Not cached: a contract may hold a static table and the receiver differs under notification.
to_pk_uint64 at the three secondary sites (store_secondaries, remove_secondaries, update_secondaries). Independent of the above: pk_to_bytes takes uint64_t, so a name primary key combined with a secondary index did not compile at all.

`name` at the primary bounds

lower_bound/upper_bound gain a one-line name overload delegating to the uint64_t one — the shape find, require_find and get have used in this class all along.

This took three attempts and the first two are worth recording, since both are visible in the review history. A member template could not deduce lower_bound({42}). A converting-proxy parameter restored the braced form but could not reproduce a uint64_t parameter's conversion semantics — most concretely, it silently accepted lower_bound({w}) for a w converting to a narrower type, which a real uint64_t parameter rejects as narrowing. Plain overloads have neither problem, because the uint64_t parameter is still a uint64_t parameter.

Two source breaks, both inherited from the sibling shape rather than novel, both documented at the declaration and pinned by test:

&table::lower_bound is now an overload set, so the bare address cannot be taken — as has always been true of &table::find, &table::get and &table::require_find. A named static_cast<const_iterator (table::*)(uint64_t) const>(...) still resolves either overload.
a wrapper convertible to both uint64_t and name is now ambiguous, where against the single uint64_t parameter it selected the uint64_t conversion. find/get/require_find have always been ambiguous for such a type.

Behaviour change

This is a behaviour change for ported third-party multi_index consumers, deliberately:

emplacing an existing primary key now aborts instead of silently upserting;
emplace/modify/erase through a handle whose code is not the receiver now abort instead of misdirecting the write to the caller's own table.

Both restore upstream multi_index behaviour.

This reaches sysio::singleton too. singleton.hpp aliases it to kv_singleton, which owns a kv_multi_index<Name, row> as its storage member rather than being one, so get_or_create, set and remove all reach the guarded entry points through it — as does cached_kv_singleton over one. A singleton handle constructed on another account's code is now read-only, same as a table handle.

Tests

tests/unit/kv_multi_index_tests.cpp (new, native, runs in the always-on unit_tests label — registered in both tests/unit/CMakeLists.txt and tests/CMakeLists.txt). Native rather than integration-only because ENABLE_INTEGRATION_TESTS defaults OFF and CI does not enable it, so an integration-only regression leaves required CI green when a guard is deleted.

Five cases:

Case	Pins
`duplicate_primary_key_rejected`	a duplicate `emplace` aborts
`foreign_code_handle_cannot_mutate`	all three guards fire for a foreign-code handle and the receiver's row is untouched
`own_table_handle_passes_the_guard`	an own-table handle emplacing an absent key succeeds and the row lands
`own_table_handle_can_modify_and_erase`	`modify` and `erase` also succeed on an owned handle — without it an inverted guard, or one that refuses everything, still passes the suite
`primary_bounds_accept_uint64_and_name`	the bounds accept `uint64_t`, `name`, `{42}`, `{}` and a uint64-convertible wrapper, with the dual-convertible ambiguity pinned

The four receiver-dependent cases each run twice — once with sysio_contract_name set and once with it 0 — and when it is set the mocked current_receiver returns a different account, so the two branches of receiving_account() cannot be confused for one another. primary_bounds_accept_uint64_and_name runs once: it is an overload-resolution case that never reaches a guard.

Verified by mutation, not by inspection

Header variant	duplicate	foreign_code	own_table	bounds
unmodified	pass	pass	pass	pass
duplicate-key check deleted	fail	pass	pass	pass
three receiver guards deleted	pass	fail	pass	pass
dispatcher-global fast path deleted	fail	pass	fail	pass

Reverting the to_pk_uint64 calls fails to compile the contract test with no viable conversion from 'sysio::name' to 'uint64_t' at all three sites. Removing the name overloads fails the native test on the static_cast and the contract test on the call.

ctest 31/31 (30 plus the new dispatch_receiver_tests); dispatch_receiver_tests.sh 36/36; multi_index integration suite 24/24 assertions.

The gap this closed. Nothing pinned the fact receiving_account() rests on: changing the generated dispatcher from sysio_set_contract_name(r) to (c) left the entire suite green, because every in-tree action is self-sent so r == c, and the native test drives the global directly. The divergence would appear only under notification, on chain. tests/unit/dispatch_receiver_tests.sh pins it at the source, which no other test looks at.

It grew two independent checkers over the review, because neither subsumes the other:

Checker	Reads	Sees what the other cannot
`check_dispatch`	the preprocessed dispatch text	that the argument is `r` and not `c`, and that the call is the first statement
`check_dispatch_symbols`	the relocations of the emitted object	a second call however it was spelled — through an asm label whose name is split across string literals, or through a function pointer — and it refuses an indirect call outright, since a `call_indirect` names a type and not a target

Both report three outcomes, not two — accepted, rejected, and INFRA_ERROR, the check could not be performed — and every caller distinguishes all three. Folding the third into a rejection is what makes a machine with a broken cdt-cpp or llvm-objdump sweep the counterexample table green, so each analyser has a stand-in that prints plausible output and then fails, pinning both halves.

Each checker is exercised three ways: the real generated dispatch, a table of counterexamples that must each be rejected, and positive controls that must not be. Every counterexample defeated some earlier revision in review — handler names the checker did not match, a branch on the signature line, two calls on one line, a comment between the identifier and its paren, a raw string closing at column 1, a line marker whose filename carried an escaped quote, an asm label, and that same alias called through a function pointer. Each part of the marker pattern (^, the filename grammar, the trailing flags, $) has a row that fails when it alone is weakened, and each symbol-level check has exactly one discriminating row. 36 assertions.

Downstream

Correcting an earlier version of this section, which claimed no first-party contract instantiates kv_multi_index. That was wrong — it grepped the literal spelling, missing that sysio::multi_index is kv_multi_index (multi_index.hpp:15 aliases it) and that sysio::singleton reaches one through kv_singleton, which holds it as a member (kv_singleton.hpp:26) rather than deriving from it. wire-sysio origin/master has 13 test contracts under unittests/test-contracts/ and one under unittests/system-test-contracts/ that instantiate it. The system contracts under contracts/ genuinely do not — the single grep hit there is inside a /// comment.

So the guards were checked by hand rather than skipped: all of them compile clean against this CDT, and every mutating handle is constructed with get_self(), which equals the receiver even under notification. Note that wire-sysio CI would not have caught a break either way — its workflow sets SYSIO_BUILD_TEST_CONTRACTS: "OFF" and the .wasm files are committed, so those contracts are not rebuilt on a normal PR.

The downstream build also validates that making kv_table::do_insert private breaks nothing (do_insert has zero references outside kv_table.hpp).

Also in this PR

kv_table::do_insert made private — inserting over an existing key there strands a mapping the same way. Its comment is explicit that this seals do_insert only: store_secondaries and its siblings remain public and reach the same state.
A note on kv_table::emplace that writes ignore code(), so a foreign-code handle is read-only in practice.
core/sysio/context.hpp declared sysio_contract_name without the volatile its definition in sysiolib.cpp carries — ill-formed NDR, latent because extern "C" names carry no type and no TU saw both. This PR is the header's first consumer.
.gitignore: core-dump patterns, root-anchored so a tracked core.hpp is not shadowed (verified against boost/hana/core.hpp and boost/move/core.hpp); cmake-build-debug/ widened to cmake-build-*/; and .prequel/, a local review tool's state directory. The last two are unrelated to this fix — they land here only because the same hunk was duplicated across all three open PRs and conflicted pairwise, so it was consolidated onto this one.

emplace could strand a secondary-index entry. It called kv_set -- an upsert -- and then store_secondaries, an unconditional kv_idx_store, without checking that the primary key was new. Emplacing over an existing key silently overwrote the row and left the previous (sec_key -> pri_key) mapping in place, pointing at a row whose secondary value had since changed, so a later get_index<>().find(old_sec) resolved to a row that did not match the key. The host is not at fault. kv_set is a documented upsert and kv_idx_store a documented insert; on Antelope the duplicate was rejected at the chain layer by db_store_i64, and that guard was lost when the legacy DB was removed. The shim never picked it up. kv::table::emplace already does exactly this check, so the backward-compatibility wrapper was the one missing what the perf-first wrapper already pays for. Demonstrated before fixing, against the real runtime: a throwaway action emplaced pk=1 with secondary "aaa", emplaced pk=1 again with "bbb", then looked up "aaa". Pre-fix it resolved -- the orphan. Post-fix the second emplace aborts and the orphan is unreachable. That probe is now the permanent regression test, asserting the abort. Also templates lower_bound/upper_bound on the primary key type, matching upstream multi_index, which routes through to_raw_key. Taking a bare uint64_t rejected `name` primary keys that compile upstream. Making that work exposed three more sites passing primary_key() straight into pk_to_bytes; they now use the same to_pk_uint64 conversion as every other call site. A name-primary-key test covers both bound forms and pins that the uint64_t form still binds. kv_table::do_insert becomes private. It is explicitly the unchecked path and was public only because of where the access block fell; it has one caller, in the same class, and nothing downstream references it. Deliberately no emplace_unchecked: the host indexes kv_index_object ordered_unique on (code, table_id, sec_key, pri_key), so skipping the check either strands a mapping or trips that constraint -- the hazard being removed here. Also broadens the CLion build-dir ignore to cmake-build-*/ and ignores prequel's local review state. 29/29 ctest including toolchain and integration suites.

Twelve inline findings plus four from the review body, each verified against the code first. Factual corrections. The addpolicy field-name note claimed a camelCase/snake_case divergence between the C++ action and its ABI -- there is none; I had read wire-system-contracts' copy of sysio.roa.hpp, abandoned on a 2025 branch, where the authoritative wire-sysio/contracts copy is snake_case on both sides. The note is gone rather than reworded. `-wasm2wast` becomes `eosio-wasm2wast`; `cdt-init -bare` emits four files, not two, since write_ricardian runs unconditionally; the clean-machine prerequisites now install build-essential and jq, which the guide's own `make` and `jq` commands needed; and `network_gen` is a placeholder with instructions for finding the issuer's generation, because addpolicy scopes nodeowners to the value passed and hard-coding 0 breaks after a rollover. The `_n` fallback for short `_i` names was itself impossible. `_n`'s alphabet is `.12345a-z`, so the guide's own `user_table` example is a compile error. Only short names that are already valid Antelope names can switch; others must be renamed or lengthened until abigen is fixed. Over-absolute claims scoped. An unprovisioned contract blocks ordinary contract-paid calls, not every call -- with the added caveat that the sysio.payer escape hatch covers bandwidth only, so RAM the contract bills itself still needs headroom. Separating user-paid RAM from contract-paid bandwidth is possible via a persistent `<contract>@sysio.code` delegation and an inline action, so that claim is now scoped to the direct top-level call shown. Subjective billing meters each top-level action's first authorizer, so several accounts can be throttled, not one signer. Billing mechanics. The NET overhead split is `overhead / actions + 1` -- integer division then an unconditional +1, which over-bills by up to a byte per action where the division is exact, rather than being a ceiling. And only CPU covers the whole call tree; NET is fixed by what reaches the wire, so the conclusion is split. Compatibility framing. multi_index is a compatibility shim, not a drop-in, here and in kv-storage-guide.md; the postfix iterator rewrite is the source change, and the duplicate-emplace and templated-bounds semantics are called out as matching upstream. The checklist no longer says direct db_* callers have nothing to do. README now says the contract is billed by default and that RAM follows the contract's payer argument. The ROA overview is referenced through wire-sysio#583 rather than a master URL that does not resolve yet. Depends on #113 for the emplace and lower_bound/upper_bound semantics this describes.

Reads honour the handle's code -- kv_get and kv_contains take a code argument -- but writes do not: kv_set, kv_erase and kv_idx_store have no such parameter and always land on the receiver. So the duplicate-key probe added in this PR could consult one account while the write landed on another, and a handle opened on a foreign account would pass the check and then upsert the receiver's row, leaving its old secondary mapping stranded. That is the corruption this PR set out to remove, reached a different way. The host cannot catch it. Writes take no code, so a contract can never reach another account's namespace and there is nothing for the host to reject; it sees two well-formed calls. table_id derives from the table NAME alone and the key is [scope][pk] with no account component, so the misdirected write lands on the receiver's own row of the same table name. Upstream guards this in the wrapper, and Wire had dropped all three checks. Restored with upstream's messages, since a ported contract may assert on them, at the three points the mutators funnel through -- emplace, modify(const T&) and erase(const T&) -- which covers the iterator overloads and the index-level modify/erase that delegate to them. emplace checks before running the constructor lambda, as upstream does. receiving_account() avoids the host call where it can. The generated dispatcher records the receiver in sysio_contract_name at the top of apply(), so that path is a plain global read; SYSIO_DISPATCH emits its own strong apply() and the native dispatch sets nothing, leaving it 0 -- not a valid account name, so a safe sentinel -- and there we pay the current_receiver intrinsic, as upstream always does. Not cached on the object: a contract may hold a static table, and the receiver differs between an action and a notification handler. The guards are deliberately NOT extended to kv::table, kv::scoped_table or kv::global. Those are new APIs with no upstream behaviour to honour, and kv::global already documents foreign-code handles as read-only by contract. kv::table carried the same hazard undocumented, so it gains the equivalent note. Primary lower_bound/upper_bound go back to concrete overloads on uint64_t plus a name forwarder. Templating them was not source-widening as this PR claimed: lower_bound({42}) cannot deduce from a braced list and &table_type::lower_bound cannot form a pointer to an undeduced template. Two overloads reach exactly what to_pk_uint64 accepts, without the break. New native kv_multi_index_tests covers all of it. The on-chain cases needed ENABLE_INTEGRATION_TESTS, which defaults OFF and is not enabled in CI, so the guard could have been deleted with required CI green. Registered in both tests/unit/CMakeLists.txt and tests/CMakeLists.txt -- missing the second builds the test but never runs it. Each guard case runs twice, once with the dispatcher global set and once with it 0, so both branches of receiving_account() are exercised rather than only the native fallback. Verified by removing the emplace guard: the native test fails with "expect_assert, no assert" on both branches, so it genuinely gates. 30/30 ctest; wire-sysio's contracts rebuild against this CDT with 187 of its own test cases green.

…rameter The two-overload form from the previous round fixed the braced-initializer break but introduced a narrower one: `auto lower = &table_t::lower_bound;` compiled against the base revision and fails against an overload set with `<overloaded function type>`. Three call shapes pull in different directions -- a member template breaks both `lower_bound({42})` and the bare member-pointer, two overloads fix the first and still break the second. A single non-template function taking an implicitly-constructible primary_key_arg satisfies all three at once, and reaches exactly the types to_pk_uint64 accepts. Callers never name the type; they pass a uint64_t or a name as before. The test was masking this rather than catching it. Its static_cast selected the uint64_t overload from the set, so it passed even while the bare form did not compile -- the same shape of vacuous assertion as the earlier "name": "hiproto" check, in a new disguise. It now takes the address with no cast, and asserts all three call shapes. Confirmed to have teeth: reverting to two overloads fails the build with exactly the reported `<overloaded function type>` error on those lines. 30/30 ctest; wire-sysio's contracts rebuild against this CDT.

The proxy fixed bare address-taking but narrowed what the bounds accept. With a fixed uint64_t parameter, a caller's key wrapper with operator uint64_t() needs wrapper -> uint64_t -> primary_key_arg, two user-defined conversions, so it is rejected -- while find, get and require_find take uint64_t directly and accept the same type today, and upstream's templated bound accepts it through to_raw_key. The bounds were the only part of the API refusing it. `{}` regressed too: it meant key zero against a plain uint64_t parameter and the proxy had no default constructor. The converting constructor is now a constrained template taking PK by value, so a uint64-convertible type costs one user-defined conversion rather than two, and the proxy is default-constructible at zero. The constraint keeps it from swallowing `name`, which has no implicit uint64_t conversion and so still selects its own overload, and leaves copy construction alone. PK rather than T because T is the row type of the enclosing kv_multi_index. Test gains the two shapes that regressed -- an empty brace and a wrapped key -- alongside the four already covered, plus static asserts that the conversions produce the right value. Confirmed to have teeth: with the narrow proxy restored the build fails on `no matching constructor` for {} and `no viable conversion from wrapped_key`. Also ignores core.* / vgcore.*, which a deliberate-crash test run leaves behind. 30/30 ctest.

…xactly The constrained constructor accepted the right set of types but did not convert them the way a plain uint64_t parameter would, in three separate ways. Implicit, not static_cast. A wrapper offering an implicit operator unsigned() returning 1 and an explicit operator uint64_t() returning 2 converts to 1 through a uint64_t parameter; static_cast preferred the exact explicit conversion and stored 2, so lower_bound could seek a different row than find. The argument is now passed to a uint64_t parameter -- copy-initialisation, which considers only implicit conversions -- rather than cast. Note a member initialiser could not do this: `value(x)` is direct-initialisation and would consider the explicit operator too, which is what the first attempt at this fix got wrong. Forwarded, not copied. Taking PK by value copied lvalues, rejecting the noncopyable wrappers the base accepted, and tested a different value category in the constraint than the body then used -- an &&-only conversion passed SFINAE and failed in the body. It now takes PK&& and forwards. Narrowing preserved. Letting the template consume arithmetic and enum arguments bypassed list-initialisation narrowing: the base rejects lower_bound({-1}) and ({1.5}), this accepted and silently converted them. Those types are excluded from the template and reach the uint64_t constructor, where the narrowing rules apply. Tests gain the exact cases: an implicit/explicit dual-conversion wrapper asserting the implicit result, a noncopyable wrapper passed as an lvalue, and a detection trait proving brace-initialisation still rejects a non-constant int and a double while accepting uint64_t. Confirmed to have teeth -- restoring the static_cast and by-value form fails the dual-conversion assertion. Core-dump ignore patterns narrowed to the shapes the kernel actually writes (core_pattern is core.%e.%p) and root-anchored, so core.cpp, core.hpp and tracked headers such as boost/hana/core.hpp stay visible. 30/30 ctest.

huangminghuang

Re-reviewed the current head. The latest patch fixes the earlier forwarding and implicit-vs-explicit conversion bugs, and required CI is green, but two conversion-domain edges and one regression-test gap remain below. Also, the PR description still embeds the previous by-value/static_cast constructor and says taking the wrapper by value; the header prose at lines 627 and 653 is stale in the same way. Please update those descriptions to match the PK&&/as_key implementation and document any accepted residual compatibility trade-off.

lower_bound/upper_bound go back to taking a plain uint64_t, and primary_key_arg is deleted. Templating the bounds so a `name` primary key could be passed directly was never needed by the duplicate-key and receiver guards this PR exists for -- it was an opportunistic convenience. It broke source compatibility for `lower_bound({42})` and `&table_type::lower_bound`, and the proxy type introduced to restore those could not reproduce a `uint64_t` parameter's conversion semantics exactly: copy- versus direct-initialisation, value category, and list-init narrowing each pulled in a different direction, and closing one gap reopened another. Callers with a `name` primary key pass `.value`, exactly as they did before. Kept: the to_pk_uint64 calls in store/remove/update_secondaries, which are an independent fix -- pk_to_bytes takes uint64_t, so a `name` primary key combined with a secondary index did not compile at all. The contract-side test is retargeted accordingly: name_pk_bounds becomes name_pk_secondaries and now exercises the path to_pk_uint64 actually fixed (secondary lookup, modify rewriting the mapping, erase removing it) rather than the bounds' argument types.

lower_bound and upper_bound gain a one-line `name` overload delegating to the uint64_t one -- the exact shape find, require_find and get have used in this class all along (kv_multi_index.hpp:585-605). This is the third attempt at `name` bounds and the first that changes nothing else. A member template could not deduce `lower_bound({42})`. The primary_key_arg proxy that replaced it restored the braced form but could not reproduce a uint64_t parameter's conversion semantics: it silently accepted `lower_bound({w})` for a `w` converting to a narrower type, which a real uint64_t parameter rejects as narrowing. Plain overloads keep every conversion the base performed, because the uint64_t parameter is still a uint64_t parameter. `name`'s uint64_t constructor is explicit, so `name` is never a viable candidate for a braced integer and the braced forms stay unambiguous. The one behaviour change is that `&table::lower_bound` is now an overload set, so the bare address cannot be taken. That has always been true of `&table::find`, `&table::get` and `&table::require_find` for the same reason; the bounds were the only primary accessors where it worked. A named static_cast still resolves either overload, and the test uses that form. Verified: the new assertions fail without the overloads -- a build against a header carrying only the uint64_t form fails on the static_cast with "to 'itr_t (table_t::*)(name) const' is not allowed" and on the contract-side call with "no viable conversion from 'sysio::name' to 'uint64_t'". ctest 30/30 and the multi_index integration suite 24/24 pass with them.

Three gaps found in pre-push review, all in the coverage this PR added. arrange() pointed both the sysio_contract_name global AND the mocked current_receiver at the same account, so the two branches of receiving_account() could not be told apart: deleting the global fast path outright left the whole suite green. The mock now returns a decoy account whenever the global is set, so a guard that consulted the intrinsic instead of the global reaches the wrong answer and the case fails. own_table_handle_passes_the_guard was a byte-identical copy of duplicate_primary_key_rejected -- its comment said pk=2 but the lambda wrote id=1 and it asserted the duplicate message. Nothing in the suite required a mutation to SUCCEED, so a guard that rejected everything satisfied every case. It now emplaces an absent key and asserts the row lands. That needs the iterator read path, since emplace returns find(pk); kv_it_key/kv_it_value are served from the mock store rather than stubbed, so the returned iterator is genuinely valid. The bounds doc claimed "two overloads keep every conversion the base performed, unchanged". False: a wrapper convertible to both uint64_t and name is now ambiguous, where against the single uint64_t parameter it chose the uint64_t conversion. find/get/require_find have always been ambiguous for such a type, so the overloads are consistent with their siblings rather than novel -- but it is a source break, so the claim is narrowed to name both costs and a dual_key case pins it. Verified by mutation. Deleting the duplicate check fails only duplicate_primary_key_rejected; deleting the three receiver guards fails only foreign_code_handle_cannot_mutate; deleting the dispatcher-global fast path fails duplicate_primary_key_rejected and own_table_handle_passes_the_guard. ctest 30/30, multi_index integration 24/24. Also from the same review: - context.hpp declared sysio_contract_name without the volatile its definition in sysiolib.cpp carries. Differing cv-qualification on one entity is ill-formed NDR; it linked only because extern "C" names carry no type and no TU saw both. This PR is the header's first consumer. - kv_table's do_insert comment claimed "no supported way to skip it", but store_secondaries, remove_secondaries, update_secondaries and do_erase are all still public and reach the same stranded mapping. Says what is true.

Pre-push review found the foreign-code case could not detect the corruption its own comment described. It asserted `rows.count({alice, tid, pk_key(alice,1)}) == 1`, but a misdirected emplace OVERWRITES the row under that same key, so the count is 1 either way. The property was carried entirely by the `sets == 0` line above it. Now compares the stored value: with the emplace guard deleted and the sets assertion removed, the case fails with `store().rows.at(seeded) != std::string("row")`. The positive case gets the same treatment -- the row it writes must be non-empty and must not be the seeded placeholder, so a write that landed with the wrong key or wrong contents is not read as success. Also documents why emplace's closing find() returns end() on the global-path iteration: the write lands under the decoy receiver while the handle's code is alice, so kv_contains short-circuits. That asymmetry is the point of the decoy, but the previous comment implied both iterations behaved alike. The review reported kv_it_value as dead code; it is not. Removing it aborts three of the four cases with "unsupported intrinsic" -- emplace's closing find() builds an iterator whose load_current() reads the key and then the value. Kept, with a comment saying what reaches it. ctest 30/30, multi_index integration 24/24.

The same overstatement the docs already dropped. Names the three real divergences instead: deleted postfix iterator operators, uint64_t/name bound overloads rather than upstream's member template, and the trivially-copyable secondary-key constraint.

Final review round found that nothing in the tree pins the one fact receiving_account() depends on. Changing cdt-codegen.cpp:83 from sysio_set_contract_name(r) to (c) leaves ctest 30/30 and the integration suite 24/24 green: every in-tree action is self-sent, so r == c, and the native test drives the global directly rather than through apply(). The divergence appears only under notification, on chain -- and the one downstream contract that would catch it, wire-sysio's ram_restrictions_test, is not rebuilt by that repo's CI (SYSIO_BUILD_TEST_CONTRACTS: "OFF"). dispatch_receiver_tests.sh inspects the emitted dispatch text, which no other test looks at: it asserts the call passes `r`, and that it precedes any action dispatch. Registered under unit_tests so it runs in required CI. Verified to gate: with the argument changed to `c` it fails and everything else still passes. Also from the same review: - The header comment added in the previous commit cited docs/kv-multi-index.md for the divergences, but this PR touches no docs -- those edits are on the #111 branch, and on THIS branch that file still calls the shim a drop-in replacement. The divergences are listed inline instead, and the comment now records that sysio::multi_index and sysio::singleton are both aliases of this template, so the new guards reach the singleton surface too. - The foreign-code case's `rows.count(seeded) == 1` cannot fail: the mock installs no kv_erase and kv_set only assigns, so nothing can reduce the count. The value comparison beside it is what carries the property; the comment said otherwise and now says which is which.

Seventh review round. Three matcher defects survived the earlier rounds -- each is the same shape as one already fixed in the same file, which is why they kept being missed. cdt-abidiff: - find_structs kept a success flag across its field loop and broke out of that loop on a mismatch without clearing it, so only a difference in the FIRST field was ever reported. It also seeded the flag false and set it only inside the loop, so two byte-identical zero-field structs -- which every parameterless action generates -- compared as different, making the tool emit false positives on essentially every real contract. - find_tables compared only name and type. index_type, key_names, key_types and table_id could all change and it reported nothing. That is the metadata a contract upgrade turns on, and table_id is where the row physically lives. This also corrects my own diagnosis in an earlier thread: I said the version parse was why abidiff missed table-metadata changes. It was not -- the matcher never compared those fields. - find_variants now shares one arrays_equal helper with the other two rather than open-coding its own list comparison. ABIMerger: - variant_is_same asked only whether every type in one variant appeared somewhere in the other, with no length check, so ["uint64"] and ["uint64","string"] compared equal. Depending on sorted .desc filename order, merging them either dropped the `string` alternative silently or failed the build with "v already defined". Both orders now conflict. - struct_is_same matched fields by set membership plus size, so the same struct declared with reordered fields merged as identical and the alphabetically-first descriptor won. ABI field order is serialization order, so that was a wire-layout change decided by a filename. - table_is_same never compared key_types at all; it now does, with the same empty-array tolerance already documented for key_names. - The section thresholds keyed off default_major, the emission default, where they mean the major of the FORMAT. Equal today; a bump would have moved every threshold silently. - variants was emitted unconditionally while action_results was gated, so a 1.0 document merged at 1.0 produced a 1.0 ABI carrying variants -- contradicting the variants_since rule declared a few lines below it. - <algorithm> and <stdexcept> were reached only through jsoncons. - Dead: ABIMerger::action_is_almost_same and abidiff::get_base_type. cdt-codegen: the protobuf branch stamped the CLI version unconditionally, downgrading a merged document whose descriptors declared something newer -- reachable through the fallback scan that picks up .desc files from earlier compiles run with a different -abi-version. Takes the newer of the two now. The assert beside it was tautological (parse() bounds the major to exactly max_supported_major) and compiled away under the default Release build. Removal: one stale sysio_wasm_import survived, set_kv_parameters_packed. Comparing all 103 CDT declarations against the chain's 116 intrinsics leaves exactly that one with no counterpart; wire-sysio mentions it only in a CHANGELOG. Same failure mode as the security-group four this PR removes -- declares cleanly, imports, fails at deploy. Staging: the invariant covered two of six trees. libc, libcxx, boost/preprocessor and bluegrass were still configure-time file(COPY), so deleting a header from the cdt-musl or cdt-libcxx submodule left the staged copy shipping forever -- the exact bug this rework exists to fix. All six are pruned and recopied together now. The consumer fencing was a hand-maintained three-name allowlist around a step that REMOVE_RECURSEs a directory, missing sysio_malloc, sysio_dsm, sysio_cmem, c, c++, rt, sf and the native_* variants; it enumerates the directory tree instead. Packaging: the base install excluded libnative* but not libsf.a, which is also native-only, so an ENABLE_NATIVE_COMPILER=OFF package still relied entirely on the prune. Tests: 15 new assertions across abidiff_tests.sh and abi_version_tests.sh, each verified to fail against the pre-fix code. Reverting find_structs and find_tables fails 6 of them; reverting variant_is_same and struct_is_same fails 3, and reproduces the order-dependence exactly -- the variant case merged in one descriptor order and refused in the other. ctest 31/31. The .gitignore hunk is dropped from this PR: it was identical to #113's and conflicted with it. It lands once, on #113.

Third review round. Three findings were factual errors in text presented as checked, which is worse than an omission in a guide whose whole premise is that its claims were run. - The two `clio get table` invocations were CLI parse errors. `get table` declares two required positionals (account, table) with scope as `-S/--scope` (clio/main.cpp:2697-2699); both lines passed three. `roastate` is a `kv::global` and therefore unscoped, so the scope argument was wrong in concept as well. Added in round 2 to answer a review comment, never run. - The duplicate-key abort and the `name` bounds were stated as verified, but both arrive with wire-cdt#113 and are absent from any CDT built before it. The external dependency (wire-sysio#583) was already caveated; the in-repo one was not. Now says so, and records #113's receiver guards, which change behaviour for a port that opens a handle on another account. - "Before anyone can call it, a node owner must issue it a policy" is contradicted twenty lines later by the note that a `sysio.payer` caller reaches an unprovisioned contract fine. Now "before an ordinary caller". Also corrected, all verified against the tree rather than reasoned about: - `kv::global` + `_i` is broken at every name length, and the guide's advice to lengthen past 13 characters makes it worse. Below 14 the ABI carries two entries (`app_config` -> 38424 alongside a decoded-hash name `idrzzw4ktxljf` -> 21489, which is where the row actually lands); at 16 it fails to link outright with `table_id collision: 'app_config_table' and 'wdfp4hyupu.q2' both have table_id 42322`. The rule as written holds for `kv::table`. Documented with both rows rather than half-fixed; the in-tree `_i` globals are left alone because renaming them hits the second row. - The `kv::table` sample using `kv::table` included only `hash_id.hpp`, which pulls serialize.hpp and name.hpp and nothing else. - `table_id` is DJB2 over the eight big-endian bytes of the name's raw uint64, not over the string. - The byte-count row contradicted the row above it: Antelope's key is four 8-byte fields (32 B) and Wire's is a 2-byte table_id plus 16 (18 B). - `sed -i` is GNU-only and this project supports macOS. - Failed transactions are free objectively but still accrue subjective CPU against the first authorizer, which the same section says can throttle. - A KV iterator is copyable; the cost of duplicating its handle is why the postfix operators are deleted, not an inability to copy. - kv-multi-index.md called the shim a "drop-in replacement", the stronger form of the claim this PR already softened in kv-storage-guide.md. The `.gitignore` hunk is dropped from this PR. It was byte-identical to a strict subset of the one on #112 and #113 and conflicted with both; it now lands once, on #113.

huangminghuang

Reviewed exact head (569539e). The production/API changes remain sound, both previously reported mutations now fail as intended, and all exact-head CI checks are green. I’m not approving yet because the checker still trusts an incomplete source lexer and because its supposedly compiling regression fixtures are never compiled (inline).

Please also refresh the PR description: it still says there are four cases and that every case runs twice, while there are five runtime cases plus compile-time bounds coverage; it omits the owned modify/erase coverage and the checker-hardening follow-ups, and its singleton wording remains inaccurate. The full PR diff also still fails git diff --check on trailing whitespace at dispatcher-test line 50.

…xample Stop hand-rolling a C++ lexer. Four revisions of that normaliser were each defeated by a different lexical form -- a comment between the identifier and its paren, a backslash-newline splice, a backslash followed by spaces then a newline, and a raw string whose contents look like a comment (`R"d(" // )d"`, where the hand-rolled parser treats the opening quote as an ordinary string, the // as a comment, and erases the real call after it). Each fix was another narrow normalisation, which leaves the same class of false accepts. check_dispatch now preprocesses with the bundled clang++ -E -P, which implements phases 1-4 exactly as the compiler that builds the contract does. #include lines are dropped first; the dispatch defines no macros, so nothing in it depends on them, and this avoids needing the header tree resolved. Confirmed against the raw-string case: clang reports three occurrences of the identifier where the hand-rolled parser reported two. Two lexical counterexamples added for the forms that motivated this -- backslash-whitespace-newline and the raw string. Separately: the fixtures did not compile. mkbad emitted `uint64_t` with no <cstdint>, so `unknown type name 'uint64_t'` -- and nothing checked, so a malformed fixture would have counted as a successful rejection forever. The files are self-contained now, and every negative fixture and the positive control is syntax-checked before its verdict is trusted. Verified by corrupting a fixture: it now reports "counterexample compiles: missing_entirely" rather than passing. Ten counterexamples plus the positive control. Mutating the generator to emit (c) still fails end to end. ctest 31/31.

The macOS job failed all eleven compile checks with `'cstdint' file not found`. The check uses the bundled clang++, which targets WebAssembly and ships no host standard library, so `<cstdint>` resolved on Linux only by accident of the platform's include search path. The fixtures need the type, not the header: `typedef unsigned long long uint64_t;`. Both compile checks now pass -nostdinc -nostdinc++, so the requirement is enforced rather than left to luck -- passing on Linux with those flags is what demonstrates the macOS case, since they remove exactly what that platform lacks. I added the include last round in response to review, verified it on Linux, and did not consider that the compiler being used has no host headers at all. ctest 31/31; the suite passes with the fixtures compiled freestanding.

huangminghuang

Reviewed exact head (fb74254). Both previously reported issues are fixed: all ten negative fixtures compile and reject, the positive control compiles and passes, and the prior mutation checks now have teeth. The production/API diff remains sound and all exact-head CI checks are green. I’m not approving yet because preprocessing still uses different target/include semantics from the actual CDT compilation (inline).

Please also refresh the PR description as required after these follow-ups. It still describes four cases, but there are five total (four runtime cases that run in both receiver modes plus one compile-time-only bounds case); it omits the owned modify/erase case and the compiled Clang-preprocessed dispatcher matrix, and the downstream section still inaccurately says sysio::singleton itself is kv_multi_index.

The checker used the bundled clang++ with the HOST target and deleted #include lines first. Both changed the translation unit away from the one that ships: - the host target evaluates `#ifdef __wasm__` the wrong way, so a second setter call guarded on it was invisible. Reproduced: host preprocessing counts 2 occurrences and the checker returns OK, while `cdt-cpp -E` counts 3 and `cdt-cpp -c` compiles the branch that overwrites the receiver with the code; - dropping includes erases any macro that expands to a second call. Both steps go through cdt-cpp now, which supplies the wasm32 target, the CDT include graph and the same predefined macros as the real compile. -E emits line markers (the driver rejects -P), so those are dropped afterwards; no real directive survives preprocessing. The counterexamples are compiled with `cdt-cpp -c` rather than a host syntax check, so "this counterexample is legal" means legal in the translation unit that ships. Two counterexamples added for the forms that motivated this, each verified to fail the corresponding ablation: - target_conditional -- preprocessing with the host clang fails it; - macro_expanded, with the macro defined in an INCLUDED header -- stripping #include lines before preprocessing fails it. An inline #define would not have tested that, since any preprocessor expands it. Twelve counterexamples plus the positive control. Mutating the generator to emit (c) still fails end to end. ctest 31/31, integration 24/24.

huangminghuang

Re-reviewed exact head 7e5a89ca. The prior CDT-target/include mismatch is fixed: the committed suite passes 16/16 locally, both target/include ablations are discriminating, and Linux, macOS, package verification, and required checks are green.

One functional checker escape remains inline, so I am not approving this head.

Please also refresh the full PR description after the follow-up commits. Its Tests section still says four cases and that every case runs twice, but there are five cases, the bounds case is compile-time-only, and the owned modify/erase gate is omitted. The dispatcher section omits the CDT preprocessing/fixture-compilation matrix and 12 counterexamples. Downstream still says sysio::singleton “is too” kv_multi_index, although it actually aliases kv_singleton, which privately owns one (as the corrected header already explains).

Small cleanup while touching the script: CLANGXX is now unused, and the fixture typedef still refers readers to a removed -nostdinc note.

…claims The marker filter dropped every line beginning with '#', which also deletes a multiline raw string whose closing delimiter sits at column 1 after a '#', taking the code that follows it on that line. Reproduced: cdt-cpp compiles it and -E emits both calls, but the filter took the count from 3 to 2 and the checker returned OK. It matches `# <digits>` now, and raw_string_hash is a counterexample -- reverting to the broad filter fails it. The header comment called every listed item a source break against upstream. Two of them are not: - `&table::lower_bound` fails upstream too, because a member template's parameter cannot be deduced. The Wire reason differs (an overload set), the outcome does not. - a trivially-copyable secondary key is upstream's restriction as well; only where it is diagnosed differs, since the static_assert sits in secondary_index_view and fires at get_index<...>(). The actual template-shape break was missing: an explicit `t.template lower_bound<uint64_t>(k)` compiles against a member template and is rejected here with "'lower_bound' following the 'template' keyword does not refer to a template". Verified both bounds. The comment now separates breaks from shared restrictions and names the call form that stops compiling. Thirteen counterexamples plus the positive control. ctest 31/31.

huangminghuang

Re-reviewed exact head a25add17. The previous hash-prefixed raw-string case and the upstream-compatibility documentation are fixed. The committed suite passes 17/17 locally, and exact-head Linux, macOS, package verification, and required checks are green.

A neighboring valid raw-string spelling still escapes the prefix-only marker filter (inline), so I am not approving this head.

The full PR description also remains stale after these follow-ups: it still says four native cases and that every case runs twice, although there are five and the bounds case is compile-time-only; it omits the owned modify/erase gate and the current CDT-preprocessed 13-counterexample checker; it still says sysio::singleton “is too” kv_multi_index; and its bounds discussion omits the now-documented explicit-template-call incompatibility. Please refresh it to match the complete current diff.

Small cleanup while touching the script: CLANGXX remains unused, and the fixture typedef still points to a removed -nostdinc note.

Third raw-string escape past this filter. Dropping every '#'-prefixed line deleted `#)d"; ...`; dropping '#' followed by a digit deleted `#1)d"; ...`. Both took the executable code on the closing-delimiter line with them, and the checker returned OK on a dispatcher cdt-cpp compiles. Matching the complete grammar instead -- `# <line> "<file>"` with optional trailing flags, anchored at both ends -- leaves any line that is not literally a marker intact. Verified against the real emission: # 1 "/path/to/t.cpp" # 1 "<built-in>" 1 raw_string_hash_num added as a counterexample; reverting to the numeric prefix fails it, and the earlier raw_string_hash still fails the broad form. Eighteen assertions. ctest 31/31.

huangminghuang

Re-reviewed exact head 2b8dcd30. The reported numeric-prefix escape is fixed: the committed suite passes 18/18 locally, and exact-head Linux, macOS, package verification, and required checks are green.

The new implementation is correct for the reported case, but the checker/test harness still has the three actionable gaps noted inline, so I am not approving this head.

The full PR description also remains stale after the follow-up series. It still says four native cases and that every case runs twice (there are five, with the bounds case compile-time-only); omits the owned modify/erase gate and the CDT-preprocessed 14-counterexample/18-assertion checker; calls sysio::singleton itself a kv_multi_index; and omits the explicit upstream-template-call incompatibility now documented in the header. Please refresh it to match the complete current diff.

Small cleanup while touching the script: CLANGXX is unused, and the fixture typedef still points to a removed -nostdinc note.

…preprocess Three findings from review, each verified by mutation against the committed script. Line markers: the filename was modelled as `[^"]*`, which does not match what clang emits for a `#line` directive carrying an escaped quote -- `# 7 "a\"b.cpp"`. The marker survived normalisation and was then read as the start of apply()'s first statement, so a CORRECT dispatch was rejected. The filename now uses the complete grammar, `"([^"\\]|\\.)*"`, and `marker_escaped_ok` pins it as a positive control. Preprocessing failures were read as verdicts. Every call site invokes check_dispatch on the left of a `||`, which disables errexit for its whole body, so a non-zero driver status was ignored and whatever it had already printed was analysed: plausible output as acceptance, truncated output as a rejection -- and a rejection, in the counterexample table, reads as a PASS. The status is now checked explicitly and reported as INFRA_ERROR, distinct from REJECTED, and `classify_counterexample` separates the two at the call site rather than folding them into `-ne 0`. Section 3 pins both halves with a stand-in preprocessor that prints, then fails. The trailing `$` on the marker pattern was unpinned: the two raw-string rows only showed the old '#'-prefix filters were too broad, and dropping the anchor left all 18 checks green. `raw_string_marker` closes its raw string on a line whose prefix is a complete marker, `# 1 "fake")d"; <call>`, so dropping the anchor deletes the line and the second call with it. mkbad/bad_* renamed to mkfixture/fixture_*, now that the table has positive controls as well as counterexamples, and the two positive controls share the loop the counterexamples use.

The previous commit pinned the filter only against being too NARROW. Two independent one-token weakenings left all 24 checks green while making the checker accept a dispatch that calls the setter twice, the second time with the code: * dropping the leading `^` -- a raw string OPENING on the same line as the second call, whose remainder is a well-formed marker, matches on its tail; sed deletes the line and the call with it, 3 occurrences fall to 2; * modelling the filename as `.*` -- a closing delimiter, the second call and a later quoted string on one line let the greedy match run from the first quote to the last, swallowing the call. Both compile through the driver and both are verified against it. `marker_tail` and `marker_greedy` cover them; each fails when its own part of the pattern is weakened and nothing else does. Also: the `-nostdinc` note the fixture header pointed at went away in 7e5a89c when compilation moved to the driver, and compile_fixture returns its status rather than echoing it.

huangminghuang

Re-reviewed exact head e5dba38b. The escaped-marker, end-anchor, and preprocessing-status findings are addressed, the committed suite passes 26/26 locally, and exact-head Linux, macOS, package verification, and required checks are green.

A semantic-call escape and one missing discriminating regression remain inline, so I am not approving this head.

The full PR description is still materially stale after the follow-up series: it says four native cases and that every case runs twice although there are five and bounds is compile-time-only; omits the owned modify/erase gate and the current 17-negative/2-positive/infrastructure-error dispatch matrix; calls sysio::singleton itself a kv_multi_index; and omits the explicit upstream-template-call incompatibility now documented in the header. Please refresh it to describe the complete current PR.

Two findings from review. A source-text count sees spellings, and a second call can reach the same wasm import without adding one: extern void again(uint64_t) __asm__("sysio_set_" "contract_name"); again(c); The adjacent string literals are still two tokens after preprocessing -- concatenation is translation phase 6, which -E does not reach -- so the identifier is spelled twice in the file and the text count stays at 2. The object calls the import twice and the second overwrites the receiver with the code. check_dispatch_symbols reads the relocations of the object the driver emits: exactly one call to sysio_set_contract_name inside apply(), and it is the first call there. That states "exactly once, before any dispatch" over the code that ships rather than over the source text, and it does not care how the symbol was spelled. Neither checker subsumes the other -- the text one is what sees that the argument is `r` and not `c` -- so both run, each against the real dispatch, a counterexample and a positive control. asm_label is kept as the counterexample it is, and the run reports that the text checker accepts it rather than asserting so, since a future text checker strong enough to catch it should not fail the suite. The trailing `([[:space:]]+[0-9]+)*` of the marker pattern was unpinned: every marker in these fixtures was flagless, so removing the group left all 26 checks green. marker_flags_ok puts an #include immediately before the setter, which makes CDT emit enter and return markers carrying `1` and `2` between the opening brace and the first statement. Scope of the symbol check is apply() itself, matching the text checker: a setter call made from another function apply() calls is out of range of both. The dispatch TU defines only apply(), so there is no such function to write today, but it is a limit rather than a covered case, and the comment says so.

huangminghuang

Re-reviewed exact head 6dbf32d. The two previous findings are fixed: the direct asm-label counterexample is now rejected, and the trailing-marker-flags positive fixture is discriminating. The committed script passes 31/31 locally and all current-head required CI is green.

I found two new P2 gaps in the object-level checker, detailed inline, so I am not approving this head.

Please also refresh the full PR description as required for review follow-ups. It is materially stale: it still says four native cases and that every case runs twice, omits the owned modify/erase success coverage, describes the old text-only dispatch check rather than the current text/object matrix and infrastructure checks, and says singleton is itself kv_multi_index rather than owning one through kv_singleton.

…status Two findings from review. A relocation names the target of a DIRECT call. A call_indirect names only a type, so its target is exactly what relocations cannot show -- and the address reaches the table without a direct call appearing: setter_fn volatile fp = again; // R_WASM_TABLE_INDEX_SLEB, not FUNCTION_INDEX_LEB fp(c); // call_indirect which leaves one direct setter call, first, and a second call to the same import that a scan of call relocations cannot count. Both checkers returned OK while apply() overwrote the receiver with the code. The generated dispatch is a chain of direct calls and has no legitimate indirect one, so an indirect call in apply() is now refused rather than analysed, and indirect_alias pins it. check_dispatch_symbols never looked at llvm-objdump's status, repeating in the new checker the mistake the preprocessor path had already been fixed for: it runs on the left of a `||`, which disables errexit for its whole body, so a dump that failed after printing was read as a verdict. A complete-looking dump exiting non-zero read as acceptance; a truncated one read as a rejection, which in a reject row reads as a PASS -- every such row would pass on a machine where llvm-objdump cannot run. The status is checked and reported as INFRA_ERROR, and classify_symbols requires EXACTLY the expected status rather than merely non-zero, so a reject row is not satisfied by an analyser failure. Both halves are pinned by a stand-in objdump that prints, then exits 73, mirroring section 3. Each check has one discriminating row: relaxing the count fails asm_label, deleting the first-call branch fails after_dispatch, removing the indirect ban fails indirect_alias, and none of the three disturbs the others.

huangminghuang

Re-reviewed exact head 6920700. Both previous P2s are fixed: the indirect-call counterexample is now rejected, analyzer failures remain INFRA_ERROR rather than verdicts, and the corresponding mutations fail their intended rows. The committed script passes 36/36 locally and every current-head required CI check is green.

I found one new P2 false positive in the opcode detector, detailed inline, so I am not approving this head.

The PR description also remains materially stale despite the follow-up commits: it still says four native cases and that every case runs twice, omits owned modify/erase success coverage, describes the old text-only dispatch check instead of the current text/object and infrastructure matrix, and says singleton is itself kv_multi_index rather than owning one through kv_singleton. Please refresh the full description before approval.

The indirect-call detector searched every line inside apply() for the string `call_indirect`, and it ran before the relocation branch. A contract may legally declare an action named `call_indirect`; its generated wrapper is `__sysio_action_call_indirect_dispatchrcv`, and the direct relocation to it contains the substring. There is no indirect-call instruction anywhere in that dispatch, but the checker reported one and rejected a correct dispatch. Lines are now classified by shape before content. llvm-objdump indents a relocation record with tabs and an instruction with spaces, so the relocation branch takes the tab-led lines and the opcode is compared as the mnemonic FIELD of what remains -- never as text anywhere on the line. The contract in section 1 gains exactly that action, so the positive control on the real generated dispatch covers it. Reverting to the substring match fails that row, 35/36; removing the indirect ban still fails indirect_alias, so the two remain independently pinned.

huangminghuang

Re-reviewed exact head c63edee. The opcode detector now distinguishes instruction mnemonics from relocation-symbol text and the legal call_indirect action-name positive control passes. The prior indirect-call and analyzer-status regressions remain covered, the exact dispatch suite passes 36/36 locally, the full PR description is current, and Linux, macOS, package verification, and the required-check aggregate are all green. No further issues found.

Both PRs the docs referenced as pending are merged, and one of them made a sentence here false. Every claim below was re-verified against a CDT built from merged master, not just reworded. The "unchanged host functions" list promised "every privileged.h setter". #112 removed set_kv_parameters_packed, which was one. The list now names the setters that remain, and the two removals get rows in the "Removed on Wire" table with the diagnostic each produces -- they differ, and the difference is useful when porting: <sysio/security_group.h> is gone outright ("file not found"), while privileged.h is still there and set_kv_parameters_packed is an undeclared identifier in it. Both confirmed by compiling; set_privileged from the same header still builds. The ABI section told the reader not to reach for cdt-abidiff, on the strength of limitations #112 fixed. Inverted: it now lists the sections the tool compares -- version as the full string, structs, types, actions, tables with the full metadata, clauses, enums, protobuf_types, variants, action_results and error_messages -- and keeps the old-toolchain caveat and the jq fallback for anyone on an older CDT. The legacy-database and multi_index sections described #112 and #113 as forthcoming. They describe the current toolchain now, with the pre-merge behaviour as the caveat. Verified against merged master: `it++` is still rejected with "overload resolution selected deleted operator '++'", a hand declared db_store_i64 now fails with "wasm-ld: undefined symbol: db_store_i64", and the documented static_cast escape hatch plus lower_bound on name, uint64_t and {42} all compile. Both docs also now say the #113 receiver guard reaches sysio::singleton: get_or_create, set and remove mutate through the kv_multi_index it holds, so a singleton handle on another account's code is read-only like a table handle. The kv-multi-index singleton section also said singleton "is backed by" kv_multi_index, which reads as inheritance; it holds one.

heifner requested review from a team and huangminghuang August 31, 2026 16:09

heifner mentioned this pull request Aug 31, 2026

docs: add Antelope-to-Wire contract migration guide #111

Open

huangminghuang reviewed Aug 31, 2026

View reviewed changes