Skip to content

Commit

Permalink
[Feature] RPC blocks support positive sequence (#3824)
Browse files Browse the repository at this point in the history
* #3799 add new feature

* #3799 fix check add new feature

* Fix fmt

* fix CI nextest

* Fix bug

* Fix test

* Fix default bool

* edit comment

* add more test

* fix sync test err

* add depend

* add starcoin-rpc-api

Co-authored-by: nk_ysg <nk_ysg@163.com>
  • Loading branch information
WGB5445 and nkysg authored Dec 28, 2022
1 parent d57b055 commit ce4b2c9
Show file tree
Hide file tree
Showing 17 changed files with 140 additions and 27 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

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)
} 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);

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
1 change: 1 addition & 0 deletions testsuite/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ starcoin-miner = {path = "../miner"}
starcoin-network = {path = "../network"}
starcoin-node = {path = "../node"}
starcoin-rpc-client = {path = "../rpc/client"}
starcoin-rpc-api = {path = "../rpc/api"}
starcoin-rpc-server = {path = "../rpc/server"}
starcoin-state-api = {path = "../state/api"}
starcoin-storage = {path = "../storage"}
Expand Down
5 changes: 4 additions & 1 deletion testsuite/tests/steps/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
use crate::MyWorld;
use cucumber::{Steps, StepsBuilder};
use starcoin_logger::prelude::*;
use starcoin_rpc_api::chain::GetBlocksOption;
use starcoin_rpc_client::RpcClient;
use std::sync::Arc;
use std::thread;
Expand All @@ -22,7 +23,9 @@ 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, 1, Some(GetBlocksOption { reverse: true }))
.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

0 comments on commit ce4b2c9

Please sign in to comment.