Skip to content

Commit

Permalink
fix: lint, add: milestone related tests in downloader
Browse files Browse the repository at this point in the history
  • Loading branch information
anshalshukla committed Sep 5, 2024
1 parent ef88b4d commit a66305d
Show file tree
Hide file tree
Showing 26 changed files with 122 additions and 69 deletions.
3 changes: 1 addition & 2 deletions accounts/abi/bind/backends/simulated_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ import (
"github.com/ethereum/go-ethereum/accounts/abi/bind"
"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/common/leak"
"github.com/ethereum/go-ethereum/core"
"github.com/ethereum/go-ethereum/core/types"
"github.com/ethereum/go-ethereum/crypto"
"github.com/ethereum/go-ethereum/params"
Expand All @@ -47,7 +46,7 @@ func TestSimulatedBackend(t *testing.T) {
key, _ := crypto.GenerateKey() // nolint: gosec
auth, _ := bind.NewKeyedTransactorWithChainID(key, big.NewInt(1337))
genAlloc := make(types.GenesisAlloc)
genAlloc[auth.From] = core.GenesisAccount{Balance: big.NewInt(9223372036854775807)}
genAlloc[auth.From] = types.Account{Balance: big.NewInt(9223372036854775807)}

sim := NewSimulatedBackend(genAlloc, gasLimit)
defer sim.Close()
Expand Down
3 changes: 0 additions & 3 deletions cmd/devp2p/internal/ethtest/snap.go
Original file line number Diff line number Diff line change
Expand Up @@ -602,7 +602,6 @@ func hexToCompact(hex []byte) []byte {

// TestSnapTrieNodes various forms of GetTrieNodes requests.
func (s *Suite) TestSnapTrieNodes(t *utesting.T) {

var (
// This is the known address of the snap storage testing contract.
storageAcct = common.HexToAddress("0x8bebc8ba651aee624937e7d897853ac30c95a067")
Expand Down Expand Up @@ -867,13 +866,11 @@ func (s *Suite) snapGetStorageRanges(t *utesting.T, tc *stRangesTest) error {

res, ok := msg.(*snap.StorageRangesPacket)
if !ok {

return fmt.Errorf("account range response wrong: %T %v", msg, msg)
}

// Ensure the ranges are monotonically increasing
for i, slots := range res.Slots {

for j := 1; j < len(slots); j++ {
if bytes.Compare(slots[j-1].Hash[:], slots[j].Hash[:]) >= 0 {
return fmt.Errorf("storage slots not monotonically increasing for account #%d: #%d [%x] vs #%d [%x]", i, j-1, slots[j-1].Hash[:], j, slots[j].Hash[:])
Expand Down
2 changes: 1 addition & 1 deletion consensus/bor/bor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func TestGenesisContractChange(t *testing.T) {
}

genspec := &core.Genesis{
Alloc: map[common.Address]core.GenesisAccount{
Alloc: map[common.Address]types.Account{
addr0: {
Balance: big.NewInt(0),
Code: []byte{0x1, 0x1},
Expand Down
2 changes: 1 addition & 1 deletion consensus/bor/heimdallgrpc/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func NewHeimdallGRPCClient(address string) *HeimdallGRPCClient {
grpc_retry.WithCodes(codes.Internal, codes.Unavailable, codes.Aborted, codes.NotFound),
}

conn, err := grpc.Dial(address,
conn, err := grpc.NewClient(address,
grpc.WithStreamInterceptor(grpc_retry.StreamClientInterceptor(opts...)),
grpc.WithUnaryInterceptor(grpc_retry.UnaryClientInterceptor(opts...)),
grpc.WithTransportCredentials(insecure.NewCredentials()),
Expand Down
4 changes: 2 additions & 2 deletions core/blockchain.go
Original file line number Diff line number Diff line change
Expand Up @@ -1659,7 +1659,6 @@ func (bc *BlockChain) InsertReceiptChain(blockChain types.Blocks, receiptChain [

// writeLive writes blockchain and corresponding receipt chain into active store.
writeLive := func(blockChain types.Blocks, receiptChain []types.Receipts) (int, error) {

headers := make([]*types.Header, 0, len(blockChain))
var (
skipPresenceCheck = false
Expand Down Expand Up @@ -2520,7 +2519,7 @@ func (bc *BlockChain) insertChain(chain types.Blocks, setHead bool) (int, error)

// blockProcessingResult is a summary of block processing
// used for updating the stats.
// nolint
// nolint : unused
type blockProcessingResult struct {
usedGas uint64
procTime time.Duration
Expand All @@ -2529,6 +2528,7 @@ type blockProcessingResult struct {

// processBlock executes and validates the given block. If there was no error
// it writes the block and associated state to database.
// nolint : unused
func (bc *BlockChain) processBlock(block *types.Block, statedb *state.StateDB, start time.Time, setHead bool) (_ *blockProcessingResult, blockEndErr error) {
if bc.logger != nil && bc.logger.OnBlockStart != nil {
td := bc.GetTd(block.ParentHash(), block.NumberU64()-1)
Expand Down
2 changes: 2 additions & 0 deletions core/parallel_state_processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,8 @@ func (p *ParallelStateProcessor) Process(block *types.Block, statedb *state.Stat
shouldDelayFeeCal = false

statedb.StopPrefetcher()

// nolint
*statedb = *backupStateDB

allLogs = []*types.Log{}
Expand Down
2 changes: 0 additions & 2 deletions core/rawdb/chain_iterator.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,6 @@ func iterateTransactions(db ethdb.Database, from uint64, to uint64, reverse bool
// There is a passed channel, the whole procedure will be interrupted if any
// signal received.
func indexTransactions(db ethdb.Database, from uint64, to uint64, interrupt chan struct{}, hook func(uint64) bool, report bool) {

// short circuit for invalid range
if from >= to {
return
Expand Down Expand Up @@ -310,7 +309,6 @@ func indexTransactionsForTesting(db ethdb.Database, from uint64, to uint64, inte
// There is a passed channel, the whole procedure will be interrupted if any
// signal received.
func unindexTransactions(db ethdb.Database, from uint64, to uint64, interrupt chan struct{}, hook func(uint64) bool, report bool) {

// short circuit for invalid range
if from >= to {
return
Expand Down
1 change: 1 addition & 0 deletions core/state_processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ func (p *StateProcessor) Process(block *types.Block, statedb *state.StateDB, cfg
return nil, nil, 0, fmt.Errorf("withdrawals before shanghai")
}
// Bor does not support withdrawals
// nolint
if withdrawals != nil {
withdrawals = nil
}
Expand Down
1 change: 0 additions & 1 deletion core/state_transition.go
Original file line number Diff line number Diff line change
Expand Up @@ -481,7 +481,6 @@ func (st *StateTransition) TransitionDb(interruptCtx context.Context) (*Executio
)

if contractCreation {

// nolint : contextcheck
ret, _, st.gasRemaining, vmerr = st.evm.Create(sender, msg.Data, st.gasRemaining, value)

Expand Down
1 change: 0 additions & 1 deletion eth/catalyst/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -483,7 +483,6 @@ func (api *ConsensusAPI) NewPayloadV2(params engine.ExecutableData) (engine.Payl
return engine.PayloadStatusV1{Status: engine.INVALID}, engine.InvalidParams.With(errors.New("nil withdrawals post-shanghai"))
}
} else {

if params.Withdrawals != nil {
return engine.PayloadStatusV1{Status: engine.INVALID}, engine.InvalidParams.With(errors.New("non-nil withdrawals pre-shanghai"))
}
Expand Down
14 changes: 6 additions & 8 deletions eth/downloader/bor_downloader.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,10 @@ var (
MaxSkeletonSize = 128 // Number of header fetches to need for a skeleton assembly
MaxReceiptFetch = 256 // Amount of transaction receipts to allow fetching per request

maxQueuedHeaders = 32 * 1024 // [eth/62] Maximum number of headers to queue for import (DOS protection)
maxHeadersProcess = 2048 // Number of header download results to import at once into the chain
maxResultsProcess = 2048 // Number of content download results to import at once into the chain
fullMaxForkAncestry uint64 = params.FullImmutabilityThreshold // Maximum chain reorganisation (locally redeclared so tests can reduce it)
lightMaxForkAncestry uint64 = params.LightImmutabilityThreshold // Maximum chain reorganisation (locally redeclared so tests can reduce it)
maxQueuedHeaders = 32 * 1024 // [eth/62] Maximum number of headers to queue for import (DOS protection)
maxHeadersProcess = 2048 // Number of header download results to import at once into the chain
maxResultsProcess = 2048 // Number of content download results to import at once into the chain
fullMaxForkAncestry uint64 = params.FullImmutabilityThreshold // Maximum chain reorganisation (locally redeclared so tests can reduce it)

reorgProtThreshold = 48 // Threshold number of recent blocks to disable mini reorg protection
reorgProtHeaderDelay = 2 // Number of headers to delay delivering to cover mini reorgs
Expand Down Expand Up @@ -102,9 +101,8 @@ type Downloader struct {
mode atomic.Uint32 // Synchronisation mode defining the strategy used (per sync cycle), use d.getMode() to get the SyncMode
mux *event.TypeMux // Event multiplexer to announce sync operation events

genesis uint64 // Genesis block number to limit sync to (e.g. light client CHT)
queue *queue // Scheduler for selecting the hashes to download
peers *peerSet // Set of active peers from which download can proceed
queue *queue // Scheduler for selecting the hashes to download
peers *peerSet // Set of active peers from which download can proceed

stateDB ethdb.Database // Database to state sync into (and deduplicate via)

Expand Down
92 changes: 92 additions & 0 deletions eth/downloader/bor_downloader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import (
"github.com/ethereum/go-ethereum/params"
"github.com/ethereum/go-ethereum/rlp"
"github.com/ethereum/go-ethereum/trie"
"github.com/stretchr/testify/assert"
)

// downloadTester is a test simulator for mocking out local block chain.
Expand Down Expand Up @@ -1475,3 +1476,94 @@ func (w *whitelistFake) RemoveMilestoneID(milestoneId string) {
func (w *whitelistFake) GetMilestoneIDsList() []string {
return nil
}

// TestFakedSyncProgress67WhitelistMatch tests if in case of whitelisted
// checkpoint match with opposite peer, the sync should succeed.
func TestFakedSyncProgress68WhitelistMatch(t *testing.T) {
t.Parallel()

protocol := uint(eth.ETH68)
mode := FullSync

tester := newTester(t)
validate := func(count int) (bool, error) {
return true, nil
}
tester.downloader.ChainValidator = newWhitelistFake(validate)

defer tester.terminate()

chainA := testChainForkLightA.blocks
tester.newPeer("light", protocol, chainA[1:])

// Synchronise with the peer and make sure all blocks were retrieved
if err := tester.sync("light", nil, mode); err != nil {
t.Fatal("succeeded attacker synchronisation")
}
}

// TestFakedSyncProgress67NoRemoteCheckpoint tests if in case of missing/invalid
// checkpointed blocks with opposite peer, the sync should fail initially but
// with the retry mechanism, it should succeed eventually.
func TestFakedSyncProgress68NoRemoteCheckpoint(t *testing.T) {
t.Parallel()

protocol := uint(eth.ETH68)
mode := FullSync

tester := newTester(t)
validate := func(count int) (bool, error) {
// only return the `ErrNoRemoteCheckpoint` error for the first call
if count == 0 {
return false, whitelist.ErrNoRemote
}

return true, nil
}

tester.downloader.ChainValidator = newWhitelistFake(validate)

defer tester.terminate()

chainA := testChainForkLightA.blocks
tester.newPeer("light", protocol, chainA[1:])

// Set the max validation threshold equal to chain length to enforce validation
tester.downloader.maxValidationThreshold = uint64(len(chainA) - 1)

// Synchronise with the peer and make sure all blocks were retrieved
// Should fail in first attempt
err := tester.sync("light", nil, mode)
assert.Equal(t, whitelist.ErrNoRemote, err, "failed synchronisation")

// Try syncing again, should succeed
if err := tester.sync("light", nil, mode); err != nil {
t.Fatal("succeeded attacker synchronisation")
}
}

// TestFakedSyncProgress67BypassWhitelistValidation tests if peer validation
// via whitelist is bypassed when remote peer is far away or not
func TestFakedSyncProgress68BypassWhitelistValidation(t *testing.T) {
protocol := uint(eth.ETH68)
mode := FullSync

tester := newTester(t)
validate := func(count int) (bool, error) {
return false, whitelist.ErrNoRemote
}

tester.downloader.ChainValidator = newWhitelistFake(validate)

defer tester.terminate()

// 1223 length chain
chainA := testChainBase.blocks
tester.newPeer("light", protocol, chainA[1:])

// Although the validate function above returns an error (which says that
// remote peer doesn't have that block), sync will go through as the chain
// import length is 1223 which is more than the default threshold of 1024
err := tester.sync("light", nil, mode)
assert.NoError(t, err, "failed synchronisation")
}
1 change: 1 addition & 0 deletions eth/downloader/queue.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ func newFetchResult(header *types.Header, fastSync bool) *fetchResult {
}

// body returns a representation of the fetch result as a types.Body object.
// nolint : unused
func (f *fetchResult) body() types.Body {
return types.Body{
Transactions: f.Transactions,
Expand Down
4 changes: 0 additions & 4 deletions eth/fetcher/tx_fetcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,6 @@ const (
// txGatherSlack is the interval used to collate almost-expired announces
// with network fetches.
txGatherSlack = 100 * time.Millisecond

// maxTxArrivalWait is the longest acceptable duration for the txArrivalWait
// configuration value. Longer config values will default to this.
maxTxArrivalWait = 500 * time.Millisecond
)

var (
Expand Down
11 changes: 0 additions & 11 deletions eth/fetcher/tx_fetcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2017,17 +2017,6 @@ func containsHashInAnnounces(slice []announce, hash common.Hash) bool {
return false
}

// containsHash returns whether a hash is contained within a hash slice.
func containsHash(slice []common.Hash, hash common.Hash) bool {
for _, have := range slice {
if have == hash {
return true
}
}

return false
}

// Tests that a transaction is forgotten after the timeout.
func TestTransactionForgotten(t *testing.T) {
fetcher := NewTxFetcher(
Expand Down
13 changes: 0 additions & 13 deletions eth/filters/filter_system.go
Original file line number Diff line number Diff line change
Expand Up @@ -427,19 +427,6 @@ func (es *EventSystem) handleLogs(filters filterIndex, ev []*types.Log) {
}
}

func (es *EventSystem) handlePendingLogs(filters filterIndex, ev []*types.Log) {
if len(ev) == 0 {
return
}

for _, f := range filters[PendingLogsSubscription] {
matchedLogs := filterLogs(ev, nil, f.logsCrit.ToBlock, f.logsCrit.Addresses, f.logsCrit.Topics)
if len(matchedLogs) > 0 {
f.logs <- matchedLogs
}
}
}

func (es *EventSystem) handleTxsEvent(filters filterIndex, ev core.NewTxsEvent) {
for _, f := range filters[PendingTransactionsSubscription] {
f.txs <- ev.Txs
Expand Down
9 changes: 0 additions & 9 deletions eth/filters/filter_system_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -708,12 +708,3 @@ func TestPendingTxFilterDeadlock(t *testing.T) {
}
}
}

func flattenLogs(pl [][]*types.Log) []*types.Log {
var logs []*types.Log
for _, l := range pl {
logs = append(logs, l...)
}

return logs
}
2 changes: 1 addition & 1 deletion internal/cli/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ func (m *Meta2) NewFlagSet(n string) *flagset.Flagset {
}

func (m *Meta2) Conn() (*grpc.ClientConn, error) {
conn, err := grpc.Dial(m.addr, grpc.WithTransportCredentials(insecure.NewCredentials()))
conn, err := grpc.NewClient(m.addr, grpc.WithTransportCredentials(insecure.NewCredentials()))
if err != nil {
return nil, fmt.Errorf("failed to connect to server: %v", err)
}
Expand Down
3 changes: 2 additions & 1 deletion internal/cli/server/chains/developer.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/core"
"github.com/ethereum/go-ethereum/core/types"
"github.com/ethereum/go-ethereum/crypto"
"github.com/ethereum/go-ethereum/params"
)
Expand All @@ -29,7 +30,7 @@ func GetDeveloperChain(period uint64, gasLimitt uint64, faucet common.Address) *
GasLimit: gasLimitt,
BaseFee: big.NewInt(params.InitialBaseFee),
Difficulty: big.NewInt(1),
Alloc: map[common.Address]core.GenesisAccount{
Alloc: map[common.Address]types.Account{
common.BytesToAddress([]byte{1}): {Balance: big.NewInt(1)}, // ECRecover
common.BytesToAddress([]byte{2}): {Balance: big.NewInt(1)}, // SHA256
common.BytesToAddress([]byte{3}): {Balance: big.NewInt(1)}, // RIPEMD
Expand Down
3 changes: 1 addition & 2 deletions internal/ethapi/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -1882,7 +1882,7 @@ type TransactionAPI struct {
}

// returns block transactions along with state-sync transaction if present
// nolint: unparam
// nolint : unused
func (api *TransactionAPI) getAllBlockTransactions(ctx context.Context, block *types.Block) (types.Transactions, bool) {
txs := block.Transactions()

Expand Down Expand Up @@ -2058,7 +2058,6 @@ func (api *TransactionAPI) GetRawTransactionByHash(ctx context.Context, hash com

// GetTransactionReceipt returns the transaction receipt for the given transaction hash.
func (api *TransactionAPI) GetTransactionReceipt(ctx context.Context, hash common.Hash) (map[string]interface{}, error) {

borTx := false

found, tx, blockHash, blockNumber, index, err := api.b.GetTransaction(ctx, hash)
Expand Down
3 changes: 2 additions & 1 deletion miner/miner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ func (m *mockBackend) TxPool() *txpool.TxPool {
return m.txPool
}

// nolint : unused
type testBlockChain struct {
root common.Hash
config *params.ChainConfig
Expand Down Expand Up @@ -367,7 +368,7 @@ func waitForMiningState(t *testing.T, m *Miner, mining bool) {
// GasLimit: gasLimit,
// BaseFee: big.NewInt(params.InitialBaseFee),
// Difficulty: big.NewInt(1),
// Alloc: map[common.Address]core.GenesisAccount{
// Alloc: map[common.Address]types.Account{
// common.BytesToAddress([]byte{1}): {Balance: big.NewInt(1)}, // ECRecover
// common.BytesToAddress([]byte{2}): {Balance: big.NewInt(1)}, // SHA256
// common.BytesToAddress([]byte{3}): {Balance: big.NewInt(1)}, // RIPEMD
Expand Down
Loading

0 comments on commit a66305d

Please sign in to comment.