Work through some untested logic, and update to firefly-signer 1.1.9

Signed-off-by: Peter Broadhurst <[email protected]>

go.mod

@@ -5,7 +5,7 @@ go 1.19
 require (
 	github.com/hashicorp/golang-lru v0.5.4
 	github.com/hyperledger/firefly-common v1.2.11
-	github.com/hyperledger/firefly-signer v1.1.8
+	github.com/hyperledger/firefly-signer v1.1.9
 	github.com/hyperledger/firefly-transaction-manager v1.2.13
 	github.com/sirupsen/logrus v1.9.0
 	github.com/spf13/cobra v1.6.1

go.sum

@@ -198,8 +198,8 @@ github.com/huandu/xstrings v1.3.1 h1:4jgBlKK6tLKFvO8u5pmYjG91cqytmDCDvGh7ECVFfFs
 github.com/huandu/xstrings v1.3.1/go.mod h1:y5/lhBue+AyNmUVz9RLU9xbLR0o4KIIExikq4ovT0aE=
 github.com/hyperledger/firefly-common v1.2.11 h1:ePDHJtorKE6ss8PtoPlyqLb+cB0TDB7ziM85Gtyerqs=
 github.com/hyperledger/firefly-common v1.2.11/go.mod h1:17lOH4YufiPy82LpKm8fPa/YXJ0pUyq01zK1CmklJwM=
-github.com/hyperledger/firefly-signer v1.1.8 h1:XyJjZXesih2dWYG31m5ZYt4irH7/PdkRutMPld7AqKE=
-github.com/hyperledger/firefly-signer v1.1.8/go.mod h1:vNbbROziwqkOmO0b+9ky3devjcFg0JIkR2M1KG7seTQ=
+github.com/hyperledger/firefly-signer v1.1.9 h1:Tx1iPTOLTpdFOLtkKFUWpjNsL/+oswnAaU4CTLMDw4Q=
+github.com/hyperledger/firefly-signer v1.1.9/go.mod h1:vNbbROziwqkOmO0b+9ky3devjcFg0JIkR2M1KG7seTQ=
 github.com/hyperledger/firefly-transaction-manager v1.2.13 h1:qMBN3Eviaxn96b2zsSBpwZ/vEt3X+ojW8B+p3UzhOjg=
 github.com/hyperledger/firefly-transaction-manager v1.2.13/go.mod h1:HjSAX2HoB/XccjXPD2h3z8owUMWrPYjspxLD55sRLnE=
 github.com/ianlancetaylor/demangle v0.0.0-20181102032728-5e5cf60278f6/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc=

internal/ethereum/deploy_contract_prepare.go

@@ -1,4 +1,4 @@
-// Copyright © 2022 Kaleido, Inc.
+// Copyright © 2023 Kaleido, Inc.
 //
 // SPDX-License-Identifier: Apache-2.0
 //
@@ -81,7 +81,7 @@ func (c *ethConnector) prepareDeployData(ctx context.Context, req *ffcapi.Contra
 	var a *abi.ABI
 	err = json.Unmarshal(req.Definition.Bytes(), &a)
 	if err != nil {
-		return nil, nil, i18n.NewError(ctx, msgs.MsgUnmarshalABIFail, err)
+		return nil, nil, i18n.NewError(ctx, msgs.MsgUnmarshalABIMethodFail, err)
 	}
 
 	// Find the constructor in the ABI

internal/ethereum/deploy_contract_prepare_test.go

@@ -155,7 +155,7 @@ func TestDeployContractPrepareBadErrorABI(t *testing.T) {
 	assert.NoError(t, err)
 	_, reason, err := c.DeployContractPrepare(ctx, &req)
 
-	assert.Regexp(t, "FF23013", err)
+	assert.Regexp(t, "FF23050", err)
 	assert.Equal(t, ffcapi.ErrorReasonInvalidInputs, reason)
 
 }

internal/ethereum/estimate_gas.go

@@ -43,7 +43,7 @@ func (c *ethConnector) GasEstimate(ctx context.Context, transaction *ffcapi.Tran
 	// Parse the from address
 	from, err := ethtypes.NewAddress(transaction.From)
 	if err != nil {
-		return nil, "", i18n.NewError(ctx, msgs.MsgInvalidFromAddress, transaction.From, err)
+		return nil, ffcapi.ErrorReasonInvalidInputs, i18n.NewError(ctx, msgs.MsgInvalidFromAddress, transaction.From, err)
 	}
 	tx.From = json.RawMessage(fmt.Sprintf(`"%s"`, from))
 
@@ -52,7 +52,7 @@ func (c *ethConnector) GasEstimate(ctx context.Context, transaction *ffcapi.Tran
 	if transaction.To != "" {
 		to, err = ethtypes.NewAddress(transaction.To)
 		if err != nil {
-			return nil, "", i18n.NewError(ctx, msgs.MsgInvalidToAddress, transaction.To, err)
+			return nil, ffcapi.ErrorReasonInvalidInputs, i18n.NewError(ctx, msgs.MsgInvalidToAddress, transaction.To, err)
 		}
 		tx.To = to
 	}
@@ -71,21 +71,8 @@ func (c *ethConnector) gasEstimate(ctx context.Context, tx *ethsigner.Transactio
 	var gasEstimate ethtypes.HexInteger
 	rpcErr := c.backend.CallRPC(ctx, &gasEstimate, "eth_estimateGas", tx)
 	if rpcErr != nil {
-		// some Ethereum implementations (eg. geth 1.10) returns the revert details on the estimateGas calls,
-		// so check that before making the eth_call request
-		if rpcErr.Data != "" {
-			var revertData ethtypes.HexBytes0xPrefix
-			e1 := json.Unmarshal(rpcErr.Data.Bytes(), &revertData)
-			if e1 != nil {
-				return nil, mapError(callRPCMethods, e1), e1
-			}
-			revertReason, ok := processRevertReason(ctx, revertData, errors)
-			if revertReason != "" {
-				if ok {
-					return nil, ffcapi.ErrorReasonTransactionReverted, i18n.NewError(ctx, msgs.MsgReverted, revertReason)
-				}
-				return nil, ffcapi.ErrorReasonTransactionReverted, i18n.NewError(ctx, msgs.MsgReverted, revertReason)
-			}
+		if reason, revertErr := c.attemptProcessingRevertData(ctx, errors, rpcErr); revertErr != nil {
+			return nil, reason, revertErr
 		}
 
 		// If it fails, fall back to an eth_call to see if we get a reverted reason

internal/ethereum/estimate_gas_test.go

@@ -17,9 +17,13 @@
 package ethereum
 
 import (
+	"context"
+	"encoding/hex"
 	"encoding/json"
 	"testing"
 
+	"github.com/hyperledger/firefly-common/pkg/fftypes"
+	"github.com/hyperledger/firefly-signer/pkg/abi"
 	"github.com/hyperledger/firefly-signer/pkg/ethsigner"
 	"github.com/hyperledger/firefly-signer/pkg/ethtypes"
 	"github.com/hyperledger/firefly-signer/pkg/rpcbackend"
@@ -90,3 +94,193 @@ func TestGasEstimateFail(t *testing.T) {
 	assert.Nil(t, res)
 
 }
+
+func TestGasEstimateBadFromAddress(t *testing.T) {
+
+	ctx, c, _, done := newTestConnector(t)
+	defer done()
+
+	var req ffcapi.TransactionInput
+	err := json.Unmarshal([]byte(`{
+	"ffcapi": {
+		"version": "v1.0.0",
+		"id": "904F177C-C790-4B01-BDF4-F2B4E52E607E",
+		"type": "get_address_balance"
+	},
+	"to": "0x4a8c8f1717570f9774652075e249ded38124d708",
+	"from": "bad address",
+	"value": "100000000",
+	"nonce": "0x01"
+}`), &req)
+	assert.NoError(t, err)
+	res, reason, err := c.GasEstimate(ctx, &req)
+	assert.Regexp(t, "FF23019", err)
+	assert.Equal(t, ffcapi.ErrorReasonInvalidInputs, reason)
+	assert.Nil(t, res)
+
+}
+
+func TestGasEstimateBadToAddress(t *testing.T) {
+
+	ctx, c, _, done := newTestConnector(t)
+	defer done()
+
+	var req ffcapi.TransactionInput
+	err := json.Unmarshal([]byte(`{
+	"ffcapi": {
+		"version": "v1.0.0",
+		"id": "904F177C-C790-4B01-BDF4-F2B4E52E607E",
+		"type": "get_address_balance"
+	},
+	"to": "bad address",
+	"from": "0x4a8c8f1717570f9774652075e249ded38124d708",
+	"value": "100000000",
+	"nonce": "0x01"
+}`), &req)
+	assert.NoError(t, err)
+	res, reason, err := c.GasEstimate(ctx, &req)
+	assert.Regexp(t, "FF23020", err)
+	assert.Equal(t, ffcapi.ErrorReasonInvalidInputs, reason)
+	assert.Nil(t, res)
+
+}
+
+func TestGasEstimateFailRevertReasonInData(t *testing.T) {
+
+	ctx, c, mRPC, done := newTestConnector(t)
+	defer done()
+
+	errData, err := defaultError.EncodeCallDataValues([]string{"this reason"})
+	assert.NoError(t, err)
+	mRPC.On("CallRPC", mock.Anything, mock.Anything, "eth_estimateGas",
+		mock.MatchedBy(func(tx *ethsigner.Transaction) bool {
+			return true
+		})).
+		Return(&rpcbackend.RPCError{Message: "reverted for reason...", Data: *fftypes.JSONAnyPtr(
+			`"0x` + hex.EncodeToString(errData) + `"`,
+		)})
+
+	var req ffcapi.TransactionInput
+	err = json.Unmarshal([]byte(sampleGasEstimate), &req)
+	assert.NoError(t, err)
+	res, reason, err := c.GasEstimate(ctx, &req)
+	assert.Regexp(t, "FF23021.*this reason", err)
+	assert.Equal(t, ffcapi.ErrorReasonTransactionReverted, reason)
+	assert.Nil(t, res)
+
+}
+
+func TestGasEstimateFailThenNilCallNoMethod(t *testing.T) {
+
+	ctx, c, mRPC, done := newTestConnector(t)
+	defer done()
+
+	mRPC.On("CallRPC", mock.Anything, mock.Anything, "eth_estimateGas",
+		mock.MatchedBy(func(tx *ethsigner.Transaction) bool {
+			return true
+		})).
+		Return(&rpcbackend.RPCError{Message: "pop"})
+	mRPC.On("CallRPC", mock.Anything, mock.Anything, "eth_call",
+		mock.MatchedBy(func(tx *ethsigner.Transaction) bool {
+			return true
+		}), "latest").
+		Run(func(args mock.Arguments) {
+			hb := args[1].(*ethtypes.HexBytes0xPrefix)
+			*hb = []byte("000102030405") // ignored as there's no method to parse the outputs against
+		}).
+		Return(nil)
+
+	var req ffcapi.TransactionInput
+	err := json.Unmarshal([]byte(sampleGasEstimate), &req)
+	assert.NoError(t, err)
+	res, reason, err := c.GasEstimate(ctx, &req)
+	assert.Regexp(t, "pop", err)
+	assert.Empty(t, reason)
+	assert.Nil(t, res)
+
+}
+
+func TestGasEstimateFailThenRevertDataFromCall(t *testing.T) {
+
+	ctx, c, mRPC, done := newTestConnector(t)
+	defer done()
+
+	errData, err := defaultError.EncodeCallDataValues([]string{"this reason"})
+	assert.NoError(t, err)
+	mRPC.On("CallRPC", mock.Anything, mock.Anything, "eth_estimateGas",
+		mock.MatchedBy(func(tx *ethsigner.Transaction) bool {
+			return true
+		})).
+		Return(&rpcbackend.RPCError{Message: "pop"})
+	mRPC.On("CallRPC", mock.Anything, mock.Anything, "eth_call",
+		mock.MatchedBy(func(tx *ethsigner.Transaction) bool {
+			return true
+		}), "latest").
+		Return(&rpcbackend.RPCError{Message: "reverted for reason...", Data: *fftypes.JSONAnyPtr(
+			`"0x` + hex.EncodeToString(errData) + `"`,
+		)})
+
+	var req ffcapi.TransactionInput
+	err = json.Unmarshal([]byte(sampleGasEstimate), &req)
+	assert.NoError(t, err)
+	res, reason, err := c.GasEstimate(ctx, &req)
+	assert.Regexp(t, "FF23021.*this reason", err)
+	assert.Equal(t, ffcapi.ErrorReasonTransactionReverted, reason)
+	assert.Nil(t, res)
+
+}
+
+func TestGasEstimateFailThenRevertErrorNoExtraInfo(t *testing.T) {
+
+	ctx, c, mRPC, done := newTestConnector(t)
+	defer done()
+
+	mRPC.On("CallRPC", mock.Anything, mock.Anything, "eth_estimateGas",
+		mock.MatchedBy(func(tx *ethsigner.Transaction) bool {
+			return true
+		})).
+		Return(&rpcbackend.RPCError{Message: "pop"})
+	mRPC.On("CallRPC", mock.Anything, mock.Anything, "eth_call",
+		mock.MatchedBy(func(tx *ethsigner.Transaction) bool {
+			return true
+		}), "latest").
+		Return(&rpcbackend.RPCError{Message: "execution reverted, without return data"})
+
+	var req ffcapi.TransactionInput
+	err := json.Unmarshal([]byte(sampleGasEstimate), &req)
+	assert.NoError(t, err)
+	res, reason, err := c.GasEstimate(ctx, &req)
+	assert.Regexp(t, "FF23021.*execution reverted, without return data", err)
+	assert.Equal(t, ffcapi.ErrorReasonTransactionReverted, reason)
+	assert.Nil(t, res)
+
+}
+
+func TestGasEstimateFailCustomErrorCannotParse(t *testing.T) {
+
+	ctx, c, mRPC, done := newTestConnector(t)
+	defer done()
+
+	errData, err := defaultError.EncodeCallDataValues([]string{"this reason"})
+	assert.NoError(t, err)
+	mRPC.On("CallRPC", mock.Anything, mock.Anything, "eth_estimateGas",
+		mock.MatchedBy(func(tx *ethsigner.Transaction) bool {
+			return true
+		})).
+		Return(&rpcbackend.RPCError{Message: "reverted for reason...", Data: *fftypes.JSONAnyPtr(
+			`"0x` + hex.EncodeToString(errData) + `"`,
+		)})
+
+	var req ffcapi.TransactionInput
+	err = json.Unmarshal([]byte(sampleGasEstimate), &req)
+	assert.NoError(t, err)
+	res, reason, err := c.GasEstimate(ctx, &req)
+	assert.Regexp(t, "FF23021.*this reason", err)
+	assert.Equal(t, ffcapi.ErrorReasonTransactionReverted, reason)
+	assert.Nil(t, res)
+
+}
+
+func TestFormatErrorComponentBadCV(t *testing.T) {
+	assert.Equal(t, "?", formatErrorComponent(context.Background(), &abi.ComponentValue{}))
+}