Skip to content

Commit

Permalink
Invalid receipts in cache
Browse files Browse the repository at this point in the history
Bug/is 879 invalid receipt
  • Loading branch information
dimalit authored Nov 3, 2023
2 parents b4b996a + ad847ed commit 6f3f330
Show file tree
Hide file tree
Showing 4 changed files with 118 additions and 21 deletions.
62 changes: 62 additions & 0 deletions libethereum/BlockChain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
#include <libethcore/Exceptions.h>

#include <libskale/AmsterdamFixPatch.h>
#include <libskale/SkipInvalidTransactionsPatch.h>
#include <libskale/TotalStorageUsedPatch.h>

#include "Block.h"
Expand Down Expand Up @@ -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 ) );
}
Expand Down Expand Up @@ -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';
Expand Down
21 changes: 8 additions & 13 deletions libethereum/BlockChain.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
42 changes: 34 additions & 8 deletions test/unittests/libethereum/SkaleHost.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 ) \
{ \
Expand All @@ -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() )

Expand Down Expand Up @@ -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
Expand Down
14 changes: 14 additions & 0 deletions test/unittests/libweb3jsonrpc/jsonrpc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down

0 comments on commit 6f3f330

Please sign in to comment.