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

wallet: shortchain history should include base block #9601

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

0xFFFC0000
Copy link
Collaborator

@0xFFFC0000 0xFFFC0000 commented Dec 4, 2024

ELI5: The Problem

Your wallet keeps track of part of the blockchain, starting from a specific point (called the "offset"). Sometimes, the wallet might only have one block stored (the "base block") or no blocks at all.

The bug is that when the wallet has just the "base block" (and no newer ones), this block wasn't being included in the list of blocks that the wallet sends to check its history with the blockchain. This makes the wallet’s history incomplete and can break synchronization.

Example

Buggy Code:

  1. Wallet starts at block 1000 (offset = 1000), with no newer blocks (sz == 0).
  2. The code only adds the genesis block (block 0).
  3. Result: The list looks like this:

ids = [block_0]

Missing block 1000!

Fixed Code:

  1. Wallet starts at block 1000, with no newer blocks (sz == 0).
  2. The fix correctly adds the base block (block 1000) and the genesis block.
  3. Result: The list looks like this:

ids = [block_1000, block_0]

Now both blocks are included, and the history is accurate!

Short Summary of the Fix

The original get_short_chain_history method in the wallet2 class contained a bug where the base block (the block at m_blockchain.offset()) was not included in the ids list when sz == 0 (i.e., no blocks exist beyond the offset). This caused an incomplete short chain history in edge cases where the wallet had a minimal or empty blockchain view.

The fixed code ensures the correct behavior by:

  1. Adding the base block (m_blockchain[m_blockchain.offset()]) to the ids list if the wallet has valid data at or beyond the offset (m_blockchain.size() > m_blockchain.offset()).
  2. Always including the Genesis block (m_blockchain.genesis()), which is crucial for synchronization.

Impact of the Fix

The updated code properly includes the base block when sz == 0, ensuring the short chain history is accurate and complete in all scenarios. This fix resolves potential synchronization issues, particularly for wallets with minimal or partially synced blockchain data. It improves reliability during wallet synchronization and avoids missing critical blocks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants