[after Constantinople] test rpc #4859

winsvega · 2018-02-23T13:34:12Z

experimenting with rpc methods that would make test generation possible via rpc.

ethereum/retesteth#5
ethereum/retesteth#4

codecov-io · 2018-02-23T19:10:45Z

Codecov Report

Merging #4859 into develop will increase coverage by 2.5%.
The diff coverage is 13%.

@@            Coverage Diff             @@
##           develop    #4859     +/-   ##
==========================================
+ Coverage    58.46%   60.96%   +2.5%     
==========================================
  Files          337      350     +13     
  Lines        27024    28415   +1391     
  Branches      3164     2894    -270     
==========================================
+ Hits         15799    17323   +1524     
+ Misses       10161    10134     -27     
+ Partials      1064      958    -106

axic · 2018-03-07T13:06:28Z

libethereum/ChainParams.cpp

+        rules = Network::HomesteadTest;
+    else if (forkRules == "EIP150")
+        rules = Network::EIP150Test;
+    else if (forkRules == "EIP158")


I'd prefer the general config it would be nice to refer to these with their semi-"official" names, such as SpuriousDragon and TangerineWhistle.

Still we need docs on what those names mean exactly. What changes are applied.
ethereum/retesteth#12

chfast · 2018-03-12T22:23:51Z

When are you planning to finish this? The more you add here the longer it will take to do the review process.

axic · 2018-03-13T00:23:24Z

eth/main.cpp

@@ -1063,6 +1063,13 @@ int main(int argc, char** argv)
    web3.setIdealPeerCount(peers);
    web3.setPeerStretch(peerStretch);
 //  std::shared_ptr<eth::BasicGasPricer> gasPricer = make_shared<eth::BasicGasPricer>(u256(double(ether / 1000) / etherPrice), u256(blockFees * 1000));
+
+    // allow any gasPrice in testing mode


This would be a good candidate for a PR on its own.

winsvega · 2018-03-15T14:01:02Z

I was planning to run all existing tests with RPC.
Now I see that there are lost of issues and it will take much more time to cover this.
Another issue is that methods format might change in the future.

If you fine with that. I could start refactoring this PR and split it into multimple if you want.
Please keep the existing testeth in this repo for now. (travis automatition) I dont want to break existing test cases in cpp client.

axic · 2018-03-21T08:57:46Z

libethereum/Client.cpp

    callQueuedFunctions();

+    if (shouldStop())


Not sure what these shouldStop expressions are doing but it seems it would be better to have it as a PR on its own?

I will put small changes from this PR into a separate and then rebase this on top develop.
this shouldStop() accelerating blockchain reset. Is is a tweak by Andrey. Perhaps better be moved to TestClient.cpp

axic · 2018-03-21T08:58:09Z

libethereum/ChainParams.cpp

@@ -281,3 +291,63 @@ bytes ChainParams::genesisBlock() const
 	block.appendRaw(RLPEmptyList);
 	return block.out();
 }
+
+#include <libethashseal/GenesisInfo.h>


Shouldn't includes go to the top of the file?

axic · 2018-03-21T08:59:08Z

libethereum/ClientTest.cpp

        reopenChain(params, WithExisting::Kill);
-        setAuthor(params.author); //for some reason author is not being set
+        completeSync();  // set sync state to idle for mining


These seems to be a bugfix?

yes. it resolve setAuthor issue

Do you want to create a new PR for the bugfix?

axic · 2018-03-21T08:59:22Z

libweb3jsonrpc/JsonHelper.cpp

@@ -360,8 +360,8 @@ TransactionSkeleton toTransactionSkeleton(Json::Value const& _json)

 	if (!_json["from"].empty())
 		ret.from = jsToAddress(_json["from"].asString());
-	if (!_json["to"].empty() && _json["to"].asString() != "0x")
-		ret.to = jsToAddress(_json["to"].asString());
+    if (!_json["to"].empty() && _json["to"].asString() != "0x" && !_json["to"].asString().empty())


This seems to be a bugfix?

yes. because empty() is true if there is no "to" json section.
but the string in this section could be empty or 0x (looks like no consensus on format here)

Should there be a PR for this bugfix?

ask/bid price PR is yet not merged.

How is that relevant?

I don't want to track many PRs at the same time.
ok. I have now finished fixing the tests and could concentrate on PRs here.

@axic #4923

axic · 2018-03-21T08:59:48Z

libweb3jsonrpc/Test.cpp

@@ -33,6 +38,107 @@ using namespace jsonrpc;

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

+string prepareVersionString()


This should be moved to a generic place I'd assume. The client should also print this on start.

Also if not moved elsewhere, should be placed into the anonymous namespace.

this one copied from test namespace. however libwe3jsonrpc should not have deps on test namespace

I think it should be a generic helper available allover cpp-eth, what do you think @chfast and @gumb0 ?

chfast · 2018-03-22T17:43:39Z

When this is going to be ready for review?

axic · 2018-03-22T17:59:17Z

libweb3jsonrpc/test.json

@@ -1,4 +1,7 @@
 [
+{ "name": "test_getClientInfo", "params": [], "order": [], "returns": ""},


How is this different to https://github.com/ethereum/wiki/wiki/JSON-RPC#web3_clientversion ? It seems you are just returning the version.

in that case. no need to implement a new method.
I am removing it from the list of test RPC methods:
ethereum/retesteth#5

Can you remove it from this PR then?

winsvega · 2018-03-22T18:38:52Z

@chfast seeing how a 1 line commit takes about 2 weeks to get merged. I guess this branch stays as experimental. and to merge changes from this branch I will open series of PRs with small changes. then just rebase this branch untill no changes left.

chfast · 2018-03-22T21:12:18Z

If you refer to #4895, maybe it's time to fix the broken tests?

winsvega · 2018-07-25T13:23:42Z

libethcore/ChainOperationParams.cpp

@@ -73,7 +73,8 @@ EVMSchedule const& ChainOperationParams::scheduleForBlockNumber(u256 const& _blo

 u256 ChainOperationParams::blockReward(EVMSchedule const& _schedule) const
 {
-	if (_schedule.blockRewardOverwrite)
+    return 0;


no reward for StateTests.
for BlockchainTests reward should be enabled.

I don't understand. This change does not make sense to me.

this is not the change. its a work around. currently there is no way to set mining reward from the config option.
Andrey said that mining reward must present and be hardcoded by the fork rules.
I guess this means no StateTests support then. But we actually dont need state tests because if you run it with test_mineBlocks anyway. there would be no difference between state and blockchain test then.

so this line could be removed

still not clear, what do you want us to do with this PR now?

we need to agree on genesis scheme ethereum/tests#469

winsvega · 2019-03-26T16:28:06Z

introduced changes are merged

winsvega force-pushed the testrpc branch from ece6502 to 2dacf37 Compare February 27, 2018 19:14

axic reviewed Mar 7, 2018

View reviewed changes

axic reviewed Mar 13, 2018

View reviewed changes

winsvega force-pushed the testrpc branch from 0a421c7 to e70bc32 Compare March 16, 2018 22:53

axic reviewed Mar 21, 2018

View reviewed changes

axic mentioned this pull request Mar 22, 2018

Specify blockchain testing methods in RPC ethereum/interfaces#4

Open

axic reviewed Mar 22, 2018

View reviewed changes

winsvega force-pushed the testrpc branch from 57ef0c1 to 8f84922 Compare April 3, 2018 13:17

winsvega force-pushed the testrpc branch 2 times, most recently from 2d5f915 to d15c234 Compare April 17, 2018 16:44

winsvega force-pushed the testrpc branch from d15c234 to 5715d75 Compare April 27, 2018 20:39

winsvega force-pushed the testrpc branch 2 times, most recently from 5515bfb to f6cdf0c Compare May 25, 2018 14:04

winsvega force-pushed the testrpc branch 2 times, most recently from da73f6d to b81c41b Compare June 19, 2018 14:33

winsvega force-pushed the testrpc branch from b81c41b to bcf9b8f Compare June 27, 2018 10:47

winsvega force-pushed the testrpc branch from bcf9b8f to 6426af1 Compare July 11, 2018 10:33

winsvega force-pushed the testrpc branch 2 times, most recently from d436b7a to 73ba70b Compare July 25, 2018 13:20

winsvega commented Jul 25, 2018

View reviewed changes

gumb0 requested review from gumb0 and removed request for gumb0 July 25, 2018 13:27

chfast changed the base branch from develop to master August 2, 2018 08:38

axic added the testeth label Aug 9, 2018

winsvega changed the title ~~test rpc [WIP]~~ [after Constantinople] test rpc Oct 9, 2018

wip

2ad93da

winsvega force-pushed the testrpc branch from 73ba70b to 2ad93da Compare November 27, 2018 17:34

winsvega closed this Mar 26, 2019

		@@ -33,6 +38,107 @@ using namespace jsonrpc;

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

		string prepareVersionString()

		@@ -1,4 +1,7 @@
		[
		{ "name": "test_getClientInfo", "params": [], "order": [], "returns": ""},

[after Constantinople] test rpc #4859

[after Constantinople] test rpc #4859

Conversation

winsvega commented Feb 23, 2018

codecov-io commented Feb 23, 2018 • edited Loading

Codecov Report

Choose a reason for hiding this comment

winsvega Mar 15, 2018 • edited Loading

Choose a reason for hiding this comment

chfast commented Mar 12, 2018

Choose a reason for hiding this comment

winsvega commented Mar 15, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

winsvega Mar 29, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chfast commented Mar 22, 2018

Choose a reason for hiding this comment

winsvega Mar 22, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

winsvega commented Mar 22, 2018

chfast commented Mar 22, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

winsvega Jul 25, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

winsvega commented Mar 26, 2019

codecov-io commented Feb 23, 2018 •

edited

Loading

winsvega Mar 15, 2018 •

edited

Loading

winsvega Mar 29, 2018 •

edited

Loading

winsvega Mar 22, 2018 •

edited

Loading

winsvega Jul 25, 2018 •

edited

Loading