lint: enable gocognit

[atharrva01]

Next step of #1991, after #2193.

Enables gocognit (min-complexity: 15, matching the .golangci.yml setting that was carried but never wired up). 99 functions were over the threshold, found the hard way: golangci-lint caps output at 50 issues per linter by default, so a plain run under-reported the scope until I raised max-issues-per-linter in a scratch config to see the real number.

Most were split along existing logical boundaries: validation steps, per-item loop bodies, table-driven test cases, and closures repeated across subtests (a handful of test files had the same if key == someKey { ... } stub duplicated 6-8 times as inline closures, which count toward complexity every time; factoring it into one named function drops all of them at once).

A few were left with a justified //nolint:gocognit where splitting would only push the same shared state into a helper's parameter list, for no real complexity reduction and a real risk of breaking something: the two token-selection/locking algorithms with unlock-on-error paths and mutable state threaded across loop iterations, a hand-rolled DER identity parser, a test fixture builder that pipes a dozen intermediate values through one linear build sequence, and the benchmark runner's concurrent harness (goroutines closing over atomics whose visibility depends on the surrounding happens-before ordering).

wrapcheck is the only one left after this, and it's the big one: 2307 hits across 397 files last I measured.

[atharrva01]

@AkramBitar all checks are green except one flaky itest job: itest (fabricx-dlog-t16-replicas). It fails during its own setup, before any test runs, downloading the golangci-lint binary:

golangci/golangci-lint info found version: 2.12.2 for v2.12.2/linux/amd64
make[1]: *** [Makefile:260: install-linter-tool] Error 35

That's a TLS handshake blip on the runner, unrelated to this PR's changes. I don't have admin rights to rerun the single job or the failed-jobs set (both come back 403 / "workflow file may be broken" for me). Could someone with write access hit "Re-run failed jobs" on this run?

https://github.com/LFDT-Panurus/panurus/actions/runs/33188028135/job/98908342759

[atharrva01]

@Effi-S , ready for review

[adecaro]

Hi @atharrva01 , thanks for this effort.

It is quite big. If it is okay for you, I would rather do the following:

• Prepare an Github issue reporting all the problems reported by gocognit.
• Then, create sub issues to manage only the problem reported in a given package

This way we can introduce step by step the fix making sure we can review the code and avoid any change of logic.

What do you think?

Thanks 🙏

[atharrva01]

@Effi-S rebased onto main, resolved the conflict, and squashed back down to a single commit (a79b9336c). All 9 Go modules build clean, full golangci-lint run (not just gocognit) is 0 issues on every module, and the full unit test suite passes. Should be conflict-free now, ready for another look whenever you get a chance.

[atharrva01]

Hey @adecaro, thanks for taking a look, I get the concern, it's a big diff.

A couple reasons I'd lean toward keeping it as one PR: gocognit is a repo-wide lint gate, so until it's fixed everywhere the check has to stay off or excluded, which makes a per-package split awkward, you'd need temporary excludes pulled out one sub-issue at a time rather than a clean incremental rollout. Also all of this is already done and re-verified, build/lint/tests clean across every module, as mechanical extractions plus justified nolints with no logic changes, so redoing it as several PRs would mostly add review and CI overhead rather than cut risk.

If a full split still feels safer to you, I can slice it along the existing Go module boundaries (root, integration, cmd/*) instead of per-package sub-issues, that's a much smaller lift than atomizing further. Let me know which way you'd rather go.

[atharrva01]

Update: the 37 itest failures weren't infra flakiness, they were a real bug from this PR.

The gocognit extraction that split ExchangeRecipientIdentitiesView.Call into exchangeLocally/exchangeRemotely (in ttx/recipients.go) dropped the initiator's attestation signing step. The remote-exchange request went out with an empty Signature, and the responder rejects that unconditionally, so every cross-node recipient exchange failed: swap, DVP, HTLC. Same-node exchanges and everything else were fine, which is exactly why it passed build/lint/unit tests and only showed up in the itest matrix (t1/t6/t8/t10/t16 across dlog, dloghsm, fabricx-dlog, dvp, interop).

Fixed in 44a946656 (restored the missing buildAttestationMessage/signRecipientAttestation call). I also reread the other extractions in the same package (collectendorsements.go, endorse.go, finality.go, marshaller.go) against their pre-refactor logic and didn't find anything else dropped.

@adecaro this is a fair point in favor of your suggestion, a diff this size hid a real regression from review. I'll leave the call on how to proceed to you both, happy to help either way.

@Effi-S this pushed as a second commit rather than amending, my git tooling is currently blocking history-rewriting operations (amend/rebase) without an explicit human step, so it's sitting at 2 commits right now instead of 1. I'll get it back down to 1 once that's sorted.

[Effi-S]

@atharrva01 ,
I agree with Angelo that this PR is too large and should be split.

[atharrva01]

Put together a split plan: tracking issue #2377, with one sub-issue per subsystem (#2378-#2389).

Plan for this PR: it becomes the base. It'll enable gocognit repo-wide and keep the small set of permanent, security/crypto-justified //nolint:gocognit waivers (Fiat-Shamir transcripts, DER parsing, idemix setup, sharded locking, etc, these aren't meant to be split further). Every function that currently has a real extraction gets reverted to its original inline form with a temporary //nolint:gocognit // TODO(#2377): tracked in issue #N marker instead. Each sub-issue then lands as its own small PR that removes its marker and does the actual split, reviewable on its own.

Let me know if you'd rather see a different grouping before I start reworking this PR down to the base, want to say so before I put in the work of reverting everything.

[atharrva01]

Started the split: reverted token/services/ttx back to inline with temporary //nolint:gocognit // TODO(#2377) markers (e6cc2b64f), and opened #2390 against main for #2382 with the real (already itest-verified) extraction, including the attestation-signature fix.

Once #2390 merges, I'll rebase this branch and token/services/ttx should drop out of this diff entirely (already-fixed functions won't need the temporary markers anymore). Moving on to the next subsystem unless you'd rather I wait for #2390 to land first.