Add SCRIPT_VERIFY_CLEANSTACK script verification flag #4311

1. 
   petertodd commented at 11:02 am on June 9, 2014: contributor

   Causes VerifyScript() to fail if scriptPubKey/redeemScript execution leaves more than a single stack item. This is a generic malleability fix that can be used with all scriptPubKeys, as per @sipa’s BIP62 thoughts. Of course, the current IsStandard() rules should only allow transactions satisfying this rule; I’m working on relaxing those rules.
2. Add SCRIPT_VERIFY_CLEANSTACK script verification flag

   Causes VerifyScript() to fail if scriptPubKey/redeemScript execution
   leaves more than a single stack item. This is a generic malleability fix
   that can be used with all scriptPubKeys.

   ce00fa1d60
3. 
   sipa commented at 7:26 am on June 10, 2014: member

   Untested ACK.
4. 
   sipa commented at 7:27 am on June 10, 2014: member

   Hmm, it seems pulltester relies on sending non-standard transactions, and seeing them end up in the mempool.
5. 
   mikehearn commented at 9:23 am on June 11, 2014: contributor

   Code is:

    0    Transaction b78tx = new Transaction(params);
    1    {
    2        b78tx.addOutput(new TransactionOutput(params, b78tx, ZERO, new byte[]{OP_TRUE}));
    3        addOnlyInputToTransaction(b78tx, new TransactionOutPointWithValue(
    4                new TransactionOutPoint(params, 1, b77.getTransactions().get(1).getHash()),
    5                SATOSHI, b77.getTransactions().get(1).getOutputs().get(1).getScriptPubKey()));
    6        b78.addTransaction(b78tx);
    7    }
    8    b78.solve();
    9    blocks.add(new BlockAndValidity(blockToHeightMap, b78, true, false, b78.getHash(), chainHeadHeight + 26, "b78"));
   10
   11    Block b79 = createNextBlock(b78, chainHeadHeight + 27, out26, null);
   12    Transaction b79tx = new Transaction(params);
   13    {
   14        b79tx.addOutput(new TransactionOutput(params, b79tx, ZERO, new byte[]{OP_TRUE}));
   15        b79tx.addInput(new TransactionInput(params, b79tx, new byte[]{OP_TRUE}, new TransactionOutPoint(params, 0, b78tx.getHash())));
   16        b79.addTransaction(b79tx);
   17    }
   18    b79.solve();
   19    blocks.add(new BlockAndValidity(blockToHeightMap, b79, true, false, b79.getHash(), chainHeadHeight + 27, "b79"));
   

   It’s pretty verbose, seems we should update it to the more modern and convenient tx building APIs. But indeed it’s correct that they fail this new check: b79tx has an input script that connects to an output script, both of which are just OP_TRUE, so we end up with a combined script of OP_TRUE OP_TRUE which leaves >1 stack item.

   As far as I can tell this isn’t actually meant to test such a case, it’s just that @TheBlueMatt might have not wanted to write out a full script. Matt, is that the case? If so we can probably make it pass by just changing OP_TRUE in the input script to OP_NOP or something.

6. 
   TheBlueMatt commented at 7:35 pm on June 11, 2014: member

   Yes, @mikehearn, youre correct, I’m just lazy :)
7. 
   petertodd commented at 7:51 pm on June 11, 2014: contributor

   @TheBlueMatt Be less lazy!

   And by that I mean be more lazy with your school work… :) (or are you done for the summer?)

8. 
   mikehearn commented at 10:23 am on June 18, 2014: contributor

   I think I fixed the code in bitcoinj git master, but I don’t know anything about the pull tester or how to rebuild/upgrade it.
9. 
   BitcoinPullTester commented at 1:25 am on June 23, 2014: none

   Automatic sanity-testing: FAILED BUILD/TEST, see http://jenkins.bluematt.me/pull-tester/p4311_ce00fa1d609e6dcd233cc0859f078c469e40bf7b/ for binaries and test log.

   This could happen for one of several reasons:

   1. It chanages paths in makefile.linux-mingw or otherwise changes build scripts in a way that made them incompatible with the automated testing scripts (please tweak those patches in qa/pull-tester)
   2. It adds/modifies tests which test network rules (thanks for doing that), which conflicts with a patch applied at test time
   3. It does not build on either Linux i386 or Win32 (via MinGW cross compile)
   4. The test suite fails on either Linux i386 or Win32
   5. The block test-cases failed (lookup the first bNN identifier which failed in https://github.com/TheBlueMatt/test-scripts/blob/master/FullBlockTestGenerator.java)

   If you believe this to be in error, please ping BlueMatt on freenode or TheBlueMatt here.

   This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

10. 
    jgarzik commented at 5:32 pm on July 29, 2014: contributor

    LGTM
11. 
    sipa commented at 7:19 pm on July 29, 2014: member

    @mikehearn I can deploy a new pulltester jar (though it’d be nice if I could build it myself…)
12. laanwj added the label TX scripts on Jul 31, 2014
13. 
    petertodd commented at 8:18 am on November 10, 2014: contributor

    Closing as @sipa has a better version of this code in #5143
14. petertodd closed this on Nov 10, 2014

    ---

15. MarcoFalke locked this on Sep 8, 2021

Contributors
petertodd sipa mikehearn TheBlueMatt BitcoinPullTester jgarzik

Labels
Validation