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

PERA-1348 :: Add in architecture related ER diagrams designed in mermaid #92

Merged
merged 2 commits into from
Jan 7, 2025

Conversation

michaeltchuang
Copy link
Collaborator

@michaeltchuang michaeltchuang commented Dec 28, 2024

Pull Request Template

Description

  • Add in architecture ER diagrams designed in mermaid tool

Related Issues

Checklist

  • Have you tested your changes locally?
  • Have you reviewed the code for any potential issues?
  • Have you documented any necessary changes in the project's documentation?
  • Have you added any necessary tests for your changes?
  • Have you updated any relevant dependencies?

Additional Notes

  • Any modifications to diagram syntax should be viewable in mermaid

@michaeltchuang michaeltchuang added this to the 6.0.0 milestone Dec 28, 2024
@michaeltchuang michaeltchuang self-assigned this Dec 28, 2024
@michaeltchuang michaeltchuang force-pushed the PERA-1348-architecture branch 2 times, most recently from c63256b to fc92e96 Compare December 31, 2024 23:42
@michaeltchuang michaeltchuang added the enhancement New feature or request label Jan 1, 2025
@michaeltchuang michaeltchuang changed the title PERA-1348 :: Add in architecture related diagrams designed in mermaid PERA-1348 :: Add in architecture related ER diagrams designed in mermaid Jan 1, 2025
String seed_custom_name
}
hd_keys {
String encrypted_address PK
Copy link
Collaborator

Choose a reason for hiding this comment

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

@michaeltchuang @mitsinsar Lets not use address, but public keys

Copy link
Collaborator

Choose a reason for hiding this comment

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

Lets also avoid strings and any encoding to store raw public keys. Bytes :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@michaeltchuang @mitsinsar Lets not use address, but public keys

@ehanoc I actually had public_key as PK at first for hd_keys table...but I realized when we fetch/delete through Pera UI or deeplink, user may want to do this by algorand address...so I think algorand address should be a column in table. Is it easy to go algorand address -> public key? If so, then maybe I can create a mapper back/forth.

@yigitguler are you back this week? Did you want to weigh in what you think should be stored and what should be derived runtime?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I updated the PR now so hd code/schema is using public keys and algorand addresses are derived runtime

Choose a reason for hiding this comment

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

@michaeltchuang Algorand address can be seen as a denormalized field in the table. IMO, it is ok to keep the Algorand address in the table as well, only if it brings significant performance optimization.

Choose a reason for hiding this comment

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

In a scenario where the user has 100 accounts (I know it is above our current limits) How much computation time does it require to get the algorand address from the public key? If it is really fast, maybe we can just store the public key and prepare some utility functions to lookup the account with the algorand address.

@michaeltchuang michaeltchuang force-pushed the PERA-1348-architecture branch 2 times, most recently from 712006e to 335dff8 Compare January 2, 2025 04:13
@michaeltchuang michaeltchuang force-pushed the PERA-1348-architecture branch from 49c3a59 to 461818c Compare January 7, 2025 04:38
@michaeltchuang michaeltchuang marked this pull request as ready for review January 7, 2025 18:21
@michaeltchuang michaeltchuang merged commit 59bac46 into dev Jan 7, 2025
11 checks passed
@michaeltchuang michaeltchuang deleted the PERA-1348-architecture branch January 7, 2025 18:37
@github-actions github-actions bot locked and limited conversation to collaborators Jan 7, 2025
String entropy_custom_name
}
hd_addresses {
String encrypted_address PK
Copy link
Collaborator

Choose a reason for hiding this comment

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

@michaeltchuang Why encrypted address & public key? there's zero risk and adds extra steps at no gain.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants