Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update reporting plugin factory constructors #20

Merged
merged 24 commits into from
Jul 17, 2024
Merged

Update reporting plugin factory constructors #20

Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
31dbb8b
commit: update factory constructor
makramkd Jul 4, 2024
709c6e6
Merge branch 'ccip-develop' into mk/plugin-factory-constructors
makramkd Jul 9, 2024
ce92963
Merge branch 'ccip-develop' into mk/plugin-factory-constructors
makramkd Jul 9, 2024
059bfe2
add contract writers, plugin limits
makramkd Jul 10, 2024
639e06f
add exec plugin factory constructor
makramkd Jul 10, 2024
99b5944
add consts for chain writer
makramkd Jul 12, 2024
b6dfb9d
more method names
makramkd Jul 12, 2024
a1a9150
disambiguate common methods further
makramkd Jul 12, 2024
923e0ca
add home chain reader consts
makramkd Jul 15, 2024
e7841aa
Merge branch 'ccip-develop' into mk/plugin-factory-constructors
makramkd Jul 15, 2024
8ce5cf3
use consts in home chain reader
makramkd Jul 16, 2024
c4ab63b
remove panic
makramkd Jul 16, 2024
c4748dc
hacky hacks
makramkd Jul 16, 2024
fb15b58
don't fetch source chain cfg for dest
makramkd Jul 16, 2024
c210e1f
add todo
makramkd Jul 16, 2024
e39aca3
pass in filled config
makramkd Jul 16, 2024
b40ce4d
info level logs
makramkd Jul 17, 2024
fcace5e
cast to ptr
makramkd Jul 17, 2024
32e475b
another log
makramkd Jul 17, 2024
5c32485
Merge branch 'ccip-develop' into mk/plugin-factory-constructors
makramkd Jul 17, 2024
2183765
fix lint
makramkd Jul 17, 2024
1bc85fe
address comments
makramkd Jul 17, 2024
688d42c
Merge branch 'ccip-develop' into mk/plugin-factory-constructors
makramkd Jul 17, 2024
3929cb0
use const from merklemulti
makramkd Jul 17, 2024
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
102 changes: 79 additions & 23 deletions commit/factory.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,29 @@ package commit

import (
"context"
"errors"
"math/big"

"github.com/smartcontractkit/chainlink-ccip/internal/reader"
"github.com/smartcontractkit/chainlink-ccip/pluginconfig"

"google.golang.org/grpc"

"github.com/smartcontractkit/chainlink-common/pkg/logger"
"github.com/smartcontractkit/chainlink-common/pkg/types"
cciptypes "github.com/smartcontractkit/chainlink-common/pkg/types/ccipocr3"
"github.com/smartcontractkit/chainlink-common/pkg/types/core"

"github.com/smartcontractkit/libocr/commontypes"
"github.com/smartcontractkit/libocr/offchainreporting2plus/ocr3types"
ocr2types "github.com/smartcontractkit/libocr/offchainreporting2plus/types"
libocrtypes "github.com/smartcontractkit/libocr/ragep2p/types"
ragep2ptypes "github.com/smartcontractkit/libocr/ragep2p/types"

"github.com/smartcontractkit/chainlink-ccip/internal/reader"
"github.com/smartcontractkit/chainlink-ccip/pluginconfig"
)

const (
// defaultMsgScanBatchSize is the default batch size for sequence number range queries.
// Since 256 messages is the limit for a merkle root, this is set to 256.
// NOTE: maybe we can also set this in the OCR config, offchainConfig.
winder marked this conversation as resolved.
Show resolved Hide resolved
defaultMsgScanBatchSize = 256
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like there's already a constant for this

Suggested change
defaultMsgScanBatchSize = 256
defaultMsgScanBatchSize = merklemulti.MaxNumberTreeLeaves

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed, just used that const directly.

)

// PluginFactoryConstructor implements common OCR3ReportingPluginClient and is used for initializing a plugin factory
Expand All @@ -35,42 +45,88 @@ func (p PluginFactoryConstructor) NewReportingPluginFactory(
keyValueStore core.KeyValueStore,
relayerSet core.RelayerSet,
) (core.OCR3ReportingPluginFactory, error) {
return NewPluginFactory(), nil
return nil, errors.New("unimplemented")
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'll revisit this when the loopification work starts.

}

func (p PluginFactoryConstructor) NewValidationService(ctx context.Context) (core.ValidationService, error) {
panic("implement me")
}

// PluginFactory implements common ReportingPluginFactory and is used for (re-)initializing commit plugin instances.
type PluginFactory struct{}
type PluginFactory struct {
lggr logger.Logger
ocrConfig reader.OCR3ConfigWithMeta
commitCodec cciptypes.CommitPluginCodec
msgHasher cciptypes.MessageHasher
homeChainReader reader.HomeChain
contractReaders map[cciptypes.ChainSelector]types.ContractReader
chainWriters map[cciptypes.ChainSelector]types.ChainWriter
}

func NewPluginFactory() *PluginFactory {
return &PluginFactory{}
func NewPluginFactory(
lggr logger.Logger,
ocrConfig reader.OCR3ConfigWithMeta,
commitCodec cciptypes.CommitPluginCodec,
msgHasher cciptypes.MessageHasher,
homeChainReader reader.HomeChain,
contractReaders map[cciptypes.ChainSelector]types.ContractReader,
chainWriters map[cciptypes.ChainSelector]types.ChainWriter,
) *PluginFactory {
return &PluginFactory{
lggr: lggr,
ocrConfig: ocrConfig,
commitCodec: commitCodec,
msgHasher: msgHasher,
homeChainReader: homeChainReader,
contractReaders: contractReaders,
chainWriters: chainWriters,
}
}

func (p PluginFactory) NewReportingPlugin(config ocr3types.ReportingPluginConfig,
func (p *PluginFactory) NewReportingPlugin(config ocr3types.ReportingPluginConfig,
) (ocr3types.ReportingPlugin[[]byte], ocr3types.ReportingPluginInfo, error) {
// TODO: Get this from ocr config, it's the mapping of the oracleId index in the DON
var oracleIDToP2pID map[commontypes.OracleID]libocrtypes.PeerID
var oracleIDToP2PID = make(map[commontypes.OracleID]ragep2ptypes.PeerID)
for oracleID, p2pID := range p.ocrConfig.Config.P2PIds {
oracleIDToP2PID[commontypes.OracleID(oracleID)] = p2pID
}

onChainTokenPricesReader := reader.NewOnchainTokenPricesReader(
reader.TokenPriceConfig{ // TODO: Inject config
StaticPrices: map[ocr2types.Account]big.Int{},
},
nil, // TODO: Inject this
)
ccipReader := reader.NewCCIPChainReader(
p.lggr,
p.contractReaders,
p.chainWriters,
p.ocrConfig.Config.ChainSelector,
)
return NewPlugin(
context.Background(),
config.OracleID,
oracleIDToP2pID,
pluginconfig.CommitPluginConfig{},
nil, // ccipReader
onChainTokenPricesReader,
nil, // reportCodec
nil, // msgHasher
nil, // lggr
nil, // homeChain
), ocr3types.ReportingPluginInfo{}, nil
context.Background(),
config.OracleID,
oracleIDToP2PID,
pluginconfig.CommitPluginConfig{
DestChain: p.ocrConfig.Config.ChainSelector,
NewMsgScanBatchSize: defaultMsgScanBatchSize,
},
ccipReader,
onChainTokenPricesReader,
p.commitCodec,
p.msgHasher,
p.lggr,
p.homeChainReader,
), ocr3types.ReportingPluginInfo{
Name: "CCIPRoleCommit",
Limits: ocr3types.ReportingPluginLimits{
// No query for this commit implementation.
MaxQueryLength: 0,
MaxObservationLength: 20_000, // 20kB
MaxOutcomeLength: 10_000, // 10kB
MaxReportLength: 10_000, // 10kB
Comment on lines +118 to +120
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These are ballpark, we'll revisit as we test.

Copy link
Contributor

Choose a reason for hiding this comment

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

Will this be read from config in the future?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No these will likely be constants, but might not be these values.

MaxReportCount: 10,
},
}, nil
}

func (p PluginFactory) Name() string {
Expand Down
18 changes: 9 additions & 9 deletions commit/plugin.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ func (p *Plugin) Observation(
// If there's no previous outcome (first round ever), we only observe the latest committed sequence numbers.
// and on the next round we use those to look for messages.
if outctx.PreviousOutcome == nil {
p.lggr.Debugw("first round ever, can't observe new messages yet")
p.lggr.Infow("first round ever, can't observe new messages yet")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Making these info because turning on debug logs in the CL node brings an avalanche of logs that are not relevant.

Copy link
Contributor

Choose a reason for hiding this comment

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

but then it will always show even without turning on debug logs, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Log levels filter downards, so Debug means Debug and all levels above, Info means Info and all levels above, etc.

return plugintypes.NewCommitPluginObservation(
msgBaseDetails, gasPrices, tokenPrices, latestCommittedSeqNumsObservation, fChain,
).Encode()
Expand All @@ -167,7 +167,7 @@ func (p *Plugin) Observation(
if err != nil {
return types.Observation{}, fmt.Errorf("decode commit plugin previous outcome: %w", err)
}
p.lggr.Debugw("previous outcome decoded", "outcome", prevOutcome.String())
p.lggr.Infow("previous outcome decoded", "outcome", prevOutcome.String())
Copy link
Contributor

Choose a reason for hiding this comment

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

isn't String() used by default? (not sure)

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'm not 100% sure but doesn't hurt. I think depending on the logger it sometimes tries to json encode it.


// Always observe based on previous outcome. We'll filter out stale messages in the outcome phase.
newMsgs, err := observeNewMsgs(
Expand Down Expand Up @@ -262,25 +262,25 @@ func (p *Plugin) Outcome(
}

maxSeqNums := maxSeqNumsConsensus(p.lggr, fChainDest, decodedObservations)
p.lggr.Debugw("max sequence numbers consensus", "maxSeqNumsConsensus", maxSeqNums)
p.lggr.Infow("max sequence numbers consensus", "maxSeqNumsConsensus", maxSeqNums)

merkleRoots, err := newMsgsConsensus(p.lggr, maxSeqNums, decodedObservations, fChains)
if err != nil {
return ocr3types.Outcome{}, fmt.Errorf("new messages consensus: %w", err)
}
p.lggr.Debugw("new messages consensus", "merkleRoots", merkleRoots)
p.lggr.Infow("new messages consensus", "merkleRoots", merkleRoots)

tokenPrices := tokenPricesConsensus(decodedObservations, fChainDest)

gasPrices := gasPricesConsensus(p.lggr, decodedObservations, fChainDest)
p.lggr.Debugw("gas prices consensus", "gasPrices", gasPrices)
p.lggr.Infow("gas prices consensus", "gasPrices", gasPrices)

outcome := plugintypes.NewCommitPluginOutcome(maxSeqNums, merkleRoots, tokenPrices, gasPrices)
if outcome.IsEmpty() {
p.lggr.Debugw("empty outcome")
p.lggr.Infow("empty outcome")
return ocr3types.Outcome{}, nil
}
p.lggr.Debugw("sending outcome", "outcome", outcome)
p.lggr.Infow("sending outcome", "outcome", outcome)

return outcome.Encode()
}
Expand Down Expand Up @@ -332,7 +332,7 @@ func (p *Plugin) ShouldTransmitAcceptedReport(
return false, fmt.Errorf("can't know if it's a writer: %w", err)
}
if !isWriter {
p.lggr.Debugw("not a writer, skipping report transmission")
p.lggr.Infow("not a writer, skipping report transmission")
return false, nil
}

Expand All @@ -349,7 +349,7 @@ func (p *Plugin) ShouldTransmitAcceptedReport(
return false, fmt.Errorf("validate merkle roots state: %w", err)
}

p.lggr.Debugw("transmitting report",
p.lggr.Infow("transmitting report",
Copy link
Contributor

Choose a reason for hiding this comment

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

Making this one Info level seems fine.... not sure about the others. Making everything Info means there would be an avalanche of Info logs as well

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'll have to revisit these later, for now I think this is slightly better than turning on Debug and getting all the debug logs of the app.

"roots", len(decodedReport.MerkleRoots),
"tokenPriceUpdates", len(decodedReport.PriceUpdates.TokenPriceUpdates),
"gasPriceUpdates", len(decodedReport.PriceUpdates.GasPriceUpdates),
Expand Down
8 changes: 7 additions & 1 deletion commit/plugin_e2e_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
helpers "github.com/smartcontractkit/chainlink-ccip/internal/libs/testhelpers"
"github.com/smartcontractkit/chainlink-ccip/internal/mocks"
"github.com/smartcontractkit/chainlink-ccip/internal/reader"
"github.com/smartcontractkit/chainlink-ccip/pkg/consts"
"github.com/smartcontractkit/chainlink-ccip/pluginconfig"
"github.com/smartcontractkit/chainlink-ccip/plugintypes"
)
Expand Down Expand Up @@ -402,7 +403,12 @@ func newNode(
func setupHomeChainPoller(lggr logger.Logger, chainConfigInfos []reader.ChainConfigInfo) reader.HomeChain {
homeChainReader := mocks.NewContractReaderMock()
homeChainReader.On(
"GetLatestValue", mock.Anything, "CCIPConfig", "getAllChainConfigs", mock.Anything, mock.Anything,
"GetLatestValue",
mock.Anything,
consts.ContractNameCCIPConfig,
consts.MethodNameGetAllChainConfigs,
mock.Anything,
mock.Anything,
).Run(
func(args mock.Arguments) {
arg := args.Get(4).(*[]reader.ChainConfigInfo)
Expand Down
6 changes: 4 additions & 2 deletions commit/plugin_functions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,12 @@ func observeLatestCommittedSeqNums(
sort.Slice(knownSourceChains, func(i, j int) bool { return knownSourceChains[i] < knownSourceChains[j] })
latestCommittedSeqNumsObservation := make([]plugintypes.SeqNumChain, 0)
if readableChains.Contains(destChain) {
lggr.Debugw("reading sequence numbers", "chains", knownSourceChains, "destChain", destChain)
lggr.Infow("reading sequence numbers", "chains", knownSourceChains, "destChain", destChain)
expectedNextSeqNums, err := ccipReader.NextSeqNum(ctx, knownSourceChains)
if err != nil {
return latestCommittedSeqNumsObservation, fmt.Errorf("get next seq nums: %w", err)
}
lggr.Debugw("observing latest committed sequence numbers",
lggr.Infow("observing latest committed sequence numbers",
"onChainNextSeqNums", expectedNextSeqNums, "destChain", destChain)
for i, ch := range knownSourceChains {
committedSeqNum := expectedNextSeqNums[i] - 1
Expand All @@ -50,6 +50,8 @@ func observeLatestCommittedSeqNums(
plugintypes.NewSeqNumChain(ch, committedSeqNum),
)
}
} else {
lggr.Infow("dest chain is not supported", "destChain", destChain)
}
return latestCommittedSeqNumsObservation, nil
}
Expand Down
85 changes: 71 additions & 14 deletions execute/factory.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,21 @@ package execute

import (
"context"
"errors"
"time"

"google.golang.org/grpc"

"github.com/smartcontractkit/chainlink-common/pkg/logger"
"github.com/smartcontractkit/chainlink-common/pkg/types"
cciptypes "github.com/smartcontractkit/chainlink-common/pkg/types/ccipocr3"
"github.com/smartcontractkit/libocr/offchainreporting2plus/ocr3types"

"github.com/smartcontractkit/chainlink-common/pkg/types/core"
"github.com/smartcontractkit/libocr/commontypes"
ragep2ptypes "github.com/smartcontractkit/libocr/ragep2p/types"

"github.com/smartcontractkit/chainlink-ccip/internal/reader"
"github.com/smartcontractkit/chainlink-ccip/pluginconfig"
)

Expand All @@ -30,34 +38,83 @@ func (p PluginFactoryConstructor) NewReportingPluginFactory(
keyValueStore core.KeyValueStore,
relayerSet core.RelayerSet,
) (core.OCR3ReportingPluginFactory, error) {
return NewPluginFactory(), nil
return nil, errors.New("unimplemented")
}

func (p PluginFactoryConstructor) NewValidationService(ctx context.Context) (core.ValidationService, error) {
panic("implement me")
}

// PluginFactory implements common ReportingPluginFactory and is used for (re-)initializing commit plugin instances.
type PluginFactory struct{}
type PluginFactory struct {
lggr logger.Logger
ocrConfig reader.OCR3ConfigWithMeta
execCodec cciptypes.ExecutePluginCodec
msgHasher cciptypes.MessageHasher
homeChainReader reader.HomeChain
contractReaders map[cciptypes.ChainSelector]types.ContractReader
chainWriters map[cciptypes.ChainSelector]types.ChainWriter
}

func NewPluginFactory() *PluginFactory {
return &PluginFactory{}
func NewPluginFactory(
lggr logger.Logger,
ocrConfig reader.OCR3ConfigWithMeta,
execCodec cciptypes.ExecutePluginCodec,
msgHasher cciptypes.MessageHasher,
homeChainReader reader.HomeChain,
contractReaders map[cciptypes.ChainSelector]types.ContractReader,
chainWriters map[cciptypes.ChainSelector]types.ChainWriter,
) *PluginFactory {
return &PluginFactory{
lggr: lggr,
ocrConfig: ocrConfig,
execCodec: execCodec,
msgHasher: msgHasher,
homeChainReader: homeChainReader,
contractReaders: contractReaders,
chainWriters: chainWriters,
}
}

func (p PluginFactory) NewReportingPlugin(
config ocr3types.ReportingPluginConfig,
) (ocr3types.ReportingPlugin[[]byte], ocr3types.ReportingPluginInfo, error) {
var oracleIDToP2PID = make(map[commontypes.OracleID]ragep2ptypes.PeerID)
for oracleID, p2pID := range p.ocrConfig.Config.P2PIds {
oracleIDToP2PID[commontypes.OracleID(oracleID)] = p2pID
}

ccipReader := reader.NewCCIPChainReader(
p.lggr,
p.contractReaders,
p.chainWriters,
p.ocrConfig.Config.ChainSelector,
)

return NewPlugin(
config,
pluginconfig.ExecutePluginConfig{},
nil,
nil,
nil,
nil,
nil,
nil,
nil,
), ocr3types.ReportingPluginInfo{}, nil
config,
pluginconfig.ExecutePluginConfig{
DestChain: p.ocrConfig.Config.ChainSelector,
MessageVisibilityInterval: 8 * time.Hour,
},
oracleIDToP2PID,
ccipReader,
p.execCodec,
p.msgHasher,
p.homeChainReader,
nil, // TODO: token data reader
p.lggr,
), ocr3types.ReportingPluginInfo{
Name: "CCIPRoleExecute",
Limits: ocr3types.ReportingPluginLimits{
// No query for this execute implementation.
MaxQueryLength: 0,
MaxObservationLength: 20_000, // 20kB
MaxOutcomeLength: 20_000, // 20kB
MaxReportLength: maxReportSizeBytes, // 250kB
MaxReportCount: 10,
},
}, nil
}

func (p PluginFactory) Name() string {
Expand Down
8 changes: 7 additions & 1 deletion execute/plugin_e2e_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/smartcontractkit/chainlink-ccip/internal/mocks"
"github.com/smartcontractkit/chainlink-ccip/internal/mocks/inmem"
"github.com/smartcontractkit/chainlink-ccip/internal/reader"
"github.com/smartcontractkit/chainlink-ccip/pkg/consts"
"github.com/smartcontractkit/chainlink-ccip/pluginconfig"
"github.com/smartcontractkit/chainlink-ccip/plugintypes"
)
Expand Down Expand Up @@ -78,7 +79,12 @@ type nodeSetup struct {
func setupHomeChainPoller(lggr logger.Logger, chainConfigInfos []reader.ChainConfigInfo) reader.HomeChain {
homeChainReader := mocks.NewContractReaderMock()
homeChainReader.On(
"GetLatestValue", mock.Anything, "CCIPConfig", "getAllChainConfigs", mock.Anything, mock.Anything,
"GetLatestValue",
mock.Anything,
consts.ContractNameCCIPConfig,
consts.MethodNameGetAllChainConfigs,
mock.Anything,
mock.Anything,
).Run(
func(args mock.Arguments) {
arg := args.Get(4).(*[]reader.ChainConfigInfo)
Expand Down
Loading
Loading