call eth transaction receipt directly (#10344)

* call eth transaction receipt directly

* update

* update and fix tests

* fix tests

* tests

* add tests

* update

* fix types

* update

* update

* udpate

* update

* addressed comments

core/services/ocr2/plugins/ocr2keeper/evm21/block_subscriber.go

@@ -127,7 +127,7 @@ func (bs *BlockSubscriber) cleanup() {
 	bs.lggr.Infof("lastClearedBlock is set to %d", bs.lastClearedBlock)
 }
 
-func (bs *BlockSubscriber) Start(ctx context.Context) error {
+func (bs *BlockSubscriber) Start(_ context.Context) error {
 	bs.lggr.Info("block subscriber started.")
 	return bs.sync.StartOnce("BlockSubscriber", func() error {
 		bs.mu.Lock()
@@ -227,9 +227,15 @@ func (bs *BlockSubscriber) processHead(h *evmtypes.Head) {
 	// head parent is a linked list with EVM finality depth
 	// when re-org happens, new heads will have pointers to the new blocks
 	i := int64(0)
-	for cp := h; cp != nil; cp = cp.Parent {
-		if cp != h && bs.blocks[cp.Number] != cp.Hash.Hex() {
-			bs.lggr.Warnf("overriding block %d old hash %s with new hash %s due to re-org", cp.Number, bs.blocks[cp.Number], cp.Hash.Hex())
+	for cp := h; ; cp = cp.Parent {
+		if cp == nil || bs.blocks[cp.Number] == cp.Hash.Hex() {
+			break
+		}
+		existingHash, ok := bs.blocks[cp.Number]
+		if !ok {
+			bs.lggr.Debugf("filling block %d with new hash %s", cp.Number, cp.Hash.Hex())
+		} else if existingHash != cp.Hash.Hex() {
+			bs.lggr.Warnf("overriding block %d old hash %s with new hash %s due to re-org", cp.Number, existingHash, cp.Hash.Hex())
 		}
 		bs.blocks[cp.Number] = cp.Hash.Hex()
 		i++

core/services/ocr2/plugins/ocr2keeper/evm21/core/utils.go

@@ -0,0 +1,26 @@
+package core
+
+import (
+	"context"
+	"math/big"
+
+	"github.com/ethereum/go-ethereum/common"
+
+	"github.com/smartcontractkit/chainlink/v2/core/chains/evm/client"
+	"github.com/smartcontractkit/chainlink/v2/core/chains/evm/types"
+)
+
+// GetTxBlock calls eth_getTransactionReceipt on the eth client to obtain a tx receipt
+func GetTxBlock(client client.Client, txHash common.Hash) (*big.Int, common.Hash, error) {
+	receipt := types.Receipt{}
+	err := client.CallContext(context.Background(), &receipt, "eth_getTransactionReceipt", txHash)
+	if err != nil {
+		return nil, common.Hash{}, err
+	}
+
+	if receipt.Status != 1 {
+		return nil, common.Hash{}, nil
+	}
+
+	return receipt.GetBlockNumber(), receipt.GetBlockHash(), nil
+}

core/services/ocr2/plugins/ocr2keeper/evm21/core/utils_test.go

@@ -0,0 +1,72 @@
+package core
+
+import (
+	"fmt"
+	"math/big"
+	"testing"
+
+	"github.com/ethereum/go-ethereum/common"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+
+	evmClientMocks "github.com/smartcontractkit/chainlink/v2/core/chains/evm/client/mocks"
+
+	"github.com/smartcontractkit/chainlink/v2/core/chains/evm/types"
+)
+
+func TestUtils_GetTxBlock(t *testing.T) {
+	tests := []struct {
+		name         string
+		txHash       common.Hash
+		ethCallError error
+		receipt      *types.Receipt
+		status       uint64
+	}{
+		{
+			name:   "success",
+			txHash: common.HexToHash("0xc48fbf05edaf18f6aaa7de24de28528546b874bb03728d624ca407b8fed582a3"),
+			receipt: &types.Receipt{
+				Status:      1,
+				BlockNumber: big.NewInt(2000),
+			},
+			status: 1,
+		},
+		{
+			name:         "failure - eth call error",
+			txHash:       common.HexToHash("0xc48fbf05edaf18f6aaa7de24de28528546b874bb03728d624ca407b8fed582a3"),
+			ethCallError: fmt.Errorf("eth call failed"),
+		},
+		{
+			name:   "failure - tx does not exist",
+			txHash: common.HexToHash("0xc48fbf05edaf18f6aaa7de24de28528546b874bb03728d624ca407b8fed582a3"),
+			receipt: &types.Receipt{
+				Status: 0,
+			},
+			status: 0,
+		},
+	}
+
+	for _, tt := range tests {
+		client := new(evmClientMocks.Client)
+		client.On("CallContext", mock.Anything, mock.Anything, "eth_getTransactionReceipt", tt.txHash).
+			Return(tt.ethCallError).Run(func(args mock.Arguments) {
+			receipt := tt.receipt
+			if receipt != nil {
+				res := args.Get(1).(*types.Receipt)
+				res.Status = receipt.Status
+				res.TxHash = receipt.TxHash
+				res.BlockNumber = receipt.BlockNumber
+				res.BlockHash = receipt.BlockHash
+			}
+		})
+
+		bn, bh, err := GetTxBlock(client, tt.txHash)
+		if tt.ethCallError != nil {
+			assert.Equal(t, tt.ethCallError, err)
+		} else {
+			assert.Equal(t, tt.status, tt.receipt.Status)
+			assert.Equal(t, tt.receipt.BlockNumber, bn)
+			assert.Equal(t, tt.receipt.BlockHash, bh)
+		}
+	}
+}

core/services/ocr2/plugins/ocr2keeper/evm21/feed_lookup.go

@@ -106,7 +106,7 @@ func (r *EvmRegistry) feedLookup(ctx context.Context, checkResults []ocr2keepers
 		lggr.Infof("at block %d upkeep %s trying to decodeFeedLookup performData=%s", block, upkeepId, hexutil.Encode(checkResults[i].PerformData))
 		l, err := r.decodeFeedLookup(res.PerformData)
 		if err != nil {
-			lggr.Warnf("upkeep %s block %d decodeFeedLookup failed: %v", upkeepId, block, err)
+			lggr.Warnf("at block %d upkeep %s decodeFeedLookup failed: %v", block, upkeepId, err)
 			// Not feed lookup error, nothing to do here
 			continue
 		}

core/services/ocr2/plugins/ocr2keeper/evm21/logprovider/recoverer.go

@@ -24,8 +24,6 @@ import (
 )
 
 var (
-	ErrNotFound = errors.New("not found")
-
 	DefaultRecoveryInterval = 5 * time.Second
 	RecoveryCacheTTL        = 10*time.Minute - time.Second
 	GCInterval              = RecoveryCacheTTL
@@ -182,7 +180,7 @@ func (r *logRecoverer) getLogTriggerCheckData(ctx context.Context, proposal ocr2
 	logBlock := int64(proposal.Trigger.LogTriggerExtension.BlockNumber)
 	if logBlock == 0 {
 		var number *big.Int
-		number, _, err = r.getTxBlock(proposal.Trigger.LogTriggerExtension.TxHash)
+		number, _, err = core.GetTxBlock(r.client, proposal.Trigger.LogTriggerExtension.TxHash)
 		if err != nil {
 			return nil, err
 		}
@@ -243,16 +241,6 @@ func (r *logRecoverer) getLogTriggerCheckData(ctx context.Context, proposal ocr2
 	return nil, fmt.Errorf("no log found for upkeepID %v and trigger %+v", proposal.UpkeepID, proposal.Trigger)
 }
 
-func (r *logRecoverer) getTxBlock(txHash common.Hash) (*big.Int, common.Hash, error) {
-	// TODO: do manual eth_getTransactionReceipt call to get block number and hash
-	txr, err := r.client.TransactionReceipt(context.Background(), txHash)
-	if err != nil {
-		return nil, common.Hash{}, err
-	}
-
-	return txr.BlockNumber, txr.BlockHash, nil
-}
-
 func (r *logRecoverer) GetRecoveryProposals(ctx context.Context) ([]ocr2keepers.UpkeepPayload, error) {
 	r.lock.Lock()
 	defer r.lock.Unlock()

core/services/ocr2/plugins/ocr2keeper/evm21/logprovider/recoverer_test.go

@@ -9,7 +9,6 @@ import (
 	"time"
 
 	"github.com/ethereum/go-ethereum/common"
-	"github.com/ethereum/go-ethereum/core/types"
 	"github.com/pkg/errors"
 	ocr2keepers "github.com/smartcontractkit/ocr2keepers/pkg/v3/types"
 	"github.com/stretchr/testify/assert"
@@ -19,6 +18,7 @@ import (
 	"github.com/smartcontractkit/chainlink/v2/core/chains/evm/client"
 	"github.com/smartcontractkit/chainlink/v2/core/chains/evm/logpoller"
 	lpmocks "github.com/smartcontractkit/chainlink/v2/core/chains/evm/logpoller/mocks"
+	"github.com/smartcontractkit/chainlink/v2/core/chains/evm/types"
 	"github.com/smartcontractkit/chainlink/v2/core/internal/testutils"
 	"github.com/smartcontractkit/chainlink/v2/core/logger"
 	"github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ocr2keeper/evm21/core"
@@ -370,7 +370,7 @@ func TestLogRecoverer_Recover(t *testing.T) {
 }
 
 func TestLogRecoverer_SelectFilterBatch(t *testing.T) {
-	n := (recoveryBatchSize*2 + 2)
+	n := recoveryBatchSize*2 + 2
 	filters := []upkeepFilter{}
 	for i := 0; i < n; i++ {
 		filters = append(filters, upkeepFilter{
@@ -547,8 +547,8 @@ func TestLogRecoverer_GetProposalData(t *testing.T) {
 				},
 			},
 			client: &mockClient{
-				TransactionReceiptFn: func(ctx context.Context, txHash common.Hash) (*types.Receipt, error) {
-					return nil, errors.New("tx receipt boom")
+				CallContextFn: func(ctx context.Context, receipt *types.Receipt, method string, args ...interface{}) error {
+					return errors.New("tx receipt boom")
 				},
 			},
 			expectErr: true,
@@ -575,8 +575,8 @@ func TestLogRecoverer_GetProposalData(t *testing.T) {
 				},
 			},
 			client: &mockClient{
-				TransactionReceiptFn: func(ctx context.Context, txHash common.Hash) (*types.Receipt, error) {
-					return &types.Receipt{}, nil
+				CallContextFn: func(ctx context.Context, receipt *types.Receipt, method string, args ...interface{}) error {
+					return nil
 				},
 			},
 			expectErr: true,
@@ -603,10 +603,10 @@ func TestLogRecoverer_GetProposalData(t *testing.T) {
 				},
 			},
 			client: &mockClient{
-				TransactionReceiptFn: func(ctx context.Context, txHash common.Hash) (*types.Receipt, error) {
-					return &types.Receipt{
-						BlockNumber: big.NewInt(200),
-					}, nil
+				CallContextFn: func(ctx context.Context, receipt *types.Receipt, method string, args ...interface{}) error {
+					receipt.Status = 1
+					receipt.BlockNumber = big.NewInt(200)
+					return nil
 				},
 			},
 			expectErr: true,
@@ -942,11 +942,12 @@ func (p *mockLogPoller) LatestBlock(qopts ...pg.QOpt) (int64, error) {
 
 type mockClient struct {
 	client.Client
-	TransactionReceiptFn func(ctx context.Context, txHash common.Hash) (*types.Receipt, error)
+	CallContextFn func(ctx context.Context, receipt *types.Receipt, method string, args ...interface{}) error
 }
 
-func (c *mockClient) TransactionReceipt(ctx context.Context, txHash common.Hash) (*types.Receipt, error) {
-	return c.TransactionReceiptFn(ctx, txHash)
+func (c *mockClient) CallContext(ctx context.Context, r interface{}, method string, args ...interface{}) error {
+	receipt := r.(*types.Receipt)
+	return c.CallContextFn(ctx, receipt, method, args)
 }
 
 type mockStateReader struct {

core/services/ocr2/plugins/ocr2keeper/evm21/registry.go

@@ -40,13 +40,12 @@ const (
 )
 
 var (
-	ErrLogReadFailure                = fmt.Errorf("failure reading logs")
-	ErrHeadNotAvailable              = fmt.Errorf("head not available")
-	ErrInitializationFailure         = fmt.Errorf("failed to initialize registry")
-	ErrContextCancelled              = fmt.Errorf("context was cancelled")
-	ErrABINotParsable                = fmt.Errorf("error parsing abi")
-	ActiveUpkeepIDBatchSize    int64 = 1000
-	FetchUpkeepConfigBatchSize       = 10
+	ErrLogReadFailure              = fmt.Errorf("failure reading logs")
+	ErrHeadNotAvailable            = fmt.Errorf("head not available")
+	ErrInitializationFailure       = fmt.Errorf("failed to initialize registry")
+	ErrContextCancelled            = fmt.Errorf("context was cancelled")
+	ErrABINotParsable              = fmt.Errorf("error parsing abi")
+	ActiveUpkeepIDBatchSize  int64 = 1000
 	// This is the interval at which active upkeep list is fully refreshed from chain
 	refreshInterval = 15 * time.Minute
 	// This is the lookback for polling upkeep state event logs from latest block

core/services/ocr2/plugins/ocr2keeper/evm21/registry_check_pipeline.go

@@ -97,16 +97,6 @@ func (r *EvmRegistry) getBlockHash(blockNumber *big.Int) (common.Hash, error) {
 	return blocks[0].BlockHash, nil
 }
 
-func (r *EvmRegistry) getTxBlock(txHash common.Hash) (*big.Int, common.Hash, error) {
-	// TODO: do manual eth_getTransactionReceipt call to get block number and hash
-	txr, err := r.client.TransactionReceipt(r.ctx, txHash)
-	if err != nil {
-		return nil, common.Hash{}, err
-	}
-
-	return txr.BlockNumber, txr.BlockHash, nil
-}
-
 // verifyCheckBlock checks that the check block and hash are valid, returns the pipeline execution state and retryable
 func (r *EvmRegistry) verifyCheckBlock(ctx context.Context, checkBlock, upkeepId *big.Int, checkHash common.Hash) (state encoding.PipelineExecutionState, retryable bool) {
 	// verify check block number is not too old
@@ -115,7 +105,7 @@ func (r *EvmRegistry) verifyCheckBlock(ctx context.Context, checkBlock, upkeepId
 		r.lggr.Warnf("latest block is %d, check block number %s is too old for upkeepId %s", r.bs.latestBlock.Load(), checkBlock, upkeepId)
 		return encoding.CheckBlockTooOld, false
 	}
-	r.lggr.Warnf("latestBlock=%d checkBlock=%d", r.bs.latestBlock.Load(), checkBlock.Int64())
+	r.lggr.Warnf("latestBlock=%d checkBlock=%d", r.bs.latestBlock.Load().Number, checkBlock.Int64())
 
 	var h string
 	var ok bool
@@ -153,7 +143,7 @@ func (r *EvmRegistry) verifyLogExists(upkeepId *big.Int, p ocr2keepers.UpkeepPay
 		r.lggr.Debugf("log block not provided, querying eth client for tx hash %s for upkeepId %s", hexutil.Encode(p.Trigger.LogTriggerExtension.TxHash[:]), upkeepId)
 	}
 	// query eth client as a fallback
-	bn, _, err := r.getTxBlock(p.Trigger.LogTriggerExtension.TxHash)
+	bn, _, err := core.GetTxBlock(r.client, p.Trigger.LogTriggerExtension.TxHash)
 	if err != nil {
 		// primitive way of checking errors
 		if strings.Contains(err.Error(), "missing required field") || strings.Contains(err.Error(), "not found") {

core/services/ocr2/plugins/ocr2keeper/evm21/registry_check_pipeline_test.go

@@ -8,7 +8,6 @@ import (
 	"testing"
 
 	"github.com/ethereum/go-ethereum/common"
-	"github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/rpc"
 
 	ocr2keepers "github.com/smartcontractkit/ocr2keepers/pkg/v3/types"
@@ -17,6 +16,7 @@ import (
 
 	evmClientMocks "github.com/smartcontractkit/chainlink/v2/core/chains/evm/client/mocks"
 	"github.com/smartcontractkit/chainlink/v2/core/chains/evm/logpoller"
+	"github.com/smartcontractkit/chainlink/v2/core/chains/evm/types"
 	"github.com/smartcontractkit/chainlink/v2/core/logger"
 	"github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ocr2keeper/evm21/core"
 	"github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ocr2keeper/evm21/encoding"
@@ -272,7 +272,7 @@ func TestRegistry_VerifyLogExists(t *testing.T) {
 			blocks: map[int64]string{
 				500: "0xb2173b4b75f23f56b7b2b6b2cc5fa9ed1079b9d1655b12b40fdb4dbf59006419",
 			},
-			receipt: &types.Receipt{},
+			receipt: &types.Receipt{Status: 0},
 		},
 		{
 			name:     "eth client returns a matching block",
@@ -289,6 +289,7 @@ func TestRegistry_VerifyLogExists(t *testing.T) {
 			},
 			makeEthCall: true,
 			receipt: &types.Receipt{
+				Status:      1,
 				BlockNumber: big.NewInt(550),
 				BlockHash:   common.HexToHash("0x5bff03de234fe771ac0d685f9ee0fb0b757ea02ec9e6f10e8e2ee806db1b6b83"),
 			},
@@ -322,8 +323,16 @@ func TestRegistry_VerifyLogExists(t *testing.T) {
 
 			if tc.makeEthCall {
 				client := new(evmClientMocks.Client)
-				client.On("TransactionReceipt", mock.Anything, common.HexToHash("0xc8def8abdcf3a4eaaf6cc13bff3e4e2a7168d86ea41dbbf97451235aa76c3651")).
-					Return(tc.receipt, tc.ethCallErr)
+				client.On("CallContext", mock.Anything, mock.Anything, "eth_getTransactionReceipt", common.BytesToHash(tc.payload.Trigger.LogTriggerExtension.TxHash[:])).
+					Return(tc.ethCallErr).Run(func(args mock.Arguments) {
+					if tc.receipt != nil {
+						res := args.Get(1).(*types.Receipt)
+						res.Status = tc.receipt.Status
+						res.TxHash = tc.receipt.TxHash
+						res.BlockNumber = tc.receipt.BlockNumber
+						res.BlockHash = tc.receipt.BlockHash
+					}
+				})
 				e.client = client
 			}
 
@@ -440,12 +449,7 @@ func TestRegistry_CheckUpkeeps(t *testing.T) {
 			ethCalls: map[string]bool{
 				uid1.String(): true,
 			},
-			receipts: map[string]*types.Receipt{
-				//uid1.String(): {
-				//	BlockNumber: big.NewInt(550),
-				//	BlockHash:   common.HexToHash("0x5bff03de234fe771ac0d685f9ee0fb0b757ea02ec9e6f10e8e2ee806db1b6b83"),
-				//},
-			},
+			receipts: map[string]*types.Receipt{},
 			ethCallErrors: map[string]error{
 				uid1.String(): fmt.Errorf("error"),
 			},
@@ -467,8 +471,17 @@ func TestRegistry_CheckUpkeeps(t *testing.T) {
 			for _, i := range tc.inputs {
 				uid := i.UpkeepID.String()
 				if tc.ethCalls[uid] {
-					client.On("TransactionReceipt", mock.Anything, common.HexToHash("0xc8def8abdcf3a4eaaf6cc13bff3e4e2a7168d86ea41dbbf97451235aa76c3651")).
-						Return(tc.receipts[uid], tc.ethCallErrors[uid])
+					client.On("CallContext", mock.Anything, mock.Anything, "eth_getTransactionReceipt", common.HexToHash("0xc8def8abdcf3a4eaaf6cc13bff3e4e2a7168d86ea41dbbf97451235aa76c3651")).
+						Return(tc.ethCallErrors[uid]).Run(func(args mock.Arguments) {
+						receipt := tc.receipts[uid]
+						if receipt != nil {
+							res := args.Get(1).(*types.Receipt)
+							res.Status = receipt.Status
+							res.TxHash = receipt.TxHash
+							res.BlockNumber = receipt.BlockNumber
+							res.BlockHash = receipt.BlockHash
+						}
+					})
 				}
 			}
 			e.client = client