Adopt wire-sysio table_id namespace isolation; pass table names as strings

[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) + secondary_indexes for KV-table per-table namespace isolation. The companion abieos PR Wire-Network/abieos#12 changed the C API entry point abieos_get_type_for_table() to take a const char* table name instead of a uint64 sysio name.

This PR picks up the new abieos and updates the binding to match. End result: long table names (>12 chars) now work end-to-end through node-abieos.

Companion PRs:

• KV table_id namespace isolation wire-sysio#288 — chain-side table_id namespace isolation
• Adapt to wire-sysio table_id namespace isolation; long table name support abieos#12 — abieos C API + binary format update
• Update abi_def schema for wire-sysio table_id namespace isolation wirejs-native#4 — TS schema mirror

Changes

abieos submodule

Bumped to feature/wire-sysio-table-id (commit 9c1000a, the head of Wire-Network/abieos#12). The bump pulls in:

• table_def long-name support (name widened to std::string)
• The breaking C API signature change for abieos_get_type_for_table
• abi.table_types map keyed by std::string with std::less<> transparent comparator
• Regenerated tokenHexAbi test fixture for the new on-wire encoding

src/main.cpp — get_type_for_table()

Drop the table-name string → uint64 → string round trip:

 string get_type_for_table(const char *contract_name, const char *table_name)
 {
     if(global_context != nullptr) {
         uint64_t contract = abieos_string_to_name(global_context, contract_name);
-        uint64_t table = abieos_string_to_name(global_context, table_name);
-        auto result = abieos_get_type_for_table(global_context,contract, table);
+        auto result = abieos_get_type_for_table(global_context, contract, table_name);
         if(result == nullptr) {
             return "NOT_FOUND";

The round trip was lossy: any table name longer than 12 characters or containing characters outside the sysio name alphabet was silently truncated, making the table type unreachable. The binding now passes the JS string straight through. The contract name still goes through abieos_string_to_name() because contract identifiers ARE sysio::name on both sides of the API.

examples/ABIs/eosio.token.raw — regenerated

The example test fixture is base64-encoded binary ABI; the old fixture used the legacy table_def shape (uint64 name) and produced Stream overrun on load against the new abieos. Regenerated against the new abieos binary table_def encoding.

The contained ABI shape (eosio::abi/1.1, two tables: accounts/stat) is unchanged — only the encoding of tables[].name changes from "fixed 8 bytes" to "varint length + bytes".

lib/abieos.node — rebuilt

Built against the new abieos with clang++-18 via cmake-js compile --CDSKIP_SUBMODULE_CHECK=1 (the parent submodule ref is intentionally ahead of master while abieos PR #12 is in flight).

Test plan

examples/basic.mjs:

• 6/6 positive checks pass (eosio voteproducer, eosio.token transfer + stat, eosio.msig approvals2 + proposal, eosio producers)
• The seventh check is a deliberate negative test that looks up a non-existent table on a non-existent contract — still errors as expected
• jsonToHex + hexToJson serialization round-trip on eosio::voteproducer passes 50/50 runs at ~6 µs/run

Note on the submodule pointer

The abieos submodule ref points at the in-flight feature/wire-sysio-table-id branch of Wire-Network/abieos. Once Wire-Network/abieos#12 merges, this PR's submodule pointer should be updated to the merged commit on master before merging here.

[dtaghavi]

Code Review

Overview

This PR fixes a real, impactful bug: abieos_get_type_for_table() previously took a uint64_t sysio name, so any table name longer than 12 characters or containing characters outside the sysio name alphabet was silently truncated, making its ABI type unreachable. The fix passes the table name straight through as a const char* after adopting the updated C API in abieos PR #12. The change is minimal, correct, and well-motivated.

---

Issues and Suggestions

Blocking before merge

• Submodule points to an in-flight feature branch (feature/wire-sysio-table-id, commit 9c1000a), not a stable ref. The PR description acknowledges this and states the pointer must be updated once abieos#12 merges. This should be a hard prerequisite — do not merge until the submodule ref is pinned to a merged commit on master/main. A CI check or a PR checkbox would make this impossible to accidentally skip.

• Committed binary lib/abieos.node. A pre-built, platform-specific native addon committed to the repo is a significant concern:

  • It is non-auditable: reviewers cannot verify it was actually built from the accompanying source.
  • It is built against a non-merged upstream branch, so it may diverge silently from what CI would produce.
  • It won't work on other platforms/architectures without a rebuild, yet there is no build gate to enforce this.
  • Recommendation: remove lib/abieos.node from git tracking (add to .gitignore) and have CI build it as an artifact, or document the exact build command and platform contract in a README so users know they must rebuild it locally. If it must be committed, at minimum document the exact compiler version, flags, and source commit it was built from — the PR description mentions clang++-18 and cmake-js compile --CDSKIP_SUBMODULE_CHECK=1, but that should live in a BUILD.md or Makefile target rather than only in the PR body.

Non-blocking suggestions

• Comment verbosity in src/main.cpp. The 5-line block comment added to get_type_for_table() largely restates what the diff already shows. A single line like // table_name is now a free-form string (abieos#12); no name-encoding round trip needed is sufficient. The detailed rationale belongs in the commit message / PR description, where it already is.

• Asymmetric name handling is correct but worth a brief note. Contract names still go through abieos_string_to_name() (they remain sysio::name uint64 on-chain), while table names are now passed as const char*. This is intentional and correct, but a reader unfamiliar with the upstream change might find the asymmetry surprising. A one-line comment on the contract line would preempt that confusion.

• No automated regression test for the long-name case. The test plan is manual (examples/basic.mjs). A test that explicitly exercises a table name longer than 12 characters (e.g. currency_stats) would prove the fix and guard against regressions if the abieos API changes again.

• examples/ABIs/eosio.token.raw diff is unauditable. The file is base64-encoded binary, so the diff is noise. Consider adding a comment in the commit or a companion .md describing what ABI shape it represents and how to regenerate it (abieos binary + source ABI JSON), so future maintainers can verify or update it without guessing.

---

Summary

The core fix (src/main.cpp) is correct and the PR description is thorough. The two hard blockers are: (1) merge only after the submodule is pinned to a merged, stable abieos commit; and (2) the committed binary needs a reproducibility story — either drop it from the repo or add a documented, verifiable build path.

[dtaghavi]

Code Review

Overview

This PR updates the node-abieos Node.js binding to match the breaking C API change in abieos#12, where abieos_get_type_for_table() now takes a const char* table name instead of a uint64_t sysio name. The core fix is a one-line change that eliminates a lossy string → uint64 → string round-trip, unblocking table names longer than 12 characters or containing characters outside the sysio name alphabet. The PR also ships a regression test, updated example fixtures, and documentation.

---

Code Quality

src/main.cpp — the core fix

The fix itself is correct and minimal. One observation:

• The comment added to get_type_for_action is useful for explaining the asymmetry between actions (still uint64) and tables (now free-form string). However, it could be simplified — the existing code already shows what it does; the comment would be stronger if it just captured why the asymmetry exists (i.e., action names are constrained to the sysio name alphabet by the chain, but table names are not after wire-sysio#288).

examples/tests.mjs — regression test

• Good: the test uses a 20-char table name that demonstrably would have been truncated pre-fix, making it a genuine regression guard.
• Concern: test failures surface as console.log('ERROR ...') rather than thrown exceptions or process exit codes. This means CI would need to scrape stdout to detect failures — a silent failure mode if output is not checked. Consider throw new Error(...) or process.exit(1) on failure so the test is self-enforcing.
• The finally block calling deleteContract is good hygiene.
• The JSDoc comment is detailed and helpful, though it could be trimmed — the references to companion PRs will rot as those PRs merge and get renumbered.

examples/ABIs/README.md

Clean and well-structured. The regeneration instructions are valuable. Minor note: the abieos --abi-to-bin CLI referenced in the snippet is noted as "not included — build from submodule", which is honest, but a pointer to a more accessible regeneration path (e.g., via a dev node's get_raw_abi) would reduce friction.

---

Potential Issues / Risks

1. Committed binary (lib/abieos.node): The repo commits the compiled .node binary, which is platform/arch-specific. The binary was built with clang++-18 on (presumably) Linux x86-64. Any contributor or consumer on a different platform or OS will silently get a stale or mismatched binary unless they rebuild. This isn't introduced by this PR, but it's worth flagging as ongoing technical debt — the CDSKIP_SUBMODULE_CHECK=1 flag used during the build also means the binary can quietly diverge from the submodule.

2. Submodule pointing at an in-flight branch: The PR explicitly calls this out in the description. The abieos submodule ref targets feature/wire-sysio-table-id rather than a stable commit on master. Until abieos#12 merges and the pointer is updated, anyone who git submodule updates will land on a transient branch head. The PR description says to update before merging here — just make sure that step isn't forgotten.

3. eosio.token.raw encoding change: The binary fixture changed encoding (fixed 8 bytes → varint length + bytes for tables[].name). The README explains this well. The risk is that any downstream consumer who hardcodes expectations about the raw bytes of this fixture will silently break. Not a blocker, just worth a heads-up to fixture consumers.

4. No test for the NOT_FOUND / error path with a long name: The regression test covers the happy path (long name resolves correctly) but doesn't verify that a non-existent long-name table returns NOT_FOUND as expected. Low priority, but it would close the loop on the null check in get_type_for_table.

---

Summary

The core fix is correct, well-scoped, and well-explained. The main actionable suggestions before merge:

• Make the regression test fail loudly (exit code / thrown exception) rather than just logging ERROR.
• Pin the submodule to the merged commit on master once abieos#12 lands before merging this PR (as already noted in the description).

Otherwise this looks good to go.

[dtaghavi]

Code Review — Round 3 (Approval)

What changed since my last review

Commit 301a6f7 ("Make tests self-enforcing: throw on failure so CI exits non-zero") addresses the main actionable feedback from my 2026-04-21 review. Combined with commits 2 and 3, every hard blocker from my initial 2026-04-16 review is now resolved:

Prior feedback	Status
Submodule must be pinned to merged master, not feature/wire-sysio-table-id	✅ Resolved — commit 3650d24 moves the pointer from 9c1000a to merged 7ca8512
Over-verbose 5-line comment in get_type_for_table()	✅ Resolved in bf2a58c — now a one-line note, with a matching rationale in get_type_for_action() explaining the name-encoding asymmetry
No automated regression test for >12-char table names	✅ Resolved in bf2a58c — longNameRegressionTest loads an inline eosio::abi/1.2 with a 20-char table name and asserts resolution
eosio.token.raw diff is unauditable	✅ Resolved in bf2a58c — examples/ABIs/README.md describes every fixture and documents a regeneration path
examples/tests.mjs failures surfaced only as console.log('ERROR ...'), so CI could pass with broken tests	✅ Resolved in 301a6f7 — both typeTests and longNameRegressionTest now throw; process exits non-zero on failure

Review of commit 301a6f7 (self-enforcing tests)

The failure-propagation logic is correct. Walking through the cases for typeTests:

• Positive test, got expected type → OK, no increment.
• Positive test, got wrong type → failures++, logs ERROR - Got: X, Expected: Y.
• Positive test, unexpectedly threw → failures++, logs ERROR - <msg>.
• Negative test (expects === ''), got a type → failures++, logs ERROR - expected throw, got: X.
• Negative test, threw → OK - throws as expected: <msg>, no increment.

All four failure branches are counted; the final if (failures > 0) throw ensures non-zero exit. The negative test relies on getTypeForTable throwing on NOT_FOUND — verified in lib/abieos.js:

getTypeForTable(contractName, table_name) {
    const result = Abieos.native.get_type_for_table(contractName, table_name);
    if (result === 'NOT_FOUND') {
        throw new Error(...);
    }
    return result;
}

So test #7 (code: '2', name: 'null') will throw via the JS wrapper's NOT_FOUND check, satisfying the "throws as expected" branch. Good.

longNameRegressionTest throws directly on unexpected type or failed loadAbi, and the try/finally guarantees deleteContract runs regardless. Clean.

What remains

Non-blocking / pre-existing:

• lib/abieos.node committed binary is still in the repo. This isn't introduced by this PR — it's ongoing technical debt. Worth a follow-up issue to either gitignore it with a CI build step or document the exact build contract (compiler version, flags, source commit) in a BUILD.md. Not a merge blocker for this PR.
• In examples/tests.mjs, the inner const typeTests = [...] shadows the outer function name typeTests. Pre-existing quirk — fine to leave.

Verdict

Approved. The core fix is minimal and correct, the regression test is a genuine guard (20-char name would have been truncated pre-fix), the submodule is pinned to a merged commit, and tests now fail loudly under CI. Ready to merge.