Bug/is 879 invalid receipt

libethereum/BlockChain.cpp

@@ -44,6 +44,7 @@
 #include <libethcore/Exceptions.h>
 
 #include <libskale/AmsterdamFixPatch.h>
+#include <libskale/SkipInvalidTransactionsPatch.h>
 #include <libskale/TotalStorageUsedPatch.h>
 
 #include "Block.h"
@@ -172,6 +173,24 @@ unsigned c_maxCacheSize = 1024 * 1024 * 64;
 /// Min size, below which we don't bother flushing it.
 unsigned c_minCacheSize = 1024 * 1024 * 32;
 
+bool hasPotentialInvalidTransactionsInBlock( BlockNumber _bn, const BlockChain& _bc ) {
+    if ( _bn == 0 )
+        return false;
+
+    if ( SkipInvalidTransactionsPatch::getActivationTimestamp() == 0 )
+        return true;
+
+    if ( _bn == PendingBlock )
+        return !SkipInvalidTransactionsPatch::isEnabled();
+
+    if ( _bn == LatestBlock )
+        _bn = _bc.number();
+
+    time_t prev_ts = _bc.info( _bc.numberHash( _bn - 1 ) ).timestamp();
+
+    return prev_ts < SkipInvalidTransactionsPatch::getActivationTimestamp();
+}
+
 string BlockChain::getChainDirName( const ChainParams& _cp ) {
     return toHex( BlockHeader( _cp.genesisBlock() ).hash().ref().cropped( 0, 4 ) );
 }
@@ -334,6 +353,49 @@ void BlockChain::close() {
     m_lastBlockHashes->clear();
 }
 
+std::pair< h256, unsigned > BlockChain::transactionLocation( h256 const& _transactionHash ) const {
+    // cached transactionAddresses for transactions with gasUsed==0 should be re-queried from DB
+    bool cached = false;
+    {
+        ReadGuard g( x_transactionAddresses );
+        cached = m_transactionAddresses.count( _transactionHash ) > 0;
+    }
+
+    // get transactionAddresses from DB or cache
+    TransactionAddress ta = queryExtras< TransactionAddress, ExtraTransactionAddress >(
+        _transactionHash, m_transactionAddresses, x_transactionAddresses, NullTransactionAddress );
+
+    if ( !ta )
+        return std::pair< h256, unsigned >( h256(), 0 );
+
+    auto blockNumber = this->number( ta.blockHash );
+
+    if ( !hasPotentialInvalidTransactionsInBlock( blockNumber, *this ) )
+        return std::make_pair( ta.blockHash, ta.index );
+
+    // rest is for blocks with possibility of invalid transactions
+
+    // compute gas used
+    TransactionReceipt receipt = transactionReceipt( ta.blockHash, ta.index );
+    u256 cumulativeGasUsed = receipt.cumulativeGasUsed();
+    u256 prevGasUsed =
+        ta.index == 0 ? 0 : transactionReceipt( ta.blockHash, ta.index - 1 ).cumulativeGasUsed();
+    u256 gasUsed = cumulativeGasUsed - prevGasUsed;
+
+    // re-query receipt from DB if gasUsed==0 (and cache might have wrong value)
+    if ( gasUsed == 0 && cached ) {
+        // remove from cache
+        {
+            WriteGuard g( x_transactionAddresses );
+            m_transactionAddresses.erase( _transactionHash );
+        }
+        // re-read from DB
+        ta = queryExtras< TransactionAddress, ExtraTransactionAddress >( _transactionHash,
+            m_transactionAddresses, x_transactionAddresses, NullTransactionAddress );
+    }
+    return std::make_pair( ta.blockHash, ta.index );
+}
+
 string BlockChain::dumpDatabase() const {
     ostringstream oss;
     oss << m_lastBlockHash << '\n';

libethereum/BlockChain.h

@@ -217,12 +217,13 @@ class BlockChain {
 
     /// Get the transaction receipt by transaction hash. Thread-safe.
     TransactionReceipt transactionReceipt( h256 const& _transactionHash ) const {
-        TransactionAddress ta =
-            queryExtras< TransactionAddress, ExtraTransactionAddress >( _transactionHash,
-                m_transactionAddresses, x_transactionAddresses, NullTransactionAddress );
-        if ( !ta )
+        std::pair< h256, unsigned > tl = transactionLocation( _transactionHash );
+
+        if ( !tl.first )
             return bytesConstRef();
-        return transactionReceipt( ta.blockHash, ta.index );
+
+        // in most cases will hit cache because of transactionLocation() call above
+        return transactionReceipt( tl.first, tl.second );
     }
 
     /// Get a list of transaction hashes for a given block. Thread-safe.
@@ -311,14 +312,8 @@ class BlockChain {
             return bytes();
         return transaction( ta.blockHash, ta.index );
     }
-    std::pair< h256, unsigned > transactionLocation( h256 const& _transactionHash ) const {
-        TransactionAddress ta =
-            queryExtras< TransactionAddress, ExtraTransactionAddress >( _transactionHash,
-                m_transactionAddresses, x_transactionAddresses, NullTransactionAddress );
-        if ( !ta )
-            return std::pair< h256, unsigned >( h256(), 0 );
-        return std::make_pair( ta.blockHash, ta.index );
-    }
+
+    std::pair< h256, unsigned > transactionLocation( h256 const& _transactionHash ) const;
 
     /// Get a block's transaction (RLP format) for the given block hash (or the most recent mined if
     /// none given) & index. Thread-safe.

test/unittests/libethereum/SkaleHost.cpp

@@ -194,8 +194,8 @@ struct SkaleHostFixture : public TestOutputHelperFixture {
 #define CHECK_BLOCK_BEGIN auto blockBefore = client->number()
 
 #define REQUIRE_BLOCK_INCREASE( increase ) \
-    auto blockAfter = client->number();    \
-    BOOST_REQUIRE_EQUAL( blockAfter - blockBefore, increase )
+    { auto blockAfter = client->number();    \
+    BOOST_REQUIRE_EQUAL( blockAfter - blockBefore, increase ); }
 
 #define REQUIRE_BLOCK_SIZE( number, s )                                             \
     {                                                                               \
@@ -214,18 +214,18 @@ struct SkaleHostFixture : public TestOutputHelperFixture {
 #define CHECK_NONCE_BEGIN( senderAddress ) u256 nonceBefore = client->countAt( senderAddress )
 
 #define REQUIRE_NONCE_INCREASE( senderAddress, increase ) \
-    u256 nonceAfter = client->countAt( senderAddress );   \
-    BOOST_REQUIRE_EQUAL( nonceAfter - nonceBefore, increase )
+    { u256 nonceAfter = client->countAt( senderAddress );   \
+    BOOST_REQUIRE_EQUAL( nonceAfter - nonceBefore, increase ); }
 
 #define CHECK_BALANCE_BEGIN( senderAddress ) u256 balanceBefore = client->balanceAt( senderAddress )
 
 #define REQUIRE_BALANCE_DECREASE( senderAddress, decrease ) \
-    u256 balanceAfter = client->balanceAt( senderAddress ); \
-    BOOST_REQUIRE_EQUAL( balanceBefore - balanceAfter, decrease )
+    { u256 balanceAfter = client->balanceAt( senderAddress ); \
+    BOOST_REQUIRE_EQUAL( balanceBefore - balanceAfter, decrease ); }
 
 #define REQUIRE_BALANCE_DECREASE_GE( senderAddress, decrease ) \
-    u256 balanceAfter = client->balanceAt( senderAddress );    \
-    BOOST_REQUIRE_GE( balanceBefore - balanceAfter, decrease )
+    { u256 balanceAfter = client->balanceAt( senderAddress );    \
+    BOOST_REQUIRE_GE( balanceBefore - balanceAfter, decrease ); }
 
 BOOST_FIXTURE_TEST_SUITE( SkaleHostSuite, SkaleHostFixture )  //, *boost::unit_test::disabled() )
 
@@ -752,6 +752,32 @@ BOOST_DATA_TEST_CASE( transactionBalanceBad, skipInvalidTransactionsVariants, sk
 
     REQUIRE_NONCE_INCREASE( senderAddress, 0 );
     REQUIRE_BALANCE_DECREASE( senderAddress, 0 );
+
+    // step 2: check that receipt "moved" to another block after successfull re-execution of the same transaction
+
+    if(!skipInvalidTransactionsFlag){
+        LocalisedTransactionReceipt r1 = client->localisedTransactionReceipt(txHash);
+        BOOST_REQUIRE_EQUAL(r1.blockNumber(), 1);
+        BOOST_REQUIRE_EQUAL(r1.gasUsed(), 0);
+        LocalisedTransaction lt = client->localisedTransaction(txHash);
+        BOOST_REQUIRE_EQUAL(lt.blockNumber(), 1);
+    }
+
+    // make money
+    dev::eth::simulateMining( *client, 1, senderAddress );
+
+    stub->createBlock( ConsensusExtFace::transactions_vector{stream.out()}, utcTime(), 2U );
+
+    REQUIRE_BLOCK_SIZE( 2, 1 );
+    REQUIRE_BLOCK_TRANSACTION( 2, 0, txHash );
+    REQUIRE_NONCE_INCREASE( senderAddress, 1 );
+    REQUIRE_BALANCE_DECREASE_GE( senderAddress, 1 );
+
+    LocalisedTransactionReceipt r2 = client->localisedTransactionReceipt(txHash);
+    BOOST_REQUIRE_EQUAL(r2.blockNumber(), 2);
+    BOOST_REQUIRE_GE(r2.gasUsed(), 21000);
+    LocalisedTransaction lt = client->localisedTransaction(txHash);
+    BOOST_REQUIRE_EQUAL(lt.blockNumber(), 2);
 }
 
 // Transaction should be IGNORED during execution or absent if skipInvalidTransactionsFlag

test/unittests/libweb3jsonrpc/jsonrpc.cpp

@@ -2988,6 +2988,20 @@ BOOST_AUTO_TEST_CASE( skip_invalid_transactions ) {
     BOOST_REQUIRE_EQUAL(cnt.asString(), "0x3");
     cnt = fixture.rpcClient->eth_getBlockTransactionCountByHash(bh);
     BOOST_REQUIRE_EQUAL(cnt.asString(), "0x3");
+
+    // send it successfully
+
+    // make money
+    dev::eth::simulateMining( *fixture.client, 1);
+
+    h2 = fixture.client->importTransaction( tx2 ); // invalid
+
+    dev::eth::mineTransaction(*(fixture.client), 1);
+
+    // checks:
+    Json::Value r2;
+    BOOST_REQUIRE_NO_THROW(r2 = fixture.rpcClient->eth_getTransactionReceipt(toJS(h2)));
+    BOOST_REQUIRE_EQUAL(r2["blockNumber"], toJS(fixture.client->number()));
 #endif
 }