Skip to content

Commit

Permalink
fix(api): Fix getting pending block (#2186)
Browse files Browse the repository at this point in the history
## What ❔

Fixes getting data for the pending block in `eth_getBlockByNumber` and a
couple of other methods.

## Why ❔

Getting a pending block should always return `null`, but right now it
actually doesn't. Caused by non-atomic reads from Postgres in the method
implementation: first, a pending block number is resolved, and then a
block with this number is fetched. Between these two reads, a block with
this number may be inserted to Postgres.

While it's somewhat unlikely that anyone will query a pending block,
it's still a correctness issue.

## Checklist

- [x] PR title corresponds to the body of PR (we generate changelog
entries from PRs).
- [x] Documentation comments have been added / updated.
- [x] Code has been formatted via `zk fmt` and `zk lint`.
- [x] Spellcheck has been run via `zk spellcheck`.
  • Loading branch information
slowli authored Jun 7, 2024
1 parent 9bcdabc commit 93315ba
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 3 deletions.
4 changes: 4 additions & 0 deletions core/node/api_server/src/web3/namespaces/debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,10 @@ impl DebugNamespace {
options: Option<TracerConfig>,
) -> Result<Vec<ResultDebugCall>, Web3Error> {
self.current_method().set_block_id(block_id);
if matches!(block_id, BlockId::Number(BlockNumber::Pending)) {
// See `EthNamespace::get_block_impl()` for an explanation why this check is needed.
return Ok(vec![]);
}

let only_top_call = options
.map(|options| options.tracer_config.only_top_call)
Expand Down
26 changes: 23 additions & 3 deletions core/node/api_server/src/web3/namespaces/eth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,8 +223,15 @@ impl EthNamespace {
full_transactions: bool,
) -> Result<Option<Block<TransactionVariant>>, Web3Error> {
self.current_method().set_block_id(block_id);
let mut storage = self.state.acquire_connection().await?;
if matches!(block_id, BlockId::Number(BlockNumber::Pending)) {
// Shortcut here on a somewhat unlikely case of the client requesting a pending block.
// Otherwise, since we don't read DB data in a transaction,
// we might resolve a block number to a block that will be inserted to the DB immediately after,
// and return `Ok(Some(_))`.
return Ok(None);
}

let mut storage = self.state.acquire_connection().await?;
self.state
.start_info
.ensure_not_pruned(block_id, &mut storage)
Expand Down Expand Up @@ -288,8 +295,12 @@ impl EthNamespace {
block_id: BlockId,
) -> Result<Option<U256>, Web3Error> {
self.current_method().set_block_id(block_id);
let mut storage = self.state.acquire_connection().await?;
if matches!(block_id, BlockId::Number(BlockNumber::Pending)) {
// See `get_block_impl()` for an explanation why this check is needed.
return Ok(None);
}

let mut storage = self.state.acquire_connection().await?;
self.state
.start_info
.ensure_not_pruned(block_id, &mut storage)
Expand Down Expand Up @@ -319,8 +330,12 @@ impl EthNamespace {
block_id: BlockId,
) -> Result<Option<Vec<TransactionReceipt>>, Web3Error> {
self.current_method().set_block_id(block_id);
let mut storage = self.state.acquire_connection().await?;
if matches!(block_id, BlockId::Number(BlockNumber::Pending)) {
// See `get_block_impl()` for an explanation why this check is needed.
return Ok(None);
}

let mut storage = self.state.acquire_connection().await?;
self.state
.start_info
.ensure_not_pruned(block_id, &mut storage)
Expand Down Expand Up @@ -457,6 +472,11 @@ impl EthNamespace {
.map_err(DalError::generalize)?,

TransactionId::Block(block_id, idx) => {
if matches!(block_id, BlockId::Number(BlockNumber::Pending)) {
// See `get_block_impl()` for an explanation why this check is needed.
return Ok(None);
}

let Ok(idx) = u32::try_from(idx) else {
return Ok(None); // index overflow means no transaction
};
Expand Down

0 comments on commit 93315ba

Please sign in to comment.