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.

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

Ccip-5315 ocr params defaults #16573

Merged
merged 25 commits into from
Feb 26, 2025
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
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
7 changes: 0 additions & 7 deletions .github/integration-in-memory-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,6 @@ runner-test-matrix:
- PR Integration CCIP Tests
test_cmd: cd integration-tests/smoke/ccip && go test -run "Test_CCIPMessageLimitations" -timeout 12m -test.parallel=2 -count=1 -json

- id: smoke/ccip/ccip_fee_boosting_test.go:*
Copy link
Contributor Author

Choose a reason for hiding this comment

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

no feeboosting anymore.

path: integration-tests/smoke/ccip/ccip_fee_boosting_test.go
test_env_type: in-memory
runs_on: ubuntu-latest
triggers:
- PR Integration CCIP Tests
test_cmd: cd integration-tests/smoke/ccip && go test ccip_fee_boosting_test.go -timeout 12m -test.parallel=2 -count=1 -json
- id: smoke/ccip/ccip_batching_test.go:*
path: integration-tests/smoke/ccip/ccip_batching_test.go
test_env_type: in-memory
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/integration-in-memory-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ jobs:
contents: read
needs: changes
if: github.event_name == 'pull_request' && ( needs.changes.outputs.ccip_changes == 'true' || needs.changes.outputs.core_changes == 'true' || needs.changes.outputs.github_ci_changes == 'true')
uses: smartcontractkit/.github/.github/workflows/run-integration-tests.yml@bb2d725fba3a42858bcb91e816987ca7cd063488
uses: smartcontractkit/.github/.github/workflows/run-integration-tests.yml@18ee2276811ff4ad56a2284590c9917bec33b748
Copy link
Contributor Author

Choose a reason for hiding this comment

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

it was using an obsolete commit which is why the workflow was not running at all for past few weeks

with:
workflow_name: Run CCIP Integration Tests For PR
test_path: .github/integration-in-memory-tests.yml
Expand All @@ -98,7 +98,7 @@ jobs:
contents: read
needs: changes
if: github.event_name == 'merge_group' && ( needs.changes.outputs.ccip_changes == 'true' || needs.changes.outputs.core_changes == 'true' || needs.changes.outputs.github_ci_changes == 'true')
uses: smartcontractkit/.github/.github/workflows/run-integration-tests.yml@d7f8e299e891eafa428a37b5e856d929232c6e18
uses: smartcontractkit/.github/.github/workflows/run-integration-tests.yml@18ee2276811ff4ad56a2284590c9917bec33b748
with:
workflow_name: Run CCIP Integration Tests For Merge Queue
test_path: .github/integration-in-memory-tests.yml
Expand Down
64 changes: 42 additions & 22 deletions deployment/ccip/changeset/globals/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ package globals

import (
"time"

"github.com/smartcontractkit/chainlink-ccip/pluginconfig"
"github.com/smartcontractkit/chainlink-common/pkg/config"
"github.com/smartcontractkit/chainlink-common/pkg/merklemulti"
)

type ConfigType string
Expand All @@ -10,28 +14,16 @@ const (
ConfigTypeActive ConfigType = "active"
ConfigTypeCandidate ConfigType = "candidate"
// ========= Changeset Defaults =========
PermissionLessExecutionThreshold = 8 * time.Hour
RemoteGasPriceBatchWriteFrequency = 30 * time.Minute
TokenPriceBatchWriteFrequency = 30 * time.Minute
BatchGasLimit = 6_500_000
InflightCacheExpiry = 10 * time.Minute
RootSnoozeTime = 30 * time.Minute
BatchingStrategyID = 0
DeltaProgress = 10 * time.Second
DeltaResend = 10 * time.Second
DeltaInitial = 20 * time.Second
DeltaRound = 2 * time.Second
DeltaGrace = 2 * time.Second
DeltaCertifiedCommitRequest = 10 * time.Second
DeltaStage = 10 * time.Second
Rmax = 50
MaxDurationQuery = 500 * time.Millisecond
MaxDurationObservation = 5 * time.Second
MaxDurationShouldAcceptAttestedReport = 10 * time.Second
MaxDurationShouldTransmitAcceptedReport = 10 * time.Second
GasPriceDeviationPPB = 1000
DAGasPriceDeviationPPB = 0
OptimisticConfirmations = 1
PermissionLessExecutionThreshold = 8 * time.Hour
RemoteGasPriceBatchWriteFrequency = 30 * time.Minute
TokenPriceBatchWriteFrequency = 30 * time.Minute
BatchGasLimit = 6_500_000
InflightCacheExpiry = 10 * time.Minute
RootSnoozeTime = 30 * time.Minute
BatchingStrategyID = 0
GasPriceDeviationPPB = 1000
DAGasPriceDeviationPPB = 0
OptimisticConfirmations = 1
// ======================================

// ========= Onchain consts =========
Expand All @@ -40,3 +32,31 @@ const (
CCIPLockOrBurnV1RetBytes = 32
// ======================================
)

var (
DefaultCommitOffChainCfg = pluginconfig.CommitOffchainConfig{
RemoteGasPriceBatchWriteFrequency: *config.MustNewDuration(30 * time.Minute),
TokenPriceBatchWriteFrequency: *config.MustNewDuration(30 * time.Minute),
NewMsgScanBatchSize: merklemulti.MaxNumberTreeLeaves,
MaxReportTransmissionCheckAttempts: 5,
RMNSignaturesTimeout: 6900 * time.Millisecond,
RMNEnabled: true,
MaxMerkleTreeSize: merklemulti.MaxNumberTreeLeaves,
SignObservationPrefix: "chainlink ccip 1.6 rmn observation",
TransmissionDelayMultiplier: 1 * time.Minute,
InflightPriceCheckRetries: 10,
MerkleRootAsyncObserverDisabled: false,
MerkleRootAsyncObserverSyncFreq: 4 * time.Second,
MerkleRootAsyncObserverSyncTimeout: 12 * time.Second,
ChainFeeAsyncObserverSyncFreq: 10 * time.Second,
ChainFeeAsyncObserverSyncTimeout: 12 * time.Second,
}
DefaultExecuteOffChainCfg = pluginconfig.ExecuteOffchainConfig{
BatchGasLimit: 6_500_000, // Building batches with 6.5m and transmit with 8m to account for overhead. Clarify with offchain
InflightCacheExpiry: *config.MustNewDuration(5 * time.Minute),
RootSnoozeTime: *config.MustNewDuration(5 * time.Minute), // does not work now
MessageVisibilityInterval: *config.MustNewDuration(8 * time.Hour),
BatchingStrategyID: 0,
TransmissionDelayMultiplier: 1 * time.Minute, // Clarify with offchain
}
)
97 changes: 97 additions & 0 deletions deployment/ccip/changeset/globals/ocr3.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
package globals

import (
"fmt"
"time"

"dario.cat/mergo"

"github.com/smartcontractkit/chainlink/deployment/common/types"
)

// Intention of this file is to be a single source of the truth for OCR3 parameters used by CCIP plugins.
//
// Assumptions:
// - Although, some values are similar between Commit and Execute, we should keep them separate, because
// these plugins have different requirements and characteristics. This way we can avoid misconfiguration
// by accidentally changing parameter for one plugin while adjusting it for the other
// - OCR3 parameters are chain agnostic and should be reused across different chains. There might be some use cases
// for overrides to accommodate specific chain characteristics (e.g. Ethereum).
// However, for most of the cases we should strive to rely on defaults under CommitOCRParams and ExecOCRParams.
// This makes the testing process much easier and increase our confidence that the configuration is safe to use.
// - The fewer overrides the better. Introducing new overrides should be done with caution and only if there's a strong
// justification for it. Moreover, it requires detailed chaos / load testing to ensure that the new parameters are safe to use
// and meet CCIP SLOs
// - Single params must not be stored under const or exposed outside of this file to limit the risk of
// accidental configuration or partial configuration
// - MaxDurations should be set on the latencies observed on various environments using p99 OCR3 latencies
// These values should be specific to the plugin type and should not depend on the chain family
// or the environment in which plugin runs
var (
// CommitOCRParams represents the default OCR3 parameters for all chains (beside Ethereum, see CommitOCRParamsForEthereum).
// Most of the intervals here should be generic enough (and chain agnostic) to be reused across different chains.
CommitOCRParams = types.OCRParameters{
DeltaProgress: 120 * time.Second,
DeltaResend: 30 * time.Second,
DeltaInitial: 20 * time.Second,
DeltaRound: 15 * time.Second,
DeltaGrace: 5 * time.Second,
DeltaCertifiedCommitRequest: 10 * time.Second,
// TransmissionDelayMultiplier overrides DeltaStage
DeltaStage: 25 * time.Second,
Rmax: 3,
MaxDurationQuery: 7 * time.Second,
MaxDurationObservation: 13 * time.Second,
MaxDurationShouldAcceptAttestedReport: 5 * time.Second,
MaxDurationShouldTransmitAcceptedReport: 10 * time.Second,
}

// CommitOCRParamsForEthereum represents a dedicated set of OCR3 parameters for Ethereum.
// It's driven by the fact that Ethereum block time is slow (12 seconds) and chain is considered
// more expensive to other EVM compatible chains
CommitOCRParamsForEthereum = withOverrides(
CommitOCRParams,
types.OCRParameters{
DeltaRound: 90 * time.Second,
DeltaStage: 60 * time.Second,
},
)
)

var (
// ExecOCRParams represents the default OCR3 parameters for all chains (beside Ethereum, see ExecOCRParamsForEthereum).
ExecOCRParams = types.OCRParameters{
DeltaProgress: 100 * time.Second,
DeltaResend: 30 * time.Second,
DeltaInitial: 20 * time.Second,
DeltaRound: 15 * time.Second,
DeltaGrace: 5 * time.Second,
DeltaCertifiedCommitRequest: 10 * time.Second,
// TransmissionDelayMultiplier overrides DeltaStage
DeltaStage: 25 * time.Second,
Rmax: 3,
// MaxDurationQuery is set to very low value, because Execution plugin doesn't use Query
MaxDurationQuery: 200 * time.Millisecond,
MaxDurationObservation: 13 * time.Second,
MaxDurationShouldAcceptAttestedReport: 5 * time.Second,
MaxDurationShouldTransmitAcceptedReport: 10 * time.Second,
}

// ExecOCRParamsForEthereum represents a dedicated set of OCR3 parameters for Ethereum.
// Similarly to Commit, it's here to accommodate Ethereum specific characteristics
ExecOCRParamsForEthereum = withOverrides(
ExecOCRParams,
types.OCRParameters{
DeltaRound: 90 * time.Second,
DeltaStage: 60 * time.Second,
},
)
)

func withOverrides(base types.OCRParameters, overrides types.OCRParameters) types.OCRParameters {
outcome := base
if err := mergo.Merge(&outcome, overrides, mergo.WithOverride); err != nil {
panic(fmt.Sprintf("error while building an OCR config %v", err))
}
return outcome
}
25 changes: 25 additions & 0 deletions deployment/ccip/changeset/globals/ocr3_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package globals

import (
"github.com/stretchr/testify/assert"

Check failure on line 4 in deployment/ccip/changeset/globals/ocr3_test.go

View workflow job for this annotation

GitHub Actions / GolangCI Lint (deployment)

File is not properly formatted (goimports)
"testing"
"time"
)

func Test_MergeWithOverrides(t *testing.T) {
assert.Equal(t, ExecOCRParams.DeltaProgress, ExecOCRParamsForEthereum.DeltaProgress)
assert.Equal(t, ExecOCRParams.DeltaResend, ExecOCRParamsForEthereum.DeltaResend)
assert.Equal(t, ExecOCRParams.DeltaInitial, ExecOCRParamsForEthereum.DeltaInitial)
assert.Equal(t, ExecOCRParams.DeltaGrace, ExecOCRParamsForEthereum.DeltaGrace)
assert.Equal(t, ExecOCRParams.DeltaCertifiedCommitRequest, ExecOCRParamsForEthereum.DeltaCertifiedCommitRequest)
assert.Equal(t, ExecOCRParams.MaxDurationQuery, ExecOCRParamsForEthereum.MaxDurationQuery)
assert.Equal(t, ExecOCRParams.MaxDurationObservation, ExecOCRParamsForEthereum.MaxDurationObservation)
assert.Equal(t, ExecOCRParams.MaxDurationShouldAcceptAttestedReport, ExecOCRParamsForEthereum.MaxDurationShouldAcceptAttestedReport)
assert.Equal(t, ExecOCRParams.MaxDurationShouldTransmitAcceptedReport, ExecOCRParamsForEthereum.MaxDurationShouldTransmitAcceptedReport)
assert.Equal(t, ExecOCRParams.MaxDurationQuery, ExecOCRParamsForEthereum.MaxDurationQuery)

assert.Equal(t, 90*time.Second, ExecOCRParamsForEthereum.DeltaRound)
assert.Equal(t, 60*time.Second, ExecOCRParamsForEthereum.DeltaStage)
assert.Equal(t, 200*time.Millisecond, ExecOCRParams.MaxDurationQuery)
assert.Equal(t, 200*time.Millisecond, ExecOCRParamsForEthereum.MaxDurationQuery)
}
47 changes: 22 additions & 25 deletions deployment/ccip/changeset/testhelpers/test_environment.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ type TestConfigs struct {
IsUSDCAttestationMissing bool
IsMultiCall3 bool
IsStaticLink bool
OCRConfigOverride func(*v1_6.CCIPOCRParams)
OCRConfigOverride func(v1_6.CCIPOCRParams) v1_6.CCIPOCRParams
RMNEnabled bool
NumOfRMNNodes int
LinkPrice *big.Int
Expand Down Expand Up @@ -174,7 +174,7 @@ func WithRMNEnabled(numOfNode int) TestOps {
}
}

func WithOCRConfigOverride(override func(*v1_6.CCIPOCRParams)) TestOps {
func WithOCRConfigOverride(override func(v1_6.CCIPOCRParams) v1_6.CCIPOCRParams) TestOps {
return func(testCfg *TestConfigs) {
testCfg.OCRConfigOverride = override
}
Expand Down Expand Up @@ -662,7 +662,8 @@ func AddCCIPContractsToEnvironment(t *testing.T, allChains []uint64, tEnv TestEn
require.NoError(t, err)
// Build the per chain config.
chainConfigs := make(map[uint64]v1_6.ChainConfig)
ocrConfigs := make(map[uint64]v1_6.CCIPOCRParams)
commitOCRConfigs := make(map[uint64]v1_6.CCIPOCRParams)
execOCRConfigs := make(map[uint64]v1_6.CCIPOCRParams)
for _, chain := range evmChains {
timelockContractsPerChain[chain] = &proposalutils.TimelockExecutionContracts{
Timelock: state.Chains[chain].Timelock,
Expand All @@ -674,22 +675,23 @@ func AddCCIPContractsToEnvironment(t *testing.T, allChains []uint64, tEnv TestEn
} else {
linkTokenAddr = state.Chains[chain].LinkToken.Address()
}
tokenInfo := tokenConfig.GetTokenInfo(e.Env.Logger, linkTokenAddr, state.Chains[chain].Weth9.Address())
ocrOverride := tc.OCRConfigOverride
if tc.RMNEnabled {
ocrOverride = func(ocrParams *v1_6.CCIPOCRParams) {
if tc.OCRConfigOverride != nil {
tc.OCRConfigOverride(ocrParams)
ocrOverride := func(ocrParams v1_6.CCIPOCRParams) v1_6.CCIPOCRParams {
if tc.OCRConfigOverride != nil {
tc.OCRConfigOverride(ocrParams)
}
if tc.RMNEnabled {
if ocrParams.CommitOffChainConfig != nil {
ocrParams.CommitOffChainConfig.RMNEnabled = true
}
} else {
if ocrParams.CommitOffChainConfig != nil {
ocrParams.CommitOffChainConfig.RMNEnabled = false
}
ocrParams.CommitOffChainConfig.RMNEnabled = true
}
return ocrParams
}
ocrParams := v1_6.DeriveCCIPOCRParams(
v1_6.WithDefaultCommitOffChainConfig(e.FeedChainSel, tokenInfo),
v1_6.WithDefaultExecuteOffChainConfig(tokenDataProviders),
v1_6.WithOCRParamOverride(ocrOverride),
)
ocrConfigs[chain] = ocrParams
commitOCRConfigs[chain] = v1_6.DeriveOCRParamsForCommit(v1_6.SimulationTest, e.FeedChainSel, tokenConfig.GetTokenInfo(e.Env.Logger, linkTokenAddr, state.Chains[chain].Weth9.Address()), ocrOverride)
execOCRConfigs[chain] = v1_6.DeriveOCRParamsForExec(v1_6.SimulationTest, tokenDataProviders, ocrOverride)
chainConfigs[chain] = v1_6.ChainConfig{
Readers: nodeInfo.NonBootstraps().PeerIDs(),
FChain: uint8(len(nodeInfo.NonBootstraps().PeerIDs()) / 3),
Expand All @@ -703,13 +705,8 @@ func AddCCIPContractsToEnvironment(t *testing.T, allChains []uint64, tEnv TestEn

for _, chain := range solChains {
ocrOverride := tc.OCRConfigOverride
ocrParams := v1_6.DeriveCCIPOCRParams(
// TODO: tokenInfo is nil for solana
v1_6.WithDefaultCommitOffChainConfig(e.FeedChainSel, nil),
v1_6.WithDefaultExecuteOffChainConfig(tokenDataProviders),
v1_6.WithOCRParamOverride(ocrOverride),
)
ocrConfigs[chain] = ocrParams
commitOCRConfigs[chain] = v1_6.DeriveOCRParamsForCommit(v1_6.SimulationTest, e.FeedChainSel, nil, ocrOverride)
execOCRConfigs[chain] = v1_6.DeriveOCRParamsForExec(v1_6.SimulationTest, tokenDataProviders, ocrOverride)
chainConfigs[chain] = v1_6.ChainConfig{
Readers: nodeInfo.NonBootstraps().PeerIDs(),
// #nosec G115 - Overflow is not a concern in this test scenario
Expand Down Expand Up @@ -750,7 +747,7 @@ func AddCCIPContractsToEnvironment(t *testing.T, allChains []uint64, tEnv TestEn
MCMS: mcmsConfig,
},
PluginInfo: v1_6.SetCandidatePluginInfo{
OCRConfigPerRemoteChainSelector: ocrConfigs,
OCRConfigPerRemoteChainSelector: commitOCRConfigs,
PluginType: types.PluginTypeCCIPCommit,
},
},
Expand All @@ -767,7 +764,7 @@ func AddCCIPContractsToEnvironment(t *testing.T, allChains []uint64, tEnv TestEn
},
PluginInfo: []v1_6.SetCandidatePluginInfo{
{
OCRConfigPerRemoteChainSelector: ocrConfigs,
OCRConfigPerRemoteChainSelector: execOCRConfigs,
PluginType: types.PluginTypeCCIPExec,
},
},
Expand Down
Loading
Loading