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

[TT-791] Migrate OCRv1/v2 tests from EVMClient to Seth #12076

Merged
merged 24 commits into from
Feb 28, 2024

Conversation

Tofel
Copy link
Contributor

@Tofel Tofel commented Feb 19, 2024

What was done:

  • all OCRv1/v2 load & soak tests use Seth
  • OCRv2 smoke tests use Seth
  • Seth was added as an option to docker test environment builder
  • added Seth-based versions of OCR helpers
  • left existing OCR helper mainly unchanged, so that no downstream projects are forced to migrate, when they merge core repo
  • added Seth-specific wrappers for Ethereum contracts, renamed ones using EVMClient to Legacy*
  • added a small fix for forwarder OCR tests to make them compatible with OCR v2 (and made sure that this variant passes)
  • removed some methods from old contract interfaces that weren't used anywhere
  • added chain of responsibility-based handling of errors during return of funds (something that imho should be placed in CTF) [tested on Polygon Mumbai]
  • unified some functions with their _local equivalents by introducing smallest possible surface interfaces that define methods common to local and k8s Chainlink client (because method signature was the only difference between two functions)
  • included bash script to set base64 override, when running soak/load tests via Make

General approach to migration:

  • do not remove any existing helper functions and limit their modifications to minimum
  • add parallel Seth-based helpers and use them in tests
  • once all tests and helpers have been migrated sync with downstream projects and schedule their migration
  • once all downstream projects are migrated remove legacy helper functions and contract wrappers

Log created by funds returning retrier:

    log.go:43: 12:09:00.57 INF Attempting to return Chainlink node funds to default network wallets
    log.go:43: 12:09:04.54 INF Overshot error detected, retrying with less funds
    log.go:43: 12:09:34.97 DBG New amount to send diff=2100000000000000 new amount=97430379995303806 old amount=99530379995303806 retier=OvershotTransferRetrier
    log.go:43: 12:09:52.18 DBG Awaiting transaction
    log.go:43: 12:09:53.18 INF Transaction accepted BlockNumber=46380080
    log.go:43: 12:09:53.18 INF Retry successful retier=OvershotTransferRetrier

Sample WASP run logs:

1:52PM INF Finished all schedule segments Component=ocr RPS=0 Segment=1 VUs=1
1:52PM INF Scheduler exited Component=ocr
1:52PM INF Load stats CallTimeout=0 Component=ocr Failed=0 Success=25
1:52PM INF Stats loop exited Component=ocr
1:52PM INF Collect data exited Component=ocr
1:52PM INF Loki responses exited Component=ocr
1:53PM INF Loki stats exited Component=ocr
1:53PM INF Stopping Loki Component=ocr
1:53PM INF Loki exited Component=ocr
--- PASS: TestOCRVolume (220.23s)

…they weren't used outside of OCR soak tests, renamed old Ethereum contracts to Legacy* (only ones reimplemented with Seth), removed unused methods from contract interfaces, added Seth to testconfig
@Tofel Tofel changed the title Tt 791 seth ocr tests [TT-791] seth ocr tests Feb 22, 2024
@Tofel Tofel changed the title [TT-791] seth ocr tests [TT-791] Migrate OCRv1/v2 tests from EVMClient to Seth Feb 26, 2024
Copy link
Contributor

I see that you haven't updated any README files. Would it make sense to do so?

Copy link
Contributor

I see that you haven't updated any CHANGELOG files. Would it make sense to do so?

@skudasov skudasov marked this pull request as ready for review February 27, 2024 17:02
@skudasov skudasov requested review from a team as code owners February 27, 2024 17:02
integration-tests/actions/seth/actions.go Show resolved Hide resolved
integration-tests/actions/seth/actions.go Show resolved Hide resolved
integration-tests/actions/seth/actions.go Show resolved Hide resolved
integration-tests/actions/seth/actions.go Outdated Show resolved Hide resolved
integration-tests/actions/seth/actions.go Show resolved Hide resolved
integration-tests/actions/seth/actions.go Show resolved Hide resolved
numberOfContracts int,
linkTokenContractAddress common.Address,
workerNodes []*client.ChainlinkK8sClient,
getTransmitterAndPayeesFn func() ([]string, []string, error),
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm curious why we pass in functions for this instead of just asking for the data directly? Is it more concise?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought that this way we don't have copy & paste or encapsulate in yet another method the different codes that retrieve these addresses from nodes or forwarder contracts

integration-tests/actions/seth/refund.go Outdated Show resolved Hide resolved
integration-tests/scripts/check_base64_env_var.sh Outdated Show resolved Hide resolved
@skudasov skudasov self-requested a review February 28, 2024 16:18
@skudasov skudasov requested a review from lukaszcl February 28, 2024 16:20
@cl-sonarqube-production
Copy link

@skudasov skudasov added this pull request to the merge queue Feb 28, 2024
Merged via the queue into develop with commit fb8b399 Feb 28, 2024
97 checks passed
@skudasov skudasov deleted the tt_791_seth_ocr_tests branch February 28, 2024 18:09
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.

3 participants