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

Integration tests creating existing DID's #2855

Closed
jamshale opened this issue Mar 22, 2024 · 5 comments · Fixed by #2862
Closed

Integration tests creating existing DID's #2855

jamshale opened this issue Mar 22, 2024 · 5 comments · Fixed by #2862
Assignees
Labels
Discuss Issues to be raised for discussion at an ACA-Pug Meeting

Comments

@jamshale
Copy link
Contributor

The integration tests are recreating the same did's they previously created causing fails because the schema/cred_def's they are trying to create already exist on the ledger. Should be able to make the seed/did creation more random to prevent this from happening.

@swcurran swcurran added the Discuss Issues to be raised for discussion at an ACA-Pug Meeting label Apr 2, 2024
@PatStLouis
Copy link
Contributor

have we found in the code where the seed is generated? I was thinking in here but I couldn't see it

@ianco
Copy link
Member

ianco commented Apr 3, 2024

The aca-py seed is created here: https://github.com/hyperledger/aries-cloudagent-python/blob/main/demo/runners/support/agent.py#L211

It's kind of hokey and not really robust enough for running integration tests, where we may create hundreds of DID's in a single run of the tests. (It was initially implemented for a simple demo but should be updated ...)

@PatStLouis
Copy link
Contributor

I can suggest to switch to python secrets.token_urlsafe to create a random seed with large entropy instead of randint.

@ianco
Copy link
Member

ianco commented Apr 4, 2024

I can suggest to switch to python secrets.token_urlsafe to create a random seed with large entropy instead of randint.

I'll take a look, thanks for the suggestion.

@PatStLouis
Copy link
Contributor

@ianco looks like @dbluhm is addressing this in another pr with a similar approach, using token_hex instead. Less entropy but good enough!

@ianco ianco assigned dbluhm and unassigned ianco Apr 4, 2024
@dbluhm dbluhm linked a pull request Apr 8, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discuss Issues to be raised for discussion at an ACA-Pug Meeting
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants