From a4376cd942bb31a9dd90801efcaca96ab4505ab5 Mon Sep 17 00:00:00 2001 From: bro Date: Fri, 17 Jul 2026 11:59:50 +0200 Subject: [PATCH 1/7] fix: default peer port --- wallet/core/default_peers.cpp | 56 ++++++++++++++++++++++------------- 1 file changed, 36 insertions(+), 20 deletions(-) diff --git a/wallet/core/default_peers.cpp b/wallet/core/default_peers.cpp index cb7aed448b..40294b3ca9 100644 --- a/wallet/core/default_peers.cpp +++ b/wallet/core/default_peers.cpp @@ -27,49 +27,65 @@ namespace beam { std::vector result; + // In the WASM/browser build every connection must go over wss://, so the + // default peers listen on the WebSocket port (:8200). The raw-TCP P2P + // ports (:8100) used by native builds are unreachable from the browser. +#ifdef __EMSCRIPTEN__ + constexpr const char* kDefaultPort = ":8200"; +#else + constexpr const char* kDefaultPort = ":8100"; +#endif + + auto addPeers = [&result, kDefaultPort](const char* hosts[], uint32_t n) + { + result.reserve(n); + for (uint32_t i = 0; i < n; i++) + result.emplace_back(std::string(hosts[i]) + kDefaultPort); + }; + switch (Rules::get().m_Network) { case Rules::Network::testnet: { - static const char* psz[] = { - "us-nodes.testnet.beam.mw:8100", - "eu-nodes.testnet.beam.mw:8100", - "ap-nodes.testnet.beam.mw:8100" + static const char* hosts[] = { + "us-nodes.testnet.beam.mw", + "eu-nodes.testnet.beam.mw", + "ap-nodes.testnet.beam.mw" }; - Arr2Vec(result, psz, _countof(psz)); + addPeers(hosts, _countof(hosts)); } break; case Rules::Network::mainnet: { - static const char* psz[] = { - "eu-nodes.mainnet.beam.mw:8100", - "us-nodes.mainnet.beam.mw:8100", + static const char* hosts[] = { + "eu-nodes.mainnet.beam.mw", + "us-nodes.mainnet.beam.mw", }; - Arr2Vec(result, psz, _countof(psz)); + addPeers(hosts, _countof(hosts)); } break; case Rules::Network::dappnet: { - static const char* psz[] = { - "eu-node01.dappnet.beam.mw:8100", - "eu-node02.dappnet.beam.mw:8100", - "eu-node03.dappnet.beam.mw:8100" + static const char* hosts[] = { + "eu-node01.dappnet.beam.mw", + "eu-node02.dappnet.beam.mw", + "eu-node03.dappnet.beam.mw" }; - Arr2Vec(result, psz, _countof(psz)); + addPeers(hosts, _countof(hosts)); } break; case Rules::Network::masternet: { - static const char* psz[] = { - "eu-node01.masternet.beam.mw:8100", - "eu-node02.masternet.beam.mw:8100", - "eu-node03.masternet.beam.mw:8100", - "eu-node04.masternet.beam.mw:8100" + static const char* hosts[] = { + "eu-node01.masternet.beam.mw", + "eu-node02.masternet.beam.mw", + "eu-node03.masternet.beam.mw", + "eu-node04.masternet.beam.mw" }; - Arr2Vec(result, psz, _countof(psz)); + addPeers(hosts, _countof(hosts)); } break; From f7933630d2db371a82cdbced85eca9e54f5d6df6 Mon Sep 17 00:00:00 2001 From: bro Date: Fri, 17 Jul 2026 12:02:08 +0200 Subject: [PATCH 2/7] fix: wasmclient MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Password logged in plaintext (CheckPasswordImpl:941) — removed TRACE(pass) from the debug log. dbName and res are still logged. 2. isValidPassword called twice (939-940) — deleted the leftover discarded call; the key-derivation check now runs once. 3. Unsubscribe signed/unsigned OOB (455-465) — added an explicit key < 0 || key >= size guard before any index math, eliminating the size() - 1 underflow path. The last-element case pops; anything else nulls in place. 4. Dead duplicate AddCallback (425-437) — removed. Only Subscribe/Unsubscribe are bound (subscribe/unsubscribe at 1053-1054), and nothing referenced AddCallback. 5. Moved-from recovery filename (StartWallet:555) — added m_CurrentRecoveryFile.clear(); after the std::move, so a subsequent StartWallet without a fresh ImportRecovery sees a deterministically empty filename. --- wasmclient/wasmclient.cpp | 29 +++++++++-------------------- 1 file changed, 9 insertions(+), 20 deletions(-) diff --git a/wasmclient/wasmclient.cpp b/wasmclient/wasmclient.cpp index 2d2c359caa..f9ccdb0324 100644 --- a/wasmclient/wasmclient.cpp +++ b/wasmclient/wasmclient.cpp @@ -422,21 +422,6 @@ class WasmWalletClient m_Client->SendResult(result); } - uint32_t AddCallback(val&& callback) - { - for (uint32_t i = 0; i < m_Callbacks.size(); ++i) - { - auto& cb = m_Callbacks[i]; - if (cb.isNull()) - { - cb = std::move(callback); - return i; - } - } - m_Callbacks.push_back(std::move(callback)); - return static_cast(m_Callbacks.size() - 1); - } - int Subscribe(val callback) { for (uint32_t i = 0; i < m_Callbacks.size(); ++i) @@ -454,11 +439,15 @@ class WasmWalletClient void Unsubscribe(int key) { - if (key == m_Callbacks.size() - 1) + if (key < 0 || key >= static_cast(m_Callbacks.size())) + { + return; + } + if (key == static_cast(m_Callbacks.size()) - 1) { m_Callbacks.pop_back(); } - else if (key < m_Callbacks.size() - 1) + else { m_Callbacks[key] = val::null(); } @@ -552,7 +541,8 @@ class WasmWalletClient if (!m_CurrentRecoveryFile.empty()) { - m_Client->getAsync()->importRecovery(std::move(m_CurrentRecoveryFile)); // m_CurrentRecoveryFile should be cleared + m_Client->getAsync()->importRecovery(std::move(m_CurrentRecoveryFile)); + m_CurrentRecoveryFile.clear(); } m_Client->getAsync()->enableBodyRequests(true); m_Client->start({}, true, additionalTxCreators); @@ -936,9 +926,8 @@ class WasmWalletClient static void CheckPasswordImpl(const std::string& dbName, const std::string& pass, std::shared_ptr cb) { - WalletDB::isValidPassword(dbName, SecString(pass)); auto res = WalletDB::isValidPassword(dbName, SecString(pass)); - BEAM_LOG_DEBUG() << __FUNCTION__ << TRACE(dbName) << TRACE(pass) << TRACE(res); + BEAM_LOG_DEBUG() << __FUNCTION__ << TRACE(dbName) << TRACE(res); auto cbPtr = std::make_unique(std::move(cb), res); emscripten_async_run_in_main_runtime_thread( EM_FUNC_SIG_VI, From 4b5f5d5b2a8792fd1c93ed760356d34f8f703cb8 Mon Sep 17 00:00:00 2001 From: bro Date: Fri, 17 Jul 2026 12:05:42 +0200 Subject: [PATCH 3/7] fix: wasmclient MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 4. Recovery blob left on persistent FS — recovery.bin is seed-equivalent material. Added RemoveRecoveryFile(), called from OnImportRecoveryProgress at completion (done == total || error), so it fires on both success and failure. It uses the fs::remove + FS.syncfs(false) pattern copied from DeleteWallet, uses an error_code overload (no throw), and removes only the literal recovery.bin we wrote — files passed to ImportRecoveryFromFile are untouched. m_CurrentRecoveryFile is already cleared in StartWallet, so no member reset needed here. 5. Unused WalletClient2::m_WalletApi (273) — removed. Confirmed it's dead: the class spans 97-275 and the member appeared only in its own declaration; all m_WalletApi uses (484/489/492/568) belong to WasmWalletClient's own member (still at 1015). --- wasmclient/wasmclient.cpp | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/wasmclient/wasmclient.cpp b/wasmclient/wasmclient.cpp index f9ccdb0324..4e0aaf48bf 100644 --- a/wasmclient/wasmclient.cpp +++ b/wasmclient/wasmclient.cpp @@ -270,7 +270,6 @@ class WalletClient2 std::queue m_Messages; ICallbackHandler* m_CbHandler = nullptr; Callback m_StoppedHandler; - IWalletApi::Ptr m_WalletApi; std::vector m_Apis; }; @@ -645,6 +644,7 @@ class WasmWalletClient { BEAM_LOG_DEBUG() << "Recovery done"; m_RecoveryCallback.reset(); + RemoveRecoveryFile(); } } } @@ -658,6 +658,25 @@ class WasmWalletClient } } + // recovery.bin holds seed-equivalent material; remove it from the + // persistent FS once import has finished (whether it succeeded or failed) + // so it is not left behind in IDBFS. Only the blob we wrote ourselves is + // touched; files passed to ImportRecoveryFromFile are not. + static void RemoveRecoveryFile() + { + std::error_code ec; + fs::remove(std::string(RecoveryFileName), ec); + if (ec) + { + BEAM_LOG_WARNING() << "Failed to remove recovery file: " << ec.message(); + return; + } + EM_ASM + ( + FS.syncfs(false, function() {}); + ); + } + void CreateAppAPI(const std::string& appid, const std::string& appname, val cb) { CreateAppAPI2(kApiVerCurrent, kApiVerCurrent, appid, appname, cb); From e0a3ce2eeeb2ee1780306ec3107bc484ccc60362 Mon Sep 17 00:00:00 2001 From: bro Date: Fri, 17 Jul 2026 12:16:13 +0200 Subject: [PATCH 4/7] wasmclient build MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - #1 ASSERTIONS — CMakeLists.txt:61, -s ASSERTIONS=1 → 0. Drops the pervasive runtime checks/stack-trace glue from the shipping link. - #5 Logger level — wasmclient.cpp:405, BEAM_LOG_LEVEL_DEBUG → BEAM_LOG_LEVEL_INFO (both args). Kills the per-request/per-sync-tick DEBUG spam and its string-formatting cost on the main thread. I chose INFO over WARNING to keep operational logs; one-word change to WARNING if you want it quieter. Pairs with the earlier TRACE(pass) removal. - #6 Dispatch coalescing — wasmclient.cpp:191. Now captures wasEmpty under the lock and only schedules the proxied main-thread call + WeakPtr alloc on the empty→non-empty transition. I verified ProcessMessageOnMainThread (243-265) drains the entire queue in a while(true) loop, so this can't drop messages — worst case is one redundant no-op dispatch. Collapses toggleEvents bursts (N events) from N cross-thread calls + N allocs down to 1. Written -Werror-clean. --- wallet/core/default_peers.cpp | 2 +- wasmclient/CMakeLists.txt | 2 +- wasmclient/wasmclient.cpp | 22 ++++++++++++++++------ 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/wallet/core/default_peers.cpp b/wallet/core/default_peers.cpp index 40294b3ca9..9973d59484 100644 --- a/wallet/core/default_peers.cpp +++ b/wallet/core/default_peers.cpp @@ -36,7 +36,7 @@ namespace beam constexpr const char* kDefaultPort = ":8100"; #endif - auto addPeers = [&result, kDefaultPort](const char* hosts[], uint32_t n) + auto addPeers = [&result](const char* hosts[], uint32_t n) { result.reserve(n); for (uint32_t i = 0; i < n; i++) diff --git a/wasmclient/CMakeLists.txt b/wasmclient/CMakeLists.txt index e647ee749e..11be1fd504 100644 --- a/wasmclient/CMakeLists.txt +++ b/wasmclient/CMakeLists.txt @@ -58,7 +58,7 @@ if(EMSCRIPTEN) -s ALLOW_MEMORY_GROWTH=0 \ -s DYNAMIC_EXECUTION=0 \ -s USE_BOOST_HEADERS=1 \ - -s ASSERTIONS=1 \ + -s ASSERTIONS=0 \ -s PTHREAD_POOL_SIZE='window.navigator.hardwareConcurrency < ${BEAM_WEB_WALLET_THREADS_NUM} ? \ window.navigator.hardwareConcurrency : ${BEAM_WEB_WALLET_THREADS_NUM}' \ -s EXPORT_NAME='BeamModule' \ diff --git a/wasmclient/wasmclient.cpp b/wasmclient/wasmclient.cpp index 4e0aaf48bf..26242aa9b1 100644 --- a/wasmclient/wasmclient.cpp +++ b/wasmclient/wasmclient.cpp @@ -190,15 +190,25 @@ class WalletClient2 void onPostFunctionToClientContext(MessageFunction&& func) override { + bool wasEmpty; { std::unique_lock lock(m_Mutex); + wasEmpty = m_Messages.empty(); m_Messages.push(std::move(func)); } - auto thisWeakPtr = std::make_unique(weak_from_this()); - emscripten_async_run_in_main_runtime_thread( - EM_FUNC_SIG_VI, - &WalletClient2::ProcessMessageOnMainThread, - reinterpret_cast(thisWeakPtr.release())); + // ProcessMessageOnMainThread drains the whole queue, so only dispatch + // on the empty->non-empty transition; messages enqueued after the + // trigger are picked up by the running drain loop. This coalesces + // event bursts (subscribe-all in toggleEvents) into a single proxied + // main-thread call + allocation instead of one per message. + if (wasEmpty) + { + auto thisWeakPtr = std::make_unique(weak_from_this()); + emscripten_async_run_in_main_runtime_thread( + EM_FUNC_SIG_VI, + &WalletClient2::ProcessMessageOnMainThread, + reinterpret_cast(thisWeakPtr.release())); + } } void onStopped() override @@ -402,7 +412,7 @@ class WasmWalletClient } WasmWalletClient(const std::string& dbName, const std::string& pass, const std::string& node, Rules::Network network) - : m_Logger(beam::Logger::create(BEAM_LOG_LEVEL_DEBUG, BEAM_LOG_LEVEL_DEBUG)) + : m_Logger(beam::Logger::create(BEAM_LOG_LEVEL_INFO, BEAM_LOG_LEVEL_INFO)) , m_Reactor(io::Reactor::create()) , m_DbPath(dbName) , m_Pass(pass) From b10a078b582c60364547d032299afa440bcd7cf6 Mon Sep 17 00:00:00 2001 From: bro Date: Fri, 17 Jul 2026 12:33:05 +0200 Subject: [PATCH 5/7] Update default_peers.cpp --- wallet/core/default_peers.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/wallet/core/default_peers.cpp b/wallet/core/default_peers.cpp index 9973d59484..1bc6b9bece 100644 --- a/wallet/core/default_peers.cpp +++ b/wallet/core/default_peers.cpp @@ -58,10 +58,19 @@ namespace beam case Rules::Network::mainnet: { +#ifdef __EMSCRIPTEN__ + // us-nodes.mainnet.beam.mw has no WebSocket (:8200) endpoint, only + // raw TCP (:8100). Listing it in the browser build causes endless + // failed wss reconnect attempts, so restrict to hosts that serve wss. + static const char* hosts[] = { + "eu-nodes.mainnet.beam.mw", + }; +#else static const char* hosts[] = { "eu-nodes.mainnet.beam.mw", "us-nodes.mainnet.beam.mw", }; +#endif addPeers(hosts, _countof(hosts)); } break; From 801c4690b29263a4e3bc3013a6f340c2de875c14 Mon Sep 17 00:00:00 2001 From: bro Date: Fri, 17 Jul 2026 12:51:51 +0200 Subject: [PATCH 6/7] Update wasmclient.cpp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bugs - B1 — MountFS error path (wasmclient.cpp:~1017-1055). OnMountFS now takes (val* nullVal, int success); the JS syncfs callback calls dynCall('vii', $0, [$1, error == null ? 1 : 0]). s_Mounted is set only on success, the bogus pointer-deref is gone (always hands &s_Null, a valid val*), the FS error text is logged to console.error, and the mount promise resolves with a real Error on failure so JS can distinguish it. Also dropped the "mounting..." log (part of I2). - B2 — OnResult reentrancy/UAF (:625). Index-based loop, and the val is copied before invoking, so a callback that synchronously (un)subscribes can't leave the loop on reallocated/freed storage. - B3 — recovery blob leak with null callback (:658). finished is computed independently of the callback; RemoveRecoveryFile() now runs on completion even when ImportRecovery was called without a progress callback. - B4 — stop-time abort (:235-243, :596-600). Confirmed reachable: the wallet thread calls onStopped() after run_ex returns (wallet_client.cpp:920), and queued SendResult messages hold shared_from_this(), so use_count can legitimately exceed 1 during stop. The five fatal Assert(wp.use_count()==N) are replaced with a non-fatal CheckUseCount (logs a warning, doesn't abort) — the custom Assert fires even under -s ASSERTIONS=0, so this was a real crash-on-stop-under-load. shared_ptr still guarantees destruction once the last owner releases, so correctness is unchanged. - B5 — reinterpret_cast(ptr) (:45-51). The emscripten headers aren't available locally (CI-only toolchain), so rather than an untestable EM_FUNC_SIG_VP migration I added a static_assert(sizeof(void*) == sizeof(int)). Correct on wasm32; a future MEMORY64 build now fails to compile at that guard instead of silently truncating pointers, with a comment pointing at the real fix. Improvements - I1 (:434) — Subscribe returns int consistently (size_t loop, static_cast on both returns). - I2 — removed the four debug lifecycle console.logs (wallet created / headless wallet created / wallet deleted / mounting), keeping the functionally-required FS.syncfs calls. - I3 (:~968) — documented why CheckPassword's detached thread is safe (touches only copied strings + static isValidPassword). - I4 (:~1055) — documented the m_WalletApi invariant (created/reset/used only inside makeIWTCall, i.e. the wallet thread) so the earlier de-dup isn't reintroduced as a bug. --- wasmclient/wasmclient.cpp | 116 ++++++++++++++++++++++++-------------- 1 file changed, 75 insertions(+), 41 deletions(-) diff --git a/wasmclient/wasmclient.cpp b/wasmclient/wasmclient.cpp index 26242aa9b1..02e6b4d0bf 100644 --- a/wasmclient/wasmclient.cpp +++ b/wasmclient/wasmclient.cpp @@ -42,8 +42,29 @@ using namespace beam::wallet; #define Assert(x) ((void)((x) || (__assert_fail(#x, __FILE__, __LINE__, __func__),0))) +// Pointers are marshalled to the main thread through EM_FUNC_SIG_VI (a 32-bit +// int slot) in several places below. That is correct on wasm32, where pointers +// are 32-bit, but would silently truncate under MEMORY64. Guard the assumption +// so a future wasm64 build fails to compile here instead of corrupting pointers +// (the fix then is a pointer-sized signature such as EM_FUNC_SIG_VP). +static_assert(sizeof(void*) == sizeof(int), + "pointer/int size mismatch: revisit i32 proxying (EM_FUNC_SIG_VI) for MEMORY64"); + namespace { + // Non-fatal refcount check used around wallet stop. The custom Assert above + // always aborts (even with -s ASSERTIONS=0), but a WalletClient2 can + // legitimately have extra owners mid-stop: queued SendResult messages + // capture shared_from_this(). shared_ptr still guarantees destruction once + // the last owner releases, so an off-count is worth logging, not crashing. + void CheckUseCount(long actual, long expected, const char* where) + { + if (actual != expected) + { + BEAM_LOG_WARNING() << "Unexpected WalletClient2 use_count at " << where + << ": " << actual << " (expected " << expected << ")"; + } + } void GetWalletSeed(NoLeak& walletSeed, const std::string& s) { SecString seed; @@ -74,10 +95,7 @@ namespace GenerateDefaultAddress(db); EM_ASM ( - FS.syncfs(false, function() - { - console.log("wallet created!"); - }); + FS.syncfs(false, function() {}); ); return db; } @@ -214,15 +232,15 @@ class WalletClient2 void onStopped() override { WalletClient2::WeakPtr wp = weak_from_this(); - Assert(wp.use_count() == 1); + CheckUseCount(wp.use_count(), 1, "onStopped"); postFunctionToClientContext([sp = shared_from_this(), wp]() mutable { - Assert(wp.use_count() == 2); + CheckUseCount(wp.use_count(), 2, "onStopped.dispatch"); if (sp->m_StoppedHandler) { auto h = std::move(sp->m_StoppedHandler); sp.reset(); // handler may hold WalletClient too, but can destroy it, we don't want to prevent this - Assert(wp.use_count() == 1); + CheckUseCount(wp.use_count(), 1, "onStopped.afterReset"); h(); } }); @@ -433,17 +451,17 @@ class WasmWalletClient int Subscribe(val callback) { - for (uint32_t i = 0; i < m_Callbacks.size(); ++i) + for (size_t i = 0; i < m_Callbacks.size(); ++i) { auto& cb = m_Callbacks[i]; if (cb.isNull()) { cb = std::move(callback); - return i; + return static_cast(i); } } m_Callbacks.push_back(std::move(callback)); - return static_cast(m_Callbacks.size() - 1); + return static_cast(m_Callbacks.size() - 1); } void Unsubscribe(int key) @@ -523,11 +541,6 @@ class WasmWalletClient io::Reactor::Scope scope(*r); WalletDB::initNoKeeper(m_DbPath, m_Pass); s_Mounted = true; - - EM_ASM - ( - console.log("headless wallet created!"); - ); } BEAM_LOG_INFO() << "Rules signature: " << m_Rules.get_SignatureStr(); @@ -580,11 +593,11 @@ class WasmWalletClient [](const boost::any&) { }); std::weak_ptr wp = m_Client; - Assert(wp.use_count() == 1); + CheckUseCount(wp.use_count(), 1, "StopWallet"); m_Client->Stop([wp, sp = std::move(m_Client), handler = std::move(handler)]() mutable { AssertMainThread(); - Assert(wp.use_count() == 1); + CheckUseCount(wp.use_count(), 1, "StopWallet.stopped"); sp.reset(); // release client, at this point destructor should be called and handler code can rely on that client is really stopped and destroyed if (!handler.isNull()) { @@ -612,10 +625,15 @@ class WasmWalletClient { AssertMainThread(); auto r = result.dump(); - for (auto& cb : m_Callbacks) + // A callback may (un)subscribe synchronously, which can reallocate + // m_Callbacks and invalidate a range-for reference to the val being + // invoked. Index by position and copy the val (cheap refcount bump) + // before calling so reentrant mutation can't leave us on freed storage. + for (size_t i = 0; i < m_Callbacks.size(); ++i) { - if (!cb.isNull()) + if (!m_Callbacks[i].isNull()) { + val cb = m_Callbacks[i]; cb(r); } } @@ -647,15 +665,18 @@ class WasmWalletClient void OnImportRecoveryProgress(val error, uint64_t done, uint64_t total) { AssertMainThread(); + const bool finished = (done == total) || !error.isNull(); if (m_RecoveryCallback && !m_RecoveryCallback->isNull()) { (*m_RecoveryCallback)(error, static_cast(done), static_cast(total)); - if (done == total || !error.isNull()) - { - BEAM_LOG_DEBUG() << "Recovery done"; - m_RecoveryCallback.reset(); - RemoveRecoveryFile(); - } + } + // Remove the seed-equivalent blob once import ends, regardless of whether + // a progress callback was supplied (ImportRecovery permits a null one). + if (finished) + { + BEAM_LOG_DEBUG() << "Recovery done"; + m_RecoveryCallback.reset(); + RemoveRecoveryFile(); } } @@ -926,10 +947,7 @@ class WasmWalletClient fs::remove(dbName); EM_ASM ( - FS.syncfs(false, function() - { - console.log("wallet deleted!"); - }); + FS.syncfs(false, function() {}); ); } catch (const std::exception& ex) @@ -967,6 +985,11 @@ class WasmWalletClient static void CheckPassword(const std::string& dbName, const std::string& pass, val cb) { + // isValidPassword runs a deliberately-slow KDF, so it is offloaded to a + // detached thread to keep the main thread responsive. This is safe + // without lifetime tracking: the thread only touches its own copies of + // dbName/pass and the static WalletDB::isValidPassword (no shared + // instance state), and hands the result back via the main-thread proxy. auto pcb = std::make_shared(cb); MyThread(&WasmWalletClient::CheckPasswordImpl, dbName, pass, pcb).detach(); } @@ -998,31 +1021,39 @@ class WasmWalletClient { FS.mkdir("/beam_wallet"); FS.mount(IDBFS, {}, "/beam_wallet"); - console.log("mounting..."); FS.syncfs(true, function(error) { - if (error == null) { - dynCall('vi', $0, [$1]); - } - else { - dynCall('vi', $0, [error]); + // Hand OnMountFS a valid val* ($1 = &s_Null) plus a success + // flag. The JS error object can't be marshalled through the + // pointer slot (doing so derefs a bogus pointer), so log its + // text here and signal failure via the flag instead. + if (error != null) { + console.error("Beam: filesystem sync failed:", error); } + dynCall('vii', $0, [$1, error == null ? 1 : 0]); }); - }, OnMountFS, &s_Null ); } private: - static void OnMountFS(val* error) + static void OnMountFS(val* nullVal, int success) { - s_Mounted = true; - if (!s_MountCB.isNull()) + if (success) { - s_MountCB(*error); + s_Mounted = true; } - else + if (s_MountCB.isNull()) { BEAM_LOG_WARNING() << "Callback for mount is not set"; + return; + } + if (success) + { + s_MountCB(*nullVal); // null -> mount promise resolves as success + } + else + { + s_MountCB(val::global("Error").new_(val("Failed to mount filesystem"))); } } @@ -1041,6 +1072,9 @@ class WasmWalletClient std::unique_ptr m_ApproveContractInfoHandler; std::unique_ptr m_RecoveryCallback; WalletClient2::Ptr m_Client; + // Only ever created/reset/used inside makeIWTCall (ExecuteAPIRequest, + // StopWallet), i.e. on the wallet thread. Do not touch it from the main + // thread or add a second owner without revisiting that invariant. IWalletApi::Ptr m_WalletApi; bool m_Headless = false; std::string m_CurrentRecoveryFile; From 05bea8b5dd96ab9d0c05327aa6ebee1d3cf23241 Mon Sep 17 00:00:00 2001 From: bro Date: Fri, 17 Jul 2026 13:53:57 +0200 Subject: [PATCH 7/7] fix: post review --- wasmclient/wasmclient.cpp | 55 +++++++++++++++++++++++++-------------- 1 file changed, 36 insertions(+), 19 deletions(-) diff --git a/wasmclient/wasmclient.cpp b/wasmclient/wasmclient.cpp index 02e6b4d0bf..092d1cf98b 100644 --- a/wasmclient/wasmclient.cpp +++ b/wasmclient/wasmclient.cpp @@ -593,12 +593,21 @@ class WasmWalletClient [](const boost::any&) { }); std::weak_ptr wp = m_Client; + // May legitimately be >1 here: a SendResult posted by the wallet thread + // can still be sitting undrained in m_Messages holding shared_from_this() + // when JS calls stopWallet(). That is the load race the assert-downgrade + // is for; it does NOT contradict the destruction guarantee below. CheckUseCount(wp.use_count(), 1, "StopWallet"); m_Client->Stop([wp, sp = std::move(m_Client), handler = std::move(handler)]() mutable { AssertMainThread(); + // Guaranteed 1 here: this handler is posted from onStopped(), which + // runs only after the reactor loop returns, i.e. after every + // SendResult was enqueued. They share this FIFO queue, so all prior + // SendResults (and their shared_from_this refs) are drained before + // this runs -- so sp is the sole owner and sp.reset() destroys it. CheckUseCount(wp.use_count(), 1, "StopWallet.stopped"); - sp.reset(); // release client, at this point destructor should be called and handler code can rely on that client is really stopped and destroyed + sp.reset(); // last owner: client is really stopped and destroyed here, handler code can rely on that if (!handler.isNull()) { auto handlerPtr = std::make_unique(std::move(handler)); @@ -689,23 +698,27 @@ class WasmWalletClient } } - // recovery.bin holds seed-equivalent material; remove it from the - // persistent FS once import has finished (whether it succeeded or failed) - // so it is not left behind in IDBFS. Only the blob we wrote ourselves is - // touched; files passed to ImportRecoveryFromFile are not. - static void RemoveRecoveryFile() + // recovery.bin is written with a relative path and (absent any chdir) + // resolves to /recovery.bin in the transient MEMFS root, NOT the persisted + // IDBFS mount at /beam_wallet -- so it never reaches IndexedDB and needs no + // syncfs. It still holds seed-equivalent material and occupies space in the + // fixed heap (INITIAL_MEMORY, ALLOW_MEMORY_GROWTH=0), so free it once import + // has finished, whether it succeeded or failed. Only the blob we wrote + // ourselves is removed; a file supplied to ImportRecoveryFromFile is left + // alone even if it happens to be named recovery.bin (m_OwnsRecoveryFile). + void RemoveRecoveryFile() { + if (!m_OwnsRecoveryFile) + { + return; + } + m_OwnsRecoveryFile = false; std::error_code ec; fs::remove(std::string(RecoveryFileName), ec); if (ec) { BEAM_LOG_WARNING() << "Failed to remove recovery file: " << ec.message(); - return; } - EM_ASM - ( - FS.syncfs(false, function() {}); - ); } void CreateAppAPI(const std::string& appid, const std::string& appname, val cb) @@ -821,6 +834,7 @@ class WasmWalletClient m_RecoveryCallback = std::make_unique(std::move(callback)); } m_CurrentRecoveryFile = RecoveryFileName; + m_OwnsRecoveryFile = true; // we wrote the blob; safe to delete on completion } catch (const std::exception& ex) { @@ -843,6 +857,7 @@ class WasmWalletClient m_RecoveryCallback = std::make_unique(std::move(callback)); } m_CurrentRecoveryFile = fileName; + m_OwnsRecoveryFile = false; // user-supplied file; never delete it } catch (const std::exception& ex) { @@ -1023,20 +1038,21 @@ class WasmWalletClient FS.mount(IDBFS, {}, "/beam_wallet"); FS.syncfs(true, function(error) { - // Hand OnMountFS a valid val* ($1 = &s_Null) plus a success - // flag. The JS error object can't be marshalled through the - // pointer slot (doing so derefs a bogus pointer), so log its - // text here and signal failure via the flag instead. + // The JS error object can't be marshalled to C++ through the + // dynCall arg slot, so log its text here and pass only a + // success flag. Reuse the existing 'vi' signature: dynCall is + // not in EXPORTED_RUNTIME_METHODS and DYNAMIC_EXECUTION=0, so + // avoid introducing a new (possibly unavailable) signature. if (error != null) { console.error("Beam: filesystem sync failed:", error); } - dynCall('vii', $0, [$1, error == null ? 1 : 0]); + dynCall('vi', $0, [error == null ? 1 : 0]); }); - }, OnMountFS, &s_Null + }, OnMountFS ); } private: - static void OnMountFS(val* nullVal, int success) + static void OnMountFS(int success) { if (success) { @@ -1049,7 +1065,7 @@ class WasmWalletClient } if (success) { - s_MountCB(*nullVal); // null -> mount promise resolves as success + s_MountCB(s_Null); // null -> mount promise resolves as success } else { @@ -1078,6 +1094,7 @@ class WasmWalletClient IWalletApi::Ptr m_WalletApi; bool m_Headless = false; std::string m_CurrentRecoveryFile; + bool m_OwnsRecoveryFile = false; // true only for the recovery.bin blob we write ourselves }; val WasmWalletClient::s_MountCB = val::null();