From 5ca6f13195425afd08f09f2e2f66f022d3122179 Mon Sep 17 00:00:00 2001 From: Andreas Schildbach Date: Thu, 6 Feb 2014 12:12:01 +0100 Subject: [PATCH] Replace usage of BigInteger.compareTo(BigInteger.ZERO) with BigInteger.signum() as it's easier to read and more performant. Passes all unit tests. --- .../java/com/google/bitcoin/core/Block.java | 2 +- .../java/com/google/bitcoin/core/ECKey.java | 4 ++-- .../bitcoin/core/FullPrunedBlockChain.java | 4 ++-- .../com/google/bitcoin/core/Transaction.java | 2 +- .../bitcoin/core/TransactionOutput.java | 2 +- .../java/com/google/bitcoin/core/Utils.java | 6 +++--- .../java/com/google/bitcoin/core/Wallet.java | 20 +++++++++---------- .../channels/PaymentChannelClientState.java | 8 ++++---- .../channels/PaymentChannelServer.java | 2 +- .../channels/PaymentChannelServerState.java | 4 ++-- .../StoredPaymentChannelClientStates.java | 4 ++-- .../StoredPaymentChannelServerStates.java | 2 +- .../com/google/bitcoin/script/Script.java | 2 +- .../com/google/bitcoin/uri/BitcoinURI.java | 2 +- .../google/bitcoin/core/BlockChainTest.java | 2 +- .../store/WalletProtobufSerializerTest.java | 4 ++-- 16 files changed, 35 insertions(+), 35 deletions(-) diff --git a/core/src/main/java/com/google/bitcoin/core/Block.java b/core/src/main/java/com/google/bitcoin/core/Block.java index d255c323..94fe861f 100644 --- a/core/src/main/java/com/google/bitcoin/core/Block.java +++ b/core/src/main/java/com/google/bitcoin/core/Block.java @@ -627,7 +627,7 @@ public class Block extends Message { public BigInteger getDifficultyTargetAsInteger() throws VerificationException { maybeParseHeader(); BigInteger target = Utils.decodeCompactBits(difficultyTarget); - if (target.compareTo(BigInteger.ZERO) <= 0 || target.compareTo(params.proofOfWorkLimit) > 0) + if (target.signum() <= 0 || target.compareTo(params.proofOfWorkLimit) > 0) throw new VerificationException("Difficulty target is bad: " + target.toString()); return target; } diff --git a/core/src/main/java/com/google/bitcoin/core/ECKey.java b/core/src/main/java/com/google/bitcoin/core/ECKey.java index b18bbbbd..d339d99e 100644 --- a/core/src/main/java/com/google/bitcoin/core/ECKey.java +++ b/core/src/main/java/com/google/bitcoin/core/ECKey.java @@ -707,8 +707,8 @@ public class ECKey implements Serializable { @Nullable public static ECKey recoverFromSignature(int recId, ECDSASignature sig, Sha256Hash message, boolean compressed) { Preconditions.checkArgument(recId >= 0, "recId must be positive"); - Preconditions.checkArgument(sig.r.compareTo(BigInteger.ZERO) >= 0, "r must be positive"); - Preconditions.checkArgument(sig.s.compareTo(BigInteger.ZERO) >= 0, "s must be positive"); + Preconditions.checkArgument(sig.r.signum() >= 0, "r must be positive"); + Preconditions.checkArgument(sig.s.signum() >= 0, "s must be positive"); Preconditions.checkNotNull(message); // 1.0 For j from 0 to h (h == recId here and the loop is outside this function) // 1.1 Let x = r + jn diff --git a/core/src/main/java/com/google/bitcoin/core/FullPrunedBlockChain.java b/core/src/main/java/com/google/bitcoin/core/FullPrunedBlockChain.java index d5cd3ee0..acda43ef 100644 --- a/core/src/main/java/com/google/bitcoin/core/FullPrunedBlockChain.java +++ b/core/src/main/java/com/google/bitcoin/core/FullPrunedBlockChain.java @@ -224,7 +224,7 @@ public class FullPrunedBlockChain extends AbstractBlockChain { } // All values were already checked for being non-negative (as it is verified in Transaction.verify()) // but we check again here just for defence in depth. Transactions with zero output value are OK. - if (valueOut.compareTo(BigInteger.ZERO) < 0 || valueOut.compareTo(params.MAX_MONEY) > 0) + if (valueOut.signum() < 0 || valueOut.compareTo(params.MAX_MONEY) > 0) throw new VerificationException("Transaction output value out of rage"); if (isCoinBase) { coinbaseValue = valueOut; @@ -346,7 +346,7 @@ public class FullPrunedBlockChain extends AbstractBlockChain { } // All values were already checked for being non-negative (as it is verified in Transaction.verify()) // but we check again here just for defence in depth. Transactions with zero output value are OK. - if (valueOut.compareTo(BigInteger.ZERO) < 0 || valueOut.compareTo(params.MAX_MONEY) > 0) + if (valueOut.signum() < 0 || valueOut.compareTo(params.MAX_MONEY) > 0) throw new VerificationException("Transaction output value out of rage"); if (isCoinBase) { coinbaseValue = valueOut; diff --git a/core/src/main/java/com/google/bitcoin/core/Transaction.java b/core/src/main/java/com/google/bitcoin/core/Transaction.java index 9ff5fb9e..25c4ab6d 100644 --- a/core/src/main/java/com/google/bitcoin/core/Transaction.java +++ b/core/src/main/java/com/google/bitcoin/core/Transaction.java @@ -1184,7 +1184,7 @@ public class Transaction extends ChildMessage implements Serializable { BigInteger valueOut = BigInteger.ZERO; for (TransactionOutput output : outputs) { - if (output.getValue().compareTo(BigInteger.ZERO) < 0) + if (output.getValue().signum() < 0) throw new VerificationException("Transaction output negative"); valueOut = valueOut.add(output.getValue()); } diff --git a/core/src/main/java/com/google/bitcoin/core/TransactionOutput.java b/core/src/main/java/com/google/bitcoin/core/TransactionOutput.java index 8257a4f6..df8c22ed 100644 --- a/core/src/main/java/com/google/bitcoin/core/TransactionOutput.java +++ b/core/src/main/java/com/google/bitcoin/core/TransactionOutput.java @@ -109,7 +109,7 @@ public class TransactionOutput extends ChildMessage implements Serializable { super(params); // Negative values obviously make no sense, except for -1 which is used as a sentinel value when calculating // SIGHASH_SINGLE signatures, so unfortunately we have to allow that here. - checkArgument(value.compareTo(BigInteger.ZERO) >= 0 || value.equals(Utils.NEGATIVE_ONE), "Negative values not allowed"); + checkArgument(value.signum() >= 0 || value.equals(Utils.NEGATIVE_ONE), "Negative values not allowed"); checkArgument(value.compareTo(NetworkParameters.MAX_MONEY) < 0, "Values larger than MAX_MONEY not allowed"); this.value = value; this.scriptBytes = scriptBytes; diff --git a/core/src/main/java/com/google/bitcoin/core/Utils.java b/core/src/main/java/com/google/bitcoin/core/Utils.java index b4a11e44..b91a0981 100644 --- a/core/src/main/java/com/google/bitcoin/core/Utils.java +++ b/core/src/main/java/com/google/bitcoin/core/Utils.java @@ -120,7 +120,7 @@ public class Utils { */ public static BigInteger toNanoCoins(String coins) { BigInteger bigint = new BigDecimal(coins).movePointRight(8).toBigIntegerExact(); - if (bigint.compareTo(BigInteger.ZERO) < 0) + if (bigint.signum() < 0) throw new ArithmeticException("Negative coins specified"); if (bigint.compareTo(NetworkParameters.MAX_MONEY) > 0) throw new ArithmeticException("Amount larger than the total quantity of Bitcoins possible specified."); @@ -331,7 +331,7 @@ public class Utils { */ public static String bitcoinValueToFriendlyString(BigInteger value) { // TODO: This API is crap. This method should go away when we encapsulate money values. - boolean negative = value.compareTo(BigInteger.ZERO) < 0; + boolean negative = value.signum() < 0; if (negative) value = value.negate(); BigDecimal bd = new BigDecimal(value, 8); @@ -404,7 +404,7 @@ public class Utils { else return new byte[] {0x00, 0x00, 0x00, 0x00}; } - boolean isNegative = value.compareTo(BigInteger.ZERO) < 0; + boolean isNegative = value.signum() < 0; if (isNegative) value = value.negate(); byte[] array = value.toByteArray(); diff --git a/core/src/main/java/com/google/bitcoin/core/Wallet.java b/core/src/main/java/com/google/bitcoin/core/Wallet.java index 29a28a6c..4b3c18c4 100644 --- a/core/src/main/java/com/google/bitcoin/core/Wallet.java +++ b/core/src/main/java/com/google/bitcoin/core/Wallet.java @@ -732,8 +732,8 @@ public class Wallet implements Serializable, BlockChainListener, PeerFilterProvi public boolean isTransactionRelevant(Transaction tx) throws ScriptException { lock.lock(); try { - return tx.getValueSentFromMe(this).compareTo(BigInteger.ZERO) > 0 || - tx.getValueSentToMe(this).compareTo(BigInteger.ZERO) > 0 || + return tx.getValueSentFromMe(this).signum() > 0 || + tx.getValueSentToMe(this).signum() > 0 || checkForDoubleSpendAgainstPending(tx, false); } finally { lock.unlock(); @@ -901,7 +901,7 @@ public class Wallet implements Serializable, BlockChainListener, PeerFilterProvi BigInteger newBalance = getBalance(); // This is slow. log.info("Balance is now: " + bitcoinValueToFriendlyString(newBalance)); if (!wasPending) { - int diff = valueDifference.compareTo(BigInteger.ZERO); + int diff = valueDifference.signum(); // We pick one callback based on the value difference, though a tx can of course both send and receive // coins from the wallet. if (diff > 0) { @@ -1000,7 +1000,7 @@ public class Wallet implements Serializable, BlockChainListener, PeerFilterProvi // Now make sure it ends up in the right pool. Also, handle the case where this TX is double-spending // against our pending transactions. Note that a tx may double spend our pending transactions and also send // us money/spend our money. - boolean hasOutputsToMe = tx.getValueSentToMe(this, true).compareTo(BigInteger.ZERO) > 0; + boolean hasOutputsToMe = tx.getValueSentToMe(this, true).signum() > 0; if (hasOutputsToMe) { // Needs to go into either unspent or spent (if the outputs were already spent by a pending tx). if (tx.isEveryOwnedOutputSpent(this)) { @@ -1010,7 +1010,7 @@ public class Wallet implements Serializable, BlockChainListener, PeerFilterProvi log.info(" tx {} ->unspent", tx.getHashAsString()); addWalletTransaction(Pool.UNSPENT, tx); } - } else if (tx.getValueSentFromMe(this).compareTo(BigInteger.ZERO) > 0) { + } else if (tx.getValueSentFromMe(this).signum() > 0) { // Didn't send us any money, but did spend some. Keep it around for record keeping purposes. log.info(" tx {} ->spent", tx.getHashAsString()); addWalletTransaction(Pool.SPENT, tx); @@ -1224,11 +1224,11 @@ public class Wallet implements Serializable, BlockChainListener, PeerFilterProvi BigInteger valueSentFromMe = tx.getValueSentFromMe(this); BigInteger valueSentToMe = tx.getValueSentToMe(this); BigInteger newBalance = balance.add(valueSentToMe).subtract(valueSentFromMe); - if (valueSentToMe.compareTo(BigInteger.ZERO) > 0) { + if (valueSentToMe.signum() > 0) { checkBalanceFuturesLocked(null); queueOnCoinsReceived(tx, balance, newBalance); } - if (valueSentFromMe.compareTo(BigInteger.ZERO) > 0) + if (valueSentFromMe.signum() > 0) queueOnCoinsSent(tx, balance, newBalance); maybeQueueOnWalletChanged(); @@ -1876,7 +1876,7 @@ public class Wallet implements Serializable, BlockChainListener, PeerFilterProvi log.info(" with {} coins change", bitcoinValueToFriendlyString(bestChangeOutput.getValue())); } final BigInteger calculatedFee = totalInput.subtract(totalOutput); - if (calculatedFee.compareTo(BigInteger.ZERO) > 0) { + if (calculatedFee.signum() > 0) { log.info(" with a fee of {}", bitcoinValueToFriendlyString(calculatedFee)); } @@ -3411,7 +3411,7 @@ public class Wallet implements Serializable, BlockChainListener, PeerFilterProvi int size = 0; TransactionOutput changeOutput = null; - if (change.compareTo(BigInteger.ZERO) > 0) { + if (change.signum() > 0) { // The value of the inputs is greater than what we want to send. Just like in real life then, // we need to take back some coins ... this is called "change". Add another output that sends the change // back to us. The address comes either from the request or getChangeAddress() as a default. @@ -3450,7 +3450,7 @@ public class Wallet implements Serializable, BlockChainListener, PeerFilterProvi // include things we haven't added yet like input signatures/scripts or the change output. size += req.tx.bitcoinSerialize().length; size += estimateBytesForSigning(selection); - if (size/1000 > lastCalculatedSize/1000 && req.feePerKb.compareTo(BigInteger.ZERO) > 0) { + if (size/1000 > lastCalculatedSize/1000 && req.feePerKb.signum() > 0) { lastCalculatedSize = size; // We need more fees anyway, just try again with the same additional value additionalValueForNextCategory = additionalValueSelected; diff --git a/core/src/main/java/com/google/bitcoin/protocols/channels/PaymentChannelClientState.java b/core/src/main/java/com/google/bitcoin/protocols/channels/PaymentChannelClientState.java index 40a68f52..89bea849 100644 --- a/core/src/main/java/com/google/bitcoin/protocols/channels/PaymentChannelClientState.java +++ b/core/src/main/java/com/google/bitcoin/protocols/channels/PaymentChannelClientState.java @@ -157,7 +157,7 @@ public class PaymentChannelClientState { */ public PaymentChannelClientState(Wallet wallet, ECKey myKey, ECKey serverMultisigKey, BigInteger value, long expiryTimeInSeconds) throws VerificationException { - checkArgument(value.compareTo(BigInteger.ZERO) > 0); + checkArgument(value.signum() > 0); this.wallet = checkNotNull(wallet); initWalletListeners(); this.serverMultisigKey = checkNotNull(serverMultisigKey); @@ -396,15 +396,15 @@ public class PaymentChannelClientState { checkState(state == State.READY); checkNotExpired(); checkNotNull(size); // Validity of size will be checked by makeUnsignedChannelContract. - if (size.compareTo(BigInteger.ZERO) < 0) + if (size.signum() < 0) throw new ValueOutOfRangeException("Tried to decrement payment"); BigInteger newValueToMe = valueToMe.subtract(size); - if (newValueToMe.compareTo(Transaction.MIN_NONDUST_OUTPUT) < 0 && newValueToMe.compareTo(BigInteger.ZERO) > 0) { + if (newValueToMe.compareTo(Transaction.MIN_NONDUST_OUTPUT) < 0 && newValueToMe.signum() > 0) { log.info("New value being sent back as change was smaller than minimum nondust output, sending all"); size = valueToMe; newValueToMe = BigInteger.ZERO; } - if (newValueToMe.compareTo(BigInteger.ZERO) < 0) + if (newValueToMe.signum() < 0) throw new ValueOutOfRangeException("Channel has too little money to pay " + size + " satoshis"); Transaction tx = makeUnsignedChannelContract(newValueToMe); log.info("Signing new payment tx {}", tx); diff --git a/core/src/main/java/com/google/bitcoin/protocols/channels/PaymentChannelServer.java b/core/src/main/java/com/google/bitcoin/protocols/channels/PaymentChannelServer.java index bbd8968c..86275759 100644 --- a/core/src/main/java/com/google/bitcoin/protocols/channels/PaymentChannelServer.java +++ b/core/src/main/java/com/google/bitcoin/protocols/channels/PaymentChannelServer.java @@ -315,7 +315,7 @@ public class PaymentChannelServer { boolean stillUsable = state.incrementPayment(refundSize, msg.getSignature().toByteArray()); BigInteger bestPaymentChange = state.getBestValueToMe().subtract(lastBestPayment); - if (bestPaymentChange.compareTo(BigInteger.ZERO) > 0) + if (bestPaymentChange.signum() > 0) conn.paymentIncrease(bestPaymentChange, state.getBestValueToMe()); if (sendAck) { diff --git a/core/src/main/java/com/google/bitcoin/protocols/channels/PaymentChannelServerState.java b/core/src/main/java/com/google/bitcoin/protocols/channels/PaymentChannelServerState.java index a83437bc..5d72a106 100644 --- a/core/src/main/java/com/google/bitcoin/protocols/channels/PaymentChannelServerState.java +++ b/core/src/main/java/com/google/bitcoin/protocols/channels/PaymentChannelServerState.java @@ -227,7 +227,7 @@ public class PaymentChannelServerState { throw new VerificationException("Multisig contract's first output was not a standard 2-of-2 multisig to client and server in that order."); this.totalValue = multisigContract.getOutput(0).getValue(); - if (this.totalValue.compareTo(BigInteger.ZERO) <= 0) + if (this.totalValue.signum() <= 0) throw new VerificationException("Not accepting an attempt to open a contract with zero value."); } catch (VerificationException e) { // We couldn't parse the multisig transaction or its output. @@ -294,7 +294,7 @@ public class PaymentChannelServerState { if (refundSize.compareTo(clientOutput.getMinNonDustValue()) < 0 && !fullyUsedUp) throw new ValueOutOfRangeException("Attempt to refund negative value or value too small to be accepted by the network"); BigInteger newValueToMe = totalValue.subtract(refundSize); - if (newValueToMe.compareTo(BigInteger.ZERO) < 0) + if (newValueToMe.signum() < 0) throw new ValueOutOfRangeException("Attempt to refund more than the contract allows."); if (newValueToMe.compareTo(bestValueToMe) < 0) throw new ValueOutOfRangeException("Attempt to roll back payment on the channel."); diff --git a/core/src/main/java/com/google/bitcoin/protocols/channels/StoredPaymentChannelClientStates.java b/core/src/main/java/com/google/bitcoin/protocols/channels/StoredPaymentChannelClientStates.java index 3d6efa2d..efaa9e12 100644 --- a/core/src/main/java/com/google/bitcoin/protocols/channels/StoredPaymentChannelClientStates.java +++ b/core/src/main/java/com/google/bitcoin/protocols/channels/StoredPaymentChannelClientStates.java @@ -220,8 +220,8 @@ public class StoredPaymentChannelClientStates implements WalletExtension { ClientState.StoredClientPaymentChannels.Builder builder = ClientState.StoredClientPaymentChannels.newBuilder(); for (StoredClientChannel channel : mapChannels.values()) { // First a few asserts to make sure things won't break - checkState(channel.valueToMe.compareTo(BigInteger.ZERO) >= 0 && channel.valueToMe.compareTo(NetworkParameters.MAX_MONEY) < 0); - checkState(channel.refundFees.compareTo(BigInteger.ZERO) >= 0 && channel.refundFees.compareTo(NetworkParameters.MAX_MONEY) < 0); + checkState(channel.valueToMe.signum() >= 0 && channel.valueToMe.compareTo(NetworkParameters.MAX_MONEY) < 0); + checkState(channel.refundFees.signum() >= 0 && channel.refundFees.compareTo(NetworkParameters.MAX_MONEY) < 0); checkNotNull(channel.myKey.getPrivKeyBytes()); checkState(channel.refund.getConfidence().getSource() == TransactionConfidence.Source.SELF); final ClientState.StoredClientPaymentChannel.Builder value = ClientState.StoredClientPaymentChannel.newBuilder() diff --git a/core/src/main/java/com/google/bitcoin/protocols/channels/StoredPaymentChannelServerStates.java b/core/src/main/java/com/google/bitcoin/protocols/channels/StoredPaymentChannelServerStates.java index 681e01f0..d12991c0 100644 --- a/core/src/main/java/com/google/bitcoin/protocols/channels/StoredPaymentChannelServerStates.java +++ b/core/src/main/java/com/google/bitcoin/protocols/channels/StoredPaymentChannelServerStates.java @@ -150,7 +150,7 @@ public class StoredPaymentChannelServerStates implements WalletExtension { ServerState.StoredServerPaymentChannels.Builder builder = ServerState.StoredServerPaymentChannels.newBuilder(); for (StoredServerChannel channel : mapChannels.values()) { // First a few asserts to make sure things won't break - checkState(channel.bestValueToMe.compareTo(BigInteger.ZERO) >= 0 && channel.bestValueToMe.compareTo(NetworkParameters.MAX_MONEY) < 0); + checkState(channel.bestValueToMe.signum() >= 0 && channel.bestValueToMe.compareTo(NetworkParameters.MAX_MONEY) < 0); checkState(channel.refundTransactionUnlockTimeSecs > 0); checkNotNull(channel.myKey.getPrivKeyBytes()); ServerState.StoredServerPaymentChannel.Builder channelBuilder = ServerState.StoredServerPaymentChannel.newBuilder() diff --git a/core/src/main/java/com/google/bitcoin/script/Script.java b/core/src/main/java/com/google/bitcoin/script/Script.java index 57b16f37..fec68a8c 100644 --- a/core/src/main/java/com/google/bitcoin/script/Script.java +++ b/core/src/main/java/com/google/bitcoin/script/Script.java @@ -883,7 +883,7 @@ public class Script { numericOPnum = numericOPnum.negate(); break; case OP_ABS: - if (numericOPnum.compareTo(BigInteger.ZERO) < 0) + if (numericOPnum.signum() < 0) numericOPnum = numericOPnum.negate(); break; case OP_NOT: diff --git a/core/src/main/java/com/google/bitcoin/uri/BitcoinURI.java b/core/src/main/java/com/google/bitcoin/uri/BitcoinURI.java index 4dccbcd4..14893034 100644 --- a/core/src/main/java/com/google/bitcoin/uri/BitcoinURI.java +++ b/core/src/main/java/com/google/bitcoin/uri/BitcoinURI.java @@ -326,7 +326,7 @@ public class BitcoinURI { public static String convertToBitcoinURI(String address, @Nullable BigInteger amount, @Nullable String label, @Nullable String message) { checkNotNull(address); - if (amount != null && amount.compareTo(BigInteger.ZERO) < 0) { + if (amount != null && amount.signum() < 0) { throw new IllegalArgumentException("Amount must be positive"); } diff --git a/core/src/test/java/com/google/bitcoin/core/BlockChainTest.java b/core/src/test/java/com/google/bitcoin/core/BlockChainTest.java index af3e5281..9b079623 100644 --- a/core/src/test/java/com/google/bitcoin/core/BlockChainTest.java +++ b/core/src/test/java/com/google/bitcoin/core/BlockChainTest.java @@ -127,7 +127,7 @@ public class BlockChainTest { wallet.getKeys().get(0).toAddress(unitTestParams)); Block b1 = createFakeBlock(blockStore, tx1).block; chain.add(b1); - assertTrue(wallet.getBalance().compareTo(BigInteger.ZERO) > 0); + assertTrue(wallet.getBalance().signum() > 0); } @Test diff --git a/core/src/test/java/com/google/bitcoin/store/WalletProtobufSerializerTest.java b/core/src/test/java/com/google/bitcoin/store/WalletProtobufSerializerTest.java index d1b43c12..9d441df0 100644 --- a/core/src/test/java/com/google/bitcoin/store/WalletProtobufSerializerTest.java +++ b/core/src/test/java/com/google/bitcoin/store/WalletProtobufSerializerTest.java @@ -181,11 +181,11 @@ public class WalletProtobufSerializerTest { // Start by building two blocks on top of the genesis block. Block b1 = params.getGenesisBlock().createNextBlock(myAddress); BigInteger work1 = b1.getWork(); - assertTrue(work1.compareTo(BigInteger.ZERO) > 0); + assertTrue(work1.signum() > 0); Block b2 = b1.createNextBlock(myAddress); BigInteger work2 = b2.getWork(); - assertTrue(work2.compareTo(BigInteger.ZERO) > 0); + assertTrue(work2.signum() > 0); assertTrue(chain.add(b1)); assertTrue(chain.add(b2));