Skip to content
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.

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

zcash_client_sqlite: v_tx_outputs describes one of two tx outputs as not from the account #1330

Open
AArnott opened this issue Mar 27, 2024 · 8 comments
Assignees
Labels
bug S-in-progress Status: Work is currently in progress on this item.

Comments

@AArnott
Copy link
Contributor

AArnott commented Mar 27, 2024

My MainNet test account with UFVK:

uview1lkkp8j2cp4tww0xe89yry3jdsc7dz6ga3dklgqptragp599v96q0zrcsdpzldqlw36v7rgjtwgac806n0duduyr2x2ry4hu78vmsrr2yw9gq2kty95r6989krxarw79rg2tz8fct45d5jhr8jr4qxt7andm5q3c6tt5qry4angpkg409ptus0sxegkh4p07n6ysxfvsklt9q7fddz3nwu88qgt9s48v93j84yrals506x5tk6e0fsuekxm29sykxu0vv3a3xqlzlu7dtrhrtpe2khptgf68c7yhszjw4062uhypz6zefjhymf8u8n6lgyexpncr9749qkgwygpu7ec6refeeq3qvzkfrw7dvkym4z9ya76ts7nzdzp4zapa5pwc0wga8pqx4kgkp289zvlyhh59hq93k8pzmwl8k5sxp3k3vj6rpljrl3ntr3ch3xwwc0qzc6470nzpawl64wgpkxwtj0gfalmhh583x4l75qw2lrqw8kc0y

Birthday height: 2,224,314

txid: e1565f350459b9a048c93454e3f370bc945133ed80658ea238f73c26647ad41e
block height: 2420013

Note how of the top two rows below, which belong to the same transaction, exactly one of the rows has a from_account_id of 1 while the other is NULL, despite belonging to the same transaction:
image
The transaction in question effectively unshielded funds within the same account. The shielded 'change' was correctly attributed with from_account_id of 1, while the unshielded output was not, making it look like novel income.

@str4d
Copy link
Contributor

str4d commented Mar 27, 2024

Not fixing this for Zashi 1.0; manually created "send to self" transactions (i.e. not using the shield_transparent_funds API) are not in scope.

@str4d str4d self-assigned this Jul 31, 2024
@str4d
Copy link
Contributor

str4d commented Jul 31, 2024

I just tested this with current main, and I see the expected database contents:
image

Note that in my database, both rows have from_account_id set to 1, while to_account_id is only set to 1 for the change (and is NULL for the other). That is the opposite of what you saw. However, it shouldn't have been the case that to_account_id and to_address were both set at once. That makes me think there was a bug in the view at the time you opened this issue, which has subsequently been fixed. If so, it was most likely fixed by #1402 which altered the database and views so transparent outputs are treated as similarly as possible to shielded notes.

@str4d str4d added the S-in-progress Status: Work is currently in progress on this item. label Jul 31, 2024
@daira daira added this to the ZIP 320 and related changes milestone Aug 9, 2024
@nuttycom
Copy link
Contributor

@AArnott I believe that this is now fixed; please reopen if this still occurs after upgrading to zcash_client_sqlite 0.11

@AArnott
Copy link
Contributor Author

AArnott commented Sep 26, 2024

I can't re-open. Can you re-open please, @nuttycom? I just tested it, after verifying that I'm using the crate version 0.11.2, and it repros as before (though this time, with the to_address column NULL for both rows as @str4d expected).
image

@nuttycom
Copy link
Contributor

@AArnott here is what I see using bf8b39a

I think this is fixed; the distinction might be in the sync process you're using?

image

@AArnott
Copy link
Contributor Author

AArnott commented Nov 14, 2024

the distinction might be in the sync process you're using?

It could be. But I wonder whether the difference would be due to a bug in my code or because I call into LRZ differently than your client does.
If you're interested in trying it out, I can give you directions to repro using my CLI client.

@nuttycom
Copy link
Contributor

the distinction might be in the sync process you're using?

It could be. But I wonder whether the difference would be due to a bug in my code or because I call into LRZ differently than your client does. If you're interested in trying it out, I can give you directions to repro using my CLI client.

Sure, that'd be great.

@nuttycom
Copy link
Contributor

One thing I notice when running this with zec-sqlite-cli: prior to running the "enhance" action, this is the state of the database:
image

I believe that you have access to this repository; it might be worth looking at how your transaction download and transaction graph traversal code compares to https://github.com/Electric-Coin-Company/zec-sqlite-cli/blob/main/src/commands/enhance.rs#L107-L190

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug S-in-progress Status: Work is currently in progress on this item.
Projects
None yet
Development

No branches or pull requests

4 participants