#985 add cutsom macro for block finalize download TCP fallback & add tests for it

PropzSaladaz · PropzSaladaz · commit 0cdc081ba0cc · 2026-03-27T12:58:36.000Z
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -339,6 +339,7 @@ target_link_libraries(consensusd consensus_full)
 file(GLOB_RECURSE COMMON_TEST_SOURCES
         "tests/unit/datastructures/*.cpp"
         "tests/unit/db/*.cpp"
+        "tests/unit/network/*.cpp"
         "tests/consensus_tests.cpp"
         "tests/sgx_tests.cpp"
         "tests/e2e/BlockFinalizeTransportCompatTests.cpp"
diff --git a/SkaleCommon.h b/SkaleCommon.h
@@ -182,6 +182,8 @@ static constexpr uint64_t STUCK_RESTART_INTERVAL_MS = 3 * 60 * 60 * 1000;  // th
 
 static constexpr uint64_t WAIT_AFTER_NETWORK_ERROR_MS = 3000;
 
+static constexpr uint64_t BLOCK_FINALIZE_DOWNLOAD_TCP_FALLBACK_MS = 30 * 60 * 1000; // 30 minutes
+
 static constexpr uint64_t CONNECTION_REFUSED_LOG_INTERVAL_MS = 10 * 60 * 1000;
 
 static constexpr uint64_t CATCHUP_TIMEOUT_SEC = 30;
diff --git a/blockfinalize/client/BlockFinalizeDownloader.cpp b/blockfinalize/client/BlockFinalizeDownloader.cpp
@@ -119,7 +119,7 @@ bool BlockFinalizeDownloader::shouldUseTCPFallback( schain_index _dstIndex ) {
 void BlockFinalizeDownloader::markTCPFallback( schain_index _dstIndex ) {
     LOCK( m );
     tcpFallbackUntilMs[_dstIndex] =
-        Time::getCurrentTimeMs() + getNode()->getWaitAfterNetworkErrorMs();
+        Time::getCurrentTimeMs() + getNode()->getBlockFinalizeDownloadTcpFallbackMs();
 }
 
 void BlockFinalizeDownloader::clearTCPFallback( schain_index _dstIndex ) {
diff --git a/blockfinalize/client/BlockFinalizeDownloader.h b/blockfinalize/client/BlockFinalizeDownloader.h
@@ -151,8 +151,8 @@ class BlockFinalizeDownloader : public Agent {
     bool shouldUseTCPFallback( schain_index _dstIndex );
 
     /**
-     * Mark '_dstIndex' node to use TCP fallback for some time in the future, 
-     * based on current time + configured wait time after network error.
+     * Mark '_dstIndex' node to use TCP fallback for some time in the future,
+     * based on current time + configured TCP fallback duration.
      */
     void markTCPFallback( schain_index _dstIndex );
 
diff --git a/node/Node.cpp b/node/Node.cpp
@@ -278,6 +278,8 @@ void Node::initParamsFromConfig() {
     stuckRestartIntervalMs = getParamUint64( "stuckRestartIntervalMs", STUCK_RESTART_INTERVAL_MS );
     waitAfterNetworkErrorMs =
         getParamUint64( "waitAfterNetworkErrorMs", WAIT_AFTER_NETWORK_ERROR_MS );
+    blockFinalizeDownloadTcpFallbackMs =
+        getParamUint64( "blockFinalizeDownloadTcpFallbackMs", BLOCK_FINALIZE_DOWNLOAD_TCP_FALLBACK_MS );
     maxCatchupDownloadBytes =
         getParamUint64( "maxCatchupDownloadBytes", MAX_CATCHUP_DOWNLOAD_BYTES );
     maxTransactionsPerBlock =
diff --git a/node/Node.h b/node/Node.h
@@ -56,6 +56,7 @@ class TestConfig;
 class BlockSigShareDB;
 class DASigShareDB;
 class DAProofDB;
+class NodeTestAccess;
 
 #ifdef BITE
     class TEDecryptionDB;
@@ -79,6 +80,7 @@ enum PricingStrategyEnum { ZERO, DOS_PROTECT };
 
 class Node {
     friend class Schain;  // Allow Schain to access private barrier methods for teardown
+    friend class NodeTestAccess; // Allow test access to private members
     
     ConsensusEngine* consensusEngine;
 
@@ -217,6 +219,8 @@ class Node {
 
     uint64_t waitAfterNetworkErrorMs = 0;
 
+    uint64_t blockFinalizeDownloadTcpFallbackMs = 0;
+
     uint64_t emptyBlockIntervalMs = 0;
 
     uint64_t emptyBlockIntervalAfterCatchupMs = 0;
@@ -485,6 +489,8 @@ class Node {
 
     uint64_t getWaitAfterNetworkErrorMs();
 
+    uint64_t getBlockFinalizeDownloadTcpFallbackMs();
+
 #ifdef FAIR
     uint64_t getConstantGasPrice() const;
 #endif
diff --git a/node/NodeGettersSetters.cpp b/node/NodeGettersSetters.cpp
@@ -279,6 +279,10 @@ uint64_t Node::getWaitAfterNetworkErrorMs() {
     return waitAfterNetworkErrorMs;
 }
 
+uint64_t Node::getBlockFinalizeDownloadTcpFallbackMs() {
+    return blockFinalizeDownloadTcpFallbackMs;
+}
+
 uint64_t Node::getEmptyBlockIntervalMs() const {
     return emptyBlockIntervalMs;
 }
diff --git a/tests/TestUtils.h b/tests/TestUtils.h
@@ -23,6 +23,7 @@
 
 #pragma once
 
+#include <cstdlib>
 #include <memory>
 #include <string>
 
@@ -36,6 +37,59 @@
 
 namespace TestUtils {
 
+struct TestEnvVarSnapshot {
+    bool hadPreviousValue = false;
+    string previousValue;
+};
+
+inline TestEnvVarSnapshot captureTestEnvVar( const string& _envName ) {
+    TestEnvVarSnapshot snapshot;
+    const char* previousValue = std::getenv( _envName.c_str() );
+    if ( previousValue != nullptr ) {
+        snapshot.hadPreviousValue = true;
+        snapshot.previousValue = previousValue;
+    }
+    return snapshot;
+}
+
+inline TestEnvVarSnapshot setTestEnvVar(
+    const string& _envName, const string& _envValue, int _overwrite = 1 ) {
+    auto snapshot = captureTestEnvVar( _envName );
+    CATCH_REQUIRE( setenv( _envName.c_str(), _envValue.c_str(), _overwrite ) == 0 );
+    return snapshot;
+}
+
+inline void unsetTestEnvVar( const string& _envName ) {
+    CATCH_REQUIRE( unsetenv( _envName.c_str() ) == 0 );
+}
+
+inline void restoreTestEnvVar( const string& _envName, const TestEnvVarSnapshot& _snapshot ) {
+    if ( _snapshot.hadPreviousValue ) {
+        CATCH_REQUIRE( setenv( _envName.c_str(), _snapshot.previousValue.c_str(), 1 ) == 0 );
+    } else {
+        unsetTestEnvVar( _envName );
+    }
+}
+
+class TestEnvVarGuard {
+    string envName;
+    TestEnvVarSnapshot snapshot;
+
+public:
+    explicit TestEnvVarGuard( const string& _envName )
+        : envName( _envName ), snapshot( captureTestEnvVar( _envName ) ) {
+        unsetTestEnvVar( envName );
+    }
+
+    ~TestEnvVarGuard() {
+        if ( snapshot.hadPreviousValue ) {
+            setenv( envName.c_str(), snapshot.previousValue.c_str(), 1 );
+        } else {
+            unsetenv( envName.c_str() );
+        }
+    }
+};
+
 /**
  * @brief Creates a minimal test Node with the given configuration
  * 
diff --git a/tests/e2e/BlockFinalizeTransportCompatTests.cpp b/tests/e2e/BlockFinalizeTransportCompatTests.cpp
@@ -40,7 +40,7 @@ struct ScenarioResult {
 static ScenarioResult runScenario( const string& _configDir ) {
     ConsensusEngine* testEngine = nullptr;
     auto finalizationDownloadOnlySnapshot =
-        E2EHelper::setTestEnvVar( FINALIZATION_DOWNLOAD_ONLY_ENV, "1", 1 );
+        TestUtils::setTestEnvVar( FINALIZATION_DOWNLOAD_ONLY_ENV, "1", 1 );
 
     try {
         E2EHelper::configureTestEnvironment( true, _configDir );
@@ -56,15 +56,15 @@ static ScenarioResult runScenario( const string& _configDir ) {
         CATCH_REQUIRE( stats.totalRequests() > 0 );
 
         E2EHelper::stopEngineGracefully( testEngine );
-        E2EHelper::restoreTestEnvVar(
+        TestUtils::restoreTestEnvVar(
             FINALIZATION_DOWNLOAD_ONLY_ENV, finalizationDownloadOnlySnapshot );
 
         return { block_id( lastId ), stats };
     } catch ( ... ) {
         if ( testEngine != nullptr ) {
             E2EHelper::stopEngineGracefully( testEngine );
         }
-        E2EHelper::restoreTestEnvVar(
+        TestUtils::restoreTestEnvVar(
             FINALIZATION_DOWNLOAD_ONLY_ENV, finalizationDownloadOnlySnapshot );
         throw;
     }
diff --git a/tests/e2e/E2ETestHelper.h b/tests/e2e/E2ETestHelper.h
@@ -59,10 +59,10 @@ class E2ETestHelper {
 
         boost::filesystem::create_directories( TEST_DATA_ROOT_DIR );
 
-        setTestEnvVar( "DATA_DIR", TEST_DATA_ROOT_DIR.c_str() );
+        TestUtils::setTestEnvVar( "DATA_DIR", TEST_DATA_ROOT_DIR.c_str() );
 
         // Keep default runtime for fast tests unless caller already set TEST_TIME_S.
-        setTestEnvVar( "TEST_TIME_S", "4", 0 );
+        TestUtils::setTestEnvVar( "TEST_TIME_S", "4", 0 );
     }
 
     static void startEngine(
@@ -126,37 +126,6 @@ class E2ETestHelper {
         delete _engine;
         _engine = nullptr;
     }
-
-    // ========= Environment variable helpers for tests ========= 
-    struct TestEnvVarSnapshot {
-        bool hadPreviousValue = false;
-        string previousValue;
-    };
-
-    static TestEnvVarSnapshot setTestEnvVar(
-        const string& _envName, const string& _envValue, int _overwrite = 1 ) {
-        TestEnvVarSnapshot snapshot;
-        const char* previousValue = std::getenv( _envName.c_str() );
-        if ( previousValue != nullptr ) {
-            snapshot.hadPreviousValue = true;
-            snapshot.previousValue = previousValue;
-        }
-
-        CATCH_REQUIRE( setenv( _envName.c_str(), _envValue.c_str(), _overwrite ) == 0 );
-        return snapshot;
-    }
-
-    static void unsetTestEnvVar( const string& _envName ) {
-        CATCH_REQUIRE( unsetenv( _envName.c_str() ) == 0 );
-    }
-
-    static void restoreTestEnvVar( const string& _envName, const TestEnvVarSnapshot& _snapshot ) {
-        if ( _snapshot.hadPreviousValue ) {
-            CATCH_REQUIRE( setenv( _envName.c_str(), _snapshot.previousValue.c_str(), 1 ) == 0 );
-        } else {
-            unsetTestEnvVar( _envName );
-        }
-    }
 };
 
 }  // namespace E2ETestUtils
diff --git a/tests/unit/network/NodeConfigTests.cpp b/tests/unit/network/NodeConfigTests.cpp
@@ -0,0 +1,45 @@
+#include "thirdparty/catch.hpp"
+
+#include "SkaleCommon.h"
+#include "tests/TestUtils.h"
+
+namespace {
+
+nlohmann::json makeTestNodeConfig() {
+    nlohmann::json cfg;
+    cfg["nodeID"] = 1;
+    cfg["nodeName"] = "testNode";
+    cfg["bindIP"] = "127.0.0.1";
+    cfg["basePort"] = 31000;
+    return cfg;
+}
+
+}  // namespace
+
+CATCH_TEST_CASE( "BlockFinalize TCP fallback timeout uses config override",
+    "[network][node][unit]" ) {
+    constexpr uint64_t overrideTimeoutMs = 4242;
+    TestUtils::TestEnvVarGuard envGuard( "blockFinalizeDownloadTcpFallbackMs" );
+
+    auto cfg = makeTestNodeConfig();
+    cfg["blockFinalizeDownloadTcpFallbackMs"] = overrideTimeoutMs;
+
+    ConsensusEngine engine( 0, 1000000000 );
+    auto node = TestUtils::createTestNode( cfg, &engine, "http://127.0.0.1:8545" );
+
+    CATCH_REQUIRE( node->getBlockFinalizeDownloadTcpFallbackMs() == overrideTimeoutMs );
+}
+
+CATCH_TEST_CASE( "BlockFinalize TCP fallback timeout uses default when config is unset",
+    "[network][node][unit]" ) {
+    TestUtils::TestEnvVarGuard envGuard( "blockFinalizeDownloadTcpFallbackMs" );
+
+    auto cfg = makeTestNodeConfig();
+
+    ConsensusEngine engine( 0, 1000000000 );
+    auto node = TestUtils::createTestNode( cfg, &engine, "http://127.0.0.1:8545" );
+
+    CATCH_REQUIRE(
+        node->getBlockFinalizeDownloadTcpFallbackMs() ==
+        BLOCK_FINALIZE_DOWNLOAD_TCP_FALLBACK_MS );
+}

Original file line number	Diff line number	Diff line change
`@@ -119,7 +119,7 @@ bool BlockFinalizeDownloader::shouldUseTCPFallback( schain_index _dstIndex ) {`
`119`	`119`	`void BlockFinalizeDownloader::markTCPFallback( schain_index _dstIndex ) {`
`120`	`120`	`LOCK( m );`
`121`	`121`	`tcpFallbackUntilMs[_dstIndex] =`
`122`		`- Time::getCurrentTimeMs() + getNode()->getWaitAfterNetworkErrorMs();`
	`122`	`+ Time::getCurrentTimeMs() + getNode()->getBlockFinalizeDownloadTcpFallbackMs();`
`123`	`123`	`}`
`124`	`124`
`125`	`125`	`void BlockFinalizeDownloader::clearTCPFallback( schain_index _dstIndex ) {`
Original file line number	Diff line number	Diff line change
`@@ -279,6 +279,10 @@ uint64_t Node::getWaitAfterNetworkErrorMs() {`
`279`	`279`	`return waitAfterNetworkErrorMs;`
`280`	`280`	`}`
`281`	`281`
	`282`	`+uint64_t Node::getBlockFinalizeDownloadTcpFallbackMs() {`
	`283`	`+ return blockFinalizeDownloadTcpFallbackMs;`
	`284`	`+}`
	`285`	`+`
`282`	`286`	`uint64_t Node::getEmptyBlockIntervalMs() const {`
`283`	`287`	`return emptyBlockIntervalMs;`
`284`	`288`	`}`