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

Check for topic when parsing deposit contract logs #1152

Merged
merged 1 commit into from
Mar 11, 2025

Conversation

gumb0
Copy link
Member

@gumb0 gumb0 commented Mar 5, 2025

Turns out on Sepolia deposit contract is different and can emit other logs (ERC20's Transfer)

@gumb0 gumb0 force-pushed the fix-deposit-requests branch 3 times, most recently from 1740931 to 14f4a2b Compare March 5, 2025 10:37
@shemnon
Copy link

shemnon commented Mar 5, 2025

LGTM. This is how all the other clients approach it.

Copy link
Member

@chfast chfast left a comment

Choose a reason for hiding this comment

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

Can we have a regression test for this?

@gumb0 gumb0 added the Prague Changes for Prague upgrade label Mar 5, 2025
Copy link

codecov bot commented Mar 5, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.54%. Comparing base (4d363c3) to head (e9ba681).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1152   +/-   ##
=======================================
  Coverage   94.54%   94.54%           
=======================================
  Files         168      168           
  Lines       18246    18256   +10     
=======================================
+ Hits        17251    17261   +10     
  Misses        995      995           
Flag Coverage Δ
eof_execution_spec_tests 22.46% <16.66%> (-0.01%) ⬇️
ethereum_tests 28.28% <16.66%> (-0.02%) ⬇️
ethereum_tests_silkpre 20.97% <0.00%> (-0.02%) ⬇️
execution_spec_tests 20.95% <25.00%> (-0.01%) ⬇️
unittests 89.82% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
test/state/requests.cpp 96.66% <100.00%> (+0.23%) ⬆️
test/state/requests.hpp 90.00% <ø> (ø)
test/unittests/state_deposit_requests_test.cpp 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Member

@chfast chfast left a comment

Choose a reason for hiding this comment

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

Rebase on top of #1159 and add a regression test.

Verified

This commit was signed with the committer’s verified signature.
gumb0 Andrei Maiboroda
@gumb0 gumb0 force-pushed the fix-deposit-requests branch from 14f4a2b to e9ba681 Compare March 11, 2025 11:52
@gumb0 gumb0 requested a review from chfast March 11, 2025 12:57
@chfast chfast merged commit f9f8d51 into master Mar 11, 2025
25 checks passed
@chfast chfast deleted the fix-deposit-requests branch March 11, 2025 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Prague Changes for Prague upgrade
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants