Wallet reorg, async helper test, drop dead MANIFEST.in / old_tests

[fpelliccioni]

Summary

Three independent cleanups bundled into one PR. None of them are functional changes — the C extension still exposes the same surface. All 6 tests pass locally.

1. Move payment_address.{h,cpp} from chain/ to wallet/

The Knuth C-API has always had `kth/capi/wallet/payment_address.h` under `wallet/`, not `chain/`. py-native carried the historical misclassification. Move both the wrapper header and the wrapper implementation to mirror the upstream layout:

• `include/kth/py-native/chain/payment_address.h` → `include/kth/py-native/wallet/payment_address.h`
• `src/chain/payment_address.cpp` → `src/wallet/payment_address.cpp`

Includes in `src/module.c` and `src/wallet/payment_address.cpp`, plus the source list in `setup.py`, follow.

2. Salvage the `async_exec` helper from `old_tests/`, drop the rest

`old_tests/` had two stale Python "tests" — really standalone scripts, not pytest tests:

• `test_native_py` (715 lines) — first ~165 dup the printers in `examples/settings.py`, the `main()` duplicates the node bootstrap covered by `tests/test_node.py`, and the last ~550 lines are commented-out JavaScript (a historical reference catalog of API calls).
• `test_native_async_py` (210 lines) — mostly the same printers again, but with a real gem hiding in the middle: an `async_exec()` helper that wraps the kth_native callback API (`callback(error, *result)`) into an awaitable via `asyncio.Future` + `call_soon_threadsafe`. Idiomatic Python bridge that downstream consumers (py-api) will want to copy.

Extract `async_exec` into a new `tests/test_node_async.py` and delete `old_tests/` entirely.

The new file is a unit test of the helper, not a node test: a single Python process can only bootstrap one Knuth node — global logging/thread state from the first `node_construct/destruct` cycle doesn't reset, the second `construct` trips on it. So we drive `async_exec` with three synthetic callable shapes:

• inline callback (sync, on the calling thread)
• threaded callback (fired from a worker thread → `call_soon_threadsafe` brings it back)
• error path (`pick=0` to grab the error code)

`tests/test_node.py` keeps covering the live-node path. No new dependency: just `asyncio.run()` from a regular sync test, no `pytest-asyncio` needed.

3. Delete `MANIFEST.in` (redundant + wrong)

`setuptools-scm` picks up every git-tracked file for the sdist automatically. The `MANIFEST.in` we had was both redundant and incorrect:

• Referenced `version.py` (deleted in the previous PR — versioning is now `setuptools-scm`-driven)
• Referenced the old `chain/payment_address.h` location (moved by this PR)
• Listed nonexistent paths: `chain/.h`, `p2p/.h`, `doc/`, `test/`
• Listed build artifacts (`.so`, `.dll`, `*.dylib`) that have no business in the sdist

Drop it. Verified the sdist still ships 111 files including `tests/`, `examples/`, `kth_native.pyi`, `py.typed`, `src/wallet/payment_address.cpp`, and the `include/wallet/` directory.

Test plan

• `pip install --no-build-isolation -e .` (succeeds after reorg)
• `pytest tests/ -v` → 6 passed (3 config + 1 live-node + 3 async helper)
• `python -m build --sdist` → 111 files, all the right ones present
• CI green

Summary by CodeRabbit

Release Notes

• Chores

  • Updated native extension build configuration and reorganized internal module structure.
  • Removed legacy packaging directives and test scripts.

• Tests

  • Added asyncio integration tests for callback-based API validation patterns.

[coderabbitai]

📝 Walkthrough

Walkthrough

The changes involve removing packaging directives from MANIFEST.in, deleting two legacy test scripts, relocating payment_address.cpp from the chain to wallet directory with corresponding header updates, and introducing a new async test module that validates asyncio callback-to-promise bridging patterns.

Changes

Cohort / File(s)	Summary
Package Configuration MANIFEST.in	Removed 28 lines of packaging include directives for shared libraries, native headers/sources, documentation, and test artifacts.
Legacy Test Removal old_tests/test_native_async_py, old_tests/test_native_py	Deleted two legacy test scripts that performed node initialization, configuration inspection, and signal-based shutdown (926 lines total).
Payment Address Relocation setup.py, src/module.c, src/wallet/payment_address.cpp	Moved payment_address implementation from chain to wallet directory; updated corresponding build source and include directives to reference the new location.
New Async Test Module tests/test_node_async.py	Added new test module (101 lines) introducing async_exec helper that bridges callback-based APIs into awaitable futures, with unit tests validating both synchronous and threaded callback scenarios.

