Skip to content

Support ICRC tokens multiple decimals#54

Merged
ielashi merged 2 commits intomainfrom
lm-test-icrc-txs
Nov 25, 2025
Merged

Support ICRC tokens multiple decimals#54
ielashi merged 2 commits intomainfrom
lm-test-icrc-txs

Conversation

@lmuntaner
Copy link
Copy Markdown
Contributor

No description provided.

@lmuntaner lmuntaner marked this pull request as ready for review November 19, 2025 13:34
@lmuntaner lmuntaner requested a review from a team as a code owner November 19, 2025 13:34
@lmuntaner lmuntaner requested a review from ielashi November 19, 2025 13:34
Comment thread src/ledger/identity.ts Outdated
if (resp.returnCode == 28161) {
throw "Please open the Internet Computer app on your wallet and try again.";
} else if (resp.returnCode == LedgerError.TransactionRejected) {
} else if (resp.returnCode == 65535) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Line 116 below also checks else if (resp.returnCode == 65535), so IIUC, with this change, we'd never end up there? Would it make sense to set this to 0x6986 (LedgerError.TransactionRejected) instead, or 0x5515 (DeviceLocked)?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point, done. I also changed all the codes to hexadecimal for better reference and add the link to the codes.

Comment thread src/ledger/identity.ts Outdated
public async getVersion(): Promise<Version> {
return this._executeWithApp(async (app: LedgerApp) => {
const res = await app.getVersion();
// TODO: What would it mean if there is no version? Should we throw an error?
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should this be part of this change? It's not clear why it's needed, but in theory I'd rather crash/return an error if there is no version rather than return a version of zero.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point, done!

@ielashi ielashi merged commit 8678676 into main Nov 25, 2025
4 checks passed
@ielashi ielashi deleted the lm-test-icrc-txs branch November 25, 2025 10:04
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.

3 participants