Skip to content
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

BLS-aggregation integration tests #91

Merged
merged 159 commits into from
Aug 30, 2024

Conversation

TomasArrachea
Copy link
Collaborator

Implemented integration tests for bls_aggregation crate.
Added 5 tests:

  • 2 quorums 1 operator (ignored)
  • 2 quorums 2 operators shared (ignored)
  • 1 quorum 1 operator
  • 1 quorum 2 operators
  • 2 quorums 2 operators separated

Two cases are ignored as they are failing due to this bug: bls aggregation service doesn't support >1 quorum

@TomasArrachea TomasArrachea marked this pull request as ready for review August 27, 2024 18:50
crates/crypto/bls/src/lib.rs Outdated Show resolved Hide resolved
crates/crypto/bls/src/lib.rs Outdated Show resolved Hide resolved
crates/crypto/bls/src/lib.rs Outdated Show resolved Hide resolved
crates/crypto/bls/src/lib.rs Outdated Show resolved Hide resolved
@supernovahs
Copy link
Collaborator

regarding the issue of adding separate conversion for different bindings.
Maybe we can reuse the original conversion functions, instead of adding separate for each bindings by unwrapping the X and Y values and wrapping in appropriate binding variant in the crate we need. (we can also use the above conversion functions in the lib.rs of those crates and use those their as helper also)

If you think above is a better way . Otherwise all good. some small nits

@TomasArrachea
Copy link
Collaborator Author

regarding the issue of adding separate conversion for different bindings. Maybe we can reuse the original conversion functions, instead of adding separate for each bindings by unwrapping the X and Y values and wrapping in appropriate binding variant in the crate we need. (we can also use the above conversion functions in the lib.rs of those crates and use those their as helper also)

If you think above is a better way . Otherwise all good. some small nits

@supernovahs I think it's better to merge the PR as is, and then make the refactor in a separate PR if needed.

@supernovahs
Copy link
Collaborator

needs a main merge

@supernovahs supernovahs merged commit 410b53a into Layr-Labs:main Aug 30, 2024
1 check passed
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.

4 participants