-
Notifications
You must be signed in to change notification settings - Fork 0
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.
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
Update reporting plugin factory constructors #20
Changes from all commits
31dbb8b
709c6e6
ce92963
059bfe2
639e06f
99b5944
b6dfb9d
a1a9150
923e0ca
e7841aa
8ce5cf3
c4ab63b
c4748dc
fb15b58
c210e1f
e39aca3
b40ce4d
fcace5e
32e475b
5c32485
2183765
1bc85fe
688d42c
3929cb0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,19 +2,23 @@ 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/merklemulti" | ||
"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" | ||
) | ||
|
||
// PluginFactoryConstructor implements common OCR3ReportingPluginClient and is used for initializing a plugin factory | ||
|
@@ -35,42 +39,88 @@ 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 | ||
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: merklemulti.MaxNumberTreeLeaves, | ||
}, | ||
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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These are ballpark, we'll revisit as we test. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will this be read from config in the future? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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() | ||
|
@@ -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()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. isn't There was a problem hiding this comment. Choose a reason for hiding this commentThe 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( | ||
|
@@ -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() | ||
} | ||
|
@@ -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 | ||
} | ||
|
||
|
@@ -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", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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), | ||
|
There was a problem hiding this comment.
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.