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

Introduce a new type wrapping PartialAddress, PublicKey and AztecAddress #1516

Closed
3 tasks
benesjan opened this issue Aug 11, 2023 · 0 comments · Fixed by #1524
Closed
3 tasks

Introduce a new type wrapping PartialAddress, PublicKey and AztecAddress #1516

benesjan opened this issue Aug 11, 2023 · 0 comments · Fixed by #1524
Assignees
Labels
T-refactor Type: this code needs refactoring

Comments

@benesjan
Copy link
Contributor

benesjan commented Aug 11, 2023

As @spalladino pointed out here it would be valuable to introduce a new type wrapping the aforementioned types because these 3 values are needed to send user a note and for this reason it's common that they are "appearing" together.

Possible names are:

  1. ExtendedAddress,
  2. CompleteAddress,
  3. FullAddress.

I am personally ok with any of these names.

I think the introduction of this type will be a bit confusing so explain the class docs why the type is needed.

  • Introduce the type,
  • make getAccounts return the new type instance and drop the getPublicKeyAndPartialAddress function,
  • write docs explaining why the type is needed.
@github-project-automation github-project-automation bot moved this to Todo in A3 Aug 11, 2023
@benesjan benesjan self-assigned this Aug 11, 2023
@benesjan benesjan moved this from Todo to In Progress in A3 Aug 11, 2023
@iAmMichaelConnor iAmMichaelConnor added the T-refactor Type: this code needs refactoring label Aug 14, 2023
@spalladino spalladino moved this from In Progress to In Review in A3 Aug 15, 2023
benesjan added a commit that referenced this issue Aug 15, 2023
1. Fixes #1516 
2. Fixes #1527
3. Fixes #1529 
4. Documents `e2e_aztec_js_browser`.
5. Fixes unexpected "previous test run dependency" in
e2e_aztec.js_browser test.
@github-project-automation github-project-automation bot moved this from In Review to Done in A3 Aug 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-refactor Type: this code needs refactoring
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants