-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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-4333 Handle all test setups with unified interface for memory and docker tests #15581
Changes from 13 commits
955c365
dd03e54
49bc15a
dd31ca0
ea6796d
0450e47
682fa2b
398bbe5
46d0455
cabda62
06b14c2
ae72929
c05af5a
f28b309
8b8c7ca
ddf63ba
5620eaa
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 |
---|---|---|
|
@@ -10,6 +10,7 @@ import ( | |
|
||
"github.com/ethereum/go-ethereum/accounts/abi/bind" | ||
"github.com/ethereum/go-ethereum/common" | ||
"github.com/imdario/mergo" | ||
"github.com/smartcontractkit/ccip-owner-contracts/pkg/proposal/timelock" | ||
|
||
"github.com/smartcontractkit/chainlink-ccip/chainconfig" | ||
|
@@ -59,6 +60,16 @@ type CCIPOCRParams struct { | |
ExecuteOffChainConfig pluginconfig.ExecuteOffchainConfig | ||
} | ||
|
||
// Override overrides non-empty dst CCIPOCRParams attributes with non-empty src CCIPOCRParams attributes values | ||
// and returns the updated CCIPOCRParams. | ||
func (c CCIPOCRParams) Override(overrides CCIPOCRParams) (CCIPOCRParams, error) { | ||
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. Not particularly against this approach but we already have a paradigm (if we want to call it that) of overriding via a 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. Think I prefer the approach we have now due to no new dependency and less magic / reflection, which it seems like this 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 did not know we already have one. Should I completely remove this? 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. oh you mean the one in test config. ah I see okay cool I can remove this and make it similar to what used in test config |
||
err := mergo.Merge(&c, &overrides, mergo.WithOverride) | ||
if err != nil { | ||
return CCIPOCRParams{}, err | ||
} | ||
return c, nil | ||
} | ||
|
||
func (c CCIPOCRParams) Validate() error { | ||
if err := c.OCRParameters.Validate(); err != nil { | ||
return fmt.Errorf("invalid OCR parameters: %w", err) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
package changeset | ||
|
||
import ( | ||
"testing" | ||
"time" | ||
|
||
chainselectors "github.com/smartcontractkit/chain-selectors" | ||
"github.com/smartcontractkit/chainlink-ccip/pluginconfig" | ||
"github.com/smartcontractkit/chainlink-common/pkg/config" | ||
"github.com/stretchr/testify/require" | ||
) | ||
|
||
func TestOverrideCCIPParams(t *testing.T) { | ||
params := DefaultOCRParams(chainselectors.ETHEREUM_TESTNET_SEPOLIA.Selector, nil, nil) | ||
overrides := CCIPOCRParams{ | ||
ExecuteOffChainConfig: pluginconfig.ExecuteOffchainConfig{ | ||
RelativeBoostPerWaitHour: 10, | ||
}, | ||
CommitOffChainConfig: pluginconfig.CommitOffchainConfig{ | ||
TokenPriceBatchWriteFrequency: *config.MustNewDuration(1_000_000 * time.Hour), | ||
RemoteGasPriceBatchWriteFrequency: *config.MustNewDuration(1_000_000 * time.Hour), | ||
}, | ||
} | ||
newParams, err := params.Override(overrides) | ||
require.NoError(t, err) | ||
require.Equal(t, overrides.ExecuteOffChainConfig.RelativeBoostPerWaitHour, newParams.ExecuteOffChainConfig.RelativeBoostPerWaitHour) | ||
require.Equal(t, overrides.CommitOffChainConfig.TokenPriceBatchWriteFrequency, newParams.CommitOffChainConfig.TokenPriceBatchWriteFrequency) | ||
require.Equal(t, overrides.CommitOffChainConfig.RemoteGasPriceBatchWriteFrequency, newParams.CommitOffChainConfig.RemoteGasPriceBatchWriteFrequency) | ||
require.Equal(t, params.OCRParameters, newParams.OCRParameters) | ||
} |
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.
Idea is we will use DefaultOCRParams and only mention the non-nil param values to override from chainlink-deployments