Skip to content

fix: handle_gossip_message nesting store into a tuple #1443

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

Merged
merged 2 commits into from
Apr 23, 2025

Conversation

LeanSerra
Copy link
Contributor

@LeanSerra LeanSerra commented Apr 23, 2025

Motivation

When receiving blobs by gossip the GenServer was crashing because of a pattern matching error as inside of handle_gossip_message we were calling the function PendingBlocks.process_blobs that returned {:ok, store} but the function handle_gossip_message was only supposed to return store

Description

  • When calling PendingBlocks.process_blobs extract the second element of the tuple and return only that

@LeanSerra LeanSerra changed the title fix PendingBlocks.process_blobs returns {:ok, store} we only need to … fix: handle_gossip_message Apr 23, 2025
@LeanSerra LeanSerra changed the title fix: handle_gossip_message fix: handle_gossip_message nesting store Apr 23, 2025
@LeanSerra LeanSerra changed the title fix: handle_gossip_message nesting store fix: handle_gossip_message nesting store into a tuple Apr 23, 2025
@LeanSerra LeanSerra marked this pull request as ready for review April 23, 2025 18:49
@LeanSerra LeanSerra requested a review from a team as a code owner April 23, 2025 18:49
@@ -19,7 +19,7 @@ defmodule LambdaEthereumConsensus.P2P.Gossip.BlobSideCar do
Logger.debug("[Gossip] Blob sidecar received, with index #{blob_index}")
Libp2pPort.validate_message(msg_id, :accept)
# TODO: (#1406) Enhance the API to reduce unnecessary wrappers (:ok + list)
PendingBlocks.process_blobs(store, {:ok, [blob]})
PendingBlocks.process_blobs(store, {:ok, [blob]}) |> then(&elem(&1, 1))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Couldn't this return an {:error, reason} in which case we'll be returning the reason here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PendingBlocks.process_blobs returns the previous store as {:ok, store} even if the blob isn't processed

@LeanSerra
Copy link
Contributor Author

Left #1444 as an issue to add tests for this so we don't have regressions again

@LeanSerra LeanSerra merged commit c3b54c4 into main Apr 23, 2025
15 checks passed
@LeanSerra LeanSerra deleted the fix_handle_gossip_message branch April 23, 2025 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

2 participants