Skip to content
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.

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

feat(rpc/ots): implement ots_getTransactionBySenderAndNonce #9263

Merged
merged 18 commits into from
Jul 9, 2024
4 changes: 2 additions & 2 deletions crates/rpc/rpc-api/src/otterscan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use reth_rpc_types::{
BlockDetails, ContractCreator, InternalOperation, OtsBlockTransactions, TraceEntry,
TransactionsWithReceipts,
},
Header, Transaction,
Header,
};

/// Otterscan rpc interface.
Expand Down Expand Up @@ -87,7 +87,7 @@ pub trait Otterscan {
&self,
sender: Address,
nonce: u64,
) -> RpcResult<Option<Transaction>>;
) -> RpcResult<Option<TxHash>>;

/// Gets the transaction hash and the address who created a contract.
#[method(name = "getContractCreator")]
Expand Down
12 changes: 5 additions & 7 deletions crates/rpc/rpc-builder/tests/it/http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -336,12 +336,10 @@ where
.err()
.unwrap()
));
assert!(is_unimplemented(
OtterscanClient::get_transaction_by_sender_and_nonce(client, sender, nonce,)
.await
.err()
.unwrap()
));
assert!(OtterscanClient::get_transaction_by_sender_and_nonce(client, sender, nonce)
.await
.err()
.is_none());
assert!(OtterscanClient::get_contract_creator(client, address).await.unwrap().is_none());
}

Expand Down Expand Up @@ -552,7 +550,7 @@ async fn test_eth_logs_args() {
let client = handle.http_client().unwrap();

let mut params = ArrayParams::default();
params.insert( serde_json::json!({"blockHash":"0x58dc57ab582b282c143424bd01e8d923cddfdcda9455bad02a29522f6274a948"})).unwrap();
params.insert(serde_json::json!({"blockHash":"0x58dc57ab582b282c143424bd01e8d923cddfdcda9455bad02a29522f6274a948"})).unwrap();

let resp = client.request::<Vec<Log>, _>("eth_getLogs", params).await;
// block does not exist
Expand Down
134 changes: 112 additions & 22 deletions crates/rpc/rpc/src/otterscan.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use alloy_primitives::Bytes;
use async_trait::async_trait;
use futures::future::BoxFuture;
use jsonrpsee::core::RpcResult;
use reth_primitives::{Address, BlockId, BlockNumberOrTag, TxHash, B256};
use reth_rpc_api::{EthApiServer, OtterscanServer};
Expand All @@ -14,7 +15,7 @@ use reth_rpc_types::{
},
parity::{Action, CreateAction, CreateOutput, TraceOutput},
},
BlockTransactions, Header, Transaction,
BlockTransactions, Header,
};
use revm_inspectors::{
tracing::TracingInspectorConfig,
Expand All @@ -30,6 +31,41 @@ pub struct OtterscanApi<Eth> {
eth: Eth,
}

/// Performs a binary search within a given block range to find the desired block number.
///
/// The binary search is performed by calling the provided asynchronous `check` closure on the
/// blocks of the range. The closure should return a future representing the result of performing
/// the desired logic at a given block. The future resolves to an `bool` where:
/// - `true` indicates that the condition has been matched, but we can try to find a lower block to
/// make the condition more matchable.
/// - `false` indicates that the condition not matched, so the target is not present in the current
/// block and should continue searching in a higher range.
///
/// Args:
/// - `low`: The lower bound of the block range (inclusive).
/// - `high`: The upper bound of the block range (inclusive).
/// - `check`: A closure that performs the desired logic at a given block.
async fn binary_search<'a, F>(low: u64, high: u64, check: F) -> RpcResult<u64>
where
F: Fn(u64) -> BoxFuture<'a, RpcResult<bool>>,
{
let mut low = low;
let mut high = high;
let mut num = high;

while low <= high {
let mid = (low + high) / 2;
if check(mid).await? {
high = mid - 1;
num = mid;
} else {
low = mid + 1
}
}

Ok(num)
}

impl<Eth> OtterscanApi<Eth> {
/// Creates a new instance of `Otterscan`.
pub const fn new(eth: Eth) -> Self {
Expand Down Expand Up @@ -208,10 +244,51 @@ where
/// Handler for `getTransactionBySenderAndNonce`
async fn get_transaction_by_sender_and_nonce(
&self,
_sender: Address,
_nonce: u64,
) -> RpcResult<Option<Transaction>> {
Err(internal_rpc_err("unimplemented"))
sender: Address,
nonce: u64,
) -> RpcResult<Option<TxHash>> {
// Check if the sender is a contract
if self.has_code(sender, None).await? {
return Ok(None)
}

let highest =
EthApiServer::transaction_count(&self.eth, sender, None).await?.saturating_to::<u64>();
emhane marked this conversation as resolved.
Show resolved Hide resolved

// If the nonce is higher or equal to the highest nonce, the transaction is pending or not
// exists.
if nonce >= highest {
jsvisa marked this conversation as resolved.
Show resolved Hide resolved
return Ok(None)
}

// perform a binary search over the block range to find the block in which the sender's
// nonce reached the requested nonce.
let num = binary_search(1, self.eth.block_number()?.saturating_to(), |mid| {
Box::pin(async move {
let mid_nonce =
EthApiServer::transaction_count(&self.eth, sender, Some(mid.into()))
.await?
.saturating_to::<u64>();

// The `transaction_count` returns the `nonce` after the transaction was
// executed, which is the state of the account after the block, and we need to find
// the transaction whose nonce is the pre-state, so need to compare with `nonce`(no
// equal).
Ok(mid_nonce > nonce)
})
})
.await?;

let Some(BlockTransactions::Full(transactions)) =
self.eth.block_by_number(num.into(), true).await?.map(|block| block.inner.transactions)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
self.eth.block_by_number(num.into(), true).await?.map(|block| block.inner.transactions)
self.eth.block_by_number((num - 1).into(), true).await?.map(|block| block.inner.transactions)

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 think the change is not correct, if found nonce in block N, need to get txs in block N, not N-1

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Erigon is searching nonce from the AccountsHistory table by tx index, but we are searching in the block body instead, so the implementation is different.

eg: if we found the nonce changed in block 10, then iterate the txs in block 9 can't get anything.

Copy link
Member

Choose a reason for hiding this comment

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

if you added some test to show this, I would be strongly convinced! you can compare without output from region tests for same inputs

else {
return Err(EthApiError::UnknownBlockNumber.into());
};

Ok(transactions
.into_iter()
.find(|tx| tx.from == sender && tx.nonce == nonce)
.map(|tx| tx.hash))
}

/// Handler for `getContractCreator`
Expand All @@ -220,23 +297,12 @@ where
return Ok(None);
}

// use binary search from block [1, latest block number] to find the first block where the
// contract was deployed
let mut low = 1;
let mut high = self.eth.block_number()?.saturating_to::<u64>();
let mut num = high;

while low <= high {
let mid = (low + high) / 2;
if self.eth.get_code(address, Some(mid.into())).await?.is_empty() {
// not found in current block, need to search in the later blocks
low = mid + 1;
} else {
// found in current block, try to find a lower block
high = mid - 1;
num = mid;
}
}
let num = binary_search(1, self.eth.block_number()?.saturating_to(), |mid| {
Box::pin(
async move { Ok(!self.eth.get_code(address, Some(mid.into())).await?.is_empty()) },
)
})
.await?;

let traces = self
.eth
Expand Down Expand Up @@ -281,3 +347,27 @@ where
Ok(found)
}
}

#[cfg(test)]
mod tests {
use super::*;

#[tokio::test]
async fn test_binary_search() {
// in the middle
let num = binary_search(1, 10, |mid| Box::pin(async move { Ok(mid >= 5) })).await;
assert_eq!(num, Ok(5));

// in the upper
let num = binary_search(1, 10, |mid| Box::pin(async move { Ok(mid >= 7) })).await;
assert_eq!(num, Ok(7));

// in the lower
let num = binary_search(1, 10, |mid| Box::pin(async move { Ok(mid >= 1) })).await;
assert_eq!(num, Ok(1));

// high than the upper
let num = binary_search(1, 10, |mid| Box::pin(async move { Ok(mid >= 11) })).await;
assert_eq!(num, Ok(10));
}
}
Loading