cybercongress / cyber-search

🚀 Toolchain for transactions parsing and processing
https://cybercongress.github.io/cyber-search/
Other
33 stars 14 forks source link

#185 ETH: calculate block & uncle rewards from block traces #256

Closed KevinLiLu closed 6 years ago

KevinLiLu commented 6 years ago

resolves #185

KevinLiLu commented 6 years ago

pinging @hleb-albau @arturalbov for review 😄

arturalbov commented 6 years ago

Hello @KevinLiLu Currently we are a little bit far away from a good internet connection :) we'll review your request as soon as possible. Thanks!

KevinLiLu commented 6 years ago

@arturalbov no worries!

hleb-albau commented 6 years ago

@KevinLiLu also, if you need parity traces, just put block number here, i will provide it to you.

KevinLiLu commented 6 years ago

@hleb-albau can you get the parity trace for https://etherscan.io/block/6149845

thanks 😄

hleb-albau commented 6 years ago

Sure, @KevinLiLu ! parity-0x5DD6D5.txt

KevinLiLu commented 6 years ago

Thanks @hleb-albau !

Here's a question. So the reward traces in the parity output you attached show this:

{"action":{"author":"0x4bb96091ee9d802ed039c4d1a5f6216f90f81b01","rewardType":"block","value":"0x2c3c465ca58ec000"},"blockHash":"0x02e2ee54556a80710af3f80691781bf4eae37ae869d40bcf46b4186aeb6ce4d7","blockNumber":6149845,"result":null,"subtraces":0,"traceAddress":[],"transactionHash":null,"transactionPosition":null,"type":"reward"},
{"action":{"author":"0x829bd824b016326a401d083b33d092293333a830","rewardType":"uncle","value":"0x246ddf9797668000"},"blockHash":"0x02e2ee54556a80710af3f80691781bf4eae37ae869d40bcf46b4186aeb6ce4d7","blockNumber":6149845,"result":null,"subtraces":0,"traceAddress":[],"transactionHash":null,"transactionPosition":null,"type":"reward"},
{"action":{"author":"0x52bc44d5378309ee2abf1539bf71de1b7d7be3b5","rewardType":"uncle","value":"0x1f399b1438a10000"},"blockHash":"0x02e2ee54556a80710af3f80691781bf4eae37ae869d40bcf46b4186aeb6ce4d7","blockNumber":6149845,"result":null,"subtraces":0,"traceAddress":[],"transactionHash":null,"transactionPosition":null,"type":"reward"}

The two uncle traces have almost identical fields and values, with the only differing fields are value and author. Is the author the miner? Can I assume that the uncle rewards are returned in order of their position? Uncle at position 0 would be the 2nd line, and uncle at position 1 would be the 3rd line in my attachment above?

I guess I would have to keep a counter when I iterate through the traces to get an uncle block's reward.

hleb-albau commented 6 years ago

@KevinLiLu

  1. Yes, author is Miner of uncle. So, value is uncle miner reward.
  2. In method parityUnclesToDao you already know uncle position. You can get reward from calls by uncle miner(author).
KevinLiLu commented 6 years ago

@hleb-albau

Makes sense.

Although it is possible that a single miner mines both uncles?

If so, this is what I am thinking:

Since we have uncle.miner in parityUnclesToDao, then we can filter and get all reward traces that have author matching uncle.miner.

If there is only 1 result, then that is the corresponding uncle reward.

If there is more than 1 result, then we just get the trace at the uncle position in the result list?

hleb-albau commented 6 years ago

@KevinLiLu Cool catch. Yeah, position in list is ok. Also, we can double check here by comparing values. Closest uncles have more value.

KevinLiLu commented 6 years ago

Using https://etherscan.io/block/6154914 as the test case for two uncle rewards but same miner (changed the miner to a fake on 0x123).

KevinLiLu commented 6 years ago

@hleb-albau Ok I have pushed my fixes.

ParityToEthereumBundleConverterTest has two test cases now:

convertTwoUnclesDifferentMinerTest contains the original tests before. I changed the data fields to use some actual block data.

The CI failed, but when I run :search-api:test locally it passes. Can you re-trigger the test?

hleb-albau commented 6 years ago

@KevinLiLu Beside general unstable tests problem(#255 ), there is one correctly failing test EthereumBlockchainInterfaceTest.blockBundleByNumberTest(). I cloned this pr, run tests locally and reproduce test failing.

KevinLiLu commented 6 years ago

@hleb-albau Ok, I have fixed the block reward calculation and tests.

Please pull in the new commit and test it out.

KevinLiLu commented 6 years ago

@hleb-albau bumping this as it has been a week since the last response

KevinLiLu commented 6 years ago

Hi @hleb-albau, I have pushed the fix so please give that a look and let me know if the formula can be improved...

I also had to bump up jacoco version as they released 0.8.2 and the SNAPSHOT is no longer available.