From 4a3d702397598fc2f217cc7b9da6782f8500f606 Mon Sep 17 00:00:00 2001 From: Ross Nicoll Date: Sun, 18 Oct 2015 15:17:12 +0100 Subject: [PATCH] Correct coinbase height script validation Changes coinbase height validation to check for the bytes at the start of the coinbase script, and ignore everything after them. The validation then matches Bitcoin Core. Also update error messages to more closely match latest Bitcoin Core. --- .../java/org/bitcoinj/core/Transaction.java | 28 +++++++------------ .../org/bitcoinj/core/TransactionTest.java | 22 ++++++++++++--- 2 files changed, 28 insertions(+), 22 deletions(-) diff --git a/core/src/main/java/org/bitcoinj/core/Transaction.java b/core/src/main/java/org/bitcoinj/core/Transaction.java index 7cea2009..1bb55f4b 100644 --- a/core/src/main/java/org/bitcoinj/core/Transaction.java +++ b/core/src/main/java/org/bitcoinj/core/Transaction.java @@ -1177,25 +1177,17 @@ public class Transaction extends ChildMessage { // Check block height is in coinbase input script final TransactionInput in = this.getInputs().get(0); - final List chunks; - - try { - final Script scriptSig = in.getScriptSig(); - chunks = scriptSig.getChunks(); - } catch(ScriptException e) { - throw new VerificationException("Coinbase input script signature is invalid.", e); + final ScriptBuilder builder = new ScriptBuilder(); + builder.data(ScriptBuilder.createHeightScriptData(height)); + final byte[] expected = builder.build().getProgram(); + final byte[] actual = in.getScriptBytes(); + if (actual.length < expected.length) { + throw new VerificationException.CoinbaseHeightMismatch("Block height mismatch in coinbase."); } - if (chunks.isEmpty()) { - throw new VerificationException("Coinbase input script signature is empty."); - } - final ScriptChunk chunk = chunks.get(0); - if (!chunk.isPushData()) { - throw new VerificationException("First element of coinbase input script signature is not pushdata."); - } - final byte[] data = chunk.data; - final byte[] expected = ScriptBuilder.createHeightScriptData(height); - if (!Arrays.equals(data, expected)) { - throw new VerificationException.CoinbaseHeightMismatch("Coinbase height mismatch."); + for (int scriptIdx = 0; scriptIdx < expected.length; scriptIdx++) { + if (actual[scriptIdx] != expected[scriptIdx]) { + throw new VerificationException.CoinbaseHeightMismatch("Block height mismatch in coinbase."); + } } } diff --git a/core/src/test/java/org/bitcoinj/core/TransactionTest.java b/core/src/test/java/org/bitcoinj/core/TransactionTest.java index 812f063e..001dc28b 100644 --- a/core/src/test/java/org/bitcoinj/core/TransactionTest.java +++ b/core/src/test/java/org/bitcoinj/core/TransactionTest.java @@ -307,10 +307,6 @@ public class TransactionTest { assertEquals(79, tx1.getMessageSizeForPriorityCalc()); } - /** - * - * @throws VerificationException - */ @Test public void testCoinbaseHeightCheck() throws VerificationException { // Coinbase transaction from block 300,000 @@ -319,4 +315,22 @@ public class TransactionTest { final Transaction transaction = params.getDefaultSerializer().makeTransaction(transactionBytes); transaction.checkCoinBaseHeight(height); } + + /** + * Test a coinbase transaction whose script has nonsense after the block height. + * See https://github.com/bitcoinj/bitcoinj/issues/1097 + */ + @Test + public void testCoinbaseHeightCheckWithDamagedScript() throws VerificationException { + // Coinbase transaction from block 224,430 + final byte[] transactionBytes = HEX.decode( + "010000000100000000000000000000000000000000000000000000000000000000" + + "00000000ffffffff3b03ae6c0300044bd7031a0400000000522cfabe6d6d0000" + + "0000000000b7b8bf0100000068692066726f6d20706f6f6c7365727665726aac" + + "1eeeed88ffffffff01e0587597000000001976a91421c0d001728b3feaf11551" + + "5b7c135e779e9f442f88ac00000000"); + final int height = 224430; + final Transaction transaction = params.getDefaultSerializer().makeTransaction(transactionBytes); + transaction.checkCoinBaseHeight(height); + } } \ No newline at end of file