Fix for duplicatiing transactions (IS-869)

libethcore/ChainOperationParams.h

@@ -175,6 +175,7 @@ struct SChain {
     time_t verifyDaSigsPatchTimestamp = 0;
     time_t storageDestructionPatchTimestamp = 0;
     time_t powCheckPatchTimestamp = 0;
+    time_t skipInvalidTransactionsPatchTimestamp = 0;
 
     SChain() {
         name = "TestChain";

libethereum/Block.cpp

@@ -35,6 +35,7 @@
 #include <libethcore/Exceptions.h>
 #include <libethcore/SealEngine.h>
 #include <libevm/VMFactory.h>
+#include <libskale/SkipInvalidTransactionsPatch.h>
 #include <boost/filesystem.hpp>
 #include <boost/timer.hpp>
 #include <ctime>
@@ -483,31 +484,38 @@
                 LOG( m_logger ) << "Transaction " << tr.sha3() << " WouldNotBeInBlock: gasPrice "
                                 << tr.gasPrice() << " < " << _gasPrice;
 
-                // Add to the user-originated transactions that we've executed.
-                m_transactions.push_back( tr );
-                m_transactionSet.insert( tr.sha3() );
+                if ( SkipInvalidTransactionsPatch::isEnabled() ) {
+                    // Add to the user-originated transactions that we've executed.
+                    m_transactions.push_back( tr );
+                    m_transactionSet.insert( tr.sha3() );
 
-                // TODO deduplicate
-                // "bad" transaction receipt for failed transactions
-                TransactionReceipt const null_receipt =
-                    info().number() >= sealEngine()->chainParams().byzantiumForkBlock ?
-                        TransactionReceipt( 0, info().gasUsed(), LogEntries() ) :
-                        TransactionReceipt( EmptyTrie, info().gasUsed(), LogEntries() );
+                    // TODO deduplicate
+                    // "bad" transaction receipt for failed transactions
+                    TransactionReceipt const null_receipt =
+                        info().number() >= sealEngine()->chainParams().byzantiumForkBlock ?
+                            TransactionReceipt( 0, info().gasUsed(), LogEntries() ) :
+                            TransactionReceipt( EmptyTrie, info().gasUsed(), LogEntries() );
 
-                m_receipts.push_back( null_receipt );
-                receipts.push_back( null_receipt );
+                    m_receipts.push_back( null_receipt );
+                    receipts.push_back( null_receipt );
 
-                ++count_bad;
+                    ++count_bad;
+                }
 
                 continue;
             }
 
             ExecutionResult res =
                 execute( _bc.lastBlockHashes(), tr, Permanence::Committed, OnOpFunc() );
-            receipts.push_back( m_receipts.back() );
 
-            if ( res.excepted == TransactionException::WouldNotBeInBlock )
-                ++count_bad;
+            if ( !SkipInvalidTransactionsPatch::isEnabled() ||
+                 res.excepted != TransactionException::WouldNotBeInBlock ) {
+                receipts.push_back( m_receipts.back() );
+
+                // if added but bad
+                if ( res.excepted == TransactionException::WouldNotBeInBlock )
+                    ++count_bad;
+            }
 
             //
             // Debug only, related SKALE-2814 partial catchup testing
@@ -862,9 +870,12 @@
     if ( _p == Permanence::Committed || _p == Permanence::CommittedWithoutState ||
          _p == Permanence::Uncommitted ) {
         // Add to the user-originated transactions that we've executed.
-        m_transactions.push_back( _t );
-        m_receipts.push_back( resultReceipt.second );
-        m_transactionSet.insert( _t.sha3() );
+        if ( !SkipInvalidTransactionsPatch::isEnabled() ||
+             resultReceipt.first.excepted != TransactionException::WouldNotBeInBlock ) {
+            m_transactions.push_back( _t );
+            m_receipts.push_back( resultReceipt.second );
+            m_transactionSet.insert( _t.sha3() );
+        }
     }
     if ( _p == Permanence::Committed || _p == Permanence::Uncommitted ) {
         m_state = stateSnapshot.createStateModifyCopyAndPassLock();

libethereum/ChainParams.cpp

@@ -266,6 +266,11 @@ ChainParams ChainParams::loadConfig(
                                        sChainObj.at( "powCheckPatchTimestamp" ).get_int64() :
                                        0;
 
+        s.skipInvalidTransactionsPatchTimestamp =
+            sChainObj.count( "skipInvalidTransactionsPatchTimestamp" ) ?
+                sChainObj.at( "skipInvalidTransactionsPatchTimestamp" ).get_int64() :
+                0;
+
         if ( sChainObj.count( "nodeGroups" ) ) {
             std::vector< NodeGroup > nodeGroups;
             for ( const auto& nodeGroupConf : sChainObj["nodeGroups"].get_obj() ) {

libethereum/Client.cpp

@@ -55,6 +55,7 @@
 #include <libskale/ContractStorageZeroValuePatch.h>
 #include <libskale/POWCheckPatch.h>
 #include <libskale/RevertableFSPatch.h>
+#include <libskale/SkipInvalidTransactionsPatch.h>
 #include <libskale/State.h>
 #include <libskale/StorageDestructionPatch.h>
 #include <libskale/TotalStorageUsedPatch.h>
@@ -162,6 +163,8 @@ Client::Client( ChainParams const& _params, int _networkID,
     RevertableFSPatch::setTimestamp( chainParams().sChain.revertableFSPatchTimestamp );
     StorageDestructionPatch::setTimestamp( chainParams().sChain.storageDestructionPatchTimestamp );
     POWCheckPatch::setTimestamp( chainParams().sChain.powCheckPatchTimestamp );
+    SkipInvalidTransactionsPatch::setTimestamp(
+        this->chainParams().sChain.skipInvalidTransactionsPatchTimestamp );
 }
 
 
@@ -654,7 +657,7 @@ size_t Client::syncTransactions(
     RevertableFSPatch::lastBlockTimestamp = blockChain().info().timestamp();
     StorageDestructionPatch::lastBlockTimestamp = blockChain().info().timestamp();
     POWCheckPatch::lastBlockTimestamp = blockChain().info().timestamp();
-
+    SkipInvalidTransactionsPatch::lastBlockTimestamp = blockChain().info().timestamp();
 
     DEV_WRITE_GUARDED( x_working ) {
         assert( !m_working.isSealed() );

libethereum/ValidationSchemes.cpp

@@ -268,7 +268,9 @@ void validateConfigJson( js::mObject const& _obj ) {
             { "storageDestructionPatchTimestamp",
                 { { js::int_type }, JsonFieldPresence::Optional } },
             { "powCheckPatchTimestamp", { { js::int_type }, JsonFieldPresence::Optional } },
-            { "nodeGroups", { { js::obj_type }, JsonFieldPresence::Optional } } } );
+            { "nodeGroups", { { js::obj_type }, JsonFieldPresence::Optional } },
+            { "skipInvalidTransactionsPatchTimestamp",
+                { { js::int_type }, JsonFieldPresence::Optional } } } );
 
     js::mArray const& nodes = sChain.at( "nodes" ).get_array();
     for ( auto const& obj : nodes ) {

libskale/CMakeLists.txt

@@ -20,6 +20,7 @@ set(sources
     OverlayFS.cpp
     StorageDestructionPatch.cpp
     POWCheckPatch.cpp
+    SkipInvalidTransactionsPatch.cpp
 )
 
 set(headers
@@ -39,6 +40,7 @@ set(headers
     RevertableFSPatch.h
     POWCheckPatch.h
     OverlayFS.h
+    SkipInvalidTransactionsPatch.h
 )
 
 add_library(skale ${sources} ${headers})

libskale/SkipInvalidTransactionsPatch.cpp

@@ -0,0 +1,12 @@
+#include "SkipInvalidTransactionsPatch.h"
+
+using namespace dev::eth;
+
+time_t SkipInvalidTransactionsPatch::activationTimestamp;
+time_t SkipInvalidTransactionsPatch::lastBlockTimestamp;
+
+bool SkipInvalidTransactionsPatch::isEnabled() {
+    if ( activationTimestamp == 0 )
+        return false;
+    return lastBlockTimestamp >= activationTimestamp;
+}

libskale/SkipInvalidTransactionsPatch.h

@@ -0,0 +1,52 @@
+#ifndef SKIPINVALIDTRANSACTIONSPATCH_H
+#define SKIPINVALIDTRANSACTIONSPATCH_H
+
+#include <libethcore/BlockHeader.h>
+#include <libethereum/ChainParams.h>
+#include <libethereum/Interface.h>
+#include <libethereum/SchainPatch.h>
+#include <libethereum/Transaction.h>
+
+namespace dev {
+namespace eth {
+class Client;
+}
+}  // namespace dev
+
+// What this patch does:
+// 1. "Invalid" transactions that came with winning block proposal from consensus
+// are skipped, and not included in block.
+// Their "validity is determined in Block::syncEveryone:
+// a) Transactions should have gasPrice >= current block min gas price
+// b) State::execute should not throw (it causes WouldNotBeInBlock exception).
+//    Usually this exception is caused by Executive::verifyTransaction() failure.
+//
+// 2. Specifically for historic node - we ignore "invalid" transactions that
+//    are already in block as though they never came.
+// This affects following JSON-RPC calls:
+// 1) eth_getBlockByHash/Number
+// 2) eth_getTransactionReceipt (affects "transactionIndex" field)
+// 3) eth_getBlockTransactionCountByHash/Number
+// 4) eth_getTransactionByHash (invalid transactions are treated as never present)
+// 5) eth_getTransactionByBlockHash/NumberAndIndex
+// Transactions are removed from Transaction Queue as usually.
+
+// TODO better start to apply patches from 1st block after timestamp, not second
+class SkipInvalidTransactionsPatch : public SchainPatch {
+public:
+    static bool isEnabled();
+
+    static void setTimestamp( time_t _activationTimestamp ) {
+        activationTimestamp = _activationTimestamp;
+        printInfo( __FILE__, _activationTimestamp );
+    }
+
+    static time_t getActivationTimestamp() { return activationTimestamp; }
+
+private:
+    friend class dev::eth::Client;
+    static time_t activationTimestamp;
+    static time_t lastBlockTimestamp;
+};
+
+#endif  // SKIPINVALIDTRANSACTIONSPATCH_H