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

feat: group ipex presentations (offer, grant) with leader #866

Merged
merged 3 commits into from
Dec 12, 2024

Conversation

iFergal
Copy link
Collaborator

@iFergal iFergal commented Dec 11, 2024

Description

Update to group IPEX presentations based on recent discusses in WebOfTrust/keripy#884. Now the core expects the group initiator to reply with /ipex/offer and /ipex/grant messages first, and other members to join. Also takes the simplifications of letting KERIA track who has joined, rather than locally, like in the recent receive credential ticket.

The UI for this is vastly different, so I've made some adjustments to get this working e2e but will of course require a lot of UI changes, refactoring and simplifications.

2 follow up PR notes:

  1. Agreed with Robert that when the long running operations complete, instead of deleting the notification we will "refresh" the notification to appear unread on the top of the notification stack. This requires core changes but can be coordinated after UI is done I think.
  2. If the /ipex/* notification is deleted before the final /multisig/exn message is received (i.e. long running operation checked before notifications checked), we end up with an "out of order" notification that re-tries forever. So the initiator tends to get constant errors in the console. This doesn't happen the receive credential (unless you delete the pending credential) because we check if the credential exists. My plan here across the board is to check the IPEX history on KERIA. This means we need to store /ipex/agree history items, but hide from the UI - right now we aren't storing it.

Checklist before requesting a review

Issue ticket number and link

  • This PR has a valid ticket number or issue: [link]

Testing & Validation

  • This PR has been tested/validated in IOS, Android and browser.
  • The code has been tested locally with test coverage match expectations.
  • Added new Unit/Component testing (if relevant).

Security

  • No secrets are being committed (i.e. credentials, PII)
  • This PR does not have any significant security implications

Code Review

  • There is no unused functionality or blocks of commented out code (otherwise, please explain below)
  • In addition to this PR, all relevant documentation (e.g. Confluence) and architecture diagrams (e.g. Miro) were updated

sdisalvo-crd
sdisalvo-crd previously approved these changes Dec 12, 2024
@iFergal iFergal merged commit e64a571 into develop Dec 12, 2024
1 check passed
@iFergal iFergal deleted the feat/DTIS-1399-present-credential-initiator branch December 12, 2024 17:06
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.

3 participants