backend: fix balance time series computation with failed txs

In ETH, there can be failed transactions that are mined (gas limit too
low). The fee is deducted, but the amount is not transferred.

We need to take this into account, otherwise the chart can go out of
whack.

Author: benma

CHANGELOG.md

@@ -8,6 +8,7 @@
 - Fix build on M-processor Apple machines
 - Add support for Ethereum EIP-1559 transactions: https://eips.ethereum.org/EIPS/eip-1559
 - Replace deprecated Ethgasstation with Etherscan for fee estimation (including base + priority fee for EIP-1559)
+- Fixed a bug where the portfolio chart could show wrong values in an Ethereum account containing failed transactions
 
 ## 4.40.0
 - Add support for watch-only - see your accounts and portfolio without connecting your BitBox02

backend/accounts/transaction.go

@@ -163,15 +163,21 @@ func NewOrderedTransactions(txs []*TransactionData) OrderedTransactions {
 		tx := txs[i]
 		switch tx.Type {
 		case TxTypeReceive:
-			balance.Add(balance, tx.Amount.BigInt())
+			if tx.Status != TxStatusFailed {
+				balance.Add(balance, tx.Amount.BigInt())
+			}
 		case TxTypeSend:
-			balance.Sub(balance, tx.Amount.BigInt())
-			// Subtract fee as well.
+			if tx.Status != TxStatusFailed {
+				balance.Sub(balance, tx.Amount.BigInt())
+			}
+			// Subtract fee as well. Ethereum: it is deducted even if the tx failed, as the tx was
+			// mined.
 			if tx.Fee != nil && !tx.FeeIsDifferentUnit {
 				balance.Sub(balance, tx.Fee.BigInt())
 			}
 		case TxTypeSendSelf:
-			// Subtract only fee.
+			// Subtract only fee. Ethereum: it is deducted even if the tx failed, as the tx was
+			// mined.
 			if tx.Fee != nil && !tx.FeeIsDifferentUnit {
 				balance.Sub(balance, tx.Fee.BigInt())
 			}

backend/accounts/transaction_test.go

@@ -149,3 +149,67 @@ func TestOrderedTransactions(t *testing.T) {
 		},
 	}, timeseries)
 }
+
+// TestOrderedTransactionsWithFailedTransactions tests that the cumulative balance takes into
+// account failed transactions.  Ethereum transactions can be mined and fail anyway due to a too low
+// gas limit, in which case the amount is not transferred, but the fees are still paid.
+func TestOrderedTransactionsWithFailedTransactions(t *testing.T) {
+	tt := func(t time.Time) *time.Time { return &t }
+	fee := coin.NewAmountFromInt64(1)
+	txs := []*TransactionData{
+		{
+			Timestamp: tt(time.Date(2020, 9, 15, 12, 0, 0, 0, time.UTC)),
+			Height:    15,
+			Type:      TxTypeReceive,
+			Amount:    coin.NewAmountFromInt64(100),
+		},
+		{
+			Timestamp: tt(time.Date(2020, 9, 10, 12, 0, 0, 0, time.UTC)),
+			Height:    10,
+			Type:      TxTypeReceive,
+			Amount:    coin.NewAmountFromInt64(200),
+		},
+		{
+			Timestamp: tt(time.Date(2020, 9, 20, 12, 0, 0, 0, time.UTC)),
+			Height:    20,
+			Type:      TxTypeReceive,
+			Amount:    coin.NewAmountFromInt64(300),
+		},
+		{
+			Timestamp: tt(time.Date(2020, 9, 21, 12, 0, 0, 0, time.UTC)),
+			Height:    21,
+			Type:      TxTypeSendSelf,
+			Amount:    coin.NewAmountFromInt64(50),
+			Fee:       &fee,
+		},
+		{
+			Timestamp: tt(time.Date(2020, 9, 22, 12, 0, 0, 0, time.UTC)),
+			Height:    22,
+			Type:      TxTypeSend,
+			Amount:    coin.NewAmountFromInt64(50),
+			Fee:       &fee,
+			Status:    TxStatusFailed,
+		},
+		{
+			Timestamp: tt(time.Date(2020, 9, 23, 12, 0, 0, 0, time.UTC)),
+			Height:    23,
+			Type:      TxTypeReceive,
+			Amount:    coin.NewAmountFromInt64(1000),
+			Fee:       &fee,
+			Status:    TxStatusFailed,
+		},
+	}
+
+	ordered := NewOrderedTransactions(txs)
+	expectedBalances := []int64{
+		598, // failed receive tx, nothing changes
+		598, // failed send tx, only fee deducted
+		599,
+		600,
+		300,
+		200,
+	}
+	for i := range ordered {
+		require.Equal(t, coin.NewAmountFromInt64(expectedBalances[i]), ordered[i].Balance, i)
+	}
+}