Skip to content
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.

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

all: remove notion of trusted checkpoints in the post-merge world #27147

Merged
merged 4 commits into from
Apr 24, 2023

Conversation

karalabe
Copy link
Member

Prior to the merge, we used trusted checkpoints for a few things: to have LES sync from a relatively recent starting point; and to have ETH challenge the remote side for a relatively recent block to ensure they are on the same chain / synced.

The merge made these checkpoints a bit moot. Since we need a beacon client to function (or the new LES might do it on the beacon network), the starting point and or trustedness originates from the beacon chain, not a hard coded local value. That use case is non-existent any more.

For the ETH challenge dropping the CHTs is a tiny bit of a step back as we have no challenge now to cross check the remote side for synced-ness, but since we haven't updated the CHTs for half a year now, it's not like we're losing much on that front. Here we might want to introduce a new challenge during snap sync which could cross check what we receive in our local beacon client (say latest - a few blocks) with a remote peer and use that as a synced-ness flag (or chain challenge). Either way, we need something that's automatic based on all the information we have from our beacon client and not something hard coded.

@karalabe karalabe added this to the 1.11.7 milestone Apr 22, 2023
@rjl493456442 rjl493456442 self-assigned this Apr 23, 2023
return
}
}

Copy link
Member

Choose a reason for hiding this comment

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

Just to clarity, light client has to sync headers since genesis by running light sync.
But light client is not working anyway, I think it's fine.

handler.fetcher = newLightFetcher(backend.blockchain, backend.engine, backend.peers, handler.ulc, backend.chainDb, backend.reqDist, handler.synchronise)
handler.downloader = downloader.New(height, backend.chainDb, backend.eventMux, nil, backend.blockchain, handler.removePeer)
handler.downloader = downloader.New(0, backend.chainDb, backend.eventMux, nil, backend.blockchain, handler.removePeer)
Copy link
Member

Choose a reason for hiding this comment

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

The notion of checkpoint is still kept in LES. But it's fine seems the entire downloader in les package should be removed.

Copy link
Member

@rjl493456442 rjl493456442 left a comment

Choose a reason for hiding this comment

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

Pushed two more commits on top by removing more unused stuffs, lgtm!

@karalabe karalabe merged commit 1e556d2 into ethereum:master Apr 24, 2023
shekhirin pushed a commit to shekhirin/go-ethereum that referenced this pull request Jun 6, 2023
…hereum#27147)

* all: remove notion of trusted checkpoints in the post-merge world

* light: remove unused function

* eth/ethconfig, les: remove unused config option

* les: make linter happy

---------

Co-authored-by: Gary Rong <garyrong0905@gmail.com>
brilliant-lx added a commit to bnb-chain/bsc that referenced this pull request Aug 30, 2023
TestCheckpointChallenge was removed in:
ethereum/go-ethereum#27147
since after merge, it is useless for ethereum, but might be useful for BSC.
disable the case right now, as it is not a big issue.
brilliant-lx pushed a commit to bnb-chain/bsc that referenced this pull request Sep 7, 2023
* fix: crash of highestVerifiedHeader

* fix: panic of blobpool

* fix: genesis set up

* 1. modify NewDatabaseWithNodeDB to upstream
2. fix race use of hasher in statedb
3. fix use wrong value when updateTrie

* fix dir legacypool

* fix dir blobpool

* fix dir vote

* remove diffsync related code

* fix core/state/snapshot

* disable pipeCommit for now

* fix applyTransaction for bloom setting

* CI: fast finality in gasprice test

* CI: diffFetcher was removed

* CI: downloader, remove beaconsync test

* CI: no beaconsync in downloader, remove a failed case

TestCheckpointChallenge was removed in:
ethereum/go-ethereum#27147
since after merge, it is useless for ethereum, but might be useful for BSC.
disable the case right now, as it is not a big issue.

* CI: bsc protocol decHandlers

* CI: receipt Bloom process

* 1. skip CheckConfigForkOrder for non-parlia engine
2. all test cases in core work well now
	cd core && go test ./... -v

* fix test cases in trie dir

* CI: no beaconsync in downloader, remove a failed case(redo)

* fix dir miner

* fix dir cmd/geth

* CI: filter test, BaseFee & Finality

* fix dir graphql

* remove diffStore

* fix ethclient

* fix TestRPCGetTransactionReceipt

* fix dir internal

* ut add dir ethstats and signer

* disable pipeCommit thoroughly; fix concurrent map iteration and map write in statedb

* CI: fix snap sync

it could be changed by mistake

* fix tests/Run to generate snapshot

* prepare for merge

* remove useless

* use common hasher in getDeletedStateObject, no race here

* an critical comment for state.Prepare

* do not copy nil accessList

* add omitempty tag for unused new fields of core.Genesis

* remove totalFees

* calculate fees before FinalizeAndAssemble

* revert interface Finalize of consensus

* do not double gas limit upon london block

* use Leveldb as default

* Revert "remove diffStore"

This reverts commit df343b1.

* Revert "remove diffsync related code"

This reverts commit 8d84b81.

* compile pass after revert

* remove diffsync

* fix dir eth/protocols/trust

* fix TestFastNode

* decHandlers for trust protocol

* keep persist diff in test
devopsbo3 pushed a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
…hereum#27147)

* all: remove notion of trusted checkpoints in the post-merge world

* light: remove unused function

* eth/ethconfig, les: remove unused config option

* les: make linter happy

---------

Co-authored-by: Gary Rong <garyrong0905@gmail.com>
devopsbo3 added a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
devopsbo3 added a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
lgray added a commit to lgray/chainindex that referenced this pull request Dec 31, 2023
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.

2 participants