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

Jump to bottom

fix: OOM when eth_getLogs response too large #42

Closed
wants to merge 3 commits into from
Closed

fix: OOM when eth_getLogs response too large #42

wants to merge 3 commits into from

Conversation

yihuang
Copy link
Collaborator

@yihuang yihuang commented Dec 28, 2021 •
edited
Loading

Description

backport: evmos#860

  • add limit to number of logs of filter response
  • make block limit and log limit configurable

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 mentioned this pull request Dec 28, 2021
Merged
13 tasks
@yihuang @fedekunze
fix: OOM when eth_getLogs response too large (evmos#860)
8109421 
* fix: OOM when eth_getLogs response too large

Closes: evmos#858

- add limit to number of logs of filter response
- make block limit and log limit configurable

* return error if exceeds log limit

* Apply suggestions from code review

* parse from config

* read cli flags

* add to config template

* fix bloomFilter

* changelog

* add validation

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>
tomtau
tomtau previously approved these changes Dec 31, 2021
View reviewed changes
Copy link

@tomtau tomtau left a comment

Choose a reason for hiding this comment

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

there's some formatting error, but it looks ok to be ported

@yihuang
Copy link
Collaborator Author

yihuang commented Dec 31, 2021

there's some formatting error, but it looks ok to be ported

formatting issue fixed.

yihuang
yihuang commented Dec 31, 2021
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
@yihuang yihuang requested a review from tomtau December 31, 2021 05:04
@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days-before-close if no further activity occurs.

@yihuang
Copy link
Collaborator Author

yihuang commented Feb 15, 2022

I've pushed directly, closing now.

@yihuang yihuang closed this Feb 15, 2022
@yihuang yihuang deleted the crypto-org-main branch February 15, 2022 02:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thomas-nguy thomas-nguy Awaiting requested review from thomas-nguy

@tomtau tomtau Awaiting requested review from tomtau

Assignees
No one assigned
Labels
Status: Stale
Projects
None yet
Milestone
No milestone
Development

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