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

Make transparent tests generic over backend implementation and move to zcash_client_backend crate #1576

Merged
merged 1 commit into from
Oct 18, 2024

Conversation

willemolding
Copy link
Contributor

Continuing the work of #1543 this moves the remainder of the tests specific to handling transparent transactions out of the zcash_client_sqlite crate and allows them to be consumed by different back-end implementations

Comment on lines -938 to -949
// Artificially delete the address from the addresses table so that
// we can ensure the update fails if the join doesn't work.
st.wallet()
.conn()
.execute(
"DELETE FROM addresses WHERE cached_transparent_receiver_address = ?",
[Some(taddr.encode(st.network()))],
)
.unwrap();

let res2 = st.wallet_mut().put_received_transparent_utxo(&utxo2);
assert_matches!(res2, Err(_));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This last part of the test was not migrated across as it requires a new wallet trait method. I decided to drop it instead as I am not convinced this is a useful thing to test.

@nuttycom @str4d are you able to comment if we actually do want to keep this and should require wallets to implement a delete_address method?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need to keep this, but it should probably be replaced by the equivalent test that we get an error if putting a UTXO with an address that isn't in the wallet (which can be done by crafting such a UTXO in the test).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opened #1581.

Copy link
Contributor

@str4d str4d left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK c266e60

@str4d str4d merged commit bf8b39a into zcash:main Oct 18, 2024
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants