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

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.

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.

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.

…caveat Third review round on the guide. Three findings, all of them cases where the text was confident and wrong. - The `-S <gen>` I added last round parses, but silently returns nothing for the generation a reader is most likely to try. CDT declares every scoped table's scope as a `name` and the chain honours that, trying `name(scope)` before falling back to an integer. `1`-`5` are valid name characters, so `-S 1` is read as the name "1" -- 576460752303423488 -- and the query returns no rows, which reads as "your issuer is not a node owner". Only generations containing a 0 or a 6-9 work, because name() rejects those characters and the fallback runs. Verified by computing the encoding for 0,1,2,5,6,9,10,11,21,100. The command now omits -S and reads network_gen off the rows. - The "Bytes per row" row was wrong on both sides and inverted the conclusion. Antelope's key_value_object is 108 + value with a 16-byte key, not a 32-byte one; Wire's kv_object is 112 + key + value. More importantly the guide implied Wire is cheaper per row. wire-sysio's own kv-ram-billing.md -- which this guide links -- gives 124 legacy vs 144 for sysio::multi_index (+16%) and 136 for kv::table (+10%) on a dense table. A ported contract needs MORE RAM, not less, and telling an author otherwise is how a provisioning policy comes up short. Says so now. - kv-multi-index.md gained a bullet asserting #113's guards as shipped fact, in the same commit that added the "arrives with #113" caveat to migrating-from-antelope.md. The caveat landed in one of the two files. It is in both now, and the "one known divergence" line -- there are three at this commit -- lists them. Also: - macOS sed guidance was insufficient and named the wrong package. `sed -i ''` alone does not help: BSD sed has neither \b nor \|, so \beosio\b matches a literal "beosiob" and renames nothing, silently, while the $assert\|...$ group is a syntax error. gsed, from gnu-sed. - The db_* paragraph said "links but fails at deploy" while citing #112 in the same breath; after #112 removes them from the --allow-undefined-file list it is a link error. Both states described. - The table_id collision diagnostic comes from cdt-codegen's link-stage ABI finalize, not wasm-ld; `cdt-cpp -c` on the same file succeeds. - Upstream's bounds are member templates whose parameter cannot be deduced, not an overload set -- same outcome for &table::lower_bound, different reason, and the named static_cast escape hatch is Wire-only. - hash_id::max_length is 128 but nothing validates against it or against an alphabet; presented as a convention rather than a check. - kv-storage-guide.md still said "Drop-in EOSIO replacement", the stronger form of the claim this PR already softened elsewhere in the same file.

Fourth review round. The "Budget more RAM, not less" paragraph was inserted INSIDE the comparison table, which terminated it: the `get_table_rows` row that followed became a lazy continuation of the paragraph and stopped rendering as a table row -- the very row the next sentence refers to. Moved out. More importantly the advice it carried was over-corrected. Last round fixed a claim that Wire is cheaper per row by asserting the opposite; both are wrong, because the direction depends on rows-per-scope. The `table_id_object` is billed per (code, scope, table), so a table with one row per scope -- a token balance, a per-user settings row, the most common Antelope shape -- sheds a whole 108-byte object per scope: wire-sysio's figures are 232 -> 144, a 38% SAVING. Only dense tables with few scopes pay more (124 -> 144, +16%), and across EOS mainnet the net is a 2-6% saving. The guide now gives the full four-row table and says to size from your own ratio. Also: - The BSD sed note claimed `$assert\|...$` is a syntax error. It is not: without REG_ENHANCED macOS sed treats `\|` as a literal `|`, so the group matches the literal text "assert|assert_message|..." and, like `\b`, simply renames nothing. Both halves fail silently, which is the part worth stating -- a reader told to expect an error concludes the script worked. - kv-multi-index.md's divergence list gained a `name`-bounds bullet last round that was itself an uncaveated #113 claim, in the same file where the guards bullet had just been caveated. Now carries the caveat, describes upstream's member template correctly, and gives the static_cast form that works on Wire. - Added the fourth real divergence, the trivially-copyable secondary-key static_assert (kv_multi_index.hpp:769), which the migration guide lists and its sibling did not. - Fixed a duplicated "the", a swallowed sentence, a bare #113 that does not autolink, a duplicated #112 mention, and a summary bullet that restated the _i 128-character limit as a rule after the body had just called it a convention.

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.

The guide told readers to use _i only above 13 characters, because a shorter annotated name was routed through string_to_name by the ABI generator while the literal hashed -- user_table ran at 61956 and was advertised as 3509. wire-cdt#115 makes the template-derived table_id authoritative, so both lengths now agree. Restated as a pre-#115 caveat, matching how this guide already handles #112 and #113, and the "has to be renamed, or lengthened past 13 characters, until abigen is fixed" advice is dropped -- _i is now the answer for a name outside the _n alphabet at any length. The _n alphabet note also gains the 13th-position restriction (.12345a-j), which is 4 bits rather than 5.