Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Relay OP_RETURN data TxOut as standard transaction type. #2738

Merged
merged 1 commit into from Oct 22, 2013

Conversation

jgarzik
Copy link
Contributor

@jgarzik jgarzik commented Jun 4, 2013

Add new standard transaction type, that permits small amount of data to be attached to a transaction, in the form of an additional TxOut that is provably prunable.

@jgarzik
Copy link
Contributor Author

jgarzik commented Jun 4, 2013

This is along the same vein as #1809 except this is per-transaction, not per-TxOut.

@BitcoinPullTester
Copy link

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/e25168f78df4fcec52a88590dfa1b043c318b4fa 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.

@TheBlueMatt
Copy link
Contributor

Concept/Code skim ACK, after payment protocol gets merged - too much ability to take the path of least resistance and just use OP_RETURN for adding data to a txn because there is no alternative.

@petertodd
Copy link
Contributor

The current UI has no mechanism to show the user any messages in any OP_RETURN outputs, so I don't think we'll see people using OP_RETURN for stuff that would be better done with the payment protocol. What we do need is an alternative to data and hashes in unspendable outputs so we can nudge the users using Bitcoin for timestamping and similar things towards methods that are less harmful.

I'm still of the opinion that using OP_RETURN should always be as easy and cheap as creating an unspendable txout. That would mean allowing as many OP_RETURN outputs in a transaction as you want, and allowing up to 192 bytes of data per one. (OP_CHECKMULTISIG equivalent, either bare or with P2SH) There should never be an excuse to use an unspendable UTXO rather than OP_RETURN and pressuring people into not doing it via social means isn't working.

Still, if a compromise is what it takes, it's a good step forward.

Once implemented widely, something @gmaxwell suggested was to change the UI so that creating a zero-value out transaction actually creates an OP_RETURN with the digest as the data. I think we should also see if we can convince blockchain.info to implement this on their API in some way. All the timestamping sites and phone apps that have popped up recently seem to use blockchain.info so it'd be great to use that as a way of pushing people onto OP_RETURN. In particular it'd be great if blockchain.info could make 20-byte out OP_RETURN txouts be indexed in their database as though they were addresses to give users a way to look up their timestamps, thus making the overall experience of doing the better thing strictly easier than harming the network.

I'll review/test code later.

@petertodd
Copy link
Contributor

FWIW I did take a careful look at the code and found some minor issues, but I'd like to hear more about people's thoughts on the idea in general - some of the issues are fairly specific to implementation.

@jgarzik
Copy link
Contributor Author

jgarzik commented Jun 24, 2013

Rebased, and fixed @petertodd issue

@gmaxwell
Copy link
Contributor

Should this be merged prior to OP_RETURN UTXO being excluded from the coinstate being widely deployed?

@sipa
Copy link
Member

sipa commented Jun 24, 2013

I agree with @TheBlueMatt that this shouldn't be deployed before there's a alternative for cases where it'd otherwise just be used a communication through the blockchain.

I'm working on a patch that prunes OP_RETURN-starting pubkeyscripts.

@luke-jr
Copy link
Member

luke-jr commented Jul 17, 2013

@jgarzik Rebase needed again.

@jgarzik
Copy link
Contributor Author

jgarzik commented Aug 25, 2013

Rebased.

@dacoinminster
Copy link

MasterCoin may end up being the first real-world usage of this. I'm currently discussing with Jeff what would be involved: https://bitcointalk.org/index.php?topic=284178.msg3168249#msg3168249

@gmaxwell
Copy link
Contributor

@dacoinminster There is already usage of this.

@maciej-trebacz
Copy link

Is there any target client version for this?

@jgarzik
Copy link
Contributor Author

jgarzik commented Sep 23, 2013

Mainly, people do not want this to go in before #2791 which seems like a reasonable request.

