Skip to content

Conversation

@Effi-S
Copy link
Contributor

@Effi-S Effi-S commented Aug 6, 2025

Type of change

  • Testing

Description

  1. envelope: nil checks + unit tests + minor refactor
  2. crypto: nil checka + unit tests

Related issues

@Effi-S Effi-S requested a review from liran-funaro August 7, 2025 08:49
@cendhu cendhu requested a review from Copilot August 7, 2025 10:32
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds comprehensive unit tests for two utility modules - envelope wrapper and crypto - while adding nil checks for improved error handling. The changes enhance test coverage for cryptographic key serialization/parsing functions and envelope wrapping/unwrapping operations.

  • Added nil input validation to crypto functions with corresponding error messages
  • Created comprehensive test suites covering both valid and invalid input scenarios
  • Refactored envelope wrapper to separate payload wrapping from full envelope wrapping

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File Description
utils/signature/sigtest/crypto_test.go New comprehensive test suite for crypto functions with edge cases
utils/signature/sigtest/crypto.go Added nil check for signing key serialization
utils/serialization/envelope_wrapper_test.go New test suite for envelope operations with error scenarios
utils/serialization/envelope_wrapper.go Added nil header validation and refactored envelope wrapping

Copy link
Contributor

@liran-funaro liran-funaro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some more comments

Effi-S and others added 13 commits November 3, 2025 16:15
Signed-off-by: Effi-S <[email protected]>
Co-authored-by: Copilot <[email protected]>
Signed-off-by: Effi-S <[email protected]>
Signed-off-by: Effi-S <[email protected]>
Co-authored-by: Copilot <[email protected]>
Signed-off-by: Effi-S <[email protected]>
Signed-off-by: Effi-S <[email protected]>
Signed-off-by: Effi-S <[email protected]>
#### Type of change

- Test update

#### Description

- [ServerConfig] Add retry mechanism when binding to a predefined port
(`c.Listener()`).
- [TestBroadcastDeliver] Use the above retry mechanism to ensure the
port is not used by other tests.

#### Related issues

- resolves hyperledger#109
- resolves hyperledger#137

---------

Signed-off-by: Liran Funaro <[email protected]>
Signed-off-by: Effi-S <[email protected]>
…o_modules group across 1 directory (hyperledger#168)

Bumps the go_modules group with 1 update in the / directory:
[github.com/consensys/gnark-crypto](https://github.com/consensys/gnark-crypto).

Updates `github.com/consensys/gnark-crypto` from 0.14.0 to 0.18.1

Signed-off-by: dependabot[bot] <[email protected]>
Signed-off-by: Liran Funaro <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Liran Funaro <[email protected]>
Signed-off-by: Effi-S <[email protected]>
@Effi-S Effi-S requested a review from liran-funaro November 5, 2025 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants