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

depsbuild(deps): Bump zcash_client_backend for zebra-scan #8567

Closed
wants to merge 4 commits into from

Conversation

oxarbitrage
Copy link
Contributor

Motivation

We want to upgrade zcash_client_backend for the zebra-scan crate.

I believe the combination of #8522 and this PR (which is based in #8560) will make all the builds and tests to pass. That is what i am going to be trying next.

However, there are some open questions on this PR that might need discussion. I will add review comments in the places i am most concerned.

@oxarbitrage oxarbitrage added A-dependencies Area: Dependency file updates do-not-merge Tells Mergify not to merge this PR A-blockchain-scanner Area: Blockchain scanner of shielded transactions P-Medium ⚡ labels May 24, 2024
Comment on lines +221 to +237
/// Convert keys from the Zebra parsed format to the scanning format using a dummy account and the diversifiable full viewing key.
pub fn new_parsed_keys(
parsed_keys: HashMap<String, (Vec<DiversifiableFullViewingKey>, Vec<SaplingIvk>)>,
) -> HashMap<String, ScanningKeys<AccountId, (AccountId, Scope)>> {
let mut new_parsed_keys = HashMap::new();
for keys in parsed_keys.iter() {
for dfvk in keys.1 .0.iter() {
let ufvk = UnifiedFullViewingKey::new(Some(dfvk.clone()));
let scanning_keys =
ScanningKeys::from_account_ufvks([(AccountId::ZERO, ufvk.unwrap())]);

new_parsed_keys.insert(keys.0.clone(), scanning_keys);
}
}
new_parsed_keys
}

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 is dirty code, need cleanup.

ufvk.unwrap(),
)]);

let res = scan_block(&Network::Mainnet, &block, sapling_tree_size, &scanning_keys).unwrap();

// The response should have one transaction relevant to the key we provided.
assert_eq!(res.transactions().len(), 1);
Copy link
Contributor Author

@oxarbitrage oxarbitrage May 24, 2024

Choose a reason for hiding this comment

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

This is passing, means that the modified scan_block is working.

@oxarbitrage oxarbitrage changed the title deps(zcash_client_backend): Upgrade for zebra-scan depsbuild(deps): Bump zcash_client_backend for zebra-scan May 24, 2024
@oxarbitrage
Copy link
Contributor Author

Closing in favor of #8568

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-blockchain-scanner Area: Blockchain scanner of shielded transactions A-dependencies Area: Dependency file updates do-not-merge Tells Mergify not to merge this PR P-Medium ⚡
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant