Skip to content

Commit

Permalink
[CAPPL-525] return error when receiver reverts or is invalid (#16343)
Browse files Browse the repository at this point in the history 
* return error when receiver reverts or is invalid

* address comment and lint
  • Loading branch information
@shileiwill
shileiwill authored Feb 14, 2025
1 parent c38d0b0 commit 3a0b638
Show file tree
Hide file tree
Showing 3 changed files with 157 additions and 14 deletions.
5 changes: 5 additions & 0 deletions .changeset/blue-ducks-compete.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"chainlink": minor
---

#bugfix
23 changes: 14 additions & 9 deletions core/capabilities/targets/write_target.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ import (
)

var (
_ capabilities.TargetCapability = &WriteTarget{}
_ capabilities.TargetCapability = &WriteTarget{}
ErrTxFailed = errors.New("submitted transaction failed")
)

const transactionStatusCheckInterval = 2 * time.Second
Expand Down Expand Up @@ -264,15 +265,15 @@ func (cap *WriteTarget) Execute(ctx context.Context, rawRequest capabilities.Cap
return capabilities.CapabilityResponse{}, nil
case transmissionInfo.State == TransmissionStateInvalidReceiver:
cap.lggr.Infow("returning without a transmission attempt - transmission already attempted, receiver was marked as invalid", "executionID", request.Metadata.WorkflowExecutionID)
return capabilities.CapabilityResponse{}, nil
return capabilities.CapabilityResponse{}, ErrTxFailed
case transmissionInfo.State == TransmissionStateFailed:
receiverGasMinimum := cap.receiverGasMinimum
if request.Config.GasLimit != nil {
receiverGasMinimum = *request.Config.GasLimit - ForwarderContractLogicGasCost
}
if transmissionInfo.GasLimit.Uint64() > receiverGasMinimum {
cap.lggr.Infow("returning without a transmission attempt - transmission already attempted and failed, sufficient gas was provided", "executionID", request.Metadata.WorkflowExecutionID, "receiverGasMinimum", receiverGasMinimum, "transmissionGasLimit", transmissionInfo.GasLimit)
return capabilities.CapabilityResponse{}, nil
return capabilities.CapabilityResponse{}, ErrTxFailed
} else {
cap.lggr.Infow("non-empty report - retrying a failed transmission - attempting to push to txmgr", "request", request, "reportLen", len(request.Inputs.SignedReport.Report), "reportContextLen", len(request.Inputs.SignedReport.Context), "nSignatures", len(request.Inputs.SignedReport.Signatures), "executionID", request.Metadata.WorkflowExecutionID, "receiverGasMinimum", receiverGasMinimum, "transmissionGasLimit", transmissionInfo.GasLimit)
}
Expand Down Expand Up @@ -350,13 +351,13 @@ func (cap *WriteTarget) Execute(ctx context.Context, rawRequest capabilities.Cap

// This is counterintuitive, but the tx manager is currently returning unconfirmed whenever the tx is confirmed
// current implementation here: https://github.com/smartcontractkit/chainlink-framework/blob/main/chains/txmgr/txmgr.go#L697
// so we need to check if we where able to write to the consumer contract to determine if the transaction was successful
// so we need to check if we were able to write to the consumer contract to determine if the transaction was successful
if transmissionInfo.State == TransmissionStateSucceeded {
cap.lggr.Debugw("Transaction confirmed", "request", request, "transaction", txID)
return capabilities.CapabilityResponse{}, nil
} else {
} else if transmissionInfo.State == TransmissionStateFailed || transmissionInfo.State == TransmissionStateInvalidReceiver {
cap.lggr.Errorw("Transaction written to the forwarder, but failed to be written to the consumer contract", "request", request, "transaction", txID, "transmissionState", transmissionInfo.State)
msg := "failed to submit transaction with ID: " + txID.String()
msg := "transaction written to the forwarder, but failed to be written to the consumer contract, transaction ID: " + txID.String()
err = cap.emitter.With(
platform.KeyWorkflowID, request.Metadata.WorkflowID,
platform.KeyWorkflowName, request.Metadata.DecodedWorkflowName,
Expand All @@ -366,15 +367,19 @@ func (cap *WriteTarget) Execute(ctx context.Context, rawRequest capabilities.Cap
if err != nil {
cap.lggr.Errorf("failed to send custom message with msg: %s, err: %v", msg, err)
}
return capabilities.CapabilityResponse{}, errors.New("submitted transaction failed")
return capabilities.CapabilityResponse{}, ErrTxFailed
} else {
// TransmissionStateNotAttempted is not expected here, but we'll log it just in case
cap.lggr.Debugw("Transaction confirmed but transmission not attempted, this should never happen", "request", request, "transaction", txID)
return capabilities.CapabilityResponse{}, errors.New("transmission not attempted")
}
case commontypes.Finalized:
cap.lggr.Debugw("Transaction finalized", "request", request, "transaction", txID)
return capabilities.CapabilityResponse{}, nil
case commontypes.Failed, commontypes.Fatal:
cap.lggr.Error("Transaction failed", "request", request, "transaction", txID)

msg := "failed to submit transaction with ID: " + txID.String()
msg := "transaction failed to be written to the forwarder, transaction ID: " + txID.String()
err = cap.emitter.With(
platform.KeyWorkflowID, request.Metadata.WorkflowID,
platform.KeyWorkflowName, request.Metadata.DecodedWorkflowName,
Expand All @@ -384,7 +389,7 @@ func (cap *WriteTarget) Execute(ctx context.Context, rawRequest capabilities.Cap
if err != nil {
cap.lggr.Errorf("failed to send custom message with msg: %s, err: %v", msg, err)
}
return capabilities.CapabilityResponse{}, errors.New("submitted transaction failed")
return capabilities.CapabilityResponse{}, ErrTxFailed
default:
cap.lggr.Debugw("Unexpected transaction status", "request", request, "transaction", txID, "status", txStatus)
}
Expand Down
143 changes: 138 additions & 5 deletions core/capabilities/targets/write_target_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"testing"

"github.com/ethereum/go-ethereum/common"

"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"

Expand All @@ -33,6 +32,7 @@ type testHarness struct {
writeTarget *targets.WriteTarget
forwarderAddr string
binding types.BoundContract
gasLimit uint64
}

func setup(t *testing.T) testHarness {
Expand All @@ -44,7 +44,8 @@ func setup(t *testing.T) testHarness {
forwarderA := testutils.NewAddress()
forwarderAddr := forwarderA.Hex()

writeTarget := targets.NewWriteTarget(lggr, "test-write-target@1.0.0", cr, cw, forwarderAddr, 400_000)
var txGasLimit uint64 = 400_000
writeTarget := targets.NewWriteTarget(lggr, "test-write-target@1.0.0", cr, cw, forwarderAddr, txGasLimit)
require.NotNil(t, writeTarget)

config, err := values.NewMap(map[string]any{
Expand Down Expand Up @@ -106,6 +107,7 @@ func setup(t *testing.T) testHarness {
writeTarget: writeTarget,
forwarderAddr: forwarderAddr,
binding: binding,
gasLimit: txGasLimit - targets.ForwarderContractLogicGasCost,
}
}
func TestWriteTarget(t *testing.T) {
Expand Down Expand Up @@ -320,7 +322,7 @@ func TestWriteTarget_UnconfirmedTransaction(t *testing.T) {
GasLimit: big.NewInt(0),
InvalidReceiver: false,
State: targets.TransmissionStateSucceeded,
Success: false,
Success: true,
TransmissionId: [32]byte{},
Transmitter: common.HexToAddress("0x0"),
}
Expand All @@ -341,7 +343,7 @@ func TestWriteTarget_UnconfirmedTransaction(t *testing.T) {
require.NotNil(t, response)
})

t.Run("transaction written to the forwarder, but failed to be written to the consumer contract", func(t *testing.T) {
t.Run("getTransmissionInfo twice, transaction written to the forwarder, but failed to be written to the consumer contract because of revert in receiver", func(t *testing.T) {
th := setup(t)
th.cw.On("SubmitTransaction", mock.Anything, "forwarder", "report", mock.Anything, mock.Anything, th.forwarderAddr, mock.Anything, mock.Anything).Return(nil).Once()
callCount := 0
Expand Down Expand Up @@ -378,6 +380,137 @@ func TestWriteTarget_UnconfirmedTransaction(t *testing.T) {
ctx := testutils.Context(t)
_, err2 := th.writeTarget.Execute(ctx, req)
require.Error(t, err2)
require.Contains(t, err2.Error(), "submitted transaction failed")
require.ErrorIs(t, err2, targets.ErrTxFailed)
})

t.Run("getTransmissionInfo twice, transaction written to the forwarder, but failed to be written to the consumer contract because of invalid receiver", func(t *testing.T) {
th := setup(t)
th.cw.On("SubmitTransaction", mock.Anything, "forwarder", "report", mock.Anything, mock.Anything, th.forwarderAddr, mock.Anything, mock.Anything).Return(nil).Once()
callCount := 0
th.cr.On("GetLatestValue", mock.Anything, th.binding.ReadIdentifier("getTransmissionInfo"), mock.Anything, mock.Anything, mock.Anything).Return(nil).Run(func(args mock.Arguments) {
transmissionInfo := args.Get(4).(*targets.TransmissionInfo)
if callCount == 0 {
*transmissionInfo = targets.TransmissionInfo{
GasLimit: big.NewInt(0),
InvalidReceiver: false,
State: targets.TransmissionStateNotAttempted,
Success: false,
TransmissionId: [32]byte{},
Transmitter: common.HexToAddress("0x0"),
}
} else {
*transmissionInfo = targets.TransmissionInfo{
GasLimit: big.NewInt(0),
InvalidReceiver: true,
State: targets.TransmissionStateInvalidReceiver,
Success: false,
TransmissionId: [32]byte{},
Transmitter: common.HexToAddress("0x0"),
}
}
callCount++
})
req := capabilities.CapabilityRequest{
Metadata: th.validMetadata,
Config: th.config,
Inputs: th.validInputs,
}

th.cw.On("GetTransactionStatus", mock.Anything, mock.Anything).Return(types.Unconfirmed, nil).Once()
ctx := testutils.Context(t)
_, err2 := th.writeTarget.Execute(ctx, req)
require.Error(t, err2)
require.ErrorIs(t, err2, targets.ErrTxFailed)
})

t.Run("getTransmissionInfo once, transaction written to the forwarder, but failed to be written to the consumer contract because of invalid receiver", func(t *testing.T) {
th := setup(t)
th.cr.On("GetLatestValue", mock.Anything, th.binding.ReadIdentifier("getTransmissionInfo"), mock.Anything, mock.Anything, mock.Anything).Once().Return(nil).Run(func(args mock.Arguments) {
transmissionInfo := args.Get(4).(*targets.TransmissionInfo)
*transmissionInfo = targets.TransmissionInfo{
GasLimit: big.NewInt(0),
InvalidReceiver: true,
State: targets.TransmissionStateInvalidReceiver,
Success: false,
TransmissionId: [32]byte{},
Transmitter: common.HexToAddress("0x0"),
}
})
req := capabilities.CapabilityRequest{
Metadata: th.validMetadata,
Config: th.config,
Inputs: th.validInputs,
}

ctx := testutils.Context(t)
_, err2 := th.writeTarget.Execute(ctx, req)
require.Error(t, err2)
require.ErrorIs(t, err2, targets.ErrTxFailed)
})

t.Run("getTransmissionInfo once, transaction written to the forwarder, but failed to be written to the consumer contract because of revert in receiver", func(t *testing.T) {
th := setup(t)
th.cr.On("GetLatestValue", mock.Anything, th.binding.ReadIdentifier("getTransmissionInfo"), mock.Anything, mock.Anything, mock.Anything).Once().Return(nil).Run(func(args mock.Arguments) {
transmissionInfo := args.Get(4).(*targets.TransmissionInfo)
*transmissionInfo = targets.TransmissionInfo{
GasLimit: big.NewInt(0).SetUint64(th.gasLimit + 1), // has sufficient gas
InvalidReceiver: false,
State: targets.TransmissionStateFailed,
Success: false,
TransmissionId: [32]byte{},
Transmitter: common.HexToAddress("0x0"),
}
})
req := capabilities.CapabilityRequest{
Metadata: th.validMetadata,
Config: th.config,
Inputs: th.validInputs,
}

ctx := testutils.Context(t)
_, err2 := th.writeTarget.Execute(ctx, req)
require.Error(t, err2)
require.ErrorIs(t, err2, targets.ErrTxFailed)
})

t.Run("getTransmissionInfo twice, first time receiver reverted because of insufficient gas, second time succeeded", func(t *testing.T) {
th := setup(t)
th.cw.On("SubmitTransaction", mock.Anything, "forwarder", "report", mock.Anything, mock.Anything, th.forwarderAddr, mock.Anything, mock.Anything).Return(nil).Once()
callCount := 0
th.cr.On("GetLatestValue", mock.Anything, th.binding.ReadIdentifier("getTransmissionInfo"), mock.Anything, mock.Anything, mock.Anything).Return(nil).Run(func(args mock.Arguments) {
transmissionInfo := args.Get(4).(*targets.TransmissionInfo)
if callCount == 0 {
*transmissionInfo = targets.TransmissionInfo{
GasLimit: big.NewInt(0).SetUint64(th.gasLimit - 1), // has insufficient gas
InvalidReceiver: false,
State: targets.TransmissionStateFailed,
Success: false,
TransmissionId: [32]byte{},
Transmitter: common.HexToAddress("0x0"),
}
} else {
*transmissionInfo = targets.TransmissionInfo{
GasLimit: big.NewInt(0).SetUint64(th.gasLimit + 1), // has sufficient gas
InvalidReceiver: false,
State: targets.TransmissionStateSucceeded,
Success: true,
TransmissionId: [32]byte{},
Transmitter: common.HexToAddress("0x0"),
}
}
callCount++
})
req := capabilities.CapabilityRequest{
Metadata: th.validMetadata,
Config: th.config,
Inputs: th.validInputs,
}

th.cw.On("GetTransactionStatus", mock.Anything, mock.Anything).Return(types.Unconfirmed, nil).Once()

ctx := testutils.Context(t)
response, err2 := th.writeTarget.Execute(ctx, req)
require.NoError(t, err2)
require.NotNil(t, response)
})
}

0 comments on commit 3a0b638

Please sign in to comment.