walletapi: SyncHistory mutates EntriesNative without w.Lock() — reader RLocks do not exclude the writer

[Dirtybird99]

Summary

Commit 6f9bd2c converted the transaction-history readers (Show_Transfers, Get_Payments_DestinationPort, Get_Payments_TXID, GetTXKey) from w.Lock() to w.RLock(). But the writer of w.account.EntriesNative — SyncHistory — never takes w.Lock() at all; it holds only the package-level sync_multilock, which readers don't touch. The RWMutex therefore provides no exclusion against the one goroutine that actually mutates the map.

Writer (holds only sync_multilock):

derohe/walletapi/daemon_communication.go

Lines 596 to 604 in 6f9bd2c

	entries = entries[:i]
	i--
	w.account.EntriesNative[scid] = entries
	logger.Info("syncing loop skipped ", "i", i, "skip", skip)
	}
	if i <= 0 {
	w.account.EntriesNative[scid] = entries[:0] // discard all entries
	logger.Info("syncing loop discarding all entries", "i", i)

Readers (RLock only — no shared lock with the writer):

derohe/walletapi/wallet.go

Lines 266 to 268 in 6f9bd2c

	func (w *Wallet_Memory) Show_Transfers(scid crypto.Hash, coinbase bool, in bool, out bool, min_height, max_height uint64, sender, receiver string, dstport, srcport uint64) []rpc.Entry {
	w.RLock()
	defer w.RUnlock()

Additionally, Save_Wallet serializes the whole account with json.Marshal(w.account) — which iterates EntriesNative (map + entry slices) — without any lock shared with SyncHistory:

derohe/walletapi/wallet_memory.go

Line 283 in 6f9bd2c

account_serialized, err := json.Marshal(w.account)

Impact

• Wallet RPC server calls (GetTransfers, etc.) reading entries while the sync loop's SyncHistory prunes/appends them: concurrent map read/write → runtime fatal throw (not a recoverable panic), or torn slice reads.
• Save_Wallet marshaling during a history scan: same fatal class, or a corrupted account snapshot encrypted to disk.
• This is also the root cause that makes PR fix(walletapi): run SyncHistory in goroutine to avoid blocking sync loop #26 (go w.SyncHistory(scid)) unsafe as written — see the review discussion there. Fixing locking here first would unblock that PR.

Fix

Have SyncHistory (and synchistory_block's entry appends) perform all w.account.EntriesNative mutations under w.Lock() — the readers' RLocks then actually exclude the writer — and take w.RLock() around the json.Marshal(w.account) in Save_Wallet. Run go test -race ./walletapi/... with a simulated concurrent sync to confirm.

[Dirtybird99]

Moved to PR #26 (the async-SyncHistory change this concerns): #26 (comment) — closing in favor of the PR thread.