Skip to content

Commit

Permalink
<fix>(scheduler): fix call empty address cause assert core bug.
Browse files Browse the repository at this point in the history
  • Loading branch information
kyonRay committed Feb 20, 2022
1 parent 8ea916e commit c0ab943
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 28 deletions.
43 changes: 29 additions & 14 deletions bcos-executor/src/precompiled/extension/ContractAuthPrecompiled.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,12 @@ void ContractAuthPrecompiled::getAdmin(
codec->decode(data, contractAddress);
path = contractAddress.hex();
}
PRECOMPILED_LOG(DEBUG) << LOG_BADGE("ContractAuthPrecompiled") << LOG_DESC("getAdmin")
<< LOG_KV("path", path);
path = getAuthTableName(path);
std::string adminStr = getContractAdmin(_executive, path);
PRECOMPILED_LOG(DEBUG) << LOG_BADGE("ContractAuthPrecompiled") << LOG_DESC("getAdmin success")
<< LOG_KV("admin", adminStr);
if (adminStr.empty())
{
callResult->setExecResult(codec->encode(Address()));
Expand Down Expand Up @@ -214,19 +218,22 @@ void ContractAuthPrecompiled::resetAdmin(
{
codec->decode(data, path, admin);
}
PRECOMPILED_LOG(DEBUG) << LOG_BADGE("ContractAuthPrecompiled") << LOG_DESC("resetAdmin")
<< LOG_KV("path", path) << LOG_KV("admin", admin);
if (!checkSender(_sender))
{
PRECOMPILED_LOG(ERROR) << LOG_BADGE("ContractAuthPrecompiled")
<< LOG_DESC("sender is not from sys") << LOG_KV("path", path);
<< LOG_DESC("sender is not from sys") << LOG_KV("path", path)
<< LOG_KV("sender", _sender);
getErrorCodeOut(callResult->mutableExecResult(), CODE_NO_AUTHORIZED, *codec);
return;
}
path = getAuthTableName(path);
auto table = _executive->storage().openTable(path);
if (!table || !table->getRow(ADMIN_FIELD))
{
PRECOMPILED_LOG(ERROR) << LOG_BADGE("ContractAuthPrecompiled") << LOG_DESC("path not found")
<< LOG_KV("path", path);
PRECOMPILED_LOG(ERROR) << LOG_BADGE("ContractAuthPrecompiled")
<< LOG_DESC("resetAdmin: path not found") << LOG_KV("path", path);
BOOST_THROW_EXCEPTION(
protocol::PrecompiledError() << errinfo_comment("Contract address not found."));
}
Expand Down Expand Up @@ -359,8 +366,9 @@ bool ContractAuthPrecompiled::checkMethodAuth(
(lastStorage) ? lastStorage->openTable(path) : _executive->storage().openTable(path);
if (!table)
{
// only precompiled contract in /sys/, or pre-built-in contract
PRECOMPILED_LOG(DEBUG) << LOG_BADGE("ContractAuthPrecompiled")
<< LOG_DESC("path not found, maybe a callback message.")
<< LOG_DESC("auth table not found, auth pass through by default.")
<< LOG_KV("path", path);
return true;
}
Expand All @@ -386,8 +394,9 @@ bool ContractAuthPrecompiled::checkMethodAuth(
auto entry = table->getRow(getTypeStr);
if (!entry || entry->getField(SYS_VALUE).empty())
{
PRECOMPILED_LOG(ERROR) << LOG_BADGE("ContractAuthPrecompiled")
<< LOG_DESC("auth row not found") << LOG_KV("path", path);
PRECOMPILED_LOG(DEBUG) << LOG_BADGE("ContractAuthPrecompiled")
<< LOG_DESC("auth row not found, no method set acl")
<< LOG_KV("path", path) << LOG_KV("authType", getTypeStr);
// if entry still empty
// if white list mode, return false
// if black list mode, return true
Expand Down Expand Up @@ -438,14 +447,15 @@ void ContractAuthPrecompiled::setMethodAuth(
codec->decode(data, path, _func, account);
}
bytes func = codec::fromString32(_func).ref().getCroppedData(0, 4).toBytes();
PRECOMPILED_LOG(INFO) << LOG_BADGE("ContractAuthPrecompiled") << LOG_DESC("setAuth")
<< LOG_KV("path", path) << LOG_KV("func", toHexStringWithPrefix(func))
<< LOG_KV("account", account.hex());
PRECOMPILED_LOG(DEBUG) << LOG_BADGE("ContractAuthPrecompiled") << LOG_DESC("setAuth")
<< LOG_KV("path", path) << LOG_KV("func", toHexStringWithPrefix(func))
<< LOG_KV("account", account.hex());
path = getAuthTableName(path);
auto table = _executive->storage().openTable(path);
if (!table)
{
PRECOMPILED_LOG(ERROR) << LOG_BADGE("ContractAuthPrecompiled") << LOG_DESC("path not found")
PRECOMPILED_LOG(ERROR) << LOG_BADGE("ContractAuthPrecompiled")
<< LOG_DESC("auth table not found, should not set acl")
<< LOG_KV("path", path);
getErrorCodeOut(callResult->mutableExecResult(), CODE_TABLE_NOT_EXIST, *codec);
return;
Expand Down Expand Up @@ -540,8 +550,9 @@ s256 ContractAuthPrecompiled::getMethodAuthType(
auto entry = table->getRow(METHOD_AUTH_TYPE);
if (!entry || entry->getField(SYS_VALUE).empty())
{
PRECOMPILED_LOG(DEBUG) << LOG_BADGE("ContractAuthPrecompiled")
<< LOG_DESC("should set the method access auth type firstly");
PRECOMPILED_LOG(DEBUG)
<< LOG_BADGE("ContractAuthPrecompiled")
<< LOG_DESC("no acl strategy exist, should set the method access auth type firstly");
return (int)CODE_TABLE_AUTH_TYPE_NOT_EXIST;
}
std::string authTypeStr = std::string(entry->getField(SYS_VALUE));
Expand Down Expand Up @@ -592,6 +603,8 @@ u256 ContractAuthPrecompiled::getDeployAuthType(
const std::shared_ptr<executor::TransactionExecutive>& _executive)
{
auto lastStorage = _executive->lastStorage();
PRECOMPILED_LOG(TRACE) << LOG_BADGE("getDeployAuthType") << (lastStorage) ? "use lastStorage" :
"use latestStorage";
auto table =
(lastStorage) ? lastStorage->openTable("/apps") : _executive->storage().openTable("/apps");
// table must exist
Expand All @@ -606,6 +619,7 @@ u256 ContractAuthPrecompiled::getDeployAuthType(
{
return 0;
}
PRECOMPILED_LOG(TRACE) << LOG_BADGE("getDeployAuthType") << LOG_KV("type", type);
return type;
}

Expand Down Expand Up @@ -744,8 +758,9 @@ bool ContractAuthPrecompiled::checkDeployAuth(
auto entry = table->getRow(getAclType);
if (entry->getField(0).empty())
{
PRECOMPILED_LOG(ERROR) << LOG_BADGE("ContractAuthPrecompiled")
<< LOG_DESC("deploy auth row not found");
PRECOMPILED_LOG(DEBUG) << LOG_BADGE("ContractAuthPrecompiled")
<< LOG_DESC("not deploy acl exist, return by default")
<< LOG_KV("aclType", type);
// if entry still empty
// if white list mode, return false
// if black list mode, return true
Expand Down
2 changes: 1 addition & 1 deletion bcos-rpc/bcos-rpc/jsonrpc/JsonRpcImpl_2_0.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,7 @@ void JsonRpcImpl_2_0::call(std::string const& _groupID, std::string const& _node
const std::string& _to, const std::string& _data, RespFunc _respFunc)
{
RPC_IMPL_LOG(TRACE) << LOG_DESC("call") << LOG_KV("to", _to) << LOG_KV("group", _groupID)
<< LOG_KV("node", _nodeName);
<< LOG_KV("node", _nodeName) << LOG_KV("data", _data);

auto nodeService = getNodeService(_groupID, _nodeName, "call");
auto transactionFactory = nodeService->blockFactory()->transactionFactory();
Expand Down
7 changes: 7 additions & 0 deletions bcos-scheduler/src/SchedulerImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,13 @@ void SchedulerImpl::status(
void SchedulerImpl::call(protocol::Transaction::Ptr tx,
std::function<void(Error::Ptr&&, protocol::TransactionReceipt::Ptr&&)> callback)
{
// call but to is empty,
// it will cause tx message be marked as 'create' falsely when asyncExecute tx
if (tx->to().empty())
{
callback(BCOS_ERROR_PTR(SchedulerError::UnknownError, "Call address is empty"), nullptr);
return;
}
// Create temp block
auto block = m_blockFactory->createBlock();
block->appendTransaction(std::move(tx));
Expand Down
42 changes: 29 additions & 13 deletions bcos-scheduler/tests/testScheduler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -277,23 +277,39 @@ BOOST_AUTO_TEST_CASE(call)
auto tx = blockFactory->transactionFactory()->createTransaction(0, "address_to",
bytes(inputStr.begin(), inputStr.end()), 200, 300, "chain", "group", 500, keyPair);

bcos::protocol::TransactionReceipt::Ptr receipt;
auto empty_to = blockFactory->transactionFactory()->createTransaction(
0, "", bytes(inputStr.begin(), inputStr.end()), 200, 300, "chain", "group", 500, keyPair);

scheduler->call(
tx, [&](bcos::Error::Ptr error, bcos::protocol::TransactionReceipt::Ptr receiptResponse) {
BOOST_CHECK(!error);
BOOST_CHECK(receiptResponse);
// call
{
bcos::protocol::TransactionReceipt::Ptr receipt;

receipt = std::move(receiptResponse);
});
scheduler->call(tx,
[&](bcos::Error::Ptr error, bcos::protocol::TransactionReceipt::Ptr receiptResponse) {
BOOST_CHECK(!error);
BOOST_CHECK(receiptResponse);

BOOST_CHECK_EQUAL(receipt->blockNumber(), 0);
BOOST_CHECK_EQUAL(receipt->status(), 0);
BOOST_CHECK_GT(receipt->gasUsed(), 0);
auto output = receipt->output();
receipt = std::move(receiptResponse);
});

std::string outputStr((char*)output.data(), output.size());
BOOST_CHECK_EQUAL(outputStr, "Hello world! response");
BOOST_CHECK_EQUAL(receipt->blockNumber(), 0);
BOOST_CHECK_EQUAL(receipt->status(), 0);
BOOST_CHECK_GT(receipt->gasUsed(), 0);
auto output = receipt->output();

std::string outputStr((char*)output.data(), output.size());
BOOST_CHECK_EQUAL(outputStr, "Hello world! response");
}

// call empty to
{
scheduler->call(empty_to,
[&](bcos::Error::Ptr error, bcos::protocol::TransactionReceipt::Ptr receiptResponse) {
BOOST_CHECK(error);
BOOST_CHECK(error->errorMessage() == "Call address is empty");
BOOST_CHECK(receiptResponse == nullptr);
});
}
}

BOOST_AUTO_TEST_CASE(registerExecutor)
Expand Down

0 comments on commit c0ab943

Please sign in to comment.