Skip to content
This repository was archived by the owner on Nov 30, 2021. It is now read-only.

Conversation

@ansermino
Copy link

@ansermino ansermino commented Sep 10, 2019

This updates the return types for the eth_ API and simplifies the types used in queries.

  • Updates RPC return types
  • Removes custom query types in favour of default eth
    • This is largely to allow for proper hexadecimal formatting (provided by hexutil), as the API is very specific about formatting.

@austinabell
Copy link

austinabell commented Sep 10, 2019

LGTM given the ToInt lint fix from the type change (can just remove the ToInt function)

x/evm/querier.go Outdated
num := ctx.BlockHeight()
bnRes := types.QueryResBlockNumber{Number: big.NewInt(num)}
res, err := codec.MarshalJSONIndent(keeper.cdc, bnRes)
hexBig := hexutil.Uint64(num)

Choose a reason for hiding this comment

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

Small nitpick but confusing name since it's a hexutil uint. Also, the type that is being received on the RPC api is just a uint64, reason for marshalling it as this type?

Copy link
Author

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

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

ah, my bad actually (was looking at old type I think), if you do another commit though to change hexBig to hexUint or something more accurate would be great, but fine with this

Copy link

@austinabell austinabell left a comment

Choose a reason for hiding this comment

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

small detail I just want to be sure about

@ansermino ansermino merged commit 1cac4fe into development Sep 24, 2019
@ansermino ansermino deleted the david/fixes branch September 24, 2019 14:49
@austinabell austinabell mentioned this pull request Sep 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants