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.

Sign up for GitHub

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

Jump to bottom

feat: Add blob topic handler #18

Merged
merged 1 commit into from
May 17, 2024
Merged

feat: Add blob topic handler #18

merged 1 commit into from
May 17, 2024

Conversation

0x00101010
Copy link
Contributor

@0x00101010 0x00101010 commented May 2, 2024 •
edited
Loading

Description

Add blob topic handler to decode blob p2p message and produce TraceEvent.

info  | 17:51:25.943235 | Handling blob gossip message              
PeerID=16Uiu2HAmTA28PLzU7oYBQujqRWN8oG8wBmgF6zhUe99HzWuU7GyV 
RemotePeerID=16Uiu2HAmAcLyYnyjgrznMaCXwRUfWJGChVyr2Qdz3cr6sKwT2Hvn 
MsgID=0e0002377b3edbb39a1c486e02fb421962f7466b 
MsgSize=9598 
Topic=/eth2/d31f6191/blob_sidecar_2/ssz_snappy 
Seq=[] 
Slot=5005157 
ValIdx=260 
TimeInSlot=1.943217312 
StateRoot=0xab69b4b0e8904205840bafaeca266a86c78eeb16a9eee13aa5cf2a9db18747bd 
BodyRoot=0x718cb2446668e05c3fe63c326902a7d656b72635e277b2f6266d134a53ce6a6c 
ParentRoot=0xf27c0e927ac5670184eec5ebdf05740997d0cfd56d643b26e368c84b7e547b4f

@0x00101010 0x00101010 force-pushed the 0x00101010/blobs branch 2 times, most recently from 813d9a3 to e600d84 Compare May 2, 2024 23:47
@0x00101010 0x00101010 marked this pull request as ready for review May 2, 2024 23:47
@codecov-commenter
Copy link

codecov-commenter commented May 3, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 0% with 44 lines in your changes are missing coverage. Please review.

Project coverage is 3.15%. Comparing base (b94d32c) to head (82a7227).
Report is 85 commits behind head on main.

Files Patch % Lines
eth/pubsub.go 0.00% 41 Missing ⚠️
eth/node_config.go 0.00% 2 Missing ⚠️
host/host.go 0.00% 1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files 
@@           Coverage Diff            @@
##            main     #18      +/-   ##
========================================
- Coverage   7.75%   3.15%   -4.60%     
========================================
  Files          7      26      +19     
  Lines        400    3519    +3119     
========================================
+ Hits          31     111      +80     
- Misses       361    3397    +3036     
- Partials       8      11       +3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@0x00101010 0x00101010 force-pushed the 0x00101010/blobs branch 4 times, most recently from 8cb3216 to efa312a Compare May 15, 2024 17:49
guillaumemichel
guillaumemichel reviewed May 16, 2024
View reviewed changes
eth/pubsub.go Outdated
Comment on lines 204 to 218
slog.Info(
"Handling blob gossip message",
"PeerID", p.host.ID(),
"RemotePeerID", msg.ReceivedFrom.String(),
"MsgID", hex.EncodeToString([]byte(msg.ID)),
"MsgSize", len(msg.Data),
"Topic", msg.GetTopic(),
"Seq", msg.GetSeqno(),
"Slot", slot,
"ValIdx", proposerIndex,
"TimeInSlot", now.Sub(slotStart).Seconds(),
"StateRoot", hexutil.Encode(blob.GetSignedBlockHeader().GetHeader().GetStateRoot()),
"BodyRoot", hexutil.Encode(blob.GetSignedBlockHeader().GetHeader().GetBodyRoot()),
"ParentRoot", hexutil.Encode(blob.GetSignedBlockHeader().GetHeader().GetParentRoot()),
)
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand why the slog logging is required, this information is duplicate with the evt.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not necessarily "required", but helps a little in terms of debugging and showing what's coming from those topics.

Removed it for now, and if we need them in the future, we can add back with Debug or Trace level logging.

cortze
cortze approved these changes May 16, 2024
View reviewed changes
Copy link
Contributor

@cortze cortze left a comment

Choose a reason for hiding this comment

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

LGTM! many thanks @0x00101010 for the PR!

guillaumemichel
guillaumemichel approved these changes May 17, 2024
View reviewed changes
Copy link
Contributor

@guillaumemichel guillaumemichel left a comment

Choose a reason for hiding this comment

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

Thanks @0x00101010 for your contribution!

@guillaumemichel guillaumemichel merged commit b3e91a8 into probe-lab:main May 17, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cortze cortze cortze approved these changes

@guillaumemichel guillaumemichel guillaumemichel approved these changes

@Savid Savid Savid approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@0x00101010 @codecov-commenter @Savid @guillaumemichel @cortze