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

clean up the block fetch logic in json-rpc #1081

Merged
merged 2 commits into from
May 23, 2022
Merged

clean up the block fetch logic in json-rpc #1081

merged 2 commits into from
May 23, 2022

Conversation

yihuang
Copy link
Contributor

@yihuang yihuang commented May 13, 2022

deduplicate some codes

Description

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)

@yihuang yihuang force-pushed the cleanup-get-block branch 5 times, most recently from 99279db to 82f0632 Compare May 16, 2022 04:29
fedekunze
fedekunze approved these changes May 19, 2022
View reviewed changes
Copy link
Contributor

@fedekunze fedekunze left a comment •
edited
Loading

Choose a reason for hiding this comment

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

ACK! Pending new changelog and conflict fixes

@yihuang
Copy link
Contributor Author

yihuang commented May 19, 2022

ACK! Pending new changelog and conflict fixes

done

@fedekunze fedekunze enabled auto-merge (squash) May 19, 2022 10:12
yihuang
yihuang commented May 23, 2022
View reviewed changes
rpc/backend/evm_backend.go Outdated Show resolved Hide resolved
@yihuang yihuang marked this pull request as draft May 23, 2022 08:57
auto-merge was automatically disabled May 23, 2022 08:57

Pull request was converted to draft

@yihuang
clean up the block fetch logic in json-rpc
8cd780e 
deduplicate some codes

fix EthBlockFromTm

fix latest block height
@yihuang yihuang marked this pull request as ready for review May 23, 2022 09:11
@yihuang yihuang requested a review from fedekunze May 23, 2022 09:12
@fedekunze
Copy link
Contributor

=== RUN TestEth_Pending_GetTransactionByHash
rpc_pending_test.go:298:
Error Trace: rpc_pending_test.go:298
Error: Should NOT be empty, but was []
Test: TestEth_Pending_GetTransactionByHash

@yihuang some RPC tests are failing, can you look into it?

@yihuang
Copy link
Contributor Author

yihuang commented May 23, 2022

=== RUN TestEth_Pending_GetTransactionByHash
rpc_pending_test.go:298:
Error Trace: rpc_pending_test.go:298
Error: Should NOT be empty, but was []
Test: TestEth_Pending_GetTransactionByHash

@yihuang some RPC tests are failing, can you look into it?

I think that failure is a common non-deterministic failure we've been seeing, should not be due to this PR. maybe we should open an issue for that. You can try to re-run the test.

@yihuang
Copy link
Contributor Author

yihuang commented May 23, 2022

=== RUN TestEth_Pending_GetTransactionByHash
rpc_pending_test.go:298:
Error Trace: rpc_pending_test.go:298
Error: Should NOT be empty, but was []
Test: TestEth_Pending_GetTransactionByHash

@yihuang some RPC tests are failing, can you look into it?

#1092
opened an issue for now.

@fedekunze fedekunze merged commit f06df8c into evmos:main May 23, 2022
@yihuang yihuang deleted the cleanup-get-block branch May 23, 2022 16:37
yihuang added a commit to yihuang/ethermint that referenced this pull request May 25, 2022
@yihuang
imp: clean up the block fetch logic in json-rpc (evmos#1081)
17f543a 
* clean up the block fetch logic in json-rpc

deduplicate some codes

fix EthBlockFromTm

fix latest block height

* add bug fix changelog
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@khoslaventures khoslaventures Awaiting requested review from khoslaventures

@jolube jolube Awaiting requested review from jolube

@fedekunze fedekunze Awaiting requested review from fedekunze

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@yihuang @fedekunze