hyperledger / hyperledger/fabric-x-sdk

Test coverage for the identity package

Open
#52 0 comments 0 reactions 0 assignees View on GitHub
good-first-issue
Dominant language
Go
Stars
3
Forks
5
Avg merge
2d 11h
Merged PRs (30d)
9

Description

## Context

`identity` is the only package in the SDK that no test reaches. Several packages have no test
file of their own — `endorsement`, `network/fabric`, `state/sqlite` — but a cross-package profile
(`go test ./... -short -coverpkg=./...`) shows each of them exercised indirectly, at 67-80% of
functions. `identity` is at 0%. It is also the package every real deployment depends on first: `identity.SignerFromMSP` is what turns an MSP
folder on disk into the `sdk.Signer` that the `Submitter`, `Synchronizer` and every
`EndorsementBuilder` sign with. A failure here is the difference between a client that starts and
one that does not.

The integration tests do exercise it, but only on the happy path and only where crypto material
happens to exist: `TestFablo` and `TestFabricXCommitter` call `SignerFromMSP`, and both are skipped
in `-short` mode, so CI's unit-test job never touches the package. `TestFabric`/`TestFabricX` use
`integration.testSigner` (a fixed-bytes stub) instead.

There is more behaviour in these 107 lines than the coverage number suggests:

- `SignerFromMSP` accepts two keystore naming conventions — `keystore/*_sk` (cryptogen) and, as a
fallback, `keystore/*.pem` (fabric-ca). The fallback is reached only when the first glob returns
nothing, and nothing verifies it works.
- The error path in that fallback is subtle: `filepath.Glob` returns `nil` error for "no match", so
the `err != nil || len(keyFiles) == 0` conditions carry the logic. The final error message wraps
`err`, which is normally nil at that point.
- `parsePrivateKey` accepts PKCS#8 only. A PKCS#1 or SEC1 key — which `openssl` produces by default
— fails with `parse pkcs8 private key`, and a non-ECDSA PKCS#8 key with `not an ECDSA private key`.
- `Sign` applies low-S normalisation via `utils.ToLowS`. Fabric rejects high-S signatures, so a
regression here produces signatures that every peer refuses, with no local symptom.
- `Serialize` marshals `msp.SerializedIdentity`, whose wire compatibility with
`fabric-x-common`'s `msppb.Identity` the Fabric-X packager depends on
(`network/fabricx/packager.go` unmarshals `Endorsement.Endorser` as `msppb.Identity`).

## Approach

New `identity/msp_test.go`, self-contained: generate an ECDSA key and a self-signed certificate in
the test with `crypto/ecdsa` + `crypto/x509`, write them into a temporary MSP layout under
`t.TempDir()`, and load it. No fixtures to check in and no dependency on `make init-x`.

Cases worth covering:

1. **Happy path, `*_sk` keystore** — `SignerFromMSP` succeeds; `Serialize` round-trips through
`proto.Unmarshal` into a `msp.SerializedIdentity` with the expected MspId and the signcert PEM.
2. **Happy path, `*.pem` keystore** — the fabric-ca fallback resolves the key.
3. **Signature verification** — `Sign` output parses as a DER ECDSA signature that verifies against
the certificate's public key over the SHA-256 digest of the message, and `s` is in the lower
half of the curve order (the `ToLowS` guarantee).
4. **Wire compatibility** — `Serialize` output unmarshals as `msppb.Identity` with a matching
MspId, pinning the assumption `network/fabricx/packager.go` relies on.
5. **Error paths** — missing directory, empty `keystore`, missing `signcerts`, a PKCS#1 key, and a
PKCS#8 RSA key. Assert on the error, not on a panic.

## Validation

`go test ./identity/ -race` passes, and `go tool cover -func` shows the package covered. No new
dependencies: `crypto/*` and `google.golang.org/protobuf` are already in the module.

Contributor guide

No contributing guide indexed for this repository

Research direction

Add the self-contained tests in identity/msp_test.go, using the existing identity package and the wire-compatibility consumer in network/fabricx/packager.go as entry points. Start by running go test ./identity/ -race and inspect the SignerFromMSP, parsePrivateKey, Sign, and Serialize paths. Done means the listed keystore, signing, serialization, compatibility, and error cases pass and go tool cover -func shows identity is exercised.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
security, testing
Issue type
Feature
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.