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: ICRC Index canister next gen #380

Merged
merged 37 commits into from
Mar 4, 2024
Merged

Conversation

peterpeterparker
Copy link
Member

@peterpeterparker peterpeterparker commented Jul 5, 2023

Motivation

Use the Index canister "next gen", implement balance for Index canister and make fetching transactions available as query.

Changes

  • use index-ng/index-ng.did as source for the Index canister
  • update JS accordingly
  • implement balance for Index canister
  • getTransactions accept queries

@github-actions
Copy link
Contributor

github-actions bot commented Jul 5, 2023

size-limit report 📦

Path Size
@dfinity/ckbtc 7.23 KB (0%)
@dfinity/cketh 1.97 KB (0%)
@dfinity/cmc 1.29 KB (0%)
@dfinity/ledger-icrc 3.15 KB (+6.26% 🔺)
@dfinity/ledger-icp 15.83 KB (0%)
@dfinity/nns 35.74 KB (0%)
@dfinity/nns-proto 76.44 KB (0%)
@dfinity/sns 15.43 KB (0%)
@dfinity/utils 4.44 KB (0%)
@dfinity/ic-management 2.44 KB (0%)

@peterpeterparker peterpeterparker marked this pull request as ready for review March 4, 2024 10:36
@peterpeterparker
Copy link
Member Author

The Snses are finally able to use the index-ng canister (ref forum). Therefore we can go on with this PR which is already used since two months in Oisy Wallet.

Copy link
Contributor

@lmuntaner lmuntaner left a comment

Choose a reason for hiding this comment

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

Thanks! Just a couple of comments.

packages/ledger-icrc/src/canister.ts Show resolved Hide resolved
packages/ledger-icrc/src/index.canister.ts Show resolved Hide resolved
Copy link
Contributor

@lmuntaner lmuntaner left a comment

Choose a reason for hiding this comment

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

Thanks!

@peterpeterparker peterpeterparker merged commit d2a5294 into main Mar 4, 2024
11 checks passed
@peterpeterparker peterpeterparker deleted the feat/ledger-index-next-gen branch March 4, 2024 12:25
peterpeterparker added a commit that referenced this pull request Mar 4, 2024
# Motivation

`index-ng` isn't backwards compatible and introduces an issue in NNS dapp when calling "old index" canister:

```
Sorry, there was an error loading the transactions for this account. Cannot find required field balance
```

This PR reverts the code changes to support the index-ng canister that were merged in `main`.

Those PRs were:

- #380
- #561
peterpeterparker added a commit that referenced this pull request Mar 5, 2024
…-icrc (#562)

# Motivation

Provide support for the `index-ng` canister in `@dfinity/ledger-icrc`.

# Notes

The support was originally introduced as a replacement of the existing `IndexCanister` but, after testing a next release, we figured out it introduced a breaking changes as a variable `balance` was added as a non optional argument in the `GetTransactions` results.

This lead to following error when the canister was using to query "old-index" canister:

```
Sorry, there was an error loading the transactions for this account. Cannot find required field balance
```

That's why ultimately we decided to introduce support for the `index-ng` canister in a dedicated module.

# Previous PRs

For tracability:

- #380
- #561
- #564

# Changes

- use `index-ng/index-ng.did` as source for the Index-ng canister
- extract an interface for `icrc_balance_of` for Index-ng and ledger canister (we assume in the future there might be other duplicate functions that are shared by both canisters)
- Implement `getTransactions` which accept queries
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