Adapt to wire-sysio table_id namespace isolation; long table name support

[heifner]

Summary

Wire-sysio PR Wire-Network/wire-sysio#288 widened table_def.name from sysio::name (uint64) to free-form std::string and added table_id (uint16, DJB2 hash of the table name % 65536) and secondary_indexes for KV-table per-table namespace isolation. The binary wire format break means abieos must be updated in lockstep with the chain side or every field after tables in abi_def parses misaligned, and any contract whose table name is longer than 12 chars is unreachable through the C API.

This PR takes the chance to make the long-table-name story end-to-end by switching the internal table_types map and the C API entry point to free-form strings.

Companion PRs:

• KV table_id namespace isolation wire-sysio#288 — chain-side table_id namespace isolation
• Update abi_def schema for wire-sysio table_id namespace isolation wirejs-native#4 — TS schema mirror of this PR

⚠️ Breaking C API change

abieos_get_type_for_table now takes the table name as a const char* instead of a uint64-encoded sysio name:

// Before
const char* abieos_get_type_for_table(abieos_context*, uint64_t contract, uint64_t table);
// After
const char* abieos_get_type_for_table(abieos_context*, uint64_t contract, const char* table);

The only known consumer of this entry point in the Wire ecosystem is node-abieos (the native binding used by Wire-Network/hyperion-history-api), which was already converting JS strings to uint64 via abieos_string_to_name() before calling this function — that round trip is now redundant and node-abieos can pass the string straight through. node-abieos will be updated in a follow-on PR.

Changes

include/sysio/abi.hpp

• New index_def struct: {name: string, key_type: string, table_id: uint16} mirroring sysio::chain::index_def
• table_def: name: name → string, +table_id: uint16, +secondary_indexes: index_def[]
• abi_def: +protobuf_types as a might_not_exist<std::string> extension to mirror wire-sysio's reflection layout (variants, action_results, enums, protobuf_types). Keeps binary forward-compat for ABIs that include the field even though abieos itself doesn't consume it.
• abi.table_types: std::map<sysio::name, string> → std::map<std::string, std::string, std::less<>>. The std::less<> transparent comparator lets callers do find(string_view) without constructing a temporary std::string per lookup.
• abi.abi_types likewise gets std::less<>.
• New aliases: table_type_map, abi_type_map (used in abi.cpp signatures).
• to_json(abi_def) writes protobuf_types unconditionally for consistency with the surrounding extension fields (variants, action_results, enums) which always emit even when empty.

src/abieos.hpp

• jobject typedef gets std::less<> for the same transparent-lookup reason — the JSON object map is the hottest map in abieos.

src/abi.cpp

• convert(abi_def, abi) populates table_types directly with the new string-typed table_def.name; no name conversion needed.
• Function signatures updated to use abi_type_map alias.

src/abieos.h, src/abieos.cpp

• abieos_get_type_for_table signature change as described above.
• Internal lookup uses std::string_view via the std::less<> transparent comparator to avoid an allocation per call.

src/test.cpp

• tokenHexAbi regenerated for the new binary format. The token ABI shape (sysio::abi/1.0) is unchanged, but the on-wire encoding of tables[].name is now a length-prefixed string instead of a uint64. The new hex was produced from the canonical token ABI JSON via abieos_abi_json_to_bin().

external/wirejs-native

• Submodule pointer bumped to feature/wire-sysio-table-id (Update abi_def schema for wire-sysio table_id namespace isolation wirejs-native#4) which mirrors the schema changes on the TypeScript side.

Tests

All four C++ tests pass against the updated layout:

• test_abieos
• test_abieos_template
• test_abieos_key
• test_abieos_reflect

[dtaghavi]

Code Review

Overview

This PR adapts abieos to mirror wire-sysio PR #288's binary format change: table_def.name widens from sysio::name (uint64) to std::string, gains table_id (uint16 DJB2 hash) and secondary_indexes, and a forward-compat protobuf_types extension is added to abi_def. The C API's abieos_get_type_for_table is updated from uint64_t table to const char* table. The rationale and companion PRs are well-documented.

---

Correctness & Logic

abieos_get_type_for_table null-check placement — The null guard is good, but the function will still crash with a SIGSEGV if a caller passes a non-null but non-null-terminated char*. This is a C API boundary; consider also documenting that the string must be null-terminated in the header comment.

std::string_view{table} lifetime — table_it->second.c_str() returns a pointer owned by the map value. If a concurrent writer reloads the ABI between the find and the return, the pointer dangles. This is an existing issue with the function (not introduced here), but worth noting since the signature change makes the function more likely to be called from JS/native bindings with looser threading assumptions.

secondary_indexes in abi.cpp — The diff doesn't show the convert(abi_def, abi) body; it would be useful to confirm that secondary_indexes is either fully consumed or explicitly skipped. If it's silently ignored during conversion, add a comment saying so — otherwise future readers will wonder whether it's a bug.

protobuf_types emission — to_json_write_helper(def.protobuf_types.value, ...) emits "protobuf_types": "" when the field was absent in the binary input. For variants/action_results/enums (vectors), emitting an empty array is harmless; for a string field, emitting "" could confuse a consumer that checks if (protobuf_types) for presence. Consider guarding with if (def.protobuf_types.exists) or equivalent.

---

Breaking Change Concerns

C API binary-compatibility — Changing uint64_t table to const char* is a silent ABI break on any platform where both parameters fit in the same register class. Code compiled against the old header that dynamically links to the new library will misbehave without a linker error. A symbol-version bump (@@ABIEOS_2) or a version macro (ABIEOS_VERSION) would let consumers detect the mismatch at build time.

action_types not updated — abi::action_types remains std::map<sysio::name, std::string> while table_types and abi_types now use std::less<>. This inconsistency is likely intentional (actions are always 12-char names), but a short comment would prevent someone from "fixing" it later by accident.

---

Test Coverage

• No new test exercises the updated abieos_get_type_for_table with a string argument, a long (>12 char) table name, or the null-table error path.
• No round-trip test for protobuf_types (serialize → deserialize → compare).
• No test that a secondary_indexes-bearing ABI deserializes cleanly.

The tokenHexAbi regeneration is the primary coverage; it would be helpful to add at least one targeted test for each of the above.

---

Minor / Style

• Comment in test.cpp references /tmp/regen_token_hex (a temp file). The regeneration script should either be committed to the repo or replaced with a reproducible make target / inline command so future maintainers can re-derive the hex without guessing.
• table_id comment says "DJB2 hash of the table name % 65536" — worth noting the collision domain (65 536 slots) in a comment on index_def::table_id, or at minimum pointing to where wire-sysio guarantees uniqueness, so abieos maintainers know not to rely on it as a true key.
• Several new comments include wire-sysio PR references (e.g. // Wire-sysio PR Wire-Network/wire-sysio#288). These are useful now but will rot. Keep them in the commit message / PR description rather than inline, or at least use a stable tag rather than a PR number.

---

Summary

The core logic is sound and the transparent-comparator upgrades (std::less<> on jobject, table_types, abi_types) are a clean, low-risk improvement. The main concerns before merging are:

1. Versioning / detection of the breaking C API change.
2. Test coverage for the new string-based table lookup, long names, and null input.
3. Clarification on whether secondary_indexes is consumed or silently skipped in convert().
4. protobuf_types JSON emission guard when the field is absent.

Everything else is polish.

[dtaghavi]

Follow-up Review

All four blocking items from the previous round have been addressed. The PR is in good shape; a few small notes below.

---

What was fixed ✓

• protobuf_types emission guard — the if (!def.protobuf_types.value.empty()) guard is correct and the comment explains the consumer-facing rationale.
• secondary_indexes not consumed — the explicit comment in abi.cpp makes the intentional skip clear for future maintainers.
• table_id collision domain — well-documented on index_def::table_id; the "uniqueness guaranteed chain-side" note sets the right expectation.
• action_types inconsistency — the new comment on abi::action_types explains exactly why the key type difference is intentional.
• Null-termination requirement — documented clearly in abieos.h.
• /tmp/regen_token_hex — replaced with an inline, self-contained regeneration recipe in the comment; the new check_token_abi_hex_round_trip test closes the loop by failing with a ready-to-paste hex diff on mismatch.
• Test coverage — four new test functions cover all the gaps from the prior review (hex round-trip, long table names, abieos_get_type_for_table error paths, protobuf_types presence/absence).

---

Remaining minor notes

CMake version bump only — CMakeLists.txt is bumped 0.1 → 0.2, which is good for build systems. However, C consumers who include abieos.h directly (especially when using the shared library as a system package) have no compile-time way to detect the uint64_t → const char* signature change. A one-liner like #define ABIEOS_VERSION 2 in abieos.h would let them guard with #if ABIEOS_VERSION >= 2.

Stale call in check_protobuf_types_round_trip — lines around "Absence case":

check_context(context, abieos_set_abi(context, contract, R"({"version":"sysio::abi/1.2"})"));
check_context(context, abieos_abi_json_to_bin(context, R"({"version":"sysio::abi/1.2","protobuf_types":""})"));

The abieos_set_abi call's result is immediately overwritten by abieos_abi_json_to_bin into the same context buffer — the set-abi output is never used and can be removed without changing test behavior.

c_str() lifetime (pre-existing) — abieos_get_type_for_table still returns table_it->second.c_str(), a pointer into the live map. This was pre-existing and is out of scope here, but worth tracking as a follow-on: a concurrent ABI reload would dangle the returned pointer for the caller.

---

Overall

The implementation is correct, the binary format alignment is well-guarded by comments, and the new tests are thorough. The two items above are polish-level. LGTM to merge once the stale abieos_set_abi line is dropped (or left with an explanatory comment if there's a reason for it).

[dtaghavi]

Follow-up Review (round 3)

What changed

The only substantive change from the previous round is in check_protobuf_types_round_trip. The stale abieos_set_abi call is gone and has been replaced with a loop over two "absence" inputs:

const char* absent_inputs[] = {
    R"({"version":"sysio::abi/1.2","protobuf_types":""})", // explicit empty
    R"({"version":"sysio::abi/1.2"})",                     // fully omitted
};

This is better than what we asked for — it now tests both the explicit-empty-string and the field-entirely-absent paths in one pass, which is the right thing to assert. Good fix.

Everything else is unchanged from the previous version (all prior blocking items remain resolved).

---

One open item

The ABIEOS_VERSION header macro mentioned last round is still absent. The CMake version is 0.2, but a C consumer who builds against a system-installed header has no #if ABIEOS_VERSION >= 2 guard available to detect the uint64_t → const char* signature change at compile time. Adding #define ABIEOS_VERSION 2 to abieos.h is a one-liner and closes that gap permanently.

This is the only remaining note. It's non-blocking if the Wire ecosystem is the only consumer (the PR description covers that), but worth doing while the change is fresh.

---

Overall

Implementation is correct, binary alignment is well-guarded, tests are thorough and now self-documenting. LGTM to merge.

[heifner]

Follow-up Review (round 3)

One open item

The ABIEOS_VERSION header macro mentioned last round is still absent. The CMake version is 0.2, but a C consumer who builds against a system-installed header has no #if ABIEOS_VERSION >= 2 guard available to detect the uint64_t → const char* signature change at compile time. Adding #define ABIEOS_VERSION 2 to abieos.h is a one-liner and closes that gap permanently.

I don't think we should do this. It is new functionality.