From f904c4bd4057ab7333c7d701fb40f3e87fedf7fd Mon Sep 17 00:00:00 2001 From: Bitflash Date: Fri, 31 Jul 2026 11:27:53 -0300 Subject: gui: stop stacking every orphan the node ever produced on top of the list (#67) The wallet list sorted on GetDepthInMainChain() ascending so pending transactions came first. An orphaned coinbase is also depth 0 -- and it stays depth 0 forever, because it is never going to be confirmed. So every orphan a node had ever produced sat permanently at the top of the list, above every block it had actually won. A miner with a week of ordinary orphaned races opened the wallet and saw nothing but failures. The balance was right the whole time; the list was telling a different story. That is how this was found: a screenshot of a healthy, correctly-mining node that looked like it was failing at everything. Sort on (rank, depth) instead, where rank is 0 for pending, 1 for confirmed, 2 for orphaned. Orphans go to the bottom, pending stays on top, confirmed stays ordered most-recent-first. Verified against a datadir whose chain had been reorganised away, so every coinbase in it is orphaned: all 17 wallet transactions classify as rank 2. Not tested against a mixed wallet -- the machine that has one is in production with its datadir locked by the running node. --- src/gui.cpp | 33 +++++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-) (limited to 'src/gui.cpp') diff --git a/src/gui.cpp b/src/gui.cpp index 28f5ffa..53cb6db 100644 --- a/src/gui.cpp +++ b/src/gui.cpp @@ -124,15 +124,32 @@ static void RefreshWallet() g_txRows.clear(); TRY_CRITICAL_BLOCK(cs_mapWallet) { - // Sort: unconfirmed first (depth 0), then by depth ascending (least - // deep = most recent). We can't use nTimeReceived because a node - // resync re-stamps all txs with the same time. Depth is always correct. - std::vector> vs; - for (auto& kv : mapWallet) - vs.push_back({kv.second.GetDepthInMainChain(), kv.second.GetHash()}); + // Sort: pending first, then confirmed by depth ascending (least deep = + // most recent), then orphaned coinbases at the very bottom. + // + // Orphans have to be pushed down explicitly. They are depth 0, exactly + // like a genuinely pending transaction, so sorting on depth alone put + // every orphan the node had ever produced at the top of the list -- and + // they never age out, because depth 0 is where they stay forever. A + // miner with a week of ordinary orphaned races opened the wallet and + // saw nothing but failures stacked above every block it had actually + // won. The balance was right; the list was telling a different story. + // + // We can't sort on nTimeReceived because a resync re-stamps every + // transaction with the same time. Depth is always correct. + // + // 0 = pending, 1 = confirmed, 2 = orphaned. + std::vector,uint256>> vs; + for (auto& kv : mapWallet) { + int d = kv.second.GetDepthInMainChain(); + bool orphaned = kv.second.IsCoinBase() && kv.second.hashBlock != 0 && d == 0; + int rank = orphaned ? 2 : (d == 0 ? 0 : 1); + vs.push_back({{rank, d}, kv.second.GetHash()}); + } std::sort(vs.begin(), vs.end(), - [](const std::pair& a, const std::pair& b) { - return a.first < b.first; // 0 (unconfirmed) first, then shallowest + [](const std::pair,uint256>& a, + const std::pair,uint256>& b) { + return a.first < b.first; }); for (auto& sv : vs) { auto mi = mapWallet.find(sv.second); -- cgit v1.2.3