Skip to content

Commit

Permalink
fix: potential max block size (#6725)
Browse files Browse the repository at this point in the history
Description
---
Makes sure that the template does not exceed max weight

Motivation and Context
---
P2Pool could generate a lot of coinbases, currently the template
generation assumes only 1 coinbase is present. This PR takes into
account the coinbases when developing the template size

---------

Co-authored-by: Hansie Odendaal <[email protected]>
  • Loading branch information
SWvheerden and hansieodendaal authored Dec 13, 2024
1 parent 686e7fd commit c61f706
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 17 deletions.
2 changes: 1 addition & 1 deletion applications/minotari_node/src/commands/command/status.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ impl CommandContext {
if mempool_stats.unconfirmed_weight == 0 {
0
} else {
1 + mempool_stats.unconfirmed_weight / constants.max_block_weight_excluding_coinbase()?
1 + mempool_stats.unconfirmed_weight / constants.max_block_weight_excluding_coinbases(1)?
},
),
);
Expand Down
24 changes: 21 additions & 3 deletions applications/minotari_node/src/grpc/base_node_grpc_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -800,10 +800,14 @@ impl tari_rpc::base_node_server::BaseNode for BaseNodeGrpcServer {
})?;

let mut handler = self.node_service.clone();

let mut new_template = handler
.get_new_block_template(algo, request.max_weight)
let meta = handler
.get_metadata()
.await
.map_err(|e| obscure_error_if_true(report_error_flag, Status::internal(e.to_string())))?;
let constants_weight = self
.consensus_rules
.consensus_constants(meta.best_block_height().saturating_add(1))
.max_block_weight_excluding_coinbases(request.coinbases.len())
.map_err(|e| {
warn!(
target: LOG_TARGET,
Expand All @@ -812,6 +816,20 @@ impl tari_rpc::base_node_server::BaseNode for BaseNodeGrpcServer {
);
obscure_error_if_true(report_error_flag, Status::internal(e.to_string()))
})?;
let asking_weight = if request.max_weight > constants_weight || request.max_weight == 0 {
constants_weight
} else {
request.max_weight
};

let mut new_template = handler.get_new_block_template(algo, asking_weight).await.map_err(|e| {
warn!(
target: LOG_TARGET,
"Could not get new block template: {}",
e.to_string()
);
obscure_error_if_true(report_error_flag, Status::internal(e.to_string()))
})?;

let pow = algo as i32;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ where B: BlockchainBackend + 'static
header.pow.pow_algo = request.algo;

let constants_weight = constants
.max_block_weight_excluding_coinbase()
.max_block_weight_excluding_coinbases(1)
.map_err(|e| CommsInterfaceError::InternalError(e.to_string()))?;
let asking_weight = if request.max_weight > constants_weight || request.max_weight == 0 {
constants_weight
Expand Down
14 changes: 9 additions & 5 deletions base_layer/core/src/consensus/consensus_constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,20 +233,24 @@ impl ConsensusConstants {

/// Maximum transaction weight used for the construction of new blocks. It leaves place for 1 kernel and 1 output
/// with default features, as well as the maximum possible value of the `coinbase_extra` field
pub fn max_block_weight_excluding_coinbase(&self) -> std::io::Result<u64> {
Ok(self.max_block_transaction_weight - self.calculate_1_output_kernel_weight()?)
pub fn max_block_weight_excluding_coinbases(&self, number_of_coinbases: usize) -> std::io::Result<u64> {
Ok(self.max_block_transaction_weight - self.calculate_n_output_kernel_weight(number_of_coinbases)?)
}

fn calculate_1_output_kernel_weight(&self) -> std::io::Result<u64> {
fn calculate_n_output_kernel_weight(&self, num_outputs: usize) -> std::io::Result<u64> {
let output_features = OutputFeatures { ..Default::default() };
let max_extra_size = self.coinbase_output_features_extra_max_length() as usize;

let features_and_scripts_size = self.transaction_weight.round_up_features_and_scripts_size(
output_features.get_serialized_size()? +
max_extra_size +
script![Nop].map_err(|e| e.to_std_io_error())?.get_serialized_size()?,
script!(PushPubKey(Box::default()))
.map_err(|e| e.to_std_io_error())?
.get_serialized_size()?,
);
Ok(self.transaction_weight.calculate(1, 0, 1, features_and_scripts_size))
Ok(self
.transaction_weight
.calculate(1, 0, num_outputs, features_and_scripts_size * num_outputs))
}

pub fn coinbase_output_features_extra_max_length(&self) -> u32 {
Expand Down
2 changes: 1 addition & 1 deletion base_layer/core/src/mempool/mempool_storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ impl MempoolStorage {
let target_weight = self
.rules
.consensus_constants(tip_height)
.max_block_weight_excluding_coinbase()
.max_block_weight_excluding_coinbases(1)
.map_err(|e| MempoolError::InternalError(e.to_string()))?;
let stats = self.unconfirmed_pool.get_fee_per_gram_stats(count, target_weight)?;
Ok(stats)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,14 @@ impl<B: BlockchainBackend> TransactionValidator for TransactionChainLinkedValida
.map_err(|e| {
ValidationError::SerializationError(format!("Unable to calculate the transaction weight: {}", e))
})? >
consensus_constants.max_block_weight_excluding_coinbase().map_err(|e| {
ValidationError::ConsensusError(format!(
"Unable to get max block weight from consensus constants: {}",
e
))
})?
consensus_constants
.max_block_weight_excluding_coinbases(1)
.map_err(|e| {
ValidationError::ConsensusError(format!(
"Unable to get max block weight from consensus constants: {}",
e
))
})?
{
return Err(ValidationError::MaxTransactionWeightExceeded);
}
Expand Down

0 comments on commit c61f706

Please sign in to comment.