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 Zebra's tip estimate match zcashd exactly #3954

Closed
teor2345 opened this issue Mar 24, 2022 · 1 comment
Closed

Make Zebra's tip estimate match zcashd exactly #3954

teor2345 opened this issue Mar 24, 2022 · 1 comment
Labels
A-rpc Area: Remote Procedure Call interfaces C-bug Category: This is a bug lightwalletd any work associated with lightwalletd

Comments

@teor2345
Copy link
Contributor

teor2345 commented Mar 24, 2022

Motivation

Zebra's chain tip estimate is slightly different to zcashd's.

We don't think this will cause any issues, but if it does, they will come up in tests like:

Specifications

Here is what the zcashd code does:

  1. If zcashd has stopped syncing, return the current height: https://github.com/zcash/zcash/blob/6cd5b8792bbe0a3cc1064e1884914415f0c6e7d7/src/rpc/blockchain.cpp#L1068-L1071
  2. If the block time is ahead of the current time, return the current height: https://github.com/zcash/zcash/blob/f7006e9a33ca37ae88520e434ee5c2b2498c1cc6/src/metrics.cpp#L141-L143
  3. Otherwise, estimate the tip height: https://github.com/zcash/zcash/blob/f7006e9a33ca37ae88520e434ee5c2b2498c1cc6/src/metrics.cpp#L145-L160

Zebra APIs

Here's the SyncStatus method that returns if we're close to the tip:

/// Check if the synchronization is likely close to the chain tip.
pub fn is_close_to_tip(&self) -> bool {

Here's the buggy NetworkChainTipHeightEstimator code. Removing negative times should simplify it a lot.
(The bug fix will probably make some tests fail, Janito can help fix them.)
https://github.com/ZcashFoundation/zebra/blob/main/zebra-chain/src/chain_tip/network_chain_tip_height_estimator.rs#L111-L120

Tasks

Here's one way to do this fix:

  1. Move SyncStatus to zebra_node_services
  2. Add SyncStatus to RpcImpl, and use it to check if we're close to the tip
  3. Fix NetworkChainTipHeightEstimator to return the current height if the block time is ahead of the current time

Related Work

This ticket was split out of:

@teor2345 teor2345 added C-bug Category: This is a bug S-needs-triage Status: A bug report needs triage P-Optional ✨ A-rpc Area: Remote Procedure Call interfaces lightwalletd any work associated with lightwalletd labels Mar 24, 2022
@teor2345
Copy link
Contributor Author

teor2345 commented Apr 1, 2022

I've been running tests with lightwalletd, and it keeps on running without this fix.

We can re-open this ticket if we discover that it is actually needed during testing.

@teor2345 teor2345 closed this as completed Apr 1, 2022
@mpguerra mpguerra removed the S-needs-triage Status: A bug report needs triage label Sep 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rpc Area: Remote Procedure Call interfaces C-bug Category: This is a bug lightwalletd any work associated with lightwalletd
Projects
None yet
Development

No branches or pull requests

2 participants