Skip to content

Add Mintlayer Python SDK (port of go-sdk) - #1

Merged
erubboli merged 31 commits into
mainfrom
feat/initial-sdk-port
Sep 18, 2026
Merged

erubboli merged 31 commits into
mainfrom
feat/initial-sdk-port

Conversation

@nullPointerEnjoyer

@nullPointerEnjoyer nullPointerEnjoyer commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Complete Python port of mintlayer/go-sdk: sync-first, requests + wasmtime, MIT.

What's included

Module Contents
mintlayer.node JSON-RPC 2.0 client, 38 methods — wire-exact (durations as [secs, nanos], tuple wire shapes, OrderInfo.nonce nullability fix over Go)
mintlayer.indexer REST client, 33 routes under /api/v2 — lenient numerics (string uint64s, "3.5%" margin ratios), path-traversal-encoded IDs
mintlayer.wallet JSON-RPC client, 56 routes — every Go wire quirk pinned by tests (account_index, dropped account, hardcoded Trusted, OutputValue/CurrencyFilter custom encodings, null-vs-omitempty rules); mnemonic/passphrase redacted from reprs
mintlayer.wasm Full wasm-bindgen host shim on wasmtime (31 imports): keys, addresses, tx building, signing, fees, intents — byte-for-byte parity with Go verified (same derived keys/addresses/fees from the standard test mnemonic)
top-level Client(Config) selective wiring, lazy init_wasm(), convenience re-exports
docs + examples 7 ported guides, full README, examples/send_coins.py + examples/issue_token.py

Quality

  • 386 tests passing, 95.5% coverage (pytest --cov); all wasm mixins at 100%
  • ruff + mypy clean
  • Vendored WASM binary pinned by sha256, verified fail-closed at import; host result buffers zeroed after calls
  • .netrc ambient-credential fallback suppressed; Basic Auth gated on username

