Skip to content

Selected Zebra upstream issues

Jump to bottom
dimxy edited this page Jun 13, 2023 · 38 revisions

Here is selected Zebra upstream repo's issues since Aug, 1, 2023 (we forked from it on Aug, 4, 2023)
State on: Mar, 21, 2023 (and some added after this date)

Sample query to filter the issues: is:issue closed:>=2022-08-01 label:C-bug

See current Epic with the release goals

Epic: Zebra Release Candidate #3096
Epic: Improvements from Zebra Audit #6277

Closed and fixed bugs since 2022 Aug 01 (when we forked)

Fix "duplicate transparent spend" caused by mempool validation race condition #6132
Fixed in PR fix(mempool): Re-verify transactions that were verified at a different tip height #6154 - but it is stated that for readability this PR was split further into PR #6155.
Btw this looks similar to the algo in ActivateBestChain() in the komodod codebase

Zebra returns "error: could not convert the input string to a hash or height" with zcash/lightwalletd #6085
Seems fixes some concurrency issue when height is passed to getblock

Use error code -1 for incorrect RPC parameters #6054
This issue probably is not relevant until we merge the getblocktemplate code

Transaction nLockTIme check (CheckFinalTx) against MTP when adding to mempool possible missed #5984
Already fixed

Unclear implementation of block "timestamp against prev" (time-too-old) consensus rule #5914
This was actually implemented in our code but seems they refactored this

Remove duplicate read::hash() and read::hash_by_height() consensus-critical code #5910
Consensus refactoring

relay (fRelayTxes) field when parse Message::Version from the remote node should be optional #5713
already implemented in our code

Fix frequent "synced block height too far ahead of the tip: dropped downloaded block" errors #5420
We probably have seen such errors too

Use BestChainUtxo rather than AwaitUtxo in the mempool transaction validator
Looks like a bug important to fix
Do they remove txns from mempool if best chain changes like komodod's CTxMempool::removeForReorg?
Note that zebra always checks mempool transactions inputs only aganist txns in blocks (never for txns in mempool) - this is their TODO

Make StateService requests into concurrent ReadStateService requests #5102
This is actually a big refactoring to make writing async
Note: closed and reopened, it is stated that also fixes https://github.com/ZcashFoundation/zebra/issues/4937
Two PRs:

Disable rpc_server_spawn_port_conflict* tests on macOS #5043
I think I saw this on my mac running cargo test
Not high priority

Run JSON-RPC queries in multiple threads #5012
Wallet client performance improvement

Tracking: Commit blocks to state using a separate task #4937
Big performance improvement announced, worth merging
Quote: "Zebra takes 10-15 minutes to commit some blocks to the state while checkpointing... This is unacceptable performance"
Stated that this issue is fixed in #5102 however there are more PRs:
change(state): Run most StateService read requests without shared mutable chain state #5132
change(state): Write finalized blocks to the state in a separate thread, to avoid network and RPC hangs #5134
change(state): Move the finalized queue to the StateService #5152
change(state): Write non-finalized blocks to the state in a separate thread, to avoid network and RPC hangs #5257

Send the state tip height in Version messages #4875
Before zebra sent ht 0 what was not good for dns seeders (not sure this is needed for kmd)

Switch zebrad to a non-blocking tracing logger #4834
Performance improvement, might be deadlocks when the log buffers fill up.

Revert deserializing state transactions in rayon threads #4831
We probably wont see this issue until merge PR #4805

Open the database in a blocking tokio thread #4821
allows to do other tasks, Fixed by PR #5228

Revert note commitment and history trees when forking non-finalized chains #4794
we do not need it as it's for private txns

Move CPU-heavy proof preparation into the batch cryptography thread #4789
seems this is for private txns

Mini-Epic: Stop tokio tasks running for a long time and blocking other tasks #4747
Issue: Zebra can't sync all the way to the tip, because some tokio tasks run for a long time, and block other tasks
I saw similar things but I thought it was related to differences in komodo p2p, but maybe this was the reason

