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

refactor: extract chunk producer logic from Client #12852

Merged
merged 15 commits into from
Feb 4, 2025
10 changes: 2 additions & 8 deletions chain/chain/src/chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2365,13 +2365,7 @@ impl Chain {
prev_prev_hash: &CryptoHash,
prev_hash: &CryptoHash,
) -> Result<bool, Error> {
// Needs to be used with care: for the first block of each epoch the semantic is slightly
// different, since the prev_block is in a different epoch. So for all the blocks but the
// first one in each epoch this method returns true if the block is ready to have state
// applied for the next epoch, while for the first block in a particular epoch this method
// returns true if the block is ready to have state applied for the current epoch (and
// otherwise should be orphaned)
Ok(!self.chain_store.get_blocks_to_catchup(prev_prev_hash)?.contains(prev_hash))
Ok(ChainStore::prev_block_is_caught_up(&self.chain_store, prev_prev_hash, prev_hash)?)
}

/// Check if any block with missing chunk is ready to be processed and start processing these blocks
Expand Down Expand Up @@ -2598,7 +2592,7 @@ impl Chain {
pub fn transaction_validity_check<'a>(
&'a self,
prev_block_header: BlockHeader,
) -> impl FnMut(&SignedTransaction) -> bool + 'a {
) -> impl Fn(&SignedTransaction) -> bool + 'a {
move |tx: &SignedTransaction| -> bool {
self.chain_store()
.check_transaction_validity_period(&prev_block_header, tx.transaction.block_hash())
Expand Down
2 changes: 1 addition & 1 deletion chain/chain/src/runtime/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -607,7 +607,7 @@ impl RuntimeAdapter for NightshadeRuntime {
chunk: PrepareTransactionsChunkContext,
prev_block: PrepareTransactionsBlockContext,
transaction_groups: &mut dyn TransactionGroupIterator,
chain_validate: &mut dyn FnMut(&SignedTransaction) -> bool,
chain_validate: &dyn Fn(&SignedTransaction) -> bool,
time_limit: Option<Duration>,
) -> Result<PreparedTransactions, Error> {
let start_time = std::time::Instant::now();
Expand Down
2 changes: 1 addition & 1 deletion chain/chain/src/stateless_validation/chunk_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ pub fn pre_validate_chunk_state_witness(
} else {
let prev_block_header =
store.get_block_header(last_chunk_block.header().prev_hash())?;
let mut check = chain.transaction_validity_check(prev_block_header);
let check = chain.transaction_validity_check(prev_block_header);
state_witness.transactions.iter().map(|t| check(t)).collect::<Vec<_>>()
}
} else {
Expand Down
38 changes: 34 additions & 4 deletions chain/chain/src/store/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,22 @@ impl ChainStore {
.collect()
}

pub fn get_outgoing_receipts_for_shard(
&self,
epoch_manager: &dyn EpochManagerAdapter,
prev_block_hash: CryptoHash,
shard_id: ShardId,
last_included_height: BlockHeight,
) -> Result<Vec<Receipt>, Error> {
Self::get_outgoing_receipts_for_shard_from_store(
&self.chain_store(),
epoch_manager,
prev_block_hash,
shard_id,
last_included_height,
)
}

/// Get outgoing receipts that will be *sent* from shard `shard_id` from block whose prev block
/// is `prev_block_hash`
/// Note that the meaning of outgoing receipts here are slightly different from
Expand All @@ -352,8 +368,8 @@ impl ChainStore {
/// But we need to implement a more theoretically correct algorithm if shard layouts will change
/// more often in the future
/// <https://github.com/near/nearcore/issues/4877>
pub fn get_outgoing_receipts_for_shard(
&self,
pub fn get_outgoing_receipts_for_shard_from_store(
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice! Does it make sense to push this in the same helper module we had created last time?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. I just don't want to move too much code at once.

chain_store: &ChainStoreAdapter,
epoch_manager: &dyn EpochManagerAdapter,
prev_block_hash: CryptoHash,
shard_id: ShardId,
Expand All @@ -362,7 +378,7 @@ impl ChainStore {
let shard_layout = epoch_manager.get_shard_layout_from_prev_block(&prev_block_hash)?;
let mut receipts_block_hash = prev_block_hash;
loop {
let block_header = self.get_block_header(&receipts_block_hash)?;
let block_header = chain_store.get_block_header(&receipts_block_hash)?;

if block_header.height() != last_included_height {
receipts_block_hash = *block_header.prev_hash();
Expand All @@ -377,7 +393,7 @@ impl ChainStore {
shard_id
};

let mut receipts = self
let mut receipts = chain_store
.get_outgoing_receipts(&receipts_block_hash, receipts_shard_id)
.map(|v| v.to_vec())
.unwrap_or_default();
Expand Down Expand Up @@ -849,6 +865,20 @@ impl ChainStore {
}
store_update.commit().map_err(|err| err.into())
}

pub fn prev_block_is_caught_up(
chain_store: &ChainStoreAdapter,
prev_prev_hash: &CryptoHash,
prev_hash: &CryptoHash,
) -> Result<bool, Error> {
// Needs to be used with care: for the first block of each epoch the semantic is slightly
// different, since the prev_block is in a different epoch. So for all the blocks but the
// first one in each epoch this method returns true if the block is ready to have state
// applied for the next epoch, while for the first block in a particular epoch this method
// returns true if the block is ready to have state applied for the current epoch (and
// otherwise should be orphaned)
Ok(!chain_store.get_blocks_to_catchup(prev_prev_hash)?.contains(prev_hash))
}
}

impl ChainStoreAccess for ChainStore {
Expand Down
2 changes: 1 addition & 1 deletion chain/chain/src/test_utils/kv_runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1011,7 +1011,7 @@ impl RuntimeAdapter for KeyValueRuntime {
_chunk: PrepareTransactionsChunkContext,
_prev_block: PrepareTransactionsBlockContext,
transaction_groups: &mut dyn TransactionGroupIterator,
_chain_validate: &mut dyn FnMut(&SignedTransaction) -> bool,
_chain_validate: &dyn Fn(&SignedTransaction) -> bool,
_time_limit: Option<Duration>,
) -> Result<PreparedTransactions, Error> {
let mut res = vec![];
Expand Down
2 changes: 1 addition & 1 deletion chain/chain/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,7 @@ pub trait RuntimeAdapter: Send + Sync {
chunk: PrepareTransactionsChunkContext,
prev_block: PrepareTransactionsBlockContext,
transaction_groups: &mut dyn TransactionGroupIterator,
chain_validate: &mut dyn FnMut(&SignedTransaction) -> bool,
chain_validate: &dyn Fn(&SignedTransaction) -> bool,
time_limit: Option<Duration>,
) -> Result<PreparedTransactions, Error>;

Expand Down
Loading
Loading