Notable fixes over the Go original

  • Externref slot double-dealloc: the wasm-bindgen callee owns the index-array table slots; Go's host-side dealloc is a double-free that corrupts the free list ("array contains a value of the wrong type" after repeated multi-element array calls). Fixed + regression-tested (worth upstreaming to go-sdk).
  • OrderInfo.nonce is Optional (daemon sends null for active orders; Go's uint64 unmarshal silently skips it).
  • Null results raise typed errors instead of silently producing zero values.

Process

Every commit was gated on security-reviewer + code-reviewer agents (findings fixed pre-commit), tests were written/mutation-verified by the test agent, and wire parity was live-diffed against the Go SDK during review.

Full port of the Go SDK's wasm-bindgen host shim to wasmtime-py:
- all 31 host imports from './wasm_wrappers_bg.js' (RNG via os.urandom,
  JSON serde round-trip for TxAdditionalInfo, error side-channel capture,
  externref table index arrays with callee-ownership-correct cleanup)
- vendored wasm_wrappers_bg.wasm with load-time sha256 pinning
- complete API surface: key derivation, addresses, ids, inputs/outputs
  encoding, timelocks, fees, transactions, witnesses/signing, staking,
  intents (78 public methods across 11 area mixins)
- byte-for-byte parity with the Go SDK verified (deterministic mnemonic
  derivation, addresses, fees, timelocks, spend maturity)
- result buffers zeroed before free; thread-safe via RLock + @synchronized
- ruff + mypy clean
Shared JSON-RPC 2.0 transport (mintlayer/_jsonrpc.py) mirroring the Go
semantics: params always objects, ids from 1, HTTP status never inspected,
null result = not-found, basic auth gated on username (netrc fallback
suppressed for owned sessions).

Node client with all 38 methods across node/chainstate/mempool/p2p
mixins: full wire parity with Go (durations as [secs, nanos], FeeRatePoint
and BannedPeer tuple wire shapes, Currency omitempty vs OutpointSourceID
null content, orders filters always sent). Deliberate fix over Go:
OrderInfo.nonce is Optional (daemon sends null for active orders).

57 pytest tests mirroring and extending Go's client_test.go matrix,
including wire-shape pinning and concurrency. ruff + mypy clean.
All 33 routes under /api/v2: chain, blocks, transactions (incl. the
text/plain submit route), addresses, delegations, pools, tokens/NFTs,
orders, statistics and fee rate. HTTPError for status >= 400 with trimmed
body; base URL trailing-slash trimmed.

Lenient numeric decoding per server behavior: uint64 fields arrive as
numbers or strings, margin_ratio_per_thousand carries a trailing '%'.
Strict uint64 parsing (no negatives/underscores), finite per-thousand
values, path segments URL-encoded against traversal, and malformed
payloads surface as IndexerError instead of KeyError. Zero-valued
pagination params are omitted; block-stats from/to always sent.

59 new tests mirroring Go's indexer client_test.go (116 total passing).
ruff + mypy clean.
All 56 daemon routes across management/transactions/tokens/staking/orders
mixins with exact wire fidelity: TxOptions always-both-keys null pattern,
selected_utxos omitempty for nil and empty, OutputValue/CurrencyFilter
custom encodings with client-side validation before any HTTP request,
account_index wire key for lock_token_supply, account dropped for
get_pool_balance, hardcoded Trusted trust policy and Confirmed-only
balances, empty-string-to-null coercion, CoinFilter without content key.

Mnemonic/passphrase fields redacted from dataclass reprs. 101 new tests
mirror Go's wallet client_test.go and orders_test.go (217 total passing);
wallet package at 100% statement coverage. ruff + mypy clean.
Top-level mintlayer.Client(Config) constructs only the sub-clients whose
URL is set, with lazy WASM init (init_wasm) and convenience re-exports
mirroring the Go SDK root package (Amount, Network, sighash/source
constants).

Examples ported from go-sdk/examples: send_coins.py (full manual
transaction flow: key derivation, UTXO fetch, tx build, witness signing,
submit) and issue_token.py (wallet-daemon token issuance + mint).

15 new top-level client tests (232 total passing). ruff + mypy clean.
Port all 7 Go SDK guides to Python (node, indexer, wallet, wasm,
transactions, staking, tokens) with verified-signature method references,
wire-shape quirks and security guidance; full README with working
quick-start and module references.

Wallet params dataclasses redact mnemonic/passphrase in repr (6
regression tests). Misc: transport-security note in node docs, argv
secrets warning in examples, dead code removal. 238 tests passing;
ruff + mypy clean.
Node and wallet _core mixins repeated _call/_call_ignore/close over
JSONRPCClient; hoist them into a shared base in _jsonrpc.py. Also
replaces node/_core's function-local Amount import with a module-level
import (no cycle: node/types.py has no intra-package imports).
IndexerCore only aliased IndexerHTTP.get/post_text as _get/_post_text.
Mixins now subclass IndexerHTTP directly and call the transport methods;
the _seg path-segment encoder moves next to the HTTP layer it serves.
block.py re-exported Transaction with noqa F401 but nothing imports it
from there (public path is mintlayer.indexer.Transaction). Drop the
noqa comments on JSONRPCError imports: both modules list it in
__all__, so F401 never fires.
Baseline: 77.5% (WASM module undertested in-suite; node/indexer/wallet
at 96-100%). Threshold target: >80%.
The wasm-bindgen callee takes ownership of the externref table slots
referenced by passArrayJsValueToWasm0 index arrays and deallocs them
itself; the host's post-call _dealloc_indices therefore double-freed
free-list entries, corrupting later allocations ('array contains a value
of the wrong type' on subsequent multi-element calls to
estimate_transaction_size / encode_signed_transaction_intent /
verify_transaction_intent).

Host-side slot release is now restricted to pre-call rollback (where the
callee never ran); post-call, only the host-malloc'd Uint8Array backing
buffers are freed (the callee to_vec-copies and never frees the
originals). Stale docstrings corrected; ownership contract documented,
including the known wasm-bindgen failing-call slot leak.
148 new offline tests covering the full wasm client: lifecycle (close,
integrity pin fail-closed, unknown exports, memory errors, result-buffer
zeroing via memory spy), the complete Go client_test.go matrix plus
change-key/equality chains, all encode_input_for_* / encode_output_* /
timelocks / fees with real mintlayer-core test vectors (VRF key), ids on
real encoded inputs, transaction encode/sign/decode flows, witness and
HTLC paths, intent roundtrips, types wire shapes, and fault-injected
rollback branches (mutation-verified).

Regression suite for the slot-ownership fix: multi-destination
estimate calls, mixed-load table-stability stress, and dealloc rollback
semantics. 386 tests passing; total coverage 95.5% (wasm/_core 92.5%,
all other wasm modules 100%).
Mirrors go-sdk's ci.yml structure adapted to Python/uv:
- lint job: ruff check + scoped format check + mypy
- test matrix on Python 3.10-3.13 with pytest-cov and a hard
  --cov-fail-under=80 gate (current: 95.5%)
- build job: sdist/wheel + smoke test that the vendored wasm binary and
  its sha256 pin ship in the wheel
- least-privilege permissions (contents: read), SHA-pinned actions,
  persist-credentials: false, cache keyed on pyproject.toml (uv.lock is
  gitignored), concurrency cancellation, per-job timeouts
alibaba/open-code-review v1.12.0 (SHA-pinned) on pull_request events,
reviewing with glm-5.3-flash via the z.ai API using the OCR_LLM_TOKEN
secret. Fork PRs are skipped cleanly (no secret exposure); concurrency
grouped per PR; npm CLI additionally pinned via ocr_version.
The pinned-version install flow leaves the ocr binary unresolvable in
the run step (exit 127). Restore byte parity with the known-working
rust-sdk code-review.yml (@latest install).
@github-actions

github-actions Bot commented Sep 17, 2026

Copy link
Copy Markdown

🔍 OpenCodeReview found 21 issue(s) in this PR.

  • ✅ Successfully posted inline: 5 comment(s)
  • 📋 Routed to summary by policy: 12 comment(s)
  • ⏭️ Skipped (overlap with history): 4 comment(s)

⚠️ 2 warning(s) occurred during review.


maintainability · low

📄 mintlayer/indexer/_http.py (L22-L23)

⚠️ GitHub could not post this as an inline comment: Routed to summary (severity low · category maintainability)

HTTPError does not inherit from IndexerError, so error handling is split across two unrelated exception types: a caller writing except IndexerError will still let HTTP-level failures (e.g. 404 from get_tip/get_block) escape. Consider deriving HTTPError from IndexerError (or a common base) so a single except IndexerError covers all client failures.

💡 Suggested Change

Before:

class HTTPError(Exception):
    """Non-2xx HTTP response from the indexer."""

After:

class HTTPError(IndexerError):
    """Non-2xx HTTP response from the indexer."""

maintainability · low

📄 mintlayer/indexer/number.py (L54-L56)

⚠️ GitHub could not post this as an inline comment: Routed to summary (severity low · category maintainability)

data.strip('"') strips all leading/trailing quote characters rather than removing a single pair of surrounding JSON quotes; e.g. '""50""' or '"50%%' are silently accepted. Since the string here is never actually JSON-quoted (it comes from resp.json(), which already decoded quotes), the strip appears to be dead leniency — parsing the value directly (after removing a trailing '%') would be clearer and more predictable.

💡 Suggested Change

Before:

        stripped = data.strip('"')
        if stripped.endswith("%"):
            stripped = stripped[:-1]

After:

        stripped = data.rstrip("%") if data.endswith("%") else data

maintainability · low

📄 mintlayer/client.py (L156-L159)

⚠️ GitHub could not post this as an inline comment: Routed to summary (severity low · category maintainability)

The wasm property releases self._mu before returning, so a concurrent close() can tear down the WASM runtime (sets memory/table/_instance to None) while the caller still holds a reference to the closed client. This fails loudly rather than crashing (subsequent calls raise WasmError("mintlayer: client is closed")), but the returned handle outlives the lock's protection window and the race window is not covered by the docstring's "safe for concurrent use" claim. Consider documenting this lifetime contract on the property.

💡 Suggested Change

Before:

        with self._mu:
            if self._wasm is None:
                raise WasmError("mintlayer: init_wasm() must be called before using client.wasm")
            return self._wasm

After:

        with self._mu:
            if self._wasm is None:
                raise WasmError("mintlayer: init_wasm() must be called before using client.wasm")
            wasm = self._wasm
        # Note: a concurrent close() may shut the returned client down after
        # the lock is released; use of a closed client raises WasmError.
        return wasm

bug · low

📄 mintlayer/_jsonrpc.py (L142-L142)

⚠️ GitHub could not post this as an inline comment: Routed to summary (severity low · category bug)

When the daemon returns a JSON-RPC error object that omits code or message, this fabricates the defaults 0 and "". A missing code becomes the valid-looking 0, which callers matching on RPCError.code may misinterpret as a server-defined code, and an empty message hides the actual failure reason. Since the surrounding code already rejects a non-dict error object as a malformed shape (JSONRPCError), a dict missing these required JSON-RPC fields should be treated the same way rather than silently filled in.

💡 Suggested Change

Before:

            raise RPCError(error.get("code", 0), error.get("message", ""))

After:

            code = error.get("code")
            message = error.get("message")
            if not isinstance(code, int) or isinstance(code, bool) or not isinstance(message, str):
                raise JSONRPCError("decode response: JSON-RPC error object has unexpected shape")
            raise RPCError(code, message)

maintainability · low

📄 mintlayer/wallet/__init__.py (L121-L122)

⚠️ GitHub could not post this as an inline comment: Routed to summary (severity low · category maintainability)

all omits public types that appear in returned results: MnemonicContent (via CreateWalletResult.mnemonic.content), WalletExtraInfo (via WalletInfo.extra_info), OutputValue (via ActiveOrder/OwnOrder), and TxStats (via TxInspection.stats). Users cannot import these names from the package for typing or introspection even though they are part of the public API surface; export them alongside the other types.

💡 Suggested Change

Before:

    "MnemonicResult",
    "NFTMetadata",

After:

    "MnemonicContent",
    "MnemonicResult",
    "NFTMetadata",
    "OutputValue",
    "TxStats",
    "WalletExtraInfo",

maintainability · low

📄 .github/workflows/ci.yml (L47-L48)

⚠️ GitHub could not post this as an inline comment: Routed to summary (severity low · category maintainability)

mypy runs only on mintlayer/ and ruff format check only on mintlayer/ tests/ examples/, while ruff check . covers everything. Type errors and API drift in examples/ (shipped documentation) are never caught; an example calling a renamed/removed SDK method can merge undetected. Consider adding mypy for examples/ or at least an import/smoke check.


maintainability · low

📄 .github/workflows/publish.yml (L109-L111)

⚠️ GitHub could not post this as an inline comment: Routed to summary (severity low · category maintainability)

publish-testpypi has no pre-flight guard for TestPyPI's immutable-version rule: re-pushing a tag for an already-published version fails mid-pipeline with a confusing registry error rather than failing fast in build, next to the existing tag/version match check.


other · low

📄 .gitignore (L17-L19)

⚠️ GitHub could not post this as an inline comment: Routed to summary (severity low · category other)

Broad ignore patterns (*.key, *.pem, *.seed, secrets/, .env*) could silently exclude legitimate files (e.g., future test fixture keys) from commits and wheels; there is no !tests/ negation. Currently no tracked files match, but consider scoping these patterns or adding negations for test/example directories.


bug · low

📄 examples/send_coins.py (L230-L234)

⚠️ GitHub could not post this as an inline comment: Routed to summary (severity low · category bug)

If the fee rate loop hits the non-convergence branch (the for...else), the subsequent build() may raise ValueError for insufficient balance, but at that point tx and size from the last iteration have already been produced and the earlier except ValueError handler exits — that is handled. However, if build() succeeds here, the stale size used to recompute needed was measured against the previous outputs blob; when the previous build dropped dust change (change <= DUST_THRESHOLD_ATOMS) the new outputs blob differs, so the reported/charged fee can be inconsistent with the actual final transaction. Recomputing needed from the size of the just-built transaction (i.e. one extra iteration) would keep fee and size consistent.


test · low

📄 tests/test_wire_shapes.py (L123-L128)

⚠️ GitHub could not post this as an inline comment: Routed to summary (severity low · category test)

client.close() is only reached if both assertions pass. If an assert fails (or invoke(client) raises), the client's HTTP resources are never released. Wrap the body in try/finally so the client is closed on every path, mirroring the teardown guarantee the rpc_server fixture provides for the server side.

💡 Suggested Change

Before:

    srv = rpc_server(result=None)
    client = Client(srv.url)
    invoke(client)
    assert srv.capture.method == rpc_method
    assert srv.capture.params == expected_params
    client.close()

After:

    srv = rpc_server(result=None)
    client = Client(srv.url)
    try:
        invoke(client)
        assert srv.capture.method == rpc_method
        assert srv.capture.params == expected_params
    finally:
        client.close()

test · low

📄 tests/conftest.py (L50-L53)

⚠️ GitHub could not post this as an inline comment: Routed to summary (severity low · category test)

Capture fields (method, params, request_count, payloads, ...) are written under self._lock but read directly by tests without it. Under CPython's GIL this is currently safe for simple reads, but the asymmetric locking is fragile if tests ever issue concurrent client calls and read state during writes. Consider exposing accessor methods (or a snapshot() method) that take the lock, so synchronization is enforced at the Capture API level.


test · low

📄 tests/conftest.py (L125-L128)

⚠️ GitHub could not post this as an inline comment: Routed to summary (severity low · category test)

ServerHandle.stop() ignores the return value of thread.join(timeout=5); a hung handler thread would silently linger as a daemon thread with no diagnostic. Consider logging or asserting on join completion (thread.is_alive() after join) so leaks are visible when debugging test shutdown failures.


⚠️ Warnings:

  • mintlayer/indexer/__init__.py (token_budget_reached): skipped round 2 of group "mintlayer/indexer/init.py,mintlayer/indexer/_http.py,mintlayer/indexer/address.py,mintlayer/indexer/block.py,mintlayer/indexer/chain.py,mintlayer/indexer/delegation.py,mintlayer/indexer/number.py,mintlayer/indexer/order.py,mintlayer/indexer/pool.py,mintlayer/indexer/statistics.py": used 551114 tokens exceeds budget 500000
  • tests/test_chainstate.py (token_budget_reached): stopped dispatch: used 551114 tokens + group estimate 189680 = projected 740794 exceeds budget 500000

Comment thread examples/send_coins.py Outdated
Comment thread examples/send_coins.py Outdated
Comment thread examples/send_coins.py Outdated
Comment thread mintlayer/client.py
Comment thread mintlayer/client.py
Comment thread mintlayer/wallet/staking.py Outdated
Comment thread mintlayer/wasm/host.py
Comment thread mintlayer/wasm/host.py
Comment thread mintlayer/wasm/host.py
Comment thread mintlayer/wasm/types.py
Library fixes:
- Config.password redacted from repr (same treatment as wallet params)
- Client.close() now also closes node/indexer/wallet sessions
- indexer: get_token_statistics URL-encodes token id (_seg);
  submit_transaction and get_pool_block_stats validate response shape
  into IndexerError; from_json wrapper also catches AttributeError;
  parse_per_thousand rejects invalid types with IndexerError
- wallet: _call_model_list validates shape (null -> [] Go nil-slice
  parity, non-list -> JSONRPCError); staking_status simplification
- wasm host: signal_exception records the host-side message in the
  per-call side channel (first-wins; cast_2 still overwrites with the
  richer Rust message); _subarray clamps the range like JS
  TypedArray.subarray (closes OOB view arithmetic)
- wasm _core: WasmError preserves the cause chain (from exc instead of
  from None); zero-length return buffers now uniformly read/zero/free

Examples:
- send_coins: mnemonic via env/hidden prompt (no argv requirement),
  Coin-only UTXO selection with warnings, fatal re-encode failure
  instead of silent 0x00 downgrade, change output + iterative fee
  estimation from the indexer fee rate
- issue_token: poll the indexer for issuance confirmation (404-tolerant,
  bounded by --wait-timeout) before minting

Meta: hatchling>=1.26 floor (PEP 639 fields), wheel-glob guard in CI,
gitignore dedupe + .env.example negation.

20 new regression tests (406 total, 95.7% coverage); ruff + mypy clean.

Rejected findings (documented): cast_2 last-wins error capture (Go/wasm-
bindgen error convention), lazy Amount validation (Rust validates at
encode time), strict response-id matching (sync client + JSON-RPC null-id
error envelopes), naive-datetime handling (already documented).
Comment thread .github/workflows/ci.yml Outdated
Comment thread .github/workflows/code-review.yml
Comment thread examples/issue_token.py Outdated
Comment thread examples/send_coins.py
Comment thread examples/send_coins.py Outdated
Comment thread mintlayer/node/_core.py Outdated
Comment thread mintlayer/node/_core.py Outdated
Comment thread mintlayer/node/p2p.py Outdated
Comment thread mintlayer/wasm/_core.py
Comment thread mintlayer/wasm/_core.py
- CI reproducibility: commit uv.lock, sync --locked in all jobs, cache
  keyed on uv.lock
- _jsonrpc: non-dict JSON-RPC error object -> JSONRPCError (was raw
  AttributeError)
- node: strict _call_opt_int (no bool/float/str coercion) and
  _call_str_list (list-of-strings validation, None -> [])
- p2p: _duration_to_wire integer math (no float precision loss; old code
  could emit nanos == 1e9)
- wasm: return-count guards in the bool/u32/u64 helpers (consistent with
  the string/bytes helpers)
- examples: issue_token password via $WALLET_PASSWORD / hidden prompt;
  send_coins --mnemonic help text warns about ps/shell-history exposure

Rejected (documented): code-review.yml pull-requests:write is required
to post reviews; send_coins fee-loop 'oversized fee' claim is incorrect
(the remainder always goes to the change output); naive-datetime
behavior is documented Go parity; __wbindgen_free align=1 matches the
JS glue and Go free convention (dlmalloc ignores the align hint).
Comment thread examples/issue_token.py
Comment thread examples/send_coins.py
Comment thread examples/send_coins.py Outdated
Comment thread mintlayer/indexer/pool.py
Comment thread mintlayer/node/_core.py Outdated
Comment thread mintlayer/wallet/types.py
Comment thread mintlayer/wasm/_core.py
Comment thread mintlayer/wasm/addresses.py
Comment thread mintlayer/wasm/host.py Outdated
Comment thread mintlayer/wasm/intent.py
- examples/issue_token: treat confirmations "" and "0" as unconfirmed
  (the field is a string; a bare truthiness check counted "0")
- examples/send_coins: guard the fee-convergence loop (ValueError ->
  log.fatal), estimate size from the encoded INPUTS blob per the
  estimate_transaction_size contract, allow exact-sweep change == 0
- node: strict _call_opt_str (parity with _call_str; no silent str()
  coercion of numbers/objects)
@nullPointerEnjoyer

Copy link
Copy Markdown
Contributor Author

OCR findings triage — all rounds addressed

Round 1 (27 findings): 22 fixed, 4 rejected, 1 already documented

Fixed: Config password repr redaction; Client.close() closes sub-client sessions; get_token_statistics path encoding; submit_transaction/get_pool_block_stats shape validation into IndexerError; from_json wrapper catches AttributeError; parse_per_thousand invalid types; StakingStatus direct lookup; signal_exception records the host message; _subarray JS-semantics clamping; WasmError cause-chain preservation; zero-length buffer free parity; SimpleCurrencyAmount invariant; _call_model_list shape validation; wheel-glob guard; hatchling floor; .gitignore dedupe/!.env.example; all three example hardenings (mnemonic env/prompt, Coin-only selection + fatal re-encode, change output + fee estimation, confirmation polling).

Rejected (with reasoning):

  1. _cast_string last-wins error capture — matches Go and the wasm-bindgen convention (the error Display string is the final cast before the flag check); first-wins would report stale benign strings.
  2. Eager Amount.atoms validation — Go validates lazily in Rust at encode time with precise messages; eager validation breaks the Amount("") zero-value pattern and message parity.
  3. Strict JSON-RPC response-id matching — sync client (one request/response per call) cannot cross responses; error envelopes may carry id: null per the JSON-RPC spec, making strict matching a footgun. Go explicitly does not validate.
  4. Naive-datetime rejection in get_pool_block_stats — behavior is documented in the docstring and matches Go's time.Time handling.

Round 2 (12 findings): 8 fixed, 4 rejected

Fixed: uv.lock committed + uv sync --locked in CI (reproducible installs, cache keyed on the lockfile); non-dict JSON-RPC error object → JSONRPCError; strict _call_opt_int/_call_str_list/_call_opt_str; integer duration wire math (old float code could emit nanos == 1e9); return-count guards in bool/u32/u64 helpers; issue_token password via $WALLET_PASSWORD/hidden prompt; --mnemonic help-text warning.

Rejected: code-review.yml pull-requests: write is required to post reviews (inherent); "oversized fee" claim is incorrect — the remainder always goes to the change output; naive-datetime (see above); __wbindgen_free align=1 matches the JS glue's own free convention and the working Go SDK (dlmalloc reads the block header, ignoring the align hint).

Round 3 (5 findings): 3 fixed, 2 rejected

Fixed: confirmations string check ("" and "0" = unconfirmed); fee-loop guarded + size estimated from the inputs blob per contract + exact-sweep allowed; strict _call_opt_str.

Rejected: single-key assumption is incorrect — UTXOs at a pubkeyhash address are only spendable by that address's key; fee-loop guard was already present (stale diff context).

Each round's changes were gated on security + code review agents and the full suite: 406 tests, 95.7% coverage, ruff + mypy clean.

Comment thread examples/issue_token.py
Comment thread examples/issue_token.py
Comment thread mintlayer/wallet/__init__.py
Comment thread mintlayer/wallet/staking.py Outdated
Comment thread mintlayer/wasm/_core.py
Comment thread mintlayer/wasm/inputs.py
Comment thread mintlayer/wasm/intent.py
Comment thread mintlayer/wasm/outputs.py
Comment thread mintlayer/wasm/signing.py
Comment thread mintlayer/wasm/transactions.py
Comment thread .github/workflows/publish.yml Outdated
Comment thread .github/workflows/publish.yml
Comment thread .github/workflows/publish.yml
Comment thread mintlayer/_jsonrpc.py
Comment thread mintlayer/indexer/number.py
Comment thread mintlayer/wallet/management.py
Comment thread mintlayer/wasm/addresses.py
Comment thread mintlayer/wasm/host.py Outdated
Comment thread mintlayer/wasm/transactions.py
Comment thread tests/conftest.py
@erubboli
erubboli self-requested a review September 18, 2026 07:32
- host.py: populate the externref slot passed to __wbindgen_exn_store with
  the exception message (was passing an empty slot); fail loudly in
  fill_random when the memory write fails (stale bytes as key material)
- _jsonrpc.py: refuse basic-auth credentials over cleartext http:// to
  non-loopback hosts (fail fast at client construction)
- indexer/number.py: reject strings outside the uint64 range (incl. the
  20-digit boundary) with IndexerError instead of a bare int() failure
- wallet/management.py: raise a clear JSONRPCError on null results in
  new_address / reveal_public_key instead of TypeError
- wasm/_core.py: document the verified memory-ownership protocol; the
  OCR-reported input-buffer 'leak' is a false positive - plain inputs are
  callee-owned (Rust String/Vec<u8> by value), confirmed against the
  wasm-bindgen glue of the byte-identical vendored binary
- publish.yml: require the exact WASM ABI private-key length, gate the
  PyPI upload on sha256 equality with the TestPyPI dry-run, drop the
  unused id-token permission (attestations disabled under token auth),
  run lint/mypy/tests in the build job
- tests: cleartext-guard, null-result, uint64-range coverage; resilient
  rpc_server fixture teardown
@nullPointerEnjoyer

Copy link
Copy Markdown
Contributor Author

Round-4 OCR triage (70a3781)

The bug · high "input-buffer leak" cluster is a false positive — do not "fix" it. Ownership was verified against the wasm-bindgen JS glue that accompanies the byte-identical vendored binary (sha256 0c5411e3… matches mintlayer/web-gui app/wasm-wrappers):

  • Plain _write_string/_write_bytes inputs: the callee takes ownership (Rust String/Vec<u8> by value) and frees them. The glue never frees them host-side (pubkey_to_pubkeyhash_address's finally frees the result, not the input). Host-side frees would be double frees. Note: naive repeat-call probes are inconclusive here — uniform allocation patterns self-heal double-free freelist entries.
  • String-array args (verify_transaction_intent): slots + index array are callee-owned; nothing to release post-call. The round-3 comment is correct, and go-sdk's post-call freeStringArray/freeUint8ArrayArray are latent double-free bugs in the Go SDK (worth reporting upstream).
  • Byte-array-array args (encode_signed_transaction_intent): slots + index array callee-owned (post-call dealloc traps — verified), only the host-malloc'd backing buffers are freed host-side; intent.py already does exactly this.

mintlayer/wasm/_core.py now documents this protocol with the evidence, so future review rounds stop flagging it.

Fixed in this round:

  • host.py: __wbindgen_exn_store now receives a slot populated with the exception message (was an empty slot → wrong error payload on the unwind path); fill_random fails loudly if the memory write fails (was silently returning stale bytes as key material).
  • _jsonrpc.py: refuse basic-auth over cleartext http:// to non-loopback hosts at construction.
  • indexer/number.py: reject out-of-uint64-range strings (incl. the 20-digit boundary 2^64) with IndexerError instead of a bare int() failure.
  • wallet/management.py: new_address/reveal_public_key raise a clear JSONRPCError on JSON-null results instead of TypeError.
  • publish.yml: exact WASM ABI privkey length asserted; PyPI upload gated on sha256 equality with the TestPyPI dry-run; unused id-token permission dropped (attestations need OIDC trusted publishing — re-enable if migrated); lint/mypy/tests now gate the build job.
  • tests/conftest.py: resilient fixture teardown.

Already fixed in round 3 (stale threads): wallet/staking.py StakingStatus lookup, host.py _subarray clamping, examples/send_coins.py fail-loud UTXO encoding + guarded fee loop + change output.

Status: 418 tests pass, coverage 95.4%, ruff/mypy clean. Earlier bug · high findings (signing.py:36, transactions.py:63, intent.py:44, addresses.py:21, host.py:133, _core.py:168, staking.py, send_coins.py) are either addressed above or false positives per the ownership analysis.

Comment thread examples/send_coins.py
Comment thread mintlayer/indexer/chain.py Outdated
Comment thread mintlayer/node/types.py
Comment thread mintlayer/wasm/_core.py Outdated
Comment thread tests/conftest.py
- indexer/chain.py: get_block_id_at_height returns str | None instead of
  the ambiguous empty-string sentinel (docs updated)
- node/types.py: Amount.from_json validates atoms is a decimal string,
  rejecting JSON numbers that would corrupt round-trips
- wasm/_core.py: WASM integrity check raises WasmError (missing binary or
  pin file) instead of raw FileNotFoundError/IndexError at import time
- examples/send_coins.py: verify the fee still covers the final tx size
  if the convergence loop exits without stabilising
- tests: null block-id, Amount validation coverage; resilient rest_server
  fixture teardown
Comment thread .github/workflows/code-review.yml
Comment thread .github/workflows/publish.yml Outdated
Comment thread examples/send_coins.py Outdated
Comment thread mintlayer/_jsonrpc.py
Comment thread mintlayer/node/p2p.py
Comment thread mintlayer/wasm/_core.py Outdated
- wasm/_core.py: read the WASM binary inside _load_and_verify_wasm so a
  packaging mistake raises WasmError instead of a raw FileNotFoundError
  at import time
- node/p2p.py: _duration_to_wire rejects negative timedeltas (Python's
  normalisation would silently ban for ~1 year instead of the intent)
- _jsonrpc.py: fail loudly on JSON-RPC response id mismatch instead of
  misattributing another call's payload
- examples/send_coins.py: start the fee loop from a zero fee so the
  first build only requires balance >= amount (a full-1KB start fee
  aborted affordable sends)
- publish.yml: publish-pypi reuses the EXACT dist/ artifact from the
  successful tag-push run via gh run download (rebuilds are not
  byte-reproducible; sha256 comparison removed as it could spuriously
  fail); adds actions:read
- code-review.yml llm_auth_token-as-input finding: considered and
  declined - it is the action's documented input and GitHub auto-masks
  secret values in logs
- tests: updated for the new integrity API; negative-duration, id-
  mismatch and null-id coverage
Comment thread .github/workflows/ci.yml
Comment thread .github/workflows/publish.yml
Comment thread examples/issue_token.py Outdated
Comment thread examples/send_coins.py Outdated
Comment thread examples/send_coins.py Outdated
Comment thread mintlayer/indexer/__init__.py
Comment thread mintlayer/indexer/types.py Outdated
Comment thread mintlayer/node/types.py Outdated
Comment thread mintlayer/wasm/host.py
Comment thread mintlayer/wasm/outputs.py
CRITICAL fix verified against pinned wire fixtures:
- examples/send_coins.py: the indexer UTXO output is a tagged union
  ({"Transfer": {...}}), not {"type": ..., "value": {...}} - the example
  would never find spendable UTXOs against a real indexer. Rewritten to
  the actual shape (tests/test_indexer_address.py fixtures)
- examples/issue_token.py: only HTTP 404 counts as 'not indexed yet';
  5xx/transport failures now propagate instead of masking a server error
- publish.yml: sha256 verification of the downloaded artifact against
  TestPyPI's published digests (the dry-run uploads are the trust anchor)
- wasm/outputs.py: python-side supply_amount/total_supply invariant check
- node/types.py: _require_int validation for timestamps/block heights
  (int(1.9) truncation, bool/str rejection)
- indexer/types.py: Amount.from_json requires atoms/decimal keys instead
  of silently decoding truncated payloads to empty strings
- client.py: wasm property reads under the init lock (consistent with
  init_wasm/close)
- indexer/__init__.py: thread-safety docstring clarified (shared session
  cookie jar is not synchronised)
- wasm/host.py: _new_with_length must NOT free its backing store - the
  module caches such Uint8Arrays in externref table slots (RNG scratch,
  verified by test) so a call-end free is a use-after-free. OCR 'leak'
  declined with evidence; a regression test pins the caching behaviour
- ci.yml per-leg mypy finding: declined (python_version=3.10 floor
  already configured; matrix-wide mypy quadruples CI time for marginal
  value)
Comment thread examples/send_coins.py
Comment thread mintlayer/_jsonrpc.py
Comment thread mintlayer/wasm/_core.py
Comment thread mintlayer/wasm/_core.py
Comment thread mintlayer/wasm/_core.py Outdated
- _jsonrpc.py: a JSON null (or missing) response id on a success payload
  is now rejected - JSON-RPC 2.0 reserves null ids for server-side error
  notifications, so such payloads cannot be attributed to this call
- wasm/_core.py: externref index-array writes now check the memory.write
  return (consistent with _write_bytes); a failed write triggers the
  existing pre-call rollback instead of leaving a garbage index array
- declined, with rationale documented in code: freeing result buffers on
  fallible-call error paths (reference glue zeroes rather than frees -
  ret[0] is not a valid allocation when the error flag is set);
  _read_amount bypassing _call (mirrors go-sdk; _call would reset the
  parent call's captured error state); speculative token-Transfer shape
  hardening in the send_coins example
- tests: null-id expectation flipped to match the tightened contract
Comment thread .github/workflows/publish.yml
Comment thread examples/issue_token.py
Comment thread examples/send_coins.py
Comment thread mintlayer/_jsonrpc.py
Comment thread mintlayer/client.py Outdated
Comment thread mintlayer/wallet/__init__.py
Comment thread mintlayer/wasm/_core.py
- wallet/__init__.py: export OwnOrder, Outpoint, OutpointSourceID,
  RevealPublicKeyResult, OrderState, TxStats and Timestamp (types needed
  to use the public API)
- client.py: close already-created sub-clients if a later constructor
  fails (e.g. cleartext-credential guard) instead of leaking sessions
- _jsonrpc.py: document the actual thread-safety contract of the shared
  session (synchronised id/flow, cookie-less daemons, urllib3 pool)
- examples/send_coins.py: drop dust change outputs below a documented
  threshold (remainder goes to fees) with a warning
- publish.yml: distinguish 'artifact missing on TestPyPI' from 'digest
  mismatch' in the integrity gate
- declined: OCR bug-high claiming HTTPError is not exported - it is
  (mintlayer/indexer/__init__.py exports and documents it)
- _free_wasm: document why cleanup failures are swallowed
Comment thread .github/workflows/publish.yml Outdated
Comment thread mintlayer/client.py
Comment thread mintlayer/node/_core.py Outdated
Comment thread mintlayer/wasm/host.py
Comment thread mintlayer/wasm/host.py
- node/_core.py: _call_opt_amount wraps Amount decoding failures into
  JSONRPCError (was a bare ValueError/KeyError, breaking the documented
  error contract)
- publish.yml: scope actions:read to the publish-pypi job only; build
  runs project code and must not carry cross-run artifact access
- declined with rationale: init_wasm holding the lock through
  construction is intentional (no half-built client can escape);
  signal_exception returning normally matches the wasm-bindgen contract
  and go-sdk/JS hosts (the host cannot force an Err return);
  _cast_string error-message capture is inert on success paths
Comment thread mintlayer/client.py
Comment thread mintlayer/indexer/types.py Outdated
Comment thread mintlayer/node/_core.py Outdated
Comment thread mintlayer/wasm/_core.py
- indexer/number.py: parse_per_thousand converts OverflowError (float of
  a hostile oversized int) into IndexerError; the _safe_from_json
  wrapper also catches OverflowError
- node: new _decode_model helper converts malformed-payload decode
  failures into the documented JSONRPCError contract at all typed
  call sites (chainstate_info, token/tokens/order info, orders info by
  currencies, mempool tx/fee rates); one test expectation updated to
  the converted exception type
- wasm/_core.py: _load_and_verify_wasm docstring no longer contradicts
  the import-time fail-fast design
- declined: wasm property TOCTOU vs concurrent close() - inherent to
  the property API; the failure is loud (client is closed) by design,
  and the lock already prevents the half-constructed escape
Comment thread .github/workflows/publish.yml
Comment thread examples/send_coins.py
Comment thread mintlayer/node/p2p.py
Comment thread mintlayer/node/p2p.py
Comment thread mintlayer/wallet/staking.py
Comment thread mintlayer/wallet/types.py
- node/_core.py: _call_opt_str rejects non-string results instead of
  silently coercing dicts/numbers to garbage strings (all three callers
  are documented string endpoints; get_block_json covers object payloads)
- wallet/staking.py: get_staking_status raises JSONRPCError with the
  valid enum values instead of a bare ValueError on unknown statuses
- wallet/types.py: MnemonicResult.from_json validates content (non-dict
  and empty dicts are malformed) and wraps kwargs TypeErrors into a
  descriptive ValueError
- tests: coverage for all three guards (6 new tests)
Comment thread examples/issue_token.py
Comment thread mintlayer/client.py
Comment thread mintlayer/indexer/pool.py
Comment thread mintlayer/node/p2p.py
Comment thread mintlayer/node/p2p.py
Comment thread mintlayer/wasm/host.py
Comment thread mintlayer/wasm/staking.py
Comment thread mintlayer/wasm/transactions.py
- examples/issue_token.py: wallet sync failures catch RPC/JSON-RPC errors
  specifically instead of Exception (programming errors stay loud)
- indexer/pool.py: document naive-datetime interpretation for block stats
Comment thread .github/workflows/publish.yml
Comment thread .github/workflows/publish.yml
Comment thread examples/issue_token.py
Comment thread mintlayer/node/types.py
Comment thread mintlayer/node/types.py
@erubboli
erubboli merged commit bb02c1a into main Sep 18, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants