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

Merge did:indy support #166

Merged
merged 93 commits into from
Aug 8, 2023
Merged

Merge did:indy support #166

merged 93 commits into from
Aug 8, 2023

Conversation

andrewwhitehead
Copy link
Member

Currently, tests are failing with the JS wrapper due to a timeout in GetAttribRequest. I think this might be a compatibility issue with the did:indy-supporting indy-node version, but I'm not certain.

berendsliedrecht and others added 4 commits January 3, 2023 14:51
Signed-off-by: blu3beri <blu3beri@proton.me>
Signed-off-by: blu3beri <blu3beri@proton.me>
Work funded by the Government of Ontario.

Signed-off-by: blu3beri <blu3beri@proton.me>
@swcurran
Copy link
Member

swcurran commented Mar 1, 2023

Who can/should look at this? @blu3beri — is that something you can look at?

@andrewwhitehead — any details that would help — any details on how to reproduce, what is happening/where?

@andrewwhitehead
Copy link
Member Author

There are more details in the CI logs, specifically

FAIL tests/GetAttribRequest.test.ts (120.218 s)
  GetAttribRequest
    ✕ Submit request (120001 ms)

  ● GetAttribRequest › Submit request

    thrown: "Exceeded timeout of 120000 ms for a test.
    Use jest.setTimeout(newTimeout) to increase the timeout value, if this is a long-running test."

      10 |   beforeAll(() => (pool = setupPool()))
      11 |
    > 12 |   test('Submit request', async () => {
         |   ^
      13 |     const request = new GetAttribRequest({ targetDid: DID, raw: 'endpoint' })
      14 |     const response: GetAttribResponse = await pool.submitRequest(request)
      15 |

      at tests/GetAttribRequest.test.ts:12:3
      at Object.<anonymous> (tests/GetAttribRequest.test.ts:7:1)

But there also seems to be a segfault after the tests complete (could be related to the Python test shutdown issue where it needs to stop the tokio threads first, or could be the additional parameters to the indy_vdr_build_nym_request FFI method).

@c2bo
Copy link
Member

c2bo commented Mar 1, 2023

This still has a custom old build of indy-node in it (from here: https://github.com/domwoe/indy-node/tree/feature/did-indy-new). Should we test #112 on this branch and figure out if this fixes things?

@c2bo
Copy link
Member

c2bo commented Mar 1, 2023

Hmm, I tried testing this locally with the current and updated docker image and it seems to have no impact (both times the GetAttribRequest timeouts). Interesting is that I see another test failing:

 FAIL  tests/GetNymRequest.test.ts
  ● GetNymRequest › Submit request

    Invalid pointer for result value

      14 |   const indyVdrErrorObject = JSON.parse(nativeError.deref() as string) as IndyVdrErrorObject
      15 |
    > 16 |   throw new IndyVdrError(indyVdrErrorObject)
         |         ^
      17 | }
      18 |

      at handleError (src/error.ts:16:9)
      at NodeJSIndyVdr.buildGetNymRequest (src/NodeJSIndyVdr.ts:267:5)
      at new buildGetNymRequest (../indy-vdr-shared/src/builder/GetNymRequest.ts:28:28)
      at _callee$ (tests/GetNymRequest.test.ts:13:21)
      at tryCatch (../node_modules/regenerator-runtime/runtime.js:63:40)
      at Generator.invoke [as _invoke] (../node_modules/regenerator-runtime/runtime.js:294:22)
      at Generator.next (../node_modules/regenerator-runtime/runtime.js:119:21)
      at tryCatch (../node_modules/regenerator-runtime/runtime.js:63:40)
      at invoke (../node_modules/regenerator-runtime/runtime.js:155:20)
      at ../node_modules/regenerator-runtime/runtime.js:190:11

@c2bo
Copy link
Member

c2bo commented Mar 1, 2023

Nevermind - the typescript tests are running with the sovrin buildernet instead of the local pool setup?
The libindy_vdr header (in libindy_vdr/include) file seems to be the old one before the changes - shouldn't this always create problems with a changed interface (for attrib and nym that added timestamp/version to the calls)?

#167 Should be a quick fix to get things running again (only fixes the parameters for the ffi calls - does not add the functionality properly to the javascript library).

@andrewwhitehead
Copy link
Member Author

@c2bo Not sure if the separate did-indy-pool.dockerfile is still needed or we can just use indy-node-container 1.13.2-rc5 for all tests?

@c2bo
Copy link
Member

c2bo commented Mar 2, 2023

@c2bo Not sure if the separate did-indy-pool.dockerfile is still needed or we can just use indy-node-container 1.13.2-rc5 for all tests?

We don't need the separate dockerifle anymore as far as i know -> That would be fixed by #112. I will re-base to this branch and create a new PR

@TimoGlastra
Copy link
Member

We'll try to update the js wrapper to use the local pool instead!

@c2bo
Copy link
Member

c2bo commented Mar 2, 2023

I added a small bash script in #169 that we can use for the different tests that want to use a local node pool. I just realized the same is true for the python wrapper - that one currently uses the idunion test network for its tests

./test.sh up
./test.sh down

domwoe added 14 commits July 19, 2023 10:51
Signed-off-by: Dominic Wörner <dom.woe@gmail.com>
Signed-off-by: Dominic Wörner <dom.woe@gmail.com>
Signed-off-by: Dominic Wörner <dom.woe@gmail.com>
Signed-off-by: Dominic Wörner <dom.woe@gmail.com>
Signed-off-by: Dominic Wörner <dom.woe@gmail.com>
Signed-off-by: Dominic Wörner <dom.woe@gmail.com>
Signed-off-by: Dominic Wörner <dom.woe@gmail.com>
Signed-off-by: Dominic Wörner <dom.woe@gmail.com>
Signed-off-by: Dominic Wörner <dom.woe@gmail.com>
Signed-off-by: Dominic Wörner <dom.woe@gmail.com>
Signed-off-by: Dominic Wörner <dom.woe@gmail.com>
Signed-off-by: Dominic Wörner <dom.woe@gmail.com>
Signed-off-by: Dominic Wörner <dom.woe@gmail.com>
Signed-off-by: Dominic Wörner <dom.woe@gmail.com>
c2bo and others added 6 commits July 19, 2023 10:58
… builds

Signed-off-by: Christian Bormann <ChristianCarl.Bormann@de.bosch.com>
Signed-off-by: Christian Bormann <ChristianCarl.Bormann@de.bosch.com>
Signed-off-by: Christian Bormann <ChristianCarl.Bormann@de.bosch.com>
Signed-off-by: Andrew Whitehead <cywolf@gmail.com>
Signed-off-by: Christian Bormann <ChristianCarl.Bormann@de.bosch.com>
Signed-off-by: Andrew Whitehead <cywolf@gmail.com>
Signed-off-by: Andrew Whitehead <cywolf@gmail.com>
Signed-off-by: Andrew Whitehead <cywolf@gmail.com>
@andrewwhitehead andrewwhitehead marked this pull request as ready for review July 19, 2023 19:45
Copy link
Contributor

@berendsliedrecht berendsliedrecht left a comment

Choose a reason for hiding this comment

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

Couple small items, nothing blocking.

test.sh Outdated Show resolved Hide resolved
libindy_vdr/src/common/error.rs Outdated Show resolved Hide resolved
libindy_vdr/src/ffi/resolver.rs Outdated Show resolved Hide resolved
libindy_vdr/src/ffi/resolver.rs Outdated Show resolved Hide resolved
libindy_vdr/src/ffi/resolver.rs Outdated Show resolved Hide resolved
libindy_vdr/src/resolver/resolver.rs Outdated Show resolved Hide resolved
libindy_vdr/src/resolver/utils.rs Outdated Show resolved Hide resolved
libindy_vdr/tests/nym.rs Outdated Show resolved Hide resolved
libindy_vdr/tests/resolver.rs Outdated Show resolved Hide resolved
libindy_vdr/tests/utils/fixtures.rs Outdated Show resolved Hide resolved
Signed-off-by: Andrew Whitehead <cywolf@gmail.com>
Signed-off-by: Andrew Whitehead <cywolf@gmail.com>
Signed-off-by: Andrew Whitehead <cywolf@gmail.com>
Signed-off-by: Andrew Whitehead <cywolf@gmail.com>
Signed-off-by: Andrew Whitehead <cywolf@gmail.com>
Signed-off-by: Andrew Whitehead <cywolf@gmail.com>
Signed-off-by: Andrew Whitehead <cywolf@gmail.com>
Signed-off-by: Andrew Whitehead <cywolf@gmail.com>
Signed-off-by: Andrew Whitehead <cywolf@gmail.com>
Signed-off-by: Andrew Whitehead <cywolf@gmail.com>
Signed-off-by: Andrew Whitehead <cywolf@gmail.com>
@andrewwhitehead andrewwhitehead merged commit 06082b8 into main Aug 8, 2023
@andrewwhitehead andrewwhitehead deleted the did-indy-merge branch August 8, 2023 16:44
@swcurran
Copy link
Member

swcurran commented Aug 8, 2023

w00t!!!

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.

6 participants