Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug/skaled 1745 invalid fork in pow gas #1768

Merged
merged 20 commits into from
Jan 10, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ jobs:
./testeth -t BlockQueueSuite -- --express && touch /tmp/BlockQueueSuitePassed
./testeth -t ClientBase -- --express && touch /tmp/ClientBasePassed
./testeth -t EstimateGas -- --express && touch /tmp/EstimateGasPassed
./testeth -t IMABLSPublicKey -- --express && touch /tmp/IMABLSPublicKeyPassed
./testeth -t getHistoricNodesData -- --express && touch /tmp/getHistoricNodesDataPassed
./testeth -t ExtVmSuite -- --express && touch /tmp/ExtVmSuitePassed
./testeth -t GasPricer -- --express && touch /tmp/GasPricerPassed
./testeth -t BasicTests -- --express && touch /tmp/BasicTestsPassed
Expand Down Expand Up @@ -239,7 +239,7 @@ jobs:
ls /tmp/BlockQueueSuitePassed || ./testeth -t BlockQueueSuite -- --express --verbosity 4
ls /tmp/ClientBasePassed || ./testeth -t ClientBase -- --express --verbosity 4
ls /tmp/EstimateGasPassed || ./testeth -t EstimateGas -- --express --verbosity 4
ls /tmp/IMABLSPublicKeyPassed || ./testeth -t IMABLSPublicKey -- --express --verbosity 4
ls /tmp/getHistoricNodesDataPassed || ./testeth -t getHistoricNodesData -- --express --verbosity 4
ls /tmp/ExtVmSuitePassed || ./testeth -t ExtVmSuite -- --express --verbosity 4
ls /tmp/GasPricerPassed || ./testeth -t GasPricer -- --express --verbosity 4
ls /tmp/BasicTestsPassed || ./testeth -t BasicTests -- --express --verbosity 4
Expand Down
1 change: 1 addition & 0 deletions libethcore/ChainOperationParams.h
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ struct SChain {
time_t precompiledConfigPatchTimestamp = 0;
time_t pushZeroPatchTimestamp = 0;
time_t skipInvalidTransactionsPatchTimestamp = 0;
time_t correctForkInPowPatchTimestamp = 0;

SChain() {
name = "TestChain";
Expand Down
5 changes: 2 additions & 3 deletions libethereum/Block.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ pair< TransactionReceipts, bool > Block::sync(
// caller if we hit the limit

for ( Transaction& transaction : transactions ) {
transaction.checkOutExternalGas( _bc.chainParams().externalGasDifficulty );
transaction.checkOutExternalGas( _bc.chainParams(), _bc.number() );
}

assert( _bc.currentHash() == m_currentBlock.parentHash() );
Expand Down Expand Up @@ -631,8 +631,7 @@ u256 Block::enact( VerifiedBlockRef const& _block, BlockChain const& _bc ) {
// << " (state #"
// << state().getNonce( tr.from() ) << ") value = " << tr.value() <<
// endl;
const_cast< Transaction& >( tr ).checkOutExternalGas(
_bc.chainParams().externalGasDifficulty );
const_cast< Transaction& >( tr ).checkOutExternalGas( _bc.chainParams(), _bc.number() );
execute( _bc.lastBlockHashes(), tr );
// cerr << "Now: "
// << "State #" << state().getNonce( tr.from() ) << endl;
Expand Down
5 changes: 5 additions & 0 deletions libethereum/ChainParams.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,11 @@ ChainParams ChainParams::loadConfig(
sChainObj.at( "skipInvalidTransactionsPatchTimestamp" ).get_int64() :
0;

s.correctForkInPowPatchTimestamp =
sChainObj.count( "correctForkInPowPatchTimestamp" ) ?
sChainObj.at( "correctForkInPowPatchTimestamp" ).get_int64() :
0;

if ( sChainObj.count( "nodeGroups" ) ) {
std::vector< NodeGroup > nodeGroups;
for ( const auto& nodeGroupConf : sChainObj["nodeGroups"].get_obj() ) {
Expand Down
19 changes: 15 additions & 4 deletions libethereum/Client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@

#include <libskale/ContractStorageLimitPatch.h>
#include <libskale/ContractStorageZeroValuePatch.h>
#include <libskale/CorrectForkInPowPatch.h>
#include <libskale/POWCheckPatch.h>
#include <libskale/PrecompiledConfigPatch.h>
#include <libskale/PushZeroPatch.h>
Expand Down Expand Up @@ -169,6 +170,7 @@ Client::Client( ChainParams const& _params, int _networkID,
SkipInvalidTransactionsPatch::setTimestamp(
this->chainParams().sChain.skipInvalidTransactionsPatchTimestamp );
PrecompiledConfigPatch::setTimestamp( chainParams().sChain.precompiledConfigPatchTimestamp );
CorrectForkInPowPatch::setTimestamp( chainParams().sChain.correctForkInPowPatchTimestamp );
}


Expand Down Expand Up @@ -600,6 +602,10 @@ size_t Client::importTransactionsAsBlock(
sealUnconditionally( false );
importWorkingBlock();

// this needs to be updated as soon as possible, as it's used in new transactions validation
CorrectForkInPowPatch::lastBlockTimestamp = blockChain().info().timestamp();
CorrectForkInPowPatch::lastBlockNumber = blockChain().number();

if ( !UnsafeRegion::isActive() ) {
LOG( m_loggerDetail ) << "Total unsafe time so far = "
<< std::chrono::duration_cast< std::chrono::seconds >(
Expand Down Expand Up @@ -668,6 +674,9 @@ size_t Client::syncTransactions(
PushZeroPatch::lastBlockTimestamp = blockChain().info().timestamp();
SkipInvalidTransactionsPatch::lastBlockTimestamp = blockChain().info().timestamp();
PrecompiledConfigPatch::lastBlockTimestamp = blockChain().info().timestamp();
CorrectForkInPowPatch::lastBlockTimestamp = blockChain().info().timestamp();
CorrectForkInPowPatch::lastBlockNumber = blockChain().number();


DEV_WRITE_GUARDED( x_working ) {
assert( !m_working.isSealed() );
Expand Down Expand Up @@ -1188,15 +1197,17 @@ h256 Client::importTransaction( Transaction const& _t ) {
// the latest block in the client's blockchain. This can throw but
// we'll catch the exception at the RPC level.

const_cast< Transaction& >( _t ).checkOutExternalGas( chainParams().externalGasDifficulty );

// throws in case of error
State state;
u256 gasBidPrice;

DEV_GUARDED( m_blockImportMutex ) {
state = this->state().createStateReadOnlyCopy();
gasBidPrice = this->gasBidPrice();

// We need to check external gas under mutex to be sure about current block bumber
// correctness
const_cast< Transaction& >( _t ).checkOutExternalGas( chainParams(), number() );
}

Executive::verifyTransaction( _t,
Expand Down Expand Up @@ -1253,7 +1264,7 @@ ExecutionResult Client::call( Address const& _from, u256 _value, Address _dest,
Transaction t( _value, gasPrice, gas, _dest, _data, nonce );
t.forceSender( _from );
t.forceChainId( chainParams().chainID );
t.checkOutExternalGas( ~u256( 0 ) );
t.ignoreExternalGas();
if ( _ff == FudgeFactor::Lenient ) {
historicBlock.mutableState().mutableHistoricState().addBalance(
_from, ( u256 )( t.gas() * t.gasPrice() + t.value() ) );
Expand All @@ -1278,7 +1289,7 @@ ExecutionResult Client::call( Address const& _from, u256 _value, Address _dest,
Transaction t( _value, gasPrice, gas, _dest, _data, nonce );
t.forceSender( _from );
t.forceChainId( chainParams().chainID );
t.checkOutExternalGas( ~u256( 0 ) );
t.ignoreExternalGas();
if ( _ff == FudgeFactor::Lenient )
temp.mutableState().addBalance( _from, ( u256 )( t.gas() * t.gasPrice() + t.value() ) );
ret = temp.execute( bc().lastBlockHashes(), t, skale::Permanence::Reverted );
Expand Down
3 changes: 2 additions & 1 deletion libethereum/ClientBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ std::pair< bool, ExecutionResult > ClientBase::estimateGasStep( int64_t _gas, Bl
t = Transaction( _value, _gasPrice, _gas, _data, nonce );
t.forceSender( _from );
t.forceChainId( chainId() );
t.checkOutExternalGas( ~u256( 0 ) );
t.ignoreExternalGas();
EnvInfo const env( _latestBlock.info(), bc().lastBlockHashes(), 0, _gas );
// Make a copy of state!! It will be deleted after step!
State tempState = _latestBlock.mutableState();
Expand Down Expand Up @@ -117,6 +117,7 @@ std::pair< u256, ExecutionResult > ClientBase::estimateGas( Address const& _from
upperBound = c_maxGasEstimate;
int64_t lowerBound = Transaction::baseGasRequired( !_dest, &_data,
bc().sealEngine()->chainParams().scheduleForBlockNumber( bc().number() ) );

Block bk = latestBlock();
if ( upperBound > bk.info().gasLimit() ) {
upperBound = bk.info().gasLimit().convert_to< int64_t >();
Expand Down
2 changes: 1 addition & 1 deletion libethereum/SkaleHost.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -665,7 +665,7 @@ void SkaleHost::createBlock( const ConsensusExtFace::transactions_vector& _appro
// ).detach();
} else {
Transaction t( data, CheckTransaction::Everything, true );
t.checkOutExternalGas( m_client.chainParams().externalGasDifficulty );
t.checkOutExternalGas( m_client.chainParams(), m_client.number() );
out_txns.push_back( t );
LOG( m_debugLogger ) << "Will import consensus-born txn!";
m_debugTracer.tracepoint( "import_consensus_born" );
Expand Down
24 changes: 19 additions & 5 deletions libethereum/Transaction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include <libdevcrypto/Common.h>
#include <libethcore/Exceptions.h>
#include <libevm/VMFace.h>
#include <libskale/CorrectForkInPowPatch.h>

using namespace std;
using namespace dev;
Expand Down Expand Up @@ -180,19 +181,32 @@
}
}

void Transaction::checkOutExternalGas( u256 const& _difficulty ) {
assert( _difficulty > 0 );
void Transaction::checkOutExternalGas( const ChainParams& _cp, uint64_t _bn ) {
u256 const& difficulty = _cp.externalGasDifficulty;
assert( difficulty > 0 );
if ( !m_externalGasIsChecked && !isInvalid() ) {
h256 hash = dev::sha3( sender().ref() ) ^ dev::sha3( nonce() ) ^ dev::sha3( gasPrice() );
if ( !hash ) {
hash = h256( 1 );
}
u256 externalGas = ~u256( 0 ) / u256( hash ) / _difficulty;
u256 externalGas = ~u256( 0 ) / u256( hash ) / difficulty;
if ( externalGas > 0 )
ctrace << "Mined gas: " << externalGas << endl;
if ( externalGas >= baseGasRequired( ConstantinopleSchedule ) ) {
m_externalGas = externalGas;

EVMSchedule scheduleForUse = ConstantinopleSchedule;
if ( CorrectForkInPowPatch::isEnabled() )
scheduleForUse = _cp.scheduleForBlockNumber( _bn );

// !! never call checkOutExternalGas with non-last block!
dimalit marked this conversation as resolved.
Show resolved Hide resolved
if ( _bn != CorrectForkInPowPatch::getLastBlockNumber() ) {
ctrace << _bn << " != " << CorrectForkInPowPatch::getLastBlockNumber();
DmytroNazarenko marked this conversation as resolved.
Show resolved Hide resolved
BOOST_THROW_EXCEPTION( std::runtime_error(

Check warning on line 203 in libethereum/Transaction.cpp

View check run for this annotation

Codecov / codecov/patch

libethereum/Transaction.cpp#L202-L203

Added lines #L202 - L203 were not covered by tests
"Internal error: checkOutExternalGas() has invalid block number" ) );
}

if ( externalGas >= baseGasRequired( scheduleForUse ) )
m_externalGas = externalGas;

m_externalGasIsChecked = true;
}
}
Expand Down
9 changes: 8 additions & 1 deletion libethereum/Transaction.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
#include <libethcore/Common.h>
#include <libethcore/TransactionBase.h>

#include "ChainParams.h"

namespace dev {
namespace eth {

Expand Down Expand Up @@ -120,7 +122,12 @@ class Transaction : public TransactionBase {

u256 gasPrice() const;

void checkOutExternalGas( u256 const& _difficulty );
void checkOutExternalGas( const ChainParams& _cp, uint64_t _bn );

void ignoreExternalGas() {
m_externalGasIsChecked = true;
m_externalGas = 0;
}

private:
bool m_externalGasIsChecked = false;
Expand Down
2 changes: 2 additions & 0 deletions libethereum/ValidationSchemes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,8 @@ void validateConfigJson( js::mObject const& _obj ) {
{ "skipInvalidTransactionsPatchTimestamp",
{ { js::int_type }, JsonFieldPresence::Optional } },
{ "precompiledConfigPatchTimestamp",
{ { js::int_type }, JsonFieldPresence::Optional } },
{ "correctForkInPowPatchTimestamp",
{ { js::int_type }, JsonFieldPresence::Optional } } } );

js::mArray const& nodes = sChain.at( "nodes" ).get_array();
Expand Down
2 changes: 2 additions & 0 deletions libskale/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ set(sources
PrecompiledConfigPatch.cpp
PushZeroPatch.cpp
SkipInvalidTransactionsPatch.cpp
CorrectForkInPowPatch.cpp
)

set(headers
Expand All @@ -44,6 +45,7 @@ set(headers
PrecompiledConfigPatch.h
OverlayFS.h
SkipInvalidTransactionsPatch.h
CorrectForkInPowPatch.h
)

add_library(skale ${sources} ${headers})
Expand Down
12 changes: 12 additions & 0 deletions libskale/CorrectForkInPowPatch.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
#include "CorrectForkInPowPatch.h"

time_t CorrectForkInPowPatch::activationTimestamp;
time_t CorrectForkInPowPatch::lastBlockTimestamp;
unsigned CorrectForkInPowPatch::lastBlockNumber;

bool CorrectForkInPowPatch::isEnabled() {
if ( activationTimestamp == 0 ) {
return false;
}
return activationTimestamp <= lastBlockTimestamp;
}
41 changes: 41 additions & 0 deletions libskale/CorrectForkInPowPatch.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
#ifndef CORRECTFORKINPOWPATCH_H
#define CORRECTFORKINPOWPATCH_H

#include <libethereum/SchainPatch.h>

#include <time.h>

namespace dev {
namespace eth {
class Client;
}
namespace test {
class TestBlockChain;
class TestOutputHelperFixture;
} // namespace test
} // namespace dev

/*
* Context: use current, and not Constantinople, fork in Transaction::checkOutExternalGas()
*/
class CorrectForkInPowPatch : public SchainPatch {
public:
static bool isEnabled();

static void setTimestamp( time_t _timeStamp ) {
printInfo( __FILE__, _timeStamp );
activationTimestamp = _timeStamp;
}

static unsigned getLastBlockNumber() { return lastBlockNumber; }

private:
friend class dev::eth::Client;
friend class dev::test::TestBlockChain;
friend class dev::test::TestOutputHelperFixture;
static time_t activationTimestamp;
static time_t lastBlockTimestamp;
static unsigned lastBlockNumber;
};

#endif // CORRECTFORKINPOWPATCH_H
2 changes: 1 addition & 1 deletion libweb3jsonrpc/Net.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class Net : public NetFace {
virtual bool net_listening() override;

private:
const dev::eth::ChainParams& m_chainParams;
dev::eth::ChainParams m_chainParams;
};

} // namespace rpc
Expand Down
10 changes: 10 additions & 0 deletions test/tools/libtesteth/BlockChainHelper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
* that manage block/transaction import and test mining
*/

#include <libskale/CorrectForkInPowPatch.h>

#include <libdevcore/TransientDirectory.h>
#include <libethashseal/GenesisInfo.h>
#include <libethereum/Block.h>
Expand Down Expand Up @@ -473,6 +475,10 @@ void TestBlockChain::reset( TestBlock const& _genesisBlock ) {
}

bool TestBlockChain::addBlock( TestBlock const& _block ) {

CorrectForkInPowPatch::lastBlockTimestamp = m_blockChain->info().timestamp();
CorrectForkInPowPatch::lastBlockNumber = m_blockChain->number();

while ( true ) {
try {
_block.verify( *this ); // check that block header match TestBlock contents
Expand All @@ -494,6 +500,10 @@ bool TestBlockChain::addBlock( TestBlock const& _block ) {

State st( block.state() );
m_lastBlock.setState( st );

CorrectForkInPowPatch::lastBlockTimestamp = m_blockChain->info().timestamp();
CorrectForkInPowPatch::lastBlockNumber = m_blockChain->number();

return true;
}

Expand Down
2 changes: 1 addition & 1 deletion test/tools/libtesteth/ImportTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ bytes ImportTest::executeTest( bool _isFilling ) {
continue;

for ( auto& tr : m_transactions ) {
tr.transaction.checkOutExternalGas( 100 );
// tr.transaction.checkOutExternalGas( 100 );
dimalit marked this conversation as resolved.
Show resolved Hide resolved
Options const& opt = Options::get();
if ( opt.trDataIndex != -1 && opt.trDataIndex != tr.dataInd )
continue;
Expand Down
3 changes: 3 additions & 0 deletions test/tools/libtesteth/TestOutputHelper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
* Fixture class for boost output when running testeth
*/

#include <libskale/CorrectForkInPowPatch.h>
#include <libethashseal/Ethash.h>
#include <libethcore/BasicAuthority.h>
#include <test/tools/libtesteth/Options.h>
Expand Down Expand Up @@ -102,6 +103,8 @@ void TestOutputHelper::printTestExecStats() {
}
TestOutputHelperFixture::TestOutputHelperFixture() {
TestOutputHelper::get().initTest();
CorrectForkInPowPatch::lastBlockTimestamp = 1;
CorrectForkInPowPatch::lastBlockNumber = 0;
}

TestOutputHelperFixture::~TestOutputHelperFixture() {
Expand Down
Loading
Loading