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

Stop assuming that all transparent addresses are on mainnet #1187

Closed
teor2345 opened this issue Oct 20, 2020 · 3 comments · Fixed by #1191
Closed

Stop assuming that all transparent addresses are on mainnet #1187

teor2345 opened this issue Oct 20, 2020 · 3 comments · Fixed by #1191
Labels
A-consensus Area: Consensus rule updates A-rust Area: Updates to Rust code C-bug Category: This is a bug

Comments

@teor2345
Copy link
Contributor

Currently, From<Script> for Address assumes that all addresses are on mainnet.

But Zebra needs to support mainnet and testnet.

We should delete From<Script> for Address, and use the From<ScriptAndNetwork> for Address in PR #1170.

We should also check for any other places in Zebra where we've assumed Mainnet.


Basically, given a script hash alone (as lock_script, the one we get from transaction outputs) it is not possible to know to which network it belongs, so i had to create a new type ScriptForNetwork and implement impl From<ScriptForNetwork> for Address to do the conversion.

Originally posted by @oxarbitrage in #1170 (comment)

@teor2345 teor2345 added A-consensus Area: Consensus rule updates A-rust Area: Updates to Rust code C-bug Category: This is a bug S-blocked Status: Blocked on other tasks S-needs-triage Status: A bug report needs triage labels Oct 20, 2020
@teor2345 teor2345 added this to the First Alpha Release milestone Oct 20, 2020
@teor2345
Copy link
Contributor Author

Blocked on #1170

Do we need to fix this bug in the first alpha release, or can it wait until later?

@dconnolly
Copy link
Contributor

Blocked on #1170

Do we need to fix this bug in the first alpha release, or can it wait until later?

I think we can get away with not fixing this for the first alpha, but probably not any betas and certainly not the first major/stable release.

@teor2345 teor2345 removed the S-blocked Status: Blocked on other tasks label Oct 21, 2020
@teor2345
Copy link
Contributor Author

It was easier than I thought, see #1191.

@mpguerra mpguerra removed the S-needs-triage Status: A bug report needs triage label Feb 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-consensus Area: Consensus rule updates A-rust Area: Updates to Rust code C-bug Category: This is a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants