Clean up loose ends for new Baklava network client (ethereum#941)

* default gateway fee to zero

* prevent log spam before registry is deployed

* set client version to 0.9.0

* add additional safety against blocking chain head events

* add log when disconnecting from static or trusted peer

* differentiate error and non-error disconnects

* revert changes to remove empty abi unmarshal

* run go generate

* add gateway fee to list of flags

* set version meta to unstable and fix a typo

* point ci to victor/loose-ends

* fix gateway fee not defined error

* prevent pointless warning

* add trace logging for light client transactions

* revert unnecessary changes to ethstats

* add trace log to les transaction status msg

* use gateway fee flag for clients to specify knowledge about peers

* refactor if statement

* fix bad if statement

* fix light tests

* use v2 ci image

.circleci/config.yml

@@ -6,11 +6,11 @@ executors:
     working_directory: ~/repos/geth
   e2e:
     docker:
-      - image: celohq/node10-gcloud
+      - image: celohq/node10-gcloud:v2
     working_directory: ~/repos/celo-monorepo/packages/celotool
     environment:
       GO_VERSION: "1.13.7"
-      CELO_MONOREPO_BRANCH_TO_TEST: master
+      CELO_MONOREPO_BRANCH_TO_TEST: victor/loose-ends
 jobs:
   build-bls-zexe:
     docker:

cmd/geth/main.go

@@ -95,6 +95,7 @@ var (
 		utils.LightEgressFlag,
 		utils.LightMaxPeersFlag,
 		utils.LightKDFFlag,
+		utils.LightGatewayFeeFlag,
 		utils.UltraLightServersFlag,
 		utils.UltraLightFractionFlag,
 		utils.UltraLightOnlyAnnounceFlag,

cmd/geth/usage.go

@@ -93,6 +93,7 @@ var AppHelpFlagGroups = []flagGroup{
 			utils.LightIngressFlag,
 			utils.LightEgressFlag,
 			utils.LightMaxPeersFlag,
+			utils.LightGatewayFeeFlag,
 			utils.UltraLightServersFlag,
 			utils.UltraLightFractionFlag,
 			utils.UltraLightOnlyAnnounceFlag,

cmd/utils/flags.go

@@ -231,11 +231,6 @@ var (
 		Usage: "Public address for transaction broadcasting and block mining rewards (default = first account)",
 		Value: "0",
 	}
-	GatewayFeeFlag = BigFlag{
-		Name:  "gatewayfee",
-		Usage: "Minimum value of gateway fee to serve a light client transaction",
-		Value: eth.DefaultConfig.GatewayFee,
-	}
 	BLSbaseFlag = cli.StringFlag{
 		Name:  "blsbase",
 		Usage: "Public address for block mining BLS signatures (default = first account created)",
@@ -262,6 +257,11 @@ var (
 		Usage: "Maximum number of light clients to serve, or light servers to attach to",
 		Value: eth.DefaultConfig.LightPeers,
 	}
+	LightGatewayFeeFlag = BigFlag{
+		Name:  "light.gatewayfee",
+		Usage: "Minimum value of gateway fee to serve a light client transaction",
+		Value: eth.DefaultConfig.GatewayFee,
+	}
 	UltraLightServersFlag = cli.StringFlag{
 		Name:  "ulc.servers",
 		Usage: "List of trusted ultra-light servers",
@@ -1045,6 +1045,9 @@ func setLes(ctx *cli.Context, cfg *eth.Config) {
 	if ctx.GlobalIsSet(LightMaxPeersFlag.Name) {
 		cfg.LightPeers = ctx.GlobalInt(LightMaxPeersFlag.Name)
 	}
+	if ctx.GlobalIsSet(LightGatewayFeeFlag.Name) {
+		cfg.GatewayFee = GlobalBig(ctx, LightGatewayFeeFlag.Name)
+	}
 	if ctx.GlobalIsSet(UltraLightServersFlag.Name) {
 		cfg.UltraLightServers = strings.Split(ctx.GlobalString(UltraLightServersFlag.Name), ",")
 	}
@@ -1658,10 +1661,6 @@ func SetEthConfig(ctx *cli.Context, stack *node.Node, cfg *eth.Config) {
 		cfg.RPCGasCap = new(big.Int).SetUint64(ctx.GlobalUint64(RPCGlobalGasCap.Name))
 	}
 
-	if ctx.GlobalIsSet(GatewayFeeFlag.Name) {
-		cfg.GatewayFee = GlobalBig(ctx, GatewayFeeFlag.Name)
-	}
-
 	// Override any default configs for hard coded networks.
 	switch {
 	case ctx.GlobalBool(TestnetFlag.Name):

core/tx_pool.go

@@ -22,6 +22,7 @@ import (
 	"math"
 	"math/big"
 	"sort"
+	"strconv"
 	"sync"
 	"time"
 
@@ -131,6 +132,21 @@ const (
 	TxStatusIncluded
 )
 
+func (s TxStatus) String() string {
+	switch s {
+	case TxStatusUnknown:
+		return "TxStatusUnknown"
+	case TxStatusQueued:
+		return "TxStatusQueued"
+	case TxStatusPending:
+		return "TxStatusPending"
+	case TxStatusIncluded:
+		return "TxStatusIncluded"
+	default:
+		return strconv.FormatUint(uint64(s), 10)
+	}
+}
+
 // blockChain provides the state of blockchain and current gas limit to do
 // some pre checks in tx pool and event subscribers.
 type blockChain interface {

core/vm/evm.go

@@ -580,7 +580,13 @@ func (evm *EVM) handleABICall(abi abipkg.ABI, funcName string, args []interface{
 	ret, leftoverGas, err := call(transactionData)
 
 	if err != nil {
-		log.Error("Error in calling the EVM", "funcName", funcName, "transactionData", hexutil.Encode(transactionData), "err", err)
+		// Do not log execution reverted as error for getAddressFor. This only happens before the Registry is deployed.
+		// TODO(nategraf): Find a more generic and complete solution to the problem of logging tolerated EVM call failures.
+		if funcName == "getAddressFor" {
+			log.Trace("Error in calling the EVM", "funcName", funcName, "transactionData", hexutil.Encode(transactionData), "err", err)
+		} else {
+			log.Error("Error in calling the EVM", "funcName", funcName, "transactionData", hexutil.Encode(transactionData), "err", err)
+		}
 		return leftoverGas, err
 	}
 

eth/backend.go

@@ -135,7 +135,7 @@ func New(ctx *node.ServiceContext, config *Config) (*Ethereum, error) {
 	}
 	log.Info("Allocated trie memory caches", "clean", common.StorageSize(config.TrieCleanCache)*1024*1024, "dirty", common.StorageSize(config.TrieDirtyCache)*1024*1024)
 
-	if config.GatewayFee == nil || config.GatewayFee.Cmp(common.Big0) <= 0 {
+	if config.GatewayFee == nil || config.GatewayFee.Cmp(common.Big0) < 0 {
 		log.Warn("Sanitizing invalid gateway fee", "provided", config.GatewayFee, "updated", DefaultConfig.GatewayFee)
 		config.GatewayFee = new(big.Int).Set(DefaultConfig.GatewayFee)
 	}
@@ -237,7 +237,7 @@ func New(ctx *node.ServiceContext, config *Config) (*Ethereum, error) {
 				return eth.blockchain.StateAt(stateRoot)
 			})
 
-		chainHeadCh := make(chan core.ChainHeadEvent)
+		chainHeadCh := make(chan core.ChainHeadEvent, 10)
 		chainHeadSub := eth.blockchain.SubscribeChainHeadEvent(chainHeadCh)
 
 		go func() {

eth/config.go

@@ -57,7 +57,7 @@ var DefaultConfig = Config{
 		GasPrice: big.NewInt(1),
 		Recommit: 3 * time.Second,
 	},
-	GatewayFee: big.NewInt(10000),
+	GatewayFee: big.NewInt(0),
 
 	TxPool: core.DefaultTxPoolConfig,
 

les/client.go

@@ -122,7 +122,7 @@ func New(ctx *node.ServiceContext, config *eth.Config) (*LightEthereum, error) {
 		panic(msg)
 	}
 	leth.retriever = newRetrieveManager(peers, leth.reqDist, leth.serverPool)
-	leth.relay = newLesTxRelay(peers, leth.retriever)
+	leth.relay = newLesTxRelay(peers, leth.retriever, config.GatewayFee)
 
 	leth.odr = NewLesOdr(chainDb, light.DefaultClientIndexerConfig, leth.retriever)
 	leth.chtIndexer = light.NewChtIndexer(chainDb, leth.odr, params.CHTFrequency, params.HelperTrieConfirmations, fullChainAvailable)

les/client_handler.go

@@ -322,6 +322,7 @@ func (h *clientHandler) handleMsg(p *peer) error {
 			return errResp(ErrDecode, "msg %v: %v", msg, err)
 		}
 		p.fcServer.ReceivedReply(resp.ReqID, resp.BV)
+		p.Log().Trace("Transaction status update", "status", resp.Status, "req", resp.ReqID)
 		deliverMsg = &Msg{
 			MsgType: MsgTxStatus,
 			ReqID:   resp.ReqID,

les/server_handler.go

@@ -761,6 +761,7 @@ func (h *serverHandler) handleMsg(p *peer, wg *sync.WaitGroup) error {
 			wg.Add(1)
 			go func() {
 				defer wg.Done()
+
 				stats := make([]light.TxStatus, len(req.Txs))
 				for i, tx := range req.Txs {
 					if i != 0 && !task.waitOrStop() {
@@ -769,15 +770,15 @@ func (h *serverHandler) handleMsg(p *peer, wg *sync.WaitGroup) error {
 					hash := tx.Hash()
 					stats[i] = h.txStatus(hash)
 					if stats[i].Status == core.TxStatusUnknown {
-						// Only include tx that have a validat gateway fee recipient & fee
+						// Only include transactions that have a valid gateway fee recipient & fee
 						if err := h.verifyGatewayFee(tx.GatewayFeeRecipient(), tx.GatewayFee()); err != nil {
-							fmt.Printf("Will respond with %s\n", err.Error())
+							p.Log().Trace("Rejected transaction from light peer for invalid gateway fee", "hash", hash.String(), "err", err)
 							stats[i].Error = err.Error()
 							continue
 						}
 
 						addFn := h.txpool.AddRemotes
-						// Add txs synchronously for testing purpose
+						// Add transactions synchronously for testing purpose
 						if h.addTxsSync {
 							addFn = h.txpool.AddRemotesSync
 						}
@@ -786,10 +787,11 @@ func (h *serverHandler) handleMsg(p *peer, wg *sync.WaitGroup) error {
 							continue
 						}
 						stats[i] = h.txStatus(hash)
+						p.Log().Trace("Added transaction from light peer to pool", "hash", hash.String(), "tx", tx)
 					}
 				}
+
 				reply := p.ReplyTxStatus(req.ReqID, stats)
-				fmt.Printf("Sending Response! %v %d %v\n", req.ReqID, uint64(reqCnt), reply)
 				sendResponse(req.ReqID, uint64(reqCnt), reply, task.done())
 				if metrics.EnabledExpensive {
 					miscOutTxsPacketsMeter.Mark(1)
@@ -1002,6 +1004,11 @@ func (h *serverHandler) verifyGatewayFee(gatewayFeeRecipient *common.Address, ga
 		return nil
 	}
 
+	// If this node does not specify a non-zero gateway fee accept any value.
+	if h.gatewayFee == nil || h.gatewayFee.Cmp(common.Big0) <= 0 {
+		return nil
+	}
+
 	// Otherwise, reject transactions that don't pay gas fees to this node.
 	if gatewayFeeRecipient == nil {
 		return fmt.Errorf("gateway fee recipient must be %s, got <nil>", h.etherbase.String())
@@ -1011,10 +1018,8 @@ func (h *serverHandler) verifyGatewayFee(gatewayFeeRecipient *common.Address, ga
 	}
 
 	// Check that the value of the supplied gateway fee is at least the minimum.
-	if h.gatewayFee != nil && h.gatewayFee.Cmp(common.Big0) > 0 {
-		if gatewayFee == nil || gatewayFee.Cmp(h.gatewayFee) < 0 {
-			return fmt.Errorf("gateway fee value must be at least %s, got %s", h.gatewayFee, gatewayFee)
-		}
+	if gatewayFee == nil || gatewayFee.Cmp(h.gatewayFee) < 0 {
+		return fmt.Errorf("gateway fee value must be at least %s, got %s", h.gatewayFee, gatewayFee)
 	}
 	return nil
 }

les/txrelay.go

@@ -18,6 +18,8 @@ package les
 
 import (
 	"context"
+	"errors"
+	"math/big"
 	"sync"
 
 	"github.com/ethereum/go-ethereum/common"
@@ -26,6 +28,10 @@ import (
 	"github.com/ethereum/go-ethereum/rlp"
 )
 
+var (
+	errGatewayFeeTooLow = errors.New("gateway fee too low to broadcast to peers")
+)
+
 type ltrInfo struct {
 	tx     *types.Transaction
 	sentTo map[*peer]struct{}
@@ -40,15 +46,19 @@ type lesTxRelay struct {
 	stop      chan struct{}
 
 	retriever *retrieveManager
+
+	// TODO(nategraf) Replace this field with the ability to query peers for gateway fee.
+	gatewayFee *big.Int
 }
 
-func newLesTxRelay(ps *peerSet, retriever *retrieveManager) *lesTxRelay {
+func newLesTxRelay(ps *peerSet, retriever *retrieveManager, gatewayFee *big.Int) *lesTxRelay {
 	r := &lesTxRelay{
-		txSent:    make(map[common.Hash]*ltrInfo),
-		txPending: make(map[common.Hash]struct{}),
-		ps:        ps,
-		retriever: retriever,
-		stop:      make(chan struct{}),
+		txSent:     make(map[common.Hash]*ltrInfo),
+		txPending:  make(map[common.Hash]struct{}),
+		ps:         ps,
+		retriever:  retriever,
+		stop:       make(chan struct{}),
+		gatewayFee: gatewayFee,
 	}
 	ps.notify(r)
 	return r
@@ -77,6 +87,25 @@ func (self *lesTxRelay) HasPeerWithEtherbase(etherbase *common.Address) error {
 	return err
 }
 
+func (self *lesTxRelay) CanRelayTransaction(tx *types.Transaction) error {
+	// TODO(nategraf) self.gatewayFee is used in place of the minimum gateway fee among peers.
+	// When it is possible to query peers for their gateway fee, this should be replaced.
+
+	// Check if there we have peer accepting transactions without a gateway fee.
+	if self.gatewayFee.Cmp(common.Big0) <= 0 {
+		return nil
+	}
+
+	// Should have a peer that will accept and broadcast our transaction.
+	if err := self.HasPeerWithEtherbase(tx.GatewayFeeRecipient()); err != nil {
+		return err
+	}
+	if tx.GatewayFee().Cmp(self.gatewayFee) < 0 {
+		return errGatewayFeeTooLow
+	}
+	return nil
+}
+
 // send sends a list of transactions to at most a given number of peers at
 // once, never resending any particular transaction to the same peer twice
 func (self *lesTxRelay) send(txs types.Transactions) {