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

Validate funding stream addresses #160

Closed
wants to merge 2 commits into from

Conversation

oxarbitrage
Copy link
Owner

Motivation

We need to validate the coinbase output addresses against the funding stream receivers. Part of ZcashFoundation#338

Specifications

[TBA]

Designs

[TBA]

Solution

This is based on ZcashFoundation#3017 and it still needs a bunch of work. It is published so we can have the big picture of all we need to get done.

It needs adjustments in almost all calls and further test coverage. Right now it is passing test blocks and a quick test will be to sync up to tip however i didnt made this yet as the code is constantly changed.

Review

Will be useful for @teor4321 as they are reviewing ZcashFoundation#3017

Reviewer Checklist

  • Code implements Specs and Designs
  • Tests for Expected Behaviour
  • Tests for Errors

Follow Up Work

Comment on lines +129 to +132
address_hash = address_hash[2..22].to_vec();
address_hash.insert(0, OpCode::Push20Bytes as u8);
address_hash.insert(0, OpCode::Hash160 as u8);
address_hash.insert(address_hash.len(), OpCode::Equal as u8);
Copy link

Choose a reason for hiding this comment

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

It feels a bit weird to be doing this in Rust, when the rest of our script validation happens in zcash_script, which calls zcashd's C++ implementation. But adding another unsafe C++ function could be complicated, and it's more risky. (And this function might not even exist in zcashd.)

So I think this code is simple enough to re-implement. We just need to test it carefully.

Are there any zcashd test vectors we could use?

@oxarbitrage
Copy link
Owner Author

Replaced by ZcashFoundation#3040

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