Skip to content

BlockStoreProtocol #19799

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.

Sign up for GitHub

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

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

BlockStoreProtocol #19799

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
49422b7
BlockStoreProtocol
richardkiss Jul 10, 2025
165f628
Deal with `.db_wrapper`
richardkiss Jul 12, 2025
8046b01
mypy
richardkiss Jul 12, 2025
67986f9
ruff
richardkiss Jul 12, 2025
edb6d3f
ruff
richardkiss Jul 12, 2025
f1f195e
mypy
richardkiss Jul 12, 2025
f1a3dc0
ruff
richardkiss Jul 12, 2025
6a4f2a1
minimize diffs
richardkiss Jul 12, 2025
2458f01
mindiffs
richardkiss Jul 12, 2025
7fbc2f7
Move batching and simplify protocol.
richardkiss Jul 12, 2025
6a92c7a
Move `GeneratorBlockInfo`.
richardkiss Jul 14, 2025
6cee193
Simplify protocol
richardkiss Jul 16, 2025
2c7c884
Use `BlockStore` locally, for now
richardkiss Jul 16, 2025
4ac3d58
Revert "Move `GeneratorBlockInfo`."
richardkiss Jul 17, 2025
2c71a1f
Improve exceptions
richardkiss Jul 18, 2025
c97a282
fixup
richardkiss Aug 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions chia/_tests/blockchain/blockchain_test_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from __future__ import annotations

from typing import Optional
from typing import Optional, cast

from chia_rs import FullBlock, SpendBundleConditions
from chia_rs.sized_ints import uint32, uint64
Expand All @@ -10,19 +10,20 @@
from chia.consensus.blockchain import AddBlockResult, Blockchain
from chia.consensus.difficulty_adjustment import get_next_sub_slot_iters_and_difficulty
from chia.consensus.multiprocess_validation import PreValidationResult, pre_validate_block
from chia.full_node.block_store import BlockStore
from chia.types.validation_state import ValidationState
from chia.util.errors import Err


async def check_block_store_invariant(bc: Blockchain):
db_wrapper = bc.block_store.db_wrapper
block_store = cast(BlockStore, bc.block_store)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is ugly for sure, but we can get away with it because we know that bc has been populated with a BlockStore instance here.

In future PRs, I expect I will have to rework some tests pretty significantly.


if db_wrapper.db_version == 1:
if block_store.db_wrapper.db_version == 1:
return

in_chain = set()
max_height = -1
async with bc.block_store.transaction() as conn:
async with block_store.transaction() as conn:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The BlockStoreProtocol now has a .transaction generic async context manager which for BlockStore does the same thing as the prior code.

async with conn.execute("SELECT height, in_main_chain FROM full_blocks") as cursor:
rows = await cursor.fetchall()
for row in rows:
Expand Down
36 changes: 36 additions & 0 deletions chia/consensus/block_store_protocol.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
from __future__ import annotations

from contextlib import AbstractAsyncContextManager
from typing import Any, Optional, Protocol

from chia_rs import BlockRecord, FullBlock, SubEpochChallengeSegment
from chia_rs.sized_bytes import bytes32
from chia_rs.sized_ints import uint32


class BlockStoreProtocol(Protocol):
async def add_full_block(self, header_hash: bytes32, block: FullBlock, block_record: BlockRecord) -> None: ...
def get_block_from_cache(self, header_hash: bytes32) -> Optional[FullBlock]: ...
async def get_full_block(self, header_hash: bytes32) -> Optional[FullBlock]: ...
async def get_generator(self, header_hash: bytes32) -> Optional[bytes]: ...
async def get_generators_at(self, heights: set[uint32]) -> dict[uint32, bytes]: ...
async def get_block_record(self, header_hash: bytes32) -> Optional[BlockRecord]: ...
async def get_block_records_in_range(self, start: int, stop: int) -> dict[bytes32, BlockRecord]: ...
async def get_block_records_by_hash(self, header_hashes: list[bytes32]) -> list[BlockRecord]: ...
async def get_blocks_by_hash(self, header_hashes: list[bytes32]) -> list[FullBlock]: ...
async def persist_sub_epoch_challenge_segments(
self, ses_block_hash: bytes32, segments: list[SubEpochChallengeSegment]
) -> None: ...
async def get_sub_epoch_challenge_segments(
self,
ses_block_hash: bytes32,
) -> Optional[list[SubEpochChallengeSegment]]: ...
async def rollback(self, height: int) -> None: ...
def rollback_cache_block(self, header_hash: bytes32) -> None: ...
async def set_in_chain(self, header_hashes: list[tuple[bytes32]]) -> None: ...
async def set_peak(self, header_hash: bytes32) -> None: ...
async def get_block_records_close_to_peak(
self, blocks_n: int
) -> tuple[dict[bytes32, BlockRecord], Optional[bytes32]]: ...
async def get_prev_hash(self, header_hash: bytes32) -> bytes32: ...
def transaction(self) -> AbstractAsyncContextManager[Any]: ...
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a good reason to use Any instead of aiosqlite.Connection as the type parameter here?
Doing so would preserve some type safety, and make it easier to refactor (with confidence) into a generic type in the future, when such type exists.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, right now, the only concrete implementation of this protocol returns a aiosqlite.Connection, but it's not actually used anywhere except in tests. I think full_node rather just goes to the DBWrapper2 to start a transaction. So yeah, this is pretty useless and dumb which is why I think my next step should really be a ConsensusStoreProtocol to abstract away the DBWrapper2/sqlite3 stuff. Mostly I just didn't want consensus to have to depend on aiosqlite since it's just a detail.

I think ConsensusStore should actually have an async context manager that returns a thing that you actually do the writes to, and that can have a non-trivial type (although it will be a protocol too).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree it's dumb. The main goal of this PR is to remove chia.full_node as a dep of chia.consensus. Once that's done, I can do some better refactoring of storage stuff around a new ConsensusStoreProtocol. Because of the layer violation that is DBWrapper2 (which forces all stores to be in the same sqlite3 DB), it's kind of hard to type this in a useful way.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it almost seems that those things ought to happen in the opposite order. to avoid Any and cast() as a temporary measure in between

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to remove the circular dependencies between chia.consensus and chia.full_node first so I can be assured that chia.full_node can't interfere with changes to the chia.consensus interface.

6 changes: 3 additions & 3 deletions chia/consensus/blockchain.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
from chia.consensus.block_body_validation import ForkInfo, validate_block_body
from chia.consensus.block_header_validation import validate_unfinished_header_block
from chia.consensus.block_height_map import BlockHeightMap
from chia.consensus.block_store_protocol import BlockStoreProtocol
from chia.consensus.coin_store_protocol import CoinStoreProtocol
from chia.consensus.cost_calculator import NPCResult
from chia.consensus.difficulty_adjustment import get_next_sub_slot_iters_and_difficulty
Expand All @@ -35,7 +36,6 @@
from chia.consensus.generator_tools import get_block_header
from chia.consensus.get_block_generator import get_block_generator
from chia.consensus.multiprocess_validation import PreValidationResult
from chia.full_node.block_store import BlockStore
from chia.types.blockchain_format.coin import Coin
from chia.types.blockchain_format.vdf import VDFInfo
from chia.types.coin_record import CoinRecord
Expand Down Expand Up @@ -104,7 +104,7 @@ class Blockchain:
# Unspent Store
coin_store: CoinStoreProtocol
# Store
block_store: BlockStore
block_store: BlockStoreProtocol
# Used to verify blocks in parallel
pool: Executor
# Set holding seen compact proofs, in order to avoid duplicates.
Expand All @@ -122,7 +122,7 @@ class Blockchain:
@staticmethod
async def create(
coin_store: CoinStoreProtocol,
block_store: BlockStore,
block_store: BlockStoreProtocol,
height_map: BlockHeightMap,
consensus_constants: ConsensusConstants,
reserved_cores: int,
Expand Down
13 changes: 9 additions & 4 deletions chia/full_node/block_store.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,8 @@
import logging
import sqlite3
from contextlib import AbstractAsyncContextManager
from typing import Optional
from typing import Any, Optional

import aiosqlite
import typing_extensions
import zstd
from chia_rs import BlockRecord, FullBlock, SubEpochChallengeSegment, SubEpochSegments
Expand All @@ -19,6 +18,11 @@
from chia.util.errors import Err
from chia.util.lru_cache import LRUCache


class UnsupportedDatabaseVersionError(Exception):
"""Raised when a method is called with an unsupported database version."""


log = logging.getLogger(__name__)


Expand Down Expand Up @@ -192,7 +196,7 @@ async def get_sub_epoch_challenge_segments(
return challenge_segments
return None

def transaction(self) -> AbstractAsyncContextManager[aiosqlite.Connection]:
def transaction(self) -> AbstractAsyncContextManager[Any]:
return self.db_wrapper.writer()

def get_block_from_cache(self, header_hash: bytes32) -> Optional[FullBlock]:
Expand Down Expand Up @@ -478,7 +482,8 @@ async def get_block_bytes_in_range(
No orphan blocks.
"""

assert self.db_wrapper.db_version == 2
if self.db_wrapper.db_version != 2:
raise UnsupportedDatabaseVersionError("get_block_bytes_in_range requires DB version 2")
async with self.db_wrapper.reader_no_transaction() as conn:
async with conn.execute(
"SELECT block FROM full_blocks WHERE height >= ? AND height <= ? AND in_main_chain=1",
Expand Down
1 change: 0 additions & 1 deletion tach.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ path = "chia.consensus"
depends_on = [
"chia.types",
"chia.util",
{ path = "chia.full_node", deprecated = false },
Copy link
Contributor Author

@richardkiss richardkiss Jul 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The chia.consensus module no longer depends upon chia.full_node. Hooray!

]

[[modules]]
Expand Down
Loading