Skip to content

Conversation

@thepabloaguilar
Copy link
Contributor

Fixes #73

Motivation

This PR solves the bug for reward amount for block zero!

Solution

I've removed the if statement where it checks if a given block is the zero block or not, without that if the value was returned correctly.

Request:

curl -XPOST localhost:8080/block -d '{
  "network_identifier": {
    "blockchain": "Ethereum",
    "network": "Mainnet"
  },
  "block_identifier": {
    "index": 0
  }
}'

Response:

{
    "block":{
        "block_identifier":{
            "index":0,
            "hash":"0xd4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3"
        },
        "parent_block_identifier":{
            "index":0,
            "hash":"0xd4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3"
        },
        "timestamp":0,
        "transactions":[
            {
                "transaction_identifier":{
                    "hash":"0xd4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3"
                },
                "operations":[
                    {
                        "operation_identifier":{
                            "index":0
                        },
                        "type":"MINER_REWARD",
                        "status":"SUCCESS",
                        "account":{
                            "address":"0x0000000000000000000000000000000000000000"
                        },
                        "amount":{
                            "value":"5000000000000000000",
                            "currency":{
                                "symbol":"ETH",
                                "decimals":18
                            }
                        }
                    }
                ]
            }
        ]
    }
}

I checked the go-ethereum implementation where it calculate the rewards and it doesn't have any special treatment for block zero: https://github.com/ethereum/go-ethereum/blob/045e90c8971cddbabeac0abd54240cf02bc1d94d/consensus/ethash/consensus.go#L646-L653

Open questions

Is there a way to test this with a unit test? Would it be nice to add one for this case?

@shrimalmadhur shrimalmadhur self-requested a review January 14, 2022 07:11
@shrimalmadhur
Copy link
Contributor

@thepabloaguilar This is great, thanks for solving it quickly. I think you could try adding a test like https://github.com/coinbase/rosetta-ethereum/blob/master/ethereum/client_test.go#L1173 - you can create a test like TestBlock_Genesis - you can look some block specific test where you can download the block result as json and mock it. Let me know if that doesn't make sense.

@thepabloaguilar
Copy link
Contributor Author

@shrimalmadhur I've wrote a test, I think it's make sense to have that test because we're talking about the first block on the chain

@shrimalmadhur shrimalmadhur merged commit 7081025 into coinbase:master Jan 25, 2022
@thepabloaguilar thepabloaguilar deleted the issue-73 branch January 26, 2022 01:38
xiaying-peng added a commit that referenced this pull request Apr 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Incorrect Mining Reward for Genesis Block

2 participants