Identify & Warn miners with outdated mining rpc flow (#658)

* warn miners re zero mass

* make msg more severe

* some improvements

* make the storage mass sanity check hermetically true

Author: michaelsutton

rpc/core/src/convert/tx.rs

@@ -18,20 +18,14 @@ impl From<&Transaction> for RpcTransaction {
             gas: item.gas,
             payload: item.payload.clone(),
             mass: item.mass(),
-            // TODO: Implement a populating process inspired from kaspad\app\rpc\rpccontext\verbosedata.go
             verbose_data: None,
         }
     }
 }
 
 impl From<&TransactionOutput> for RpcTransactionOutput {
     fn from(item: &TransactionOutput) -> Self {
-        Self {
-            value: item.value,
-            script_public_key: item.script_public_key.clone(),
-            // TODO: Implement a populating process inspired from kaspad\app\rpc\rpccontext\verbosedata.go
-            verbose_data: None,
-        }
+        Self { value: item.value, script_public_key: item.script_public_key.clone(), verbose_data: None }
     }
 }
 
@@ -42,7 +36,6 @@ impl From<&TransactionInput> for RpcTransactionInput {
             signature_script: item.signature_script.clone(),
             sequence: item.sequence,
             sig_op_count: item.sig_op_count,
-            // TODO: Implement a populating process inspired from kaspad\app\rpc\rpccontext\verbosedata.go
             verbose_data: None,
         }
     }

rpc/service/src/service.rs

@@ -7,6 +7,7 @@ use async_trait::async_trait;
 use kaspa_consensus_core::api::counters::ProcessingCounters;
 use kaspa_consensus_core::daa_score_timestamp::DaaScoreTimestamp;
 use kaspa_consensus_core::errors::block::RuleError;
+use kaspa_consensus_core::mass::{calc_storage_mass, UtxoCell};
 use kaspa_consensus_core::utxo::utxo_inquirer::UtxoInquirerError;
 use kaspa_consensus_core::{
     block::Block,
@@ -284,6 +285,62 @@ impl RpcCoreService {
             (false, false) => Ok(TransactionQuery::TransactionsOnly),
         }
     }
+
+    fn sanity_check_storage_mass(&self, block: Block) {
+        // [Crescendo]: warn non updated miners to upgrade their rpc flow before Crescendo activation
+        if self.config.crescendo_activation.is_active(block.header.daa_score) {
+            return;
+        }
+
+        // It is sufficient to witness a single transaction with non default mass to conclude that miner rpc flow is correct
+        if block.transactions.iter().any(|tx| tx.mass() > 0) {
+            return;
+        }
+
+        // Iterate over non-coinbase transactions and search for a transaction which is proven to have positive storage mass
+        for tx in block.transactions.iter().skip(1) {
+            /*
+                Below we apply a workaround to compute a lower bound to the storage mass even without having full UTXO context (thus lacking input amounts).
+                Notes:
+                    1. We know that plurality is always 1 for std tx ins/outs (assuming the submitted block was built via the local std mempool).
+                    2. The submitted block was accepted by consensus hence all transactions passed the basic in-isolation validity checks
+
+                |O| > |I| means that the formula used is C·|O| / H(O) - C·|I| / A(I). Additionally we know that sum(O) <= sum(I) (outs = ins minus fee).
+                Combined, we can use sum(O)/|I| as a lower bound for A(I). We simulate this by using sum(O)/|I| as the value of each (unknown) input.
+                Plugging in to the storage formula we obtain a lower bound for the real storage mass (intuitively, making inputs smaller only decreases the mass).
+            */
+            if tx.outputs.len() > tx.inputs.len() {
+                let num_ins = tx.inputs.len() as u64;
+                let sum_outs = tx.outputs.iter().map(|o| o.value).sum::<u64>();
+                if num_ins == 0 || sum_outs < num_ins {
+                    // Sanity checks
+                    continue;
+                }
+
+                let avg_ins_lower = sum_outs / num_ins; // >= 1
+                let storage_mass_lower = calc_storage_mass(
+                    tx.is_coinbase(),
+                    tx.inputs.iter().map(|_| UtxoCell { plurality: 1, amount: avg_ins_lower }),
+                    tx.outputs.iter().map(|o| o.into()),
+                    self.config.storage_mass_parameter,
+                )
+                .unwrap_or(u64::MAX);
+
+                // Despite being a lower bound, storage mass is still calculated to be positive, so we found our problem
+                if storage_mass_lower > 0 {
+                    warn!("The RPC submitted block {} contains a transaction {} with mass = 0 while it should have been strictly positive.
+This indicates that the RPC conversion flow used by the miner does not preserve the mass values received from GetBlockTemplate.
+You must upgrade your miner flow to propagate the mass field correctly prior to the Crescendo hardfork activation. 
+Failure to do so will result in your blocks being considered invalid when Crescendo activates.",
+                            block.hash(),
+                            tx.id()
+                        );
+                    // A single warning is sufficient
+                    break;
+                }
+            }
+        }
+    }
 }
 
 #[async_trait]
@@ -334,11 +391,15 @@ impl RpcApi for RpcCoreService {
 
         trace!("incoming SubmitBlockRequest for block {}", hash);
         match self.flow_context.submit_rpc_block(&session, block.clone()).await {
-            Ok(_) => Ok(SubmitBlockResponse { report: SubmitBlockReport::Success }),
+            Ok(_) => {
+                self.sanity_check_storage_mass(block);
+                Ok(SubmitBlockResponse { report: SubmitBlockReport::Success })
+            }
             Err(ProtocolError::RuleError(RuleError::BadMerkleRoot(h1, h2))) => {
                 warn!(
-                    "The RPC submitted block triggered a {} error: {}. 
-NOTE: This error usually indicates an RPC conversion error between the node and the miner. If you are on TN11 this is likely to reflect using a NON-SUPPORTED miner.",
+                    "The RPC submitted block {} triggered a {} error: {}. 
+NOTE: This error usually indicates an RPC conversion error between the node and the miner. This is likely to reflect using a NON-SUPPORTED miner.",
+                    hash,
                     stringify!(RuleError::BadMerkleRoot),
                     RuleError::BadMerkleRoot(h1, h2)
                 );
@@ -348,10 +409,7 @@ NOTE: This error usually indicates an RPC conversion error between the node and
                 Ok(SubmitBlockResponse { report: SubmitBlockReport::Reject(SubmitBlockRejectReason::BlockInvalid) })
             }
             Err(err) => {
-                warn!(
-                    "The RPC submitted block triggered an error: {}\nPrinting the full header for debug purposes:\n{:?}",
-                    err, block
-                );
+                warn!("The RPC submitted block triggered an error: {}\nPrinting the full block for debug purposes:\n{:?}", err, block);
                 Ok(SubmitBlockResponse { report: SubmitBlockReport::Reject(SubmitBlockRejectReason::BlockInvalid) })
             }
         }