Skip to content

Commit

Permalink
golangci-lint: revive: add early-return; fix issues (smartcontractkit…
Browse files Browse the repository at this point in the history
  • Loading branch information
jmank88 authored Feb 13, 2024
1 parent 28b53e5 commit 7d64b51
Show file tree
Hide file tree
Showing 21 changed files with 171 additions and 193 deletions.
2 changes: 1 addition & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ linters-settings:
- name: identical-branches
- name: get-return
# - name: flag-parameter
# - name: early-return
- name: early-return
- name: defer
- name: constant-logical-expr
# - name: confusing-naming
Expand Down
85 changes: 41 additions & 44 deletions core/chains/evm/gas/block_history_estimator.go
Original file line number Diff line number Diff line change
Expand Up @@ -340,11 +340,10 @@ func (b *BlockHistoryEstimator) checkConnectivity(attempts []EvmPriorAttempt) er
// reverse order since we want to go highest -> lowest block number and bail out early
for i := l - 1; i >= 0; i-- {
block := blockHistory[i]
if block.Number >= broadcastBeforeBlockNum {
blocks = append(blocks, block)
} else {
if block.Number < broadcastBeforeBlockNum {
break
}
blocks = append(blocks, block)
}
var eip1559 bool
switch attempt.TxType {
Expand All @@ -364,27 +363,26 @@ func (b *BlockHistoryEstimator) checkConnectivity(attempts []EvmPriorAttempt) er
b.logger.AssumptionViolationw("unexpected error while verifying transaction inclusion", "err", err, "txHash", attempt.TxHash.String())
return nil
}
if eip1559 {
sufficientFeeCap := true
for _, b := range blocks {
// feecap must >= tipcap+basefee for the block, otherwise there
// is no way this could have been included, and we must bail
// out of the check
attemptFeeCap := attempt.DynamicFee.FeeCap
attemptTipCap := attempt.DynamicFee.TipCap
if attemptFeeCap.Cmp(attemptTipCap.Add(b.BaseFeePerGas)) < 0 {
sufficientFeeCap = false
break
}
}
if sufficientFeeCap && attempt.DynamicFee.TipCap.Cmp(tipCap) > 0 {
return errors.Wrapf(commonfee.ErrConnectivity, "transaction %s has tip cap of %s, which is above percentile=%d%% (percentile tip cap: %s) for blocks %d thru %d (checking %d blocks)", attempt.TxHash, attempt.DynamicFee.TipCap, percentile, tipCap, blockHistory[l-1].Number, blockHistory[0].Number, expectInclusionWithinBlocks)
}
} else {
if !eip1559 {
if attempt.GasPrice.Cmp(gasPrice) > 0 {
return errors.Wrapf(commonfee.ErrConnectivity, "transaction %s has gas price of %s, which is above percentile=%d%% (percentile price: %s) for blocks %d thru %d (checking %d blocks)", attempt.TxHash, attempt.GasPrice, percentile, gasPrice, blockHistory[l-1].Number, blockHistory[0].Number, expectInclusionWithinBlocks)

}
continue
}
sufficientFeeCap := true
for _, b := range blocks {
// feecap must >= tipcap+basefee for the block, otherwise there
// is no way this could have been included, and we must bail
// out of the check
attemptFeeCap := attempt.DynamicFee.FeeCap
attemptTipCap := attempt.DynamicFee.TipCap
if attemptFeeCap.Cmp(attemptTipCap.Add(b.BaseFeePerGas)) < 0 {
sufficientFeeCap = false
break
}
}
if sufficientFeeCap && attempt.DynamicFee.TipCap.Cmp(tipCap) > 0 {
return errors.Wrapf(commonfee.ErrConnectivity, "transaction %s has tip cap of %s, which is above percentile=%d%% (percentile tip cap: %s) for blocks %d thru %d (checking %d blocks)", attempt.TxHash, attempt.DynamicFee.TipCap, percentile, tipCap, blockHistory[l-1].Number, blockHistory[0].Number, expectInclusionWithinBlocks)
}
}
return nil
Expand Down Expand Up @@ -560,20 +558,20 @@ func (b *BlockHistoryEstimator) Recalculate(head *evmtypes.Head) {
b.setPercentileGasPrice(percentileGasPrice)
promBlockHistoryEstimatorSetGasPrice.WithLabelValues(fmt.Sprintf("%v%%", percentile), b.chainID.String()).Set(float64(percentileGasPrice.Int64()))

if eip1559 {
float = new(big.Float).SetInt(percentileTipCap.ToInt())
gwei, _ = big.NewFloat(0).Quo(float, big.NewFloat(1000000000)).Float64()
tipCapGwei := fmt.Sprintf("%.2f", gwei)
lggrFields = append(lggrFields, []interface{}{
"tipCapWei", percentileTipCap,
"tipCapGwei", tipCapGwei,
}...)
lggr.Debugw(fmt.Sprintf("Setting new default prices, GasPrice: %v Gwei, TipCap: %v Gwei", gasPriceGwei, tipCapGwei), lggrFields...)
b.setPercentileTipCap(percentileTipCap)
promBlockHistoryEstimatorSetTipCap.WithLabelValues(fmt.Sprintf("%v%%", percentile), b.chainID.String()).Set(float64(percentileTipCap.Int64()))
} else {
if !eip1559 {
lggr.Debugw(fmt.Sprintf("Setting new default gas price: %v Gwei", gasPriceGwei), lggrFields...)
return
}
float = new(big.Float).SetInt(percentileTipCap.ToInt())
gwei, _ = big.NewFloat(0).Quo(float, big.NewFloat(1000000000)).Float64()
tipCapGwei := fmt.Sprintf("%.2f", gwei)
lggrFields = append(lggrFields, []interface{}{
"tipCapWei", percentileTipCap,
"tipCapGwei", tipCapGwei,
}...)
lggr.Debugw(fmt.Sprintf("Setting new default prices, GasPrice: %v Gwei, TipCap: %v Gwei", gasPriceGwei, tipCapGwei), lggrFields...)
b.setPercentileTipCap(percentileTipCap)
promBlockHistoryEstimatorSetTipCap.WithLabelValues(fmt.Sprintf("%v%%", percentile), b.chainID.String()).Set(float64(percentileTipCap.Int64()))
}

// FetchBlocks fetches block history leading up to the given head.
Expand Down Expand Up @@ -774,21 +772,20 @@ func (b *BlockHistoryEstimator) getPricesFromBlocks(blocks []evmtypes.Block, eip
for _, tx := range block.Transactions {
if b.IsUsable(tx, block, b.config.ChainType(), b.eConfig.PriceMin(), b.logger) {
gp := b.EffectiveGasPrice(block, tx)
if gp != nil {
gasPrices = append(gasPrices, gp)
} else {
if gp == nil {
b.logger.Warnw("Unable to get gas price for tx", "tx", tx, "block", block)
continue
}
if eip1559 {
tc := b.EffectiveTipCap(block, tx)
if tc != nil {
tipCaps = append(tipCaps, tc)
} else {
b.logger.Warnw("Unable to get tip cap for tx", "tx", tx, "block", block)
continue
}
gasPrices = append(gasPrices, gp)
if !eip1559 {
continue
}
tc := b.EffectiveTipCap(block, tx)
if tc == nil {
b.logger.Warnw("Unable to get tip cap for tx", "tx", tx, "block", block)
continue
}
tipCaps = append(tipCaps, tc)
}
}
}
Expand Down
47 changes: 21 additions & 26 deletions core/chains/evm/types/models.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,10 @@ func (h *Head) IsInChain(blockHash common.Hash) bool {
if h.Hash == blockHash {
return true
}
if h.Parent != nil {
h = h.Parent
} else {
if h.Parent == nil {
break
}
h = h.Parent
}
return false
}
Expand All @@ -121,11 +120,10 @@ func (h *Head) HashAtHeight(blockNum int64) common.Hash {
if h.Number == blockNum {
return h.Hash
}
if h.Parent != nil {
h = h.Parent
} else {
if h.Parent == nil {
break
}
h = h.Parent
}
return common.Hash{}
}
Expand All @@ -138,15 +136,14 @@ func (h *Head) ChainLength() uint32 {
l := uint32(1)

for {
if h.Parent != nil {
l++
if h == h.Parent {
panic("circular reference detected")
}
h = h.Parent
} else {
if h.Parent == nil {
break
}
l++
if h == h.Parent {
panic("circular reference detected")
}
h = h.Parent
}
return l
}
Expand All @@ -157,14 +154,13 @@ func (h *Head) ChainHashes() []common.Hash {

for {
hashes = append(hashes, h.Hash)
if h.Parent != nil {
if h == h.Parent {
panic("circular reference detected")
}
h = h.Parent
} else {
if h.Parent == nil {
break
}
if h == h.Parent {
panic("circular reference detected")
}
h = h.Parent
}
return hashes
}
Expand All @@ -186,15 +182,14 @@ func (h *Head) ChainString() string {

for {
sb.WriteString(h.String())
if h.Parent != nil {
if h == h.Parent {
panic("circular reference detected")
}
sb.WriteString("->")
h = h.Parent
} else {
if h.Parent == nil {
break
}
if h == h.Parent {
panic("circular reference detected")
}
sb.WriteString("->")
h = h.Parent
}
sb.WriteString("->nil")
return sb.String()
Expand Down
53 changes: 26 additions & 27 deletions core/cmd/shell_local.go
Original file line number Diff line number Diff line change
Expand Up @@ -1002,37 +1002,36 @@ func (s *Shell) CleanupChainTables(c *cli.Context) error {
// some tables with evm_chain_id (mostly job specs) are in public schema
tablesToDeleteFromQuery := `SELECT table_name, table_schema FROM information_schema.columns WHERE "column_name"=$1;`
// Delete rows from each table based on the chain_id.
if strings.EqualFold("EVM", c.String("type")) {
rows, err := db.Query(tablesToDeleteFromQuery, "evm_chain_id")
if err != nil {
return err
}
defer rows.Close()
if !strings.EqualFold("EVM", c.String("type")) {
return s.errorOut(errors.New("unknown chain type"))
}
rows, err := db.Query(tablesToDeleteFromQuery, "evm_chain_id")
if err != nil {
return err
}
defer rows.Close()

var tablesToDeleteFrom []string
for rows.Next() {
var name string
var schema string
if err = rows.Scan(&name, &schema); err != nil {
return err
}
tablesToDeleteFrom = append(tablesToDeleteFrom, schema+"."+name)
}
if rows.Err() != nil {
return rows.Err()
var tablesToDeleteFrom []string
for rows.Next() {
var name string
var schema string
if err = rows.Scan(&name, &schema); err != nil {
return err
}
tablesToDeleteFrom = append(tablesToDeleteFrom, schema+"."+name)
}
if rows.Err() != nil {
return rows.Err()
}

for _, tableName := range tablesToDeleteFrom {
query := fmt.Sprintf(`DELETE FROM %s WHERE "evm_chain_id"=$1;`, tableName)
_, err = db.Exec(query, c.String("id"))
if err != nil {
fmt.Printf("Error deleting rows containing evm_chain_id from %s: %v\n", tableName, err)
} else {
fmt.Printf("Rows with evm_chain_id %s deleted from %s.\n", c.String("id"), tableName)
}
for _, tableName := range tablesToDeleteFrom {
query := fmt.Sprintf(`DELETE FROM %s WHERE "evm_chain_id"=$1;`, tableName)
_, err = db.Exec(query, c.String("id"))
if err != nil {
fmt.Printf("Error deleting rows containing evm_chain_id from %s: %v\n", tableName, err)
} else {
fmt.Printf("Rows with evm_chain_id %s deleted from %s.\n", c.String("id"), tableName)
}
} else {
return s.errorOut(errors.New("unknown chain type"))
}
return nil
}
Expand Down
5 changes: 2 additions & 3 deletions core/gethwrappers/abigen.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,11 +147,10 @@ func getContractName(fileNode *ast.File) string {
if len(n.Name) < 3 {
return true
}
if n.Name[len(n.Name)-3:] == "ABI" {
contractName = n.Name[:len(n.Name)-3]
} else {
if n.Name[len(n.Name)-3:] != "ABI" {
return true
}
contractName = n.Name[:len(n.Name)-3]
}
}
return false
Expand Down
5 changes: 2 additions & 3 deletions core/gethwrappers/go_generate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,11 +138,10 @@ func getProjectRoot(t *testing.T) (rootPath string) {
root, err := os.Getwd()
require.NoError(t, err, "could not get current working directory")
for root != "/" { // Walk up path to find dir containing go.mod
if _, err := os.Stat(filepath.Join(root, "go.mod")); os.IsNotExist(err) {
root = filepath.Dir(root)
} else {
if _, err := os.Stat(filepath.Join(root, "go.mod")); !os.IsNotExist(err) {
return root
}
root = filepath.Dir(root)
}
t.Fatal("could not find project root")
return
Expand Down
5 changes: 2 additions & 3 deletions core/gethwrappers/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,10 @@ func GetProjectRoot() (rootPath string) {
err)
}
for root != "/" { // Walk up path to find dir containing go.mod
if _, err := os.Stat(filepath.Join(root, "go.mod")); os.IsNotExist(err) {
root = filepath.Dir(root)
} else {
if _, err := os.Stat(filepath.Join(root, "go.mod")); !os.IsNotExist(err) {
return root
}
root = filepath.Dir(root)
}
Exit("could not find project root", nil)
panic("can't get here")
Expand Down
7 changes: 3 additions & 4 deletions core/scripts/chaincli/handler/keeper_verifiable_load.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,12 +203,11 @@ func (k *Keeper) fetchBucketData(v verifiableLoad, opts *bind.CallOpts, id *big.
var err error
for i := 0; i < retryNum; i++ {
bucketDelays, err = v.GetBucketedDelays(opts, id, bucketNum)
if err != nil {
log.Printf("failed to get bucketed delays for upkeep id %s bucket %d: %v, retrying...", id.String(), bucketNum, err)
time.Sleep(retryDelay)
} else {
if err == nil {
break
}
log.Printf("failed to get bucketed delays for upkeep id %s bucket %d: %v, retrying...", id.String(), bucketNum, err)
time.Sleep(retryDelay)
}

var floatBucketDelays []float64
Expand Down
5 changes: 2 additions & 3 deletions core/scripts/common/polygonedge.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,11 +149,10 @@ func GetIbftExtraClean(extra []byte) (cleanedExtra []byte, err error) {
hexExtra := hex.EncodeToString(extra)
prefix := ""
for _, s := range hexExtra {
if s == '0' {
prefix = prefix + "0"
} else {
if s != '0' {
break
}
prefix = prefix + "0"
}

hexExtra = strings.TrimLeft(hexExtra, "0")
Expand Down
7 changes: 3 additions & 4 deletions core/services/ocr2/delegate.go
Original file line number Diff line number Diff line change
Expand Up @@ -491,12 +491,11 @@ func GetEVMEffectiveTransmitterID(jb *job.Job, chain legacyevm.Chain, lggr logge
effectiveTransmitterID, err := chain.TxManager().GetForwarderForEOA(common.HexToAddress(spec.TransmitterID.String))
if err == nil {
return effectiveTransmitterID.String(), nil
} else if spec.TransmitterID.Valid {
lggr.Warnw("Skipping forwarding for job, will fallback to default behavior", "job", jb.Name, "err", err)
// this shouldn't happen unless behaviour above was changed
} else {
} else if !spec.TransmitterID.Valid {
return "", errors.New("failed to get forwarder address and transmitterID is not set")
}
lggr.Warnw("Skipping forwarding for job, will fallback to default behavior", "job", jb.Name, "err", err)
// this shouldn't happen unless behaviour above was changed
}

return spec.TransmitterID.String, nil
Expand Down
Loading

0 comments on commit 7d64b51

Please sign in to comment.