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

fix(grpc): return bool if mempool is out of sync instead of returning… #6728

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions applications/minotari_app_grpc/proto/block.proto
Original file line number Diff line number Diff line change
Expand Up @@ -134,5 +134,8 @@ message NewBlockTemplate {
// This flag indicates if the inputs, outputs and kernels have been sorted internally, that is, the sort() method
// has been called. This may be false even if all components are sorted.
AggregateBody body = 2;
// Sometimes the mempool has not synced to the latest tip, this flag indicates if the mempool is out of sync.
// In most cases the next call to get_new_block_template will return a block with the mempool in sync.
bool is_mempool_in_sync = 3;
}

Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ impl TryFrom<NewBlockTemplate> for grpc::NewBlockTemplate {
.collect(),
}),
header: Some(header),
is_mempool_in_sync: block.is_mempool_in_sync,
})
}
}
Expand Down Expand Up @@ -105,6 +106,7 @@ impl TryFrom<grpc::NewBlockTemplate> for NewBlockTemplate {
target_difficulty: Default::default(),
reward: Default::default(),
total_fees: Default::default(),
is_mempool_in_sync: block.is_mempool_in_sync,
})
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,15 @@ impl BlockTemplateProtocol<'_> {
let (block_template_with_coinbase, height) = match block_templates.get_new_template(best_block_hash).await {
None => {
let new_template = match self.get_new_block_template().await {
Ok(val) => val,
Ok(val) => {
if !val.template.is_mempool_in_sync {
error!(target: LOG_TARGET, "Mempool is not in sync. Retrying to get new block template (try {})", loop_count);
return Err(MmProxyError::FailedToGetBlockTemplate(
"mempool not in sync".to_string(),
));
}
val
},
Err(err) => {
error!(target: LOG_TARGET, "grpc get_new_block_template ({})", err.to_string());
return Err(err);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -258,18 +258,15 @@ where B: BlockchainBackend + 'static
NodeCommsRequest::GetNewBlockTemplate(request) => {
let best_block_header = self.blockchain_db.fetch_tip_header().await?;
let last_seen_hash = self.mempool.get_last_seen_hash().await?;
let mut is_mempool_synced = true;
if last_seen_hash != FixedHash::default() && best_block_header.hash() != &last_seen_hash {
warn!(
target: LOG_TARGET,
"Mempool out of sync - last seen hash '{}' does not match the tip hash '{}'. This condition \
should auto correct with the next block template request",
last_seen_hash, best_block_header.hash()
);
return Err(CommsInterfaceError::InternalError(
"Mempool out of sync, blockchain db advanced passed current tip in mempool storage; this \
should auto correct with the next block template request"
.to_string(),
));
is_mempool_synced = false;
}
let mut header = BlockHeader::from_previous(best_block_header.header());
let constants = self.consensus_manager.consensus_constants(header.height);
Expand Down Expand Up @@ -313,6 +310,7 @@ where B: BlockchainBackend + 'static
self.get_target_difficulty_for_next_block(request.algo, constants, prev_hash)
.await?,
self.consensus_manager.get_block_reward_at(height),
is_mempool_synced,
)?;

debug!(target: LOG_TARGET,
Expand Down
5 changes: 5 additions & 0 deletions base_layer/core/src/blocks/new_block_template.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,17 @@ pub struct NewBlockTemplate {
pub reward: MicroMinotari,
/// The total fees is the sum of all the fees in the block.
pub total_fees: MicroMinotari,
/// Sometimes the mempool has not synced to the latest tip, this flag indicates if the mempool is out of sync.
/// In most cases the next call to get_new_block_template will return a block with the mempool in sync.
pub is_mempool_in_sync: bool,
}

impl NewBlockTemplate {
pub fn from_block(
block: Block,
target_difficulty: Difficulty,
reward: MicroMinotari,
is_mempool_in_sync: bool,
) -> Result<Self, TransactionError> {
let Block { header, body } = block;
let total_fees = body.get_total_fee()?;
Expand All @@ -67,6 +71,7 @@ impl NewBlockTemplate {
target_difficulty,
reward,
total_fees,
is_mempool_in_sync,
})
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,8 @@ mod prepare_new_block {
fn it_errors_for_genesis_block() {
let db = setup();
let genesis = db.fetch_block(0, true).unwrap();
let template = NewBlockTemplate::from_block(genesis.block().clone(), Difficulty::min(), 5000 * T).unwrap();
let template =
NewBlockTemplate::from_block(genesis.block().clone(), Difficulty::min(), 5000 * T, true).unwrap();
let err = db.prepare_new_block(template).unwrap_err();
assert!(matches!(err, ChainStorageError::InvalidArguments { .. }));
}
Expand All @@ -452,7 +453,7 @@ mod prepare_new_block {
let genesis = db.fetch_block(0, true).unwrap();
let next_block = BlockHeader::from_previous(genesis.header());
let mut template =
NewBlockTemplate::from_block(next_block.into_builder().build(), Difficulty::min(), 5000 * T).unwrap();
NewBlockTemplate::from_block(next_block.into_builder().build(), Difficulty::min(), 5000 * T, true).unwrap();
// This would cause a panic if the sanity checks were not there
template.header.height = 100;
let err = db.prepare_new_block(template.clone()).unwrap_err();
Expand All @@ -468,7 +469,7 @@ mod prepare_new_block {
let genesis = db.fetch_block(0, true).unwrap();
let next_block = BlockHeader::from_previous(genesis.header());
let template =
NewBlockTemplate::from_block(next_block.into_builder().build(), Difficulty::min(), 5000 * T).unwrap();
NewBlockTemplate::from_block(next_block.into_builder().build(), Difficulty::min(), 5000 * T, true).unwrap();
let block = db.prepare_new_block(template).unwrap();
assert_eq!(block.header.height, 1);
}
Expand Down
4 changes: 4 additions & 0 deletions base_layer/core/tests/helpers/block_builders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ async fn genesis_template(
header.into_builder().with_coinbase_utxo(utxo, kernel).build(),
Difficulty::min(),
coinbase_value,
true,
)
.unwrap();
(block, output)
Expand Down Expand Up @@ -315,6 +316,7 @@ pub async fn chain_block(
.build(),
Difficulty::min(),
reward,
true,
)
.unwrap()
}
Expand All @@ -339,6 +341,7 @@ pub fn chain_block_with_coinbase(
.build(),
achieved_difficulty.unwrap_or(Difficulty::min()),
consensus.get_block_reward_at(height),
true,
)
.unwrap()
}
Expand Down Expand Up @@ -377,6 +380,7 @@ pub async fn chain_block_with_new_coinbase(
.build(),
Difficulty::min(),
reward,
true,
)
.unwrap();
(template, coinbase_output)
Expand Down
1 change: 1 addition & 0 deletions base_layer/core/tests/helpers/database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ pub async fn create_orphan_block(
.build(),
Difficulty::min(),
coinbase_value,
true,
)
.unwrap();
Block::new(template.header.into(), template.body)
Expand Down
1 change: 1 addition & 0 deletions base_layer/core/tests/tests/block_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1351,6 +1351,7 @@ async fn test_fee_overflow() {
.build(),
Difficulty::min(),
consensus_manager.get_block_reward_at(height),
true,
);
assert!(template_result.is_err());
assert_eq!(
Expand Down
2 changes: 1 addition & 1 deletion base_layer/tari_mining_helper_ffi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -804,7 +804,7 @@ mod tests {
let error_ptr = &mut error as *mut c_int;
let header = BlockHeader::new(0);
let block =
NewBlockTemplate::from_block(header.into_builder().build(), Difficulty::min(), 0.into()).unwrap();
NewBlockTemplate::from_block(header.into_builder().build(), Difficulty::min(), 0.into(), true).unwrap();

let block_bytes = borsh::to_vec(&block).unwrap();
#[allow(clippy::cast_possible_truncation)]
Expand Down
Loading