Relay OP_RETURN TxOut as standard transaction type #3128

Also fix decoderawtransaction to not show reqSigs or addresses for null and nulldata txouts. (Previous version also left reqSigs uninitialized mistakenly)

I suspect #2740 broke the pull-tester.

Do we need both TX_NULL and TX_NULL_DATA? Seems to me it should just be TX_NULL_DATA, and supporting that would (I think) be a one-line change:

mTemplates.insert(make_pair(TX_NULL_DATA, CScript() << OP_RETURN));

If we ever write higher-level code that extracts the data from a TX_NULL_DATA, that should be trivial-- remove the first byte/opcode from the ScriptPubKey, execute whatever remains (empty CScript in the case of no SMALLDATA), and data is whatever is left on the execution stack.

... wait, no, I'm wrong, mTemplates is a map, not a multimap... Maybe it should be a multimap, though.

@gavinandresen Regardless of map/multimap, I had the same thought. TX_NULL_DATA can be defined as OP_RETURN, with optional trailing pushdata <= 80 bytes.

Well if we want to do that it'll need a separate code path in Solver(), similar to how P2SH is handled specially. I think you can make a (weak) argument that NULL and NULL_DATA make lives a little easier, because you always know that only the latter has data that your app should try to parse. (remember that it's legal to make a zero-byte PUSHDATA with 0x00)

You know, it's unfortunate that Satoshi bizarrely counts sigops in scriptPubKeys, the only time the protocol looks at them when a block is processed. If not for that I'd just say make a two line test that scriptPubKey[0] == OP_RETURN and len(scriptPubKey) < 81 (a scriptPubKey that's invalid due to invalid PUSHDATA's is allowed into blocks and would save a byte; e.g. ebc9fa1196a59e192352d76c0f6e73167046b9d37b8302b6bb6968dfd279b767)

Anyway, verging towards shed-painting IMO.

Huh? This two-line change seems to work properly:

From d485640daee9ecbbdd3f39ca7ab1cda83b99c4a5 Mon Sep 17 00:00:00 2001
From: Gavin Andresen <gavinandresen@gmail.com>
Date: Thu, 24 Oct 2013 11:20:23 +1000
Subject: [PATCH] 0-byte OP_NULL_DATA

---
 src/script.cpp                 | 3 ++-
 src/test/transaction_tests.cpp | 4 ++++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/script.cpp b/src/script.cpp
index 63f6327..79d0b2c 100644
--- a/src/script.cpp
+++ b/src/script.cpp
@@ -1195,7 +1195,7 @@ bool CheckSig(vector<unsigned char> vchSig, const vector<unsigned char> &vchPubK
 bool Solver(const CScript& scriptPubKey, txnouttype& typeRet, vector<vector<unsigned char> >& vSolutionsRet)
 {
     // Templates
-    static map<txnouttype, CScript> mTemplates;
+    static multimap<txnouttype, CScript> mTemplates;
     if (mTemplates.empty())
     {
         // Standard tx, sender provides pubkey, receiver adds signature
@@ -1208,6 +1208,7 @@ bool Solver(const CScript& scriptPubKey, txnouttype& typeRet, vector<vector<unsi
         mTemplates.insert(make_pair(TX_MULTISIG, CScript() << OP_SMALLINTEGER << OP_PUBKEYS << OP_SMALLINTEGER << OP_CHECKMULTISIG));

         // Empty, provably prunable, data-carrying output
+        mTemplates.insert(make_pair(TX_NULL_DATA, CScript() << OP_RETURN));
         mTemplates.insert(make_pair(TX_NULL_DATA, CScript() << OP_RETURN << OP_SMALLDATA));
     }

diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp
index 5dfb67c..5b69e64 100644
--- a/src/test/transaction_tests.cpp
+++ b/src/test/transaction_tests.cpp
@@ -274,6 +274,10 @@ BOOST_AUTO_TEST_CASE(test_IsStandard)
     t.vout[0].scriptPubKey = CScript() << OP_1;
     BOOST_CHECK(!IsStandardTx(t, reason));

+    // 0-byte TX_NULL_DATA (standard)
+    t.vout[0].scriptPubKey = CScript() << OP_RETURN;
+    BOOST_CHECK(IsStandardTx(t, reason));
+
     // 80-byte TX_NULL_DATA (standard)
     t.vout[0].scriptPubKey = CScript() << OP_RETURN << ParseHex("04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef3804678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38");
     BOOST_CHECK(IsStandardTx(t, reason));
--
1.8.4

@gavinandresen Oh, doh, yeah lets just do that + the bugfix and ScriptSigArgsExpected() change I made.

Now using multimap so there's just TX_NULL_DATA

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/005609539b2184a474ffe9ebfe883984c900a3fb for binaries and test log. 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.