fix: improve reliability of get block template protocol in mm proxy (#…

…3141)

<!--- Provide a general summary of your changes in the Title above -->

## Description
<!--- Describe your changes in detail -->
The main change here is to check the tip height after retrieving the
block template but before MMR roots are calculated. When mining quickly
in cucumber tests, this race condition occurs. Previously, this would be
ok, the miner would simply mine 1 behind the current tip height. But
because we can no longer calculate MMRs prior to the tip, this MMR call
(`get_new_block`) now returns an error, which fails cucumber.

`get_new_block` now returns a `FailedPrecondition` GRPC error status in the event
that the block template submitted is not for the current chain tip. This allows
clients to more easily identify this case, typically by asking for a new template. 

This refactor was performed because the protocol was gaining too much
complexity to exist in a single function.

Some extra changes to get all cucumbers to pass:
- Replace `--daemon` flags with `--non-interactive` 
- Used more 'tolerant' step for `As a user I want to change base node for a wallet` scenario

## Motivation and Context
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here. -->
The `Verify all coinbases in hybrid mining are accounted for` cucumber test is flakey
due to change in semantics for get_new_block since #3109. The possibility of this occurring 
with normal difficulties is usually tiny, but may be a symptom of a slow get_coinbase request. 

## How Has This Been Tested?
<!--- Please describe in detail how you tested your changes. -->
<!--- Include details of your testing environment, and the tests you ran to -->
<!--- see how your change affects other areas of the code, etc. -->
Merge mined on local machine
`Verify all coinbases in hybrid mining are accounted for` passes
Suggest that we run tests with monero pool and nodejs pool

## Checklist:
<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
* [x] I'm merging against the `development` branch.
* [x] I have squashed my commits into a single commit.

applications/tari_base_node/src/grpc/base_node_grpc_server.rs

@@ -36,15 +36,16 @@ use tari_app_grpc::{
     tari_rpc::{CalcType, Sorting},
 };
 use tari_app_utilities::consts;
-use tari_comms::CommsNode;
+use tari_comms::{Bytes, CommsNode};
 use tari_core::{
     base_node::{
-        comms_interface::Broadcast,
+        comms_interface::{Broadcast, CommsInterfaceError},
         state_machine_service::states::BlockSyncInfo,
         LocalNodeCommsInterface,
         StateMachineHandle,
     },
     blocks::{Block, BlockHeader, NewBlockTemplate},
+    chain_storage::ChainStorageError,
     consensus::{emission::Emission, ConsensusManager, NetworkConsensus},
     crypto::tari_utilities::{hex::Hex, ByteArray},
     mempool::{service::LocalMempoolService, TxStorageResponse},
@@ -443,10 +444,18 @@ impl tari_rpc::base_node_server::BaseNode for BaseNodeGrpcServer {
 
         let mut handler = self.node_service.clone();
 
-        let new_block = handler
-            .get_new_block(block_template)
-            .await
-            .map_err(|e| Status::internal(e.to_string()))?;
+        let new_block = match handler.get_new_block(block_template).await {
+            Ok(b) => b,
+            Err(CommsInterfaceError::ChainStorageError(ChainStorageError::CannotCalculateNonTipMmr(msg))) => {
+                let status = Status::with_details(
+                    tonic::Code::FailedPrecondition,
+                    msg,
+                    Bytes::from_static(b"CannotCalculateNonTipMmr"),
+                );
+                return Err(status);
+            },
+            Err(e) => return Err(Status::internal(e.to_string())),
+        };
         // construct response
         let block_hash = new_block.hash();
         let mining_hash = new_block.header.merged_mining_hash();

applications/tari_merge_mining_proxy/src/block_template_data.rs

@@ -22,8 +22,8 @@
 use crate::error::MmProxyError;
 use chrono::{self, DateTime, Duration, Utc};
 use std::{collections::HashMap, sync::Arc};
-use tari_app_grpc::tari_rpc::{Block, MinerData};
-use tari_core::{crypto::tari_utilities::hex::Hex, proof_of_work::monero_rx::FixedByteArray};
+use tari_app_grpc::tari_rpc as grpc;
+use tari_core::proof_of_work::monero_rx::FixedByteArray;
 use tokio::sync::RwLock;
 use tracing::trace;
 
@@ -102,8 +102,8 @@ impl BlockTemplateRepository {
 #[derive(Clone, Debug)]
 pub struct BlockTemplateData {
     pub monero_seed: FixedByteArray,
-    pub tari_block: Block,
-    pub tari_miner_data: MinerData,
+    pub tari_block: grpc::Block,
+    pub tari_miner_data: grpc::MinerData,
     pub monero_difficulty: u64,
     pub tari_difficulty: u64,
 }
@@ -112,25 +112,29 @@ impl BlockTemplateData {}
 
 #[derive(Default)]
 pub struct BlockTemplateDataBuilder {
-    monero_seed: Option<String>,
-    tari_block: Option<Block>,
-    tari_miner_data: Option<MinerData>,
+    monero_seed: Option<FixedByteArray>,
+    tari_block: Option<grpc::Block>,
+    tari_miner_data: Option<grpc::MinerData>,
     monero_difficulty: Option<u64>,
     tari_difficulty: Option<u64>,
 }
 
 impl BlockTemplateDataBuilder {
-    pub fn monero_seed(mut self, monero_seed: String) -> Self {
+    pub fn new() -> Self {
+        Default::default()
+    }
+
+    pub fn monero_seed(mut self, monero_seed: FixedByteArray) -> Self {
         self.monero_seed = Some(monero_seed);
         self
     }
 
-    pub fn tari_block(mut self, tari_block: Block) -> Self {
+    pub fn tari_block(mut self, tari_block: grpc::Block) -> Self {
         self.tari_block = Some(tari_block);
         self
     }
 
-    pub fn tari_miner_data(mut self, miner_data: MinerData) -> Self {
+    pub fn tari_miner_data(mut self, miner_data: grpc::MinerData) -> Self {
         self.tari_miner_data = Some(miner_data);
         self
     }
@@ -163,7 +167,7 @@ impl BlockTemplateDataBuilder {
             .ok_or_else(|| MmProxyError::MissingDataError("tari_difficulty not provided".to_string()))?;
 
         Ok(BlockTemplateData {
-            monero_seed: FixedByteArray::from_hex(&monero_seed).map_err(|_| MmProxyError::InvalidRandomXSeed)?,
+            monero_seed,
             tari_block,
             tari_miner_data,
             monero_difficulty,

applications/tari_merge_mining_proxy/src/block_template_protocol.rs

@@ -0,0 +1,272 @@
+//  Copyright 2021, The Tari Project
+//
+//  Redistribution and use in source and binary forms, with or without modification, are permitted provided that the
+//  following conditions are met:
+//
+//  1. Redistributions of source code must retain the above copyright notice, this list of conditions and the following
+//  disclaimer.
+//
+//  2. Redistributions in binary form must reproduce the above copyright notice, this list of conditions and the
+//  following disclaimer in the documentation and/or other materials provided with the distribution.
+//
+//  3. Neither the name of the copyright holder nor the names of its contributors may be used to endorse or promote
+//  products derived from this software without specific prior written permission.
+//
+//  THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+//  INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+//  DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+//  SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+//  SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
+//  WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
+//  USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+use crate::{
+    block_template_data::{BlockTemplateData, BlockTemplateDataBuilder},
+    common::merge_mining,
+    error::MmProxyError,
+};
+use log::*;
+use std::cmp;
+use tari_app_grpc::tari_rpc as grpc;
+use tari_core::proof_of_work::{monero_rx, monero_rx::FixedByteArray, Difficulty};
+
+const LOG_TARGET: &str = "tari_mm_proxy::proxy::block_template_protocol";
+
+pub struct BlockTemplateProtocol<'a> {
+    base_node_client: &'a mut grpc::base_node_client::BaseNodeClient<tonic::transport::Channel>,
+    wallet_client: &'a mut grpc::wallet_client::WalletClient<tonic::transport::Channel>,
+}
+
+impl<'a> BlockTemplateProtocol<'a> {
+    pub fn new(
+        base_node_client: &'a mut grpc::base_node_client::BaseNodeClient<tonic::transport::Channel>,
+        wallet_client: &'a mut grpc::wallet_client::WalletClient<tonic::transport::Channel>,
+    ) -> Self {
+        Self {
+            base_node_client,
+            wallet_client,
+        }
+    }
+}
+
+impl BlockTemplateProtocol<'_> {
+    pub async fn get_next_block_template(
+        mut self,
+        monero_mining_data: MoneroMiningData,
+    ) -> Result<FinalBlockTemplateData, MmProxyError> {
+        loop {
+            let new_template = self.get_new_block_template().await?;
+            let coinbase = self.get_coinbase(&new_template).await?;
+
+            let template_height = new_template.template.header.as_ref().map(|h| h.height).unwrap_or(0);
+            if !self.check_expected_tip(template_height).await? {
+                debug!(
+                    target: LOG_TARGET,
+                    "Chain tip has progressed past template height {}. Fetching a new block template.", template_height
+                );
+                continue;
+            }
+
+            debug!(target: LOG_TARGET, "Added coinbase to new block template");
+            let block_template_with_coinbase = merge_mining::add_coinbase(coinbase, new_template.template.clone())?;
+            info!(
+                target: LOG_TARGET,
+                "Received new block template from Tari base node for height #{}",
+                new_template
+                    .template
+                    .header
+                    .as_ref()
+                    .map(|h| h.height)
+                    .unwrap_or_default(),
+            );
+            let block = match self.get_new_block(block_template_with_coinbase).await {
+                Ok(b) => b,
+                Err(MmProxyError::FailedPreconditionBlockLostRetry) => {
+                    debug!(
+                        target: LOG_TARGET,
+                        "Chain tip has progressed past template height {}. Fetching a new block template.",
+                        template_height
+                    );
+                    continue;
+                },
+                Err(err) => return Err(err),
+            };
+
+            let final_block = self.add_monero_data(block, monero_mining_data, new_template)?;
+            return Ok(final_block);
+        }
+    }
+
+    async fn get_new_block(
+        &mut self,
+        template: grpc::NewBlockTemplate,
+    ) -> Result<grpc::GetNewBlockResult, MmProxyError> {
+        let resp = self.base_node_client.get_new_block(template).await;
+
+        match resp {
+            Ok(resp) => Ok(resp.into_inner()),
+            Err(status) => {
+                if status.code() == tonic::Code::FailedPrecondition {
+                    return Err(MmProxyError::FailedPreconditionBlockLostRetry);
+                }
+                Err(status.into())
+            },
+        }
+    }
+
+    async fn get_new_block_template(&mut self) -> Result<NewBlockTemplateData, MmProxyError> {
+        let grpc::NewBlockTemplateResponse {
+            miner_data,
+            new_block_template: template,
+            initial_sync_achieved,
+        } = self
+            .base_node_client
+            .get_new_block_template(grpc::NewBlockTemplateRequest {
+                algo: Some(grpc::PowAlgo {
+                    pow_algo: grpc::pow_algo::PowAlgos::Monero.into(),
+                }),
+                max_weight: 0,
+            })
+            .await
+            .map_err(|status| MmProxyError::GrpcRequestError {
+                status,
+                details: "failed to get new block template".to_string(),
+            })?
+            .into_inner();
+
+        let miner_data = miner_data.ok_or(MmProxyError::GrpcResponseMissingField("miner_data"))?;
+        let template = template.ok_or(MmProxyError::GrpcResponseMissingField("new_block_template"))?;
+        Ok(NewBlockTemplateData {
+            template,
+            miner_data,
+            initial_sync_achieved,
+        })
+    }
+
+    async fn check_expected_tip(&mut self, height: u64) -> Result<bool, MmProxyError> {
+        let tip = self
+            .base_node_client
+            .clone()
+            .get_tip_info(tari_app_grpc::tari_rpc::Empty {})
+            .await?
+            .into_inner();
+        let tip_height = tip.metadata.as_ref().map(|m| m.height_of_longest_chain).unwrap_or(0);
+
+        if height <= tip_height {
+            warn!(
+                target: LOG_TARGET,
+                "Base node received next block (height={}) that has invalidated the block template (height={})",
+                tip_height,
+                height
+            );
+            return Ok(false);
+        }
+        Ok(true)
+    }
+
+    async fn get_coinbase(&mut self, template: &NewBlockTemplateData) -> Result<grpc::Transaction, MmProxyError> {
+        let miner_data = &template.miner_data;
+        let tari_height = template.height();
+        let block_reward = miner_data.reward;
+        let total_fees = miner_data.total_fees;
+
+        let coinbase_response = self
+            .wallet_client
+            .get_coinbase(grpc::GetCoinbaseRequest {
+                reward: block_reward,
+                fee: total_fees,
+                height: tari_height,
+            })
+            .await
+            .map_err(|status| MmProxyError::GrpcRequestError {
+                status,
+                details: "failed to get new block template".to_string(),
+            })?;
+        let coinbase = coinbase_response
+            .into_inner()
+            .transaction
+            .ok_or_else(|| MmProxyError::MissingDataError("Coinbase Invalid".to_string()))?;
+        Ok(coinbase)
+    }
+
+    fn add_monero_data(
+        &self,
+        tari_block: grpc::GetNewBlockResult,
+        monero_mining_data: MoneroMiningData,
+        template_data: NewBlockTemplateData,
+    ) -> Result<FinalBlockTemplateData, MmProxyError> {
+        debug!(target: LOG_TARGET, "New block received from Tari: {:?}", tari_block);
+
+        let tari_difficulty = template_data.miner_data.target_difficulty;
+        let block_template_data = BlockTemplateDataBuilder::new()
+            .tari_block(
+                tari_block
+                    .block
+                    .ok_or(MmProxyError::GrpcResponseMissingField("block"))?,
+            )
+            .tari_miner_data(template_data.miner_data)
+            .monero_seed(monero_mining_data.seed_hash)
+            .monero_difficulty(monero_mining_data.difficulty)
+            .tari_difficulty(tari_difficulty)
+            .build()?;
+
+        // Deserialize the block template blob
+        debug!(target: LOG_TARGET, "Deserializing Blocktemplate Blob into Monero Block",);
+        let mut monero_block = monero_rx::deserialize_monero_block_from_hex(&monero_mining_data.blocktemplate_blob)?;
+
+        debug!(target: LOG_TARGET, "Appending Merged Mining Tag",);
+        // Add the Tari merge mining tag to the retrieved block template
+        monero_rx::append_merge_mining_tag(&mut monero_block, &tari_block.merge_mining_hash)?;
+
+        debug!(target: LOG_TARGET, "Creating blockhashing blob from blocktemplate blob",);
+        // Must be done after the tag is inserted since it will affect the hash of the miner tx
+        let blockhashing_blob = monero_rx::create_blockhashing_blob_from_block(&monero_block)?;
+        let blocktemplate_blob = monero_rx::serialize_monero_block_to_hex(&monero_block)?;
+
+        let monero_difficulty = monero_mining_data.difficulty;
+        let mining_difficulty = cmp::min(monero_difficulty, tari_difficulty);
+        info!(
+            target: LOG_TARGET,
+            "Difficulties: Tari ({}), Monero({}), Selected({})",
+            tari_difficulty,
+            monero_mining_data.difficulty,
+            mining_difficulty
+        );
+        Ok(FinalBlockTemplateData {
+            template: block_template_data,
+            target_difficulty: mining_difficulty.into(),
+            blockhashing_blob,
+            blocktemplate_blob,
+            merge_mining_hash: tari_block.merge_mining_hash,
+        })
+    }
+}
+
+/// Private convenience container struct for new template data
+struct NewBlockTemplateData {
+    pub template: grpc::NewBlockTemplate,
+    pub miner_data: grpc::MinerData,
+    pub initial_sync_achieved: bool,
+}
+
+impl NewBlockTemplateData {
+    pub fn height(&self) -> u64 {
+        self.template.header.as_ref().map(|h| h.height).unwrap_or(0)
+    }
+}
+
+/// Final outputs for required for merge mining
+pub struct FinalBlockTemplateData {
+    pub template: BlockTemplateData,
+    pub target_difficulty: Difficulty,
+    pub blockhashing_blob: String,
+    pub blocktemplate_blob: String,
+    pub merge_mining_hash: Vec<u8>,
+}
+
+/// Container struct for monero mining data inputs obtained from monerod
+pub struct MoneroMiningData {
+    pub seed_hash: FixedByteArray,
+    pub blocktemplate_blob: String,
+    pub difficulty: u64,
+}