Skip to content

fix: import should count on the delta #17819

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
Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

fix: import should count on the delta #17819

wants to merge 4 commits into from

Conversation

tmelhao
Copy link
Contributor

@tmelhao tmelhao commented Aug 12, 2025

We're using the count in table to count the imported blocks, this is incorrect, we should use the delta count to check.

@github-project-automation github-project-automation bot moved this to Backlog in Reth Tracker Aug 12, 2025
@tmelhao tmelhao force-pushed the db-import-use-delta branch from ba36e87 to 70caa5a Compare August 12, 2025 21:39
@tmelhao tmelhao requested a review from Rjected as a code owner August 13, 2025 00:01
@tmelhao
Copy link
Contributor Author

tmelhao commented Aug 13, 2025

@mattsse @Rjected please take a look, i'm not sure why the test failed, seems not related

stdout ───

running 1 test

stderr ───

thread 'test_local_rpc_tests_compat' has overflowed its stack
fatal runtime error: stack overflow, aborting

(test aborted with signal 6: SIGABRT)

DELAY 2/3 by 1.196s reth-rpc-e2e-tests::e2e_testsuite test_local_rpc_tests_compat
RETRY 2/3 [ ] reth-rpc-e2e-tests::e2e_testsuite test_local_rpc_tests_compat
TRY 2 ABRT [ 0.728s] reth-rpc-e2e-tests::e2e_testsuite test_local_rpc_tests_compat
stdout ───

running 1 test

stderr ───

thread 'test_local_rpc_tests_compat' has overflowed its stack
fatal runtime error: stack overflow, aborting

@DaniPopes
Copy link
Member

Taking a look, I can reproduce with the exact command that CI runs and stable rust toolchain, but nothing else

@DaniPopes DaniPopes mentioned this pull request Aug 13, 2025
Merged
tmel added 4 commits August 14, 2025 15:07
fix: use delta to check the imported blocks
a7945a5 
Signed-off-by: tmel <[email protected]>
reuse provider
f8903cf 
Signed-off-by: tmel <[email protected]>
finish it
9c34e8c 
Signed-off-by: tmel <[email protected]>
fix test
f3a57a2 
Signed-off-by: tmelhao <[email protected]>
@tmelhao tmelhao force-pushed the db-import-use-delta branch from 9d370fc to f3a57a2 Compare August 14, 2025 07:09
@tmelhao
Copy link
Contributor Author

tmelhao commented Aug 14, 2025

@DaniPopes please take another look, this PR is to fix the import count error

@tmelhao
Copy link
Contributor Author

tmelhao commented Aug 15, 2025

@mattsse ping

DaniPopes
DaniPopes approved these changes Aug 15, 2025
View reviewed changes
Copy link
Member

@DaniPopes DaniPopes left a comment

Choose a reason for hiding this comment

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

Makes sense, leaving to @mattsse

@github-project-automation github-project-automation bot moved this from Backlog to In Progress in Reth Tracker Aug 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DaniPopes DaniPopes DaniPopes approved these changes

@onbjerg onbjerg Awaiting requested review from onbjerg

@mattsse mattsse Awaiting requested review from mattsse mattsse is a code owner

@Rjected Rjected Awaiting requested review from Rjected Rjected is a code owner

Assignees
No one assigned
Labels
None yet
Projects
Reth Tracker
Status: In Progress
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@tmelhao @DaniPopes