bn.js: Breaking changes for the next major release

(Taking the discussion from #112 to here)

Proposed changes:

  • Make .modn() return a BN (#112)
  • Renaming .strip() to ._strip() is a breaking change too (#105)
  • Maybe reviewing the constructor method(s) could lead to such a change (see #90 & #91)
  • Rename .andln() to .andn() (see the README). Perhaps fold the functionality into toArrayLike and make .andn() safe in terms of working up to 53 bits
  • Maybe at that stage two’s complement could be made more internal (part of constructor and toString) (see #73)
  • Split out the extended functions (saving destroyed bits) off iusrhn() into a specific shift right version, because it is only used in MPrime.split() and is complex.
  • Remove internal functions from the BN context (good candidates are: _countBits, _zeroBits, etc. Others could be moved too with passing a self variable instead of using this, however that might have a speed penalty or increase?)
  • Perhaps rethink naming/functionality of the following bitwise methods: setn (should be renamed isetn as it is in-place), testn, maskn, bincn

About this issue

  • Original URL
  • State: open
  • Created 8 years ago
  • Comments: 34 (27 by maintainers)

Most upvoted comments

Never mind, I’ve only realised now that toJSON is potentially used by JSON.stringify and it should have no other use beyond that. Perhaps documenting this would be enough. See this.

Here was another potential thing: Rename toJSON, see https://github.com/indutny/bn.js/pull/164#issuecomment-303332320

I think it is useful.

Unexpected padding errors have caused countless problems for libraries.

Maybe we can make it throw for just hex, and accept without padding for base-16?

Something like this, I guess: https://gist.github.com/indutny/f37c466b4765e686b766b0b32557557c . Btw, just recalled that Buffers can’t be used for it, only Uint32Array

Perhaps also the following could be discussed as part of this: