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(frontend): optional index canister #3334

Merged
merged 44 commits into from
Nov 8, 2024

Conversation

peterpeterparker
Copy link
Member

@peterpeterparker peterpeterparker commented Nov 5, 2024

Motivation

Make the index canister ID optional in the core type definition without changing the way we load information - i.e. in a backwards compatible manner.

Notes

This PR is relatively large, so Antonio and I reviewed the code together offline.

Changes

  • custom-token.services.ts: extend toCustomToken to support optional Index canister ID
  • IcTransactions.svelte: silently ignore listing transactions if no index
  • ic-token.schema.ts: set Index Canister as optional in the base type IcCanisters
  • ic-add-custom-tokens.service.ts: extend support to, in the future, to allow user to register custom token without index canister id
  • icrc-wallet.worker.ts: assert index is set. in the future this should not be called without index
  • add few TODOs
  • adapt tests

Base automatically changed from feat/ic-token-zod to main November 5, 2024 12:01
peterpeterparker added a commit that referenced this pull request Nov 5, 2024
# Motivation

This might not suffise but, it's a first step, type wise, to defined the
"Index Canister" as optional.

# Notes

I add to include comments because we cannot just set the index as
optional. On the other side, I also need this new validation becase
otherwise I cannot split PR #3334 in smaller PRs.

# Changes

- Provide validation and types `IcCanistersStrictSchema`
- Add TODO for making the Index Canister optional typewise
peterpeterparker added a commit that referenced this pull request Nov 5, 2024
)

# Motivation

We will need to know if IC tokens are provided with or without Index
Canister ID. Unrelated but, somehow related, we also want to know if a
token is just a token or an IC token.

# Notes

Some test uncommented and will be activated in PR #3334 because they
cannot be active unless the Index Canister ID is set to optional.

# Changes

- New validators `isIcToken`, `isNotIcToken`, `isIcTokenCanistersStrict`
and `isNotIcTokenCanistersStrict`
peterpeterparker added a commit that referenced this pull request Nov 6, 2024
# Motivation

To make the index canister optional in #3334 we need to test the
`custom-token.services` which itself uses the `token.services`. That's
why we need test for that services. Of course, we also need test
generally speaking.
peterpeterparker added a commit that referenced this pull request Nov 7, 2024
# Motivation

We need few more tests before being able to complete #3334.
@peterpeterparker peterpeterparker marked this pull request as ready for review November 7, 2024 07:41
@peterpeterparker peterpeterparker requested a review from a team as a code owner November 7, 2024 07:41
Copy link
Collaborator

@AntonioVentilii AntonioVentilii left a comment

Choose a reason for hiding this comment

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

LGTM, TKs!

@peterpeterparker peterpeterparker enabled auto-merge (squash) November 8, 2024 07:47
@peterpeterparker peterpeterparker merged commit 3a4e4f0 into main Nov 8, 2024
10 of 11 checks passed
@peterpeterparker peterpeterparker deleted the feat/optional-index-canister branch November 8, 2024 07:52
@peterpeterparker
Copy link
Member Author

Relates to #2883

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.

2 participants