-
Notifications
You must be signed in to change notification settings - Fork 1
feature/packages refactor #32
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
Draft
tarakby
wants to merge
42
commits into
main
Choose a base branch
from
feature/packages-refactor
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Conversation
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
- Move SigningAlgorithm, Signature, PrivateKey, PublicKey to sign/sign.go - Add registry system for algorithm implementations in sign package - Update main sign.go to re-export types for backward compatibility - Delegate public API functions to sign package - Maintain all existing exported function/type names - First step in crypto package refactoring Co-Authored-By: [email protected] <[email protected]>
- Implement signatureFormatCheck for blsBLS12381Algo to satisfy signer interface - Method checks if signature has correct length (SignatureLenBLSBLS12381) - Fixes compilation error where BLS implementation was missing required method Co-Authored-By: [email protected] <[email protected]>
- Remove unused 'fmt' import (no longer needed after delegating to sign package) - Remove unused 'github.com/onflow/crypto/hash' import (moved to sign package) - Keep only necessary imports: crypto/elliptic, btcec/v2, and sign package - Fixes linting errors about unused imports Co-Authored-By: [email protected] <[email protected]>
- Remove newSigner function that became unused after delegating to sign package - All public API functions now delegate directly to sign package methods - Fixes linting error about unused function Co-Authored-By: [email protected] <[email protected]>
- Add signatureFormatCheck method to blsBLS12381Algo in no_cgo.go - Method panics with appropriate message for no-CGO builds - Fixes compilation error in CGO_ENABLED=0 test builds - Ensures both CGO and no-CGO builds implement complete signer interface Co-Authored-By: [email protected] <[email protected]>
- Remove public Signer interface from sign package (violated user's rule) - Remove RegisterSigner function (was not exported before) - Restore original newSigner function in main package - Update main package functions to use internal signer directly - Add missing fmt import for error formatting Co-Authored-By: [email protected] <[email protected]>
- Remove UnknownSigningAlgorithm, BLSBLS12381, ECDSAP256, ECDSASecp256k1 constants from main package - Update all references to use sign package constants (sign.ECDSAP256, etc.) - Fix test file to use sign.ECDSAP256 instead of ECDSAP256 - Maintain backward compatibility by keeping type aliases Co-Authored-By: [email protected] <[email protected]>
- Replace ECDSAP256 with sign.ECDSAP256 in ecdsa_test.go - Replace ECDSASecp256k1 with sign.ECDSASecp256k1 in ecdsa_test.go - Replace BLSBLS12381 with sign.BLSBLS12381 in sign_test_utils.go - Replace ECDSAP256 with sign.ECDSAP256 in sign_test_utils.go - Replace ECDSASecp256k1 with sign.ECDSASecp256k1 in sign_test_utils.go All test files now use the sign package constants instead of the removed crypto package constants. Co-Authored-By: [email protected] <[email protected]>
- Add import for github.com/onflow/crypto/sign to ecdsa_test.go - Add import for github.com/onflow/crypto/sign to sign_test_utils.go - Required for test files to use sign.ECDSAP256, sign.ECDSASecp256k1, sign.BLSBLS12381 constants Co-Authored-By: [email protected] <[email protected]>
- Fix function name from Testsign.BLSBLS12381Hasher to TestBLSBLS12381Hasher - Fix constant names: PrKeyLensign.BLSBLS12381 -> PrKeyLenBLSBLS12381 - Fix constant names: SignatureLensign.BLSBLS12381 -> SignatureLenBLSBLS12381 - Fix type names: pubKeysign.BLSBLS12381 -> pubKeyBLSBLS12381 - Update testEquals call to use sign.ECDSAP256 instead of ECDSAP256 Co-Authored-By: [email protected] <[email protected]>
- Fix double sign.sign. prefixes back to single sign. in ecdsa_test.go - Restore proper benchmark function names (BenchmarkECDSAP256Sign, etc.) - Fix remaining ECDSAP256 reference in BenchmarkECDSADecode function - Update invalidSK function in bls_test.go to use sign.ECDSAP256 Co-Authored-By: [email protected] <[email protected]>
- Add sign package import to bls.go, spock.go, and spock_test.go - Update all BLSBLS12381 references to use sign.BLSBLS12381 - Complete the removal of redundant constants from main crypto package Co-Authored-By: [email protected] <[email protected]>
- Fix undefined sign error in bls_thresholdsign_test.go line 291 - Complete the removal of redundant constants from main crypto package Co-Authored-By: [email protected] <[email protected]>
- Add sign package import to no_cgo_test.go - Update BLSBLS12381 references to use sign.BLSBLS12381 - Fix no-CGO build failure after removing constants from main package Co-Authored-By: [email protected] <[email protected]>
- Update signatureFormatCheck method to panic with specified message format - Address GitHub comment from @tarakby requesting panic behavior - Use fmt.Sprintf with a.algo field as requested Co-Authored-By: [email protected] <[email protected]>
…kage - Complete sign/sign.go implementation with proper algorithm registration - Replace root sign.go with backward compatibility re-exports - Maintain all existing public APIs through main crypto package - Algorithm instances initialized in main package and registered with sign package Co-Authored-By: Tarak Ben Youssef <[email protected]>
- Remove p256Instance and secp256k1Instance from ecdsa.go - Remove blsInstance from bls.go - These instances are now declared in sign.go and registered with sign package Co-Authored-By: Tarak Ben Youssef <[email protected]>
- Update all algorithm implementation methods to use sign.PrivateKey and sign.PublicKey - Update interface compliance assertions to use sign package types - Fix BLS and ECDSA key implementation method signatures - Still investigating interface conversion panic during initialization Co-Authored-By: Tarak Ben Youssef <[email protected]>
- Check return error from all sign.RegisterSigner calls - Panic if registration fails with non-nil error - Fixes errcheck linter warnings Co-Authored-By: Tarak Ben Youssef <[email protected]>
- Remove unused blsInstance variable from no_cgo.go - Resolves golangci-lint unused variable error in CI Co-Authored-By: Tarak Ben Youssef <[email protected]>
…ign-package Create sign package - First step of crypto refactoring
bls - ecdsa - bls12381 packages
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
#28
#33
#34