Initialize fees for (nearly) all transactions

src/Nerdbank.Zcash.Cli/HistoryCommand.cs

@@ -58,7 +58,7 @@ internal static void PrintTransaction(IConsole console, Transaction tx)
 			}
 		}
 
-		if (tx.IsOutgoing)
+		if (!tx.IsIncoming)
 		{
 			console.WriteLine($"{indentation} -{tx.Fee,13:N8} transaction fee");
 		}

src/Nerdbank.Zcash/LightWalletClient.cs

@@ -430,7 +430,7 @@ private static RecvItem CreateRecvItem(TransparentNote d)
 	/// </summary>
 	/// <param name="t">The uniffi transaction to copy data from.</param>
 	private static Transaction CreateTransaction(uniffi.LightWallet.Transaction t)
-		=> new(new TxId(t.txid), t.minedHeight, t.expiredUnmined, t.blockTime, ZatsToZEC(t.accountBalanceDelta), ZatsToZEC(t.fee), [.. t.outgoing.Select(CreateSendItem)], [.. t.incomingShielded.Select(CreateRecvItem), .. t.incomingTransparent.Select(CreateRecvItem)]);
+		=> new(new TxId(t.txid), t.minedHeight, t.expiredUnmined, t.blockTime, ZatsToZEC(t.accountBalanceDelta), t.fee.HasValue ? ZatsToZEC(t.fee.Value) : null, [.. t.outgoing.Select(CreateSendItem)], [.. t.incomingShielded.Select(CreateRecvItem), .. t.incomingTransparent.Select(CreateRecvItem)]);
 
 	/// <summary>
 	/// Wraps an interop invocation in a <see langword="try" /> block and wraps

src/Nerdbank.Zcash/PublicAPI.Unshipped.txt

@@ -409,9 +409,9 @@ Nerdbank.Zcash.SproutReceiver.SproutReceiver() -> void
 Nerdbank.Zcash.SproutReceiver.SproutReceiver(System.ReadOnlySpan<byte> apk, System.ReadOnlySpan<byte> pkEnc) -> void
 Nerdbank.Zcash.Transaction
 Nerdbank.Zcash.Transaction.ExpiredUnmined.get -> bool
-Nerdbank.Zcash.Transaction.Fee.get -> decimal
+Nerdbank.Zcash.Transaction.Fee.get -> decimal?
 Nerdbank.Zcash.Transaction.Incoming.get -> System.Collections.Immutable.ImmutableArray<Nerdbank.Zcash.Transaction.RecvItem>
-Nerdbank.Zcash.Transaction.IsOutgoing.get -> bool
+Nerdbank.Zcash.Transaction.IsIncoming.get -> bool
 Nerdbank.Zcash.Transaction.MinedHeight.get -> uint?
 Nerdbank.Zcash.Transaction.NetChange.get -> decimal
 Nerdbank.Zcash.Transaction.Outgoing.get -> System.Collections.Immutable.ImmutableArray<Nerdbank.Zcash.Transaction.SendItem>
@@ -442,7 +442,7 @@ Nerdbank.Zcash.Transaction.SendItem.SendItem(Nerdbank.Zcash.ZcashAddress! ToAddr
 Nerdbank.Zcash.Transaction.SendItem.ToAddress.get -> Nerdbank.Zcash.ZcashAddress!
 Nerdbank.Zcash.Transaction.SendItem.ToAddress.set -> void
 Nerdbank.Zcash.Transaction.Transaction(Nerdbank.Zcash.Transaction! original) -> void
-Nerdbank.Zcash.Transaction.Transaction(Nerdbank.Zcash.TxId transactionId, uint? minedHeight, bool expiredUnmined, System.DateTimeOffset? when, decimal netChange, decimal fee, System.Collections.Immutable.ImmutableArray<Nerdbank.Zcash.Transaction.SendItem> outgoing, System.Collections.Immutable.ImmutableArray<Nerdbank.Zcash.Transaction.RecvItem> incoming) -> void
+Nerdbank.Zcash.Transaction.Transaction(Nerdbank.Zcash.TxId transactionId, uint? minedHeight, bool expiredUnmined, System.DateTimeOffset? when, decimal netChange, decimal? fee, System.Collections.Immutable.ImmutableArray<Nerdbank.Zcash.Transaction.SendItem> outgoing, System.Collections.Immutable.ImmutableArray<Nerdbank.Zcash.Transaction.RecvItem> incoming) -> void
 Nerdbank.Zcash.Transaction.TransactionId.get -> Nerdbank.Zcash.TxId
 Nerdbank.Zcash.Transaction.When.get -> System.DateTimeOffset?
 Nerdbank.Zcash.Transparent.PrivateKey

src/Nerdbank.Zcash/RustBindings/LightWallet.cs

@@ -1661,7 +1661,7 @@ internal record Transaction(
 	uint? @minedHeight,
 	bool @expiredUnmined,
 	long @accountBalanceDelta,
-	ulong @fee,
+	ulong? @fee,
 	List<TransactionSendDetail> @outgoing,
 	List<TransparentNote> @incomingTransparent,
 	List<ShieldedNote> @incomingShielded
@@ -1680,7 +1680,7 @@ public override Transaction Read(BigEndianStream stream)
 			@minedHeight: FfiConverterOptionalUInt32.INSTANCE.Read(stream),
 			@expiredUnmined: FfiConverterBoolean.INSTANCE.Read(stream),
 			@accountBalanceDelta: FfiConverterInt64.INSTANCE.Read(stream),
-			@fee: FfiConverterUInt64.INSTANCE.Read(stream),
+			@fee: FfiConverterOptionalUInt64.INSTANCE.Read(stream),
 			@outgoing: FfiConverterSequenceTypeTransactionSendDetail.INSTANCE.Read(stream),
 			@incomingTransparent: FfiConverterSequenceTypeTransparentNote.INSTANCE.Read(stream),
 			@incomingShielded: FfiConverterSequenceTypeShieldedNote.INSTANCE.Read(stream)
@@ -1695,7 +1695,7 @@ public override int AllocationSize(Transaction value)
 			+ FfiConverterOptionalUInt32.INSTANCE.AllocationSize(value.@minedHeight)
 			+ FfiConverterBoolean.INSTANCE.AllocationSize(value.@expiredUnmined)
 			+ FfiConverterInt64.INSTANCE.AllocationSize(value.@accountBalanceDelta)
-			+ FfiConverterUInt64.INSTANCE.AllocationSize(value.@fee)
+			+ FfiConverterOptionalUInt64.INSTANCE.AllocationSize(value.@fee)
 			+ FfiConverterSequenceTypeTransactionSendDetail.INSTANCE.AllocationSize(value.@outgoing)
 			+ FfiConverterSequenceTypeTransparentNote.INSTANCE.AllocationSize(
 				value.@incomingTransparent
@@ -1711,7 +1711,7 @@ public override void Write(Transaction value, BigEndianStream stream)
 		FfiConverterOptionalUInt32.INSTANCE.Write(value.@minedHeight, stream);
 		FfiConverterBoolean.INSTANCE.Write(value.@expiredUnmined, stream);
 		FfiConverterInt64.INSTANCE.Write(value.@accountBalanceDelta, stream);
-		FfiConverterUInt64.INSTANCE.Write(value.@fee, stream);
+		FfiConverterOptionalUInt64.INSTANCE.Write(value.@fee, stream);
 		FfiConverterSequenceTypeTransactionSendDetail.INSTANCE.Write(value.@outgoing, stream);
 		FfiConverterSequenceTypeTransparentNote.INSTANCE.Write(value.@incomingTransparent, stream);
 		FfiConverterSequenceTypeShieldedNote.INSTANCE.Write(value.@incomingShielded, stream);
@@ -2393,6 +2393,45 @@ public override void Write(uint? value, BigEndianStream stream)
 	}
 }
 
+class FfiConverterOptionalUInt64 : FfiConverterRustBuffer<ulong?>
+{
+	public static FfiConverterOptionalUInt64 INSTANCE = new FfiConverterOptionalUInt64();
+
+	public override ulong? Read(BigEndianStream stream)
+	{
+		if (stream.ReadByte() == 0)
+		{
+			return null;
+		}
+		return FfiConverterUInt64.INSTANCE.Read(stream);
+	}
+
+	public override int AllocationSize(ulong? value)
+	{
+		if (value == null)
+		{
+			return 1;
+		}
+		else
+		{
+			return 1 + FfiConverterUInt64.INSTANCE.AllocationSize((ulong)value);
+		}
+	}
+
+	public override void Write(ulong? value, BigEndianStream stream)
+	{
+		if (value == null)
+		{
+			stream.WriteByte(0);
+		}
+		else
+		{
+			stream.WriteByte(1);
+			FfiConverterUInt64.INSTANCE.Write((ulong)value, stream);
+		}
+	}
+}
+
 class FfiConverterOptionalString : FfiConverterRustBuffer<String?>
 {
 	public static FfiConverterOptionalString INSTANCE = new FfiConverterOptionalString();

src/Nerdbank.Zcash/Transaction.cs

@@ -27,7 +27,7 @@ public Transaction(
 		bool expiredUnmined,
 		DateTimeOffset? when,
 		decimal netChange,
-		decimal fee,
+		decimal? fee,
 		ImmutableArray<SendItem> outgoing,
 		ImmutableArray<RecvItem> incoming)
 	{
@@ -67,9 +67,12 @@ public Transaction(
 	public decimal NetChange { get; }
 
 	/// <summary>
-	/// Gets the transaction fee.
+	/// Gets the transaction fee, as a <em>positive</em> value.
 	/// </summary>
-	public decimal Fee { get; }
+	/// <remarks>
+	/// This fee is only relevant to the account's balance when <see cref="IsIncoming"/> is <see langword="false" />.
+	/// </remarks>
+	public decimal? Fee { get; }
 
 	/// <summary>
 	/// Gets the individual sent notes in this transaction.
@@ -82,13 +85,7 @@ public Transaction(
 	public ImmutableArray<RecvItem> Incoming { get; }
 
 	/// <summary>
-	/// Gets a value indicating whether this transaction sends funds from this account.
+	/// Gets a value indicating whether this transaction did not originate from this account.
 	/// </summary>
-	/// <remarks>
-	/// Note this only indicates that funds were sent from this account,
-	/// not that they were sent to <em>another</em> account.
-	/// They may in fact have been sent to the same account.
-	/// But this value is useful to determine whether the <see cref="Fee"/> came out of this account.
-	/// </remarks>
-	public bool IsOutgoing => this.Outgoing.Length > 0;
+	public bool IsIncoming => this.Outgoing.IsEmpty;
 }

src/nerdbank-zcash-rust/src/error.rs

@@ -6,7 +6,10 @@ use zcash_client_backend::{
     zip321::Zip321Error,
 };
 use zcash_client_sqlite::{error::SqliteClientError, wallet::init::WalletMigrationError};
-use zcash_primitives::{memo, transaction::components::amount::NonNegativeAmount};
+use zcash_primitives::{
+    memo,
+    transaction::components::amount::{BalanceError, NonNegativeAmount},
+};
 
 use crate::block_source::BlockCacheError;
 
@@ -39,6 +42,8 @@ pub enum Error {
 
     TonicStatus(tonic::Status),
 
+    Balance(BalanceError),
+
     Io(std::io::Error),
 
     Minreq(minreq::Error),
@@ -68,6 +73,9 @@ pub enum Error {
 
     Zip321(Zip321Error),
 
+    /// The operation requires an outpoint whose value is not known.
+    OutPointMissing,
+
     /// The wallet has not been synced to the chain yet, and thus has no data with which to formulate a response.
     SyncFirst,
 
@@ -97,6 +105,7 @@ impl std::fmt::Display for Error {
             Error::SqliteMigrator(e) => e.fmt(f),
             Error::WalletMigrator(e) => e.fmt(f),
             Error::InvalidHeight => f.write_str("Invalid height"),
+            Error::Balance(e) => e.fmt(f),
             Error::InvalidAmount => f.write_str("Invalid amount"),
             Error::InsufficientFunds {
                 required,
@@ -115,10 +124,17 @@ impl std::fmt::Display for Error {
             Error::Anyhow(e) => e.fmt(f),
             Error::SendFailed { code, reason } => write!(f, "Send failed: {}: {}", code, reason),
             Error::Minreq(e) => e.fmt(f),
+            Error::OutPointMissing => f.write_str("OutPoint missing"),
         }
     }
 }
 
+impl From<BalanceError> for Error {
+    fn from(e: BalanceError) -> Self {
+        Error::Balance(e)
+    }
+}
+
 impl From<tonic::transport::Error> for Error {
     fn from(e: tonic::transport::Error) -> Self {
         Error::Transport(e)

src/nerdbank-zcash-rust/src/ffi.udl

@@ -26,7 +26,7 @@ dictionary Transaction {
 	u32? mined_height;
 	boolean expired_unmined;
 	i64 account_balance_delta;
-	u64 fee;
+	u64? fee;
 	sequence<TransactionSendDetail> outgoing;
 	sequence<TransparentNote> incoming_transparent;
 	sequence<ShieldedNote> incoming_shielded;

src/nerdbank-zcash-rust/src/interop.rs

@@ -98,7 +98,7 @@ pub struct Transaction {
     pub mined_height: Option<u32>,
     pub expired_unmined: bool,
     pub account_balance_delta: i64,
-    pub fee: u64,
+    pub fee: Option<u64>,
     pub outgoing: Vec<TransactionSendDetail>,
     pub incoming_transparent: Vec<TransparentNote>,
     pub incoming_shielded: Vec<ShieldedNote>,

src/nerdbank-zcash-rust/src/sync.rs

@@ -15,7 +15,10 @@ use zcash_primitives::{
     consensus::{BlockHeight, BranchId, Network, NetworkUpgrade, Parameters},
     legacy::TransparentAddress,
     merkle_tree::HashSer,
-    transaction::{components::OutPoint, Transaction, TxId},
+    transaction::{
+        components::{Amount, OutPoint},
+        Transaction, TxId,
+    },
     zip32::AccountId,
 };
 
@@ -75,6 +78,7 @@ pub async fn sync<P: AsRef<Path>>(
     let network = parse_network(&info)?;
 
     let mut db = Db::load(&data_file, network)?;
+    let conn = Connection::open(&data_file)?;
 
     // 1) Download note commitment tree data from lightwalletd
     // 2) Pass the commitment tree data to the database.
@@ -106,9 +110,11 @@ pub async fn sync<P: AsRef<Path>>(
             progress: &Option<Box<dyn SyncUpdate>>,
             data_file: &P,
             db: &mut Db,
+            conn: &Connection,
             network: Network,
         ) -> Result<(), Error> {
             if !txids.is_empty() {
+                initialize_transaction_fees(db, conn)?;
                 if let Some(sink) = progress.as_ref() {
                     let mut conn = Connection::open(data_file)?;
                     let new_transactions = get_transactions(db, &mut conn, &network, None, None)?
@@ -142,7 +148,7 @@ pub async fn sync<P: AsRef<Path>>(
                 cancellation_token.clone(),
             )
             .await?;
-            report_new_transactions(txids, &progress, &data_file, &mut db, network)?;
+            report_new_transactions(txids, &progress, &data_file, &mut db, &conn, network)?;
         }
 
         // 5) Get the suggested scan ranges from the wallet database
@@ -282,6 +288,7 @@ pub async fn sync<P: AsRef<Path>>(
                 &progress,
                 &data_file,
                 &mut db,
+                &conn,
                 network,
             )?;
 
@@ -310,6 +317,76 @@ pub async fn sync<P: AsRef<Path>>(
     }
 }
 
+/// Calculates the fee for some transaction.
+///
+/// Returns `Error::OutPointMissing` if any UTXO consumed by the transaction is not already in the `utxos` table.
+fn calculate_transaction_fee(transaction: Transaction, conn: &Connection) -> Result<Amount, Error> {
+    fn get_prevout_value(outpoint: &OutPoint, conn: &Connection) -> Result<Amount, Error> {
+        Ok(Amount::try_from(
+            conn.query_row(
+                "SELECT value_zat FROM utxos WHERE prevout_txid = :txid AND prevout_idx = :idx",
+                named_params! {
+                    ":txid": outpoint.hash(),
+                    ":idx": outpoint.n(),
+                },
+                |row| row.get::<_, i64>(0),
+            )
+            .map_err(|e| match e {
+                rusqlite::Error::QueryReturnedNoRows => Error::OutPointMissing,
+                e => e.into(),
+            })?,
+        )
+        .unwrap())
+    }
+
+    let transparent_value_balance = transaction
+        .transparent_bundle()
+        .map_or(Ok(Amount::zero()), |b| {
+            b.value_balance(|outpoint| get_prevout_value(outpoint, conn))
+        })?;
+    let sprout_value_balance = transaction.sprout_bundle().map_or(Amount::zero(), |b| {
+        b.value_balance().unwrap_or(Amount::zero())
+    });
+    let sapling_value_balance = transaction.sapling_value_balance();
+    let orchard_value_balance = transaction
+        .orchard_bundle()
+        .map_or(Amount::zero(), |b| b.value_balance().to_owned());
+
+    Ok((transparent_value_balance
+        + sprout_value_balance
+        + sapling_value_balance
+        + orchard_value_balance)
+        .unwrap())
+}
+
+/// Initializes the fee column for every transaction that is missing it.
+fn initialize_transaction_fees(db: &mut Db, conn: &Connection) -> Result<(), Error> {
+    conn.prepare("SELECT txid FROM transactions WHERE fee IS NULL")?
+        .query_map([], |r| r.get::<_, [u8; 32]>(0).map(TxId::from_bytes))?
+        .try_for_each(|txid| {
+            let txid = txid?;
+            let tx = db.data.get_transaction(txid)?;
+
+            // Some fees we'll fail to calculate because we're missing UTXOs.
+            // that should only happen when it's an incoming transparent transaction from a spent UTXO,
+            // but if it's incoming, the user didn't pay the fee anyway so it's not a big deal to not display the fee.
+            // If we want to predict whether transactions in the mempool have a ZIP-317 sufficient fee, we'll have to
+            // add a way to fetch those UTXO values.
+            if let Ok(fee) = calculate_transaction_fee(tx, conn) {
+                let txid: [u8; 32] = txid.into();
+                conn.execute(
+                    "UPDATE transactions SET fee = :fee WHERE txid = :txid",
+                    named_params! {
+                        ":fee": i64::from(fee),
+                        ":txid": txid,
+                    },
+                )?;
+            }
+            Ok::<_, Error>(())
+        })?;
+    Ok(())
+}
+
 async fn download_full_shielded_transactions<P: AsRef<Path> + Clone>(
     client: &mut CompactTxStreamerClient<Channel>,
     data_file: P,
@@ -659,7 +736,7 @@ pub fn get_transactions(
                     ),
                     None => None,
                 },
-                fee: row.get::<_, Option<u64>>("fee_paid")?.unwrap_or(0),
+                fee: row.get::<_, Option<u64>>("fee_paid")?,
                 account_balance_delta: row.get("account_balance_delta")?,
                 incoming_transparent: Vec::new(),
                 incoming_shielded: Vec::new(),