Skip to content

Commit

Permalink
Write Chain Target: Validate that signed report metadata matches requ…
Browse files Browse the repository at this point in the history
…est metadata (#14066)

* Hardcoded value for call with exact gas

* Record gasProvided in route function

* Add a getter for transmission gas limit

* Update snapshot

* Changeset

* Remove unused import

* Rename to gas limit

* Update gethwrappers

* Uncomment test code

* Remove copy/pasta comment

* Slight rename

* Allow retrying transmissions with more gas

* Only allow retrying failed transmissions

* Update snapshot

* Fix state for InvalidReceiver check

* Check for initial state

* Actually store gas limit provided to receiver

* Update gethwrappers

* Remove unused struct

* Correctly mark invalid receiver when receiver interface unsupported

* Create TransmissionInfo struct

* Update gethwrappers

* Bump gas limit

* Bump gas even more

* Update KeystoneFeedsConsumer.sol to implement IERC165

* Use getTransmissionInfo

* Use TransmissionState to determine if transmission should be created

* Fix test

* Fix trailing line

* Update a mock to the GetLatestValue("getTransmissionInfo") call in a test

* Remove TODO + replace panic with err

* Remove redundant empty lines

* Typo

* Fix nil pointer dereference in write target implementation

* Remove unused constant

* Name mapping values

* Add changeset

* Validate that report metadata matches request metadata

* Derive report metadata length from the struct

* Bytes() => Encode()

* Simplify decoding

* Undo redundant change

* More simplifications

* More cleanup

* Remove redundant comment

* Changeset

* Update tests

---------

Co-authored-by: app-token-issuer-infra-releng[bot] <120227048+app-token-issuer-infra-releng[bot]@users.noreply.github.com>
  • Loading branch information
1 parent 3015b53 commit 98b9054
Show file tree
Hide file tree
Showing 4 changed files with 198 additions and 177 deletions.
5 changes: 5 additions & 0 deletions .changeset/tall-poems-swim.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"chainlink": patch
---

#internal
145 changes: 103 additions & 42 deletions core/capabilities/targets/write_target.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package targets

import (
"bytes"
"context"
"encoding/binary"
"encoding/hex"
"fmt"
"math/big"
Expand All @@ -13,17 +15,13 @@ import (
"github.com/smartcontractkit/chainlink-common/pkg/capabilities/consensus/ocr3/types"
commontypes "github.com/smartcontractkit/chainlink-common/pkg/types"
"github.com/smartcontractkit/chainlink-common/pkg/types/query/primitives"
"github.com/smartcontractkit/chainlink-common/pkg/values"
"github.com/smartcontractkit/chainlink/v2/core/logger"
)

var (
_ capabilities.ActionCapability = &WriteTarget{}
)

// required field of target's config in the workflow spec
const signedReportField = "signed_report"

type WriteTarget struct {
cr commontypes.ContractReader
cw commontypes.ChainWriter
Expand Down Expand Up @@ -71,22 +69,105 @@ func NewWriteTarget(lggr logger.Logger, id string, cr commontypes.ContractReader
}
}

type EvmConfig struct {
// Note: This should be a shared type that the OCR3 package validates as well
type ReportV1Metadata struct {
Version uint8
WorkflowExecutionID [32]byte
Timestamp uint32
DonID uint32
DonConfigVersion uint32
WorkflowCID [32]byte
WorkflowName [10]byte
WorkflowOwner [20]byte
ReportID [2]byte
}

func (rm ReportV1Metadata) Encode() ([]byte, error) {
buf := new(bytes.Buffer)
err := binary.Write(buf, binary.BigEndian, rm)
if err != nil {
return nil, err
}
return buf.Bytes(), nil
}

func (rm ReportV1Metadata) Length() int {
bytes, err := rm.Encode()
if err != nil {
return 0
}
return len(bytes)
}

func decodeReportMetadata(data []byte) (metadata ReportV1Metadata, err error) {
if len(data) < metadata.Length() {
return metadata, fmt.Errorf("data too short: %d bytes", len(data))
}
return metadata, binary.Read(bytes.NewReader(data[:metadata.Length()]), binary.BigEndian, &metadata)
}

type Config struct {
// Address of the contract that will get the forwarded report
Address string
}

func parseConfig(rawConfig *values.Map) (config EvmConfig, err error) {
if rawConfig == nil {
return config, fmt.Errorf("missing config field")
type Inputs struct {
SignedReport types.SignedReport
}

type Request struct {
Metadata capabilities.RequestMetadata
Config Config
Inputs Inputs
}

func evaluate(rawRequest capabilities.CapabilityRequest) (r Request, err error) {
r.Metadata = rawRequest.Metadata

if rawRequest.Config == nil {
return r, fmt.Errorf("missing config field")
}

if err = rawRequest.Config.UnwrapTo(&r.Config); err != nil {
return r, err
}

if !common.IsHexAddress(r.Config.Address) {
return r, fmt.Errorf("'%v' is not a valid address", r.Config.Address)
}

if err := rawConfig.UnwrapTo(&config); err != nil {
return config, err
if rawRequest.Inputs == nil {
return r, fmt.Errorf("missing inputs field")
}
if !common.IsHexAddress(config.Address) {
return config, fmt.Errorf("'%v' is not a valid address", config.Address)

// required field of target's config in the workflow spec
const signedReportField = "signed_report"
signedReport, ok := rawRequest.Inputs.Underlying[signedReportField]
if !ok {
return r, fmt.Errorf("missing required field %s", signedReportField)
}

if err = signedReport.UnwrapTo(&r.Inputs.SignedReport); err != nil {
return r, err
}

reportMetadata, err := decodeReportMetadata(r.Inputs.SignedReport.Report)
if err != nil {
return r, err
}

if reportMetadata.Version != 1 {
return r, fmt.Errorf("unsupported report version: %d", reportMetadata.Version)
}
return config, nil

if hex.EncodeToString(reportMetadata.WorkflowExecutionID[:]) != rawRequest.Metadata.WorkflowExecutionID ||
hex.EncodeToString(reportMetadata.WorkflowOwner[:]) != rawRequest.Metadata.WorkflowOwner ||
hex.EncodeToString(reportMetadata.WorkflowName[:]) != rawRequest.Metadata.WorkflowName ||
hex.EncodeToString(reportMetadata.WorkflowCID[:]) != rawRequest.Metadata.WorkflowID {
return r, fmt.Errorf("report metadata does not match request metadata. reportMetadata: %+v, requestMetadata: %+v", reportMetadata, rawRequest.Metadata)
}

return r, nil
}

func success() <-chan capabilities.CapabilityResponse {
Expand All @@ -98,7 +179,7 @@ func success() <-chan capabilities.CapabilityResponse {
return callback
}

func (cap *WriteTarget) Execute(ctx context.Context, request capabilities.CapabilityRequest) (<-chan capabilities.CapabilityResponse, error) {
func (cap *WriteTarget) Execute(ctx context.Context, rawRequest capabilities.CapabilityRequest) (<-chan capabilities.CapabilityResponse, error) {
// Bind to the contract address on the write path.
// Bind() requires a connection to the node's RPCs and
// cannot be run during initialization.
Expand All @@ -114,47 +195,27 @@ func (cap *WriteTarget) Execute(ctx context.Context, request capabilities.Capabi
cap.bound = true
}

cap.lggr.Debugw("Execute", "request", request)
cap.lggr.Debugw("Execute", "rawRequest", rawRequest)

reqConfig, err := parseConfig(request.Config)
request, err := evaluate(rawRequest)
if err != nil {
return nil, err
}

if request.Inputs == nil {
return nil, fmt.Errorf("missing inputs field")
}

signedReport, ok := request.Inputs.Underlying[signedReportField]
if !ok {
return nil, fmt.Errorf("missing required field %s", signedReportField)
}

inputs := types.SignedReport{}
if err = signedReport.UnwrapTo(&inputs); err != nil {
return nil, err
}

if len(inputs.Report) == 0 {
// We received any empty report -- this means we should skip transmission.
cap.lggr.Debugw("Skipping empty report", "request", request)
return success(), nil
}
// TODO: validate encoded report is prefixed with workflowID and executionID that match the request meta

rawExecutionID, err := hex.DecodeString(request.Metadata.WorkflowExecutionID)
if err != nil {
return nil, err
}

// Check whether value was already transmitted on chain
queryInputs := struct {
Receiver string
WorkflowExecutionID []byte
ReportId []byte
}{
Receiver: reqConfig.Address,
Receiver: request.Config.Address,
WorkflowExecutionID: rawExecutionID,
ReportId: inputs.ID,
ReportId: request.Inputs.SignedReport.ID,
}
var transmissionInfo TransmissionInfo
if err = cap.cr.GetLatestValue(ctx, "forwarder", "getTransmissionInfo", primitives.Unconfirmed, queryInputs, &transmissionInfo); err != nil {
Expand All @@ -163,7 +224,7 @@ func (cap *WriteTarget) Execute(ctx context.Context, request capabilities.Capabi

switch {
case transmissionInfo.State == 0: // NOT_ATTEMPTED
cap.lggr.Infow("non-empty report - tranasmission not attempted - attempting to push to txmgr", "request", request, "reportLen", len(inputs.Report), "reportContextLen", len(inputs.Context), "nSignatures", len(inputs.Signatures), "executionID", request.Metadata.WorkflowExecutionID)
cap.lggr.Infow("non-empty report - tranasmission not attempted - 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)
case transmissionInfo.State == 1: // SUCCEEDED
cap.lggr.Infow("returning without a tranmission attempt - report already onchain ", "executionID", request.Metadata.WorkflowExecutionID)
return success(), nil
Expand All @@ -175,7 +236,7 @@ func (cap *WriteTarget) Execute(ctx context.Context, request capabilities.Capabi
cap.lggr.Infow("returning without a tranmission attempt - transmission already attempted and failed, sufficient gas was provided", "executionID", request.Metadata.WorkflowExecutionID, "receiverGasMinimum", cap.receiverGasMinimum, "transmissionGasLimit", transmissionInfo.GasLimit)
return success(), nil
} else {
cap.lggr.Infow("non-empty report - retrying a failed transmission - attempting to push to txmgr", "request", request, "reportLen", len(inputs.Report), "reportContextLen", len(inputs.Context), "nSignatures", len(inputs.Signatures), "executionID", request.Metadata.WorkflowExecutionID, "receiverGasMinimum", cap.receiverGasMinimum, "transmissionGasLimit", transmissionInfo.GasLimit)
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", cap.receiverGasMinimum, "transmissionGasLimit", transmissionInfo.GasLimit)
}
default:
return nil, fmt.Errorf("unexpected transmission state: %v", transmissionInfo.State)
Expand All @@ -194,7 +255,7 @@ func (cap *WriteTarget) Execute(ctx context.Context, request capabilities.Capabi
RawReport []byte
ReportContext []byte
Signatures [][]byte
}{reqConfig.Address, inputs.Report, inputs.Context, inputs.Signatures}
}{request.Config.Address, request.Inputs.SignedReport.Report, request.Inputs.SignedReport.Context, request.Inputs.SignedReport.Signatures}

if req.RawReport == nil {
req.RawReport = make([]byte, 0)
Expand Down
88 changes: 39 additions & 49 deletions core/capabilities/targets/write_target_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package targets_test

import (
"context"
"encoding/hex"
"errors"
"math/big"
"testing"
Expand Down Expand Up @@ -38,14 +39,36 @@ func TestWriteTarget(t *testing.T) {
})
require.NoError(t, err)

reportMetadata := targets.ReportV1Metadata{
Version: 1,
WorkflowExecutionID: [32]byte{},
Timestamp: 0,
DonID: 0,
DonConfigVersion: 0,
WorkflowCID: [32]byte{},
WorkflowName: [10]byte{},
WorkflowOwner: [20]byte{},
ReportID: [2]byte{},
}

reportMetadataBytes, err := reportMetadata.Encode()
require.NoError(t, err)

validInputs, err := values.NewMap(map[string]any{
"signed_report": map[string]any{
"report": []byte{1, 2, 3},
"report": reportMetadataBytes,
"signatures": [][]byte{},
},
})
require.NoError(t, err)

validMetadata := capabilities.RequestMetadata{
WorkflowID: hex.EncodeToString(reportMetadata.WorkflowCID[:]),
WorkflowOwner: hex.EncodeToString(reportMetadata.WorkflowOwner[:]),
WorkflowName: hex.EncodeToString(reportMetadata.WorkflowName[:]),
WorkflowExecutionID: hex.EncodeToString(reportMetadata.WorkflowExecutionID[:]),
}

cr.On("Bind", mock.Anything, []types.BoundContract{
{
Address: forwarderAddr,
Expand All @@ -69,11 +92,9 @@ func TestWriteTarget(t *testing.T) {

t.Run("succeeds with valid report", func(t *testing.T) {
req := capabilities.CapabilityRequest{
Metadata: capabilities.RequestMetadata{
WorkflowID: "test-id",
},
Config: config,
Inputs: validInputs,
Metadata: validMetadata,
Config: config,
Inputs: validInputs,
}

ch, err2 := writeTarget.Execute(ctx, req)
Expand All @@ -82,36 +103,11 @@ func TestWriteTarget(t *testing.T) {
require.NotNil(t, response)
})

t.Run("succeeds with empty report", func(t *testing.T) {
emptyInputs, err2 := values.NewMap(map[string]any{
"signed_report": map[string]any{
"report": []byte{},
},
"signatures": [][]byte{},
})

require.NoError(t, err2)
req := capabilities.CapabilityRequest{
Metadata: capabilities.RequestMetadata{
WorkflowExecutionID: "test-id",
},
Config: config,
Inputs: emptyInputs,
}

ch, err2 := writeTarget.Execute(ctx, req)
require.NoError(t, err2)
response := <-ch
require.Nil(t, response.Value)
})

t.Run("fails when ChainReader's GetLatestValue returns error", func(t *testing.T) {
req := capabilities.CapabilityRequest{
Metadata: capabilities.RequestMetadata{
WorkflowID: "test-id",
},
Config: config,
Inputs: validInputs,
Metadata: validMetadata,
Config: config,
Inputs: validInputs,
}
cr.On("GetLatestValue", mock.Anything, "forwarder", "getTransmissionInfo", mock.Anything, mock.Anything, mock.Anything).Return(errors.New("reader error"))

Expand All @@ -121,11 +117,9 @@ func TestWriteTarget(t *testing.T) {

t.Run("fails when ChainWriter's SubmitTransaction returns error", func(t *testing.T) {
req := capabilities.CapabilityRequest{
Metadata: capabilities.RequestMetadata{
WorkflowID: "test-id",
},
Config: config,
Inputs: validInputs,
Metadata: validMetadata,
Config: config,
Inputs: validInputs,
}
cw.On("SubmitTransaction", mock.Anything, "forwarder", "report", mock.Anything, mock.Anything, forwarderAddr, mock.Anything, mock.Anything).Return(errors.New("writer error"))

Expand All @@ -152,23 +146,19 @@ func TestWriteTarget(t *testing.T) {

t.Run("fails with nil config", func(t *testing.T) {
req := capabilities.CapabilityRequest{
Metadata: capabilities.RequestMetadata{
WorkflowID: "test-id",
},
Config: nil,
Inputs: validInputs,
Metadata: validMetadata,
Config: nil,
Inputs: validInputs,
}
_, err2 := writeTarget.Execute(ctx, req)
require.Error(t, err2)
})

t.Run("fails with nil inputs", func(t *testing.T) {
req := capabilities.CapabilityRequest{
Metadata: capabilities.RequestMetadata{
WorkflowID: "test-id",
},
Config: config,
Inputs: nil,
Metadata: validMetadata,
Config: config,
Inputs: nil,
}
_, err2 := writeTarget.Execute(ctx, req)
require.Error(t, err2)
Expand Down
Loading

0 comments on commit 98b9054

Please sign in to comment.