Sequence Diagram

sequenceDiagram
    participant Test
    participant async_exec
    participant Callable as Callable/Function
    participant Callback as Callback Closure
    participant EventLoop as Event Loop
    participant Future

    Test->>async_exec: async_exec(func, pick, *args)
    async_exec->>Future: Create asyncio.Future()
    async_exec->>Callable: Call func(callback, *args)
    Callable->>Callback: Invoke callback(error, *result)
    Callback->>EventLoop: loop.call_soon_threadsafe(set_result, args[pick])
    EventLoop->>Future: Set result from callback args
    async_exec->>Future: await future
    Future->>async_exec: Return args[pick]
    async_exec->>Test: Return awaited value

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 Old tests hop away to rest,
Payment address takes a new nest,
Async bridges built so neat,
Callback futures bittersweet,
A rabbit's cleanup, code refined! ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the three main changes: wallet reorganization, async helper test addition, and removal of MANIFEST.in and old_tests directory.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch wallet-reorg-and-async-helper

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

tests/test_node_async.py (1)

37-59: Consider adding an optional timeout for production resilience.

The bridge implementation is correct and handles both sync and threaded callback paths via call_soon_threadsafe. However, if the callback never fires (e.g., native code crashes or hangs), the await blocks indefinitely. For a utility intended for "copy verbatim into py-api", consider adding an optional timeout:

♻️ Optional enhancement with timeout

 async def async_exec(
     func: Callable[..., Any],
     pick: int,
     *args: Any,
+    timeout: float | None = None,
 ) -> Any:
-    """Call a kth_native function whose last arg is a result callback,
-    and return the `pick`-th positional argument the callback receives.
-    ...
-    """
+    """Call a kth_native function whose last arg is a result callback,
+    and return the `pick`-th positional argument the callback receives.
+
+    ...
+
+    If `timeout` is set, raises `asyncio.TimeoutError` if the callback
+    does not fire within the specified seconds.
+    """
     future: asyncio.Future[tuple[Any, ...]] = asyncio.Future()
     loop = asyncio.get_running_loop()

     def callback(*cb_args: Any) -> None:
         loop.call_soon_threadsafe(future.set_result, cb_args)

     func(*args, callback)
-    cb_args = await future
+    cb_args = await asyncio.wait_for(future, timeout)
     return cb_args[pick]

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/test_node_async.py` around lines 37 - 59, The helper async_exec should
accept an optional timeout parameter (e.g., timeout: float | None = None) and
use asyncio.wait_for when awaiting the future to avoid hanging if the native
callback never fires; update the signature of async_exec and wrap "cb_args =
await future" with "await asyncio.wait_for(future, timeout)" so a TimeoutError
is raised on timeout, keeping the existing callback/future logic (references:
async_exec function, future variable, loop, and callback inner function).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@tests/test_node_async.py`:
- Around line 37-59: The helper async_exec should accept an optional timeout
parameter (e.g., timeout: float | None = None) and use asyncio.wait_for when
awaiting the future to avoid hanging if the native callback never fires; update
the signature of async_exec and wrap "cb_args = await future" with "await
asyncio.wait_for(future, timeout)" so a TimeoutError is raised on timeout,
keeping the existing callback/future logic (references: async_exec function,
future variable, loop, and callback inner function).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a0dfa4a1-e834-4550-bacb-69a2542735c9

📥 Commits

Reviewing files that changed from the base of the PR and between 206c98b and 7868949.

📒 Files selected for processing (8)

• MANIFEST.in
• include/kth/py-native/wallet/payment_address.h
• old_tests/test_native_async_py
• old_tests/test_native_py
• setup.py
• src/module.c
• src/wallet/payment_address.cpp
• tests/test_node_async.py

💤 Files with no reviewable changes (3)

• MANIFEST.in
• old_tests/test_native_async_py
• old_tests/test_native_py