Skip to content

Commit

Permalink
Don't replace absent memo with empty memo on ICRC-1 (#423)
Browse files Browse the repository at this point in the history
# Motivation

Ledger hardware wallet displays the memo as a number, even though for
ICRC-1 transfer the memo is a byte array.
When the memo is not set at all, it renders the number 0, but when the
empty byte array is set, it renders the number 422710814078008.
Rendering no memo as 0 seems better than rendering it as 422710814078008
and setting an empty array was done for HW compatibility in the first
place so let's stop doing it.

# Changes

When the memo is absent, keep it like that instead of replacing it with
the empty byte array.

# Tests

Unit test is updated.
Also tested manually with NNS dapp to verify that it renders memo 0 now.
  • Loading branch information
dskloetd authored Sep 22, 2023
1 parent 7174c98 commit d9cf048
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 4 deletions.
3 changes: 1 addition & 2 deletions packages/nns/src/canisters/ledger/ledger.request.converts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,7 @@ export const toIcrc1TransferRawRequest = ({
to,
fee: toNullable(fee ?? TRANSACTION_FEE),
amount,
// Always explicitly set the memo for compatibility with ledger wallet - hardware wallet
memo: toNullable(memo ?? new Uint8Array()),
memo: toNullable(memo),
created_at_time: toNullable(createdAt),
from_subaccount: toNullable(fromSubAccount),
});
3 changes: 1 addition & 2 deletions packages/nns/src/ledger.canister.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -678,7 +678,6 @@ describe("LedgerCanister", () => {
Ok: BigInt(1234),
});
const fee = BigInt(10_000);
const defaultMemo = new Uint8Array();
const ledger = LedgerCanister.create({
certifiedServiceOverride: service,
});
Expand All @@ -692,7 +691,7 @@ describe("LedgerCanister", () => {
to,
fee: [fee],
amount,
memo: [defaultMemo],
memo: [],
created_at_time: [],
from_subaccount: [],
});
Expand Down

0 comments on commit d9cf048

Please sign in to comment.