Skip to content

Commit

Permalink
Fix precompiled message, fix clangd style (FISCO-BCOS#4442)
Browse files Browse the repository at this point in the history
  • Loading branch information
@morebtcg
morebtcg authored May 9, 2024
1 parent 3214342 commit 89bfa03
Show file tree
Hide file tree
Showing 9 changed files with 179 additions and 133 deletions.
32 changes: 18 additions & 14 deletions transaction-executor/bcos-transaction-executor/RollbackableStorage.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,16 @@ class Rollbackable
oldValue;
};
std::vector<Record> m_records;
Storage* m_storage;
std::reference_wrapper<Storage> m_storage;

public:
using Savepoint = int64_t;
using Key = typename Storage::Key;
using Value = typename Storage::Value;

Rollbackable(Storage& storage) : m_storage(std::addressof(storage)) {}
Rollbackable(Storage& storage) : m_storage(storage) {}

Storage& storage() { return *m_storage; }
Storage& storage() { return m_storage; }
Savepoint current() const { return static_cast<int64_t>(m_records.size()); }

task::Task<void> rollback(Savepoint savepoint)
Expand All @@ -56,11 +56,11 @@ class Rollbackable
if (record.oldValue)
{
co_await storage2::writeOne(
*m_storage, std::move(record.key), std::move(*record.oldValue));
m_storage.get(), std::move(record.key), std::move(*record.oldValue));
}
else
{
co_await storage2::removeOne(*m_storage, record.key, storage2::DIRECT);
co_await storage2::removeOne(m_storage.get(), record.key, storage2::DIRECT);
}
m_records.pop_back();
}
Expand All @@ -73,14 +73,15 @@ class Rollbackable
std::invoke_result_t<storage2::ReadSome, Storage&, decltype(keys)>>>
{
co_return co_await storage2::readSome(
*storage.m_storage, std::forward<decltype(keys)>(keys));
storage.m_storage.get(), std::forward<decltype(keys)>(keys));
}

friend auto tag_invoke(storage2::tag_t<storage2::readOne> /*unused*/, Rollbackable& storage,
auto&& key) -> task::Task<task::AwaitableReturnType<std::invoke_result_t<storage2::ReadOne,
Storage&, decltype(key)>>>
{
co_return co_await storage2::readOne(*storage.m_storage, std::forward<decltype(key)>(key));
co_return co_await storage2::readOne(
storage.m_storage.get(), std::forward<decltype(key)>(key));
}

friend auto tag_invoke(storage2::tag_t<storage2::writeSome> /*unused*/, Rollbackable& storage,
Expand All @@ -89,14 +90,15 @@ class Rollbackable
std::invoke_result_t<storage2::WriteSome, Storage&, decltype(keys), decltype(values)>>>
requires HasReadSomeDirect<Storage>
{
auto oldValues = co_await storage2::readSome(*storage.m_storage, keys, storage2::DIRECT);
auto oldValues =
co_await storage2::readSome(storage.m_storage.get(), keys, storage2::DIRECT);
for (auto&& [key, oldValue] : RANGES::views::zip(keys, oldValues))
{
storage.m_records.emplace_back(Record{.key = typename Storage::Key{key},
.oldValue = std::forward<decltype(oldValue)>(oldValue)});
}

co_return co_await storage2::writeSome(*storage.m_storage,
co_return co_await storage2::writeSome(storage.m_storage.get(),
std::forward<decltype(keys)>(keys), std::forward<decltype(values)>(values));
}

Expand All @@ -108,9 +110,10 @@ class Rollbackable
{
auto& record = storage.m_records.emplace_back();
record.key = key;
record.oldValue = co_await storage2::readOne(*storage.m_storage, key, storage2::DIRECT);
record.oldValue =
co_await storage2::readOne(storage.m_storage.get(), key, storage2::DIRECT);
co_await storage2::writeOne(
*storage.m_storage, record.key, std::forward<decltype(value)>(value));
storage.m_storage.get(), record.key, std::forward<decltype(value)>(value));
}

friend auto tag_invoke(storage2::tag_t<storage2::removeSome> /*unused*/, Rollbackable& storage,
Expand All @@ -119,7 +122,8 @@ class Rollbackable
std::add_lvalue_reference_t<Storage>, decltype(keys)>>>
{
// Store values to history
auto oldValues = co_await storage2::readSome(*storage.m_storage, keys, storage2::DIRECT);
auto oldValues =
co_await storage2::readSome(storage.m_storage.get(), keys, storage2::DIRECT);
for (auto&& [key, value] : RANGES::views::zip(keys, oldValues))
{
if (value)
Expand All @@ -129,15 +133,15 @@ class Rollbackable
}
}

co_return co_await storage2::removeSome(*storage.m_storage, keys);
co_return co_await storage2::removeSome(storage.m_storage.get(), keys);
}

friend auto tag_invoke(bcos::storage2::tag_t<storage2::range> /*unused*/, Rollbackable& storage,
auto&&... args) -> task::Task<storage2::ReturnType<std::invoke_result_t<storage2::Range,
std::add_lvalue_reference_t<Storage>, decltype(args)...>>>
{
co_return co_await storage2::range(
*(storage.m_storage), std::forward<decltype(args)>(args)...);
storage.m_storage.get(), std::forward<decltype(args)>(args)...);
}
};

Expand Down
17 changes: 9 additions & 8 deletions transaction-executor/bcos-transaction-executor/TransactionExecutorImpl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class TransactionExecutorImpl
protocol::TransactionReceiptFactory const& receiptFactory, crypto::Hash::Ptr hashImpl);

private:
protocol::TransactionReceiptFactory const& m_receiptFactory;
std::reference_wrapper<protocol::TransactionReceiptFactory const> m_receiptFactory;
crypto::Hash::Ptr m_hashImpl;
PrecompiledManager m_precompiledManager;

Expand Down Expand Up @@ -95,14 +95,15 @@ class TransactionExecutorImpl
switch (transactionVersion)
{
case bcos::protocol::TransactionVersion::V0_VERSION:
receipt = executor.m_receiptFactory.createReceipt(gasLimit - evmcResult.gas_left,
newContractAddress, logEntries, receiptStatus, output, blockHeader.number());
receipt = executor.m_receiptFactory.get().createReceipt(
gasLimit - evmcResult.gas_left, newContractAddress, logEntries, receiptStatus,
output, blockHeader.number());
break;
case bcos::protocol::TransactionVersion::V1_VERSION:
case bcos::protocol::TransactionVersion::V2_VERSION:
receipt = executor.m_receiptFactory.createReceipt2(gasLimit - evmcResult.gas_left,
newContractAddress, logEntries, receiptStatus, output, blockHeader.number(), "",
transactionVersion);
receipt = executor.m_receiptFactory.get().createReceipt2(
gasLimit - evmcResult.gas_left, newContractAddress, logEntries, receiptStatus,
output, blockHeader.number(), "", transactionVersion);
break;
default:
BOOST_THROW_EXCEPTION(std::runtime_error(
Expand All @@ -114,7 +115,7 @@ class TransactionExecutorImpl
TRANSACTION_EXECUTOR_LOG(DEBUG)
<< "Not found code exception: " << boost::diagnostic_information(e);

receipt = executor.m_receiptFactory.createReceipt(
receipt = executor.m_receiptFactory.get().createReceipt(
0, {}, {}, EVMC_REVERT, {}, blockHeader.number());
receipt->setMessage(boost::diagnostic_information(e));
}
Expand All @@ -123,7 +124,7 @@ class TransactionExecutorImpl
TRANSACTION_EXECUTOR_LOG(DEBUG)
<< "Execute exception: " << boost::diagnostic_information(e);

receipt = executor.m_receiptFactory.createReceipt(
receipt = executor.m_receiptFactory.get().createReceipt(
0, {}, {}, EVMC_INTERNAL_ERROR, {}, blockHeader.number());
receipt->setMessage(boost::diagnostic_information(e));
}
Expand Down
16 changes: 16 additions & 0 deletions transaction-executor/bcos-transaction-executor/precompiled/ExecutiveWrapper.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@ inline std::shared_ptr<precompiled::Precompiled> getInnerPrecompiled(auto const&
return std::get<std::shared_ptr<precompiled::Precompiled>>(precompiled.m_precompiled);
}

struct ErrorMessage
{
uint8_t* buffer{};
size_t size{};
};

template <class T>
concept ExternalCaller = std::is_invocable_r_v<EVMCResult, T, const evmc_message&>;

Expand Down Expand Up @@ -119,6 +125,16 @@ class ExecutiveWrapper : public executor::TransactionExecutive
callResult->gas = result.gas_left;
callResult->data.assign(result.output_data, result.output_data + result.output_size);

if (result.status_code != 0)
{
if (auto* errorMessage = (ErrorMessage*)result.create_address.bytes;
errorMessage->buffer != nullptr && errorMessage->size > 0)
{
callResult->message.assign(
errorMessage->buffer, errorMessage->buffer + errorMessage->size);
}
}

return callResult;
}
};
Expand Down
16 changes: 12 additions & 4 deletions transaction-executor/bcos-transaction-executor/precompiled/PrecompiledImpl.cpp
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,20 @@
#include "PrecompiledImpl.h"

bcos::transaction_executor::ErrorMessage bcos::transaction_executor::buildErrorMessage(
bcos::transaction_executor::ErrorMessage bcos::transaction_executor::buildEncodeErrorMessage(
std::string_view message, crypto::Hash const& hashImpl)
{
bcos::codec::abi::ContractABICodec abi(hashImpl);
auto codecOutput = abi.abiIn("Error(string)", message);
ErrorMessage errorMessage{.buffer = std::unique_ptr<uint8_t>(new uint8_t[codecOutput.size()]),
.size = codecOutput.size()};
std::uninitialized_copy(codecOutput.begin(), codecOutput.end(), errorMessage.buffer.get());
ErrorMessage errorMessage{
.buffer = new uint8_t[codecOutput.size()], .size = codecOutput.size()};
std::uninitialized_copy(codecOutput.begin(), codecOutput.end(), errorMessage.buffer);
return errorMessage;
}

bcos::transaction_executor::ErrorMessage bcos::transaction_executor::buildErrorMessage(
std::string_view message)
{
ErrorMessage errorMessage{.buffer = new uint8_t[message.size()], .size = message.size()};
std::uninitialized_copy(message.begin(), message.end(), errorMessage.buffer);
return errorMessage;
}
26 changes: 15 additions & 11 deletions transaction-executor/bcos-transaction-executor/precompiled/PrecompiledImpl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,8 @@ inline auto buildLegacyExecutive(auto& storage, protocol::BlockHeader const& blo
std::move(externalCaller), precompiledManager);
}

struct ErrorMessage
{
std::unique_ptr<uint8_t> buffer;
size_t size{};
};
ErrorMessage buildErrorMessage(std::string_view message, crypto::Hash const& hashImpl);
ErrorMessage buildEncodeErrorMessage(std::string_view message, crypto::Hash const& hashImpl);
ErrorMessage buildErrorMessage(std::string_view message);

struct Precompiled
{
Expand Down Expand Up @@ -161,20 +157,28 @@ inline constexpr struct
<< "Revert transaction: PrecompiledFailed"
<< LOG_KV("address", contractAddress) << LOG_KV("message", e.what());

auto [errorMessage, size] =
buildErrorMessage(e.what(), *executor::GlobalHashImpl::g_hashImpl);
std::string_view error(e.what());
auto encodedErrorMessage =
buildEncodeErrorMessage(error, *executor::GlobalHashImpl::g_hashImpl);

static_assert(sizeof(EVMCResult::create_address) > sizeof(ErrorMessage));
auto errorMessage = buildErrorMessage(error);
decltype(EVMCResult::create_address) wrapAddress{};
*(ErrorMessage*)wrapAddress.bytes = errorMessage;

return EVMCResult{evmc_result{
.status_code =
(evmc_status_code)protocol::TransactionStatus::PrecompiledError,
.gas_left = message.gas,
.gas_refund = 0,
.output_data = errorMessage.release(),
.output_size = size,
.output_data = encodedErrorMessage.buffer,
.output_size = encodedErrorMessage.size,
.release =
[](const struct evmc_result* result) {
delete[] result->output_data;
delete[] ((ErrorMessage*)result->create_address.bytes)->buffer;
},
.create_address = {},
.create_address = wrapAddress,
.padding = {},
}};
}
Expand Down
Loading

0 comments on commit 89bfa03

Please sign in to comment.