fix: drop support for total difficulty table #16660
Conversation
i-m-aditya
force-pushed
the
feat/drop-support-for-difficulty-table
branch
from
6103eae to
7d41def
Compare
i-m-aditya
requested review from
gakonst,
joshieDo,
onbjerg,
rakita,
rkrasiuk and
shekhirin
as code owners
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| Action::InsertCanonicalHeaders => self.insert_canonical_headers.record(duration), | ||
| Action::InsertHeaders => self.insert_headers.record(duration), | ||
| Action::InsertHeaderNumbers => self.insert_header_numbers.record(duration), | ||
| #[expect(deprecated)] |
There was a problem hiding this comment.
The #[expect(deprecated)] attribute is incorrect here - it should be #[allow(deprecated)] to properly suppress the deprecation warning. The expect attribute is used for different purposes in Rust (typically with #[expect(clippy::lint_name)] to acknowledge a clippy lint).
| #[expect(deprecated)] | |
| #[allow(deprecated)] |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
|
@mattsse quick follow-up on this — would love your input! |
mattsse
left a comment
mattsse
left a comment
There was a problem hiding this comment.
this is great!
taking this for a spin, changes all look good.
mattsse
left a comment
mattsse
left a comment
There was a problem hiding this comment.
okay, I realized we need to do a few more things first
| } else { | ||
| tx.put::<tables::CanonicalHeaders>(header.number, header.hash())?; | ||
| tx.put::<tables::HeaderTerminalDifficulties>(header.number, td.into())?; | ||
| // Note: HeaderTerminalDifficulties table is read-only in database after |
There was a problem hiding this comment.
for future prs,
these docs are totally meaningless because the code doesnt have any references to HeaderTerminalDifficulties so after this is merged this note will just be confusing.
There was a problem hiding this comment.
@mattsse Could you clarify how this approach would work if we attempt to backfill post-merge ranges from the live DB? Wouldn’t we encounter missing HTD values, given we don’t store them anymore post-merge?
github-actions
bot
added
the
S-stale
mattsse
added
M-prevent-stale
|
reopened this because this is still planned but still blocked |
|
superseded by #19151 very sorry about this @i-m-aditya |
|
actually @joshieDo we still need something from here |
|
ty @i-m-aditya ! |
4 participants
fixing: #16586
This PR refactors the handling of the HeaderTerminalDifficulties table to align with our post Paris/Merge policy. Key changes include: