Skip to content
This repository was archived by the owner on Oct 28, 2021. It is now read-only.

string test_getLogHash(txHash) #4986

Merged
merged 3 commits into from
May 9, 2018
Merged

string test_getLogHash(txHash) #4986

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
22c7cfb
test_getLogHash
winsvega May 3, 2018
576402e
fix issues
winsvega May 7, 2018
9dadd1b
Improve error handling and reporting in test_getLogHash
gumb0 May 7, 2018
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
46 changes: 45 additions & 1 deletion libweb3jsonrpc/Test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,60 @@
#include "Test.h"
#include <jsonrpccpp/common/errors.h>
#include <jsonrpccpp/common/exception.h>
#include <libethereum/ClientTest.h>
#include <libdevcore/CommonJS.h>
#include <libethereum/ChainParams.h>
#include <libethereum/ClientTest.h>

using namespace std;
using namespace dev;
using namespace dev::eth;
using namespace dev::rpc;
using namespace jsonrpc;

Test::Test(eth::Client& _eth): m_eth(_eth) {}

namespace
{
string logEntriesToLogHash(eth::LogEntries const& _logs)
{
RLPStream s;
s.appendList(_logs.size());
for (eth::LogEntry const& l : _logs)
l.streamRLP(s);
return toJS(sha3(s.out()));
}
}

string Test::test_getLogHash(string const& _txHash)
{
try
{
h256 txHash;
try
{
txHash = h256(_txHash);
}
catch (BadHexCharacter const&)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not mask all possible errors, but only catch the errors of parsing the hexstring. If at some point (maybe in the future) it start to throw other exceptions, we won't fail with confusing "invalid params" error.

{
throw JsonRpcException(Errors::ERROR_RPC_INVALID_PARAMS);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was no any sense really in using BOOST_THROW_EXCEPTION for JsonRpcException which is not a boost exception

}

if (m_eth.blockChain().isKnownTransaction(txHash))
{
LocalisedTransaction t = m_eth.localisedTransaction(txHash);
BlockReceipts const& blockReceipts = m_eth.blockChain().receipts(t.blockHash());
if (blockReceipts.receipts.size() != 0)
return logEntriesToLogHash(blockReceipts.receipts[t.transactionIndex()].log());
}
return toJS(dev::EmptyListSHA3);
}
catch (std::exception const& ex)
{
cwarn << ex.what();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When something goes wrong we don't want to just silently return "internal error", we'd like to have a way to understand what happened, so let's output the error to the log.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_ex ?

Copy link
Contributor Author

@winsvega winsvega May 8, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could still use boost exception for nicer error output like this:

BOOST_THROW_EXCEPTION(JsonRpcException(Errors::ERROR_RPC_INTERNAL_ERROR, _ex.what()));

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No

  1. BOOST_THROW_EXCEPTION wraps your exception with additional info like filename and line in debug build, but it can be useful only if it's caught and accessed with boost.exception facilities. Here JsonRpcException is always caught by our JSON-RPC framework, which doesn't know anything about boost.exception and just transforms it into JSON response.
  2. The second argument to JsonRpcException constructor will also be added to JSON response, but what() string is usually for our exceptions not user-friendly at all, I don't think it will look appropriate at the client side. We should just log it in our log.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ex is not a function parameter, so no _ prefix needed

throw JsonRpcException(Errors::ERROR_RPC_INTERNAL_ERROR);
}
}

