Skip to content

refactor(crypto): remove long dependency#3502

Merged
faustbrian merged 7 commits into2.6from
longs
Feb 17, 2020
Merged

refactor(crypto): remove long dependency#3502
faustbrian merged 7 commits into2.6from
longs

Conversation

@faustbrian
Copy link
Copy Markdown
Contributor

@faustbrian faustbrian commented Feb 14, 2020

The long@3 dependency can cause problems if people also have long@4 installed because node.js will think it should use long@4 even though we don't have it as a dependency. This will cause the value instanceof Long check in the bytebuffer dependency to fail because its own dependency is long@3 but node.js will force it to use long@4 if it is installed, which will throw an exception because the instance signatures will differ.

The bytebuffer types have been removed because they don't reflect their JavaScript code. The types state that you can only pass in a number or Long but their code will actually perform assertions and use Long internally to create an instance. This means that whatever long version is installed will be used.

ByteBufferPrototype.writeUint64 = function(value, offset) {
    var relative = typeof offset === 'undefined';
    if (relative) offset = this.offset;
    if (!this.noAssert) {
        if (typeof value === 'number')
            value = Long.fromNumber(value);
        else if (typeof value === 'string')
            value = Long.fromString(value);
        else if (!(value && value instanceof Long))
            throw TypeError("Illegal value: "+value+" (not an integer or Long)");
        if (typeof offset !== 'number' || offset % 1 !== 0)
            throw TypeError("Illegal offset: "+offset+" (not an integer)");
        offset >>>= 0;
        if (offset < 0 || offset + 0 > this.buffer.length)
            throw RangeError("Illegal offset: 0 <= "+offset+" (+"+0+") <= "+this.buffer.length);
    }
    if (typeof value === 'number')
        value = Long.fromNumber(value);
    else if (typeof value === 'string')
        value = Long.fromString(value);

@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 14, 2020

Codecov Report

Merging #3502 into 2.6 will increase coverage by <.01%.
The diff coverage is 88.88%.

Impacted file tree graph

@@            Coverage Diff             @@
##              2.6    #3502      +/-   ##
==========================================
+ Coverage   66.13%   66.14%   +<.01%     
==========================================
  Files         439      439              
  Lines       12460    12464       +4     
  Branches     1711     1711              
==========================================
+ Hits         8241     8244       +3     
- Misses       4185     4186       +1     
  Partials       34       34
Impacted Files Coverage Δ
packages/crypto/src/transactions/types/schemas.ts 100% <100%> (ø) ⬆️
packages/crypto/src/transactions/registry.ts 71.05% <100%> (+0.78%) ⬆️
.../core-transactions/src/handlers/multi-signature.ts 68.62% <80%> (ø) ⬆️
packages/core-transaction-pool/src/storage.ts 97.5% <0%> (-2.5%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8c9b225...e543457. Read the comment docs.

air1one
air1one previously approved these changes Feb 14, 2020
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.

2 participants