eth, les, params: address comments

eth/backend.go

@@ -204,9 +204,9 @@ func New(ctx *node.ServiceContext, config *Config) (*Ethereum, error) {
 	cacheLimit := cacheConfig.TrieCleanLimit + cacheConfig.TrieDirtyLimit
 	checkpoint := config.Checkpoint
 	if checkpoint == nil {
-		checkpoint, _ = params.TrustedCheckpoints[genesisHash]
+		checkpoint = params.TrustedCheckpoints[genesisHash]
 	}
-	if eth.protocolManager, err = NewProtocolManager(chainConfig, config.Checkpoint, config.SyncMode, config.NetworkId, eth.eventMux, eth.txPool, eth.engine, eth.blockchain, chainDb, cacheLimit, config.Whitelist); err != nil {
+	if eth.protocolManager, err = NewProtocolManager(chainConfig, checkpoint, config.SyncMode, config.NetworkId, eth.eventMux, eth.txPool, eth.engine, eth.blockchain, chainDb, cacheLimit, config.Whitelist); err != nil {
 		return nil, err
 	}
 	eth.miner = miner.New(eth, &config.Miner, chainConfig, eth.EventMux(), eth.engine, eth.isLocalBlock)

les/backend.go

@@ -126,7 +126,7 @@ func New(ctx *node.ServiceContext, config *eth.Config) (*LightEthereum, error) {
 
 	checkpoint := config.Checkpoint
 	if checkpoint == nil {
-		checkpoint, _ = params.TrustedCheckpoints[genesisHash]
+		checkpoint = params.TrustedCheckpoints[genesisHash]
 	}
 	// Note: NewLightChain adds the trusted checkpoint so it needs an ODR with
 	// indexers already set but not started yet
@@ -156,7 +156,7 @@ func New(ctx *node.ServiceContext, config *eth.Config) (*LightEthereum, error) {
 
 	oracle := config.CheckpointOracle
 	if oracle == nil {
-		oracle, _ = params.CheckpointOracles[genesisHash]
+		oracle = params.CheckpointOracles[genesisHash]
 	}
 	registrar := newCheckpointRegistrar(oracle, leth.getLocalCheckpoint)
 	if leth.protocolManager, err = NewProtocolManager(leth.chainConfig, checkpoint, light.DefaultClientIndexerConfig, config.ULC, true, config.NetworkId, leth.eventMux, leth.peers, leth.blockchain, nil, chainDb, leth.odr, leth.serverPool, registrar, quitSync, &leth.wg, nil); err != nil {

les/server.go

@@ -122,7 +122,7 @@ func NewLesServer(e *eth.Ethereum, config *eth.Config) (*LesServer, error) {
 
 	oracle := config.CheckpointOracle
 	if oracle == nil {
-		oracle, _ = params.CheckpointOracles[e.BlockChain().Genesis().Hash()]
+		oracle = params.CheckpointOracles[e.BlockChain().Genesis().Hash()]
 	}
 	registrar := newCheckpointRegistrar(oracle, srv.getLocalCheckpoint)
 	// TODO(rjl493456442) Checkpoint is useless for les server, separate handler for client and server.

les/sync.go

@@ -111,7 +111,7 @@ func (pm *ProtocolManager) validateCheckpoint(peer *peer) error {
 	if !valid {
 		return errInvalidCheckpoint
 	}
-	log.Warn("Verifed advertised checkpoint", "peer", peer.id, "signers", len(signers))
+	log.Warn("Verified advertised checkpoint", "peer", peer.id, "signers", len(signers))
 	return nil
 }
 
@@ -140,7 +140,7 @@ func (pm *ProtocolManager) synchronise(peer *peer) {
 	//     => Use provided checkpoint
 	var checkpoint = &peer.checkpoint
 	var hardcoded bool
-	if pm.checkpoint != nil && pm.checkpoint.SectionIndex > peer.checkpoint.SectionIndex {
+	if pm.checkpoint != nil && pm.checkpoint.SectionIndex >= peer.checkpoint.SectionIndex {
 		checkpoint = pm.checkpoint // Use the hardcoded one.
 		hardcoded = true
 	}

les/sync_test.go

@@ -17,7 +17,7 @@
 package les
 
 import (
-	"errors"
+	"fmt"
 	"math/big"
 	"testing"
 	"time"
@@ -28,9 +28,21 @@ import (
 	"github.com/ethereum/go-ethereum/params"
 )
 
-func TestCheckpointSyncingLes2(t *testing.T) { testCheckpointSyncing(t, 2) }
+// Test light syncing which will download all headers from genesis.
+func TestLightSyncingLes2(t *testing.T) { testCheckpointSyncing(t, 2, 0) }
+func TestLightSyncingLes3(t *testing.T) { testCheckpointSyncing(t, 3, 0) }
 
-func testCheckpointSyncing(t *testing.T, protocol int) {
+// Test legacy checkpoint syncing which will download tail headers
+// based on a hardcoded checkpoint.
+func TestLegacyCheckpointSyncingLes2(t *testing.T) { testCheckpointSyncing(t, 2, 1) }
+func TestLegacyCheckpointSyncingLes3(t *testing.T) { testCheckpointSyncing(t, 3, 1) }
+
+// Test checkpoint syncing which will download tail headers based
+// on a verified checkpoint.
+func TestCheckpointSyncingLes2(t *testing.T) { testCheckpointSyncing(t, 2, 2) }
+func TestCheckpointSyncingLes3(t *testing.T) { testCheckpointSyncing(t, 3, 2) }
+
+func testCheckpointSyncing(t *testing.T, protocol int, syncMode int) {
 	config := light.TestServerIndexerConfig
 
 	waitIndexers := func(cIndexer, bIndexer, btIndexer *core.ChainIndexer) {
@@ -47,17 +59,24 @@ func testCheckpointSyncing(t *testing.T, protocol int) {
 	server, client, tearDown := newClientServerEnv(t, int(config.ChtSize+config.ChtConfirms), protocol, waitIndexers, false)
 	defer tearDown()
 
-	// Register checkpoint 0 into the contract at block (512+4)+1
+	expected := config.ChtSize + config.ChtConfirms
-	s, _, head := server.chtIndexer.Sections()
-	chtRoot := light.GetChtRoot(server.db, s-1, head)
-	btRoot := light.GetBloomTrieRoot(server.db, s-1, head)
 
+	// Checkpoint syncing or legacy checkpoint syncing.
+	if syncMode == 1 || syncMode == 2 {
+		// Assemble checkpoint 0
+		s, _, head := server.chtIndexer.Sections()
 		cp := &params.TrustedCheckpoint{
 			SectionIndex: 0,
 			SectionHead:  head,
-		CHTRoot:      chtRoot,
+			CHTRoot:      light.GetChtRoot(server.db, s-1, head),
-		BloomRoot:    btRoot,
+			BloomRoot:    light.GetBloomTrieRoot(server.db, s-1, head),
 		}
+		if syncMode == 1 {
+			// Register the assembled checkpoint as hardcoded one.
+			client.pm.checkpoint = cp
+			client.pm.blockchain.(*light.LightChain).AddTrustedCheckpoint(cp)
+		} else {
+			// Register the assembled checkpoint into oracle.
 			header := server.backend.Blockchain().CurrentHeader()
 
 			data := append([]byte{0x19, 0x00}, append(registrarAddr.Bytes(), append([]byte{0, 0, 0, 0, 0, 0, 0, 0}, cp.Hash().Bytes()...)...)...)
@@ -67,7 +86,6 @@ func testCheckpointSyncing(t *testing.T, protocol int) {
 				t.Error("register checkpoint failed", err)
 			}
 			server.backend.Commit()
-	server.backend.Commit() // Inject an empty block
 
 			// Wait for the checkpoint registration
 			for {
@@ -78,14 +96,17 @@ func testCheckpointSyncing(t *testing.T, protocol int) {
 				}
 				break
 			}
+			expected += 1
+		}
+	}
 
 	done := make(chan error)
 	client.pm.reg.syncDoneHook = func() {
 		header := client.pm.blockchain.CurrentHeader()
-		if header.Number.Uint64() == config.ChtSize+config.ChtConfirms+2 {
+		if header.Number.Uint64() == expected {
 			done <- nil
 		} else {
-			done <- errors.New("blockchain mismatch")
+			done <- fmt.Errorf("blockchain length mismatch, want %d, got %d", expected, header.Number)
 		}
 	}
 

params/config.go

@@ -45,7 +45,7 @@ var TrustedCheckpoints = map[common.Hash]*TrustedCheckpoint{
 // CheckpointOracles associates each known checkpoint oracles with the genesis hash of
 // the chain it belongs to.
 var CheckpointOracles = map[common.Hash]*CheckpointOracleConfig{
-	RinkebyGenesisHash: RinkebyCheckpointOracleConfig,
+	RinkebyGenesisHash: RinkebyCheckpointOracle,
 }
 
 var (
@@ -124,8 +124,8 @@ var (
 		BloomRoot:    common.HexToHash("0xa3048fe8b7e30f77f11bc755a88478363d7d3e71c2bdfe4e8ab9e269cd804ba2"),
 	}
 
-	// RinkebyCheckpointOracleConfig contains a set of configs for the Rinkeby test network oracle.
+	// RinkebyCheckpointOracle contains a set of configs for the Rinkeby test network oracle.
-	RinkebyCheckpointOracleConfig = &CheckpointOracleConfig{
+	RinkebyCheckpointOracle = &CheckpointOracleConfig{
 		Address: common.HexToAddress("0x62652ed8e969ce7bd5e3dd13590efa1e569215f1"),
 		Signers: []common.Address{
 			common.HexToAddress("0xd9c9cd5f6779558b6e0ed4e6acf6b1947e7fa1f3"), // Peter