Skip to content

Fix double-entry accounting errors in transaction views. #807

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
nuttycom merged 5 commits into from
Apr 14, 2023
Merged

Fix double-entry accounting errors in transaction views. #807

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1782753
Fix double-entry accounting errors in transaction views.
nuttycom Apr 7, 2023
024c858
Ensure that we don't filter out sent UTXOs that collide with Sapling …
nuttycom Apr 11, 2023
25b2148
Account for received utxos in `v_transactions`.
nuttycom Apr 12, 2023
b6fd844
Fix Rust formatting errors.
nuttycom Apr 14, 2023
7d18d1d
Adds documentation for the `v_transactions` and `v_tx_outputs` views.
nuttycom Apr 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 57 additions & 0 deletions zcash_client_sqlite/src/wallet.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,63 @@
//!
//! [`WalletRead`]: zcash_client_backend::data_api::WalletRead
//! [`WalletWrite`]: zcash_client_backend::data_api::WalletWrite
//!
//! # Views
//!
//! The wallet database exposes the following views as part of its public API:
//!
//! ## `v_transactions`
//!
//! This view exposes the history of transactions that affect the balance of each account in the
//! wallet. A transaction may be represented by multiple rows in this view, one for each account in
//! the wallet that contributes funds to or receives funds from the transaction in question. Each
//! row of the view contains:
//! - `account_balance_delta`: the net effect of the transaction on the associated account's
//! balance. This value is positive when funds are received by the account, and negative when the
//! balance of the account decreases due to a spend.
//! - `fee_paid`: the total fee paid to send the transaction, as a positive value. This fee is
//! associated with the transaction (similar to e.g. `txid` or `mined_height`), and not with any
//! specific account involved with that transaction. ` If multiple rows exist for a single
//! transaction, this fee amount will be repeated for each such row. Therefore, if more than one
//! of the wallet's accounts is involved with the transaction, this fee should be considered only
//! once in determining the total value sent from the wallet as a whole.
//!
//! ### Seed Phrase with Single Account
//!
//! In the case that the seed phrase for in this wallet has only been used to create a single
//! account, this view will contain one row per transaction, in the case that
//! `account_balance_delta` is negative, it is usually safe to add `fee_paid` back to the
//! `account_balance_delta` value to determine the amount sent to addresses outside the wallet.
//!
//! ### Seed Phrase with Multiple Accounts
//!
//! In the case that the seed phrase for in this wallet has been used to create multiple accounts,
//! this view may contain multiple rows per transaction, one for each account involved. In this
//! case, the total amount sent to addresses outside the wallet can usually be calculated by
//! grouping rows by `id_tx` and then using `SUM(account_balance_delta) + MAX(fee_paid)`.
//!
//! ### Imported Seed Phrases
//!
//! If a seed phrase is imported, and not every account associated with it is loaded into the
//! wallet, this view may show partial information about some transactions. In particular, any
//! computation that involves both `account_balance_delta` and `fee_paid` is likely to be
//! inaccurate.
//!
//! ## `v_tx_outputs`
//!
//! This view exposes the history of transaction outputs received by and sent from the wallet,
//! keyed by transaction ID, pool type, and output index. The contents of this view are useful for
//! producing a detailed report of the effects of a transaction. Each row of this view contains:
//! - `from_account` for sent outputs, the account from which the value was sent.
//! - `to_account` in the case that the output was received by an account in the wallet, the
//! identifier for the account receiving the funds.
//! - `to_address` the address to which an output was sent, or the address at which value was
//! received in the case of received transparent funds.
//! - `value` the value of the output. This is always a positive number, for both sent and received
//! outputs.
//! - `is_change` a boolean flag indicating whether this is a change output belonging to the
//! wallet.
//! - `memo` the shielded memo associated with the output, if any.

