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 test for rotating single-sig delegate identifier #160

Conversation

psteniusubi
Copy link
Contributor

The last test is marked as failing. Reproduces issue #159

Keria stack trace

signify-ts-keria-1         | 2023-12-02 09:12:11 [FALCON] [ERROR] PUT /identifiers/delegate1 => Traceback (most recent call last):
signify-ts-keria-1         |   File "/keria/venv/lib/python3.10/site-packages/keri/app/habbing.py", line 2443, in processEvent
signify-ts-keria-1         |     self.kvy.processEvent(serder=serder, sigers=sigers)
signify-ts-keria-1         |   File "/keria/venv/lib/python3.10/site-packages/keri/core/eventing.py", line 2994, in processEvent
signify-ts-keria-1         |     kever.update(serder=serder, sigers=sigers, wigers=wigers,
signify-ts-keria-1         |   File "/keria/venv/lib/python3.10/site-packages/keri/core/eventing.py", line 1940, in update
signify-ts-keria-1         |     raise ValidationError("Attempted non delegated rotation on "
signify-ts-keria-1         | keri.kering.ValidationError: Attempted non delegated rotation on delegated pre = EA8nuk8Z3ZwhL4PmECqNs6w4hUk47RGeiRTn0UKvRf74 with evt = {'v': 'KERI10JSON000160_', 't': 'rot', 'd': 'EA0dFkf7S1w3uJANMeZlcWAzTQRbgqURdjpFxSbjgekk', 'i': 'EA8nuk8Z3ZwhL4PmECqNs6w4hUk47RGeiRTn0UKvRf74', 's': '1', 'p': 'EA8nuk8Z3ZwhL4PmECqNs6w4hUk47RGeiRTn0UKvRf74', 'kt': '1', 'k': ['DLmuY28mF6Golgq2SJVENE4iLXqNc2nMcb7bh6eiLSqZ'], 'nt': '1', 'n': ['ELY_6-spjz3o-ceMXis4i81maZaRgzk45vjVWKMaxOAs'], 'bt': '0', 'br': [], 'ba': [], 'a': []}.
signify-ts-keria-1         |
signify-ts-keria-1         | During handling of the above exception, another exception occurred:
signify-ts-keria-1         |
signify-ts-keria-1         | Traceback (most recent call last):
signify-ts-keria-1         |   File "falcon/app.py", line 365, in falcon.app.App.__call__
signify-ts-keria-1         |   File "/keria/src/keria/app/aiding.py", line 470, in on_put
signify-ts-keria-1         |     op = self.rotate(agent, name, body)
signify-ts-keria-1         |   File "/keria/src/keria/app/aiding.py", line 506, in rotate
signify-ts-keria-1         |     hab.rotate(serder=serder, sigers=sigers)
signify-ts-keria-1         |   File "/keria/venv/lib/python3.10/site-packages/keri/app/habbing.py", line 2404, in rotate
signify-ts-keria-1         |     self.processEvent(serder, sigers)
signify-ts-keria-1         |   File "/keria/venv/lib/python3.10/site-packages/keri/app/habbing.py", line 2445, in processEvent
signify-ts-keria-1         |     raise kering.ConfigurationError(f"Improper Habitat event type={serder.ked['t']} for "
signify-ts-keria-1         | keri.kering.ConfigurationError: Improper Habitat event type=rot for pre=EA8nuk8Z3ZwhL4PmECqNs6w4hUk47RGeiRTn0UKvRf74.
signify-ts-keria-1         |

Copy link

codecov bot commented Dec 2, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (c64e006) 81.42% compared to head (0944806) 81.42%.

❗ Current head 0944806 differs from pull request most recent head 945293e. Consider uploading reports for the commit 945293e to get more accurate results

Additional details and impacted files
@@             Coverage Diff              @@
##           development     #160   +/-   ##
============================================
  Coverage        81.42%   81.42%           
============================================
  Files               46       46           
  Lines             4141     4141           
  Branches          1030     1030           
============================================
  Hits              3372     3372           
  Misses             737      737           
  Partials            32       32           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@lenkan
Copy link
Collaborator

lenkan commented Dec 9, 2023

@psteniusubi and others:

Do we intend to wait with this until the functionality is fixed so the test passes for real? Or should we merge it regardless?

An alternative is to just add all the passing tests, then add a PR with the failing test on top of that. Just to make sure we don't forget about it and have a bunch of skipped tests in the test run.

@psteniusubi
Copy link
Contributor Author

It's a good question.
Is it better to create a PR like this with the test failing? Then people can see the error from the logs of the CI integration test run.
An other PR like this is here #157

@m00sey
Copy link
Member

m00sey commented Dec 15, 2023

Is it better to create a PR like this with the test failing?

I'd rather have an issue, with a link to a fork with the failing test.

When I see PRs I think "mergable code".

@psteniusubi psteniusubi force-pushed the feat-integration-test-singlesig-drt branch from b9ab255 to 67e86c7 Compare December 16, 2023 08:59
Copy link
Member

@pfeairheller pfeairheller left a comment

Choose a reason for hiding this comment

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

The library needs drt support.

let delegate1 = await client2.identifiers().get('delegate1');
let seal = {
i: delegate1.prefix,
s: 0,
Copy link
Member

Choose a reason for hiding this comment

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

This sn value needs to be a string like '0'

test('delegate1c', async () => {
// delegate rotates identifier
let kargs: RotateIdentifierArgs = {};
let result = await client2.identifiers().rotate('delegate1', kargs);
Copy link
Member

Choose a reason for hiding this comment

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

This rotate method does not support drt event types for delegated identifiers. It will need to be enhanced to support delegated AIDs

@pfeairheller
Copy link
Member

I'm going to add drt support to the library and then retry this test.

@pfeairheller pfeairheller self-assigned this Jan 17, 2024
@psteniusubi psteniusubi force-pushed the feat-integration-test-singlesig-drt branch from 0944806 to 945293e Compare January 18, 2024 10:40
@pfeairheller
Copy link
Member

I reintroduced this test in #197 where I also fixed the other delegation integration tests. Closing this no in favor of that one. Thanks for the script!!

@psteniusubi psteniusubi deleted the feat-integration-test-singlesig-drt branch February 6, 2024 08:13
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