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.

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

Update eth_getBlockByNumber and eth_getBlockByHash to match spec #1902

Merged
merged 6 commits into from
Jan 3, 2025

Conversation

maxconway
Copy link
Contributor

  • Removed view, mixhash, quorumCertificate, since they're not in Etherium spec
  • Added comments for why placeholders are placeholders

@maxconway maxconway force-pushed the max/bugfix/eth_getBlockByNumberOrHash branch from f77e80b to 41a623c Compare November 26, 2024 14:15
@maxconway maxconway disabled auto-merge December 3, 2024 10:48
- Removed view, mixhash, quorumCertificate, since they're not in Etherium spec
- Added comments for why placeholders are placeholders
- Moved it from the header to the body of the block return type. The header is flattened so this should work the same way, but avoided the relatively slow process of building the bloom filter for endpoints that don't need it.
@maxconway maxconway force-pushed the max/bugfix/eth_getBlockByNumberOrHash branch from 2554098 to a85a213 Compare December 4, 2024 22:35
@maxconway maxconway enabled auto-merge December 4, 2024 22:36
Fixed code to avoid getting stuck

But querying everything every time in order to build a bloom filter doesn't necessarily make sense, so we should look at this further
@maxconway maxconway force-pushed the max/bugfix/eth_getBlockByNumberOrHash branch from a85a213 to 053b498 Compare December 4, 2024 23:38
Copy link
Contributor

github-actions bot commented Dec 13, 2024

🐰 Bencher Report

Branchmax/bugfix/eth_getBlockByNumberOrHash
Testbedself-hosted
Click to view all benchmark results
BenchmarkLatencyBenchmark Result
milliseconds (ms)
(Result Δ%)
Upper Boundary
milliseconds (ms)
(Limit %)
process-empty/process-empty📈 view plot
🚷 view threshold
9.55
(+3.85%)
10.41
(91.77%)
produce-full/produce-full📈 view plot
🚷 view threshold
1,918.00
(-5.65%)
2,624.77
(73.07%)
🐰 View full continuous benchmarking report in Bencher

@maxconway maxconway removed the request for review from bzawisto January 3, 2025 15:32
@maxconway maxconway added this pull request to the merge queue Jan 3, 2025
Merged via the queue into main with commit 0e19694 Jan 3, 2025
6 checks passed
@maxconway maxconway deleted the max/bugfix/eth_getBlockByNumberOrHash branch January 3, 2025 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants