Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support BigNumber #1

Closed
wants to merge 3 commits into from
Closed

Support BigNumber #1

wants to merge 3 commits into from

Conversation

aquaflamingo
Copy link

@aquaflamingo aquaflamingo commented Mar 19, 2019

Convert the library to use ‘bn.js’ instead of number formats.

What has changed

  • converted all test specs from json format to js and using BN instead of number
  • convert split.js to use BN and pass tests
  • convert accumulative.js to use BN and pass tests
  • convert blackjack.js to use BN and pass tests
  • convert index.js to use BN and pass tests
  • Refactor various utils to convert to BN
  • Index tests
  • break tests
  • accumulate tests
  • split tests

accumulative.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
break.js Outdated Show resolved Hide resolved
split.js Outdated Show resolved Hide resolved

// skip detrimental input
if (utxoFee > utxo.value) {
if (i === utxos.length - 1) return { fee: feeRate * (bytesAccum + utxoBytes) }
var feeIsMoreThanValue = ext.gt(utxoFee, utxoValue)

Choose a reason for hiding this comment

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

1


// would it waste value?
if ((inAccum + inputValue) > (outAccum + fee + threshold)) continue
var totalInputs = ext.add(inAccum, inputValue)

Choose a reason for hiding this comment

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

2


// did we bust?
if (inAccum < (outAccum + fee + value)) {
if (ext.lt(inAccum, ext.add(outAccum, fee, value))) {
var isZero = ext.isZero(outAccum)

Choose a reason for hiding this comment

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

3

if (outputs.length === 0) return { fee: fee }

var inAccum = utils.sumOrNaN(utxos)
var outAccum = utils.sumForgiving(outputs)
var remaining = inAccum - outAccum - fee
var remaining = ext.sub(inAccum, outAccum, fee)
if (!isFinite(remaining) || remaining < 0) return { fee: fee }

Choose a reason for hiding this comment

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

shouldn't this be changed too

if (outputs.length === 0) return { fee: fee }

var inAccum = utils.sumOrNaN(utxos)
var outAccum = utils.sumForgiving(outputs)
var remaining = inAccum - outAccum - fee
var remaining = ext.sub(inAccum, outAccum, fee)
if (!isFinite(remaining) || remaining < 0) return { fee: fee }

var unspecified = outputs.reduce(function (a, x) {
return a + !isFinite(x.value)

Choose a reason for hiding this comment

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

and this

Choose a reason for hiding this comment

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

looks like they really just wanted to do outputs.filter(x => !isFinite(x.value)).length
but wanted to maintain compatibility with old js

Choose a reason for hiding this comment

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

but it also uses .every..so filter should have been fine...


var splitOutputsCount = outputs.reduce(function (a, x) {
// Counts the number of split outputs left
var splitOutputsCount = new BN(outputs.reduce(function (a, x) {
return a + !x.value

Choose a reason for hiding this comment

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

this?

@petejkim
Copy link

Why is this commit amended with BN changes: 339c1bf

@petejkim
Copy link

Moved to #2

@petejkim petejkim closed this Apr 17, 2019
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