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

CCIP Load Test connected to crib (not yet working) #15404

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

CCIP Load Test connected to crib (not yet working) #15404

wants to merge 7 commits into from
+418 −0

Conversation

0xAustinWang
Copy link
Contributor

@0xAustinWang 0xAustinWang requested review from a team as code owners November 25, 2024 15:43
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Nov 25, 2024 •
edited
Loading

AER Report: CI Core

aer_workflow , commit , Detect Changes , Scheduled Run Frequency , Clean Go Tidy & Generate , Flakeguard Root Project / Get Tests To Run , lint , Core Tests (go_core_tests) , Core Tests (go_core_tests_integration) , Core Tests (go_core_ccip_deployment_tests) , Core Tests (go_core_race_tests) , Core Tests (go_core_fuzz) , Flakeguard Deployment Project / Get Tests To Run , Flakeguard Root Project / Run Tests , Flakeguard Root Project / Report , Flakeguard Deployment Project / Run Tests , Flakeguard Deployment Project / Report , Flakey Test Detection , SonarQube Scan

1. Go module update needed:[make generate]

Source of Error: 
make generate	2024-12-17T11:44:43.3768764Z 	go: updates to go.mod needed; to update it:
make generate	2024-12-17T11:44:43.3768856Z 		go mod tidy
make generate	2024-12-17T11:44:43.3768960Z 	error: exit status 1

Why: The error indicates that the Go modules require updates, and the go.mod file is not in sync with the dependencies. The go mod tidy command needs to be run to update the go.mod file.

Suggested fix: Run go mod tidy to update the go.mod file and ensure all dependencies are correctly listed. This will resolve the discrepancy and allow the generation process to complete successfully.

AER Report: Operator UI CI ran successfully ✅

aer_workflow , commit

AnieeG
AnieeG reviewed Dec 3, 2024
View reviewed changes
deployment/environment.go Outdated
@@ -35,6 +35,7 @@ type OnchainClient interface {
bind.DeployBackend
BalanceAt(ctx context.Context, account common.Address, blockNumber *big.Int) (*big.Int, error)
NonceAt(ctx context.Context, account common.Address, blockNumber *big.Int) (uint64, error)
BlockByNumber(ctx context.Context, number *big.Int) (*types.Block, error)
Copy link
Contributor

Choose a reason for hiding this comment

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

We use HeaderByNumber(context, nil) to get the latest block. Can you use that instead?

integration-tests/load/ccip/helpers.go Outdated
SequenceNumber uint64 `json:"sequence_number"`
}

func GetAddressFromTypeAndVersion(ab deployment.AddressBook, cs uint64, tv string) (common.Address, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

is it being used anywhere?

integration-tests/load/ccip/helpers.go Outdated
Comment on lines 51 to 48
"sourceSelector": fmt.Sprintf("%d", src),
"destinationSelector": fmt.Sprintf("%d", dst),
Copy link
Contributor

Choose a reason for hiding this comment

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

would it be easier to use names/chain id instead? Labelling by selector can create confusion as selector is a large number

integration-tests/load/ccip/helpers.go Outdated

func SendMetricsToLoki(l zerolog.Logger, lc *wasp.LokiClient, updatedLabels map[string]string, metrics *LokiMetric) {
if err := lc.HandleStruct(wasp.LabelsMapToModel(updatedLabels), time.Now(), metrics); err != nil {
l.Error().Err(err).Msg(ErrLokiPush)
Copy link
Contributor

Choose a reason for hiding this comment

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

do you not want the test to fail if it fails to push?

integration-tests/load/ccip/destination_gun.go

return router.ClientEVM2AnyMessage{
Receiver: rcv,
Data: common.Hex2Bytes("hello world"),
Copy link
Contributor

Choose a reason for hiding this comment

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

How about using some identifier here like msg with id to distinguish.

integration-tests/load/ccip/destination_gun.go
m.env.Chains[src].DeployerKey.Value = fee
defer func() { m.env.Chains[src].DeployerKey.Value = nil }()
}
_, err = r.CcipSend(
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you approving router to spend fee tokens in the beginning?

integration-tests/load/ccip/destination_gun.go
m.chainSelector,
msg)
if err != nil {
return &wasp.Response{Error: err.Error(), Group: waspGroup, Failed: true}
Copy link
Contributor

Choose a reason for hiding this comment

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

same comment about using deployment.MayBeDataError(err)

integration-tests/load/ccip/destination_gun.go
}

lokiLabels := setLokiLabels(src, m.chainSelector)
SendMetricsToLoki(m.l, m.loki, lokiLabels, &LokiMetric{
Copy link
Contributor

Choose a reason for hiding this comment

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

is this a blocking call?

integration-tests/load/ccip/ccip_test.go
// Parse all events from the simulated chains, send to Loki
// step 4: teardown
// Stop the chains, cleanup the environment
func TestCCIPLoad_RPS(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

A lot of code in this can be extracted to common functions. You might need more than one load test in future

integration-tests/load/ccip/ccip_test.go Outdated Show resolved Hide resolved
@0xAustinWang 0xAustinWang requested a review from a team as a code owner December 11, 2024 11:14
@0xAustinWang 0xAustinWang marked this pull request as draft December 11, 2024 11:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AnieeG AnieeG AnieeG left review comments

@winder winder Awaiting requested review from winder winder is a code owner automatically assigned from smartcontractkit/ccip-offchain

@rstout rstout Awaiting requested review from rstout rstout is a code owner automatically assigned from smartcontractkit/ccip-offchain

@asoliman92 asoliman92 Awaiting requested review from asoliman92 asoliman92 is a code owner automatically assigned from smartcontractkit/ccip-offchain

@makramkd makramkd Awaiting requested review from makramkd makramkd is a code owner automatically assigned from smartcontractkit/ccip-offchain

@dimkouv dimkouv Awaiting requested review from dimkouv dimkouv is a code owner automatically assigned from smartcontractkit/ccip-offchain

@mateusz-sekara mateusz-sekara Awaiting requested review from mateusz-sekara mateusz-sekara is a code owner automatically assigned from smartcontractkit/ccip-offchain

@0xnogo 0xnogo Awaiting requested review from 0xnogo 0xnogo is a code owner automatically assigned from smartcontractkit/ccip-offchain

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
@0xAustinWang @AnieeG