From d9035a69c4f91260d3d51f913f47c9d00d57fd2c Mon Sep 17 00:00:00 2001 From: SW van Heerden Date: Thu, 12 Dec 2024 16:11:37 +0200 Subject: [PATCH 1/4] fix potential max block size --- .../src/commands/command/status.rs | 2 +- .../src/grpc/base_node_grpc_server.rs | 25 ++++++++++++++++--- .../comms_interface/inbound_handlers.rs | 2 +- .../core/src/consensus/consensus_constants.rs | 14 +++++++---- .../core/src/mempool/mempool_storage.rs | 2 +- .../transaction_chain_validator.rs | 14 ++++++----- 6 files changed, 42 insertions(+), 17 deletions(-) diff --git a/applications/minotari_node/src/commands/command/status.rs b/applications/minotari_node/src/commands/command/status.rs index 67b867ddf0..4e875900f9 100644 --- a/applications/minotari_node/src/commands/command/status.rs +++ b/applications/minotari_node/src/commands/command/status.rs @@ -91,7 +91,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_coinbase(1)? }, ), ); diff --git a/applications/minotari_node/src/grpc/base_node_grpc_server.rs b/applications/minotari_node/src/grpc/base_node_grpc_server.rs index 1b5f2c548d..eb8f5d62a7 100644 --- a/applications/minotari_node/src/grpc/base_node_grpc_server.rs +++ b/applications/minotari_node/src/grpc/base_node_grpc_server.rs @@ -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_coinbase(request.coinbases.len()) .map_err(|e| { warn!( target: LOG_TARGET, @@ -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; @@ -1203,6 +1221,7 @@ impl tari_rpc::base_node_server::BaseNode for BaseNodeGrpcServer { .map_err(|e| obscure_error_if_true(report_error_flag, Status::internal(e)))?, ); + let new_template = handler.get_new_block_template(algo, 0).await.map_err(|e| { warn!( target: LOG_TARGET, diff --git a/base_layer/core/src/base_node/comms_interface/inbound_handlers.rs b/base_layer/core/src/base_node/comms_interface/inbound_handlers.rs index bcfe2a0297..8ee43dac0e 100644 --- a/base_layer/core/src/base_node/comms_interface/inbound_handlers.rs +++ b/base_layer/core/src/base_node/comms_interface/inbound_handlers.rs @@ -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_coinbase(1) .map_err(|e| CommsInterfaceError::InternalError(e.to_string()))?; let asking_weight = if request.max_weight > constants_weight || request.max_weight == 0 { constants_weight diff --git a/base_layer/core/src/consensus/consensus_constants.rs b/base_layer/core/src/consensus/consensus_constants.rs index 796a68f728..1eb8aa86c6 100644 --- a/base_layer/core/src/consensus/consensus_constants.rs +++ b/base_layer/core/src/consensus/consensus_constants.rs @@ -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 { - Ok(self.max_block_transaction_weight - self.calculate_1_output_kernel_weight()?) + pub fn max_block_weight_excluding_coinbase(&self, coinbase_number: usize) -> std::io::Result { + Ok(self.max_block_transaction_weight - self.calculate_1_output_kernel_weight(coinbase_number)?) } - fn calculate_1_output_kernel_weight(&self) -> std::io::Result { + fn calculate_1_output_kernel_weight(&self, num_outputs: usize) -> std::io::Result { 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 { diff --git a/base_layer/core/src/mempool/mempool_storage.rs b/base_layer/core/src/mempool/mempool_storage.rs index e44c8189e1..4d5e504116 100644 --- a/base_layer/core/src/mempool/mempool_storage.rs +++ b/base_layer/core/src/mempool/mempool_storage.rs @@ -399,7 +399,7 @@ impl MempoolStorage { let target_weight = self .rules .consensus_constants(tip_height) - .max_block_weight_excluding_coinbase() + .max_block_weight_excluding_coinbase(1) .map_err(|e| MempoolError::InternalError(e.to_string()))?; let stats = self.unconfirmed_pool.get_fee_per_gram_stats(count, target_weight)?; Ok(stats) diff --git a/base_layer/core/src/validation/transaction/transaction_chain_validator.rs b/base_layer/core/src/validation/transaction/transaction_chain_validator.rs index 7879cd50f5..c7550a2cbe 100644 --- a/base_layer/core/src/validation/transaction/transaction_chain_validator.rs +++ b/base_layer/core/src/validation/transaction/transaction_chain_validator.rs @@ -50,12 +50,14 @@ impl 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_coinbase(1) + .map_err(|e| { + ValidationError::ConsensusError(format!( + "Unable to get max block weight from consensus constants: {}", + e + )) + })? { return Err(ValidationError::MaxTransactionWeightExceeded); } From df967b97d87b12cf7f7a2d75b9828ea324699c15 Mon Sep 17 00:00:00 2001 From: SW van Heerden Date: Fri, 13 Dec 2024 11:46:11 +0200 Subject: [PATCH 2/4] Update base_layer/core/src/consensus/consensus_constants.rs Co-authored-by: Hansie Odendaal <39146854+hansieodendaal@users.noreply.github.com> --- base_layer/core/src/consensus/consensus_constants.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/base_layer/core/src/consensus/consensus_constants.rs b/base_layer/core/src/consensus/consensus_constants.rs index 1eb8aa86c6..e2acb1c324 100644 --- a/base_layer/core/src/consensus/consensus_constants.rs +++ b/base_layer/core/src/consensus/consensus_constants.rs @@ -237,7 +237,7 @@ impl ConsensusConstants { Ok(self.max_block_transaction_weight - self.calculate_1_output_kernel_weight(coinbase_number)?) } - fn calculate_1_output_kernel_weight(&self, num_outputs: usize) -> std::io::Result { + fn calculate_n_output_kernel_weight(&self, num_outputs: usize) -> std::io::Result { let output_features = OutputFeatures { ..Default::default() }; let max_extra_size = self.coinbase_output_features_extra_max_length() as usize; From fedb7f36d737046855da3280bc61c582d046dd3a Mon Sep 17 00:00:00 2001 From: SW van Heerden Date: Fri, 13 Dec 2024 11:46:18 +0200 Subject: [PATCH 3/4] Update base_layer/core/src/consensus/consensus_constants.rs Co-authored-by: Hansie Odendaal <39146854+hansieodendaal@users.noreply.github.com> --- base_layer/core/src/consensus/consensus_constants.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/base_layer/core/src/consensus/consensus_constants.rs b/base_layer/core/src/consensus/consensus_constants.rs index e2acb1c324..4123d9d395 100644 --- a/base_layer/core/src/consensus/consensus_constants.rs +++ b/base_layer/core/src/consensus/consensus_constants.rs @@ -233,7 +233,7 @@ 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, coinbase_number: usize) -> std::io::Result { + pub fn max_block_weight_excluding_coinbases(&self, number_of_coinbases: usize) -> std::io::Result { Ok(self.max_block_transaction_weight - self.calculate_1_output_kernel_weight(coinbase_number)?) } From 2ed369622a73aac0a63746f4d6556fe54dcc88fe Mon Sep 17 00:00:00 2001 From: SW van Heerden Date: Fri, 13 Dec 2024 11:51:57 +0200 Subject: [PATCH 4/4] review --- applications/minotari_node/src/commands/command/status.rs | 2 +- applications/minotari_node/src/grpc/base_node_grpc_server.rs | 3 +-- .../core/src/base_node/comms_interface/inbound_handlers.rs | 2 +- base_layer/core/src/consensus/consensus_constants.rs | 2 +- base_layer/core/src/mempool/mempool_storage.rs | 2 +- .../src/validation/transaction/transaction_chain_validator.rs | 2 +- 6 files changed, 6 insertions(+), 7 deletions(-) diff --git a/applications/minotari_node/src/commands/command/status.rs b/applications/minotari_node/src/commands/command/status.rs index 4e875900f9..a11785493c 100644 --- a/applications/minotari_node/src/commands/command/status.rs +++ b/applications/minotari_node/src/commands/command/status.rs @@ -91,7 +91,7 @@ impl CommandContext { if mempool_stats.unconfirmed_weight == 0 { 0 } else { - 1 + mempool_stats.unconfirmed_weight / constants.max_block_weight_excluding_coinbase(1)? + 1 + mempool_stats.unconfirmed_weight / constants.max_block_weight_excluding_coinbases(1)? }, ), ); diff --git a/applications/minotari_node/src/grpc/base_node_grpc_server.rs b/applications/minotari_node/src/grpc/base_node_grpc_server.rs index eb8f5d62a7..d16e2446e1 100644 --- a/applications/minotari_node/src/grpc/base_node_grpc_server.rs +++ b/applications/minotari_node/src/grpc/base_node_grpc_server.rs @@ -807,7 +807,7 @@ impl tari_rpc::base_node_server::BaseNode for BaseNodeGrpcServer { let constants_weight = self .consensus_rules .consensus_constants(meta.best_block_height().saturating_add(1)) - .max_block_weight_excluding_coinbase(request.coinbases.len()) + .max_block_weight_excluding_coinbases(request.coinbases.len()) .map_err(|e| { warn!( target: LOG_TARGET, @@ -1221,7 +1221,6 @@ impl tari_rpc::base_node_server::BaseNode for BaseNodeGrpcServer { .map_err(|e| obscure_error_if_true(report_error_flag, Status::internal(e)))?, ); - let new_template = handler.get_new_block_template(algo, 0).await.map_err(|e| { warn!( target: LOG_TARGET, diff --git a/base_layer/core/src/base_node/comms_interface/inbound_handlers.rs b/base_layer/core/src/base_node/comms_interface/inbound_handlers.rs index 8ee43dac0e..9b8c68f569 100644 --- a/base_layer/core/src/base_node/comms_interface/inbound_handlers.rs +++ b/base_layer/core/src/base_node/comms_interface/inbound_handlers.rs @@ -277,7 +277,7 @@ where B: BlockchainBackend + 'static header.pow.pow_algo = request.algo; let constants_weight = constants - .max_block_weight_excluding_coinbase(1) + .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 diff --git a/base_layer/core/src/consensus/consensus_constants.rs b/base_layer/core/src/consensus/consensus_constants.rs index 4123d9d395..b5a595b43f 100644 --- a/base_layer/core/src/consensus/consensus_constants.rs +++ b/base_layer/core/src/consensus/consensus_constants.rs @@ -234,7 +234,7 @@ 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_coinbases(&self, number_of_coinbases: usize) -> std::io::Result { - Ok(self.max_block_transaction_weight - self.calculate_1_output_kernel_weight(coinbase_number)?) + Ok(self.max_block_transaction_weight - self.calculate_n_output_kernel_weight(number_of_coinbases)?) } fn calculate_n_output_kernel_weight(&self, num_outputs: usize) -> std::io::Result { diff --git a/base_layer/core/src/mempool/mempool_storage.rs b/base_layer/core/src/mempool/mempool_storage.rs index 4d5e504116..7c8892b68f 100644 --- a/base_layer/core/src/mempool/mempool_storage.rs +++ b/base_layer/core/src/mempool/mempool_storage.rs @@ -399,7 +399,7 @@ impl MempoolStorage { let target_weight = self .rules .consensus_constants(tip_height) - .max_block_weight_excluding_coinbase(1) + .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) diff --git a/base_layer/core/src/validation/transaction/transaction_chain_validator.rs b/base_layer/core/src/validation/transaction/transaction_chain_validator.rs index c7550a2cbe..1b20505707 100644 --- a/base_layer/core/src/validation/transaction/transaction_chain_validator.rs +++ b/base_layer/core/src/validation/transaction/transaction_chain_validator.rs @@ -51,7 +51,7 @@ impl TransactionValidator for TransactionChainLinkedValida ValidationError::SerializationError(format!("Unable to calculate the transaction weight: {}", e)) })? > consensus_constants - .max_block_weight_excluding_coinbase(1) + .max_block_weight_excluding_coinbases(1) .map_err(|e| { ValidationError::ConsensusError(format!( "Unable to get max block weight from consensus constants: {}",