From 93315ba95c54bd0730c964998bfc0c64080b3c04 Mon Sep 17 00:00:00 2001
From: Alex Ostrovski <slowli@users.noreply.github.com>
Date: Fri, 7 Jun 2024 16:37:08 +0300
Subject: [PATCH] fix(api): Fix getting pending block (#2186)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

## 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`.
---
 .../api_server/src/web3/namespaces/debug.rs   |  4 +++
 .../api_server/src/web3/namespaces/eth.rs     | 26 ++++++++++++++++---
 2 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/core/node/api_server/src/web3/namespaces/debug.rs b/core/node/api_server/src/web3/namespaces/debug.rs
index 400711de859..17d02661740 100644
--- a/core/node/api_server/src/web3/namespaces/debug.rs
+++ b/core/node/api_server/src/web3/namespaces/debug.rs
@@ -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)
diff --git a/core/node/api_server/src/web3/namespaces/eth.rs b/core/node/api_server/src/web3/namespaces/eth.rs
index b1541f7261b..e2224ce92cd 100644
--- a/core/node/api_server/src/web3/namespaces/eth.rs
+++ b/core/node/api_server/src/web3/namespaces/eth.rs
@@ -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)
@@ -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)
@@ -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)
@@ -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
                 };