Repository Modernization Plan - Phase 4B: CI: Harden CI Pipeline#79
Open
Soumya8898 wants to merge 9 commits into
Open
Repository Modernization Plan - Phase 4B: CI: Harden CI Pipeline#79Soumya8898 wants to merge 9 commits into
Soumya8898 wants to merge 9 commits into
Conversation
added 9 commits
June 7, 2026 22:53
The weak-bb (Weak Boneh-Boyen) package was only referenced in
unreachable code paths. Both aries/revocation.go and
dlog/crypto/revocation_authority.go called WbbKeyGen in an else
branch that always returned an error ('the specified revocation
algorithm is not supported'). The only supported algorithm is
AlgNoRevocation, which never uses weak-bb.
Changes:
- Delete bccsp/schemes/weak-bb/ entirely
- Remove dead else branch from aries RevocationAuthority.Sign()
- Remove dead else branch from dlog createCRI()
- Remove weak-bb test section from dlog idemix_test.go
Signed-off-by: Soumya Mohapatra <mohapatras@microsoft.com>
Replace the dlog Translator interface (G1ToProto/G1FromProto) with direct G1.Bytes()/curve.NewG1FromBytes() calls. The Translator was a trivial indirection — Gurvy's G1ToProto just splits G1.Bytes() into X and Y halves, and G1FromProto concatenates them back. - handlers/nym.go: NymSecretKey, nymPublicKey, NymKeyDerivation, NymPublicKeyImporter, NymKeyImporter now use *math.Curve instead of idemix.Translator - keystore/kvsbased.go: Store nym PK as raw []byte instead of *amcl.ECP proto; remove dlog/amcl imports Signed-off-by: Soumya Mohapatra <mohapatras@microsoft.com>
- bccsp/bccsp.go: Delete the legacy New() factory function entirely. Simplify NewAries() to no longer take a Translator parameter. - msp/idemixmsp.go: NewIdemixMsp() now uses Aries (BBS+) directly instead of the legacy dlog scheme. NewIdemixMspAries() unchanged. Remove amcl import. - tools/idemixgen/main.go: Remove all legacy curve options, the --aries flag (always Aries now), the Translator interface, and the dlog Idemix struct usage. Aries-only with BLS12_381_BBS. - tools/idemixgen/idemixca/idemixca.go: Delete (legacy-only). The Aries variant (iedmixca_aries.go) remains. Signed-off-by: Soumya Mohapatra <mohapatras@microsoft.com>
Delete the entire bccsp/schemes/dlog/ directory: - bridge/ (adapter from handlers to dlog crypto) - crypto/ (core CL-signature implementation + protobuf) - crypto/translator/amcl/ (point serialization + proto types) Delete legacy integration tests: - bccsp/bccsp_test.go (tested the legacy New() factory) - bccsp/legacy_test.go (legacy-specific test scenarios) The only remaining crypto scheme is Aries (BBS+). Signed-off-by: Soumya Mohapatra <mohapatras@microsoft.com>
- Add bccsp/helpers_test.go with NewDummyKeyStore() helper (was previously defined in the deleted bccsp_test.go) - Update aries_test.go: remove amcl/translator import and usage, update NewAries() calls to new signature (no translator param) Signed-off-by: Soumya Mohapatra <mohapatras@microsoft.com>
- nymsigner_test.go: replace amcl.Fp256bn with math.Curves, update NewNymSecretKey/NewNymPublicKey calls to new signatures - signer_test.go: same pattern — remove amcl import, use BLS12_381_BBS - user_test.go: replace Translator field with Curve field, remove amcl Signed-off-by: Soumya Mohapatra <mohapatras@microsoft.com>
- kvsbased_test.go: use BLS12_381_BBS curve, remove Translator field, fix duplicate Curve field, update NewNymSecretKey call - perf_test.go: remove amcl import, replace idemix.New with NewAries - smartcard_test.go: use BLS12_381_BBS curve, fix NewNymPublicKey and NewNymSecretKey calls to new signatures - msp/idemixmsp_test.go: replace dlog issuer key generation with Aries issuer for the bad-IPK test case - idemixca_test.go: remove legacy TestIdemixCa test, remove dlog imports Signed-off-by: Soumya Mohapatra <mohapatras@microsoft.com>
- Delete msp/audit_legacy_test.go (used legacy testdata/idemix/) - Delete msp/idemixmsp_test.go (used legacy testdata/idemix/) - Delete msp/testdata/idemix/ (legacy scheme fixtures) - Add msp/helpers_test.go (setupWithTypeAndVersion, getDefaultSigner) - Delete bccsp/smartcard_test.go (used legacy FP256BN testdata) - Delete bccsp/testdata/ (legacy smartcard fixtures) - Fix user_test.go: update expected Bytes() to match BLS12_381_BBS The Aries-specific tests (idemixmsp_aries_test.go, audit_aries_test.go, schemes/aries/smartcard_test.go) provide full coverage of these paths. Signed-off-by: Soumya Mohapatra <mohapatras@microsoft.com>
…unt=1 - Remove 'Legacy guard' step from CI (dlog/weak-bb fully removed) - Add golangci-lint step with curated ruleset: errcheck, gosimple, govet, ineffassign, staticcheck, unused - Add .golangci.yml config: suppress SA1019 (deprecated usage tracked for Phase 7), exclude mock and flogging dirs, exclude errcheck in tests and AddWrapper calls - Add -count=1 to unit-tests and unit-tests-race Makefile targets - Fix lint issues: remove unused logger var (impl.go), fix ineffectual assignment (smartcard.go), register testAries with Ginkgo suite (aries_test.go), add unreachable return for staticcheck (main.go) Signed-off-by: Soumya Mohapatra <mohapatras@microsoft.com>
Soumya8898
force-pushed
the
Soumya8898/phase4/ci-hardening
branch
from
5c42daa to
ab0ce63
Compare
Contributor
Author
|
Hi @adecaro there are still places where we need to take care of this golangci_lint errors, these are mechanical find-and-replace changes, but they touch many files across the entire codebase (not just files I modified) and require careful testing since they change serialization behaviour. Mixing them into a CI-hardening PR would make it harder to review and riskier. I will form a clean, self-contained modernization phase (may be in 6 or 7) that can be done in isolation with its own test verification. Thanks 🙏 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CI: Harden CI Pipeline
What
golangci-lintwith a curated linter set-count=1on all test targets to prevent cached resultsWhy
With dlog and weak-bb fully removed in the previous PR, the legacy-guard grep check serves no purpose — there's nothing left to guard against. Meanwhile, the CI had no static analysis beyond
go vet, and test caching could silently mask regressions when source files change in ways the Go test cache doesn't detect.Changes
Removed: Legacy guard step (
.github/workflows/go.yml)This step grepped for
schemes/dlogandschemes/weak-bbimports and failed if new ones appeared. Since both packages are deleted, the grep would never match anything. Removed entirely.Added: golangci-lint step (
.github/workflows/go.yml)Uses the official
golangci/golangci-lint-action@v6with a 5-minute timeout. Runs on every push and PR tomain.Added:
.golangci.ymlconfigEnables five linters that catch real bugs without excessive noise:
go vetbut integrated into the lint runMock directories are excluded since generated code doesn't need linting.
Changed: Makefile test targets
Added
-count=1to bothunit-testsandunit-tests-raceto disable the Go test cache. Every CI run now executes tests fresh regardless of prior runs.