-
Notifications
You must be signed in to change notification settings - Fork 16
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
icrc index getLedgerId #693
Conversation
We already do something like that in Oisy. Let me check. |
size-limit report 📦
|
Indeed @mstrasinskis, a function See here https://github.com/dfinity/ic-js/tree/main/packages/ledger-icrc#gear-ledgerid |
…edger-id # Conflicts: # packages/ledger-icrc/src/index.canister.ts
Looks like this PR adds it on the non-ng index canister which does not seem to have it yet. |
@peterpeterparker, thank you for pointing this out. However, as @dskloetd already mentioned, this PR exposes the API on the non-NG canister, which is used in nns-dapp. |
Sounds good. Can you use the same naming |
Good idea, this way it will be easier to switch to the NG canister later. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thx
# Motivation To make [getLedgerId api](dfinity/ic-js#693) available in `nns-dapp` the `ic-js` package needs to be upgraded. # Changes - `npm run upgrade:next`. - Add `[]` to some new optional fields in `convertSwapInitParams` to fix type checking errors. # Tests - Pass # Todos - [ ] Add entry to changelog (if necessary). Not necessary
Motivation
For the import token functionality, we need to ensure that the provided by users index canister is correctly associated with the same token and not mistakenly linked to a different ledger canister. To achieve this, we going to use the
ledger_id
API of the index canister, which returns the corresponding ledger canister ID. This PR introduces a function that makes theledger_id
request.Changes
Tests
Todos
Not necessary.