else if (opcode2 == OP_SMALLDATA)
{
// small pushdata, <= 80 bytes
if (vch1.size() > 80)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why 80 bytes? That's significantly larger than a hash, yet still too small for a announce-commit sacrifice.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't that be exactly the maximum size a hash can have?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is one SHA512 hash plus some metadata, enabling "BOND $hash" style usage that has been discussed in the past, published on the wiki and implemented in at least one [non-working, inactive] project.

@maciej-trebacz
Copy link

It seems that #2791 got merged, so I guess we have green light here?

As for the size, 40 bytes is probably enough to contain a 256 bit hash plus some additional data.

@gmaxwell
Copy link
Contributor

@m4v3r See my above comment. Being merged wasn't the bar I was suggesting.

Whats your interest / application here?

@sipa
Copy link
Member

sipa commented Sep 24, 2013

I'm still in the middle here.

Ideally, there is just no intent of using the blockchain as a storage mechanism for arbitrary data at all. Unfortunately, it there seems such ridiculous demand for it, that it happens anyway - costly or not, as we've seen in the past.

Given that this is inevitable, the choice is whether such usage should have a way to not burden the UTXO set, which is what this proposal does. The problem is that people may see this as a legitimation of the storage in the first place, and encourage doing so even more.

@gmaxwell
Copy link
Contributor

@sipa > "May see" I don't think there is any ambiguity there, "will see". But is it worth the trade off to shape it towards less harmful forms when it happens?

@petertodd needs? Is this just because they want to store more than 80 bytes?!

(Incidentally, I still think 80 bytes is too much, but I do find PT's existence of alternatives argument relatively convincing on that point)

@petertodd
Copy link
Contributor

Ooops, deleted my comment accidentally, reproduced below:

@sipa I think MasterCoin shows that you can't stop datastorage in the blockchain via social means, only technical ones. (or occasionally the threat of a technical change) Their protocol needs the ability to store more than one data txout per transaction, which means they have reasons to ignore OP_RETURN as implemented here (because you are limited to one OP_RETURN txout) in favor of sticking with CHECKMULTISIG.

@petertodd
Copy link
Contributor

@gmaxwell Who cares? It's easy for them to make a protocol that does what they want with CHECKMULTISIG, so that's what they'll do.

@petertodd
Copy link
Contributor

IMO what we should do is alongside this patch make anything other than P2SH and pay-to-pubkey-hash in a scriptPubKey non-standard and make OP_RETURN be allowed to be present as often as you want, with a data payload size calculated to be a bit cheaper than the alternative possible by P2SH w/ inner CHECKMULTISIG data payloads. That's the right solution because it gives the correct economic incentives with a solid technical implementation.

@gmaxwell
Copy link
Contributor

@petertodd In your second to last comment didn't you just propose limiting bare multisig? That removes datastorage in utxos in that case too. WRT who cares— part of the idea here is shaping behavior towards conservative needs.

For many abuses of bitcoin you only need a hash, and that carries a lot less risk for the system. So I care about the motivation because I want to know if enabling this is going to signal to people that this makes non-hash data "kosher".

@petertodd
Copy link
Contributor

@gmaxwell Oh, yeah, that's correct, so once you limit the UTXO-usage by getting rid of everything but P2SH and pay-to-pubkey-hash making your OP_RETURN be cheaper than either mechanism is what we want. Given that each 20-byte-hash output has a cost in terms of Bitcoins burned (due to the dust rule) it'd be enough to make the data allowed in an OP_RETURN be equal to 20 bytes, and either require the rest to destroy Bitcoins, or just make all OP_RETURN's not subject to the dust rule to make it clear that implementing OP_RETURN in your shitty app is worth the trouble. (note how in this case relative to the data storage required by legit financial transactions you're paying a large premium per-byte because you can't make use of the 8-byte txout value while they can, and eventually we can even mandate it's actually a hash a-la P2SH^2)

Anyway my point is that you don't shape behavior by just telling people, you have to actually force them through incentives and limitations.

@maciej-trebacz
Copy link

@gmaxwell I already stated that on the mailing list. My interest is to attach an
additional signature to transactions generated by my service, so anyone
receiving the transaction can see that it came from the service. This would
allow merchants or exchanges to safely accept Bitcoins after zero
confirmations, because then know they'll coming from my service, which acts
as an escrow and will never double spend.

@petertodd
Copy link
Contributor

@m4v3r Given that the additional OP_RETURN data will make your transactions easily identifiable anyway why not just use a single green address and send all transactions from it? Or just provide a way to query your server over SSL to just ask if a particular transaction is from you?

@petertodd
Copy link
Contributor

@m4v3r BTW if you're working to make a business based on securing zero-conf, I should warn you that we've got a way of making them fairly secure that could make your business plans obsolete: https://bitcointalk.org/index.php?topic=251233.msg2669189#msg2669189

@maciej-trebacz
Copy link

@petertodd If that's the case then this is great news. No, my business model is not based on securing zero-conf, I just thought it could be a valuable addition to the business, but if that will be solved another way I'm all for it.

@maciej-trebacz
Copy link

@petertodd My service is based on giving the service operators as little trust as possible (ideally, no trust at all). That means that at any point I don't want to be in 100% control of users coins. This is implemented by using multi signature transactions. With that in mind I can't use a single green address because it would require me to route all transactions to that address first, thus getting in control of the coins for that moment, which I don't want.
I could provide an SSL API for asking for transaction ownership, but that creates an additional step for receiving clients to implement, and relies on the API having very high uptime, and on the network itself. Whereas signature checking can be done without any network requests, so its more reliable.

PS. This could start a discussion whether the above goal is possible, but please leave that discussion for another time, as this has nothing to do with the issue we're discussing.

@jgarzik
Copy link
Contributor Author

jgarzik commented Oct 2, 2013

Rebased. Code-wise it is merge-ready now, though @sipa illustrated the current merge decision factors.

reason = "dust";
return false;
}
}

// only one OP_RETURN txout is permitted
if (nDataOut > 1) {
reason = "mucho-data";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"mucho-data" ? You trolling to see if we're paying attention?

"multi-op-return" would be better for non-Spanglish speakers.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Glad that someone actually reads the code changes...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Speaking as the person who wrote the "reason=string" code in IsStandardTx(), the strict requirement is that the "reason" is computer-parsable and static vis-a-vis the condition being reported, not necessarily English ;-)

This was added in the most recent rebase, just to have a little fun. If you want to be boring I'll change it :)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want you to be boring and create interesting pulls, I'm the boring-string guy you know ^^.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For historical reference: mucho-data was renamed to multi-op-return: https://github.com/bitcoin/bitcoin/pull/3589/files

@gavinandresen
Copy link
Contributor

How should I test this?

@BitcoinPullTester
Copy link

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/a79342479f577013f2fd2573fb32585d6f4981b3 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.

@petertodd
Copy link
Contributor

@gavinandresen Seems to me that we should have an -accept-nonstd option that -testnet soft-sets to true so that -accept-nonstd=0 can be used easily to test mainnet behavior exactly. (or conversely, -reject-nonstd) I've seen multiple people get confused by that difference when testing their code, usually when debugging nLockTime-using protocols where propagation differs.

https://github.com/petertodd/bitcoin/tree/accept-nonstdtxn is up to date IIRC and could be easily modified for that purpose.

@jgarzik
Copy link
Contributor Author

jgarzik commented Oct 7, 2013

@gavinandresen I made hand-created outputs using https://github.com/gasteve/node-libcoin For python this should work, https://github.com/jgarzik/python-bitcoinlib

Then I pushed to TNIAB setup and manually observed.

@jgarzik
Copy link
Contributor Author

jgarzik commented Oct 7, 2013

@petertodd I'm fine with (1) a chain param indicating accept-nonstd, and (2) a command-line param enabling alteration of that default.

Current default behavior must be preserved of course.

@petertodd
Copy link
Contributor

We should allow a bare OP_RETURN scriptPubKey - no data payload - to IsStandard() as well; sometimes it's useful to simply send Bitcoins to fees with no message or data at all. Example, https://github.com/petertodd/dust-b-gone/ which creates coinjoin tx's that destroy dust by spending inputs to fees with a single OP_RETURN txout.

@jgarzik
Copy link
Contributor Author

jgarzik commented Oct 21, 2013

Agreed. I actually think it should be valid to have zero outputs (100% fee), but as that is a hard forking change, a dummy output is about as good as it gets.

@gavinandresen
Copy link
Contributor

... mumbles about the perfect being the enemy of the good then merges....

gavinandresen added a commit that referenced this pull request Oct 22, 2013
Relay OP_RETURN data TxOut as standard transaction type.
@gavinandresen gavinandresen merged commit be484db into bitcoin:master Oct 22, 2013
FuzzyBearBTC referenced this pull request in FuzzyBearBTC/peercoin Jan 25, 2014
FuzzyBearBTC referenced this pull request in FuzzyBearBTC/peercoin Jan 25, 2014
FuzzyBearBTC referenced this pull request in FuzzyBearBTC/peercoin Jan 25, 2014
FuzzyBearBTC referenced this pull request in FuzzyBearBTC/peercoin Jan 25, 2014
mhanne added a commit to mhanne/bitcoin-ruby that referenced this pull request Feb 27, 2014
lian pushed a commit to lian/bitcoin-ruby that referenced this pull request Mar 4, 2014
@jgarzik jgarzik deleted the op_return branch August 24, 2014 04:20
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet