Add new JSON_RPC endpoints to get miner data by block number and block hash. #1538
Conversation
…d stack values Signed-off-by: David Mechler <david.mechler@consensys.net>
Signed-off-by: David Mechler <david.mechler@consensys.net>
Signed-off-by: David Mechler <david.mechler@consensys.net>
…can occur Signed-off-by: David Mechler <david.mechler@consensys.net>
…eation so that the hash can be managed. Signed-off-by: David Mechler <david.mechler@consensys.net>
Signed-off-by: David Mechler <david.mechler@consensys.net>
Signed-off-by: David Mechler <david.mechler@consensys.net>
Signed-off-by: David Mechler <david.mechler@consensys.net>
Signed-off-by: David Mechler <david.mechler@consensys.net>
Signed-off-by: David Mechler <david.mechler@consensys.net>
Signed-off-by: David Mechler <david.mechler@consensys.net>
Signed-off-by: David Mechler <david.mechler@consensys.net>
Signed-off-by: David Mechler <david.mechler@consensys.net>
Signed-off-by: David Mechler <david.mechler@consensys.net>
Signed-off-by: David Mechler <david.mechler@consensys.net>
Signed-off-by: David Mechler <david.mechler@consensys.net>
Signed-off-by: David Mechler <david.mechler@consensys.net>
|
@richardpringle and @sammy1991106 please take a look at the changes and let me know if this is what you were expecting. |
Signed-off-by: David Mechler <david.mechler@consensys.net>
| new BlockWithMetadata<>( | ||
| header, Collections.emptyList(), Collections.emptyList(), Difficulty.of(100L), 5); | ||
|
|
||
| Mockito.when(blockchainQueries.blockByNumber(Mockito.anyLong())) |
There was a problem hiding this comment.
Optional Style: Most of our mocking uses static import of .when() and (when not using the junit annotations) .mock(). Similarly for most of the Assertsions.* series to. This tends to make a more "readable" test at the expense of making it clear this is a Mockito method.
| } | ||
|
|
||
| /* | ||
| Getting static block reward |
There was a problem hiding this comment.
I'm surprised the google formatter didn't ask you to star indent this block.
There was a problem hiding this comment.
Comment removed as this is no longer relevant.
| pass in the block height (as a long) get the block reward (as wei) | ||
| each block spec has it. but it’s only meaningful w/in 6 blocks of the fork. | ||
|
|
||
| Didn’t happen at the two places we reduced block reward on mainnet. I say leave a comment but write no code. |
There was a problem hiding this comment.
Even though this is a slack quote it should be in a documentary tone. I'd also move it around 108-111 where the uncle reward calculation is calculated as it is a code relevant comment not a quasi-javadoc comment.
There was a problem hiding this comment.
Comment removed as this is no longer relevant.
| BlockWithMetadata<TransactionWithMetadata, Hash> block = | ||
| getBlockchainQueries().blockByNumber(blockNumber).orElse(null); | ||
|
|
||
| MinerDataResult minerDataResult = null; |
There was a problem hiding this comment.
This looks duplicated from what is in EthGetMinerDataByBlockHash. This will burn us one day if we don't call a shared method. I'd recommend an AbstractMinerDataCall super class where this method is present and the two subclasses marshal the block by either hash or number.
If there are differences between the two then we need code comments pointing out the differences as the subtlety will be missed when we come back in the future,
There was a problem hiding this comment.
Added static method in EthGetMinerDataByBlockHash for both classes to call since EthGetMinerDataByBlockNumber has a parent.
| if (withRevertReason && revertReason.isPresent()) { | ||
| out.writeBytes(revertReason.get()); | ||
| if (withMetadata) { | ||
| out.writeBytes(revertReason.orElse(Bytes.EMPTY)); |
There was a problem hiding this comment.
I had a comment written out about using out.writeNull if not present but when I looked at RLPOutput the impl is just to write empty bytes. Might be worth a comment pointing out that writing Bytes.EMPTY is the same as calling RLPOutput.writeNull
| throw new RLPException("Unexpected value at end of TransactionReceipt"); | ||
| } | ||
| revertReason = Optional.of(input.readBytes()); | ||
| gasRemaining = input.readLong(); |
There was a problem hiding this comment.
What about when we do a migration? What if we have an old DB with revert reason but not miner data and start extracting it? gasRemaining may not always be there.
I would wrap everything read after the metadataAllowed check in if (!input.isEndOfCurrentList()) { ... } individually as a safety measure. So two times in this case.
There was a problem hiding this comment.
Will wrap the gasRemaining read but the block starts with the check so the revertReason read is safe.
Signed-off-by: David Mechler <david.mechler@consensys.net>
Signed-off-by: David Mechler <david.mechler@consensys.net>
Signed-off-by: David Mechler <david.mechler@consensys.net>
|
Is there a new Postman def. for these included RPC calls? |
PR description
Add new JSON_RPC endpoints to get miner data by block number and block hash.
Fixed Issue(s)
#1408
Changelog