Skip to content

Conversation

@DanShaders
Copy link
Collaborator

@DanShaders DanShaders commented Nov 24, 2025

The only implemented control query is engine.validator.getActorTextStats. One can run it from inside a script via

actor_stats = await full_node.engine_console.get_actor_stats()

Adding support for more queries is as easy as adding them to a switch in is_engine_console_query in tonlib/tonlib/EngineConsoleClient.cpp and writing a wrapper in test/tontester/src/tonlib/engine_console.py.

@github-actions
Copy link

@codex review

@chatgpt-codex-connector
Copy link

To use Codex here, create a Codex account and connect to github.

Copy link
Collaborator Author

@DanShaders DanShaders left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just an ordinary self-review, nothing to see here.

@DanShaders
Copy link
Collaborator Author

DanShaders commented Nov 24, 2025

  • Extra ninja -C build install recompiles files because of git watcher

@github-actions
Copy link

github-actions bot commented Dec 4, 2025

Findings:

  • tdutils/td/utils/MpscPollableQueue.h:27 now uses std::lock_guard/std::mutex, but the header still only includes <utility> and the old td/utils/SpinLock.h. Translation units that include this header directly (for example tonlib/test/online.cpp) will see 'std::mutex' was not declared' compile errors. Please add <mutex> (and drop the unused SpinLock include) so the header is self‑contained.
  • test/tonlib/test-ffi-event-loop.cpp:1-3 declares only <chrono>, <thread>, <vector> but later uses std::set (…#209). Without including <set> this test will fail to compile. Add the missing header alongside the other standard includes.
  • test/tonlib/test-ffi-awaitable.cpp:13-15 defines static constexpr Tag tags[2]; and immediately takes &tags[2] for continuation_1. Dereferencing the third element of a two-element array is undefined behaviour, even if the pointer is only used for identity comparisons. Use tags + 1 or &tags[1] to stay within bounds.

@github-actions
Copy link

Findings

  • tonlib_event_loop_wait flattens std::optional<Continuation> into a raw pointer and returns nullptr both for “no event” and for an actual Continuation{nullptr} (tonlib/tonlib/tonlib_engine_console.cpp:24-35). FFIEventLoop explicitly supports nullptr continuations (see the NullptrContinuation test), so any caller that ever passes a null continuation to tonlib_response_await_suspend will never see it delivered—the event is indistinguishable from a timeout/cancel and gets dropped. Consider either disallowing null continuations (and validating in tonlib_response_await_suspend) or returning an explicit status so a null continuation can still be observed.

@github-actions
Copy link

  • tonlib/tonlib/EngineConsoleClient.cpp:60-69 – Possible hang on the first query: if AdnlExtClient::on_ready fires between create(...) and the ready_promise being queued, ready_ becomes true but the promise never gets resolved. The coroutine then awaits ready_awaiter indefinitely (until a disconnect triggers on_stop_ready), so initial requests can stall. Consider rechecking ready_ after creating the client and resolving the promise immediately if already ready, or registering the promise before/inside the creation path to avoid this race.

No other blocking issues found in this diff.

Keeping them in tontester.tl creates a dependency cycle between
tontester and tonlib.
This is probably not the right place to do integration testing of the
engine console (we probably want a big comprehensive test of all engine
console methods) but this is good enough for now.
@github-actions
Copy link

  • Missing headers break the new C++ test: test/tonlib/test-ffi-event-loop.cpp:1-3 uses std::set (line 209) and std::atomic (line 227) but only includes <chrono>, <thread>, and <vector>. Builds will fail with “std::set/atomic is not a member of std.” Add <set> and <atomic> includes.

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