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

eth/protocols/eth: remove Requests in block body #30562

Merged
merged 2 commits into from
Oct 10, 2024

Conversation

islishude
Copy link
Contributor

Block no longer has Requests.

It wasn't removed in #30425

@fjl fjl changed the title eth/protocols: remove Requests eth/protocols/eth: remove Requests Oct 9, 2024
@fjl fjl changed the title eth/protocols/eth: remove Requests eth/protocols/eth: remove Requests in block body Oct 9, 2024
@MariusVanDerWijden
Copy link
Member

MariusVanDerWijden commented Oct 10, 2024

I think we also need to remove the requestHashes in eth/downloader/downloader_test.go:233

Otherwise the following diff will panic:

diff --git a/eth/downloader/fetchers_concurrent_bodies.go b/eth/downloader/fetchers_concurrent_bodies.go
index 56359b33c9..3d8a1576a0 100644
--- a/eth/downloader/fetchers_concurrent_bodies.go
+++ b/eth/downloader/fetchers_concurrent_bodies.go
@@ -90,6 +90,9 @@ func (q *bodyQueue) request(peer *peerConnection, req *fetchRequest, resCh chan
 func (q *bodyQueue) deliver(peer *peerConnection, packet *eth.Response) (int, error) {
        txs, uncles, withdrawals := packet.Res.(*eth.BlockBodiesResponse).Unpack()
        hashsets := packet.Meta.([][]common.Hash) // {txs hashes, uncle hashes, withdrawal hashes}
+       if len(hashsets) != 3 {
+               panic("adsfadsf")
+       }

@MariusVanDerWijden
Copy link
Member

I think we also need to remove the emptyRequests check from EmptyBody in core/types/block.go.
Otherwise we might try to fetch a block that should not be fetched otherwise, but I'm not 100% sure here cc @lightclient @fjl

@islishude
Copy link
Contributor Author

I think we also need to remove the emptyRequests check from EmptyBody in core/types/block.go.

It was removed in #30425

@MariusVanDerWijden
Copy link
Member

Ah perfect, looks like I was on a wrong branch

Copy link
Member

@MariusVanDerWijden MariusVanDerWijden left a comment

Choose a reason for hiding this comment

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

LGTM

@fjl fjl merged commit 5b393ac into ethereum:master Oct 10, 2024
3 checks passed
@fjl fjl added this to the 1.14.12 milestone Oct 10, 2024
@islishude islishude deleted the remove-requests branch October 10, 2024 08:51
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.

3 participants