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

[DPA-1361]: feat(deployment/mcms): update common changesets #16249

Open
wants to merge 1 commit into
base: ggoh/DPA-1367/test-helpers
Choose a base branch
from

Conversation

graham-chainlink
Copy link
Collaborator

  • Added new v2 version of set_config_mcms to support new MCMS library
  • Added new v2 version of transfer_to_mcms_with_timelock to support new MCMS library

JIRA: https://smartcontract-it.atlassian.net/browse/DPA-1361

- Added new v2 version of set_config_mcms to support new MCMS library
- Added new v2 version of transfer_to_mcms_with_timelock to support new MCMS library

JIRA: https://smartcontract-it.atlassian.net/browse/DPA-1361
@cl-sonarqube-production
Copy link

Quality Gate failed Quality Gate failed

Failed conditions
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube

Catch issues before they fail your Quality Gate with our IDE extension SonarLint SonarLint

Copy link
Contributor

github-actions bot commented Feb 6, 2025

AER Report: CI Core

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

1. GolangCI Lint errors: Golang Lint (deployment)

Source of Error:
Golang Lint (deployment)	2025-02-06T04:12:31.0575019Z ##[error]deployment/common/changeset/transfer_to_mcms_with_timelock.go:163:2: Consider pre-allocating `batches` (prealloc)
Golang Lint (deployment)	2025-02-06T04:12:31.0576113Z 	var batches []mcmstypes.BatchOperation
Golang Lint (deployment)	2025-02-06T04:12:31.0576537Z 	^
Golang Lint (deployment)	2025-02-06T04:12:31.0578059Z ##[error]deployment/common/changeset/transfer_to_mcms_with_timelock.go:200:3: unnecessary trailing newline (whitespace)
Golang Lint (deployment)	2025-02-06T04:12:31.0578775Z 
Golang Lint (deployment)	2025-02-06T04:12:31.0578910Z ^

Why: The linter found two issues: a suggestion to pre-allocate the batches slice to improve performance and a trailing newline that is unnecessary.

Suggested fix: Pre-allocate the batches slice by specifying its capacity if known. Remove the trailing newline at the specified location.

2. Test failure: Core Tests (go_core_tests)

Source of Error:
Run tests	2025-02-06T04:24:03.3890864Z --- FAIL: Test_BackupLogPoller (0.23s)
Run tests	2025-02-06T04:24:03.3891855Z --- FAIL: Test_BackupLogPoller/fixed_finality_depth_without_finality_tag (0.06s)
Run tests	2025-02-06T04:24:03.3893646Z logger.go:146: 04:22:00.564380156	DEBUG	LogPoller	New logger: LogPoller	{"version": "unset@unset"}
Run tests	2025-02-06T04:24:03.3894603Z log_poller_test.go:323: 
Run tests	2025-02-06T04:24:03.3896187Z 	Error Trace:	/home/runner/work/chainlink/chainlink/core/chains/evm/logpoller/log_poller_test.go:323
Run tests	2025-02-06T04:24:03.3897773Z 	Error: 	"[0xc001162000 0xc001162080]" should have 3 item(s), but has 2
Run tests	2025-02-06T04:24:03.3899206Z 	Test: 	Test_BackupLogPoller/fixed_finality_depth_without_finality_tag

Why: The test Test_BackupLogPoller/fixed_finality_depth_without_finality_tag failed because it expected 3 items in the result but only found 2.

Suggested fix: Investigate the logic in Test_BackupLogPoller/fixed_finality_depth_without_finality_tag to ensure it correctly handles the conditions being tested. Verify that the setup and assertions in the test are accurate and reflect the expected behavior.

@@ -206,3 +210,75 @@ func SetConfigMCMS(e deployment.Environment, cfg MCMSConfig) (deployment.Changes

return deployment.ChangesetOutput{}, nil
}

// SetConfigMCMSV2 is a reimplementation of SetConfigMCMS that uses the new MCMS library.
func SetConfigMCMSV2(e deployment.Environment, cfg MCMSConfig) (deployment.ChangesetOutput, error) {

Choose a reason for hiding this comment

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

Just an early comment: @ecPablo (and also Rodrigo?) were planning on using a different folder layout for the changesets that rely on the new mcms lib.

See this PR to understand what I mean (just the mcmcnew folder).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah okay i didn know, happy to follow

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants