search

zcash / librustzcash

Rust-language assets for Zcash
Other
333 stars 249 forks source link

zcash_client_sqlite reports transaction orchard output twice, and doubles account_balance_delta #1324

Open AArnott opened 6 months ago

AArnott commented 6 months ago

My MainNet test account with UFVK:

uview1lkkp8j2cp4tww0xe89yry3jdsc7dz6ga3dklgqptragp599v96q0zrcsdpzldqlw36v7rgjtwgac806n0duduyr2x2ry4hu78vmsrr2yw9gq2kty95r6989krxarw79rg2tz8fct45d5jhr8jr4qxt7andm5q3c6tt5qry4angpkg409ptus0sxegkh4p07n6ysxfvsklt9q7fddz3nwu88qgt9s48v93j84yrals506x5tk6e0fsuekxm29sykxu0vv3a3xqlzlu7dtrhrtpe2khptgf68c7yhszjw4062uhypz6zefjhymf8u8n6lgyexpncr9749qkgwygpu7ec6refeeq3qvzkfrw7dvkym4z9ya76ts7nzdzp4zapa5pwc0wga8pqx4kgkp289zvlyhh59hq93k8pzmwl8k5sxp3k3vj6rpljrl3ntr3ch3xwwc0qzc6470nzpawl64wgpkxwtj0gfalmhh583x4l75qw2lrqw8kc0y

Birthday height: 2,224,314

txid: 89299683638fc1d183a442683b217e86205d1f9a5c167e4eafab378a6f1bd5c9 block height: 2315025

In v_tx_outputs this transaction has 3 rows:

output_pool output_index from_account_id to_account_id to_address value is_change
3 0 1 1 NULL 390000 0
3 0 1 1 u1vu9c0a589r6z5c4qs3cj5eqr2zc653f42kfkqv06kyqa90zlj28f5s8vydkf0kmar8xypfnslskfcupjcdea5sq7gjdp6l5cnqktxyrx 390000 0
3 1 1 NULL u1g6uss4x95fcydl356wysr8a2qmvc0j7ysrjtn8wuxf69p8k9aysjz5n4zsqvqvjnrr5knms87n8uq98twgm66xyghzhrdrwrggkevwnm 100000 0

Note how the first two rows claim to describe the same output, but one specifies the address and one doesn't. I believe this double representation of the same output is related to why v_transactions reports an account_balance_delta value of -220000 for this transaction instead of the expected -110000.

str4d commented 6 months ago

v_tx_outputs queries notes twice: https://github.com/zcash/librustzcash/blob/3ccc14f501c569bc948735d6320b09ce89c1d57c/zcash_client_sqlite/src/wallet/init.rs#L957-L970 https://github.com/zcash/librustzcash/blob/3ccc14f501c569bc948735d6320b09ce89c1d57c/zcash_client_sqlite/src/wallet/init.rs#L983-L997

Both of these join v_received_notes and sent_notes, so there is immediate possibility of duplication of change notes (which are added to both tables). What is meant to prevent this is the WHERE COALESCE(v_received_notes.is_change, 0) = 0 which should exclude change notes selected via the sent_notes table; there might be an Orchard bug here.

Additionally, for cross-account notes (outgoing from one, incoming from the other) that won't show up as change notes, there is no filtering. We're not addressing this for the current release series targeting Zashi 1.0 because that doesn't support multiple accounts; this should be fixed as part of #1305 or something similar.