Skip to content

Check for topic when parsing deposit contract logs#1152

Merged
chfast merged 1 commit into
masterfrom
fix-deposit-requests
Mar 11, 2025
Merged

Check for topic when parsing deposit contract logs#1152
chfast merged 1 commit into
masterfrom
fix-deposit-requests

Conversation

@gumb0

@gumb0 gumb0 commented Mar 5, 2025

Copy link
Copy Markdown
Member

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

shemnon commented Mar 5, 2025

Copy link
Copy Markdown

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

@chfast chfast left a comment

Copy link
Copy Markdown
Member

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
@codecov

codecov Bot commented Mar 5, 2025

Copy link
Copy Markdown

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.

@chfast chfast left a comment

Copy link
Copy Markdown
Member

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.

Comment thread test/state/requests.cpp Outdated
Comment thread test/state/requests.cpp Outdated
Comment thread test/state/requests.cpp Outdated
@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
@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.

3 participants