Skip to content

Conversation

steveluscher
Copy link
Contributor

@steveluscher steveluscher commented Mar 7, 2025

Problem

Consider the char code of a character. Codes >255 are prohibited when constructing an alphabet (throws new TypeError(x + ' is ambiguous')) but they are accepted when decoding a string.

const bs58 = base('123456789ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz');
const str = 'ABC' + /* char code 256 */ '\u0100' + 'DEF';
const bytes = bs58.decode(str);  // WORKS

That code should absolutely fatal with ‘non-base58 character’.

Sandbox link

Bench

Before

$ SEED=1c80dc20398124b795fe1bc8f37bd586 npm start

> [email protected] start
> node index.js

Seed: 1c80dc20398124b795fe1bc8f37bd586
--------------------------------------------------
encode x 371,690 ops/sec ±0.06% (9 runs sampled)
decode x 420,041 ops/sec ±0.12% (9 runs sampled)
==================================================

After

$ SEED=1c80dc20398124b795fe1bc8f37bd586 npm start

> [email protected] start
> node index.js

Seed: 1c80dc20398124b795fe1bc8f37bd586
--------------------------------------------------
encode x 378,646 ops/sec ±0.04% (9 runs sampled)
decode x 432,351 ops/sec ±0.14% (9 runs sampled)
==================================================

Backports

@steveluscher
Copy link
Contributor Author

I guess the 5.x backport is not needed, if you decide to release this as 5.0.1.

@fanatid fanatid merged commit e4cb9b0 into cryptocoinjs:master Mar 8, 2025
3 checks passed
@fanatid
Copy link
Member

fanatid commented Mar 8, 2025

Thanks! Published

  • v5.0.1
  • v4.0.1
  • v3.0.11

@steveluscher steveluscher deleted the prohibit-high-char-codes branch April 30, 2025 19:42
Copy link

@melissamforbs melissamforbs left a comment

Choose a reason for hiding this comment

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

reviewed

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