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

Refactor DefaultCapConfig into multiple funcs #15882

Draft
wants to merge 9 commits into
base: develop
Choose a base branch
from
Draft

Refactor DefaultCapConfig into multiple funcs #15882

wants to merge 9 commits into from
+24 −72

Conversation

vyzaldysanchez
Copy link
Contributor

Requires

Supports

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 9, 2025 •
edited
Loading

AER Report: CI Core

aer_workflow , commit , Scheduled Run Frequency , Clean Go Tidy & Generate , Detect Changes , GolangCI Lint (deployment) , Core Tests (go_core_tests) , Core Tests (go_core_tests_integration) , Core Tests (go_core_ccip_deployment_tests) , Core Tests (go_core_fuzz) , Core Tests (go_core_race_tests) , test-scripts , lint , SonarQube Scan

1. Configuration not found for capability: [job id where the error happened]

Source of Error: 
Run tests	2025-01-10T18:07:22.8818162Z 	Error Trace:	/home/runner/work/chainlink/chainlink/deployment/keystone/changeset/test/helpers.go:208
Run tests	2025-01-10T18:07:22.8819152Z 	 				/home/runner/work/chainlink/chainlink/deployment/keystone/changeset/test/helpers_test.go:17
Run tests	2025-01-10T18:07:22.8819392Z 	Error: 	Received unexpected error:
Run tests	2025-01-10T18:07:22.8821947Z 	 	failed to configure contracts: failed to configure registry: failed to register DONS: config not found for capability {{offchain_reporting 1.0.0 2 0 0x0000000000000000000000000000000000000000} [87 142 191 116 19 193 94 54 223 215 146 57 108 90 77 117 196 63 14 231 187 171 219 112 63 124 90 205 11 182 90 27] []}
Run tests	2025-01-10T18:07:22.8822145Z 	Test: 	TestSetupTestEnv

Why: The error indicates that the configuration for the specified capability (offchain_reporting 1.0.0) was not found during the contract configuration process.

Suggested fix: Ensure that the configuration for the offchain_reporting 1.0.0 capability is correctly defined and accessible in the environment where the tests are being run.

2. Configuration not found for capability: [job id where the error happened]

Source of Error: 
Run tests	2025-01-10T18:07:22.9114076Z 	Error Trace:	/home/runner/work/chainlink/chainlink/deployment/keystone/changeset/test/helpers.go:208
Run tests	2025-01-10T18:07:22.9115730Z 	 				/home/runner/work/chainlink/chainlink/deployment/keystone/changeset/workflowregistry/update_allowed_dons_test.go:75
Run tests	2025-01-10T18:07:22.9116437Z 	Error: 	Received unexpected error:
Run tests	2025-01-10T18:07:22.9119417Z 	 	failed to configure contracts: failed to configure registry: failed to register DONS: config not found for capability {{offchain_reporting 1.0.0 2 0 0x0000000000000000000000000000000000000000} [87 142 191 116 19 193 94 54 223 215 146 57 108 90 77 117 196 63 14 231 187 171 219 112 63 124 90 205 11 182 90 27] []}
Run tests	2025-01-10T18:07:22.9120541Z 	Test: 	Test_UpdateAllowedDons_WithMCMS

Why: The error indicates that the configuration for the specified capability (offchain_reporting 1.0.0) was not found during the contract configuration process.

Suggested fix: Ensure that the configuration for the offchain_reporting 1.0.0 capability is correctly defined and accessible in the environment where the tests are being run.

3. Configuration not found for capability: [job id where the error happened]

Source of Error: 
Run tests	2025-01-10T18:07:22.9490433Z 	Error Trace:	/home/runner/work/chainlink/chainlink/deployment/keystone/changeset/test/helpers.go:208
Run tests	2025-01-10T18:07:22.9492296Z 	 				/home/runner/work/chainlink/chainlink/deployment/keystone/changeset/workflowregistry/update_authorized_addresses_test.go:77
Run tests	2025-01-10T18:07:22.9493029Z 	Error: 	Received unexpected error:
Run tests	2025-01-10T18:07:22.9495769Z 	 	failed to configure contracts: failed to configure registry: failed to register DONS: config not found for capability {{offchain_reporting 1.0.0 2 0 0x0000000000000000000000000000000000000000} [87 142 191 116 19 193 94 54 223 215 146 57 108 90 77 117 196 63 14 231 187 171 219 112 63 124 90 205 11 182 90 27] []}
Run tests	2025-01-10T18:07:22.9496923Z 	Test: 	Test_UpdateAuthorizedAddresses_WithMCMS

Why: The error indicates that the configuration for the specified capability (offchain_reporting 1.0.0) was not found during the contract configuration process.

Suggested fix: Ensure that the configuration for the offchain_reporting 1.0.0 capability is correctly defined and accessible in the environment where the tests are being run.

AER Report: Operator UI CI ran successfully ✅

aer_workflow , commit

krehermann
krehermann reviewed Jan 9, 2025
View reviewed changes
deployment/keystone/changeset/internal/deploy.go Outdated
}

func FromCapabilitiesRegistryCapability(cap *capabilities_registry.CapabilitiesRegistryCapability, e deployment.Environment, registryChainSelector uint64) (*RegisteredCapability, error) {
type RegisteredCapabilityConfig struct {
DefaultConfig map[string]any
Copy link
Contributor

Choose a reason for hiding this comment

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

this doesn't seem right. is the config serialized to the chain?

krehermann
krehermann reviewed Jan 9, 2025
View reviewed changes
deployment/keystone/changeset/internal/deploy.go Outdated
ExecutableConfig *RegisteredCapabilityRemoteExecutableConfig
}

type RegisteredCapabilityRemoteTriggerConfig struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

how are you determining this type? is there some more basic building-block type that we can use?

deployment/keystone/changeset/internal/deploy.go Outdated
BatchCollectionPeriod time.Duration
}

type RegisteredCapabilityRemoteTargetConfig struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

are these structs used?

deployment/keystone/changeset/internal/deploy.go Outdated
RequestHashExcludedAttributes: []string{"signed_report.Signatures"}, // TODO: const defn in a common place
},
func GetTriggerCapConfig(refresh time.Duration, expiry time.Duration, minResponsesToAggregate uint32, defaultCfg map[string]any) (*capabilitiespb.CapabilityConfig, error) {
dCfg, err := values.WrapMap(defaultCfg)
Copy link
Contributor

Choose a reason for hiding this comment

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

better to have the caller pass a values.Map (or what ever the

deployment/keystone/changeset/internal/deploy.go Outdated
}, nil
}

func GetConsensusCapConfig(defaultCfg map[string]any) (*capabilitiespb.CapabilityConfig, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

these functions are odd - they do very little and add little abstraction. why bother? the shouldn't be public, at minimum.

@cl-sonarqube-production
Copy link

Quality Gate passed Quality Gate passed

Issues
1 New issue
6 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@krehermann krehermann Awaiting requested review from krehermann

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@vyzaldysanchez @krehermann