Skip to content
This repository has been archived by the owner on Apr 4, 2024. It is now read-only.

fix coinbase rpc endpoint to return ethereum address of the validator #153

Merged
merged 11 commits into from
Jun 22, 2021

Conversation

thomas-nguy
Copy link
Contributor

Closes: #112

Description

Coinbase previously was returning the operator address of the validator in hex format.

After this PR, eth_coinbase will return the ethereum format address of the account of the validator


For contributor use:

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer

For admin use:

  • Added appropriate labels to PR (ex. WIP, R4R, docs, etc)
  • Reviewers assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

@thomas-nguy thomas-nguy force-pushed the thomas/coinbase branch 2 times, most recently from a44fec9 to 61e491a Compare June 21, 2021 11:55
Copy link
Contributor

@fedekunze fedekunze left a comment

Choose a reason for hiding this comment

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

Logic looks good, although can be simplified (see comments). Also if you could add a test for the query on grpc_test.go 🙏

x/evm/keeper/grpc_query.go Outdated Show resolved Hide resolved
x/evm/keeper/grpc_query.go Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Jun 22, 2021

Codecov Report

Merging #153 (6018f68) into main (04bacbd) will decrease coverage by 0.85%.
The diff coverage is 18.82%.

❗ Current head 6018f68 differs from pull request most recent head 3b095d0. Consider uploading reports for the commit 3b095d0 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main     #153      +/-   ##
==========================================
- Coverage   48.84%   47.98%   -0.86%     
==========================================
  Files          43       43              
  Lines        2848     2932      +84     
==========================================
+ Hits         1391     1407      +16     
- Misses       1389     1454      +65     
- Partials       68       71       +3     
Impacted Files Coverage Δ
x/evm/types/query.pb.gw.go 0.00% <0.00%> (ø)
x/evm/keeper/grpc_query.go 77.51% <64.00%> (-2.35%) ⬇️
x/evm/keeper/abci.go 0.00% <0.00%> (ø)
x/evm/keeper/keeper.go 50.49% <0.00%> (ø)
x/evm/keeper/state_transition.go 0.00% <0.00%> (ø)

Copy link
Contributor

@fedekunze fedekunze left a comment

Choose a reason for hiding this comment

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

LGTM. Pending test and lint fixes

CHANGELOG.md Outdated Show resolved Hide resolved
ethereum/rpc/eth_api.go Outdated Show resolved Hide resolved
x/evm/keeper/grpc_query.go Outdated Show resolved Hide resolved
x/evm/keeper/grpc_query.go Show resolved Hide resolved
x/evm/keeper/grpc_query.go Outdated Show resolved Hide resolved
proto/ethermint/evm/v1alpha1/query.proto Outdated Show resolved Hide resolved
proto/ethermint/evm/v1alpha1/query.proto Outdated Show resolved Hide resolved
proto/ethermint/evm/v1alpha1/query.proto Outdated Show resolved Hide resolved
@fedekunze fedekunze merged commit fada595 into evmos:main Jun 22, 2021
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.

eth_coinbase address does not correspond to an account
2 participants