Skip to content

Fix: Prevent negative plus balance from unfiltered Scrypt pending amounts#3330

Merged
TaprootFreak merged 1 commit intodevelopfrom
fix/negative-scrypt-pending-balance
Mar 3, 2026
Merged

Fix: Prevent negative plus balance from unfiltered Scrypt pending amounts#3330
TaprootFreak merged 1 commit intodevelopfrom
fix/negative-scrypt-pending-balance

Conversation

@TaprootFreak
Copy link
Collaborator

Problem

Critical bug causing negative plus balances in financial logs, particularly affecting Scrypt/EUR (Asset 401):

  • Log 1304749 (09:12): plusBalance = -201,499 EUR
  • Log 1304742 (09:09): plusBalance = -29,999 EUR
  • Expected: Plus balances should never be negative

Impact

  • plusBalanceChf stored as NULL in logs (rejected by getJsonValue() for negative values)
  • Total balance calculations corrupted
  • Financial type summations failed (EUR had no plusBalanceChf field)
  • System-wide balance tracking compromised

Root Cause

// Line 719-726: Unfiltered values calculated
let fromScryptUnfiltered = pendingChf... + pendingEur... + pendingScrypt...;
let toScryptUnfiltered = pendingBank... + pendingChf... + pendingEur...;

// Line 796-804: Added to totalPlusPending WITHOUT clamping
const totalPlusPending = ... + 
  (useUnfilteredTx ? fromScryptUnfiltered : fromScrypt) +  // ← Can be negative!
  (useUnfilteredTx ? toScryptUnfiltered : toScrypt);       // ← Can be negative!

// Line 806: Used in totalPlus calculation
const totalPlus = liquidity + totalPlusPending + ...;  // ← Goes negative!

Filtered versions (fromScrypt, toScrypt) were clamped to 0 if negative (lines 775-793).
Unfiltered versions were NOT clamped → negative values propagated to totalPlus.

Solution

  1. Changed fromScryptUnfiltered and toScryptUnfiltered from const to let
  2. Added clamping logic after line 793:
    if (fromScryptUnfiltered < 0) {
      errors.push(`fromScryptUnfiltered < 0`);
      this.logger.verbose(...);
      fromScryptUnfiltered = 0;
    }
    
    if (toScryptUnfiltered < 0) {
      errors.push(`toScryptUnfiltered < 0`);
      this.logger.verbose(...);
      toScryptUnfiltered = 0;
    }

Testing

  • ✅ All existing tests pass
  • ✅ Lint passed
  • ✅ Build successful
  • Prevents negative totalPlusPending from corrupting plus balances
  • Maintains error logging for debugging

Verification

Used diagnostic scripts (PR #3329) to identify the issue:

  • compare-balance-logs.sh: Found Scrypt asset changes
  • sum-asset-balances.sh: Showed EUR plusBalanceChf = -114k CHF
  • inspect-asset-balance.sh: Revealed totalPlus = -201k, liquidity = 1, pending = null

Related

  • Affects all Scrypt assets when unfiltered pending amounts are used
  • Most visible on Scrypt/EUR due to high transaction volume
  • Similar pattern exists for Kraken but was not observed as negative

Problem:
- fromScryptUnfiltered and toScryptUnfiltered could be negative
- These values were added to totalPlusPending without clamping
- Result: totalPlus became negative, causing negative plusBalance for assets
- Specifically affected Scrypt/EUR (Asset 401) with -201,499 EUR plusBalance

Root Cause:
- Filtered versions (fromScrypt, toScrypt) were clamped to 0 if negative
- Unfiltered versions were NOT clamped, causing negative plus balances
- When useUnfilteredTx=true, negative unfiltered values corrupted totalPlus

Fix:
- Changed fromScryptUnfiltered and toScryptUnfiltered from const to let
- Added clamping logic to set negative unfiltered values to 0
- Log errors when unfiltered values are negative for debugging
- Prevents negative plus balances while maintaining error visibility

Impact:
- Fixes critical bug where Scrypt assets could have negative plus balances
- Prevents plusBalanceChf from being stored as NULL in financial logs
- Maintains consistency with existing filtered value clamping logic
@TaprootFreak TaprootFreak marked this pull request as ready for review March 3, 2026 09:28
@TaprootFreak TaprootFreak merged commit 1d59eb4 into develop Mar 3, 2026
8 checks passed
@TaprootFreak TaprootFreak deleted the fix/negative-scrypt-pending-balance branch March 3, 2026 09:28
TaprootFreak added a commit that referenced this pull request Mar 3, 2026
Previous Fix (PR #3330):
- Clamped only fromScryptUnfiltered and toScryptUnfiltered individually
- Left fromKrakenUnfiltered and toKrakenUnfiltered unclamped
- Incomplete protection against negative plus balances

This Fix:
- Removed individual unfiltered component clamping
- Added single clamp on totalPlusPending after calculation
- Covers ALL negative sources: Kraken, Scrypt, and future exchanges
- More robust and maintainable

Rationale:
- Simpler: One clamp instead of multiple
- Complete: Catches negative values from any component
- Better logging: Shows all component values when totalPlusPending is negative
- Semantically correct: "Total pending plus balance cannot be negative"

The filtered component clamps (fromKraken, toKraken, fromScrypt, toScrypt)
remain in place for consistency and detailed error reporting.
TaprootFreak added a commit that referenced this pull request Mar 3, 2026
…3332)

Previous Fix (PR #3330):
- Clamped only fromScryptUnfiltered and toScryptUnfiltered individually
- Left fromKrakenUnfiltered and toKrakenUnfiltered unclamped
- Incomplete protection against negative plus balances

This Fix:
- Removed individual unfiltered component clamping
- Added single clamp on totalPlusPending after calculation
- Covers ALL negative sources: Kraken, Scrypt, and future exchanges
- More robust and maintainable

Rationale:
- Simpler: One clamp instead of multiple
- Complete: Catches negative values from any component
- Better logging: Shows all component values when totalPlusPending is negative
- Semantically correct: "Total pending plus balance cannot be negative"

The filtered component clamps (fromKraken, toKraken, fromScrypt, toScrypt)
remain in place for consistency and detailed error reporting.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant