Skip to content

Commit

Permalink
Merge pull request #1698 from skalenetwork/bug/IS-869-transaction-dup…
Browse files Browse the repository at this point in the history 
…licates

IS-869 Fixed filtering condition in Eth.cpp
  • Loading branch information
@DmytroNazarenko
DmytroNazarenko authored Oct 20, 2023
2 parents b96e9c8 + a7c74c7 commit d87ca50
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 25 deletions.
39 changes: 26 additions & 13 deletions libweb3jsonrpc/Eth.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,18 @@ const uint64_t MAX_RECEIPT_CACHE_ENTRIES = 1024;
using namespace dev::rpc::_detail;

// TODO Check LatestBlock number - update!
void GappedTransactionIndexCache::ensureCached( BlockNumber _bn ) const {
// Needs external locks to exchange read one to write one
void GappedTransactionIndexCache::ensureCached( BlockNumber _bn,
std::shared_lock< std::shared_mutex >& _readLock,
std::unique_lock< std::shared_mutex >& _writeLock ) const {
if ( _bn != PendingBlock && _bn != LatestBlock && real2gappedCache.count( _bn ) )
return;

// change read lock for write lock
// they both will be destroyed externally
_readLock.unlock();
_writeLock.lock();

assert( real2gappedCache.size() <= cacheSize );
if ( real2gappedCache.size() >= cacheSize ) {
real2gappedCache.erase( real2gappedCache.begin() );
Expand Down Expand Up @@ -90,19 +98,24 @@ void GappedTransactionIndexCache::ensureCached( BlockNumber _bn ) const {
} // for
}

bool skippedInvalidTransactionsInBlock( BlockNumber _bn, const Interface& _client ) {
// returns true if block N can contain invalid transactions
// returns false if this block was created with SkipInvalidTransactionsPatch and they were skipped
bool hasPotentialInvalidTransactionsInBlock( BlockNumber _bn, const Interface& _client ) {
if ( _bn == 0 )
return false;

if ( SkipInvalidTransactionsPatch::getActivationTimestamp() == 0 )
return true;

if ( _bn == PendingBlock )
return SkipInvalidTransactionsPatch::isEnabled();
return !SkipInvalidTransactionsPatch::isEnabled();

if ( _bn == LatestBlock )
_bn = _client.number();

time_t prev_ts = _client.blockInfo( _bn - 1 ).timestamp();

return prev_ts >= SkipInvalidTransactionsPatch::getActivationTimestamp();
return prev_ts < SkipInvalidTransactionsPatch::getActivationTimestamp();
}

#endif
Expand Down Expand Up @@ -287,7 +300,7 @@ Json::Value Eth::eth_getBlockTransactionCountByHash( string const& _blockHash )

#ifdef HISTORIC_STATE
BlockNumber bn = client()->numberFromHash( blockHash );
if ( !skippedInvalidTransactionsInBlock( bn, *client() ) )
if ( !hasPotentialInvalidTransactionsInBlock( bn, *client() ) )
#endif
return toJS( client()->transactionCount( blockHash ) );
#ifdef HISTORIC_STATE
Expand All @@ -306,7 +319,7 @@ Json::Value Eth::eth_getBlockTransactionCountByNumber( string const& _blockNumbe

#ifdef HISTORIC_STATE
BlockNumber bn = jsToBlockNumber( _blockNumber );
if ( !skippedInvalidTransactionsInBlock( bn, *client() ) )
if ( !hasPotentialInvalidTransactionsInBlock( bn, *client() ) )
#endif
return toJS( client()->transactionCount( jsToBlockNumber( _blockNumber ) ) );
#ifdef HISTORIC_STATE
Expand Down Expand Up @@ -558,8 +571,8 @@ Json::Value Eth::eth_getBlockByHash( string const& _blockHash, bool _includeTran

#ifdef HISTORIC_STATE
BlockNumber bn = client()->numberFromHash( h );
if ( skippedInvalidTransactionsInBlock( bn, *client() ) ) {
// remove skipped transactions
if ( hasPotentialInvalidTransactionsInBlock( bn, *client() ) ) {
// remove invalid transactions
size_t index = 0;
Transactions::iterator newEnd = std::remove_if( transactions.begin(),
transactions.end(), [this, &index, bn]( const Transaction& ) -> bool {
Expand All @@ -575,8 +588,8 @@ Json::Value Eth::eth_getBlockByHash( string const& _blockHash, bool _includeTran

#ifdef HISTORIC_STATE
BlockNumber bn = client()->numberFromHash( h );
if ( skippedInvalidTransactionsInBlock( bn, *client() ) ) {
// remove skipped transactions
if ( hasPotentialInvalidTransactionsInBlock( bn, *client() ) ) {
// remove invalid transactions
size_t index = 0;
h256s::iterator newEnd = std::remove_if( transactions.begin(), transactions.end(),
[this, &index, bn]( const h256& ) -> bool {
Expand Down Expand Up @@ -646,7 +659,7 @@ Json::Value Eth::eth_getTransactionByBlockHashAndIndex(

#ifdef HISTORIC_STATE
BlockNumber bn = client()->numberFromHash( bh );
if ( skippedInvalidTransactionsInBlock( bn, *client() ) )
if ( hasPotentialInvalidTransactionsInBlock( bn, *client() ) )
try {
ti = m_gapCache->realIndexFromGapped( bn, ti );
} catch ( const out_of_range& ) {
Expand All @@ -670,7 +683,7 @@ Json::Value Eth::eth_getTransactionByBlockNumberAndIndex(
unsigned int ti = static_cast< unsigned int >( jsToInt( _transactionIndex ) );

#ifdef HISTORIC_STATE
if ( skippedInvalidTransactionsInBlock( bn, *client() ) )
if ( hasPotentialInvalidTransactionsInBlock( bn, *client() ) )
try {
ti = m_gapCache->realIndexFromGapped( bn, ti );
} catch ( const out_of_range& ) {
Expand Down Expand Up @@ -725,7 +738,7 @@ LocalisedTransactionReceipt Eth::eth_getTransactionReceipt( string const& _trans
auto rcp = cli->localisedTransactionReceipt( h );

#ifdef HISTORIC_STATE
if ( skippedInvalidTransactionsInBlock( rcp.blockNumber(), *client() ) ) {
if ( hasPotentialInvalidTransactionsInBlock( rcp.blockNumber(), *client() ) ) {
// skip invalid
if ( rcp.gasUsed() == 0 ) {
m_receiptsCache.put( cacheKey, nullptr );
Expand Down
30 changes: 18 additions & 12 deletions libweb3jsonrpc/Eth.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,29 +63,33 @@ class GappedTransactionIndexCache {
}

size_t realBlockTransactionCount( dev::eth::BlockNumber _bn ) const {
lock_guard< std::mutex > lock( mtx );
ensureCached( _bn );
std::shared_lock< std::shared_mutex > readLock( mtx );
std::unique_lock< std::shared_mutex > writeLock( mtx, std::defer_lock );
ensureCached( _bn, readLock, writeLock );

return real2gappedCache[_bn].size();
}
size_t gappedBlockTransactionCount( dev::eth::BlockNumber _bn ) const {
lock_guard< std::mutex > lock( mtx );
ensureCached( _bn );
std::shared_lock< std::shared_mutex > readLock( mtx );
std::unique_lock< std::shared_mutex > writeLock( mtx, std::defer_lock );
ensureCached( _bn, readLock, writeLock );

return gapped2realCache[_bn].size();
}
// can throw
size_t realIndexFromGapped( dev::eth::BlockNumber _bn, size_t _gappedIndex ) const {
lock_guard< std::mutex > lock( mtx );
ensureCached( _bn );
std::shared_lock< std::shared_mutex > readLock( mtx );
std::unique_lock< std::shared_mutex > writeLock( mtx, std::defer_lock );
ensureCached( _bn, readLock, writeLock );

// throws out_of_range!
return gapped2realCache[_bn].at( _gappedIndex );
}
// can throw
size_t gappedIndexFromReal( dev::eth::BlockNumber _bn, size_t _realIndex ) const {
lock_guard< std::mutex > lock( mtx );
ensureCached( _bn );
std::shared_lock< std::shared_mutex > readLock( mtx );
std::unique_lock< std::shared_mutex > writeLock( mtx, std::defer_lock );
ensureCached( _bn, readLock, writeLock );

// throws out_of_range!
size_t res = real2gappedCache[_bn].at( _realIndex );
Expand All @@ -98,17 +102,19 @@ class GappedTransactionIndexCache {
// can throw
// TODO rename to valid
bool transactionPresent( dev::eth::BlockNumber _bn, size_t _realIndex ) const {
lock_guard< std::mutex > lock( mtx );
ensureCached( _bn );
std::shared_lock< std::shared_mutex > readLock( mtx );
std::unique_lock< std::shared_mutex > writeLock( mtx, std::defer_lock );
ensureCached( _bn, readLock, writeLock );

return real2gappedCache[_bn].at( _realIndex ) != UNDEFINED;
}

private:
void ensureCached( dev::eth::BlockNumber _bn ) const;
void ensureCached( dev::eth::BlockNumber _bn, std::shared_lock< std::shared_mutex >& _readLock,
std::unique_lock< std::shared_mutex >& _writeLock ) const;

private:
mutable std::mutex mtx;
mutable std::shared_mutex mtx;

const dev::eth::Interface& client;
const size_t cacheSize;
Expand Down

0 comments on commit d87ca50

Please sign in to comment.