Stop assuming testnet when parsing keys and addresses #4691
Fixed by #5476

Make sync, inbound, and block verifier check if a block hash is in any chain or any queue #862 This bug was created 2,5 years ago, closed on Sept 2022, but reopened and fixed. So seems important. It actually does optimisation to prevent the block downloader from downloading blocks that are already in any chain (before it only the best chain was checked). Fixed in PR #6335. See also closed follow-up PR #6397

Begin adding bugs closed after 21 Mar 2023

NCC-E005955-XVE zebra-chain: Inconsistent error management in Add and Sub for Height #6279
Refactored the Height object after the Audit

change(state): Stop re-downloading blocks that are in non-finalized side chains #6335
Note: interesting but marked as optional task in zebra audit Epic: Improvements from Zebra Audit

can_fork_chain_at() should ignore blocks below the finalized tip #6388

NCC-E005955-7DU zebra-network: Fragile State Transition During Address Book Update #6672

Stop using private IP addresses by default #3117 ...

Closed but not implemented bugs (with reasons like not high priority etc):

sending a Request::Peers request to a zcashd node results in a spurious Response::Peers #5421
Improves zebra behaviour when addr p2p is pushed by nodes
Reopened

Disable code timers unless a specific zebrad feature is activated

Avoid an AwaitUtxo race condition when switching to the non-finalized state #5126

Check for downloaded hashes in a batch #5103

Scale lookahead limit based on upcoming checkpoint sizes #5101

Speed up opening the database #4822

Check address and key lengths when parsing strings #4692

Zebra falsely estimates that it's close to the tip when the network connection goes down #4649
I think I have seen this bug; they decided put it away

Prefer faster peers, but if all peers are slow, still connect to them #4226

Some DNS seeders and block explorers don't show Zebra nodes #1904
Not sure if this is resolved

change(state): Stop re-downloading blocks that are in state queues #6397
This is a PR, seems closed because of bad implementation, see also prev PR 6335

Return peer metadata from connect_isolated functions #4678
Note: fixes remote_version use to properly verify remote peer min ver

Still opened bugs:

TODO: check maybe some are already closed

Fix invalid transparent spends in getblocktemplate RPC responses #6195
Apparently coinbase maturity is not checked in mempool due to demanding for high performance but this creates problems for miners

Fix repeated block timeouts during initial sync #5709
Declared not a priority issue

Avoid temporary failures verifying the first non-finalized block #5125
Hah, I saw this too and fixed in our codebase (check!)
they partially fixed this in PR #5257

Zebra should support separate local bind and external advertise addresses #1890 This is about adding 'external address' param, like in komodo. Note that zebra, like komodo, also can learn its external address Zebra should auto-discover its own external IP address #1893, apparently from the connected node

Issues not marked as bugs but important or interesting

Do contextual validation on mempool transactions using the state best tip #5376
See the PR change(mempool): Contextually validates mempool transactions in best chain #5716

Implement transparent P2PKH mining address support #5719
Extends coinbase address

Epic: Support for RPC calls used by mining pools #5234 (closed)
getblocktemplate, submitblock RPC calls and more

Interesting PRs with no issue

change(mempool): Re-verify mempool transactions after a chain fork, rather than re-downloading them all #5841
Still open. Continuing mempool fixing

fix(mempool): Stop ignoring some transaction broadcasts #6230
Improves syncing

Implement the getpeerinfo RPC #5723 PR 5951
Update my trivial implementation

fix(mempool): Process chain tip actions that occur during poll_ready() execution before returning #6155
Improves tip action processing (Tip::reset) in mempool: re-verifies if tip reset. Seems worth pulling. I believe it also improves PR #5841

change(mempool): Re-verify mempool transactions after a chain fork, rather than re-downloading them all #5841
See also PR #6155

...

Clone this wiki locally