From db9d2484321fa588df3dd777deeeec74d6c80133 Mon Sep 17 00:00:00 2001 From: Dima Litvinov Date: Wed, 18 Oct 2023 17:00:23 +0100 Subject: [PATCH 1/4] IS-869 Fixed filtering condition in Eth.cpp --- libweb3jsonrpc/Eth.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/libweb3jsonrpc/Eth.cpp b/libweb3jsonrpc/Eth.cpp index b2330bc51..e9dd9b4bd 100644 --- a/libweb3jsonrpc/Eth.cpp +++ b/libweb3jsonrpc/Eth.cpp @@ -287,7 +287,7 @@ Json::Value Eth::eth_getBlockTransactionCountByHash( string const& _blockHash ) #ifdef HISTORIC_STATE BlockNumber bn = client()->numberFromHash( blockHash ); - if ( !skippedInvalidTransactionsInBlock( bn, *client() ) ) + if ( skippedInvalidTransactionsInBlock( bn, *client() ) ) #endif return toJS( client()->transactionCount( blockHash ) ); #ifdef HISTORIC_STATE @@ -306,7 +306,7 @@ Json::Value Eth::eth_getBlockTransactionCountByNumber( string const& _blockNumbe #ifdef HISTORIC_STATE BlockNumber bn = jsToBlockNumber( _blockNumber ); - if ( !skippedInvalidTransactionsInBlock( bn, *client() ) ) + if ( skippedInvalidTransactionsInBlock( bn, *client() ) ) #endif return toJS( client()->transactionCount( jsToBlockNumber( _blockNumber ) ) ); #ifdef HISTORIC_STATE @@ -558,7 +558,7 @@ Json::Value Eth::eth_getBlockByHash( string const& _blockHash, bool _includeTran #ifdef HISTORIC_STATE BlockNumber bn = client()->numberFromHash( h ); - if ( skippedInvalidTransactionsInBlock( bn, *client() ) ) { + if ( !skippedInvalidTransactionsInBlock( bn, *client() ) ) { // remove skipped transactions size_t index = 0; Transactions::iterator newEnd = std::remove_if( transactions.begin(), @@ -575,7 +575,7 @@ Json::Value Eth::eth_getBlockByHash( string const& _blockHash, bool _includeTran #ifdef HISTORIC_STATE BlockNumber bn = client()->numberFromHash( h ); - if ( skippedInvalidTransactionsInBlock( bn, *client() ) ) { + if ( !skippedInvalidTransactionsInBlock( bn, *client() ) ) { // remove skipped transactions size_t index = 0; h256s::iterator newEnd = std::remove_if( transactions.begin(), transactions.end(), @@ -646,7 +646,7 @@ Json::Value Eth::eth_getTransactionByBlockHashAndIndex( #ifdef HISTORIC_STATE BlockNumber bn = client()->numberFromHash( bh ); - if ( skippedInvalidTransactionsInBlock( bn, *client() ) ) + if ( !skippedInvalidTransactionsInBlock( bn, *client() ) ) try { ti = m_gapCache->realIndexFromGapped( bn, ti ); } catch ( const out_of_range& ) { @@ -670,7 +670,7 @@ Json::Value Eth::eth_getTransactionByBlockNumberAndIndex( unsigned int ti = static_cast< unsigned int >( jsToInt( _transactionIndex ) ); #ifdef HISTORIC_STATE - if ( skippedInvalidTransactionsInBlock( bn, *client() ) ) + if ( !skippedInvalidTransactionsInBlock( bn, *client() ) ) try { ti = m_gapCache->realIndexFromGapped( bn, ti ); } catch ( const out_of_range& ) { @@ -725,7 +725,7 @@ LocalisedTransactionReceipt Eth::eth_getTransactionReceipt( string const& _trans auto rcp = cli->localisedTransactionReceipt( h ); #ifdef HISTORIC_STATE - if ( skippedInvalidTransactionsInBlock( rcp.blockNumber(), *client() ) ) { + if ( !skippedInvalidTransactionsInBlock( rcp.blockNumber(), *client() ) ) { // skip invalid if ( rcp.gasUsed() == 0 ) { m_receiptsCache.put( cacheKey, nullptr ); From d33f1942cd2f8a8f9fef961e7a131f41a0ffa4be Mon Sep 17 00:00:00 2001 From: Dima Litvinov Date: Thu, 19 Oct 2023 12:37:27 +0100 Subject: [PATCH 2/4] IS-869 Rename skippedInvalidTransactionsInBlock --- libweb3jsonrpc/Eth.cpp | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/libweb3jsonrpc/Eth.cpp b/libweb3jsonrpc/Eth.cpp index e9dd9b4bd..76daf10f3 100644 --- a/libweb3jsonrpc/Eth.cpp +++ b/libweb3jsonrpc/Eth.cpp @@ -90,19 +90,21 @@ 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 ( _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 @@ -287,7 +289,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 @@ -306,7 +308,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 @@ -558,8 +560,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 { @@ -575,8 +577,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 { @@ -646,7 +648,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& ) { @@ -670,7 +672,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& ) { @@ -725,7 +727,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 ); From b3b81332cec00655828cd1f9ea59686248518c69 Mon Sep 17 00:00:00 2001 From: Dima Litvinov Date: Thu, 19 Oct 2023 17:26:37 +0100 Subject: [PATCH 3/4] IS-869 Read-write lock --- libweb3jsonrpc/Eth.cpp | 10 +++++++++- libweb3jsonrpc/Eth.h | 30 ++++++++++++++++++------------ 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/libweb3jsonrpc/Eth.cpp b/libweb3jsonrpc/Eth.cpp index 76daf10f3..bbe52b45d 100644 --- a/libweb3jsonrpc/Eth.cpp +++ b/libweb3jsonrpc/Eth.cpp @@ -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() ); diff --git a/libweb3jsonrpc/Eth.h b/libweb3jsonrpc/Eth.h index e9504732f..a3ce404da 100644 --- a/libweb3jsonrpc/Eth.h +++ b/libweb3jsonrpc/Eth.h @@ -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 ); @@ -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; From a7c74c74e08cd2888152aa200009304778bc99e2 Mon Sep 17 00:00:00 2001 From: Dima Litvinov Date: Thu, 19 Oct 2023 17:41:45 +0100 Subject: [PATCH 4/4] IS-869 Invalid handling of switched off patch --- libweb3jsonrpc/Eth.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libweb3jsonrpc/Eth.cpp b/libweb3jsonrpc/Eth.cpp index bbe52b45d..4b68b667b 100644 --- a/libweb3jsonrpc/Eth.cpp +++ b/libweb3jsonrpc/Eth.cpp @@ -104,6 +104,9 @@ bool hasPotentialInvalidTransactionsInBlock( BlockNumber _bn, const Interface& _ if ( _bn == 0 ) return false; + if ( SkipInvalidTransactionsPatch::getActivationTimestamp() == 0 ) + return true; + if ( _bn == PendingBlock ) return !SkipInvalidTransactionsPatch::isEnabled();