zcash_client_{backend,sqlite}: Correctly track accounts that fund each transaction output.

[nuttycom]

The complete solution here would be something like:

• Add a method to WalletRead that is something like detect_funding_accounts that checks whether any of the wallet's accounts contributed value to the transaction.
• Remove the from_account_id column from the sent_notes table, and replace it with a funding_accounts junction table between sent_notes and accounts.

Originally posted by @nuttycom in https://github.com/zcash/librustzcash/issues/1304#issuecomment-2016543402

[nuttycom]

Also consider the remaining comments on #1306 when implementing these changes.

[AArnott]

I have another case like #1304 where from_account_id is incorrectly NULL. But as that issue was closed and this one is active and seems to convey the idea of fixing "the whole problem", is this the right place to report it?

The following shows a _de_shielding transaction. Both rows were funded and directed at account_id 1, yet the top row (the deshielded value) claims to be novel income by leaving from_account_id with a NULL value:

[AArnott]

I guess it's a hard-coded limitation. The v_tx_outputs view literally hard-codes from_account_id to always be NULL when the row comes from the utxos table:

SELECT utxos.prevout_txid           AS txid,
       0                            AS output_pool,
       utxos.prevout_idx            AS output_index,
       NULL                         AS from_account_id,
       utxos.received_by_account_id AS to_account_id,
       utxos.address                AS to_address,
       utxos.value_zat              AS value,
       0                            AS is_change,
       NULL                         AS memo
FROM utxos

[AArnott]

I have my own view that is based on v_tx_outputs and joins with a few other views/tables. I was able to recover the proper from_account_id value by replacing the column in the original view with this:

coalesce(
   txo.from_account_id,
   (SELECT account_id FROM sapling_received_notes srn WHERE srn.id = (SELECT sapling_received_note_id FROM sapling_received_note_spends srns WHERE transaction_id = tx.id_tx)),
   (SELECT account_id FROM orchard_received_notes srn WHERE srn.id = (SELECT orchard_received_note_id FROM orchard_received_note_spends srns WHERE transaction_id = tx.id_tx))
) AS from_account_id,

I hope that's accurate. And that a real fix can come from librustzcash soon.

[daira]

I came across this issue when writing #1257 for ZIP 320 support, because the current code in store_decrypted_tx added in #1306 is just obviously wrong for this case. I had to determinedly talk myself out of getting derailed by trying to fix it. I can think of two approaches to do so:

1. Put the outputs in the database once for each account their tx is funded by (i.e. just loop over funding_accounts in store_decrypted_tx rather than picking one and doing if let Some(...)). The consequences of having duplicates of the same output in sent_notes across accounts would need to be considered.
2. Add a funded_by junction table between accounts and sent_notes.

In either case, we would also need a database migration that goes over sent_notes and adds the records that were missed. (In approach 1 this is only needed for a multi-account wallet; in approach 2 it always needs to change the table structure, but only does non-trivial work for a multi-account wallet.)

Overall I think 2 is better. It's definitely closer to being normalized: for approach 1 there ends up being a lot of duplicated information in sent_notes.

[nuttycom]

Overall I think 2 is better. It's definitely closer to being normalized: for approach 1 there ends up being a lot of duplicated information in sent_notes.

I tend to agree that (2) is the better option, but also think that funding spends from multiple accounts is generally not a use case we want to support. It's arguably required in order to correctly represent wallet history for wallets that were behaving in unspecified and unexpected ways, so I'm torn - do we add the complexity required to support this?

Apart from historic transactions that merge value from multiple accounts in a single tx, what are the use cases for this functionality?

[AArnott]

Apart from historic transactions that merge value from multiple accounts in a single tx, what are the use cases for this functionality?

I can't think of any. Whether I think about personal or enterprise use cases, spending from more than one account at once seems bonkers. ZECWallet probably did it due to lack of better guidance is my guess.

Is there any ZIP that forbids or strongly discourages funding across accounts? If so, that would be good grounds to not support it.

[nuttycom]

Is there any ZIP that forbids or strongly discourages funding across accounts? If so, that would be good grounds to not support it.

We should definitely recommend against it in ZIP 315; other than that there has never really been any documentation of best practices for wallet behavior.