add non-Geth revert msg check (#657)

.changeset/honest-foxes-reflect.md

@@ -0,0 +1,5 @@
+---
+"ccip": patch
+---
+
+add non-Geth revert msg check to v1.0 PriceRegistry and TokenPool

core/services/ocr2/plugins/ccip/internal/ccipcommon/shortcuts.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/hex"
 	"fmt"
+	"strings"
 
 	"golang.org/x/sync/errgroup"
 
@@ -73,3 +74,14 @@ func FlattenUniqueSlice[T comparable](slices ...[]T) []T {
 	}
 	return flattened
 }
+
+func IsTxRevertError(err error) bool {
+	if err == nil {
+		return false
+	}
+
+	// Geth eth_call reverts with "execution reverted"
+	// Nethermind, Parity, OpenEthereum eth_call reverts with "VM execution error"
+	// See: https://github.com/ethereum/go-ethereum/issues/21886
+	return strings.Contains(err.Error(), "execution reverted") || strings.Contains(err.Error(), "VM execution error")
+}

core/services/ocr2/plugins/ccip/internal/ccipcommon/shortcuts_test.go

@@ -1,6 +1,7 @@
 package ccipcommon
 
 import (
+	"fmt"
 	"math/rand"
 	"strconv"
 	"testing"
@@ -59,3 +60,22 @@ func TestFlattenUniqueSlice(t *testing.T) {
 		})
 	}
 }
+
+func TestIsTxRevertError(t *testing.T) {
+	testCases := []struct {
+		name           string
+		inputError     error
+		expectedOutput bool
+	}{
+		{name: "empty", inputError: nil, expectedOutput: false},
+		{name: "non-revert error", inputError: fmt.Errorf("nothing"), expectedOutput: false},
+		{name: "geth error", inputError: fmt.Errorf("execution reverted"), expectedOutput: true},
+		{name: "nethermind error", inputError: fmt.Errorf("VM execution error"), expectedOutput: true},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			assert.Equal(t, tc.expectedOutput, IsTxRevertError(tc.inputError))
+		})
+	}
+}

core/services/ocr2/plugins/ccip/internal/ccipdata/batchreader/token_pool_batch_reader.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"strings"
 	"sync"
 
 	"github.com/ethereum/go-ethereum/common"
@@ -15,6 +14,7 @@ import (
 	"github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/cciptypes"
 	ccipconfig "github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/config"
 	"github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/internal/ccipcalc"
+	"github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/internal/ccipcommon"
 	"github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/internal/ccipdata"
 	"github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/internal/ccipdata/v1_2_0"
 	"github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/internal/ccipdata/v1_4_0"
@@ -172,7 +172,7 @@ func getBatchedTypeAndVersion(ctx context.Context, evmBatchCaller rpclib.EvmBatc
 		if err1 != nil {
 			// typeAndVersion method do not exist for 1.0 pools. We are going to get an ErrEmptyOutput in that case.
 			// Some chains, like the simulated chains, will simply revert with "execution reverted"
-			if errors.Is(err1, rpclib.ErrEmptyOutput) || strings.Contains(err1.Error(), "execution reverted") {
+			if errors.Is(err1, rpclib.ErrEmptyOutput) || ccipcommon.IsTxRevertError(err1) {
 				return "LegacyPool " + ccipdata.V1_0_0, nil
 			}
 			return "", err1

core/services/ocr2/plugins/ccip/internal/ccipdata/factory/price_registry.go

@@ -1,8 +1,6 @@
 package factory
 
 import (
-	"strings"
-
 	"github.com/pkg/errors"
 
 	"github.com/smartcontractkit/chainlink/v2/core/chains/evm/client"
@@ -11,6 +9,7 @@ import (
 	"github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/cciptypes"
 	ccipconfig "github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/config"
 	"github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/internal/ccipcalc"
+	"github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/internal/ccipcommon"
 	"github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/internal/ccipdata"
 	"github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/internal/ccipdata/v1_0_0"
 	"github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/internal/ccipdata/v1_2_0"
@@ -35,8 +34,7 @@ func initOrClosePriceRegistryReader(lggr logger.Logger, versionFinder VersionFin
 	}
 
 	contractType, version, err := versionFinder.TypeAndVersion(priceRegistryAddress, cl)
-	isV1_0_0 := (err != nil && strings.Contains(err.Error(), "execution reverted")) ||
-		(contractType == ccipconfig.PriceRegistry && version.String() == ccipdata.V1_0_0)
+	isV1_0_0 := ccipcommon.IsTxRevertError(err) || (contractType == ccipconfig.PriceRegistry && version.String() == ccipdata.V1_0_0)
 	if isV1_0_0 {
 		lggr.Infof("Assuming %v is 1.0.0 price registry, got %v", priceRegistryEvmAddr, err)
 		// Unfortunately the v1 price registry doesn't have a method to get the version so assume if it reverts its v1.