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

Removing AID delete endpoint #262

Merged
merged 1 commit into from
Jul 11, 2024

Conversation

rodolfomiranda
Copy link
Collaborator

This PR removes the delete AID endpoint I created before. The implementation wasn't correct since it only removes the hab generation inconsistencies in the database if the AID also had registries. Rethinking the concept, I consider that deleting an AID in not what we need, but instead an implementation of the abandon function that basically rotates the identifier to null keys so it can no longer be used to sign nor add new events in into the KEL.
The abandon function can be created in the client side since its a double rotation to null keys.

Copy link

codecov bot commented Jul 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.08%. Comparing base (18d3ad7) to head (839fda5).
Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #262      +/-   ##
==========================================
+ Coverage   93.06%   93.08%   +0.01%     
==========================================
  Files          36       36              
  Lines        7121     7184      +63     
==========================================
+ Hits         6627     6687      +60     
- Misses        494      497       +3     

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

@lenkan
Copy link
Collaborator

lenkan commented Jul 1, 2024

@rodolfomiranda What about adding a check and return Bad Request if the AID to be deleted has a registry?

@rodolfomiranda
Copy link
Collaborator Author

we can delete the registry as well, but I think it's better to keep KEL and TEL in DBs for an AID. The only benefit of deleting is to prune database. However the saving in size is minimal to be of a concern.
That's why I'd prefer to abandoned it, it's more keri friendly . Detecting an abandoned AID can be simple, we can even put a filter in the identifiers.list()

@lenkan
Copy link
Collaborator

lenkan commented Jul 1, 2024

we can delete the registry as well, but I think it's better to keep KEL and TEL in DBs for an AID. The only benefit of deleting is to prune database. However the saving in size is minimal to be of a concern. That's why I'd prefer to abandoned it, it's more keri friendly . Detecting an abandoned AID can be simple, we can even put a filter in the identifiers.list()

I also think maybe they can serve different purposes. For certain environments, like demo or testing, it can sometimes be useful to be able to delete them altogether. However, I do not really have a strong opinion on it. The proposed abandon feature would certainly work for those use cases as well. 👍

@rodolfomiranda
Copy link
Collaborator Author

rodolfomiranda commented Jul 1, 2024

I opened this PR because we allowed one user to delete the AID and it worked fine until he needed to reconnect with KERIA. At that point, keria iterates through all registries for that user and crash when it hits the one with the deleted hab. We had to create a hot patch to recover from that situation. I'm worried that it will also happen with other tables/db since the dependencies between them are hard to track.

We may need a keria maintenance interface.

@pfeairheller pfeairheller merged commit 1232405 into WebOfTrust:main Jul 11, 2024
5 checks passed
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