### `BlockId` refactoring status and next steps
1. Some of core traits are already cleaned from `BlockId`, however there are still some methods that are related to `BlockId` or *block number*. The summary of use cases is in the sections to follow.
1. Reworking `header` function led to some call sequences that are far from being perfect, [example](https://github.com/paritytech/substrate/pull/12874/commits/d06359cab8e6eb78d625b7520b37496252139a22).
Namely, in some modules there is a requirement to fetch `header` using _block number_, current [PR](https://github.com/paritytech/substrate/pull/12874) works around this by calling `header(hash(n))` combo.
1. In some cases `expect_header(block_id)` is called to hide the combo, (as `expect_header` was not reworked yet)
1. To solve this _"problem"_ `HeaderBackend` trait could be extended with dedicated functions for fetching header by _block
number_:
-- `header_for_number(NumberFor<Block>) -> Result<Option<Block::Header>>`
-- `expect_header_for_number(NumberFor<Block>) -> Result<Block::Header>`
-- `header(Block::Hash) -> Result<Option<Block::Header>>`
-- `expect_header(Block::Hash) -> Result<Block::Header>`
New naming may also make finding all block-number usages and their refactorization easier. I would follow with this naming convention in needed for the rest of methods (e.g. `block`, `block_status`)
Implementation of `_for_number` methods could make use of new _service_ which will move number-hash mapping out of DB.
### Where the `BlockId` is currently used:
#### Still in _client_ API:
- trait `Chain` _common block validation for consensus_
-- `block_status`
- trait `BlockIdTo`
-- `to_hash`
-- `to_number`
- trait `HeaderBackend`:
-- `status`
-- `expect_header`
-- `block_hash_from_id`
-- `block_number_from_id`
- trait `BlockBackend`
-- `block`
-- `block_status`
- impl `Client`: `Chain`, `BlockBackend`, `CallApiAt`, `HeaderBackend`, `BlockIdTo`, `BlockBuilderProvider`
-- `code_at`
-- `runtime_version_at`
-- `block_status`
- trait `ExecutionExtensions`
-- `manager_and_extensions`
-- `extensions`
#### In `runtime_api()` _subsystem_:
Refactoring here is probably doable, requires some adjustments in _runtime-related_ macros. Question if we want to get rid of `BlockId` parameter to runtime functions. This maybe inconvenient in some cases. Maybe we should introduce dual functions (following `_for_number` approach) at the top-level runtime API.
- trait `GetRuntimeVersionAt`
-- `runtime_version`
- trait `ApiExt`
-- `has_api`
-- `has_api_with`
-- `api_version`
- trait `CallApiAt`
-- `runtime_version_at`
-- `state_at`
- `WasmSubstitute`
-- `matches` _(operates on block number)_
-- `get`
- trait `CallExecutor`
-- `prove_execution`
-- `contextual_call`
-- `call`
-- `runtime_version`
- trait `LocalCallExecutor`
-- `check_override`
#### Other usages
- Benchmark::run (from - to)
-- `measure_block`
-- `consumed_weight`
- impl `PowBlockImport`
-- `check_inherents`
- trait `PowAlgorithm`
-- `verify`
- DB
Something that we want to get rid of:
-- API: `remove_from_db`, `read_header`, `read_db`
-- `HeaderBackend::status`
-- internal implementation `Backend::prune_block`
- InMem:
-- `Blockchain::id` (blockid -> hash)
-- `HeaderBackend::status`
- `AuraVerifier`
-- `check_inherents` (_runtime_api_ called)
- `BabeVerifier`
-- `check_inherents` (_runtime_api_ called)
-- `check_and_report_equivocation`
- `Beefy`
-- `expect_validator_set`
- `BlockBuilderProvider`
-- `new_block`
-- `new_block_at`
- BasicAuthorship `Proposer`
-- impl uses `BlockId`
- `BlockRequestHandler`
-- `handle_request` / `get_block_response`
#### Network tests utils
- `PeerClient`
-- `has_state_at`
- `Peer`
-- `generate_blocks_at`
-- `push_blocks_at`
-- `push_blocks_at_without_informing_sync`
-- `push_blocks_at_without_announcing`
-- `generate_tx_blocks_at`
#### utils
- impl `BlockNumberOrHash`
-- `parse`
- `pub fn check_block`
### Where _Block number_ is used
_not completed yet_
- `have_state_at(hash, number)`
- `hash(n)` usages
```
client/consensus/babe/src/lib.rs
1775 pub fn revert
client/db/src/lib.rs
fn prune_blocks:
1753 fn prune_displaced_branches
client/finality-grandpa/src/warp_proof.rs|100
WarpSyncProof::generate
client/finality-grandpa/src/environment.rs
1249 pub(crate) fn finalize_block
client/rpc/src/chain/mod.rs|83
ChainBackend::block_hash
client/finality-grandpa/src/import.rs|366
GrandpaBlockImport::make_authorities_changes
client/service/src/client/client.rs|1036
Client::block_status
client/network/sync/src/lib.rs|804
ChainSync::on_block_data / AncestorSearch
primitives/blockchain/src/backend.rs|227
trait Backend::best_containing
primitives/blockchain/src/backend.rs|53
HeaderBackend::block_hash_from_id
```