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: fix new block handling of non-tip blocks #4431

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
2 changes: 1 addition & 1 deletion applications/tari_console_wallet/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ fn main_inner() -> Result<(), ExitError> {
)?;

#[cfg_attr(not(all(unix, feature = "libtor")), allow(unused_mut))]
let mut config = ApplicationConfig::load_from(&cfg)?;
let config = ApplicationConfig::load_from(&cfg)?;
SWvheerden marked this conversation as resolved.
Show resolved Hide resolved

let runtime = tokio::runtime::Builder::new_multi_thread()
.enable_all()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -584,7 +584,36 @@ where B: BlockchainBackend + 'static
coinbase_output,
kernel_excess_sigs: excess_sigs,
} = new_block;
// If the block is empty, we dont have to check ask for the block, as we already have the full block available
// to us.
if excess_sigs.is_empty() {
let block = BlockBuilder::new(header.version)
.with_coinbase_utxo(coinbase_output, coinbase_kernel)
.with_header(header)
.build();
return Ok(Arc::new(block));
}

let block_hash = header.hash();
// We check the current tip and orphan status of the block because we cannot guarantee that mempool state is
// correct and the mmr root calculation is only valid if the block is building on the tip.
let current_meta = self.blockchain_db.get_chain_metadata().await?;
if header.prev_hash != *current_meta.best_block() {
debug!(
target: LOG_TARGET,
"Orphaned block #{}: ({}), current tip is: #{} ({}). We need to fetch the complete block from peer: \
({})",
header.height,
block_hash.to_hex(),
current_meta.height_of_longest_chain(),
current_meta.best_block().to_hex(),
source_peer,
);
let block = self.request_full_block_from_peer(source_peer, block_hash).await?;
return Ok(block);
}

// We know that the block is neither and orphan or a coinbase, so lets ask our mempool for the transactions
let (known_transactions, missing_excess_sigs) = self.mempool.retrieve_by_excess_sigs(excess_sigs).await?;
let known_transactions = known_transactions.into_iter().map(|tx| (*tx).clone()).collect();

Expand All @@ -599,7 +628,7 @@ where B: BlockchainBackend + 'static
target: LOG_TARGET,
"All transactions for block #{} ({}) found in mempool",
header.height,
header.hash().to_hex()
block_hash.to_hex()
);
} else {
debug!(
Expand All @@ -623,7 +652,6 @@ where B: BlockchainBackend + 'static
}

if !not_found.is_empty() {
let block_hash = header.hash();
warn!(
target: LOG_TARGET,
"Peer {} was not able to return all transactions for block #{} ({}). {} transaction(s) not found. \
Expand Down Expand Up @@ -655,9 +683,14 @@ where B: BlockchainBackend + 'static
// Perform a sanity check on the reconstructed block, if the MMR roots don't match then it's possible one or
// more transactions in our mempool had the same excess/signature for a *different* transaction.
// This is extremely unlikely, but still possible. In case of a mismatch, request the full block from the peer.
let (block, mmr_roots) = self.blockchain_db.calculate_mmr_roots(block).await?;
let (block, mmr_roots) = match self.blockchain_db.calculate_mmr_roots(block).await {
Err(_) => {
let block = self.request_full_block_from_peer(source_peer, block_hash).await?;
return Ok(block);
},
Ok(v) => v,
};
if let Err(e) = helpers::check_mmr_roots(&header, &mmr_roots) {
let block_hash = block.hash();
warn!(
target: LOG_TARGET,
"Reconstructed block #{} ({}) failed MMR check validation!. Requesting full block. Error: {}",
Expand Down
12 changes: 9 additions & 3 deletions base_layer/core/src/base_node/service/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ use crate::{
StateMachineHandle,
},
blocks::{Block, NewBlock},
chain_storage::BlockchainBackend,
chain_storage::{BlockchainBackend, ChainStorageError},
proto as shared_protos,
proto::base_node as proto,
};
Expand Down Expand Up @@ -297,8 +297,14 @@ where B: BlockchainBackend + 'static
task::spawn(async move {
let result = handle_incoming_block(inbound_nch, new_block).await;

if let Err(e) = result {
error!(target: LOG_TARGET, "Failed to handle incoming block message: {:?}", e);
match result {
Err(BaseNodeServiceError::CommsInterfaceError(CommsInterfaceError::ChainStorageError(
ChainStorageError::AddBlockOperationLocked,
))) => {
// Special case, dont log this again as an error
},
Err(e) => error!(target: LOG_TARGET, "Failed to handle incoming block message: {:?}", e),
_ => {},
}
});
}
Expand Down
4 changes: 2 additions & 2 deletions base_layer/core/src/chain_storage/blockchain_database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -892,7 +892,7 @@ where B: BlockchainBackend
if self.is_add_block_disabled() {
warn!(
target: LOG_TARGET,
"add_block is disabled. Ignoring candidate block #{} ({})",
"add_block is disabled, node busy syncing. Ignoring candidate block #{} ({})",
block.header.height,
block.hash().to_hex()
);
Expand Down Expand Up @@ -1230,7 +1230,7 @@ pub fn calculate_mmr_roots<T: BlockchainBackend>(db: &T, block: &Block) -> Resul
let metadata = db.fetch_chain_metadata()?;
if header.prev_hash != *metadata.best_block() {
return Err(ChainStorageError::CannotCalculateNonTipMmr(format!(
"Block (#{}) previous hash is {} but the current tip is #{} {}",
"Block (#{}) is not building on tip, previous hash is {} but the current tip is #{} {}",
header.height,
header.prev_hash.to_hex(),
metadata.height_of_longest_chain(),
Expand Down