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

Fix blockbloom in header error #258

Merged
merged 8 commits into from
Jul 14, 2021
Merged

Conversation

thomas-nguy
Copy link
Contributor

@thomas-nguy thomas-nguy commented Jul 12, 2021

Closes: #248

Description

When blockbloom is nil and empty it is currently returning an error. We should return an empty bloom instead to avoid breaking other functionnalities.

Note that I have notice a weird behavior with the context (which return a blockboom at currentheight-1 when context is set at current height but it cannot be found is context is set at current height -1


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 changed the title fix context index [WIP]fix context index Jul 12, 2021
@fedekunze
Copy link
Contributor

see #263, that should fix the issue

@thomas-nguy
Copy link
Contributor Author

I have checked on main (including #263) but the issue is still there.

It is working if I always use ctx's height = height +1 . Not sure why it is happening. I will need to investigate further

@thomas-nguy thomas-nguy changed the title [WIP]fix context index Fix blockbloom in header Jul 13, 2021
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.

few requests for comments. Make sure you pull the latest changes from main

x/evm/keeper/state_transition.go Outdated Show resolved Hide resolved
x/evm/keeper/state_transition.go Show resolved Hide resolved
x/evm/keeper/state_transition.go Show resolved Hide resolved
ethereum/rpc/types/utils.go Outdated Show resolved Hide resolved
@thomas-nguy thomas-nguy changed the title Fix blockbloom in header Fix blockbloom in header error Jul 14, 2021
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.

ACK, can you add a bugfix changelog entry?

@thomas-nguy
Copy link
Contributor Author

sure, README updated

@codecov
Copy link

codecov bot commented Jul 14, 2021

Codecov Report

Merging #258 (9763cc0) into main (e09bf23) will decrease coverage by 0.01%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #258      +/-   ##
==========================================
- Coverage   47.55%   47.54%   -0.02%     
==========================================
  Files          45       45              
  Lines        3135     3136       +1     
==========================================
  Hits         1491     1491              
- Misses       1569     1570       +1     
  Partials       75       75              
Impacted Files Coverage Δ
x/evm/keeper/state_transition.go 0.00% <0.00%> (ø)

CHANGELOG.md Outdated Show resolved Hide resolved
@fedekunze fedekunze enabled auto-merge (squash) July 14, 2021 09:33
@fedekunze fedekunze merged commit aab793e into evmos:main Jul 14, 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_getLogs throws error
2 participants