Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

internal/ethapi: support block number or hash on state-related methods #19491

Merged
merged 1 commit into from
Sep 26, 2019
Merged

internal/ethapi: support block number or hash on state-related methods #19491

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions core/blockchain.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2133,6 +2133,11 @@ func (bc *BlockChain) HasHeader(hash common.Hash, number uint64) bool {
return bc.hc.HasHeader(hash, number)
}

// GetCanonicalHash returns the canonical hash for a given block number
func (bc *BlockChain) GetCanonicalHash(number uint64) common.Hash {
return bc.hc.GetCanonicalHash(number)
}

// GetBlockHashesFromHash retrieves a number of block hashes starting at a given
// hash, fetching towards the genesis block.
func (bc *BlockChain) GetBlockHashesFromHash(hash common.Hash, max uint64) []common.Hash {
Expand Down
4 changes: 4 additions & 0 deletions core/headerchain.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,10 @@ func (hc *HeaderChain) GetHeaderByNumber(number uint64) *types.Header {
return hc.GetHeader(hash, number)
}

func (hc *HeaderChain) GetCanonicalHash(number uint64) common.Hash {
return rawdb.ReadCanonicalHash(hc.chainDb, number)
}

// CurrentHeader retrieves the current head header of the canonical chain. The
// header is retrieved from the HeaderChain's internal cache.
func (hc *HeaderChain) CurrentHeader() *types.Header {
Expand Down
59 changes: 59 additions & 0 deletions eth/api_backend.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,23 @@ func (b *EthAPIBackend) HeaderByNumber(ctx context.Context, number rpc.BlockNumb
return b.eth.blockchain.GetHeaderByNumber(uint64(number)), nil
}

func (b *EthAPIBackend) HeaderByNumberOrHash(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash) (*types.Header, error) {
if blockNr, ok := blockNrOrHash.Number(); ok {
return b.HeaderByNumber(ctx, blockNr)
}
if hash, ok := blockNrOrHash.Hash(); ok {
header := b.eth.blockchain.GetHeaderByHash(hash)
if header == nil {
return nil, errors.New("header for hash not found")
}
if blockNrOrHash.RequireCanonical && b.eth.blockchain.GetCanonicalHash(header.Number.Uint64()) != hash {
return nil, errors.New("hash is not currently canonical")
}
return header, nil
}
return nil, errors.New("invalid arguments; neither block nor hash specified")
}

func (b *EthAPIBackend) HeaderByHash(ctx context.Context, hash common.Hash) (*types.Header, error) {
return b.eth.blockchain.GetHeaderByHash(hash), nil
}
Expand All @@ -93,6 +110,27 @@ func (b *EthAPIBackend) BlockByHash(ctx context.Context, hash common.Hash) (*typ
return b.eth.blockchain.GetBlockByHash(hash), nil
}

func (b *EthAPIBackend) BlockByNumberOrHash(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash) (*types.Block, error) {
if blockNr, ok := blockNrOrHash.Number(); ok {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the spec/docs clearly define that number has priority over hash? This implementation would return the header/block at number regardless if the hash matches or not, if both are provided.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, currently the spec does not mention precedence or have any test cases for it.

@charles-cooper care to update the spec regarding this scenario? I see four options:

  • Option 1: Block Number takes priority
  • Option 2: Block Hash takes priority
  • Option 3: If both blockNumber and blockHash are specified, return an error indicating these values are exclusive
  • Option 4: If both blockNumber and blockHash are specified, enforce that both values are correct, otherwise return an error (i.e. lookup by hash, and return an error if returned header.Number != blockNumber).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I opened ethereum/EIPs#2247 to solicit feedback on which option people prefer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As Charles mentioned on the linked issue, probably makes sense to stay compatible with the behavior in EIP-234 (the one that added blockHash to eth_getLogs and friends), and that EIP says the number and hash arguments are mutually exclusive, so I'll update my code to Option 3.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated BlockNumberOrHash.UnmarshalJSON to return an error if both number and hash are specified with an error very similar to the one returned by FilterCriteria.UnmarshalJSON when both are specified as part of an EIP-234 request.

return b.BlockByNumber(ctx, blockNr)
}
if hash, ok := blockNrOrHash.Hash(); ok {
header := b.eth.blockchain.GetHeaderByHash(hash)
if header == nil {
return nil, errors.New("header for hash not found")
}
if blockNrOrHash.RequireCanonical && b.eth.blockchain.GetCanonicalHash(header.Number.Uint64()) != hash {
return nil, errors.New("hash is not currently canonical")
}
block := b.eth.blockchain.GetBlock(hash, header.Number.Uint64())
if block == nil {
return nil, errors.New("header found, but block body is missing")
}
return block, nil
}
return nil, errors.New("invalid arguments; neither block nor hash specified")
}

func (b *EthAPIBackend) StateAndHeaderByNumber(ctx context.Context, number rpc.BlockNumber) (*state.StateDB, *types.Header, error) {
// Pending state is only known by the miner
if number == rpc.PendingBlockNumber {
Expand All @@ -111,6 +149,27 @@ func (b *EthAPIBackend) StateAndHeaderByNumber(ctx context.Context, number rpc.B
return stateDb, header, err
}

func (b *EthAPIBackend) StateAndHeaderByNumberOrHash(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash) (*state.StateDB, *types.Header, error) {
if blockNr, ok := blockNrOrHash.Number(); ok {
return b.StateAndHeaderByNumber(ctx, blockNr)
}
if hash, ok := blockNrOrHash.Hash(); ok {
header, err := b.HeaderByHash(ctx, hash)
if err != nil {
return nil, nil, err
}
if header == nil {
return nil, nil, errors.New("header for hash not found")
}
if blockNrOrHash.RequireCanonical && b.eth.blockchain.GetCanonicalHash(header.Number.Uint64()) != hash {
return nil, nil, errors.New("hash is not currently canonical")
}
stateDb, err := b.eth.BlockChain().StateAt(header.Root)
return stateDb, header, err
}
return nil, nil, errors.New("invalid arguments; neither block nor hash specified")
}

func (b *EthAPIBackend) GetReceipts(ctx context.Context, hash common.Hash) (types.Receipts, error) {
return b.eth.blockchain.GetReceiptsByHash(hash), nil
}
Expand Down
Loading