Skip to content

Conversation

@joshieDo
Copy link
Collaborator

@joshieDo joshieDo commented Oct 20, 2025

  • removes total difficulty provider methods from StorageApi
  • adds era_utils::calculate_td_by_number so we are able to generate era files with the same checksums. slightly expensive, since it has to iterate through all headers up to num.
  • static_file_provider.append_header now always sets TTD column to zero. This has the side-effect that static files will have a different checksum after this PR.

closes #16586

@github-project-automation github-project-automation bot moved this to Backlog in Reth Tracker Oct 20, 2025
@joshieDo joshieDo self-assigned this Oct 20, 2025
@joshieDo joshieDo added C-debt A clean up/refactor of existing code A-db Related to the database labels Oct 20, 2025
hash,
difficulty: header.difficulty(),
total_difficulty,
total_difficulty: U256::ZERO,
Copy link
Collaborator Author

@joshieDo joshieDo Oct 20, 2025

Choose a reason for hiding this comment

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

im guessing so, but is this okay?

Copy link
Member

@Rjected Rjected Oct 20, 2025

Choose a reason for hiding this comment

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

I think this should be fine, in p2p this is deprecated as of eth/69 and the node builder only uses the Head to get the timestamp

@joshieDo joshieDo changed the title chore: remove TTD from HeaderProvider chore: remove total from HeaderProvider Oct 20, 2025
@joshieDo joshieDo changed the title chore: remove total from HeaderProvider chore: remove total difficulty from HeaderProvider Oct 20, 2025
@joshieDo joshieDo marked this pull request as ready for review October 20, 2025 19:26
Copy link
Member

@Rjected Rjected left a comment

Choose a reason for hiding this comment

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

just a q about simplifying append_header args otherwise lgtm


// Append to Headers segment
writer.append_header(header, td, header_hash)?;
writer.append_header(header, U256::ZERO, header_hash)?;
Copy link
Member

Choose a reason for hiding this comment

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

do we ever call this with something nonzero after this PR? if not then maybe we should just remove the arg?

Copy link
Collaborator

Choose a reason for hiding this comment

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

yeah I think we should remove this from the function arg entirely

hash,
difficulty: header.difficulty(),
total_difficulty,
total_difficulty: U256::ZERO,
Copy link
Member

@Rjected Rjected Oct 20, 2025

Choose a reason for hiding this comment

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

I think this should be fine, in p2p this is deprecated as of eth/69 and the node builder only uses the Head to get the timestamp

Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

very nice, I only have the same pedantic suggestion as @Rjected

imo it would be fine to always use zero for the export as well but we can keep this


// Append to Headers segment
writer.append_header(header, td, header_hash)?;
writer.append_header(header, U256::ZERO, header_hash)?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

yeah I think we should remove this from the function arg entirely

@github-project-automation github-project-automation bot moved this from Backlog to In Progress in Reth Tracker Oct 21, 2025
@joshieDo joshieDo requested a review from mattsse October 21, 2025 10:38
Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

lgtm!

@joshieDo joshieDo added this pull request to the merge queue Oct 21, 2025
Merged via the queue into main with commit e210483 Oct 21, 2025
42 checks passed
@joshieDo joshieDo deleted the joshie/header-provider-bye-ttd branch October 21, 2025 11:12
@github-project-automation github-project-automation bot moved this from In Progress to Done in Reth Tracker Oct 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-db Related to the database C-debt A clean up/refactor of existing code

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

drop support for total difficulty table entirely

4 participants