bool Test::test_setChainParams(Json::Value const& param1)
{
try
Expand Down
46 changes: 23 additions & 23 deletions libweb3jsonrpc/Test.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
/*
This file is part of cpp-ethereum.
This file is part of cpp-ethereum.

cpp-ethereum is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation, either version 3 of the License, or
(at your option) any later version.
cpp-ethereum is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation, either version 3 of the License, or
(at your option) any later version.

cpp-ethereum is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.
cpp-ethereum is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.

You should have received a copy of the GNU General Public License
along with cpp-ethereum. If not, see <http://www.gnu.org/licenses/>.
You should have received a copy of the GNU General Public License
along with cpp-ethereum. If not, see <http://www.gnu.org/licenses/>.
*/
/** @file Test.h
* @authors:
Expand All @@ -37,20 +37,20 @@ namespace rpc
class Test: public TestFace
{
public:
Test(eth::Client& _eth);
virtual RPCModules implementedModules() const override
{
return RPCModules{RPCModule{"test", "1.0"}};
}

virtual bool test_setChainParams(const Json::Value &param1) override;
virtual bool test_mineBlocks(int _number) override;
virtual bool test_modifyTimestamp(int _timestamp) override;
virtual bool test_addBlock(std::string const& _rlp) override;
virtual bool test_rewindToBlock(int _number) override;
Test(eth::Client& _eth);
virtual RPCModules implementedModules() const override
{
return RPCModules{RPCModule{"test", "1.0"}};
}
virtual std::string test_getLogHash(std::string const& _param1) override;
virtual bool test_setChainParams(const Json::Value& _param1) override;
virtual bool test_mineBlocks(int _number) override;
virtual bool test_modifyTimestamp(int _timestamp) override;
virtual bool test_addBlock(std::string const& _rlp) override;
virtual bool test_rewindToBlock(int _number) override;

private:
eth::Client& m_eth;
eth::Client& m_eth;
};

}
Expand Down
11 changes: 10 additions & 1 deletion libweb3jsonrpc/TestFace.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,21 @@ namespace dev {
public:
TestFace()
{
this->bindAndAddMethod(
jsonrpc::Procedure("test_getLogHash", jsonrpc::PARAMS_BY_POSITION,
jsonrpc::JSON_BOOLEAN, "param1", jsonrpc::JSON_STRING, NULL),
&dev::rpc::TestFace::test_getLogHashI);
this->bindAndAddMethod(jsonrpc::Procedure("test_setChainParams", jsonrpc::PARAMS_BY_POSITION, jsonrpc::JSON_BOOLEAN, "param1",jsonrpc::JSON_OBJECT, NULL), &dev::rpc::TestFace::test_setChainParamsI);
this->bindAndAddMethod(jsonrpc::Procedure("test_mineBlocks", jsonrpc::PARAMS_BY_POSITION, jsonrpc::JSON_BOOLEAN, "param1",jsonrpc::JSON_INTEGER, NULL), &dev::rpc::TestFace::test_mineBlocksI);
this->bindAndAddMethod(jsonrpc::Procedure("test_modifyTimestamp", jsonrpc::PARAMS_BY_POSITION, jsonrpc::JSON_BOOLEAN, "param1",jsonrpc::JSON_INTEGER, NULL), &dev::rpc::TestFace::test_modifyTimestampI);
this->bindAndAddMethod(jsonrpc::Procedure("test_addBlock", jsonrpc::PARAMS_BY_POSITION, jsonrpc::JSON_BOOLEAN, "param1",jsonrpc::JSON_STRING, NULL), &dev::rpc::TestFace::test_addBlockI);
this->bindAndAddMethod(jsonrpc::Procedure("test_rewindToBlock", jsonrpc::PARAMS_BY_POSITION, jsonrpc::JSON_BOOLEAN, "param1",jsonrpc::JSON_INTEGER, NULL), &dev::rpc::TestFace::test_rewindToBlockI);
}

inline virtual void test_getLogHashI(
const Json::Value& request, Json::Value& response)
{
response = this->test_getLogHash(request[0u].asString());
}
inline virtual void test_setChainParamsI(const Json::Value &request, Json::Value &response)
{
response = this->test_setChainParams(request[0u]);
Expand All @@ -41,6 +49,7 @@ namespace dev {
{
response = this->test_rewindToBlock(request[0u].asInt());
}
virtual std::string test_getLogHash(const std::string& param1) = 0;
virtual bool test_setChainParams(const Json::Value& param1) = 0;
virtual bool test_mineBlocks(int param1) = 0;
virtual bool test_modifyTimestamp(int param1) = 0;
Expand Down
1 change: 1 addition & 0 deletions libweb3jsonrpc/test.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[
{ "name": "test_getLogHash", "params": [""], "order": [], "returns": ""},
{ "name": "test_setChainParams", "params": [{}], "order": [], "returns": false},
{ "name": "test_mineBlocks", "params": [0], "returns": false },
{ "name": "test_modifyTimestamp", "params": [0], "returns": false },
Expand Down