Skip to content

Commit

Permalink
Add script verification flags for DER format
Browse files Browse the repository at this point in the history 
Add script verification flags for DER format encoding, low-S signatures
and strict encoding.
Pass script verification flags through to signature parser.
Convert block verification flags to a sub-enum of Block.
Remove DER signature format verification flag from block flags.
Add test transaction with a non-standard DER signature, with the verify
flag set/unset accordingly, to tx_invalid.json and tx_valid.json
  • Loading branch information
@schildbach
Ross Nicoll authored and schildbach committed Oct 20, 2015
1 parent 1c74aac commit 829e147
Show file tree
Hide file tree
Showing 13 changed files with 135 additions and 75 deletions.
8 changes: 6 additions & 2 deletions core/src/main/java/org/bitcoinj/core/AbstractBlockChain.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,7 @@ private boolean add(Block block, boolean tryConnecting,

final StoredBlock storedPrev;
final int height;
final EnumSet<VerificationFlags> flags;
final EnumSet<Block.VerifyFlag> flags;

// Prove the block is internally valid: hash is lower than target, etc. This only checks the block contents
// if there is a tx sending or receiving coins using an address in one of our wallets. And those transactions
Expand All @@ -465,7 +465,7 @@ private boolean add(Block block, boolean tryConnecting,
} else {
height = Block.BLOCK_HEIGHT_UNKNOWN;
}
flags = params.getValidationFlags(block, versionTally, height);
flags = params.getBlockVerificationFlags(block, versionTally, height);
if (shouldVerifyTransactions())
block.verifyTransactions(height, flags);
} catch (VerificationException e) {
Expand Down Expand Up @@ -1073,4 +1073,8 @@ public void resetFalsePositiveEstimate() {
falsePositiveTrend = 0;
previousFalsePositiveRate = 0;
}

protected VersionTally getVersionTally() {
return versionTally;
}
}
17 changes: 13 additions & 4 deletions core/src/main/java/org/bitcoinj/core/Block.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,15 @@
* specifically using {@link Peer#getBlock(Sha256Hash)}, or grab one from a downloaded {@link BlockChain}.
*/
public class Block extends Message {
/**
* Flags used to control which elements of block validation are done on
* received blocks.
*/
public enum VerifyFlag {
/** Check that block height is in coinbase transaction (BIP 34). */
HEIGHT_IN_COINBASE
}

private static final Logger log = LoggerFactory.getLogger(Block.class);

/** How many bytes are required to represent a block header WITHOUT the trailing 00 length byte. */
Expand Down Expand Up @@ -631,12 +640,12 @@ private List<byte[]> buildMerkleTree() {
* to validate the coinbase input script of v2 and above blocks.
* @throws VerificationException if there was an error verifying the block.
*/
private void checkTransactions(final int height, final EnumSet<VerificationFlags> flags)
private void checkTransactions(final int height, final EnumSet<VerifyFlag> flags)
throws VerificationException {
// The first transaction in a block must always be a coinbase transaction.
if (!transactions.get(0).isCoinBase())
throw new VerificationException("First tx is not coinbase");
if (flags.contains(VerificationFlags.HEIGHT_IN_COINBASE) && height >= BLOCK_HEIGHT_GENESIS) {
if (flags.contains(Block.VerifyFlag.HEIGHT_IN_COINBASE) && height >= BLOCK_HEIGHT_GENESIS) {
transactions.get(0).checkCoinBaseHeight(height);
}
// The rest must not be.
Expand Down Expand Up @@ -673,7 +682,7 @@ public void verifyHeader() throws VerificationException {
* whether to test for height in the coinbase transaction).
* @throws VerificationException if there was an error verifying the block.
*/
public void verifyTransactions(final int height, final EnumSet<VerificationFlags> flags) throws VerificationException {
public void verifyTransactions(final int height, final EnumSet<VerifyFlag> flags) throws VerificationException {
// Now we need to check that the body of the block actually matches the headers. The network won't generate
// an invalid block, but if we didn't validate this then an untrusted man-in-the-middle could obtain the next
// valid block from the network and simply replace the transactions in it with their own fictional
Expand All @@ -697,7 +706,7 @@ public void verifyTransactions(final int height, final EnumSet<VerificationFlags
* whether to test for height in the coinbase transaction).
* @throws VerificationException if there was an error verifying the block.
*/
public void verify(final int height, final EnumSet<VerificationFlags> flags) throws VerificationException {
public void verify(final int height, final EnumSet<VerifyFlag> flags) throws VerificationException {
verifyHeader();
verifyTransactions(height, flags);
}
Expand Down
16 changes: 8 additions & 8 deletions core/src/main/java/org/bitcoinj/core/FullPrunedBlockChain.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

package org.bitcoinj.core;

import org.bitcoinj.core.listeners.TransactionReceivedInBlockListener;
import org.bitcoinj.script.Script;
import org.bitcoinj.script.Script.VerifyFlag;
import org.bitcoinj.store.BlockStoreException;
Expand All @@ -28,7 +27,6 @@

import javax.annotation.Nullable;
import java.util.ArrayList;
import java.util.EnumSet;
import java.util.LinkedList;
import java.util.List;
import java.util.ListIterator;
Expand Down Expand Up @@ -221,9 +219,6 @@ protected TransactionOutputChanges connectTransactions(int height, Block block)
LinkedList<UTXO> txOutsSpent = new LinkedList<UTXO>();
LinkedList<UTXO> txOutsCreated = new LinkedList<UTXO>();
long sigOps = 0;
final Set<VerifyFlag> verifyFlags = EnumSet.noneOf(VerifyFlag.class);
if (block.getTimeSeconds() >= NetworkParameters.BIP16_ENFORCE_TIME)
verifyFlags.add(VerifyFlag.P2SH);

if (scriptVerificationExecutor.isShutdown())
scriptVerificationExecutor = Executors.newFixedThreadPool(Runtime.getRuntime().availableProcessors());
Expand All @@ -235,6 +230,7 @@ protected TransactionOutputChanges connectTransactions(int height, Block block)
// checkpoints list and we therefore only check non-checkpoints for duplicated transactions here. See the
// BIP30 document for more details on this: https://github.com/bitcoin/bips/blob/master/bip-0030.mediawiki
for (Transaction tx : block.transactions) {
final Set<VerifyFlag> verifyFlags = params.getTransactionVerificationFlags(block, tx, getVersionTally(), height);
Sha256Hash hash = tx.getHash();
// If we already have unspent outputs for this hash, we saw the tx already. Either the block is
// being added twice (bug) or the block is a BIP30 violator.
Expand All @@ -251,6 +247,7 @@ protected TransactionOutputChanges connectTransactions(int height, Block block)
Coin valueIn = Coin.ZERO;
Coin valueOut = Coin.ZERO;
final List<Script> prevOutScripts = new LinkedList<Script>();
final Set<VerifyFlag> verifyFlags = params.getTransactionVerificationFlags(block, tx, getVersionTally(), height);
if (!isCoinBase) {
// For each input of the transaction remove the corresponding output from the set of unspent
// outputs.
Expand Down Expand Up @@ -366,9 +363,7 @@ protected synchronized TransactionOutputChanges connectTransactions(StoredBlock
LinkedList<UTXO> txOutsSpent = new LinkedList<UTXO>();
LinkedList<UTXO> txOutsCreated = new LinkedList<UTXO>();
long sigOps = 0;
final Set<VerifyFlag> verifyFlags = EnumSet.noneOf(VerifyFlag.class);
if (newBlock.getHeader().getTimeSeconds() >= NetworkParameters.BIP16_ENFORCE_TIME)
verifyFlags.add(VerifyFlag.P2SH);

if (!params.isCheckpoint(newBlock.getHeight())) {
for (Transaction tx : transactions) {
Sha256Hash hash = tx.getHash();
Expand All @@ -383,10 +378,13 @@ protected synchronized TransactionOutputChanges connectTransactions(StoredBlock
scriptVerificationExecutor = Executors.newFixedThreadPool(Runtime.getRuntime().availableProcessors());
List<Future<VerificationException>> listScriptVerificationResults = new ArrayList<Future<VerificationException>>(transactions.size());
for (final Transaction tx : transactions) {
final Set<VerifyFlag> verifyFlags =
params.getTransactionVerificationFlags(newBlock.getHeader(), tx, getVersionTally(), Integer.SIZE);
boolean isCoinBase = tx.isCoinBase();
Coin valueIn = Coin.ZERO;
Coin valueOut = Coin.ZERO;
final List<Script> prevOutScripts = new LinkedList<Script>();

if (!isCoinBase) {
for (int index = 0; index < tx.getInputs().size(); index++) {
final TransactionInput in = tx.getInputs().get(index);
Expand All @@ -404,6 +402,8 @@ protected synchronized TransactionOutputChanges connectTransactions(StoredBlock
throw new VerificationException("Too many P2SH SigOps in block");
}

// TODO: Enforce DER signature format

prevOutScripts.add(prevOut.getScript());

blockStore.removeUnspentTransactionOutput(prevOut);
Expand Down
30 changes: 21 additions & 9 deletions core/src/main/java/org/bitcoinj/core/NetworkParameters.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -487,22 +487,34 @@ public int getMajorityWindow() {
* @param height height of the block, if known, null otherwise. Returned
* tests should be a safe subset if block height is unknown.
*/
public EnumSet<VerificationFlags> getValidationFlags(final Block block,
public EnumSet<Block.VerifyFlag> getBlockVerificationFlags(final Block block,
final VersionTally tally, final Integer height) {
final EnumSet<VerificationFlags> flags = EnumSet.noneOf(VerificationFlags.class);
final EnumSet<Block.VerifyFlag> flags = EnumSet.noneOf(Block.VerifyFlag.class);

if (block.getVersion() >= Block.BLOCK_VERSION_BIP34) {
final Integer count = tally.getCountAtOrAbove(Block.BLOCK_VERSION_BIP34);
if (null != count && count >= getMajorityEnforceBlockUpgrade()) {
flags.add(VerificationFlags.HEIGHT_IN_COINBASE);
}
}
if (block.getVersion() >= Block.BLOCK_VERSION_BIP66) {
final Integer count = tally.getCountAtOrAbove(Block.BLOCK_VERSION_BIP66);
if (null != count && count >= getMajorityEnforceBlockUpgrade()) {
flags.add(VerificationFlags.DER_SIGNATURE_FORMAT);
flags.add(Block.VerifyFlag.HEIGHT_IN_COINBASE);
}
}
return flags;
}

/**
* The flags indicating which script validation tests should be applied to
* the given transaction. Enables support for alternative blockchains which enable
* tests based on different criteria.
*
* @param block block the transaction belongs to.
* @param transaction to determine flags for.
* @param height height of the block, if known, null otherwise. Returned
* tests should be a safe subset if block height is unknown.
*/
public EnumSet<Script.VerifyFlag> getTransactionVerificationFlags(final Block block,
final Transaction transaction, final VersionTally tally, final Integer height) {
final EnumSet<Script.VerifyFlag> verifyFlags = EnumSet.noneOf(Script.VerifyFlag.class);
if (block.getTimeSeconds() >= NetworkParameters.BIP16_ENFORCE_TIME)
verifyFlags.add(Script.VerifyFlag.P2SH);
return verifyFlags;
}
}
27 changes: 0 additions & 27 deletions core/src/main/java/org/bitcoinj/core/VerificationFlags.java
View file

This file was deleted.

4 changes: 0 additions & 4 deletions core/src/main/java/org/bitcoinj/params/AbstractBitcoinNetParams.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@
package org.bitcoinj.params;

import java.math.BigInteger;
import java.util.EnumSet;
import java.util.Set;

import org.bitcoinj.core.Block;
import org.bitcoinj.core.Coin;
Expand All @@ -35,8 +33,6 @@
import org.slf4j.LoggerFactory;

import org.bitcoinj.core.BitcoinSerializer;
import org.bitcoinj.core.VerificationFlags;
import org.bitcoinj.utils.VersionTally;

/**
* Parameters for Bitcoin-like networks.
Expand Down

0 comments on commit 829e147

Please sign in to comment.