zingocli: summaries crash with funded viewkey

[fluidvanadium]

summaries
thread '' panicked at 'attempt to subtract with overflow', zingolib/src/wallet/data.rs:948:9
note: run with RUST_BACKTRACE=1 environment variable to display a backtrace
Error executing command summaries: receiving on a closed channel
2023-10-02T20:43:06.458196Z ERROR zingo_cli: Error executing command summaries: receiving on a closed channel
thread 'main' panicked at 'called Result::unwrap() on an Err value: SendError { .. }', zingocli/src/lib.rs:130:55

[fluidvanadium]

data.rs:948

pub fn get_transaction_fee(&self) -> u64 {
    self.total_value_spent() - (self.value_outgoing() + self.total_change_returned())
}

[AArnott]

This happens to me as well in a wallet that includes the spending key.

[AArnott]

The calculation seems suspicious in that it may assume that the transaction is an outbound one. e.g. if it's inbound, one cannot calculate the total_change_returned.

[AArnott]

I wonder if this is related to #567, which reports one spend twice. I could see that causing the fee to be negative.

[AArnott]

Ultimately, in order to support incoming transactions, isn't the only reliable way to calculate fee to look at the turnstiles and transparent elements of a transaction to find out how much ZEC wasn't accounted for?

Something like:
fee = orchard_pool_decrease + sapling_pool_decrease + transparent_pool_utxos - transparent_pool_sends

[AloeareV]

#565 should fix this, once it's finished.
self.total_value_spent() - (self.value_outgoing() + self.total_change_returned()) makes no sense, and only works due to us having conflicting definitions of what change is.

[fluidvanadium]

trying to catch this in a test. it doesnt happen in the basic modern test case... hmm

[fluidvanadium]

zingocli treasury viewkey

list_transactions
...
TXID: f88...
amount: -15_000_010_000
outgoing_metadata: [
{
address: u1blahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblah
value: 15_000_000_000
memo: "Septmeber!"
},
{
address: u1blahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblah
value: 15_000_000_000
memo: "Septmeber!"
},
]

summaries
...
TXID f88...
spent 18_063_706_992
received 3_063_696_992
sent out 30_000_000_000
underflow panic at zingolib/src/wallet/data.rs:963
<

please note that sent out 30_000_000_000 = 15_000_000_000 + 15_000_000_000
this suggests that the unified address and the orchard address are being summed, improperly doubling the apparent send out.
the root cause is confusing the unified address, a data point we want to store somewhere, with the on chain orchard address which an appropriate sole source of truth for a transaction history

[AloeareV]

Ultimately, in order to support incoming transactions, isn't the only reliable way to calculate fee to look at the turnstiles and transparent elements of a transaction to find out how much ZEC wasn't accounted for?

Something like: fee = orchard_pool_decrease + sapling_pool_decrease + transparent_pool_utxos - transparent_pool_sends

Generally, we don't care about the fee for an incoming-only transaction, and it's not strictly necessary for the outgoing case. I've looked into chasing down the transparent part of the turnstile before and it's not as easy as I would expect it to be, but given the amount of bugs we've been dealing with in order to avoid it it's probably the right solution at this point, rather than continuing to jump through hoops to avoid it.

[fluidvanadium]

another, address independent, duplicate "outgoing_metadata"