ronantakizawa/codereview-bench huggingface dataset:
Repository: Chia-Network/chia-blockchain
File: chia/consensus/coin_store_protocol.py (Python)

Code before:
        names: list[bytes32],
        start_height: uint32 = uint32(0),
        end_height: uint32 = uint32((2**32) - 1),
    ) -> list[CoinRecord]:
        """
        Returns the coin records for a list of coin names
        """

    async def get_coin_states_by_puzzle_hashes(
        self,
        include_spent_coins: bool,
        puzzle_hashes: set[bytes32],
        min_height: uint32 = uint32(0),
        *,
        max_items: int = 50000,
    ) -> set[CoinState]:
        """
        Returns the coin states for a set of puzzle hashes
        """

    async def get_coin_records_by_parent_ids(
        self,
        include_spent_coins: bool,
        parent_ids: list[bytes32],
        start_height: uint32 = uint32(0),
        end_height: uint32 = uint32((2**32) - 1),
    ) -> list[CoinRecord]:
        """
        Returns the coin records for a list of parent ids
        """

    async def get_coin_states_by_ids(
        self,
        include_spent_coins: bool,
        coin_ids: Collection[bytes32],
        min_height: uint32 = uint32(0),
        *,
        max_height: uint32 = uint32.MAXIMUM,
        max_items: int = 50000,
    ) -> list[CoinState]:
        """
        Returns the coin states for a collection of coin ids
        """

    async def batch_coin_states_by_puzzle_hashes(
        self,
        puzzle_hashes: list[bytes32],
        *,
        min_height: uint32 = uint32(0),
        include_spent: bool = True,
        include_unspent: bool = True,

Review comment (suggestion):
```suggestion
        start_height: uint32 = ...,
        end_height: uint32 = ...,
```

Code after:
        names: list[bytes32],
        start_height: uint32 = ...,
        end_height: uint32 = ...,
    ) -> list[CoinRecord]:
        """
        Returns the coin records for a list of coin names
        """

    async def get_coin_states_by_puzzle_hashes(
        self,
        include_spent_coins: bool,
        puzzle_hashes: set[bytes32],
        min_height: uint32 = uint32(0),
        *,
        max_items: int = ...,
    ) -> set[CoinState]:
        """
        Returns the coin states for a set of puzzle hashes
        """

    async def get_coin_records_by_parent_ids(
        self,
        include_spent_coins: bool,
        parent_ids: list[bytes32],
        start_height: uint32 = ...,
        end_height: uint32 = ...,
    ) -> list[CoinRecord]:
        """
        Returns the coin records for a list of parent ids
        """

    async def get_coin_states_by_ids(
        self,
        include_spent_coins: bool,
        coin_ids: Collection[bytes32],
        min_height: uint32 = uint32(0),
        *,
        max_height: uint32 = ...,
        max_items: int = ...,
    ) -> list[CoinState]:
        """
        Returns the coin states for a collection of coin ids
        """

    async def batch_coin_states_by_puzzle_hashes(
        self,
        puzzle_hashes: list[bytes32],
        *,
        min_height: uint32 = ...,
        include_spent: bool = ...,
        include_unspent: bool = ...,
Repository: Chia-Network/chia-blockchain
File: chia/consensus/coin_store_protocol.py (Python)

Code before:
        Returns the coin records for a list of parent ids
        """

    async def get_coin_states_by_ids(
        self,
        include_spent_coins: bool,
        coin_ids: Collection[bytes32],
        min_height: uint32 = uint32(0),
        *,
        max_height: uint32 = uint32.MAXIMUM,
        max_items: int = 50000,
    ) -> list[CoinState]:
        """
        Returns the coin states for a collection of coin ids
        """

    async def batch_coin_states_by_puzzle_hashes(
        self,
        puzzle_hashes: list[bytes32],
        *,
        min_height: uint32 = uint32(0),
        include_spent: bool = True,
        include_unspent: bool = True,
        include_hinted: bool = True,
        min_amount: uint64 = uint64(0),
        max_items: int = 50000,
    ) -> tuple[list[CoinState], Optional[uint32]]:
        """
        Returns the coin states, as well as the next block height (or `None` if finished).
        """

    async def get_unspent_lineage_info_for_puzzle_hash(self, puzzle_hash: bytes32) -> Optional[UnspentLineageInfo]:
        """
        Lookup the most recent unspent lineage that matches a puzzle hash
        """

    async def rollback_to_block(self, block_index: int) -> dict[bytes32, CoinRecord]:
        """
        Rolls back the blockchain to the specified block index
        """


Review comment (suggestion):
```suggestion
        min_height: uint32 = ...,
        include_spent: bool = ...,
        include_unspent: bool = ...,
        include_hinted: bool = ...,
        min_amount: uint64 = ...,
        max_items: int = ...,
```

Code after:
        Returns the coin records for a list of parent ids
        """

    async def get_coin_states_by_ids(
        self,
        include_spent_coins: bool,
        coin_ids: Collection[bytes32],
        min_height: uint32 = uint32(0),
        *,
        max_height: uint32 = ...,
        max_items: int = ...,
    ) -> list[CoinState]:
        """
        Returns the coin states for a collection of coin ids
        """

    async def batch_coin_states_by_puzzle_hashes(
        self,
        puzzle_hashes: list[bytes32],
        *,
        min_height: uint32 = ...,
        include_spent: bool = ...,
        include_unspent: bool = ...,
        include_hinted: bool = ...,
        min_amount: uint64 = ...,
        max_items: int = ...,
    ) -> tuple[list[CoinState], Optional[uint32]]:
        """
        Returns the coin states, as well as the next block height (or `None` if finished).
        """

    async def get_unspent_lineage_info_for_puzzle_hash(self, puzzle_hash: bytes32) -> Optional[UnspentLineageInfo]:
        """
        Lookup the most recent unspent lineage that matches a puzzle hash
        """

    async def rollback_to_block(self, block_index: int) -> dict[bytes32, CoinRecord]:
        """
        Rolls back the blockchain to the specified block index
        """

    # DEPRECATED: do not use in new code
    async def is_empty(self) -> bool:
        """
        Returns True if the coin store is empty
        """
Repository: Chia-Network/chia-blockchain
File: tools/validate_rpcs.py (Python)

Code before:
    "--block_spends",
    help="Test get_block_spends",
    is_flag=True,
    type=bool,
    default=False,
)
@click.option(
    "-ar",
    "--additions_and_removals",
    help="Test get_additions_and_removals",
    is_flag=True,
    type=bool,
    default=False,
)
@click.option(
    "-s",
    "--start-height",
    help="Start height for the RPC calls",
    type=int,
    default=None,
)
@click.option(
    "-e",
    "--end-height",
    help="End height for the RPC calls",
    type=int,
    default=None,
)
@click.option(
    "-c",
    "--concurrent-requests",
    help="Number of concurrent requests to make to the RPC endpoints",
    type=int,
    default=DEFAULT_REQUESTS_PER_BATCH,
)
def cli(
    root_path: str,
    rpc_port: Optional[int],
    spends_with_conditions: bool,
    block_spends: bool,
    additions_and_removals: bool,
    concurrent_requests: int,
    start_height: Optional[int] = None,
    end_height: Optional[int] = None,
) -> None:
    root_path_path = Path(root_path)
    requests_per_batch = 0
    if spends_with_conditions:
        requests_per_batch += 1
    if block_spends:
        requests_per_batch += 1

Review comment (suggestion):
as I understand it, the only thing you need from the config is the `height-to-hash`. you could just take a path to this file and keep things simple. It's not a bit deal since you've already made it be pulled from config, it's just another moving part that can break in the future

Code after:
    type=bool,
    default=False,
)
@click.option(
    "-s",
    "--start-height",
    help="Start height for the RPC calls",
    type=int,
    default=None,
)
@click.option(
    "-e",
    "--end-height",
    help="End height for the RPC calls",
    type=int,
    default=None,
)
@click.option(
    "-r",
    "--pipeline-depth",
    help="Set the number of concurrent RPC calls to make.",
    type=int,
    default=DEFAULT_PIPELINE_DEPTH,
)
def cli(
    root_path: str,
    rpc_port: Optional[int],
    spends_with_conditions: bool,
    block_spends: bool,
    additions_and_removals: bool,
    pipeline_depth: int,
    start_height: Optional[int] = None,
    end_height: Optional[int] = None,
) -> None:
    root_path_path = Path(root_path)
    requests_per_batch = 0
    if spends_with_conditions:
        requests_per_batch += 1
    if block_spends:
        requests_per_batch += 1
    if additions_and_removals:
        requests_per_batch += 1
    if requests_per_batch == 0:
        print("No RPC calls selected. Exiting.")
        return
    if start_height is None:
        start_height = 0
    asyncio.run(
        cli_async(
            root_path=root_path_path,
            rpc_port=rpc_port,
Repository: Chia-Network/chia-blockchain
File: chia/_tests/core/data_layer/test_data_store.py (Python)

Code before:
        store_id=store_id,
        existing_generation=0,
        target_generation=1,
        root_hashes=[root_hash],
        server_info=sinfo,
        client_foldername=files_path,
        timeout=aiohttp.ClientTimeout(total=15, sock_connect=5),
        log=log,
        proxy_url="",
        downloader=None,
    )
    assert is_success == success

    async with data_store.db_wrapper.reader() as reader:
        async with reader.execute("SELECT COUNT(*) FROM ids") as cursor:
            row_count = await cursor.fetchone()
            assert row_count is not None
            if success:
                assert row_count[0] > 0
            else:
                assert row_count[0] == 0

    if success:
        assert sum(1 for _ in keys_value_path.rglob("*") if _.is_file()) > 0
    else:
        assert sum(1 for _ in keys_value_path.rglob("*") if _.is_file()) == 0


Review comment (suggestion):
```suggestion
    if success:
        assert sum(1 for path in keys_value_path.rglob("*") if path.is_file()) > 0
    else:
        assert sum(1 for path in keys_value_path.rglob("*") if path.is_file()) == 0
```

Code after:
    is_success = await insert_from_delta_file(
        data_store=data_store,
        store_id=store_id,
        existing_generation=0,
        target_generation=1,
        root_hashes=[root_hash],
        server_info=sinfo,
        client_foldername=files_path,
        timeout=aiohttp.ClientTimeout(total=15, sock_connect=5),
        log=log,
        proxy_url="",
        downloader=None,
    )
    assert is_success == success

    async with data_store.db_wrapper.reader() as reader:
        async with reader.execute("SELECT COUNT(*) FROM ids") as cursor:
            row_count = await cursor.fetchone()
            assert row_count is not None
            if success:
                assert row_count[0] > 0
            else:
                assert row_count[0] == 0

    if success:
        assert sum(1 for path in keys_value_path.rglob("*") if path.is_file()) > 0
    else:
        assert sum(1 for path in keys_value_path.rglob("*") if path.is_file()) == 0


@pytest.mark.anyio
async def test_manage_kv_files(
    data_store: DataStore,
    store_id: bytes32,
    seeded_random: random.Random,
) -> None:
    num_keys = 1000
    num_files = 0
    keys_value_path = data_store.key_value_blobs_path.joinpath(store_id.hex())

    with pytest.raises(Exception, match="Test exception"):
        async with data_store.db_wrapper.writer() as writer:
            with data_store.manage_kv_files(store_id):
                for _ in range(num_keys):
                    use_file = seeded_random.choice([True, False])
                    assert data_store.prefer_db_kv_blob_length > 7
                    size = data_store.prefer_db_kv_blob_length + 1 if use_file else 8
                    key = seeded_random.randbytes(size)
                    value = seeded_random.randbytes(size)
                    await data_store.add_key_value(key, value, store_id, writer)
                    num_files += 2 * use_file
Repository: Chia-Network/chia-blockchain
File: chia/util/streamable.py (Python)

Code before:

@streamable
@dataclasses.dataclass(frozen=True)
class VersionedBlob(Streamable):
    version: uint16
    blob: bytes


@streamable
@dataclasses.dataclass(frozen=True)
class UInt32Range(Streamable):
    start: uint32 = uint32(0)
    stop: uint32 = uint32.MAXIMUM


@streamable
@dataclasses.dataclass(frozen=True)
class UInt64Range(Streamable):
    start: uint64 = uint64(0)
    stop: uint64 = uint64.MAXIMUM


_T_Enum = TypeVar("_T_Enum", bound=EnumMeta)


def streamable_enum(proxy: type[Any]) -> Callable[[_T_Enum], _T_Enum]:
    def streamable_enum_wrapper(cls: _T_Enum) -> _T_Enum:
        setattr(cls, "_streamable_proxy", proxy)
        setattr(cls, "_ignore_", ["_streamable_proxy"])
        return cls

    return streamable_enum_wrapper


Review comment (suggestion):
this, or can we require the proxy itself be streamable?

```suggestion
def streamable_enum(proxy: type[object]) -> Callable[[_T_Enum], _T_Enum]:
```

maybe not in this pr, but maybe we need a type that is like a union of the rust streamableness and `Streamable`.  or...

Code after:

@streamable
@dataclasses.dataclass(frozen=True)
class VersionedBlob(Streamable):
    version: uint16
    blob: bytes


@streamable
@dataclasses.dataclass(frozen=True)
class UInt32Range(Streamable):
    start: uint32 = uint32(0)
    stop: uint32 = uint32.MAXIMUM


@streamable
@dataclasses.dataclass(frozen=True)
class UInt64Range(Streamable):
    start: uint64 = uint64(0)
    stop: uint64 = uint64.MAXIMUM


_T_Enum = TypeVar("_T_Enum", bound=EnumMeta)


def streamable_enum(proxy: type[object]) -> Callable[[_T_Enum], _T_Enum]:
    def streamable_enum_wrapper(cls: _T_Enum) -> _T_Enum:
        setattr(cls, "_streamable_proxy", proxy)
        setattr(cls, "_ignore_", ["_streamable_proxy"])
        return cls

    return streamable_enum_wrapper
Repository: Chia-Network/chia-blockchain
File: chia/protocols/harvester_protocol.py (Python)

Code before:
    difficulty: uint64
    sub_slot_iters: uint64
    pool_contract_puzzle_hash: bytes32


@streamable
@dataclass(frozen=True)
class HarvesterHandshake(Streamable):
    farmer_public_keys: list[G1Element]
    pool_public_keys: list[G1Element]


@streamable
@dataclass(frozen=True)
class NewSignagePointHarvester(Streamable):
    challenge_hash: bytes32
    difficulty: uint64
    sub_slot_iters: uint64
    signage_point_index: uint8
    sp_hash: bytes32
    pool_difficulties: list[PoolDifficulty]
    filter_prefix_bits: uint8


# this message has the same message ID as NewSignagePointHarvester, but this
# message format is used if the protocol version is 0.0.27 or higher
@streamable
@dataclass(frozen=True)
class NewSignagePointHarvester2(Streamable):
    challenge_hash: bytes32
    difficulty: uint64
    sub_slot_iters: uint64
    signage_point_index: uint8
    sp_hash: bytes32
    pool_difficulties: list[PoolDifficulty]
    peak_height: uint32
    last_tx_height: uint32


@streamable
@dataclass(frozen=True)
class ProofOfSpaceFeeInfo(Streamable):
    applied_fee_threshold: uint32


@streamable
@dataclass(frozen=True)
class NewProofOfSpace(Streamable):
    challenge_hash: bytes32
    sp_hash: bytes32
    plot_identifier: str

Review comment (suggestion):
```suggestion
# message format is used if the protocol version is 0.0.37 or higher
```

Code after:
    difficulty: uint64
    sub_slot_iters: uint64
    pool_contract_puzzle_hash: bytes32


@streamable
@dataclass(frozen=True)
class HarvesterHandshake(Streamable):
    farmer_public_keys: list[G1Element]
    pool_public_keys: list[G1Element]


@streamable
@dataclass(frozen=True)
class NewSignagePointHarvester(Streamable):
    challenge_hash: bytes32
    difficulty: uint64
    sub_slot_iters: uint64
    signage_point_index: uint8
    sp_hash: bytes32
    pool_difficulties: list[PoolDifficulty]
    filter_prefix_bits: uint8


# this message has the same message ID as NewSignagePointHarvester, but this
# message format is used if the protocol version is 0.0.37 or higher
@streamable
@dataclass(frozen=True)
class NewSignagePointHarvester2(Streamable):
    challenge_hash: bytes32
    difficulty: uint64
    sub_slot_iters: uint64
    signage_point_index: uint8
    sp_hash: bytes32
    pool_difficulties: list[PoolDifficulty]
    peak_height: uint32
    last_tx_height: uint32


@streamable
@dataclass(frozen=True)
class ProofOfSpaceFeeInfo(Streamable):
    applied_fee_threshold: uint32


@streamable
@dataclass(frozen=True)
class NewProofOfSpace(Streamable):
    challenge_hash: bytes32
    sp_hash: bytes32
    plot_identifier: str
Repository: Chia-Network/chia-blockchain
File: chia/consensus/get_block_challenge.py (Python)

Code before:
                reversed_challenge_hashes += reversed(curr.finished_challenge_slot_hashes)
                if len(reversed_challenge_hashes) >= challenges_to_look_for:
                    break
            if curr.height == 0:
                assert curr.finished_challenge_slot_hashes is not None
                assert len(curr.finished_challenge_slot_hashes) > 0
                break
            curr = blocks.block_record(curr.prev_hash)
        challenge = reversed_challenge_hashes[challenges_to_look_for - 1]
    return challenge


def prev_tx_block(
    constants: ConsensusConstants,
    blocks: BlockRecordsProtocol,
    prev_b_hash: bytes32,
    sp_index: uint8,
    first_in_sub_slot: bool,
) -> uint32:
    if prev_b_hash == constants.GENESIS_CHALLENGE:
        return uint32(0)
    curr = blocks.block_record(prev_b_hash)
    before_slot = first_in_sub_slot
    while curr.height > 0:
        if curr.is_transaction_block and (before_slot or curr.signage_point_index < sp_index):
            break
        if curr.first_in_sub_slot:
            before_slot = True
        curr = blocks.block_record(curr.prev_hash)
    return curr.height


Review comment (performance):
if `blocks` is an empty cache (`BlockCache`), this would throw, wouldn't it? I think this is a change in behavior in some call sites

Code after:
                reversed_challenge_hashes += reversed(curr.finished_challenge_slot_hashes)
                if len(reversed_challenge_hashes) >= challenges_to_look_for:
                    break
            if curr.height == 0:
                assert curr.finished_challenge_slot_hashes is not None
                assert len(curr.finished_challenge_slot_hashes) > 0
                break
            curr = blocks.block_record(curr.prev_hash)
        challenge = reversed_challenge_hashes[challenges_to_look_for - 1]
    return challenge


# Returns the height of the previous transaction block up to the blocks signage point
# we use this for block validation since when the block is farmed we do not know the latest transaction block
# since a new one might be infused by the time the block is infused
def prev_tx_block(
    constants: ConsensusConstants,
    blocks: BlockRecordsProtocol,
    *,
    prev_b_hash: bytes32,
    sp_index: uint8,
    first_in_sub_slot: bool,
) -> uint32:
    if prev_b_hash == constants.GENESIS_CHALLENGE:
        return uint32(0)
    curr = blocks.block_record(prev_b_hash)
    before_slot = first_in_sub_slot
    while curr.height > 0:
        if curr.is_transaction_block and (before_slot or curr.signage_point_index < sp_index):
            break
        if curr.first_in_sub_slot:
            before_slot = True
        curr = blocks.block_record(curr.prev_hash)
    return curr.height
Repository: Chia-Network/chia-blockchain
File: chia/consensus/augmented_chain.py (Python)

Code before:
from __future__ import annotations

from typing import TYPE_CHECKING, ClassVar, cast

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

from chia.consensus.blockchain_interface import BlocksProtocol
from chia.util.errors import Err


class AugmentedBlockchainValidationError(Exception):
    pass


class AugmentedBlockchain:
    """
    This class wraps a BlocksProtocol and forwards calls to it, when
    looking up block records. It allows an in-memory cache of block records to
    fall back onto in case a block is not available in the underlying
    BlocksProtocol.
    This is especially useful when validating blocks in parallel. The batch of
    blocks will not have been added to the underlying blockchain until they've
    all been validated, but the validation requires them to be available as-if
    they were valid.
    """

    if TYPE_CHECKING:
        _protocol_check: ClassVar[BlocksProtocol] = cast("AugmentedBlockchain", None)

    _underlying: BlocksProtocol
    _extra_blocks: dict[bytes32, tuple[FullBlock, BlockRecord]]
    _height_to_hash: dict[uint32, bytes32]

    def __init__(self, underlying: BlocksProtocol) -> None:
        self._underlying = underlying
        self._extra_blocks = {}

Review comment (bug):
my understanding is that this is raised when we encounter a bug. Do you think it would make sense to derive from `AssertionError` to make that clear?

Code after:
from __future__ import annotations

from typing import TYPE_CHECKING, ClassVar, cast

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

from chia.consensus.blockchain_interface import BlocksProtocol
from chia.util.errors import Err


class AugmentedBlockchainValidationError(AssertionError):
    pass


class AugmentedBlockchain:
    """
    This class wraps a BlocksProtocol and forwards calls to it, when
    looking up block records. It allows an in-memory cache of block records to
    fall back onto in case a block is not available in the underlying
    BlocksProtocol.
    This is especially useful when validating blocks in parallel. The batch of
    blocks will not have been added to the underlying blockchain until they've
    all been validated, but the validation requires them to be available as-if
    they were valid.
    """

    if TYPE_CHECKING:
        _protocol_check: ClassVar[BlocksProtocol] = cast("AugmentedBlockchain", None)

    _underlying: BlocksProtocol
    _extra_blocks: dict[bytes32, tuple[FullBlock, BlockRecord]]
    _height_to_hash: dict[uint32, bytes32]

    def __init__(self, underlying: BlocksProtocol) -> None:
        self._underlying = underlying
        self._extra_blocks = {}