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

EVM: Implement getBlockByNumber query #884

Merged
merged 13 commits into from
Sep 19, 2023

Conversation

orkunkilic
Copy link
Contributor

@orkunkilic orkunkilic commented Sep 15, 2023

Description

Implements get_block_by_number method in sov-evm query. Implemented "earliest", "latest", and hex parameters. Implemented both details(full / hash) on transaction return type.

Block_number defaults to latest block if not present.

Looking forward for efficiency feedbacks!

Linked Issues

Testing

Describe how these changes were tested. If you've added new features, have you added unit tests?
Tests WIP

Docs

Describe where this code is documented. If it changes a documented interface, have the docs been updated?
No docs added yet.

@orkunkilic orkunkilic marked this pull request as draft September 15, 2023 08:59
@codecov
Copy link

codecov bot commented Sep 15, 2023

Codecov Report

Merging #884 (b349df9) into nightly (d9e2075) will decrease coverage by 0.3%.
The diff coverage is 0.0%.

Files Changed Coverage Δ
...system/module-implementations/sov-evm/src/query.rs 0.0% <0.0%> (ø)

... and 3 files with indirect coverage changes

@orkunkilic orkunkilic changed the title Implement getBlockByNumber query EVM: Implement getBlockByNumber query Sep 15, 2023
Copy link
Contributor

@LukaszRozmej LukaszRozmej left a comment

Choose a reason for hiding this comment

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

needs a test

@LukaszRozmej LukaszRozmej dismissed their stale review September 19, 2023 09:50

not relevant

@orkunkilic orkunkilic marked this pull request as ready for review September 19, 2023 11:40
@orkunkilic
Copy link
Contributor Author

Tests are added, I think good to go after final review.

@orkunkilic orkunkilic requested a review from bkolad September 19, 2023 11:41
@bkolad bkolad enabled auto-merge September 19, 2023 11:56
@bkolad bkolad added this pull request to the merge queue Sep 19, 2023
Merged via the queue into Sovereign-Labs:nightly with commit 1879a45 Sep 19, 2023
.expect("Transaction must be set");
(id, tx)
})
.collect::<Vec<_>>();
Copy link
Contributor

Choose a reason for hiding this comment

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

You shouldn't need to collect it here, this causes unnecessary allocation on heap.

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.

EVM: Implement eth_getBlockByNumber endpoint.
3 participants