Merge pull request #1325 from GitGab19/fix-timestamp-bug

Fix timestamp bug - `min_ntime` becomes always the one sent by TP

Author: plebhash

protocols/v2/roles-logic-sv2/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "roles_logic_sv2"
-version = "1.2.2"
+version = "1.2.3"
 authors = ["The Stratum V2 Developers"]
 edition = "2018"
 description = "Common handlers for use within SV2 roles"

protocols/v2/roles-logic-sv2/src/job_creator.rs

@@ -27,6 +27,7 @@ pub struct JobsCreators {
     templte_to_job_id: HashMap<u64, u32, BuildNoHashHasher<u64>>,
     ids: Id,
     last_target: mining_sv2::Target,
+    last_ntime: Option<u32>,
     extranonce_len: u8,
 }
 
@@ -56,6 +57,7 @@ impl JobsCreators {
             templte_to_job_id: HashMap::with_hasher(BuildNoHashHasher::default()),
             ids: Id::new(),
             last_target: mining_sv2::Target::new(0, 0),
+            last_ntime: None,
             extranonce_len,
         }
     }
@@ -91,6 +93,7 @@ impl JobsCreators {
             next_job_id,
             version_rolling_allowed,
             self.extranonce_len,
+            self.last_ntime,
         )
     }
 
@@ -106,6 +109,7 @@ impl JobsCreators {
     /// we clear all the saved templates.
     pub fn on_new_prev_hash(&mut self, prev_hash: &SetNewPrevHash<'static>) -> Option<u32> {
         self.last_target = prev_hash.target.clone().into();
+        self.last_ntime = prev_hash.header_timestamp.into(); // set correct ntime
         let template: Vec<NewTemplate<'static>> = self
             .lasts_new_template
             .clone()
@@ -162,6 +166,7 @@ pub fn extended_job_from_custom_job(
         0,
         true,
         extranonce_len,
+        Some(referenced_job.min_ntime),
     )
 }
 
@@ -181,6 +186,7 @@ fn new_extended_job(
     job_id: u32,
     version_rolling_allowed: bool,
     extranonce_len: u8,
+    ntime: Option<u32>,
 ) -> Result<NewExtendedMiningJob<'static>, Error> {
     coinbase_outputs[0].value = match new_template.coinbase_tx_value_remaining.checked_mul(1) {
         //check that value_remaining is updated by TP
@@ -205,16 +211,11 @@ fn new_extended_job(
         extranonce_len,
     );
 
-    let min_ntime = match new_template.future_template {
-        true => binary_sv2::Sv2Option::new(None),
-        false => {
-            let now = std::time::SystemTime::now()
-                .duration_since(std::time::UNIX_EPOCH)
-                .unwrap()
-                .as_secs() as u32;
-            binary_sv2::Sv2Option::new(Some(now))
-        }
-    };
+    let min_ntime = binary_sv2::Sv2Option::new(if new_template.future_template {
+        None
+    } else {
+        ntime
+    });
 
     let new_extended_mining_job: NewExtendedMiningJob<'static> = NewExtendedMiningJob {
         channel_id: 0,
@@ -432,11 +433,11 @@ impl StrippedCoinbaseTx {
         let mut inputs = self.inputs.clone();
         let last_input = inputs.last_mut().ok_or(Error::BadPayloadSize)?;
         let new_last_input_len =
-        32 // outpoint
-        + 4 // vout
-        + 1 // script length byte -> TODO can be also 3 (based on TODO in `coinbase_tx_prefix()`)
-        + self.bip141_bytes_len // space for bip34 bytes
-        ;
+            32 // outpoint
+                + 4 // vout
+                + 1 // script length byte -> TODO can be also 3 (based on TODO in `coinbase_tx_prefix()`)
+                + self.bip141_bytes_len // space for bip34 bytes
+            ;
         last_input.truncate(new_last_input_len);
         let mut prefix: Vec<u8> = vec![];
         prefix.extend_from_slice(&self.version.to_le_bytes());

roles/Cargo.lock

@@ -81,20 +81,21 @@ pub struct TemplateProvider {
 }
 
 impl TemplateProvider {
-    pub fn start(port: u16) -> Self {
+    pub fn start(port: u16, sv2_interval: u32) -> Self {
         let temp_dir = PathBuf::from("/tmp/.template-provider");
         let mut conf = Conf::default();
         let staticdir = format!(".bitcoin-{}", port);
         conf.staticdir = Some(temp_dir.join(staticdir));
-        let port = format!("-sv2port={}", port);
+        let port_arg = format!("-sv2port={}", port);
+        let sv2_interval_arg = format!("-sv2interval={}", sv2_interval);
         conf.args.extend(vec![
             "-txindex=1",
             "-sv2",
-            &port,
+            &port_arg,
             "-debug=rpc",
             "-debug=sv2",
-            "-sv2interval=20",
-            "-sv2feedelta=1000",
+            &sv2_interval_arg,
+            "-sv2feedelta=0",
             "-loglevel=sv2:trace",
             "-logtimemicros=1",
         ]);
@@ -284,8 +285,9 @@ pub async fn start_pool(
     pool
 }
 
-pub async fn start_template_provider(tp_port: u16) -> TemplateProvider {
-    let template_provider = TemplateProvider::start(tp_port);
+pub async fn start_template_provider(tp_port: u16, sv2_interval: Option<u32>) -> TemplateProvider {
+    let sv2_interval = sv2_interval.unwrap_or(20);
+    let template_provider = TemplateProvider::start(tp_port, sv2_interval);
     template_provider.generate_blocks(16);
     template_provider
 }

roles/tests-integration/tests/common/mod.rs

@@ -1,15 +1,15 @@
 mod common;
 
-use std::convert::TryInto;
-
 use common::{InterceptMessage, MessageDirection};
 use const_sv2::{
-    MESSAGE_TYPE_NEW_TEMPLATE, MESSAGE_TYPE_SETUP_CONNECTION_ERROR, MESSAGE_TYPE_SET_NEW_PREV_HASH,
+    MESSAGE_TYPE_NEW_EXTENDED_MINING_JOB, MESSAGE_TYPE_NEW_TEMPLATE,
+    MESSAGE_TYPE_SETUP_CONNECTION_ERROR,
 };
 use roles_logic_sv2::{
     common_messages_sv2::{Protocol, SetupConnection, SetupConnectionError},
-    parsers::{CommonMessages, PoolMessages, TemplateDistribution},
+    parsers::{AnyMessage, CommonMessages, Mining, PoolMessages, TemplateDistribution},
 };
+use std::convert::TryInto;
 
 // This test starts a Template Provider and a Pool, and checks if they exchange the correct
 // messages upon connection.
@@ -20,7 +20,7 @@ async fn success_pool_template_provider_connection() {
     let sniffer_addr = common::get_available_address();
     let tp_addr = common::get_available_address();
     let pool_addr = common::get_available_address();
-    let _tp = common::start_template_provider(tp_addr.port()).await;
+    let _tp = common::start_template_provider(tp_addr.port(), None).await;
     let sniffer_identifier =
         "success_pool_template_provider_connection tp_pool sniffer".to_string();
     let sniffer_check_on_drop = true;
@@ -64,12 +64,113 @@ async fn success_pool_template_provider_connection() {
     assert_tp_message!(sniffer.next_message_from_upstream(), SetNewPrevHash);
 }
 
+// This test starts a Template Provider, a Pool, and a Translator Proxy, and verifies the
+// correctness of the exchanged messages during connection and operation.
+//
+// Two Sniffers are used:
+// - Between the Template Provider and the Pool.
+// - Between the Pool and the Translator Proxy.
+//
+// The test ensures that:
+// - The Template Provider sends valid `SetNewPrevHash` and `NewTemplate` messages.
+// - The `minntime` field in the second `NewExtendedMiningJob` message sent to the Translator Proxy
+//   matches the `header_timestamp` from the `SetNewPrevHash` message, addressing a bug that
+//   occurred with non-future jobs.
+//
+// Related issue: https://github.com/stratum-mining/stratum/issues/1324
+#[tokio::test]
+async fn header_timestamp_value_assertion_in_new_extended_mining_job() {
+    let tp_pool_sniffer_addr = common::get_available_address();
+    let pool_translator_sniffer_addr = common::get_available_address();
+    let tp_addr = common::get_available_address();
+    let pool_addr = common::get_available_address();
+    let sv2_interval = Some(5);
+    let _tp = common::start_template_provider(tp_addr.port(), sv2_interval).await;
+    let tp_pool_sniffer_identifier =
+        "header_timestamp_value_assertion_in_new_extended_mining_job tp_pool sniffer".to_string();
+    let tp_pool_sniffer = common::start_sniffer(
+        tp_pool_sniffer_identifier,
+        tp_pool_sniffer_addr,
+        tp_addr,
+        false,
+        None,
+    )
+    .await;
+    let _ = common::start_pool(Some(pool_addr), Some(tp_pool_sniffer_addr)).await;
+    let pool_translator_sniffer_identifier =
+        "header_timestamp_value_assertion_in_new_extended_mining_job pool_translator sniffer"
+            .to_string();
+    let pool_translator_sniffer = common::start_sniffer(
+        pool_translator_sniffer_identifier,
+        pool_translator_sniffer_addr,
+        pool_addr,
+        false,
+        None,
+    )
+    .await;
+    let _tproxy_addr = common::start_sv2_translator(pool_translator_sniffer_addr).await;
+    assert_common_message!(
+        &tp_pool_sniffer.next_message_from_upstream(),
+        SetupConnectionSuccess
+    );
+    // Wait for a NewTemplate message from the Template Provider
+    tp_pool_sniffer
+        .wait_for_message_type(MessageDirection::ToDownstream, MESSAGE_TYPE_NEW_TEMPLATE)
+        .await;
+    assert_tp_message!(&tp_pool_sniffer.next_message_from_upstream(), NewTemplate);
+    // Extract header timestamp from SetNewPrevHash message
+    let header_timestamp_to_check = match tp_pool_sniffer.next_message_from_upstream() {
+        Some((_, AnyMessage::TemplateDistribution(TemplateDistribution::SetNewPrevHash(msg)))) => {
+            msg.header_timestamp
+        }
+        _ => panic!("SetNewPrevHash not found!"),
+    };
+    // Assertions of messages between Pool and Translator Proxy (these are not necessary for the
+    // test itself, but they are used to pop from the sniffer's message queue)
+    assert_common_message!(
+        &pool_translator_sniffer.next_message_from_upstream(),
+        SetupConnectionSuccess
+    );
+    assert_mining_message!(
+        &pool_translator_sniffer.next_message_from_upstream(),
+        OpenExtendedMiningChannelSuccess
+    );
+    assert_mining_message!(
+        &pool_translator_sniffer.next_message_from_upstream(),
+        NewExtendedMiningJob
+    );
+    assert_mining_message!(
+        &pool_translator_sniffer.next_message_from_upstream(),
+        SetNewPrevHash
+    );
+    // Wait for a second NewExtendedMiningJob message
+    pool_translator_sniffer
+        .wait_for_message_type(
+            MessageDirection::ToDownstream,
+            MESSAGE_TYPE_NEW_EXTENDED_MINING_JOB,
+        )
+        .await;
+    // Extract min_ntime from the second NewExtendedMiningJob message
+    let second_job_ntime = match pool_translator_sniffer.next_message_from_upstream() {
+        Some((_, AnyMessage::Mining(Mining::NewExtendedMiningJob(job)))) => {
+            job.min_ntime.into_inner()
+        }
+        _ => panic!("Second NewExtendedMiningJob not found!"),
+    };
+    // Assert that min_ntime matches header_timestamp
+    assert_eq!(
+        second_job_ntime,
+        Some(header_timestamp_to_check),
+        "The `minntime` field of the second NewExtendedMiningJob does not match the `header_timestamp`!"
+    );
+}
+
 #[tokio::test]
 async fn test_sniffer_interrupter() {
     let sniffer_addr = common::get_available_address();
     let tp_addr = common::get_available_address();
     let pool_addr = common::get_available_address();
-    let _tp = common::start_template_provider(tp_addr.port()).await;
+    let _tp = common::start_template_provider(tp_addr.port(), None).await;
     use const_sv2::MESSAGE_TYPE_SETUP_CONNECTION_SUCCESS;
     let message =
         PoolMessages::Common(CommonMessages::SetupConnectionError(SetupConnectionError {

roles/tests-integration/tests/pool_integration.rs

@@ -21,7 +21,7 @@ async fn translation_proxy() {
         None,
     )
     .await;
-    let _tp = common::start_template_provider(tp_addr.port()).await;
+    let _tp = common::start_template_provider(tp_addr.port(), None).await;
     let _pool = common::start_pool(Some(pool_addr), Some(tp_addr)).await;
     let tproxy_addr = common::start_sv2_translator(pool_translator_sniffer_addr).await;
     let _mining_device = common::start_mining_device_sv1(tproxy_addr).await;