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

[Feature] RPC blocks support positive sequence #3824

Merged
merged 13 commits into from
Dec 28, 2022
10 changes: 8 additions & 2 deletions chain/api/src/chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,15 @@ pub trait ChainReader {
fn get_header(&self, hash: HashValue) -> Result<Option<BlockHeader>>;
fn get_header_by_number(&self, number: BlockNumber) -> Result<Option<BlockHeader>>;
fn get_block_by_number(&self, number: BlockNumber) -> Result<Option<Block>>;
/// Get latest `count` blocks before `number`. if `number` is absent, use head block number.
/// if `reverse` is true , get latest `count` blocks before `number`. if `number` is absent, use head block number.
/// if `reverse` is false , get `count` blocks after `number` . if `number` is absent, use head block number.
/// the block of `number` is inclusive.
fn get_blocks_by_number(&self, number: Option<BlockNumber>, count: u64) -> Result<Vec<Block>>;
fn get_blocks_by_number(
&self,
number: Option<BlockNumber>,
reverse: bool,
count: u64,
) -> Result<Vec<Block>>;
fn get_block(&self, hash: HashValue) -> Result<Option<Block>>;
/// Get block hash by block number, if not exist, return None
fn get_hash_by_number(&self, number: BlockNumber) -> Result<Option<HashValue>>;
Expand Down
2 changes: 1 addition & 1 deletion chain/api/src/message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ pub enum ChainRequest {
GetEventsByTxnHash {
txn_hash: HashValue,
},
GetBlocksByNumber(Option<BlockNumber>, u64),
GetBlocksByNumber(Option<BlockNumber>, bool, u64),
MainEvents(Filter),
GetBlockIds {
start_number: BlockNumber,
Expand Down
11 changes: 9 additions & 2 deletions chain/api/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,12 @@ pub trait ReadableChainService {
fn main_block_header_by_number(&self, number: BlockNumber) -> Result<Option<BlockHeader>>;
fn main_block_info_by_number(&self, number: BlockNumber) -> Result<Option<BlockInfo>>;
fn main_startup_info(&self) -> StartupInfo;
fn main_blocks_by_number(&self, number: Option<BlockNumber>, count: u64) -> Result<Vec<Block>>;
fn main_blocks_by_number(
&self,
number: Option<BlockNumber>,
reverse: bool,
count: u64,
) -> Result<Vec<Block>>;
fn get_main_events(&self, filter: Filter) -> Result<Vec<ContractEventInfo>>;
fn get_block_ids(
&self,
Expand Down Expand Up @@ -104,6 +109,7 @@ pub trait ChainAsyncService:
async fn main_blocks_by_number(
&self,
number: Option<BlockNumber>,
reverse: bool,
count: u64,
) -> Result<Vec<Block>>;
async fn main_block_header_by_number(&self, number: BlockNumber)
Expand Down Expand Up @@ -307,10 +313,11 @@ where
async fn main_blocks_by_number(
&self,
number: Option<BlockNumber>,
reverse: bool,
count: u64,
) -> Result<Vec<Block>> {
if let ChainResponse::BlockVec(blocks) = self
.send(ChainRequest::GetBlocksByNumber(number, count))
.send(ChainRequest::GetBlocksByNumber(number, reverse, count))
.await??
{
Ok(blocks)
Expand Down
13 changes: 9 additions & 4 deletions chain/service/src/chain_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,8 @@ impl ServiceHandler<Self, ChainRequest> for ChainReaderService {
ChainRequest::GetTransactionInfo(hash) => Ok(ChainResponse::TransactionInfo(
self.inner.get_transaction_info(hash)?,
)),
ChainRequest::GetBlocksByNumber(number, count) => Ok(ChainResponse::BlockVec(
self.inner.main_blocks_by_number(number, count)?,
ChainRequest::GetBlocksByNumber(number, reverse, count) => Ok(ChainResponse::BlockVec(
self.inner.main_blocks_by_number(number, reverse, count)?,
)),
ChainRequest::GetBlockTransactionInfos(block_id) => Ok(
ChainResponse::TransactionInfos(self.inner.get_block_txn_infos(block_id)?),
Expand Down Expand Up @@ -366,8 +366,13 @@ impl ReadableChainService for ChainReaderServiceInner {
fn main_startup_info(&self) -> StartupInfo {
self.startup_info.clone()
}
fn main_blocks_by_number(&self, number: Option<BlockNumber>, count: u64) -> Result<Vec<Block>> {
self.main.get_blocks_by_number(number, count)
fn main_blocks_by_number(
&self,
number: Option<BlockNumber>,
reverse: bool,
count: u64,
) -> Result<Vec<Block>> {
self.main.get_blocks_by_number(number, reverse, count)
}

fn get_main_events(&self, filter: Filter) -> Result<Vec<ContractEventInfo>> {
Expand Down
21 changes: 17 additions & 4 deletions chain/src/chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -561,17 +561,30 @@ impl ChainReader for BlockChain {
})
}

fn get_blocks_by_number(&self, number: Option<BlockNumber>, count: u64) -> Result<Vec<Block>> {
fn get_blocks_by_number(
&self,
number: Option<BlockNumber>,
reverse: bool,
count: u64,
) -> Result<Vec<Block>> {
let end_num = match number {
None => self.current_header().number(),
Some(number) => number,
};

let num_leaves = self.block_accumulator.num_leaves();
if end_num > num_leaves {

if end_num > num_leaves.saturating_sub(1) {
bail!("Can not find block by number {}", end_num);
}
let ids = self.get_block_ids(end_num, true, count)?;
};

let len = if !reverse && (end_num.saturating_add(count) > num_leaves.saturating_sub(1)) {
num_leaves.saturating_sub(end_num)
nkysg marked this conversation as resolved.
Show resolved Hide resolved
} else {
count
};

let ids = self.get_block_ids(end_num, reverse, len)?;
let block_opts = self.storage.get_blocks(ids)?;
let mut blocks = vec![];
for (idx, block) in block_opts.into_iter().enumerate() {
Expand Down
36 changes: 32 additions & 4 deletions chain/tests/test_block_chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -464,25 +464,53 @@ fn test_get_blocks_by_number() -> Result<()> {
let mut mock_chain = MockChain::new(ChainNetwork::new_test()).unwrap();
let blocks = mock_chain
.head()
.get_blocks_by_number(None, u64::max_value())?;
.get_blocks_by_number(None, true, u64::max_value())?;
assert_eq!(blocks.len(), 1, "at least genesis block should contains.");
let times = 10;
mock_chain.produce_and_apply_times(times).unwrap();

let blocks = mock_chain
.head()
.get_blocks_by_number(None, u64::max_value())?;
.get_blocks_by_number(None, true, u64::max_value())?;
assert_eq!(blocks.len(), 11);

let number = blocks.len() as u64;
let number = number.saturating_add(1);
let result =
mock_chain
.head()
.get_blocks_by_number(Some(blocks.len() as u64), true, u64::max_value());
assert!(
result.is_err(),
"result cannot find block by number {}",
number
);

let number = blocks.len() as u64;
let number = number.saturating_add(2);
let result = mock_chain
.head()
.get_blocks_by_number(Some(blocks.len() as u64 + 1), u64::max_value());
.get_blocks_by_number(Some(number), true, u64::max_value());
assert!(
result.is_err(),
"result cannot find block by number {}",
number
);

let blocks = mock_chain.head().get_blocks_by_number(Some(9), true, 3)?;
assert_eq!(blocks.len(), 3);

let blocks = mock_chain
.head()
.get_blocks_by_number(Some(0), false, u64::max_value())?;
assert_eq!(blocks.len(), 11);

let blocks = mock_chain
.head()
.get_blocks_by_number(Some(9), false, u64::max_value())?;
assert_eq!(blocks.len(), 2);
Copy link
Collaborator

Choose a reason for hiding this comment

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

这里需要加个Some(9), true, 3 类似这种测试用例吗


let blocks = mock_chain.head().get_blocks_by_number(Some(6), false, 3)?;
assert_eq!(blocks.len(), 3);

Ok(())
}
11 changes: 10 additions & 1 deletion cmd/starcoin/src/chain/list_block_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use crate::StarcoinOpt;
use anyhow::Result;
use clap::Parser;
use scmd::{CommandAction, ExecContext};
use starcoin_rpc_api::chain::GetBlocksOption;
use starcoin_rpc_api::types::BlockHeaderView;
use starcoin_types::block::BlockNumber;

Expand All @@ -17,6 +18,8 @@ pub struct ListBlockOpt {
number: Option<BlockNumber>,
#[clap(name = "count", long, short = 'c', default_value = "10")]
count: u64,
#[clap(name = "reverse", long, short = 'r', default_value = "true")]
reverse: bool,
}

pub struct ListBlockCommand;
Expand All @@ -33,7 +36,13 @@ impl CommandAction for ListBlockCommand {
) -> Result<Self::ReturnItem> {
let client = ctx.state().client();
let opt = ctx.opt();
let blocks = client.chain_get_blocks_by_number(opt.number, opt.count)?;
let blocks = client.chain_get_blocks_by_number(
opt.number,
opt.count,
Some(GetBlocksOption {
reverse: opt.reverse,
}),
)?;
let block_view = blocks.into_iter().map(|block| block.header).collect();
Ok(block_view)
}
Expand Down
2 changes: 1 addition & 1 deletion config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -562,6 +562,6 @@ pub fn check_open_fds_limit(max_files: u64) -> Result<(), ConfigError> {
}

#[cfg(not(unix))]
pub fn check_open_fds_limit(max_files: u64) -> Result<(), ConfigError> {
pub fn check_open_fds_limit(_max_files: u64) -> Result<(), ConfigError> {
Ok(())
}
17 changes: 17 additions & 0 deletions rpc/api/generated_rpc_schema/chain.json
Original file line number Diff line number Diff line change
Expand Up @@ -1366,6 +1366,23 @@
"format": "uint64",
"minimum": 0.0
}
},
{
"name": "option",
"schema": {
"$schema": "http://json-schema.org/draft-07/schema#",
"title": "Nullable_GetBlocksOption",
"type": [
"object",
"null"
],
"properties": {
"reverse": {
"default": true,
"type": "boolean"
}
}
}
}
],
"result": {
Expand Down
11 changes: 11 additions & 0 deletions rpc/api/src/chain/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ pub trait ChainApi {
&self,
number: Option<BlockNumber>,
count: u64,
option: Option<GetBlocksOption>,
) -> FutureResult<Vec<BlockView>>;
#[rpc(name = "chain.get_block_info_by_number")]
fn get_block_info_by_number(&self, number: BlockNumber) -> FutureResult<Option<BlockInfoView>>;
Expand Down Expand Up @@ -138,6 +139,16 @@ pub struct GetBlockOption {
pub raw: bool,
}

#[derive(Copy, Clone, Default, Serialize, Deserialize, JsonSchema)]
pub struct GetBlocksOption {
#[serde(default = "defautl_true")]
pub reverse: bool,
}

fn defautl_true() -> bool {
true
}

#[derive(Copy, Clone, Default, Serialize, Deserialize, JsonSchema)]
pub struct GetEventOption {
#[serde(default)]
Expand Down
13 changes: 10 additions & 3 deletions rpc/client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ use starcoin_abi_types::{FunctionABI, ModuleABI, StructInstantiation};
use starcoin_account_api::AccountInfo;
use starcoin_crypto::HashValue;
use starcoin_logger::{prelude::*, LogPattern};
use starcoin_rpc_api::chain::{GetBlockOption, GetEventOption, GetTransactionOption};
use starcoin_rpc_api::chain::{
GetBlockOption, GetBlocksOption, GetEventOption, GetTransactionOption,
};
use starcoin_rpc_api::node::NodeInfo;
use starcoin_rpc_api::service::RpcAsyncService;
use starcoin_rpc_api::state::{
Expand Down Expand Up @@ -753,9 +755,14 @@ impl RpcClient {
&self,
number: Option<BlockNumber>,
count: u64,
option: Option<GetBlocksOption>,
) -> anyhow::Result<Vec<BlockView>> {
self.call_rpc_blocking(|inner| inner.chain_client.get_blocks_by_number(number, count))
.map_err(map_err)
self.call_rpc_blocking(|inner| {
inner
.chain_client
.get_blocks_by_number(number, count, option)
})
.map_err(map_err)
}

pub fn chain_get_transaction(
Expand Down
8 changes: 6 additions & 2 deletions rpc/server/src/module/chain_rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ use starcoin_config::NodeConfig;
use starcoin_crypto::HashValue;
use starcoin_logger::prelude::*;
use starcoin_resource_viewer::MoveValueAnnotator;
use starcoin_rpc_api::chain::{ChainApi, GetBlockOption, GetEventOption, GetTransactionOption};
use starcoin_rpc_api::chain::{
ChainApi, GetBlockOption, GetBlocksOption, GetEventOption, GetTransactionOption,
};
use starcoin_rpc_api::types::pubsub::EventFilter;
use starcoin_rpc_api::types::{
BlockHeaderView, BlockInfoView, BlockTransactionsView, BlockView, ChainId, ChainInfoView,
Expand Down Expand Up @@ -141,6 +143,7 @@ where
&self,
number: Option<BlockNumber>,
count: u64,
option: Option<GetBlocksOption>,
) -> FutureResult<Vec<BlockView>> {
let service = self.service.clone();
let config = self.config.clone();
Expand All @@ -153,8 +156,9 @@ where
let max_return_num = count
.min(end_block_number + 1)
.min(config.rpc.block_query_max_range());
let reverse = !option.unwrap_or_default().reverse;
let block = service
.main_blocks_by_number(number, max_return_num)
.main_blocks_by_number(number, reverse, max_return_num)
.await?;

block
Expand Down
2 changes: 1 addition & 1 deletion scripts/nextest.sh
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ ulimit -a

# install cargo-nextest
echo "Setup cargo-nextest."
cargo nextest -V >/dev/null 2>&1 || cargo install cargo-nextest
cargo nextest -V >/dev/null 2>&1 || cargo install cargo-nextest --locked

# following options are tuned for current self hosted CI machine
# --test-threads 12, proper test concurrency level, balance failure rate and test speed
Expand Down
2 changes: 1 addition & 1 deletion testsuite/tests/steps/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ pub fn steps() -> Steps<MyWorld> {
let local_client = world.rpc_client2.as_ref().take().unwrap();
let status = client.clone().node_status();
assert!(status.is_ok());
let list_block = client.chain_get_blocks_by_number(None, 1).unwrap();
let list_block = client.chain_get_blocks_by_number(None, true, 1).unwrap();
let max_num = list_block[0].header.number.0;
let local_max_block = local_client
.chain_get_block_by_number(max_num, None)
Expand Down
3 changes: 2 additions & 1 deletion vm/starcoin-transactional-test-harness/src/fork_chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ use starcoin_abi_decoder::decode_txn_payload;
use starcoin_accumulator::{node::AccumulatorStoreType, Accumulator, MerkleAccumulator};
use starcoin_config::{BuiltinNetworkID, ChainNetworkID};
use starcoin_crypto::HashValue;
use starcoin_rpc_api::chain::ChainApiClient;
use starcoin_rpc_api::chain::{ChainApi, GetBlockOption};
use starcoin_rpc_api::chain::{ChainApiClient, GetBlocksOption};
use starcoin_rpc_api::types::{
BlockInfoView, BlockTransactionsView, BlockView, ChainId, ChainInfoView,
SignedUserTransactionView, TransactionInfoView, TransactionView,
Expand Down Expand Up @@ -299,6 +299,7 @@ impl ChainApi for MockChainApi {
&self,
_number: Option<BlockNumber>,
_count: u64,
_option: Option<GetBlocksOption>,
) -> FutureResult<Vec<BlockView>> {
let fut = async move {
bail!("not implemented.");
Expand Down