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

Use DateTime32 for block times and lock times #2211

Closed
2 tasks
Tracked by #2311
teor2345 opened this issue May 27, 2021 · 1 comment
Closed
2 tasks
Tracked by #2311

Use DateTime32 for block times and lock times #2211

teor2345 opened this issue May 27, 2021 · 1 comment
Labels
A-network Area: Network protocol updates or fixes C-security Category: Security issues I-consensus Zebra breaks a Zcash consensus rule I-invalid-data Zebra relies on invalid or untrusted data, or sends invalid data I-panic Zebra panics with an internal error message I-slow Problems with performance or responsiveness I-usability Zebra is hard to understand or use

Comments

@teor2345
Copy link
Contributor

teor2345 commented May 27, 2021

Scheduling

This risk is acceptable for the stable release, but we need to fix it before we support lightwalletd.

Is your feature request related to a problem? Please describe.

In #2210, we created a DateTime32 type, and used it in MetaAddr.

But Zcash also uses 32-bit times in the block header and transaction lock time. This risks panics or networking bugs, particularly when Zebra starts generating transactions.

Describe the solution you'd like

  • Replace block::Header.time with DateTime32
  • Replace Transaction lock times with DateTime32, across all versions

We should also change any APIs that use those fields.

These changes should allow us to simplify some related code.

Describe alternatives you've considered

We could not do this, and accept the risk.

We could use a u32, but then we'd have to do a bunch of conversions manually.

@teor2345 teor2345 added C-bug Category: This is a bug A-rust Area: Updates to Rust code S-needs-triage Status: A bug report needs triage P-Medium C-security Category: Security issues I-consensus Zebra breaks a Zcash consensus rule I-panic Zebra panics with an internal error message I-slow Problems with performance or responsiveness I-usability Zebra is hard to understand or use I-invalid-data Zebra relies on invalid or untrusted data, or sends invalid data A-network Area: Network protocol updates or fixes labels May 27, 2021
@mpguerra mpguerra removed the S-needs-triage Status: A bug report needs triage label Jun 4, 2021
@teor2345 teor2345 removed C-bug Category: This is a bug A-rust Area: Updates to Rust code labels Dec 16, 2021
@teor2345
Copy link
Contributor Author

teor2345 commented Mar 1, 2022

This is a useful validation improvement, but it's not urgent.

@mpguerra mpguerra closed this as not planned Won't fix, can't repro, duplicate, stale May 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-network Area: Network protocol updates or fixes C-security Category: Security issues I-consensus Zebra breaks a Zcash consensus rule I-invalid-data Zebra relies on invalid or untrusted data, or sends invalid data I-panic Zebra panics with an internal error message I-slow Problems with performance or responsiveness I-usability Zebra is hard to understand or use
Projects
None yet
Development

No branches or pull requests

2 participants