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

Add ccip message received assertion #16072

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

joaoluisam
Copy link
Contributor

This pull request includes significant updates to the MaybeRevertMessageReceiver contract and related integration tests. The changes enhance the contract's functionality, improve error handling, and extend the test coverage to ensure robustness.

Contract Enhancements:

  • Added new errors (Unauthorized, InsufficientBalance, TransferFailed) and events (FundsWithdrawn, TokensWithdrawn) to the MaybeRevertMessageReceiver contract.
  • Introduced onlyManager modifier to restrict certain functions to the contract manager.
  • Added functions to withdraw Ether and ERC-20 tokens from the contract, and to check the balance of ERC-20 tokens.

Integration Test Updates:

  • Updated the SendRequest function in ccip_helpers.go to return message data and handle additional error cases. [1] [2] [3] [4]
  • Added a new watcher for the MessageReceived event and a function to assert that the message content matches the expected content. [1] [2]
  • Modified the DeployContracts function to log the deployment of the receiver Dapp and set a flag indicating its deployment status. [1] [2]
  • Enhanced the CCIPLane struct and related functions to handle and validate message data. [1] [2] [3] [4] [5]

Dependency Updates:

  • Updated the dependency version for MaybeRevertMessageReceiver in generated-wrapper-dependency-versions-do-not-edit.txt.
  • Added import for maybe_revert_message_receiver in ccip_helpers.go.

Copy link
Contributor

I see you updated files related to contracts. Please run pnpm changeset in the contracts directory to add a changeset.

@cl-sonarqube-production
Copy link

Quality Gate failed Quality Gate failed

Failed conditions
C Reliability Rating on New Code (required ≥ A)
7 New Major Issues (required ≤ 5)

See analysis details on SonarQube

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

Copy link
Contributor

AER Report: CI Core

aer_workflow , commit , Clean Go Tidy & Generate , Detect Changes , Scheduled Run Frequency , 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) , GolangCI Lint (.) , Core Tests (go_core_race_tests) , GolangCI Lint (integration-tests) , lint , SonarQube Scan

1. Linting errors in ccip_helpers.go file:[Golang Lint (integration-tests)]

Source of Error:
Golang Lint (integration-tests)	2025-01-26T14:25:22.3285915Z ##[error]integration-tests/ccip-tests/actions/ccip_helpers.go:3662:29: unslice: could simplify e.Sender[:] to e.Sender (gocritic)
Golang Lint (integration-tests)	2025-01-26T14:25:22.3287047Z 				messageSender := string(e.Sender[:])
Golang Lint (integration-tests)	2025-01-26T14:25:22.3287639Z 				 ^
Golang Lint (integration-tests)	2025-01-26T14:25:22.3289396Z ##[error]integration-tests/ccip-tests/actions/ccip_helpers.go:3660:5: var-naming: var messageId should be messageID (revive)
Golang Lint (integration-tests)	2025-01-26T14:25:22.3290309Z 				messageId := string(e.MessageId[:])
Golang Lint (integration-tests)	2025-01-26T14:25:22.3290719Z 				^
Golang Lint (integration-tests)	2025-01-26T14:25:22.3291688Z ##[error]integration-tests/ccip-tests/actions/ccip_helpers.go:2727:42: unnecessary conversion (unconvert)
Golang Lint (integration-tests)	2025-01-26T14:25:22.3292611Z 		Str("MsgID", fmt.Sprintf("0x%x", string(messageID))).
Golang Lint (integration-tests)	2025-01-26T14:25:22.3293483Z 		 ^
Golang Lint (integration-tests)	2025-01-26T14:25:22.3294579Z ##[error]integration-tests/ccip-tests/actions/ccip_helpers.go:2747:45: unnecessary conversion (unconvert)
Golang Lint (integration-tests)	2025-01-26T14:25:22.3295693Z 					Str("MsgID", fmt.Sprintf("0x%x", string(messageID))).
Golang Lint (integration-tests)	2025-01-26T14:25:22.3296526Z 					 ^
Golang Lint (integration-tests)	2025-01-26T14:25:22.3297581Z ##[error]integration-tests/ccip-tests/actions/ccip_helpers.go:2755:34: unnecessary conversion (unconvert)
Golang Lint (integration-tests)	2025-01-26T14:25:22.3298507Z 					Str("MessageID 0x%x", string(messageID)).
Golang Lint (integration-tests)	2025-01-26T14:25:22.3299234Z 					 ^
Golang Lint (integration-tests)	2025-01-26T14:25:22.3300421Z ##[error]integration-tests/ccip-tests/actions/ccip_helpers.go:2749:12: fmt.Errorf can be replaced with errors.New (perfsprint)
Golang Lint (integration-tests)	2025-01-26T14:25:22.3301544Z 				return fmt.Errorf("invalid content type in MessageReceivedWatcher")
Golang Lint (integration-tests)	2025-01-26T14:25:22.3302209Z 				 ^
Golang Lint (integration-tests)	2025-01-26T14:25:22.3303314Z ##[error]integration-tests/ccip-tests/actions/ccip_helpers.go:3653:10: fmt.Errorf can be replaced with errors.New (perfsprint)
Golang Lint (integration-tests)	2025-01-26T14:25:22.3304364Z 		return fmt.Errorf("failed to subscribe to message received event")
Golang Lint (integration-tests)	2025-01-26T14:25:22.3305103Z 		 ^

Why: The errors are caused by code style and best practice violations detected by the golangci-lint tool. These include unnecessary slicing, improper variable naming, unnecessary type conversions, and inefficient error handling.

Suggested fix: Refactor the code to adhere to the linting rules. For example, simplify slices, rename variables to follow naming conventions, remove unnecessary type conversions, and replace fmt.Errorf with errors.New where applicable.

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.

1 participant