Possible incorrect "Extra Btyes" value set in ICRC1 ledger utils
Closed this issue · 4 comments
While working through some E2E tests involving tokens using the ICRC1 standard, I was checking whether the text format of the accounts involved with the principal/subaccounts being used matched...
I noticed that using the encodeICRC1 generated a different text value for the same principal and subaccount of an account that was generated a the canister using the verbatim code from the ICRC1 ledger ref Account.
The text value of the subaccount concatenation was the same, but the leading portion was different:
(from canister) aekzj-ryaaa-aaaaa-aaaia-cant7-uy6cn-hw4fy-sac2s-ihmjf-fuomz-glraf-2ahr6-nmkbk-toby7-y
(from e2e/node env) 5swrp-zyaaa-aaaaa-aaaia-cant7-uy6cn-hw4fy-sac2s-ihmjf-fuomz-glraf-2ahr6-nmkbk-tobz7-y
Looking a bit more closely, if I changed the EXTRA_BYTES:
const EXTRA_BYTES = parseInt("FF", 16);
to
const EXTRA_BYTES = parseInt("7F", 16);
in accordance with the ICRC1 spec it produces same value as the canister. This leads me to believe the FF should be 7F.
For reference here's the account I was converting:
ownerPrincipalAsText: "sgymv-uiaaa-aaaaa-aaaia-cai";
subaccountAsUInt8Array: Uint8Array(32) [ 0, 0, 0, 0, 179, 253, 49, 225, 52, 246, 225, 113, 32, 11, 82, 65, 216, 146, 150, 142, 102, 76, 184, 128, 186, 1, 227, 230, 177, 65, 84, 220 ]
Thanks for the input! Indeed the referenced ICRC1 docs also points to 7F.
@lmuntaner what you think?
Yes, that's true, thanks!
I can't understand why I used FF, it must have been a type and since we haven't used subaccounts yet we didn't catch it.
Thanks @atengberg we'll change it.
Just FYI though, the text encoding will probably change again. The working group of ICRC-1 decided to change the standard for the text representation.
Fix has been released to npm @dfinity/ledger@0.0.4 in mono-repo release v0.13.0