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

Using amountSatStr on output gives wrong results #522

Closed
nilnullzip opened this issue Oct 1, 2014 · 8 comments
Closed

Using amountSatStr on output gives wrong results #522

nilnullzip opened this issue Oct 1, 2014 · 8 comments

Comments

@nilnullzip
Copy link

On outputs, specifying amountSatStr of "990000.00000000" gives a different result from "990000". The former appears to be interpreted as Bitcoins, while the latter satoshis. Not sure where this happens. And not sure if the scaling is a result of being interpreted as Bitcoins, or whether the scaling is a result of the eight fractional zeros.

(Sorry, don't have a means to create a test case at the moment. Will try to do so. For the moment, I just want to record the observation.)

@nilnullzip nilnullzip changed the title amountSatStr on output gives wrong results Using amountSatStr on output gives wrong results Oct 1, 2014
@nilnullzip
Copy link
Author

The reason appears to be in this line 403 in TransactionBuilder:

var amountSat = outs[i].amountSat || outs[i].amountSatStr ? bignum(outs[i].amountSatStr) : util.parseValue(outs[i].amount);

Specifically this case:

bignum(outs[i].amountSatStr)

bignum only accepts integers strings.

@timolson
Copy link

timolson commented Oct 2, 2014

I tried this with bignum 0.9.0:

var bignum = require('bignum');

console.log('bignumtest');
console.log( bignum('123') );
console.log( bignum('123.0') );
console.log( bignum('123.4567') );
bignumtest
<BigNum 123>
<BigNum 123>
<BigNum 123>

@timolson
Copy link

timolson commented Oct 2, 2014

See also
justmoon/node-bignum#1

@nilnullzip
Copy link
Author

Try in the browser with bitcore loaded there:

var bignum = require('bitcore').bignum
console.log(Number(bignum("990000")))
console.log(Number(bignum("990000.00000000")))

Prints:

990000
990000000000000

@timolson
Copy link

timolson commented Oct 2, 2014

On the server-side, in Meteor, using bitcore 0.1.34:

var bignum = require('bitcore').bignum;

console.log('bignumtest');
console.log( bignum('123') );
console.log( bignum('123.000') );
console.log( bignum('123.4567') );

=>

bignum (with a lower-case "b") is deprecated. Use bitcore.Bignum (capital "B") instead.
bignumtest
<BigNum 123>
<BigNum 123>
<BigNum 123>

In my browser, Opera, it breaks the same way you said:

var bignum = require('bitcore').bignum
console.log(Number(bignum("990000")))
console.log(Number(bignum("990000.00000000")))

=>

bignum (with a lower-case "b") is deprecated. Use bitcore.Bignum (capital "B") instead.
990000
990000000000000 

The JS code should be the exact same, being pushed by Meteor to the client. Very strange, perhaps this is a bug in a parsing library beneath bignum?

@nilnullzip
Copy link
Author

Perhaps the node and browser bignum implementations differ in some respects. If I understand correctly, the node library has content in C++. Also bitcore has the following file that appears to be used for the browser build: /lib/browser/Bignum.js.

@maraoz
Copy link
Contributor

maraoz commented Oct 2, 2014

Good catch. Will write a test and see if it fails in node or in the browser

@maraoz
Copy link
Contributor

maraoz commented Dec 19, 2014

oudated. Please use our new Transaction API: https://github.com/bitpay/bitcore/blob/master/docs/guide/transaction.md

@maraoz maraoz closed this as completed Dec 19, 2014
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

No branches or pull requests

3 participants