golangci-lint: enable containedctx #13171

jmank88 · 2024-05-10T00:42:48Z

https://smartcontract-it.atlassian.net/browse/BCF-3203

This PR enabled the containedctx rule and cleans up all outstanding cases.

containedctx is a linter that detects struct contained context.Context field. This is discouraged technique in favour of passing context as first argument of method or function. For rationale please read Contexts and structs the Go blog post.

github-actions · 2024-05-10T00:43:06Z

I see you updated files related to core. Please run pnpm changeset in the root directory to add a changeset as well as in the text include at least one of the following tags:

#added For any new functionality added.
#breaking_change For any functionality that requires manual action for the node to boot.
#bugfix For bug fixes.
#changed For any change to the existing functionality.
#db_update For any feature that introduces updates to database schema.
#deprecation_notice For any upcoming deprecation functionality.
#internal For changesets that need to be excluded from the final changelog.
#nops For any feature that is NOP facing and needs to be in the official Release Notes for the release.
#removed For any functionality/config that is removed.
#updated For any functionality that is updated.
#wip For any change that is not ready yet and external communication about it should be held off till it is feature complete.

core/services/pipeline/orm.go

jmank88 · 2024-05-10T12:06:43Z

core/services/ocr2/plugins/ocr2keeper/evmregistry/v20/registry.go

+	ctx, cancel := r.stopCh.NewCtx()
+	defer cancel()
+	if err := r.registerEvents(ctx, client.ID().Uint64(), addr); err != nil {


This seem problematic. Does anyone know why this is being done from the constructor instead of Start?

common/client/node_lifecycle.go

core/chains/evm/client/node_lifecycle.go

cl-sonarqube-production · 2024-05-21T23:13:44Z

Quality Gate passed

Issues
1 New issue
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
93.5% Coverage on New Code
1.7% Duplication on New Code

See analysis details on SonarQube

patrickhuie19 · 2024-05-22T15:40:40Z

core/services/pipeline/orm.go

 				o.lggr.Debugw("Pruning runs", "jobID", jobID, "count", val, "every", every, "maxSuccessfulRuns", o.maxSuccessfulRuns)
 				defer o.wg.Done()
-				ctx, cancel := context.WithTimeout(sqlutil.WithoutDefaultTimeout(o.ctx), time.Minute)
+				ctx, cancel := o.stopCh.CtxCancel(context.WithTimeout(sqlutil.WithoutDefaultTimeout(ctx), time.Minute))


time.Minute seems like something that should be configured :), out of scope

Do we want our sqlutil timeout and context timeouts to be different keys? Is that why we need both invocations here to end up with the right timeout that consumers will respect?

WithoutDefaultTimeout just tells the downstream WrappedDB Monitor not to apply the default timeout. Setting our own timeout is optional, and using one minute preserves the old behavior from LongQueryTimeout or whatever that was called. Normal timeouts are not communicated via key/vals, so these are not obviously combinable AFAICT. Although we could provider a helper for this kind of case where we do both 🤷

patrickhuie19 · 2024-05-22T15:48:54Z

core/services/blockheaderfeeder/delegate.go

 		go func() {
 			defer close(s.done)
+			ctx, cancel := s.stopCh.NewCtx()
+			defer cancel()
+			ticker := time.NewTicker(utils.WithJitter(s.pollPeriod))


patrickhuie19 · 2024-05-22T15:51:28Z

core/chains/evm/txmgr/evm_tx_store.go

-
-// Returns a context that contains the values of the provided context,
-// and which is canceled when either the provided context or TxStore parent context is canceled.
-func (o *evmTxStore) mergeContexts(ctx context.Context) (context.Context, context.CancelFunc) {


* Add special transmitter for OCR2 feeds * Add ocr2FeedsTransmitter FromAddress() * Cherry-pick some forwarders context changes from (#13171)

* Bump version and update CHANGELOG for core v2.12.0 Signed-off-by: Sneha Agnihotri <sneha.agnihotri@smartcontract.com> * core/services: fix ocrWrapper saveError contexts (#13139) (cherry picked from commit 15fab1d) * changed spammy error log to debug (#13153) (cherry picked from commit 3502176) * Updating changelog for 2.12.0 with cherry pick fix Signed-off-by: Sneha Agnihotri <sneha.agnihotri@smartcontract.com> * BCF-3225 - Implement forwarder fallback if forwarder not present as a transmitter on OCR2 aggregator (#13221) * Implement forwarder OCR2 fallback if fwd not present as a transmitter * Add changeset * Fix panic on mercury server error (#13231) (#13256) * Fix changelog * Fixed CPU usage issues caused by inefficiencies in HeadTracker (#13230) * Fixed CPU usage issues caused by inefficiencies in HeadTracker * added comments * revert heads back to the fix (cherry picked from commit 6f1ebca) * Update CHANGELOG for 2.12.0 bugfixes Signed-off-by: Sneha Agnihotri <sneha.agnihotri@smartcontract.com> * Decouple ChainType from config string [SHIP-2001] (#13272) * fix: Decouple ChainType from config string * fix: receiver name and failing test * test: enhance config test to test for xdai specifically * refactor: directly unmarshal into ChainType * fix: validation * test: fix TestDoc/EVM * test: add xdai to warnings.xtar * Index only the fifth word to reduce the db size overhead (#13315) Co-authored-by: Domino Valdano <domino.valdano@smartcontract.com> * Add special transmitter for OCR2 feeds (#13323) * Add special transmitter for OCR2 feeds * Add ocr2FeedsTransmitter FromAddress() * Cherry-pick some forwarders context changes from (#13171) * Improve ocr2FeedsTransmitter FromAddress() fallback (#13343) * Improve ocr2FeedsTransmitter FromAddress() fallback * Fix forwarders test error assert * Update 2.12.0 Changelog with bugfixes Signed-off-by: Sneha Agnihotri <sneha.agnihotri@smartcontract.com> * core/services/pipeline: bridge task must continue after cancellation * core/services/pipeline: use request context for deletion (#13404) * core/services/pipeline: hide deadline from monitor * Finalize date on changelog for 2.12.0 Signed-off-by: Sneha Agnihotri <sneha.agnihotri@smartcontract.com> * Fix merge conflicts * Remove `overtimeThresholds` * Return to old version of pipeline-runner --------- Signed-off-by: Sneha Agnihotri <sneha.agnihotri@smartcontract.com> Co-authored-by: Jordan Krage <jmank88@gmail.com> Co-authored-by: Patrick <patrick.huie@smartcontract.com> Co-authored-by: ilija42 <57732589+ilija42@users.noreply.github.com> Co-authored-by: Sam <samsondav@protonmail.com> Co-authored-by: Dmytro Haidashenko <34754799+dhaidashenko@users.noreply.github.com> Co-authored-by: Friedemann Fürst <friedemann.fuerst@smartcontract.com> Co-authored-by: Mateusz Sekara <mateusz.sekara@smartcontract.com> Co-authored-by: Domino Valdano <domino.valdano@smartcontract.com> Co-authored-by: ilija <pavlovicilija42@gmail.com> Co-authored-by: frank zhu <fr@nkzhu.com> Co-authored-by: george-dorin <george.dorin@smartcontract.com>

jmank88 temporarily deployed to sdlc May 10, 2024 00:42 — with GitHub Actions Inactive

jmank88 commented May 10, 2024

View reviewed changes

core/services/pipeline/orm.go Outdated Show resolved Hide resolved

jmank88 requested review from krehermann and patrickhuie19 May 10, 2024 00:46

jmank88 force-pushed the BCF-3203-lint-containedctx branch from 742516c to 6be0d13 Compare May 10, 2024 00:51

jmank88 temporarily deployed to sdlc May 10, 2024 00:51 — with GitHub Actions Inactive

jmank88 force-pushed the BCF-3203-lint-containedctx branch from 6be0d13 to 68d65bc Compare May 10, 2024 00:56

jmank88 temporarily deployed to sdlc May 10, 2024 00:56 — with GitHub Actions Inactive

jmank88 force-pushed the BCF-3203-lint-containedctx branch from 68d65bc to c620551 Compare May 10, 2024 01:01

jmank88 temporarily deployed to sdlc May 10, 2024 01:01 — with GitHub Actions Inactive

jmank88 force-pushed the BCF-3203-lint-containedctx branch from c620551 to 0c504c8 Compare May 10, 2024 02:55

jmank88 temporarily deployed to sdlc May 10, 2024 02:55 — with GitHub Actions Inactive

jmank88 force-pushed the BCF-3203-lint-containedctx branch from 0c504c8 to c58c3fb Compare May 10, 2024 03:04

jmank88 temporarily deployed to sdlc May 10, 2024 03:04 — with GitHub Actions Inactive

jmank88 force-pushed the BCF-3203-lint-containedctx branch from c58c3fb to 84dc378 Compare May 10, 2024 11:31

jmank88 temporarily deployed to sdlc May 10, 2024 11:31 — with GitHub Actions Inactive

jmank88 force-pushed the BCF-3203-lint-containedctx branch from 84dc378 to beadb40 Compare May 10, 2024 12:02

jmank88 temporarily deployed to sdlc May 10, 2024 12:02 — with GitHub Actions Inactive

jmank88 force-pushed the BCF-3203-lint-containedctx branch from beadb40 to 5ab6368 Compare May 10, 2024 12:04

jmank88 temporarily deployed to sdlc May 10, 2024 12:04 — with GitHub Actions Inactive

jmank88 commented May 10, 2024

View reviewed changes

jmank88 force-pushed the BCF-3203-lint-containedctx branch from 5ab6368 to de36108 Compare May 10, 2024 12:14

jmank88 temporarily deployed to sdlc May 10, 2024 12:14 — with GitHub Actions Inactive

jmank88 force-pushed the BCF-3203-lint-containedctx branch from de36108 to 2876d8e Compare May 10, 2024 22:26

jmank88 temporarily deployed to sdlc May 10, 2024 22:26 — with GitHub Actions Inactive

jmank88 force-pushed the BCF-3203-lint-containedctx branch from 2876d8e to bc4e1bb Compare May 13, 2024 19:30

jmank88 requested a deployment to sdlc May 13, 2024 19:30 — with GitHub Actions Abandoned

jmank88 force-pushed the BCF-3203-lint-containedctx branch from bc4e1bb to 8c0fc29 Compare May 13, 2024 21:14

jmank88 temporarily deployed to sdlc May 13, 2024 21:14 — with GitHub Actions Inactive

jmank88 requested a review from a team May 15, 2024 20:13

patrickhuie19 requested a review from dhaidashenko May 17, 2024 00:11

dhaidashenko reviewed May 20, 2024

View reviewed changes

common/client/node_lifecycle.go Outdated Show resolved Hide resolved

common/client/node_lifecycle.go Outdated Show resolved Hide resolved

core/chains/evm/client/node_lifecycle.go Outdated Show resolved Hide resolved

jmank88 dismissed samsondav’s stale review via f42cb79 May 21, 2024 14:16

jmank88 force-pushed the BCF-3203-lint-containedctx branch from 963db46 to f42cb79 Compare May 21, 2024 14:16

jmank88 temporarily deployed to sdlc May 21, 2024 14:16 — with GitHub Actions Inactive

jmank88 force-pushed the BCF-3203-lint-containedctx branch from f42cb79 to 0462c15 Compare May 21, 2024 14:27

jmank88 temporarily deployed to sdlc May 21, 2024 14:27 — with GitHub Actions Inactive

jmank88 force-pushed the BCF-3203-lint-containedctx branch from 0462c15 to 20ccce5 Compare May 21, 2024 14:34

jmank88 temporarily deployed to sdlc May 21, 2024 14:34 — with GitHub Actions Inactive

jmank88 requested review from dhaidashenko and samsondav May 21, 2024 14:54

golangci-lint: enable containedctx

57cbe00

jmank88 force-pushed the BCF-3203-lint-containedctx branch from 20ccce5 to 57cbe00 Compare May 21, 2024 22:57

jmank88 temporarily deployed to sdlc May 21, 2024 22:57 — with GitHub Actions Inactive

patrickhuie19 reviewed May 22, 2024

View reviewed changes

patrickhuie19 approved these changes May 22, 2024

View reviewed changes

jmank88 added this pull request to the merge queue May 22, 2024

dhaidashenko approved these changes May 22, 2024

View reviewed changes

Merged via the queue into develop with commit 677abe1 May 22, 2024
106 of 107 checks passed

jmank88 deleted the BCF-3203-lint-containedctx branch May 22, 2024 17:28

ilija42 added a commit that referenced this pull request May 28, 2024

Add special transmitter for OCR2 feeds (#13323)

8585cc9

* Add special transmitter for OCR2 feeds * Add ocr2FeedsTransmitter FromAddress() * Cherry-pick some forwarders context changes from (#13171)

ilija42 mentioned this pull request May 28, 2024

BCF-3243 - Cherry pick Add special transmitter for OCR2 feeds (#13323) #13341

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

golangci-lint: enable containedctx #13171

golangci-lint: enable containedctx #13171

jmank88 commented May 10, 2024 •

edited

Loading

github-actions bot commented May 10, 2024

jmank88 May 10, 2024

patrickhuie19 May 22, 2024

cl-sonarqube-production bot commented May 21, 2024

patrickhuie19 May 22, 2024

jmank88 May 22, 2024

patrickhuie19 May 22, 2024

patrickhuie19 May 22, 2024

golangci-lint: enable containedctx #13171

golangci-lint: enable containedctx #13171

Conversation

jmank88 commented May 10, 2024 • edited Loading

github-actions bot commented May 10, 2024

jmank88 May 10, 2024

Choose a reason for hiding this comment

patrickhuie19 May 22, 2024

Choose a reason for hiding this comment

cl-sonarqube-production bot commented May 21, 2024

Quality Gate passed

patrickhuie19 May 22, 2024

Choose a reason for hiding this comment

jmank88 May 22, 2024

Choose a reason for hiding this comment

patrickhuie19 May 22, 2024

Choose a reason for hiding this comment

patrickhuie19 May 22, 2024

Choose a reason for hiding this comment

jmank88 commented May 10, 2024 •

edited

Loading