From e622fde7e71aad898f5e090de50665d7f6269ac2 Mon Sep 17 00:00:00 2001 From: Josh Rickmar Date: Sat, 12 Apr 2014 16:20:11 -0500 Subject: [PATCH] Remove bounds check for NewAmount. Amount should still be a usable type even if the monetary amount being described is not an amount at a single instance in time, for example, the total of all BTC received by an address. Therefore, the bounds checks that the amount is within the total amount of bitcoin ever producable have been removed. The checks for NaN and +-Infinity remain. --- amount.go | 21 ++++++++++++--------- amount_test.go | 26 ++++++++++++++------------ test_coverage.txt | 18 +++++++++--------- 3 files changed, 35 insertions(+), 30 deletions(-) diff --git a/amount.go b/amount.go index f3fba8a..6383aa2 100644 --- a/amount.go +++ b/amount.go @@ -54,20 +54,23 @@ func (u AmountUnit) String() string { type Amount int64 // NewAmount creates an Amount from a floating point value representing -// some value in bitcoin. +// some value in bitcoin. NewAmount errors if f is NaN or +-Infinity, but +// does not check that the amount is within the total amount of bitcoin +// producable as f may not refer to an amount at a single moment in time. func NewAmount(f float64) (Amount, error) { - a := f * float64(SatoshiPerBitcoin) - - // The amount is only valid if it does not exceed the total amount - // of bitcoin producable, and is not a floating point number that - // would otherwise fail that check such as NaN or +-Inf. - switch abs := math.Abs(a); { - case abs > float64(MaxSatoshi): + // The amount is only considered invalid if it cannot be represented + // as an integer type. This may happen if f is NaN or +-Infinity. + switch { + case math.IsNaN(f): fallthrough - case math.IsNaN(abs) || math.IsInf(abs, 1): + case math.IsInf(f, 1): + fallthrough + case math.IsInf(f, -1): return 0, errors.New("invalid bitcoin amount") } + a := f * float64(SatoshiPerBitcoin) + // Depending on the sign, add or subtract 0.5 and rely on integer // truncation to correctly round the value up or down. if a < 0 { diff --git a/amount_test.go b/amount_test.go index a2fe778..7361ddb 100644 --- a/amount_test.go +++ b/amount_test.go @@ -25,17 +25,29 @@ func TestAmountCreation(t *testing.T) { expected: 0, }, { - name: "max", + name: "max producable", amount: 21e6, valid: true, expected: Amount(MaxSatoshi), }, { - name: "min", + name: "min producable", amount: -21e6, valid: true, expected: Amount(-MaxSatoshi), }, + { + name: "exceeds max producable", + amount: 21e6 + 1e-8, + valid: true, + expected: Amount(MaxSatoshi + 1), + }, + { + name: "exceeds min producable", + amount: -21e6 - 1e-8, + valid: true, + expected: Amount(-MaxSatoshi - 1), + }, { name: "one hundred", amount: 100, @@ -62,16 +74,6 @@ func TestAmountCreation(t *testing.T) { }, // Negative tests. - { - name: "exceeds max", - amount: 21e6 + 1, - valid: false, - }, - { - name: "exceeds min", - amount: -21e6 - 1, - valid: false, - }, { name: "not-a-number", amount: math.NaN(), diff --git a/test_coverage.txt b/test_coverage.txt index e827b37..ac55a98 100644 --- a/test_coverage.txt +++ b/test_coverage.txt @@ -3,20 +3,20 @@ github.com/conformal/btcutil/base58.go Base58Decode 100.00% (20/20) github.com/conformal/btcutil/base58.go Base58Encode 100.00% (15/15) github.com/conformal/btcutil/block.go Block.Tx 100.00% (12/12) github.com/conformal/btcutil/block.go Block.Transactions 100.00% (11/11) +github.com/conformal/btcutil/amount.go NewAmount 100.00% (9/9) github.com/conformal/btcutil/address.go encodeAddress 100.00% (9/9) github.com/conformal/btcutil/amount.go AmountUnit.String 100.00% (8/8) -github.com/conformal/btcutil/amount.go NewAmount 100.00% (8/8) github.com/conformal/btcutil/block.go NewBlockFromBytes 100.00% (7/7) github.com/conformal/btcutil/tx.go NewTxFromBytes 100.00% (7/7) -github.com/conformal/btcutil/block.go Block.Sha 100.00% (5/5) github.com/conformal/btcutil/tx.go Tx.Sha 100.00% (5/5) +github.com/conformal/btcutil/address.go checkBitcoinNet 100.00% (5/5) +github.com/conformal/btcutil/block.go Block.Sha 100.00% (5/5) github.com/conformal/btcutil/amount.go Amount.Format 100.00% (2/2) github.com/conformal/btcutil/address.go NewAddressScriptHash 100.00% (2/2) github.com/conformal/btcutil/hash160.go calcHash 100.00% (2/2) github.com/conformal/btcutil/block.go Block.MsgBlock 100.00% (1/1) github.com/conformal/btcutil/address.go AddressPubKeyHash.EncodeAddress 100.00% (1/1) github.com/conformal/btcutil/address.go AddressPubKeyHash.ScriptAddress 100.00% (1/1) -github.com/conformal/btcutil/address.go AddressPubKeyHash.String 100.00% (1/1) github.com/conformal/btcutil/address.go AddressScriptHash.EncodeAddress 100.00% (1/1) github.com/conformal/btcutil/address.go AddressScriptHash.ScriptAddress 100.00% (1/1) github.com/conformal/btcutil/address.go AddressScriptHash.String 100.00% (1/1) @@ -35,25 +35,25 @@ github.com/conformal/btcutil/tx.go Tx.MsgTx 100.00% (1/1) github.com/conformal/btcutil/tx.go Tx.Index 100.00% (1/1) github.com/conformal/btcutil/tx.go Tx.SetIndex 100.00% (1/1) github.com/conformal/btcutil/tx.go NewTx 100.00% (1/1) +github.com/conformal/btcutil/address.go AddressPubKeyHash.String 100.00% (1/1) github.com/conformal/btcutil/address.go DecodeAddress 95.65% (22/23) github.com/conformal/btcutil/appdata.go appDataDir 92.00% (23/25) github.com/conformal/btcutil/address.go NewAddressScriptHashFromHash 91.67% (11/12) github.com/conformal/btcutil/address.go NewAddressPubKeyHash 91.67% (11/12) github.com/conformal/btcutil/address.go EncodePrivateKey 90.91% (20/22) -github.com/conformal/btcutil/block.go Block.Bytes 88.89% (8/9) github.com/conformal/btcutil/block.go Block.TxLoc 88.89% (8/9) +github.com/conformal/btcutil/block.go Block.Bytes 88.89% (8/9) github.com/conformal/btcutil/address.go AddressPubKey.serialize 85.71% (6/7) github.com/conformal/btcutil/address.go DecodePrivateKey 83.33% (20/24) github.com/conformal/btcutil/address.go NewAddressPubKey 83.33% (15/18) -github.com/conformal/btcutil/address.go checkBitcoinNet 83.33% (5/6) github.com/conformal/btcutil/block.go Block.TxSha 75.00% (3/4) -github.com/conformal/btcutil/address.go AddressScriptHash.IsForNet 60.00% (3/5) -github.com/conformal/btcutil/address.go AddressPubKeyHash.IsForNet 60.00% (3/5) github.com/conformal/btcutil/address.go AddressPubKey.IsForNet 60.00% (3/5) +github.com/conformal/btcutil/address.go AddressPubKeyHash.IsForNet 60.00% (3/5) +github.com/conformal/btcutil/address.go AddressScriptHash.IsForNet 60.00% (3/5) github.com/conformal/btcutil/certgen.go NewTLSCertPair 0.00% (0/50) github.com/conformal/btcutil/address.go AddressPubKey.AddressPubKeyHash 0.00% (0/3) github.com/conformal/btcutil/appdata.go AppDataDir 0.00% (0/1) -github.com/conformal/btcutil/address.go AddressPubKey.Format 0.00% (0/1) github.com/conformal/btcutil/address.go AddressPubKey.SetFormat 0.00% (0/1) -github.com/conformal/btcutil ------------------------------- 78.51% (296/377) +github.com/conformal/btcutil/address.go AddressPubKey.Format 0.00% (0/1) +github.com/conformal/btcutil ------------------------------- 78.78% (297/377)