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

Fix: present-proof v2 send-proposal [issue#1474] #1667

Merged
merged 22 commits into from
Apr 5, 2022

Conversation

shaangill025
Copy link
Contributor

Signed-off-by: Shaanjot Gill shaangill025@users.noreply.github.com

resolve #1474

Signed-off-by: Shaanjot Gill <shaangill025@users.noreply.github.com>
@andrewwhitehead
Copy link
Contributor

andrewwhitehead commented Mar 14, 2022

Setting an argument default value to an object is generally not a good idea, because each call to the function will receive the same object. I think this method and create_pres should default to None and handle that appropriately.

Signed-off-by: Shaanjot Gill <shaangill025@users.noreply.github.com>
@shaangill025
Copy link
Contributor Author

Updated request_data dict to default to None for create_bound_request and create_pres methods.

@swcurran
Copy link
Contributor

Is the failing test related to this change, or something in the overall flow?

@andrewwhitehead
Copy link
Contributor

@swcurran The two errors appear to be due to the ursa_bbs_signatures library not being found when executing the integration tests?

@swcurran
Copy link
Contributor

So a configuration error in the integration suite, I gather? Who can look at this -- @shaangill025, I assume, or perhaps @ianco in the integration suite setup?

@andrewwhitehead
Copy link
Contributor

Actually it's stranger than that.. ursa_bbs_signatures is installed but it can't find the binary shared library that comes with it (at least I think it does). I don't think this would be affected by the addition of --platform in #1697 so maybe the GHA base image was updated?

@shaangill025
Copy link
Contributor Author

shaangill025 commented Mar 29, 2022

Yes, I think the integration tests have begun failing since PR #1697 merge about 2 hours back [includes merged #1676]. The last successful tests are with #1696 merge about 4 hours back.
Error:

File "/home/indy/aries_cloudagent/wallet/bbs.py", line 103, in create_bls12381g2_keypair
7hAries      |     key_pair = BlsKeyPair.generate_g2(seed)
7hAries      |   File "/home/indy/.pyenv/versions/3.6.13/lib/python3.6/site-packages/ursa_bbs_signatures/models/keys/BlsKeyPair.py", line 65, in generate_g2
7hAries      |     res = bls_generate_g2_key(seed)
7hAries      |   File "/home/indy/.pyenv/versions/3.6.13/lib/python3.6/site-packages/ursa_bbs_signatures/_ffi/bindings/bls.py", line 78, in bls_generate_g2_key
7hAries      |     POINTER(ExternError),
7hAries      |   File "/home/indy/.pyenv/versions/3.6.13/lib/python3.6/site-packages/ursa_bbs_signatures/_ffi/ffi_util.py", line 41, in wrap_native_func
7hAries      |     lib_func = getattr(get_library(), function_name)
7hAries      |   File "/home/indy/.pyenv/versions/3.6.13/lib/python3.6/site-packages/ursa_bbs_signatures/_ffi/ffi_util.py", line 56, in get_library
7hAries      |     LIB = _load_library("bbs")
7hAries      |   File "/home/indy/.pyenv/versions/3.6.13/lib/python3.6/site-packages/ursa_bbs_signatures/_ffi/ffi_util.py", line 84, in _load_library
7hAries      |     raise FfiException(f"Library not found in path: {lib_path}")
7hAries      | ursa_bbs_signatures._ffi.FfiException.FfiException: Library not found in path: None
7hAries      | 
7hAries      | The above exception was the direct cause of the following exception:
7hAries      | 
7hAries      | Traceback (most recent call last):
7hAries      |   File "/home/indy/aries_cloudagent/wallet/routes.py", line 364, in wallet_create_did
7hAries      |     info = await wallet.create_local_did(method=method, key_type=key_type)
7hAries      |   File "/home/indy/aries_cloudagent/wallet/indy.py", line 455, in create_local_did
7hAries      |     method, key_type, metadata, seed
7hAries      |   File "/home/indy/aries_cloudagent/wallet/indy.py", line 385, in __create_keypair_local_did
7hAries      |     public_key, secret_key = create_keypair(key_type, validate_seed(seed))
7hAries      |   File "/home/indy/aries_cloudagent/wallet/crypto.py", line 46, in create_keypair
7hAries      |     return create_bls12381g2_keypair(seed)
7hAries      |   File "/home/indy/aries_cloudagent/wallet/bbs.py", line 106, in create_bls12381g2_keypair
7hAries      |     raise BbsException("Unable to create keypair") from error
7hAries      | aries_cloudagent.wallet.bbs.BbsException: Unable to create keypair

Signed-off-by: Shaanjot Gill <shaangill025@users.noreply.github.com>
Signed-off-by: Shaanjot Gill <shaangill025@users.noreply.github.com>
@shaangill025
Copy link
Contributor Author

shaangill025 commented Mar 30, 2022

I ran integration tests locally on PR #1675 which was merged 8 days back [also #1696 merged 5 hours back] and I get the same error, looks like an issue with the ursa-bbs-signatures package itself.

@swcurran
Copy link
Contributor

Please keep pushing on this -- we need to get it resolved.

Signed-off-by: Shaanjot Gill <shaangill025@users.noreply.github.com>
@shaangill025
Copy link
Contributor Author

shaangill025 commented Mar 30, 2022

I have fixed the ursa_bbs_signatures._ffi.FfiException.FfiException: Library not found in path: None error . ursa-bbs-signatures 1.0.2 was released yesterday which is when tests started failing. As a temporary fix, I have fixed it to 1.0.1 which is working.

Signed-off-by: Shaanjot Gill <shaangill025@users.noreply.github.com>
@karimStekelenburg
Copy link
Contributor

I have just deleted the ursa-bbs-signatures 1.0.2 release from PyPi, so hopefully this resolved the issue.

@shaangill025
Copy link
Contributor Author

@andrewwhitehead No change was made since the last time you approved this. Locking ursa-bbs-signature change was reverted as 1.0.2 release was deleted from PyPi.

Copy link
Contributor

@ianco ianco left a comment

Choose a reason for hiding this comment

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

LGTM

@ianco ianco merged commit ef74cf9 into openwallet-foundation:main Apr 5, 2022
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.

/present-proof-2.0/send-proposal throws exception on the verifying side
5 participants