Updated made to handle block parameter as an object #1784
Conversation
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>
| protected abstract BlockParameterOrBlockHash blockParameterOrBlockHash( | ||
| JsonRpcRequestContext request); | ||
|
|
||
| protected abstract Object resultByBlockNumber(JsonRpcRequestContext request, long blockNumber); |
There was a problem hiding this comment.
I think this should be requestByBlockHash. There can be multiple hashes at a particular height, only one of which is "cannonical". We still want to be able to query non-cannonical blocks and those cannot be accessed by a block height lookup.
So where we have a number, do a lookup for the canonical hash. And where we have a hash and do a number lookup, just pass in the number.
Or we need two methods and all children implement it twice.
| } | ||
|
|
||
| result = | ||
| resultByBlockNumber( |
There was a problem hiding this comment.
If a hash on an orphan chain is passed in the net result of converting the hash to block number to abstract method call is that the subclass will marshal the wrong block because we get the canonical one at the height, not the orphan one we asked for.
… accordingly Signed-off-by: David Mechler <david.mechler@consensys.net>
| final long blockNumber = | ||
| getBlockHeaderByHash(blockHash).map(BlockHeader::getNumber).orElse(Long.MAX_VALUE); | ||
|
|
||
| return storageAt(address, storageIndex, blockNumber); |
There was a problem hiding this comment.
We can't simply do a hash->BlockNumber query here. What if they are querying from an orphaned block hash? The numbers will be wrong.
storageAt and the whole call series needs to be primarily hash based, where the calls to get it by block number quickly go to the hash based call chain.
| final long blockNumber = | ||
| getBlockHeaderByHash(blockHash).map(BlockHeader::getNumber).orElse(Long.MAX_VALUE); | ||
|
|
||
| return accountBalance(address, blockNumber); |
There was a problem hiding this comment.
Again, same concern with translating a block hash to a block number. We return possibly false info for non canonical hashes.
accountBalance and the whole call series needs to be primarily hash based, where the calls to get it by block number quickly go to the hash based call chain.
| * @return The code associated with this address. | ||
| */ | ||
| public Optional<Bytes> getCode(final Address address, final Hash blockHash) { | ||
| final long blockNumber = |
There was a problem hiding this comment.
Again, we lose orphan hash data going to a number and using that. The most likely scenario for failure is that a contract was deployed on an orphan chain but not the main chain, or vice versa, because the TXes committed were different.
We can't simply do a hash->BlockNumber query here. What if they are querying from an orphaned block hash? The numbers will be wrong.
getCode and the whole call series needs to be primarily hash based, where the calls to get it by block number quickly go to the hash based call chain.
| * @return the world state at the block hash | ||
| */ | ||
| public Optional<WorldState> getWorldState(final Hash blockHash) { | ||
| final long blockNumber = |
There was a problem hiding this comment.
Again, we cannot go hash->blockNumber and do the rest of the calls. We lose data.
Signed-off-by: David Mechler <david.mechler@consensys.net>
Signed-off-by: David Mechler <david.mechler@consensys.net>
* hyperledger#1733 - add support for eip-1898 Signed-off-by: David Mechler <david.mechler@consensys.net> * hyperledger#1733 - update changelog Signed-off-by: David Mechler <david.mechler@consensys.net> * hyperledger#1733 - Fix merge issues Signed-off-by: David Mechler <david.mechler@consensys.net> * Updated code to handle block parameter as an object; added new tests Signed-off-by: David Mechler <david.mechler@consensys.net> * hyperledger#1733 - Update to handle new block parameter; update tests accordingly Signed-off-by: David Mechler <david.mechler@consensys.net> * hyperledger#1733 - Updates for PR comments Signed-off-by: David Mechler <david.mechler@consensys.net> * hyperledger#1733 - fix broken test Signed-off-by: David Mechler <david.mechler@consensys.net>
PR description
Updated made to handle block parameter as an object
Fixed Issue(s)
#1733
Changelog