use group::ff::PrimeField;
use rusqlite::{named_params, OptionalExtension, ToSql};
Expand Down
215 changes: 117 additions & 98 deletions zcash_client_sqlite/src/wallet/init.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -453,33 +453,12 @@ mod tests {
let expected_views = vec![
// v_transactions
"CREATE VIEW v_transactions AS
SELECT notes.id_tx,
notes.mined_height,
notes.tx_index,
notes.txid,
notes.expiry_height,
notes.raw,
SUM(notes.value) + MAX(notes.fee) AS net_value,
MAX(notes.fee) AS fee_paid,
SUM(notes.sent_count) == 0 AS is_wallet_internal,
SUM(notes.is_change) > 0 AS has_change,
SUM(notes.sent_count) AS sent_note_count,
SUM(notes.received_count) AS received_note_count,
SUM(notes.memo_present) AS memo_count,
blocks.time AS block_time
FROM (
SELECT transactions.id_tx AS id_tx,
transactions.block AS mined_height,
transactions.tx_index AS tx_index,
transactions.txid AS txid,
transactions.expiry_height AS expiry_height,
transactions.raw AS raw,
0 AS fee,
CASE
WHEN received_notes.is_change THEN 0
ELSE value
END AS value,
0 AS sent_count,
WITH
notes AS (
SELECT received_notes.account AS account_id,
received_notes.tx AS id_tx,
2 AS pool,
received_notes.value AS value,
CASE
WHEN received_notes.is_change THEN 1
ELSE 0
Expand All @@ -492,80 +471,120 @@ mod tests {
WHEN received_notes.memo IS NULL THEN 0
ELSE 1
END AS memo_present
FROM transactions
JOIN received_notes ON transactions.id_tx = received_notes.tx
FROM received_notes
UNION
SELECT transactions.id_tx AS id_tx,
transactions.block AS mined_height,
transactions.tx_index AS tx_index,
transactions.txid AS txid,
transactions.expiry_height AS expiry_height,
transactions.raw AS raw,
transactions.fee AS fee,
-sent_notes.value AS value,
CASE
WHEN sent_notes.from_account = sent_notes.to_account THEN 0
ELSE 1
END AS sent_count,
SELECT utxos.received_by_account AS account_id,
transactions.id_tx AS id_tx,
0 AS pool,
utxos.value_zat AS value,
0 AS is_change,
1 AS received_count,
0 AS memo_present
FROM utxos
JOIN transactions
ON transactions.txid = utxos.prevout_txid
UNION
SELECT received_notes.account AS account_id,
received_notes.spent AS id_tx,
2 AS pool,
-received_notes.value AS value,
0 AS is_change,
0 AS received_count,
CASE
WHEN sent_notes.memo IS NULL THEN 0
ELSE 1
END AS memo_present
FROM transactions
JOIN sent_notes ON transactions.id_tx = sent_notes.tx
) AS notes
LEFT JOIN blocks ON notes.mined_height = blocks.height
GROUP BY notes.id_tx",
// v_tx_received
"CREATE VIEW v_tx_received AS
SELECT transactions.id_tx AS id_tx,
transactions.block AS mined_height,
transactions.tx_index AS tx_index,
transactions.txid AS txid,
transactions.expiry_height AS expiry_height,
transactions.raw AS raw,
MAX(received_notes.account) AS received_by_account,
SUM(received_notes.value) AS received_total,
COUNT(received_notes.id_note) AS received_note_count,
SUM(
CASE
WHEN received_notes.memo IS NULL THEN 0
ELSE 1
END
) AS memo_count,
blocks.time AS block_time
FROM transactions
JOIN received_notes
ON transactions.id_tx = received_notes.tx
LEFT JOIN blocks
ON transactions.block = blocks.height
GROUP BY received_notes.tx, received_notes.account",
// v_tx_received
"CREATE VIEW v_tx_sent AS
SELECT transactions.id_tx AS id_tx,
transactions.block AS mined_height,
transactions.tx_index AS tx_index,
transactions.txid AS txid,
transactions.expiry_height AS expiry_height,
transactions.raw AS raw,
MAX(sent_notes.from_account) AS sent_from_account,
SUM(sent_notes.value) AS sent_total,
COUNT(sent_notes.id_note) AS sent_note_count,
SUM(
CASE
WHEN sent_notes.memo IS NULL THEN 0
ELSE 1
END
) AS memo_count,
blocks.time AS block_time
FROM transactions
JOIN sent_notes
ON transactions.id_tx = sent_notes.tx
LEFT JOIN blocks
ON transactions.block = blocks.height
GROUP BY sent_notes.tx, sent_notes.from_account",
0 AS memo_present
FROM received_notes
WHERE received_notes.spent IS NOT NULL
),
sent_note_counts AS (
SELECT sent_notes.from_account AS account_id,
sent_notes.tx AS id_tx,
COUNT(DISTINCT sent_notes.id_note) as sent_notes,
SUM(
CASE
WHEN sent_notes.memo IS NULL THEN 0
ELSE 1
END
) AS memo_count
FROM sent_notes
LEFT JOIN received_notes
ON (sent_notes.tx, sent_notes.output_pool, sent_notes.output_index) =
(received_notes.tx, 2, received_notes.output_index)
WHERE received_notes.is_change IS NULL
OR received_notes.is_change = 0
GROUP BY account_id, id_tx
),
blocks_max_height AS (
SELECT MAX(blocks.height) as max_height FROM blocks
)
SELECT notes.account_id AS account_id,
transactions.id_tx AS id_tx,
transactions.block AS mined_height,
transactions.tx_index AS tx_index,
transactions.txid AS txid,
transactions.expiry_height AS expiry_height,
transactions.raw AS raw,
SUM(notes.value) AS account_balance_delta,
transactions.fee AS fee_paid,
SUM(notes.is_change) > 0 AS has_change,
MAX(COALESCE(sent_note_counts.sent_notes, 0)) AS sent_note_count,
SUM(notes.received_count) AS received_note_count,
SUM(notes.memo_present) + MAX(COALESCE(sent_note_counts.memo_count, 0)) AS memo_count,
blocks.time AS block_time,
(
blocks.height IS NULL
AND transactions.expiry_height <= blocks_max_height.max_height
) AS expired_unmined
FROM transactions
JOIN notes ON notes.id_tx = transactions.id_tx
JOIN blocks_max_height
LEFT JOIN blocks ON blocks.height = transactions.block
LEFT JOIN sent_note_counts
ON sent_note_counts.account_id = notes.account_id
AND sent_note_counts.id_tx = notes.id_tx
GROUP BY notes.account_id, transactions.id_tx",
// v_tx_outputs
"CREATE VIEW v_tx_outputs AS
SELECT received_notes.tx AS id_tx,
2 AS output_pool,
received_notes.output_index AS output_index,
sent_notes.from_account AS from_account,
received_notes.account AS to_account,
NULL AS to_address,
received_notes.value AS value,
received_notes.is_change AS is_change,
received_notes.memo AS memo
FROM received_notes
LEFT JOIN sent_notes
ON (sent_notes.tx, sent_notes.output_pool, sent_notes.output_index) =
(received_notes.tx, 2, sent_notes.output_index)
UNION
SELECT transactions.id_tx AS id_tx,
0 AS output_pool,
utxos.prevout_idx AS output_index,
NULL AS from_account,
utxos.received_by_account AS to_account,
utxos.address AS to_address,
utxos.value_zat AS value,
false AS is_change,
NULL AS memo
FROM utxos
JOIN transactions
ON transactions.txid = utxos.prevout_txid
UNION
SELECT sent_notes.tx AS id_tx,
sent_notes.output_pool AS output_pool,
sent_notes.output_index AS output_index,
sent_notes.from_account AS from_account,
received_notes.account AS to_account,
sent_notes.to_address AS to_address,
sent_notes.value AS value,
false AS is_change,
sent_notes.memo AS memo
FROM sent_notes
LEFT JOIN received_notes
ON (sent_notes.tx, sent_notes.output_pool, sent_notes.output_index) =
(received_notes.tx, 2, received_notes.output_index)
WHERE received_notes.is_change IS NULL
OR received_notes.is_change = 0",
];

let mut views_query = db_data
Expand Down
4 changes: 4 additions & 0 deletions zcash_client_sqlite/src/wallet/init/migrations.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ mod initial_setup;
mod sent_notes_to_internal;
mod ufvk_support;
mod utxos_table;
mod v_transactions_net;

use schemer_rusqlite::RusqliteMigration;
use secrecy::SecretVec;
Expand All @@ -25,6 +26,8 @@ pub(super) fn all_migrations<P: consensus::Parameters + 'static>(
// add_utxo_account /
// \ /
// add_transaction_views
// /
// v_transactions_net
vec![
Box::new(initial_setup::Migration {}),
Box::new(utxos_table::Migration {}),
Expand All @@ -40,5 +43,6 @@ pub(super) fn all_migrations<P: consensus::Parameters + 'static>(
}),
Box::new(sent_notes_to_internal::Migration {}),
Box::new(add_transaction_views::Migration),
Box::new(v_transactions_net::Migration),
]
}
6 changes: 3 additions & 3 deletions zcash_client_sqlite/src/wallet/init/migrations/add_transaction_views.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ mod tests {

use crate::{
tests,
wallet::init::{init_wallet_db, init_wallet_db_internal, migrations::addresses_table},
wallet::init::{init_wallet_db_internal, migrations::addresses_table},
WalletDb,
};

Expand Down Expand Up @@ -345,7 +345,7 @@ mod tests {
VALUES (0, 4, 0, '', 7, '', 'c', true, X'63');",
).unwrap();

init_wallet_db(&mut db_data, None).unwrap();
init_wallet_db_internal(&mut db_data, None, &[super::MIGRATION_ID]).unwrap();

let mut q = db_data
.conn
Expand Down Expand Up @@ -476,7 +476,7 @@ mod tests {
)
.unwrap();

init_wallet_db(&mut db_data, None).unwrap();
init_wallet_db_internal(&mut db_data, None, &[super::MIGRATION_ID]).unwrap();

let fee = db_data
.conn
Expand Down
Loading