Make send signatures in Transmit configurable (#13761)

* Make send signatures in Transmit configurable * Add changelog * fix lint * Fix tests * Update .changeset/hungry-pandas-suffer.md Co-authored-by: Jordan Krage <[email protected]> --------- Co-authored-by: Jordan Krage <[email protected]>
smartcontractkit · Jul 9, 2024 · 89196f1 · 89196f1
1 parent e065b82
commit 89196f1
Show file tree

Hide file tree

Showing 6 changed files with 180 additions and 57 deletions.
diff --git a/.changeset/hungry-pandas-suffer.md b/.changeset/hungry-pandas-suffer.md
@@ -0,0 +1,5 @@
+---
+"chainlink": patch
+---
+
+Make send signatures configurable when Transmit in Contract Transmitter #internal
diff --git a/core/internal/features/ocr2/features_ocr2_test.go b/core/internal/features/ocr2/features_ocr2_test.go
@@ -603,7 +603,7 @@ updateInterval = "1m"
 				contractABI, err2 := abi.JSON(strings.NewReader(ocr2aggregator.OCR2AggregatorABI))
 				require.NoError(t, err2)
 				apps[0].GetRelayers().LegacyEVMChains().Slice()
-				ct, err2 := evm.NewOCRContractTransmitter(testutils.Context(t), ocrContractAddress, apps[0].GetRelayers().LegacyEVMChains().Slice()[0].Client(), contractABI, nil, apps[0].GetRelayers().LegacyEVMChains().Slice()[0].LogPoller(), lggr, nil)
+				ct, err2 := evm.NewOCRContractTransmitter(testutils.Context(t), ocrContractAddress, apps[0].GetRelayers().LegacyEVMChains().Slice()[0].Client(), contractABI, nil, apps[0].GetRelayers().LegacyEVMChains().Slice()[0].LogPoller(), lggr)
 				require.NoError(t, err2)
 				configDigest, epoch, err2 := ct.LatestConfigDigestAndEpoch(testutils.Context(t))
 				require.NoError(t, err2)
@@ -916,7 +916,7 @@ updateInterval = "1m"
 	// Assert we can read the latest config digest and epoch after a report has been submitted.
 	contractABI, err := abi.JSON(strings.NewReader(ocr2aggregator.OCR2AggregatorABI))
 	require.NoError(t, err)
-	ct, err := evm.NewOCRContractTransmitter(testutils.Context(t), ocrContractAddress, apps[0].GetRelayers().LegacyEVMChains().Slice()[0].Client(), contractABI, nil, apps[0].GetRelayers().LegacyEVMChains().Slice()[0].LogPoller(), lggr, nil)
+	ct, err := evm.NewOCRContractTransmitter(testutils.Context(t), ocrContractAddress, apps[0].GetRelayers().LegacyEVMChains().Slice()[0].Client(), contractABI, nil, apps[0].GetRelayers().LegacyEVMChains().Slice()[0].LogPoller(), lggr)
 	require.NoError(t, err)
 	configDigest, epoch, err := ct.LatestConfigDigestAndEpoch(testutils.Context(t))
 	require.NoError(t, err)

diff --git a/core/services/relay/evm/contract_transmitter.go b/core/services/relay/evm/contract_transmitter.go
@@ -40,6 +40,28 @@ func reportToEvmTxMetaNoop([]byte) (*txmgr.TxMeta, error) {
 	return nil, nil
 }
 
+type OCRTransmitterOption func(transmitter *contractTransmitter)
+
+func WithExcludeSignatures() OCRTransmitterOption {
+	return func(ct *contractTransmitter) {
+		ct.excludeSigs = true
+	}
+}
+
+func WithRetention(retention time.Duration) OCRTransmitterOption {
+	return func(ct *contractTransmitter) {
+		ct.retention = retention
+	}
+}
+
+func WithReportToEthMetadata(reportToEvmTxMeta ReportToEthMetadata) OCRTransmitterOption {
+	return func(ct *contractTransmitter) {
+		if reportToEvmTxMeta != nil {
+			ct.reportToEvmTxMeta = reportToEvmTxMeta
+		}
+	}
+}
+
 type contractTransmitter struct {
 	contractAddress     gethcommon.Address
 	contractABI         abi.ABI
@@ -48,7 +70,10 @@ type contractTransmitter struct {
 	contractReader      contractReader
 	lp                  logpoller.LogPoller
 	lggr                logger.Logger
-	reportToEvmTxMeta   ReportToEthMetadata
+	// Options
+	reportToEvmTxMeta ReportToEthMetadata
+	excludeSigs       bool
+	retention         time.Duration
 }
 
 func transmitterFilterName(addr common.Address) string {
@@ -63,46 +88,37 @@ func NewOCRContractTransmitter(
 	transmitter Transmitter,
 	lp logpoller.LogPoller,
 	lggr logger.Logger,
-	reportToEvmTxMeta ReportToEthMetadata,
-) (*contractTransmitter, error) {
-	return NewOCRContractTransmitterWithRetention(ctx, address, caller, contractABI, transmitter, lp, lggr, reportToEvmTxMeta, 0)
-}
-
-func NewOCRContractTransmitterWithRetention(
-	ctx context.Context,
-	address gethcommon.Address,
-	caller contractReader,
-	contractABI abi.ABI,
-	transmitter Transmitter,
-	lp logpoller.LogPoller,
-	lggr logger.Logger,
-	reportToEvmTxMeta ReportToEthMetadata,
-	retention time.Duration,
+	opts ...OCRTransmitterOption,
 ) (*contractTransmitter, error) {
 	transmitted, ok := contractABI.Events["Transmitted"]
 	if !ok {
 		return nil, errors.New("invalid ABI, missing transmitted")
 	}
 
-	// TODO It would be better to keep MaxLogsKept = 1 for the OCR contract transmitter instead of Retention. We are always interested only in the latest log.
-	// Although MaxLogsKept is present in the Filter struct, it is not supported by LogPoller yet.
-	err := lp.RegisterFilter(ctx, logpoller.Filter{Name: transmitterFilterName(address), EventSigs: []common.Hash{transmitted.ID}, Addresses: []common.Address{address}, Retention: retention})
-	if err != nil {
-		return nil, err
-	}
-	if reportToEvmTxMeta == nil {
-		reportToEvmTxMeta = reportToEvmTxMetaNoop
-	}
-	return &contractTransmitter{
+	newContractTransmitter := &contractTransmitter{
 		contractAddress:     address,
 		contractABI:         contractABI,
 		transmitter:         transmitter,
 		transmittedEventSig: transmitted.ID,
 		lp:                  lp,
 		contractReader:      caller,
 		lggr:                lggr.Named("OCRContractTransmitter"),
-		reportToEvmTxMeta:   reportToEvmTxMeta,
-	}, nil
+		reportToEvmTxMeta:   reportToEvmTxMetaNoop,
+		excludeSigs:         false,
+		retention:           0,
+	}
+
+	for _, opt := range opts {
+		opt(newContractTransmitter)
+	}
+
+	// TODO It would be better to keep MaxLogsKept = 1 for the OCR contract transmitter instead of Retention. We are always interested only in the latest log.
+	// Although MaxLogsKept is present in the Filter struct, it is not supported by LogPoller yet.
+	err := lp.RegisterFilter(ctx, logpoller.Filter{Name: transmitterFilterName(address), EventSigs: []common.Hash{transmitted.ID}, Addresses: []common.Address{address}, Retention: newContractTransmitter.retention})
+	if err != nil {
+		return nil, err
+	}
+	return newContractTransmitter, nil
 }
 
 // Transmit sends the report to the on-chain smart contract's Transmit method.
@@ -118,9 +134,11 @@ func (oc *contractTransmitter) Transmit(ctx context.Context, reportCtx ocrtypes.
 		if err != nil {
 			panic("eventTransmit(ev): error in SplitSignature")
 		}
-		rs = append(rs, r)
-		ss = append(ss, s)
-		vs[i] = v
+		if !oc.excludeSigs {
+			rs = append(rs, r)
+			ss = append(ss, s)
+			vs[i] = v
+		}
 	}
 	rawReportCtx := evmutil.RawReportContext(reportCtx)
 

diff --git a/core/services/relay/evm/contract_transmitter_test.go b/core/services/relay/evm/contract_transmitter_test.go
@@ -8,7 +8,6 @@ import (
 
 	"github.com/ethereum/go-ethereum/accounts/abi"
 	gethcommon "github.com/ethereum/go-ethereum/common"
-	"github.com/smartcontractkit/libocr/gethwrappers2/ocr2aggregator"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
@@ -19,16 +18,27 @@ import (
 	"github.com/smartcontractkit/chainlink/v2/core/chains/evm/txmgr"
 	"github.com/smartcontractkit/chainlink/v2/core/internal/testutils"
 	"github.com/smartcontractkit/chainlink/v2/core/logger"
+
+	"github.com/smartcontractkit/libocr/commontypes"
+	"github.com/smartcontractkit/libocr/gethwrappers2/ocr2aggregator"
+	"github.com/smartcontractkit/libocr/offchainreporting2plus/chains/evmutil"
+	"github.com/smartcontractkit/libocr/offchainreporting2plus/types"
+	libocr "github.com/smartcontractkit/libocr/offchainreporting2plus/types"
+	ocrtypes "github.com/smartcontractkit/libocr/offchainreporting2plus/types"
 )
 
 var sampleAddress = testutils.NewAddress()
 
-type mockTransmitter struct{}
+type mockTransmitter struct {
+	lastPayload []byte
+}
 
-func (mockTransmitter) CreateEthTransaction(ctx context.Context, toAddress gethcommon.Address, payload []byte, _ *txmgr.TxMeta) error {
+func (m *mockTransmitter) CreateEthTransaction(ctx context.Context, toAddress gethcommon.Address, payload []byte, _ *txmgr.TxMeta) error {
+	m.lastPayload = payload
 	return nil
 }
-func (mockTransmitter) FromAddress() gethcommon.Address { return sampleAddress }
+
+func (*mockTransmitter) FromAddress() gethcommon.Address { return sampleAddress }
 
 func TestContractTransmitter(t *testing.T) {
 	t.Parallel()
@@ -43,11 +53,13 @@ func TestContractTransmitter(t *testing.T) {
 			"000130da6b9315bd59af6b0a3f5463c0d0a39e92eaa34cbcbdbace7b3bfcc776" + // config digest
 			"0000000000000000000000000000000000000000000000000000000000000002") // epoch
 	c.On("CallContract", mock.Anything, mock.Anything, mock.Anything).Return(digestAndEpochDontScanLogs, nil).Once()
-	contractABI, _ := abi.JSON(strings.NewReader(ocr2aggregator.OCR2AggregatorABI))
+	contractABI, _ := abi.JSON(strings.NewReader(ocr2aggregator.OCR2AggregatorMetaData.ABI))
 	lp.On("RegisterFilter", mock.Anything, mock.Anything).Return(nil)
-	ot, err := NewOCRContractTransmitter(ctx, gethcommon.Address{}, c, contractABI, mockTransmitter{}, lp, lggr, func(b []byte) (*txmgr.TxMeta, error) {
+	reportToEvmTxMeta := func(b []byte) (*txmgr.TxMeta, error) {
 		return &txmgr.TxMeta{}, nil
-	})
+	}
+	ot, err := NewOCRContractTransmitter(ctx, gethcommon.Address{}, c, contractABI, &mockTransmitter{}, lp, lggr,
+		WithReportToEthMetadata(reportToEvmTxMeta))
 	require.NoError(t, err)
 	digest, epoch, err := ot.LatestConfigDigestAndEpoch(testutils.Context(t))
 	require.NoError(t, err)
@@ -75,3 +87,85 @@ func TestContractTransmitter(t *testing.T) {
 	require.NoError(t, err)
 	assert.Equal(t, sampleAddress.String(), string(from))
 }
+
+func Test_contractTransmitterNoSignatures_Transmit_SignaturesAreNotTransmitted(t *testing.T) {
+	t.Parallel()
+
+	transmitter := &mockTransmitter{}
+
+	ctx := context.Background()
+	reportCtx := types.ReportContext{}
+	report := types.Report{}
+	var signatures = oneSignature()
+
+	oc := createContractTransmitter(ctx, t, transmitter, WithExcludeSignatures())
+
+	err := oc.Transmit(ctx, reportCtx, report, signatures)
+	require.NoError(t, err)
+
+	var emptyRs [][32]byte
+	var emptySs [][32]byte
+	var emptyVs [32]byte
+	emptySignaturesPayload, err := oc.contractABI.Pack("transmit", evmutil.RawReportContext(reportCtx), []byte(report), emptyRs, emptySs, emptyVs)
+	require.NoError(t, err)
+	require.Equal(t, transmitter.lastPayload, emptySignaturesPayload)
+}
+
+func Test_contractTransmitter_Transmit_SignaturesAreTransmitted(t *testing.T) {
+	t.Parallel()
+
+	transmitter := &mockTransmitter{}
+
+	ctx := context.Background()
+	reportCtx := types.ReportContext{}
+	report := types.Report{}
+	var signatures = oneSignature()
+
+	oc := createContractTransmitter(ctx, t, transmitter)
+
+	err := oc.Transmit(ctx, reportCtx, report, signatures)
+	require.NoError(t, err)
+
+	rs, ss, vs := signaturesAsPayload(t, signatures)
+	withSignaturesPayload, err := oc.contractABI.Pack("transmit", evmutil.RawReportContext(reportCtx), []byte(report), rs, ss, vs)
+	require.NoError(t, err)
+	require.Equal(t, transmitter.lastPayload, withSignaturesPayload)
+}
+
+func signaturesAsPayload(t *testing.T, signatures []ocrtypes.AttributedOnchainSignature) ([][32]byte, [][32]byte, [32]byte) {
+	var rs [][32]byte
+	var ss [][32]byte
+	var vs [32]byte
+	r, s, v, err := evmutil.SplitSignature(signatures[0].Signature)
+	require.NoError(t, err)
+	rs = append(rs, r)
+	ss = append(ss, s)
+	vs[0] = v
+	return rs, ss, vs
+}
+
+func oneSignature() []ocrtypes.AttributedOnchainSignature {
+	signaturesData := make([]byte, 65)
+	signaturesData[9] = 8
+	signaturesData[7] = 6
+	return []libocr.AttributedOnchainSignature{{Signature: signaturesData, Signer: commontypes.OracleID(54)}}
+}
+
+func createContractTransmitter(ctx context.Context, t *testing.T, transmitter Transmitter, ops ...OCRTransmitterOption) *contractTransmitter {
+	contractABI, err := abi.JSON(strings.NewReader(ocr2aggregator.OCR2AggregatorMetaData.ABI))
+	require.NoError(t, err)
+	lp := lpmocks.NewLogPoller(t)
+	lp.On("RegisterFilter", mock.Anything, mock.Anything).Return(nil)
+	contractTransmitter, err := NewOCRContractTransmitter(
+		ctx,
+		gethcommon.Address{},
+		evmclimocks.NewClient(t),
+		contractABI,
+		transmitter,
+		lp,
+		logger.TestLogger(t),
+		ops...,
+	)
+	require.NoError(t, err)
+	return contractTransmitter
+}
diff --git a/core/services/relay/evm/evm.go b/core/services/relay/evm/evm.go
@@ -7,7 +7,6 @@ import (
 	"fmt"
 	"strings"
 	"sync"
-	"time"
 
 	"github.com/ethereum/go-ethereum/accounts/abi"
 	"github.com/ethereum/go-ethereum/common"
@@ -216,7 +215,7 @@ func (r *Relayer) NewPluginProvider(rargs commontypes.RelayArgs, pargs commontyp
 		return nil, err
 	}
 
-	transmitter, err := newOnChainContractTransmitter(ctx, r.lggr, rargs, pargs.TransmitterID, r.ks.Eth(), configWatcher, configTransmitterOpts{}, OCR2AggregatorTransmissionContractABI, 0)
+	transmitter, err := newOnChainContractTransmitter(ctx, r.lggr, rargs, r.ks.Eth(), configWatcher, configTransmitterOpts{}, OCR2AggregatorTransmissionContractABI)
 	if err != nil {
 		return nil, err
 	}
@@ -532,7 +531,25 @@ type configTransmitterOpts struct {
 }
 
 // newOnChainContractTransmitter creates a new contract transmitter.
-func newOnChainContractTransmitter(ctx context.Context, lggr logger.Logger, rargs commontypes.RelayArgs, transmitterID string, ethKeystore keystore.Eth, configWatcher *configWatcher, opts configTransmitterOpts, transmissionContractABI abi.ABI, transmissionContractRetention time.Duration) (*contractTransmitter, error) {
+func newOnChainContractTransmitter(ctx context.Context, lggr logger.Logger, rargs commontypes.RelayArgs, ethKeystore keystore.Eth, configWatcher *configWatcher, opts configTransmitterOpts, transmissionContractABI abi.ABI, ocrTransmitterOpts ...OCRTransmitterOption) (*contractTransmitter, error) {
+	transmitter, err := generateTransmitterFrom(ctx, rargs, ethKeystore, configWatcher, opts)
+	if err != nil {
+		return nil, err
+	}
+
+	return NewOCRContractTransmitter(
+		ctx,
+		configWatcher.contractAddress,
+		configWatcher.chain.Client(),
+		transmissionContractABI,
+		transmitter,
+		configWatcher.chain.LogPoller(),
+		lggr,
+		ocrTransmitterOpts...,
+	)
+}
+
+func generateTransmitterFrom(ctx context.Context, rargs commontypes.RelayArgs, ethKeystore keystore.Eth, configWatcher *configWatcher, opts configTransmitterOpts) (Transmitter, error) {
 	var relayConfig types.RelayConfig
 	if err := json.Unmarshal(rargs.RelayConfig, &relayConfig); err != nil {
 		return nil, err
@@ -612,18 +629,7 @@ func newOnChainContractTransmitter(ctx context.Context, lggr logger.Logger, rarg
 	if err != nil {
 		return nil, pkgerrors.Wrap(err, "failed to create transmitter")
 	}
-
-	return NewOCRContractTransmitterWithRetention(
-		ctx,
-		configWatcher.contractAddress,
-		configWatcher.chain.Client(),
-		transmissionContractABI,
-		transmitter,
-		configWatcher.chain.LogPoller(),
-		lggr,
-		nil,
-		transmissionContractRetention,
-	)
+	return transmitter, nil
 }
 
 func (r *Relayer) NewChainWriter(_ context.Context, config []byte) (commontypes.ChainWriter, error) {
@@ -671,7 +677,7 @@ func (r *Relayer) NewMedianProvider(rargs commontypes.RelayArgs, pargs commontyp
 
 	reportCodec := evmreportcodec.ReportCodec{}
 
-	contractTransmitter, err := newOnChainContractTransmitter(ctx, lggr, rargs, pargs.TransmitterID, r.ks.Eth(), configWatcher, configTransmitterOpts{}, OCR2AggregatorTransmissionContractABI, 0)
+	contractTransmitter, err := newOnChainContractTransmitter(ctx, lggr, rargs, r.ks.Eth(), configWatcher, configTransmitterOpts{}, OCR2AggregatorTransmissionContractABI)
 	if err != nil {
 		return nil, err
 	}

diff --git a/core/services/relay/evm/ocr2keeper.go b/core/services/relay/evm/ocr2keeper.go
@@ -90,7 +90,7 @@ func (r *ocr2keeperRelayer) NewOCR2KeeperProvider(rargs commontypes.RelayArgs, p
 	}
 
 	gasLimit := cfgWatcher.chain.Config().EVM().OCR2().Automation().GasLimit()
-	contractTransmitter, err := newOnChainContractTransmitter(ctx, r.lggr, rargs, pargs.TransmitterID, r.ethKeystore, cfgWatcher, configTransmitterOpts{pluginGasLimit: &gasLimit}, OCR2AggregatorTransmissionContractABI, 0)
+	contractTransmitter, err := newOnChainContractTransmitter(ctx, r.lggr, rargs, r.ethKeystore, cfgWatcher, configTransmitterOpts{pluginGasLimit: &gasLimit}, OCR2AggregatorTransmissionContractABI)
 	if err != nil {
 		return nil, err
 	}