core/capabilities/ccip: use OCR offchain config #1264
Conversation
| GasPriceDeviationPPB: ccipocr3.NewBigIntFromInt64(1000), | ||
| DAGasPriceDeviationPPB: ccipocr3.NewBigIntFromInt64(0), | ||
| FinalityDepth: 10, | ||
| OptimisticConfirmations: 1, |
There was a problem hiding this comment.
These are not used explicitly yet, will create a ticket. Expected usage:
GasPriceDeviationPPB: used by the commit pluginDAGasPriceDeviationPPB: used by the commit pluginFinalityDepth: used by the CCIP reader (both plugins)OptimisticConfirmations: used by the exec plugin (via the CCIP reader)
| RemoteGasPriceBatchWriteFrequency: *commonconfig.MustNewDuration(RemoteGasPriceBatchWriteFrequency), | ||
| // TODO: implement token price writes | ||
| // TokenPriceBatchWriteFrequency: *commonconfig.MustNewDuration(tokenPriceBatchWriteFrequency), |
There was a problem hiding this comment.
Will create a ticket on using these in the commit plugin, post re-write. Expected usage:
RemoteGasPriceBatchWriteFrequency: commit plugin uses this to determine when to write a gas price on chain if the deviation threshold isn't metTokenPriceBatchWriteFrequency: commit plugin uses this to determine when to write a token price on chain if the deviation threshold isn't met
| } else { | ||
| encodedOffchainConfig, err2 = pluginconfig.EncodeExecuteOffchainConfig(pluginconfig.ExecuteOffchainConfig{ | ||
| BatchGasLimit: BatchGasLimit, | ||
| RelativeBoostPerWaitHour: RelativeBoostPerWaitHour, |
There was a problem hiding this comment.
Currently unused, but needed for fee boosting in exec.
| RootSnoozeTime: *commonconfig.MustNewDuration(RootSnoozeTime), | ||
| BatchingStrategyID: BatchingStrategyID, |
| @@ -46,6 +47,10 @@ import ( | |||
|
|
|||
| var _ cctypes.OracleCreator = &inprocessOracleCreator{} | |||
|
|
|||
| const ( | |||
| defaultCommitGasLimit = 500_000 | |||
There was a problem hiding this comment.
We don't define this in the offchain config for commit, we might have to at some point though to keep things chain agnostic in this launcher.
| if execOffchainConfig.BatchGasLimit == 0 && destChainFamily == relay.NetworkEVM { | ||
| return nil, fmt.Errorf("BatchGasLimit not set in execute offchain config, must be > 0") | ||
| } |
There was a problem hiding this comment.
Use the Validate function from chainlink-ccip? Does that function need to consider destChainFamily somehow?
There was a problem hiding this comment.
Yeah that's a good note, I have this zero check there but I think long term it doesn't make much sense for it to be there unless it has extra info like chain family.
There was a problem hiding this comment.
I figured I'd put this check here just in case to catch a misconfig, but maybe better to change the Validate API to accept a chain family...
There was a problem hiding this comment.
Thinking about this more not sure what is the best place to put the chain-specific validation logic. Originally chainlink-ccip was supposed to be totally chain agnostic, I feel like if we put these types there its kinda breaking that promise.
There was a problem hiding this comment.
Another thing is validation that certain fields are correct based on whether the chain is an L2 or not (e.g DA deviation ppb)
There was a problem hiding this comment.
That's something that MUST be set if the chain is an L2. We don't have that info anywhere though, far as I can tell. It's not exposed as part of the chain family.
There was a problem hiding this comment.
I think will keep as-is now, we need to do a pass where we chain-agnostify all of this code here anyways, hopefully we can find a pattern that works for all chain families and per-family quirks
|
* [CCIP Merge] Add ccip capabilities directory [CCIP-2943] (#14044) * Add ccip capabilities directory * [CCIP Merge] Capabilities fix [CCIP-2943] (#14048) * Fix compilation for launcher, diff Make application.go ready for adding more fixes * Fix launcher tests * ks-409 fix the mock trigger to ensure events are sent (#14047) * Add ccip to job orm * Add capabilities directory under BUSL license * Prep to instantiate separate registrysyncer for CCIP * Move registrySyncer creation into ccip delegate * [chore] Change registrysyncer config to bytes * Fix launcher diff tests after changing structs in syncer * Fix linting * MAke simulated backend client work with chains other than 1337 * core/capabilities/ccip: use OCR offchain config (#1264) We want to define and use the appropriate OCR offchain config for each plugin. Requires smartcontractkit/chainlink-ccip#36 * Cleaning up * Add capabilities types to mockery --------- Co-authored-by: Cedric Cordenier <[email protected]> Co-authored-by: Matthew Pendrey <[email protected]> Co-authored-by: Makram <[email protected]> * make modgraph * Add changeset * Fix test with new TxMgr constructor --------- Co-authored-by: Cedric Cordenier <[email protected]> Co-authored-by: Matthew Pendrey <[email protected]> Co-authored-by: Makram <[email protected]>
We want to define and use the appropriate OCR offchain config for each plugin. Requires smartcontractkit/chainlink-ccip#36
Motivation
We want to define and use the appropriate OCR offchain config for each plugin.
Solution
Requires smartcontractkit/chainlink-ccip#36