From d6615d3f158d604b51021ac7268919bb24cb4d46 Mon Sep 17 00:00:00 2001 From: jimmyshi <417711026@qq.com> Date: Mon, 29 Jan 2024 14:14:29 +0800 Subject: [PATCH] (execute): deploy with value& transfer to precompiled bug & support "payable" & delegatecall (#4202) --- .../executive/BillingTransactionExecutive.cpp | 2 +- .../src/executive/TransactionExecutive.cpp | 38 ++++++++++++++++- .../src/executive/TransactionExecutive.h | 41 +++---------------- .../src/precompiled/common/Utilities.cpp | 2 +- .../extension/BalancePrecompiled.cpp | 24 +++++++---- .../extension/BalancePrecompiled.h | 4 ++ bcos-executor/src/vm/EVMHostInterface.cpp | 9 ++-- bcos-executor/src/vm/HostContext.cpp | 25 ++++++++++- bcos-executor/src/vm/HostContext.h | 5 +++ bcos-scheduler/src/SchedulerImpl.cpp | 8 ++-- bcos-scheduler/src/SchedulerImpl.h | 2 +- bcos-scheduler/test/testSchedulerImpl.cpp | 2 - 12 files changed, 100 insertions(+), 62 deletions(-) diff --git a/bcos-executor/src/executive/BillingTransactionExecutive.cpp b/bcos-executor/src/executive/BillingTransactionExecutive.cpp index d327204354..26ba7408b7 100644 --- a/bcos-executor/src/executive/BillingTransactionExecutive.cpp +++ b/bcos-executor/src/executive/BillingTransactionExecutive.cpp @@ -9,7 +9,7 @@ CallParameters::UniquePtr BillingTransactionExecutive::start(CallParameters::Uni { int64_t originGas = input->gas; uint64_t currentSeq = input->seq; - std::string currentSenderAddr = input->senderAddress; + std::string currentSenderAddr = input->origin; bool staticCall = input->staticCall; u256 gasPrice = input->gasPrice; auto message = TransactionExecutive::execute(std::move(input)); diff --git a/bcos-executor/src/executive/TransactionExecutive.cpp b/bcos-executor/src/executive/TransactionExecutive.cpp index 0ba3d201a5..bab1c444fa 100644 --- a/bcos-executor/src/executive/TransactionExecutive.cpp +++ b/bcos-executor/src/executive/TransactionExecutive.cpp @@ -347,7 +347,7 @@ bool TransactionExecutive::transferBalance(std::string_view origin, std::string_ // sender = sender - value auto codec = CodecWrapper(m_blockContext.hashHandler(), m_blockContext.isWasm()); auto params = codec.encodeWithSig("subAccountBalance(uint256)", value); - auto formTableName = bcos::getContractTableName(executor::USER_APPS_PREFIX, sender); + auto formTableName = getContractTableName(sender, m_blockContext.isWasm()); std::vector fromTableNameVector = {formTableName}; auto inputParams = codec.encode(fromTableNameVector, params); auto subParams = codec.encode(std::string(ACCOUNT_ADDRESS), inputParams); @@ -368,7 +368,7 @@ bool TransactionExecutive::transferBalance(std::string_view origin, std::string_ // to add balance // receiver = receiver + value auto params1 = codec.encodeWithSig("addAccountBalance(uint256)", value); - auto toTableName = bcos::getContractTableName(executor::USER_APPS_PREFIX, receiver); + auto toTableName = getContractTableName(receiver, m_blockContext.isWasm()); std::vector toTableNameVector = {toTableName}; auto inputParams1 = codec.encode(toTableNameVector, params1); auto addParams = codec.encode(std::string(ACCOUNT_ADDRESS), inputParams1); @@ -1694,3 +1694,37 @@ uint8_t TransactionExecutive::checkAccountAvailable(const CallParameters::Unique return precompiled::AccountPrecompiled::getAccountStatus( callParameters->origin, shared_from_this()); } + +std::string TransactionExecutive::getContractTableName( + const std::string_view& _address, bool isWasm, bool isCreate) +{ + auto version = m_blockContext.blockVersion(); + + if (m_blockContext.isAuthCheck() || + protocol::versionCompareTo(version, protocol::BlockVersion::V3_3_VERSION) >= 0) + { + if (_address.starts_with(precompiled::SYS_ADDRESS_PREFIX)) + { + return std::string(USER_SYS_PREFIX).append(_address); + } + } + + std::string_view formatAddress = _address; + if (isWasm) + { + // NOTE: if version < 3.2, then it will allow deploying contracts under /tables. It's a + // bug, but it should maintain data compatibility. + // NOTE2: if it's internalCreate it should allow creating table under /tables + if (protocol::versionCompareTo(version, protocol::BlockVersion::V3_2_VERSION) < 0 || + !isCreate) + { + if (_address.starts_with(USER_TABLE_PREFIX)) + { + return std::string(formatAddress); + } + } + formatAddress = formatAddress.starts_with('/') ? formatAddress.substr(1) : formatAddress; + } + + return std::string(USER_APPS_PREFIX).append(formatAddress); +} diff --git a/bcos-executor/src/executive/TransactionExecutive.h b/bcos-executor/src/executive/TransactionExecutive.h index f4624df9ed..1c9e450773 100644 --- a/bcos-executor/src/executive/TransactionExecutive.h +++ b/bcos-executor/src/executive/TransactionExecutive.h @@ -156,9 +156,13 @@ class TransactionExecutive : public std::enable_shared_from_this getCodeByContractTableName( const std::string_view& contractTableName, bool needTryFromContractTable = true); -protected: bool transferBalance(std::string_view origin, std::string_view sender, std::string_view receiver, const u256& value, int64_t gas); + + std::string getContractTableName( + const std::string_view& _address, bool isWasm = false, bool isCreate = false); + +protected: std::tuple, CallParameters::UniquePtr> call( CallParameters::UniquePtr callParameters); CallParameters::UniquePtr callPrecompiled(CallParameters::UniquePtr callParameters); @@ -194,41 +198,6 @@ class TransactionExecutive : public std::enable_shared_from_this= 0) - { - if (_address.starts_with(precompiled::SYS_ADDRESS_PREFIX)) - { - return std::string(USER_SYS_PREFIX).append(_address); - } - } - - std::string_view formatAddress = _address; - if (isWasm) - { - // NOTE: if version < 3.2, then it will allow deploying contracts under /tables. It's a - // bug, but it should maintain data compatibility. - // NOTE2: if it's internalCreate it should allow creating table under /tables - if (protocol::versionCompareTo(version, protocol::BlockVersion::V3_2_VERSION) < 0 || - !isCreate) - { - if (_address.starts_with(USER_TABLE_PREFIX)) - { - return std::string(formatAddress); - } - } - formatAddress = - formatAddress.starts_with('/') ? formatAddress.substr(1) : formatAddress; - } - - return std::string(USER_APPS_PREFIX).append(formatAddress); - } - bool checkExecAuth(const CallParameters::UniquePtr& callParameters); int32_t checkContractAvailable(const CallParameters::UniquePtr& callParameters); uint8_t checkAccountAvailable(const CallParameters::UniquePtr& callParameters); diff --git a/bcos-executor/src/precompiled/common/Utilities.cpp b/bcos-executor/src/precompiled/common/Utilities.cpp index 69518854ed..f6958d687a 100644 --- a/bcos-executor/src/precompiled/common/Utilities.cpp +++ b/bcos-executor/src/precompiled/common/Utilities.cpp @@ -414,7 +414,7 @@ std::vector
precompiled::getGovernorList( const PrecompiledExecResult::Ptr& _callParameters, const CodecWrapper& codec) { const auto& blockContext = _executive->blockContext(); - const auto* sender = blockContext.isWasm() ? ACCOUNT_MANAGER_NAME : ACCOUNT_MGR_ADDRESS; + const auto& sender = _executive->contractAddress(); auto getCommittee = codec.encodeWithSig("_committee()"); auto getCommitteeResponse = externalRequest(_executive, ref(getCommittee), _callParameters->m_origin, sender, AUTH_COMMITTEE_ADDRESS, _callParameters->m_staticCall, diff --git a/bcos-executor/src/precompiled/extension/BalancePrecompiled.cpp b/bcos-executor/src/precompiled/extension/BalancePrecompiled.cpp index a586a4445c..ac741d41c7 100644 --- a/bcos-executor/src/precompiled/extension/BalancePrecompiled.cpp +++ b/bcos-executor/src/precompiled/extension/BalancePrecompiled.cpp @@ -102,6 +102,14 @@ std::shared_ptr BalancePrecompiled::call( return _callParameters; } + +std::string BalancePrecompiled::getContractTableName( + const std::shared_ptr& _executive, + const std::string_view& _address) +{ + return _executive->getContractTableName(_address, _executive->isWasm(), false); +} + void BalancePrecompiled::createAccount( const std::shared_ptr& _executive, const PrecompiledExecResult::Ptr& _callParameters, const bcos::CodecWrapper& codec, @@ -141,7 +149,7 @@ void BalancePrecompiled::getBalance( // get balance from account table auto params = codec.encodeWithSig("getAccountBalance()"); - auto tableName = getContractTableName(executor::USER_APPS_PREFIX, accountStr); + auto tableName = getContractTableName(_executive, accountStr); std::vector tableNameVector = {tableName}; auto params2 = codec.encode(tableNameVector, params); auto input = codec.encode(std::string(ACCOUNT_ADDRESS), params2); @@ -198,7 +206,7 @@ void BalancePrecompiled::addBalance( << LOG_KV("callerEntry", entry->get()); // check the account whether exist, if not exist, create the account - auto accountTableName = getContractTableName(executor::USER_APPS_PREFIX, accountStr); + auto accountTableName = getContractTableName(_executive, accountStr); auto table1 = _executive->storage().openTable(accountTableName); if (!table1) { @@ -259,7 +267,7 @@ void BalancePrecompiled::subBalance( } // check the account whether exist, if not exist, create the account - auto accountTableName = getContractTableName(executor::USER_APPS_PREFIX, accountStr); + auto accountTableName = getContractTableName(_executive, accountStr); auto table1 = _executive->storage().openTable(accountTableName); if (!table1) { @@ -275,7 +283,7 @@ void BalancePrecompiled::subBalance( // AccountPrecompiledAddress + subAccountBalance(value), internal call auto balanceParams = codec.encodeWithSig("subAccountBalance(uint256)", value); - auto tableName = getContractTableName(executor::USER_APPS_PREFIX, accountStr); + auto tableName = getContractTableName(_executive, accountStr); std::vector tableNameVector = {tableName}; auto inputParams = codec.encode(tableNameVector, balanceParams); @@ -329,12 +337,12 @@ void BalancePrecompiled::transfer(const std::shared_ptr fromTableNameVector = {formTableName}; auto inputParams = codec.encode(fromTableNameVector, params); // check the from account whether exist, if not exist, create the account - auto fromAccountTableName = getContractTableName(executor::USER_APPS_PREFIX, fromStr); + auto fromAccountTableName = getContractTableName(_executive, fromStr); auto fromTable = _executive->storage().openTable(fromAccountTableName); if (!fromTable) { @@ -356,13 +364,13 @@ void BalancePrecompiled::transfer(const std::shared_ptrstatus == int32_t(CODE_SUCCESS)) { auto params1 = codec.encodeWithSig("addAccountBalance(uint256)", value); - auto toTableName = getContractTableName(executor::USER_APPS_PREFIX, toStr); + auto toTableName = getContractTableName(_executive, toStr); std::vector toTableNameVector = {toTableName}; auto inputParams1 = codec.encode(toTableNameVector, params1); auto addParams = codec.encode(std::string(ACCOUNT_ADDRESS), inputParams1); // check the to account whether exist, if not exist, create the account - auto toAccountTableName = getContractTableName(executor::USER_APPS_PREFIX, toStr); + auto toAccountTableName = getContractTableName(_executive, toStr); auto toTable = _executive->storage().openTable(toAccountTableName); if (!toTable) { diff --git a/bcos-executor/src/precompiled/extension/BalancePrecompiled.h b/bcos-executor/src/precompiled/extension/BalancePrecompiled.h index 14bc52f27f..416784741e 100644 --- a/bcos-executor/src/precompiled/extension/BalancePrecompiled.h +++ b/bcos-executor/src/precompiled/extension/BalancePrecompiled.h @@ -62,6 +62,10 @@ class BalancePrecompiled : public bcos::precompiled::Precompiled PrecompiledExecResult::Ptr const& _callParameters, const CodecWrapper& codec, std::string_view accountHex); + std::string getContractTableName( + const std::shared_ptr& _executive, + const std::string_view& _address); + private: mutable std::map m_fakeBalancePrecompiled; }; diff --git a/bcos-executor/src/vm/EVMHostInterface.cpp b/bcos-executor/src/vm/EVMHostInterface.cpp index 3a3f5d6562..8b2cdaa21c 100644 --- a/bcos-executor/src/vm/EVMHostInterface.cpp +++ b/bcos-executor/src/vm/EVMHostInterface.cpp @@ -102,8 +102,7 @@ evmc_bytes32 getBalance(evmc_host_context* _context, const evmc_address* _addr) const auto& codec = bcos::CodecWrapper(blockContext.hashHandler(), blockContext.isWasm()); // get balance from account table auto params = codec.encodeWithSig("getAccountBalance()"); - auto tableName = - getContractTableName(executor::USER_APPS_PREFIX, address2HexString(_msg.recipient)); + auto tableName = hostContext.getContractTableName(address2HexString(_msg.recipient)); std::vector tableNameVector = {tableName}; auto input = codec.encode(tableNameVector, params); bytes_view getBalance = bytes_view(input.data(), input.size()); @@ -200,8 +199,7 @@ bool selfdestruct(evmc_host_context* _context, const evmc_address* _addr, // get _addr balance const auto& codec = bcos::CodecWrapper(blockContext.hashHandler(), blockContext.isWasm()); bytes params = codec.encodeWithSig("getAccountBalance()"); - auto tableName = - getContractTableName(executor::USER_APPS_PREFIX, address2HexString(_msg.recipient)); + auto tableName = hostContext.getContractTableName(address2HexString(_msg.recipient)); std::vector tableNameVector = {tableName}; auto getBalanceIn = codec.encode(tableNameVector, params); @@ -239,8 +237,7 @@ bool selfdestruct(evmc_host_context* _context, const evmc_address* _addr, // _beneficiary += balance _msg.recipient = *_beneficiary; bytes addParams = codec.encodeWithSig("addAccountBalance(uint256)", balance); - auto addTableName = - getContractTableName(executor::USER_APPS_PREFIX, address2HexString(_msg.recipient)); + auto addTableName = hostContext.getContractTableName(address2HexString(_msg.recipient)); std::vector addTableNameVector = {addTableName}; auto addBalanceIn = codec.encode(addTableNameVector, addParams); bytes_view addBalance = bytes_view(addBalanceIn.data(), addBalanceIn.size()); diff --git a/bcos-executor/src/vm/HostContext.cpp b/bcos-executor/src/vm/HostContext.cpp index 9d59adb241..fe8a904068 100644 --- a/bcos-executor/src/vm/HostContext.cpp +++ b/bcos-executor/src/vm/HostContext.cpp @@ -174,7 +174,15 @@ evmc_result HostContext::externalRequest(const evmc_message* _msg) request->delegateCall = true; request->codeAddress = evmAddress2String(_msg->code_address); request->delegateCallSender = evmAddress2String(_msg->sender); - request->receiveAddress = codeAddress(); + + if (features().get(ledger::Features::Flag::bugfix_call_noaddr_return)) + { + request->receiveAddress = myAddress(); + } + else + { + request->receiveAddress = codeAddress(); + } request->data.assign(_msg->input_data, _msg->input_data + _msg->input_size); break; } @@ -328,6 +336,21 @@ evmc_result HostContext::callBuiltInPrecompiled( m_callParameters->logEntries = std::move(_request->logEntries); } + if (features().get(ledger::Features::Flag::feature_balance) && _request->value > 0) [[unlikely]] + { + // must transfer balance + if (!m_executive->transferBalance(_request->origin, _request->senderAddress, + _request->receiveAddress, static_cast(_request->value), _request->gas)) + { + callResults->type = CallParameters::REVERT; + callResults->status = (int32_t)TransactionStatus::NotEnoughCash; + preResult.status_code = EVMC_INSUFFICIENT_BALANCE; + preResult.gas_left = _request->gas; + m_responseStore.emplace_back(std::move(callResults)); + return preResult; + } + } + if (resultCode != (int32_t)TransactionStatus::None) { callResults->type = CallParameters::REVERT; diff --git a/bcos-executor/src/vm/HostContext.h b/bcos-executor/src/vm/HostContext.h index 8c97e9cdf9..dec8f06cce 100644 --- a/bcos-executor/src/vm/HostContext.h +++ b/bcos-executor/src/vm/HostContext.h @@ -172,6 +172,11 @@ class HostContext : public evmc_host_context return m_executive->blockContext().features(); } + std::string getContractTableName(const std::string_view& _address) + { + return m_executive->getContractTableName(_address, isWasm(), isCreate()); + } + protected: const CallParameters::UniquePtr& getCallParameters() const { return m_callParameters; } virtual bcos::bytes externalCodeRequest(const std::string_view& address); diff --git a/bcos-scheduler/src/SchedulerImpl.cpp b/bcos-scheduler/src/SchedulerImpl.cpp index c8a9254164..0e2690a148 100644 --- a/bcos-scheduler/src/SchedulerImpl.cpp +++ b/bcos-scheduler/src/SchedulerImpl.cpp @@ -507,14 +507,14 @@ void SchedulerImpl::commitBlock(bcos::protocol::BlockHeader::Ptr header, auto whenQueueFront = [this, requestBlockNumber, header = std::move(header), callback]( BlockExecutive::Ptr blockExecutive) { // acquire lock - std::shared_ptr> commitLock = - std::make_shared>(m_commitMutex, std::try_to_lock); + std::shared_ptr> commitLock = + std::make_shared>(m_commitMutex, std::try_to_lock); - if (!commitLock->owns_lock()) + if (!commitLock->owns_lock() && !commitLock->try_lock_for(std::chrono::seconds(1))) { std::string message = (boost::format("commitBlock: Another block is committing! Block to commit " - "number: %ld, hash: %s") % + "number: %ld, hash: %s, waiting for it") % requestBlockNumber % header->hash().abridged()) .str(); diff --git a/bcos-scheduler/src/SchedulerImpl.h b/bcos-scheduler/src/SchedulerImpl.h index 7cc5785160..25bc95a581 100644 --- a/bcos-scheduler/src/SchedulerImpl.h +++ b/bcos-scheduler/src/SchedulerImpl.h @@ -188,7 +188,7 @@ class SchedulerImpl : public SchedulerInterface, public std::enable_shared_from_ std::mutex m_blocksMutex; std::mutex m_executeMutex; - std::mutex m_commitMutex; + std::timed_mutex m_commitMutex; std::atomic_int64_t m_calledContextID = 1; diff --git a/bcos-scheduler/test/testSchedulerImpl.cpp b/bcos-scheduler/test/testSchedulerImpl.cpp index 54323d26d2..a6af9e7e1a 100644 --- a/bcos-scheduler/test/testSchedulerImpl.cpp +++ b/bcos-scheduler/test/testSchedulerImpl.cpp @@ -472,8 +472,6 @@ BOOST_AUTO_TEST_CASE(handlerBlockTest) BOOST_CHECK_EQUAL(config->hash().hex(), h256(5).hex()); } }); - - BOOST_CHECK(!commitBlockError); }