fix(lite): gateway refresh degrades gracefully on a failed command
LiteWalletGateway::refresh() aborted the entire refresh on the first command whose bridge call or parse failed — which turned a single real-backend shape mismatch (e.g. syncstatus) into a total, empty-everything refresh. Since the balance/addresses/list real shapes are still unverified and we've already hit shape drift twice, make refresh resilient: - Run every planned command; assembleLiteWalletRefreshBundle already skips failed results. - result.ok = any usable data came back (bundle.complete still reflects all-succeeded). - One command's failure now degrades gracefully — the other sections still populate. testLiteWalletGatewayRefreshSkipsFailedCommand (fake balance returns invalid JSON) asserts the refresh still succeeds with addresses/transactions/info populated and balance skipped. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -100,7 +100,8 @@ Each milestone is independently demoable and gated by a fake-backend test. Order
|
|||||||
> 1. **FIXED — `syncstatus` parser mismatch.** `parseLiteSyncStatusResponse` hard-required `synced_blocks`/`total_blocks`, but the real backend (per `commands.rs:83-87`) returns **idle = `{"syncing":"false"}`** (string!) and only **in-progress = `{"syncing":"true","synced_blocks":N,"total_blocks":M}`**. The parser now reads `syncing` as a string and treats the block fields as in-progress-only (idle → complete, synced/total 0). Covered by `testLiteSyncStatusParserRealShapes()` and **verified against the live backend** (`syncstatus parse_ok=1`). (info/balance/addresses parsers also verified OK against real output.)
|
> 1. **FIXED — `syncstatus` parser mismatch.** `parseLiteSyncStatusResponse` hard-required `synced_blocks`/`total_blocks`, but the real backend (per `commands.rs:83-87`) returns **idle = `{"syncing":"false"}`** (string!) and only **in-progress = `{"syncing":"true","synced_blocks":N,"total_blocks":M}`**. The parser now reads `syncing` as a string and treats the block fields as in-progress-only (idle → complete, synced/total 0). Covered by `testLiteSyncStatusParserRealShapes()` and **verified against the live backend** (`syncstatus parse_ok=1`). (info/balance/addresses parsers also verified OK against real output.)
|
||||||
> 2. **ADDRESSED — blocking, uninterruptible sync.** The backend `sync` command runs `do_sync(true)`, a blocking full scan that does **not** honor the shutdown flag (`lightclient.rs`), and `balance`/`list` block until synced. Redesign: the controller runs `sync` on a **detached** thread (never joined), the bridge is a `std::shared_ptr` shared with that thread (so detaching is safe and the bridge isn't `litelib_shutdown`'d while a sync still holds it), and `startSync()` is now non-blocking (was called on the main thread → would have frozen wallet creation). The joinable **poll worker** only issues fast `syncstatus` calls while syncing (publishing progress) and fetches balance/addresses/list **once `syncDone_` is set**. Shutdown joins only the fast poll worker and detaches the sync thread → no hang. Verified deterministically by `testLiteWalletControllerShutdownDoesNotHangDuringSync()` (blocking-sync fake; destructor returns <1.5s) and the worker/refresh tests (stable across repeated runs).
|
> 2. **ADDRESSED — blocking, uninterruptible sync.** The backend `sync` command runs `do_sync(true)`, a blocking full scan that does **not** honor the shutdown flag (`lightclient.rs`), and `balance`/`list` block until synced. Redesign: the controller runs `sync` on a **detached** thread (never joined), the bridge is a `std::shared_ptr` shared with that thread (so detaching is safe and the bridge isn't `litelib_shutdown`'d while a sync still holds it), and `startSync()` is now non-blocking (was called on the main thread → would have frozen wallet creation). The joinable **poll worker** only issues fast `syncstatus` calls while syncing (publishing progress) and fetches balance/addresses/list **once `syncDone_` is set**. Shutdown joins only the fast poll worker and detaches the sync thread → no hang. Verified deterministically by `testLiteWalletControllerShutdownDoesNotHangDuringSync()` (blocking-sync fake; destructor returns <1.5s) and the worker/refresh tests (stable across repeated runs).
|
||||||
>
|
>
|
||||||
> - ⏳ **Remaining for M2 polish:** fix the syncstatus parser (above), address the blocking-sync/worker-shutdown issue (above), per-address balances (notes-correlation; currently aggregate-only), and harden the gateway's abort-on-first-failure (skip-and-continue per command).
|
> - ✅ **Gateway abort-on-first-failure hardened.** `LiteWalletGateway::refresh()` now runs every command and assembles partial data instead of aborting on the first parse/bridge failure (`result.ok` = any usable data; `bundle.complete` still reflects all-succeeded). One command's real-shape drift degrades gracefully — the other tabs still populate. Covered by `testLiteWalletGatewayRefreshSkipsFailedCommand()`.
|
||||||
|
> - ⏳ **Remaining for M2 polish:** per-address balances (notes-correlation; currently aggregate-only), and a full **real-backend end-to-end** verification (sync to tip → balance/addresses/list through the parsers — likely surfaces more real-shape mismatches, same class as chain-name/syncstatus).
|
||||||
- Implement `LiteSyncService::startSync` (replace the "not implemented" stub) + a background worker polling `syncstatus`, mirroring `NetworkRefreshService`/`RefreshScheduler` (enqueue → worker → apply on main thread).
|
- Implement `LiteSyncService::startSync` (replace the "not implemented" stub) + a background worker polling `syncstatus`, mirroring `NetworkRefreshService`/`RefreshScheduler` (enqueue → worker → apply on main thread).
|
||||||
- Drive `LiteWalletGateway` refresh (info/height/balance/addresses/notes/list/transactions) through `lite_result_parsers` → `lite_wallet_state_mapper` → `App` `WalletState` (`privateBalance`, `transparentBalance`, `addresses`, `transactions`, `sync`).
|
- Drive `LiteWalletGateway` refresh (info/height/balance/addresses/notes/list/transactions) through `lite_result_parsers` → `lite_wallet_state_mapper` → `App` `WalletState` (`privateBalance`, `transparentBalance`, `addresses`, `transactions`, `sync`).
|
||||||
- Hook the controller into `App::update()`'s refresh dispatch alongside (not inside) the full-node path.
|
- Hook the controller into `App::update()`'s refresh dispatch alongside (not inside) the full-node path.
|
||||||
|
|||||||
@@ -230,24 +230,33 @@ LiteWalletRefreshResult LiteWalletGateway::refresh(const LiteWalletRefreshReques
|
|||||||
}
|
}
|
||||||
|
|
||||||
result.attempted = true;
|
result.attempted = true;
|
||||||
|
// Run every planned command. A single command's bridge or parse failure must NOT abort the
|
||||||
|
// whole refresh — partial data beats none, and real-backend response shapes can drift per
|
||||||
|
// command. assembleLiteWalletRefreshBundle() ignores the failed results.
|
||||||
for (const auto& commandPlan : result.plan.commands) {
|
for (const auto& commandPlan : result.plan.commands) {
|
||||||
auto commandResult = executePlannedCommand(commandPlan);
|
result.commandResults.push_back(executePlannedCommand(commandPlan));
|
||||||
if (!commandResult.ok) {
|
|
||||||
result.status = commandResult.status;
|
|
||||||
result.error = commandResult.error;
|
|
||||||
result.commandResults.push_back(std::move(commandResult));
|
|
||||||
return result;
|
|
||||||
}
|
|
||||||
result.commandResults.push_back(std::move(commandResult));
|
|
||||||
}
|
}
|
||||||
|
|
||||||
result.bundle = assembleLiteWalletRefreshBundle(result.commandResults, request);
|
result.bundle = assembleLiteWalletRefreshBundle(result.commandResults, request);
|
||||||
result.ok = result.bundle.complete;
|
|
||||||
result.status = refreshStatusForRequest(request);
|
// Succeed if anything usable came back; some commands may have been skipped. (bundle.complete
|
||||||
|
// still reflects whether *all* commands succeeded, for callers that care.)
|
||||||
|
result.ok = result.bundle.successfulCommandCount > 0 || result.bundle.hasSyncStatus;
|
||||||
if (!result.ok) {
|
if (!result.ok) {
|
||||||
result.status = makeGatewayStatus(WalletBackendState::Error, "lite refresh bundle is incomplete");
|
for (const auto& commandResult : result.commandResults) {
|
||||||
result.error = result.status.message;
|
if (!commandResult.ok) {
|
||||||
|
result.status = commandResult.status;
|
||||||
|
result.error = commandResult.error;
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if (result.error.empty()) {
|
||||||
|
result.status = makeGatewayStatus(WalletBackendState::Error, "lite refresh produced no usable data");
|
||||||
|
result.error = result.status.message;
|
||||||
|
}
|
||||||
|
return result;
|
||||||
}
|
}
|
||||||
|
result.status = refreshStatusForRequest(request);
|
||||||
return result;
|
return result;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -33,6 +33,7 @@ inline bool g_liteFakeWalletExists = true;
|
|||||||
inline bool g_liteFakeServerOnline = true;
|
inline bool g_liteFakeServerOnline = true;
|
||||||
inline bool g_liteFakeShutdownCalled = false;
|
inline bool g_liteFakeShutdownCalled = false;
|
||||||
inline std::atomic<bool> g_liteFakeSyncBlock{false}; // when true, the "sync" command blocks
|
inline std::atomic<bool> g_liteFakeSyncBlock{false}; // when true, the "sync" command blocks
|
||||||
|
inline std::atomic<bool> g_liteFakeBadBalance{false}; // when true, "balance" returns invalid JSON
|
||||||
|
|
||||||
inline void resetLiteFakeCounters()
|
inline void resetLiteFakeCounters()
|
||||||
{
|
{
|
||||||
@@ -76,6 +77,8 @@ inline char* liteFakeExecute(const char* command, const char*)
|
|||||||
}
|
}
|
||||||
if (std::strcmp(c, "syncstatus") == 0) // real backend shape: "syncing" is a string
|
if (std::strcmp(c, "syncstatus") == 0) // real backend shape: "syncing" is a string
|
||||||
return liteFakeDup("{\"syncing\":\"true\",\"synced_blocks\":1000,\"total_blocks\":1000}");
|
return liteFakeDup("{\"syncing\":\"true\",\"synced_blocks\":1000,\"total_blocks\":1000}");
|
||||||
|
if (std::strcmp(c, "balance") == 0 && g_liteFakeBadBalance.load())
|
||||||
|
return liteFakeDup("not-json"); // simulate a shape/parse failure for one command
|
||||||
if (std::strcmp(c, "balance") == 0)
|
if (std::strcmp(c, "balance") == 0)
|
||||||
return liteFakeDup("{\"tbalance\":100000000,\"zbalance\":200000000,\"unconfirmed\":50000000,"
|
return liteFakeDup("{\"tbalance\":100000000,\"zbalance\":200000000,\"unconfirmed\":50000000,"
|
||||||
"\"verified_zbalance\":180000000,\"spendable_zbalance\":170000000}");
|
"\"verified_zbalance\":180000000,\"spendable_zbalance\":170000000}");
|
||||||
|
|||||||
@@ -4687,6 +4687,29 @@ void testLiteSyncStatusParserRealShapes()
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Gateway hardening: one command's parse failure must not abort the whole refresh — the
|
||||||
|
// other commands still populate the bundle (graceful degradation against real-shape drift).
|
||||||
|
void testLiteWalletGatewayRefreshSkipsFailedCommand()
|
||||||
|
{
|
||||||
|
using namespace dragonx::wallet;
|
||||||
|
const auto caps = makeWalletCapabilities(WalletBuildKind::Lite, /*embeddedDaemon*/ false, /*liteBackendLinked*/ true);
|
||||||
|
const auto conn = defaultLiteConnectionSettings();
|
||||||
|
|
||||||
|
auto bridge = LiteClientBridge::fromApi(dragonx::test::makeFakeLiteApi());
|
||||||
|
LiteWalletGateway gateway(caps, conn, &bridge, LiteWalletGatewayOptions{/*allowBridgeCalls*/ true});
|
||||||
|
|
||||||
|
dragonx::test::g_liteFakeBadBalance.store(true); // make only the balance command fail to parse
|
||||||
|
const auto result = gateway.refresh(LiteWalletRefreshRequest{});
|
||||||
|
dragonx::test::g_liteFakeBadBalance.store(false);
|
||||||
|
|
||||||
|
EXPECT_TRUE(result.ok); // partial success, not a total failure
|
||||||
|
EXPECT_FALSE(result.bundle.complete); // not all commands succeeded
|
||||||
|
EXPECT_FALSE(result.bundle.hasBalance); // the failed command is skipped
|
||||||
|
EXPECT_TRUE(result.bundle.hasAddresses); // the others still populate
|
||||||
|
EXPECT_TRUE(result.bundle.hasTransactions);
|
||||||
|
EXPECT_TRUE(result.bundle.hasInfo);
|
||||||
|
}
|
||||||
|
|
||||||
// M2b-3: opening a wallet auto-starts the background worker, which produces a refresh model
|
// M2b-3: opening a wallet auto-starts the background worker, which produces a refresh model
|
||||||
// the main thread can pick up via takeRefreshedModel() and apply to WalletState.
|
// the main thread can pick up via takeRefreshedModel() and apply to WalletState.
|
||||||
void testLiteWalletControllerWorkerProducesModel()
|
void testLiteWalletControllerWorkerProducesModel()
|
||||||
@@ -4788,6 +4811,7 @@ int main()
|
|||||||
testLiteRefreshModelAppliesToWalletState();
|
testLiteRefreshModelAppliesToWalletState();
|
||||||
testLiteSyncStatusParserRealShapes();
|
testLiteSyncStatusParserRealShapes();
|
||||||
testLiteWalletControllerRefreshPopulatesState();
|
testLiteWalletControllerRefreshPopulatesState();
|
||||||
|
testLiteWalletGatewayRefreshSkipsFailedCommand();
|
||||||
testLiteWalletControllerWorkerProducesModel();
|
testLiteWalletControllerWorkerProducesModel();
|
||||||
testLiteWalletControllerShutdownDoesNotHangDuringSync();
|
testLiteWalletControllerShutdownDoesNotHangDuringSync();
|
||||||
testLiteBridgeRuntimeShutdownIsIdempotent();
|
testLiteBridgeRuntimeShutdownIsIdempotent();
|
||||||
|
|||||||
Reference in New Issue
Block a user