Skip to content

Conversation

@jwasinger
Copy link
Contributor

@jwasinger jwasinger commented Oct 16, 2024

rebased #29766 . The downstream branch appears to have been deleted and I don't have perms to push to that fork.

TerminalTotalDifficultyPassed is removed. TerminalTotalDifficulty must now be non-nil, and it is expected that networks are already merged: we can only import PoW/Clique chains, not produce blocks on them.

@stevemilk
Copy link
Contributor

hi, I didn't notice you create one too. I just re-commit the PR #30612, including more deletion of TerminalTotalDifficultyPassed.
you can choose rebase that one or not.

@stevemilk
Copy link
Contributor

your commit contains my updates, closing mine.

@jwasinger jwasinger marked this pull request as ready for review October 16, 2024 13:35
Copy link

@namiloh namiloh left a comment

Choose a reason for hiding this comment

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

Looks good to me, let's merge if CI is green IMO

if t := cfg.ChainConfig.ShanghaiTime; cfg.ChainConfig.TerminalTotalDifficultyPassed || (t != nil && *t == 0) {
cfg.Random = &(common.Hash{})
}
cfg.Random = &(common.Hash{})
Copy link
Member

Choose a reason for hiding this comment

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

The random is only set if the Shanghai fork is configured. The behavior is changed

Copy link
Member

Choose a reason for hiding this comment

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

EDIT. Yeah, since you always expect the TTD is configured and it implies the Shanghai has already be configured (no matter is activated or not).

}
copy(gspec.ExtraData[32:], addr[:])

// chain_maker has no blockchain to retrieve the TTD from, setting to nil
Copy link
Member

Choose a reason for hiding this comment

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

I think you can delete all the pre-merge tests

Copy link
Contributor

Choose a reason for hiding this comment

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

We're still supposed to be able to verify backwards across a merge, right? In that case, I think we should leave it in

Copy link
Contributor

@holiman holiman left a comment

Choose a reason for hiding this comment

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

LGTM

@holiman holiman added this to the 1.14.12 milestone Oct 23, 2024
@holiman holiman merged commit 478012a into ethereum:master Oct 23, 2024
3 checks passed
@jwasinger jwasinger deleted the remove-ttd-passed branch October 23, 2024 10:13
holiman pushed a commit that referenced this pull request Nov 19, 2024
rebased #29766 . The
downstream branch appears to have been deleted and I don't have perms to
push to that fork.

`TerminalTotalDifficultyPassed` is removed. `TerminalTotalDifficulty`
must now be non-nil, and it is expected that networks are already
merged: we can only import PoW/Clique chains, not produce blocks on
them.

---------

Co-authored-by: stevemilk <[email protected]>
jakub-freebit pushed a commit to fblch/go-ethereum that referenced this pull request Jul 3, 2025
rebased ethereum#29766 . The
downstream branch appears to have been deleted and I don't have perms to
push to that fork.

`TerminalTotalDifficultyPassed` is removed. `TerminalTotalDifficulty`
must now be non-nil, and it is expected that networks are already
merged: we can only import PoW/Clique chains, not produce blocks on
them.

---------

Co-authored-by: stevemilk <[email protected]>
gballet pushed a commit to gballet/go-ethereum that referenced this pull request Sep 11, 2025
rebased ethereum#29766 . The
downstream branch appears to have been deleted and I don't have perms to
push to that fork.

`TerminalTotalDifficultyPassed` is removed. `TerminalTotalDifficulty`
must now be non-nil, and it is expected that networks are already
merged: we can only import PoW/Clique chains, not produce blocks on
them.

---------

Co-authored-by: stevemilk <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants