Jon/fix/xmr confs

[Jon-edge]

CHANGELOG

Does this branch warrant an entry to the CHANGELOG?

• Yes
• No

Dependencies

none

Description

Problem

Monero transaction confirmations get stuck (e.g., at 0 or 1) and never progress to 'confirmed', even as blocks are mined.

Root Cause

Two places were illegally mutating state before storing to Redux:

1. onBlockHeightChanged:

   reduxTx.confirmations = determineConfirmations(...)  // ← Illegal mutation

2. onTransactions:

   tx.confirmations = determineConfirmations(...)  // ← Mutation before storage

The second issue was the actual blocker: once a numeric value like 1 was stored in Redux, shouldCoreDetermineConfirmations(1) would return false, so onBlockHeightChanged would skip that transaction—confirmations never progressed.

The Fix

Move confirmation calculation from storage time to read time:

Before	After
onTransactions calculated confirmations before storing	onTransactions stores raw engine values (including undefined)
onBlockHeightChanged mutated reduxTx.confirmations	onBlockHeightChanged only triggers GUI updates, no mutations
Calculated once at storage, never updated	Calculated fresh every time in combineTxWithFile()
Numbers stored in Redux blocked future recalculations	undefined stays in Redux, enabling continuous recalculation

Changes

1. onTransactions() no longer pre-calculates confirmations:

   • Removed the shouldCoreDetermineConfirmations / determineConfirmations block
   • Redux now stores exactly what the engine provided (including undefined for Monero)
   • This is the key fix: previously, calculated values like 1, 2, 3 were stored, causing onBlockHeightChanged to skip them

2. combineTxWithFile() now calculates confirmations on-the-fly:

   export function combineTxWithFile(
     input: CurrencyWalletInput,
     tx: MergedTransaction,
     file: TransactionFile | undefined,
     tokenId: EdgeTokenId,
     blockHeight?: number  // ← New optional parameter
   )

   • Uses provided blockHeight if passed, otherwise falls back to input.props.walletState.height
   • Calculates confirmations only if engine didn't provide valid value:

   const confirmations = shouldCoreDetermineConfirmations(tx.confirmations)
     ? determineConfirmations(tx, height, currencyInfo.requiredConfirmations)
     : tx.confirmations

3. onBlockHeightChanged() no longer mutates state:

   • Passes height directly to combineTxWithFile() (avoids stale redux state from async dispatch)
   • Notifies GUI via throttledOnTxChanged()
   • Dispatches CURRENCY_ENGINE_CHANGED_HEIGHT at the end for other consumers
   • No longer calls reduxTx.confirmations = ...
   • No longer dispatches CHANGE_MERGE_TX

4. shouldCoreDetermineConfirmations() unchanged - still respects engine-provided values:

   • If engine provides number, 'confirmed', 'dropped', or 'failed' → use it
   • Otherwise → core calculates

Flow: Engines That Manage Confirmations (Accountbased)

1. Accountbased detects new block
   │
2. Calls updateBlockHeight()
   │
   ├─► onBlockHeightChanged(height)
   │   └─► Core dispatches CURRENCY_ENGINE_CHANGED_HEIGHT
   │
   ├─► Engine loops through txs, updates confirmations internally
   │   └─► Sets tx.confirmations = 3 (or 'confirmed', etc.)
   │
   └─► Calls onTransactions() with updated transactions
       │
       └─► Core stores tx.confirmations in Redux (exactly what engine gave)
           │
           └─► GUI requests transaction via combineTxWithFile()
               │
               └─► shouldCoreDetermineConfirmations(3) → FALSE
                   │
                   └─► Returns engine's value unchanged ✓

Flow: Engines That Don't Manage Confirmations (Monero, Bitcoin)

1. New block mined, Monero calls onBlockHeightChanged(newHeight)
   │
2. Core's handler runs:
   │
   └─► Loops through transactions in Redux:
       │
       └─► For each tx, checks shouldCoreDetermineConfirmations(tx.confirmations)
           │
           tx.confirmations = undefined (Monero didn't set it)
           │
           └─► Returns TRUE
               │
               └─► Calls combineTxWithFile(input, tx, file, tokenId, newHeight)
                   │                                              ↑
                   │                              height passed directly from closure
                   │
                   └─► Calculates confirmations ON-THE-FLY:
                       determineConfirmations(tx, newHeight, requiredConfirmations)
                       │
                       e.g., 1 + 101 - 99 = 3 confirmations
                       │
                       └─► Returns EdgeTransaction with fresh confirmations
                           │
                           └─► Added to changedTxs array
       │
       ├─► throttledOnTxChanged(changedTxs)
       │   └─► GUI re-renders with updated confirmations ✓
       │
       └─► Dispatches CURRENCY_ENGINE_CHANGED_HEIGHT (for other consumers)

Key Points

1. Redux stores source of truth - Whatever the engine gave us (or undefined if nothing). Both onTransactions and onBlockHeightChanged now preserve engine values without modification.

2. Calculations happen at read time - In combineTxWithFile(), not at storage time. This ensures every read gets fresh calculations based on current block height.

3. No mutations - We never write reduxTx.confirmations = ... or tx.confirmations = ... before storing

4. Engine values are trusted - If engine provides a valid value (number, 'confirmed', 'dropped', 'failed'), we use it without recalculation

5. Every block triggers fresh calculation - For engines that don't manage confirmations, onBlockHeightChanged causes combineTxWithFile() to recalculate using the latest height

6. Height passed directly - onBlockHeightChanged passes height directly to combineTxWithFile() because redux-pixies props don't update synchronously after dispatch

---

Note

Fixes stuck confirmation states by moving confirmation calculation to read-time and avoiding Redux mutations.

• combineTxWithFile() now accepts optional blockHeight and calculates confirmations on-the-fly via determineConfirmations when engine values are missing/invalid
• onBlockHeightChanged() no longer mutates Redux; builds affected EdgeTransactions with current height, batches via throttledOnTxChanged, then dispatches CURRENCY_ENGINE_CHANGED_HEIGHT
• Export shouldCoreDetermineConfirmations and remove deprecated in-place confirmation recalculation path in onTransactions
• Update CHANGELOG with confirmations bug fix entry

^{Written by Cursor Bugbot for commit 8703aa7. This will update automatically on new commits. Configure here.}

---

• To see the specific tasks where the Asana app for GitHub is being used, see below:
  • https://app.asana.com/0/0/1212760593125310

[swansontec]

This seems wrong. Suppose we did a cleaner like this:

const asEngineConfirmations = asEither(asNumber, asValue('confirmed', 'dropped', 'failed'))

Those are the valid values for the number of confirmations. If the engine gives us one of those, we should just pass the value along unchanged. If the engine gives us something that is not one of those, the core should calculate the number of confirmations itself, using the block height and the tx height.

By removing "number" from the list, we are putting the core back in charge when the engine has already done this work. This creates its own bugs, because there are edge cases the core cannot handle. Long term, we want all the engines to calculate confirmations all the time, and the whole determineConfirmations path goes away.

However, what Paul's Cursor agent noticed is that we are mutating the transactions in Redux. This is the actual bug. The determineConfirmations function should not be updating anything in memory. Instead, Redux should just keep exactly what the engine gave us. The actual determineConfirmations should occur in combineTxWithFile, which is where we prepare transactions for the GUI's consumption, but only if we don't already have valid engine confirmations.

So, the onBlockHeightChanged handler is where we need to make the edits - stop calling reduxTx.confirmations =, which is an illegal state mutation, and instead just allow the existing combineTxWithFile to calculate the confirmations on the fly. Of course, combineTxWithFile might need to be updated to include the determineConfirmations call for this to work.

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.}