Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions core/capabilities/launcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"time"

"github.com/Masterminds/semver/v3"
ocrtypes "github.com/smartcontractkit/libocr/offchainreporting2plus/types"
"github.com/smartcontractkit/libocr/ragep2p"
ragetypes "github.com/smartcontractkit/libocr/ragep2p/types"

Expand Down Expand Up @@ -483,7 +484,7 @@ func (w *launcher) addRemoteCapability(ctx context.Context, cid string, capabili

methodConfig := capabilityConfig.CapabilityMethodConfig
if methodConfig != nil { // v2 capability - handle via CombinedClient
errAdd := w.addRemoteCapabilityV2(ctx, capability.ID, methodConfig, myDON, remoteDON)
errAdd := w.addRemoteCapabilityV2(ctx, capability.ID, methodConfig, myDON, remoteDON, capabilityConfig.Ocr3Configs)
if errAdd != nil {
return fmt.Errorf("failed to add remote v2 capability %s: %w", capability.ID, errAdd)
}
Expand Down Expand Up @@ -574,7 +575,7 @@ func (w *launcher) addRemoteCapability(ctx context.Context, cid string, capabili
w.cachedShims.executableClients[shimKey] = execCap
}
// V1 capabilities read transmission schedule from every request
if errCfg := execCap.SetConfig(info, myDON.DON, defaultTargetRequestTimeout, nil); errCfg != nil {
if errCfg := execCap.SetConfig(info, myDON.DON, defaultTargetRequestTimeout, nil, nil); errCfg != nil {
return nil, fmt.Errorf("failed to set trigger config: %w", errCfg)
}
return execCap.(capabilityService), nil
Expand All @@ -600,7 +601,7 @@ func (w *launcher) addRemoteCapability(ctx context.Context, cid string, capabili
w.cachedShims.executableClients[shimKey] = execCap
}
// V1 capabilities read transmission schedule from every request
if errCfg := execCap.SetConfig(info, myDON.DON, defaultTargetRequestTimeout, nil); errCfg != nil {
if errCfg := execCap.SetConfig(info, myDON.DON, defaultTargetRequestTimeout, nil, nil); errCfg != nil {
return nil, fmt.Errorf("failed to set trigger config: %w", errCfg)
}
return execCap.(capabilityService), nil
Expand Down Expand Up @@ -907,7 +908,7 @@ func signersFor(don registrysyncer.DON, localRegistry *registrysyncer.LocalRegis
}

// Add a V2 capability with multiple methods, using CombinedClient.
func (w *launcher) addRemoteCapabilityV2(ctx context.Context, capID string, methodConfig map[string]capabilities.CapabilityMethodConfig, myDON registrysyncer.DON, remoteDON registrysyncer.DON) error {
func (w *launcher) addRemoteCapabilityV2(ctx context.Context, capID string, methodConfig map[string]capabilities.CapabilityMethodConfig, myDON registrysyncer.DON, remoteDON registrysyncer.DON, capabilityOcr3Configs map[string]ocrtypes.ContractConfig) error {
info, err := capabilities.NewRemoteCapabilityInfo(
capID,
capabilities.CapabilityTypeCombined,
Expand Down Expand Up @@ -962,7 +963,7 @@ func (w *launcher) addRemoteCapabilityV2(ctx context.Context, capID string, meth
Schedule: transmission.EnumToString(config.RemoteExecutableConfig.TransmissionSchedule),
DeltaStage: config.RemoteExecutableConfig.DeltaStage,
}
err := client.SetConfig(info, myDON.DON, config.RemoteExecutableConfig.RequestTimeout, transmissionConfig)
err := client.SetConfig(info, myDON.DON, config.RemoteExecutableConfig.RequestTimeout, transmissionConfig, capabilityOcr3Configs)
if err != nil {
w.lggr.Errorw("failed to update client config", "capID", capID, "method", method, "error", err)
continue
Expand Down
13 changes: 9 additions & 4 deletions core/capabilities/remote/executable/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import (
"sync/atomic"
"time"

ocrtypes "github.com/smartcontractkit/libocr/offchainreporting2plus/types"

commoncap "github.com/smartcontractkit/chainlink-common/pkg/capabilities"
"github.com/smartcontractkit/chainlink-common/pkg/capabilities/pb"
"github.com/smartcontractkit/chainlink-common/pkg/logger"
Expand Down Expand Up @@ -46,12 +48,14 @@ type dynamicConfig struct {
requestTimeout time.Duration
// Has to be set only for V2 capabilities. V1 capabilities read transmission schedule from every request.
transmissionConfig *transmission.TransmissionConfig
// Has to be set only for V2 capabilities using OCR.
ocr3Configs map[string]ocrtypes.ContractConfig
}

type Client interface {
commoncap.ExecutableCapability
Receive(ctx context.Context, msg *types.MessageBody)
SetConfig(remoteCapabilityInfo commoncap.CapabilityInfo, localDonInfo commoncap.DON, requestTimeout time.Duration, transmissionConfig *transmission.TransmissionConfig) error
SetConfig(remoteCapabilityInfo commoncap.CapabilityInfo, localDonInfo commoncap.DON, requestTimeout time.Duration, transmissionConfig *transmission.TransmissionConfig, ocr3Configs map[string]ocrtypes.ContractConfig) error
}

var _ Client = &client{}
Expand All @@ -78,7 +82,7 @@ func NewClient(capabilityID string, capMethodName string, dispatcher types.Dispa

// SetConfig sets the remote capability configuration dynamically
// TransmissionConfig has to be set only for V2 capabilities. V1 capabilities read transmission schedule from every request.
func (c *client) SetConfig(remoteCapabilityInfo commoncap.CapabilityInfo, localDonInfo commoncap.DON, requestTimeout time.Duration, transmissionConfig *transmission.TransmissionConfig) error {
func (c *client) SetConfig(remoteCapabilityInfo commoncap.CapabilityInfo, localDonInfo commoncap.DON, requestTimeout time.Duration, transmissionConfig *transmission.TransmissionConfig, ocr3Configs map[string]ocrtypes.ContractConfig) error {
if remoteCapabilityInfo.ID == "" || remoteCapabilityInfo.ID != c.capabilityID {
return fmt.Errorf("capability info provided does not match the client's capabilityID: %s != %s", remoteCapabilityInfo.ID, c.capabilityID)
}
Expand All @@ -98,8 +102,9 @@ func (c *client) SetConfig(remoteCapabilityInfo commoncap.CapabilityInfo, localD
localDONInfo: localDonInfo,
requestTimeout: requestTimeout,
transmissionConfig: transmissionConfig,
ocr3Configs: ocr3Configs,
})
c.lggr.Infow("SetConfig", "remoteDONName", remoteCapabilityInfo.DON.Name, "remoteDONID", remoteCapabilityInfo.DON.ID, "requestTimeout", requestTimeout, "transmissionConfig", transmissionConfig)
c.lggr.Infow("SetConfig", "remoteDONName", remoteCapabilityInfo.DON.Name, "remoteDONID", remoteCapabilityInfo.DON.ID, "requestTimeout", requestTimeout, "transmissionConfig", transmissionConfig, "ocr3Configs", ocr3Configs)
return nil
}

Expand Down Expand Up @@ -234,7 +239,7 @@ func (c *client) Execute(ctx context.Context, capReq commoncap.CapabilityRequest
}

req, err := request.NewClientExecuteRequest(ctx, c.lggr, capReq, cfg.remoteCapabilityInfo, cfg.localDONInfo, c.dispatcher,
cfg.requestTimeout, cfg.transmissionConfig, c.capMethodName)
cfg.requestTimeout, cfg.transmissionConfig, c.capMethodName, cfg.ocr3Configs)
if err != nil {
return commoncap.CapabilityResponse{}, fmt.Errorf("failed to create client request: %w", err)
}
Expand Down
18 changes: 9 additions & 9 deletions core/capabilities/remote/executable/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ func testClient(t *testing.T, numWorkflowPeers int, workflowNodeResponseTimeout
for i := range numWorkflowPeers {
workflowPeerDispatcher := broker.NewDispatcherForNode(workflowPeers[i])
caller := executable.NewClient(capInfo.ID, "", workflowPeerDispatcher, lggr)
err := caller.SetConfig(capInfo, workflowDonInfo, workflowNodeResponseTimeout, nil)
err := caller.SetConfig(capInfo, workflowDonInfo, workflowNodeResponseTimeout, nil, nil)
require.NoError(t, err)
servicetest.Run(t, caller)
broker.RegisterReceiverNode(workflowPeers[i], caller)
Expand Down Expand Up @@ -403,7 +403,7 @@ func TestClient_SetConfig(t *testing.T) {
DeltaStage: 10 * time.Millisecond,
}

err := client.SetConfig(validCapInfo, validDonInfo, validTimeout, transmissionConfig)
err := client.SetConfig(validCapInfo, validDonInfo, validTimeout, transmissionConfig, nil)
require.NoError(t, err)

// Verify config was set
Expand All @@ -418,7 +418,7 @@ func TestClient_SetConfig(t *testing.T) {
CapabilityType: commoncap.CapabilityTypeAction,
}

err := client.SetConfig(invalidCapInfo, validDonInfo, validTimeout, nil)
err := client.SetConfig(invalidCapInfo, validDonInfo, validTimeout, nil, nil)
require.Error(t, err)
assert.Contains(t, err.Error(), "capability info provided does not match the client's capabilityID")
assert.Contains(t, err.Error(), "different_capability@1.0.0 != test_capability@1.0.0")
Expand All @@ -431,15 +431,15 @@ func TestClient_SetConfig(t *testing.T) {
F: 0,
}

err := client.SetConfig(validCapInfo, invalidDonInfo, validTimeout, nil)
err := client.SetConfig(validCapInfo, invalidDonInfo, validTimeout, nil, nil)
require.Error(t, err)
assert.Contains(t, err.Error(), "empty localDonInfo provided")
})

t.Run("successful config update", func(t *testing.T) {
// Set initial config
initialTimeout := 10 * time.Second
err := client.SetConfig(validCapInfo, validDonInfo, initialTimeout, nil)
err := client.SetConfig(validCapInfo, validDonInfo, initialTimeout, nil, nil)
require.NoError(t, err)

// Replace with new config
Expand All @@ -450,7 +450,7 @@ func TestClient_SetConfig(t *testing.T) {
F: 1,
}

err = client.SetConfig(validCapInfo, newDonInfo, newTimeout, nil)
err = client.SetConfig(validCapInfo, newDonInfo, newTimeout, nil, nil)
require.NoError(t, err)

// Verify the config was completely replaced
Expand Down Expand Up @@ -494,7 +494,7 @@ func TestClient_SetConfig_StartClose(t *testing.T) {
})

t.Run("start succeeds after config set", func(t *testing.T) {
require.NoError(t, client.SetConfig(validCapInfo, validDonInfo, validTimeout, nil))
require.NoError(t, client.SetConfig(validCapInfo, validDonInfo, validTimeout, nil, nil))
require.NoError(t, client.Start(ctx))
require.NoError(t, client.Close())
})
Expand All @@ -504,12 +504,12 @@ func TestClient_SetConfig_StartClose(t *testing.T) {
freshClient := executable.NewClient(capabilityID, "execute", dispatcher, lggr)

// Set initial config and start
require.NoError(t, freshClient.SetConfig(validCapInfo, validDonInfo, validTimeout, nil))
require.NoError(t, freshClient.SetConfig(validCapInfo, validDonInfo, validTimeout, nil, nil))
require.NoError(t, freshClient.Start(ctx))

// Update config while running
validCapInfo.Description = "new description"
require.NoError(t, freshClient.SetConfig(validCapInfo, validDonInfo, validTimeout, nil))
require.NoError(t, freshClient.SetConfig(validCapInfo, validDonInfo, validTimeout, nil, nil))

// Verify config was updated
info, err := freshClient.Info(ctx)
Expand Down
2 changes: 1 addition & 1 deletion core/capabilities/remote/executable/endtoend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ func testRemoteExecutableCapability(ctx context.Context, t *testing.T, underlyin
for i := range numWorkflowPeers {
workflowPeerDispatcher := broker.NewDispatcherForNode(workflowPeers[i])
workflowNode := executable.NewClient(capInfo.ID, "", workflowPeerDispatcher, lggr)
err := workflowNode.SetConfig(capInfo, workflowDonInfo, workflowNodeTimeout, nil)
err := workflowNode.SetConfig(capInfo, workflowDonInfo, workflowNodeTimeout, nil, nil)
require.NoError(t, err)
servicetest.Run(t, workflowNode)
broker.RegisterReceiverNode(workflowPeers[i], workflowNode)
Expand Down
117 changes: 100 additions & 17 deletions core/capabilities/remote/executable/request/client_request.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,12 @@ import (
"sync"
"time"

ocrtypes "github.com/smartcontractkit/libocr/offchainreporting2plus/types"
"google.golang.org/protobuf/proto"

ragep2ptypes "github.com/smartcontractkit/libocr/ragep2p/types"

"github.com/smartcontractkit/chainlink-common/keystore/corekeys/ocr2key"
"github.com/smartcontractkit/chainlink-common/pkg/beholder"
commoncap "github.com/smartcontractkit/chainlink-common/pkg/capabilities"
"github.com/smartcontractkit/chainlink-common/pkg/capabilities/pb"
Expand All @@ -33,16 +35,19 @@ type clientResponse struct {
}

type ClientRequest struct {
id string
cancelFn context.CancelFunc
responseCh chan clientResponse
createdAt time.Time
responseIDCount map[[32]byte]int
meteringResponses map[[32]byte][]commoncap.MeteringNodeDetail
errorCount map[string]int
totalErrorCount int
responseReceived map[p2ptypes.PeerID]bool
lggr logger.Logger
id string
cancelFn context.CancelFunc
responseCh chan clientResponse
createdAt time.Time
responseIDCount map[[32]byte]int
meteringResponses map[[32]byte][]commoncap.MeteringNodeDetail
errorCount map[string]int
totalErrorCount int
responseReceived map[p2ptypes.PeerID]bool
lggr logger.Logger
ocr3Configs map[string]ocrtypes.ContractConfig
workflowExecutionID string
referenceID string

requiredIdenticalResponses int
remoteNodeCount int
Expand All @@ -58,6 +63,7 @@ type ClientRequest struct {
func NewClientExecuteRequest(ctx context.Context, lggr logger.Logger, req commoncap.CapabilityRequest,
remoteCapabilityInfo commoncap.CapabilityInfo, localDonInfo commoncap.DON, dispatcher types.Dispatcher,
requestTimeout time.Duration, transmissionConfig *transmission.TransmissionConfig, capMethodName string,
ocr3Configs map[string]ocrtypes.ContractConfig,
) (*ClientRequest, error) {
rawRequest, err := proto.MarshalOptions{Deterministic: true}.Marshal(pb.CapabilityRequestToProto(req))
if err != nil {
Expand Down Expand Up @@ -87,14 +93,15 @@ func NewClientExecuteRequest(ctx context.Context, lggr logger.Logger, req common
}

lggr = logger.With(lggr, "requestId", requestID) // cap ID and method name included in the parent logger
return newClientRequest(ctx, lggr, requestID, remoteCapabilityInfo, localDonInfo, dispatcher, requestTimeout, tc, types.MethodExecute, rawRequest, workflowExecutionID, req.Metadata.ReferenceID, capMethodName)
return newClientRequest(ctx, lggr, requestID, remoteCapabilityInfo, localDonInfo, dispatcher, requestTimeout, tc, types.MethodExecute, rawRequest, workflowExecutionID, req.Metadata.ReferenceID, capMethodName, ocr3Configs)
}

var defaultDelayMargin = 10 * time.Second

func newClientRequest(ctx context.Context, lggr logger.Logger, requestID string, remoteCapabilityInfo commoncap.CapabilityInfo,
localDonInfo commoncap.DON, dispatcher types.Dispatcher, requestTimeout time.Duration,
tc transmission.TransmissionConfig, methodType string, rawRequest []byte, workflowExecutionID string, stepRef string, capMethodName string,
ocr3Configs map[string]ocrtypes.ContractConfig,
) (*ClientRequest, error) {
remoteCapabilityDonInfo := remoteCapabilityInfo.DON
if remoteCapabilityDonInfo == nil {
Expand Down Expand Up @@ -200,6 +207,9 @@ func newClientRequest(ctx context.Context, lggr logger.Logger, requestID string,
responseCh: make(chan clientResponse, 1),
wg: &wg,
lggr: lggr,
ocr3Configs: ocr3Configs,
workflowExecutionID: workflowExecutionID,
referenceID: stepRef,
}, nil
}

Expand Down Expand Up @@ -301,6 +311,34 @@ func (c *ClientRequest) OnMessage(_ context.Context, msg *types.MessageBody) err
c.responseReceived[sender] = true

if msg.Error == types.Error_OK {
resp, err := pb.UnmarshalCapabilityResponse(msg.Payload)
if err != nil {
return fmt.Errorf("failed to unmarshal capability response: %w", err)
}

if resp.Metadata.OCRAttestation != nil {
rpt, err := extractMeteringFromMetadata(sender, resp.Metadata)
if err != nil {
return fmt.Errorf("failed to extract metering detail from metadata: %w", err)
}
// Since signatures are provided switch to OCR based validation. It's enough to get 1 response with F+1 signatures
// to be confident that the response is honest.
err = c.verifyAttestation(resp, rpt)
if err != nil {
c.lggr.Errorw("failed to verify capability response OCR attestation", "peer", sender, "err", err, "requestID", c.id, "msgPayload", hex.EncodeToString(msg.Payload))
return fmt.Errorf("failed to verify capability response OCR attestation: %w", err)
}

var payload []byte
payload, err = c.encodePayloadWithMetadata(msg, commoncap.ResponseMetadata{Metering: []commoncap.MeteringNodeDetail{rpt}})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could benefit from a library on the client side that crafts resp.Metadata.Metering[0] in the way we expect here

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've consolidated logic for interacting with metering details in the client_request.
If you meant a method to craft resp.Metadata.Metering[0] on the capabilities side, I agree that it's beneficial. However, it's out of scope for this PR.

if err != nil {
return fmt.Errorf("failed to encode payload with metadata: %w", err)
}

c.sendResponse(clientResponse{Result: payload})
return nil
}

// metering reports per node are aggregated into a single array of values. for any single node message, the
// metering values are extracted from the CapabilityResponse, added to an array, and the CapabilityResponse
// is marshalled without the metering value to get the hash. each node could have a different metering value
Expand All @@ -317,13 +355,11 @@ func (c *ClientRequest) OnMessage(_ context.Context, msg *types.MessageBody) err
nodeReports = make([]commoncap.MeteringNodeDetail, 0)
}

if len(metadata.Metering) == 1 {
rpt := metadata.Metering[0]
rpt.Peer2PeerID = sender.String()

nodeReports = append(nodeReports, rpt)
rpt, err := extractMeteringFromMetadata(sender, metadata)
if err != nil {
lggr.Warnw("invalid metering detail", "err", err)
} else {
lggr.Warnw("node metering detail did not contain exactly 1 record", "records", len(metadata.Metering))
nodeReports = append(nodeReports, rpt)
}

c.responseIDCount[responseID]++
Expand Down Expand Up @@ -359,6 +395,53 @@ func (c *ClientRequest) OnMessage(_ context.Context, msg *types.MessageBody) err
return nil
}

func extractMeteringFromMetadata(sender p2ptypes.PeerID, metadata commoncap.ResponseMetadata) (commoncap.MeteringNodeDetail, error) {
if len(metadata.Metering) != 1 {
return commoncap.MeteringNodeDetail{}, fmt.Errorf("unexpected number of metering records received from pperi %s: got %d, want 1", sender, len(metadata.Metering))
}

rpt := metadata.Metering[0]
rpt.Peer2PeerID = sender.String()
return rpt, nil
}

func (c *ClientRequest) verifyAttestation(resp commoncap.CapabilityResponse, metering commoncap.MeteringNodeDetail) error {
if c.ocr3Configs == nil {
return errors.New("OCR3 configs not provided, cannot verify signatures")
}

cfg, ok := c.ocr3Configs[pb.OCR3ConfigDefaultKey]
if !ok {
return fmt.Errorf("OCR3 config with key %s not found", pb.OCR3ConfigDefaultKey)
}

attestation := resp.Metadata.OCRAttestation
if len(attestation.Sigs) < int(cfg.F)+1 {
return fmt.Errorf("not enough signatures: got %d, need at least %d", len(attestation.Sigs), cfg.F+1)
}

reportData := commoncap.ResponseToReportData(c.workflowExecutionID, c.referenceID, resp.Payload.Value, metering.SpendUnit, metering.SpendValue)
sigData := ocr2key.ReportToSigData3(attestation.ConfigDigest, attestation.SequenceNumber, reportData)
signed := make([]bool, len(cfg.Signers))
for _, sig := range attestation.Sigs {
if int(sig.Signer) > len(cfg.Signers) {
return fmt.Errorf("invalid signer index: %d", sig.Signer)
}

if signed[sig.Signer] {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do all consumers of ocr signed reports have to do this validation? maybe there's a shared lib in chainlink-common or libocr we can use?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have two consumers of OCR reports:

  1. On-chain contracts (code reuse is not possible)
  2. Liboccr. Code reuse is also not possible as signatures are streamed from other nodes and in this case, we have all of them in one place.

Extracting this for loop into a dedicated function in common does not seem like a good idea at this stage, since it's not shared with any other caller right now.
And even if someone in the future needs to use something similar, I do not see any issues with duplicating the code or extracting it at that stage, as the logic is trivial and will always converge to the same code, except for some minor differences.

return fmt.Errorf("duplicate signature from signer index: %d", sig.Signer)
}

if !ocr2key.EvmVerifyBlob(cfg.Signers[sig.Signer], sigData, sig.Signature) {
return fmt.Errorf("invalid signature from signer index: %d", sig.Signer)
}

signed[sig.Signer] = true
}

return nil
}

func (c *ClientRequest) sendResponse(response clientResponse) {
c.responseCh <- response
close(c.responseCh)
Expand Down
Loading
Loading