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

script: reduce OP_RETURN standard relay bytes to 40 #3737

Merged
merged 1 commit into from
Feb 26, 2014

Conversation

jgarzik
Copy link
Contributor

@jgarzik jgarzik commented Feb 24, 2014

Per mailing list discussion.

@ghost
Copy link

ghost commented Feb 24, 2014

Much better, and I like the use of a constant.

@laanwj laanwj added this to the 0.9.0 milestone Feb 24, 2014
@laanwj
Copy link
Member

laanwj commented Feb 24, 2014

ACK

@Michagogo
Copy link
Contributor

If I had push access, this would be an ACK.

@Michagogo
Copy link
Contributor

(Except for line 287 of the tests)

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

// 81-byte TX_NULL_DATA (non-standard)
Copy link
Member

Choose a reason for hiding this comment

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

@Michagogo better to submit such comments inline ^^
And BTW you don't need push access to ACK anything. The more people that partake in code review the better.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, sorry -- I was on my phone at the time, and the mobile interface has
neither inline comments nor comment editing.

On Tue, Feb 25, 2014 at 8:48 AM, Wladimir J. van der Laan <
notifications@github.com> wrote:

In src/test/transaction_tests.cpp:

@@ -280,12 +280,12 @@
t.vout[0].scriptPubKey = CScript() << OP_1;
BOOST_CHECK(!IsStandardTx(t, reason));

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

// 81-byte TX_NULL_DATA (non-standard)

@Michagogo https://github.com/Michagogo better to submit such comments
inline ^^


Reply to this email directly or view it on GitHubhttps://github.com//pull/3737/files#r10025965
.

Copy link
Contributor

Choose a reason for hiding this comment

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

So yeah, just to have it as a line note, this comment appears to have been missed.

@Diapolo
Copy link

Diapolo commented Feb 25, 2014

@cozz Does this require changes to GUI coin-control or the comments in which you also work with magic values?

@cozz
Copy link
Contributor

cozz commented Feb 25, 2014

@Diapolo Coin control assumes 34 bytes for a typical txout. To me it looks like the current coin control code is not affected by this change.

@gavinandresen
Copy link
Contributor

ACK.

1 similar comment
@gmaxwell
Copy link
Contributor

ACK.

@jgarzik
Copy link
Contributor Author

jgarzik commented Feb 26, 2014

Updated to fix comment bug.

@BitcoinPullTester
Copy link

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

gavinandresen added a commit that referenced this pull request Feb 26, 2014
script: reduce OP_RETURN standard relay bytes to 40
@gavinandresen gavinandresen merged commit ae7e5d7 into bitcoin:master Feb 26, 2014
@gidgreen
Copy link

Just a question about this which I haven't seen addressed explicitly in the related blog posts, etc... Will transactions with multiple OP_RETURN outputs of up to 40 bytes each be considered as standard, or is there a limit of only one such output per transaction?

@luke-jr
Copy link
Member

luke-jr commented Feb 28, 2014

@gidgreen Also note that you're not supposed to use this at all.

@gidgreen
Copy link

@Michagogo - thanks for that.

@luke-jr - can you please explain what you mean? Why is a feature being added to bitcoin if we're not supposed to use it?

@luke-jr
Copy link
Member

luke-jr commented Feb 28, 2014

@gidgreen OP_RETURN is being added because some people insist on abusing the blockchain. This gives them a less painful way to do it. An analogy would be a rape victim not-resisting to try to minimise the damage.

@gidgreen
Copy link

Luke, thanks for your reply. When you say "abusing", this is a value judgment. Can you or anyone else please let me know if this judgment is only your personal opinion, or if it is also official policy of the Bitcoin Foundation? It is important for me to know because I have lots of ideas about useful things that can be done with these 40 bytes. Things that might help bitcoin be more user-friendly and applicable for a broader range of transactions. (I'm not talking about embedding images or any other nonsense like that!)

@ghost
Copy link

ghost commented Feb 28, 2014

The bitcoin foundation has nothing to do with this, it's a core developer consensus decision, and a correct one at that. It's a compromise to prevent abuse that would be even worse. The blockchain is not a dumping ground for arbitrary data - it is a financial ledger and therefore the records should be related to just that.

@gidgreen
Copy link

@Drak - thanks for your response. I had thought there was a closer relationship between the bitcoin code devs and the foundation - sorry about that. In any event I'm not talking about arbitrary data, but rather data relating to the transaction itself, that will make bitcoin viable for certain use cases that it can't currently support. Do you think I can rely on this 40-byte metadata field being available over the long term, or is it likely to be changed or reduced in size in future?

@sipa
Copy link
Member

sipa commented Feb 28, 2014

@gidgreen I think you should only use this OP_RETURN mechanism if the alternative is storing the data somehow inside amounts or other scripts. It's still abuse - it just hurts a bit less if you use OP_RETURN.

@luke-jr
Copy link
Member

luke-jr commented Feb 28, 2014

@gidgreen It has little to do with either the Foundation or the devs (though it's indeed a consensus there). When Bitcoin was released, it was understood that nodes needed to store a blockchain of financial transaction data; that was the "deal" everyone (implicitly) agreed to when they began adopting/buying bitcoins. While some subset of those users might also agree to store non-financial data, it does not justify forcing everyone else to store data they never agreed to store. Furthermore, so far all attempts/desires to store data in the blockchain have been unnecessary (eg, someone is too lazy to do things more efficiently), and/or because they intentionally want to force people to store things against their will.

@rebroad
Copy link
Contributor

rebroad commented Feb 28, 2014

@luke-jr you do like your rape analogies!

@gidgreen
Copy link

Thanks for all your responses. I understand your opinion but perhaps I can share my perspective.

I don't think it makes sense to talk about what "everyone" on the network thinks. Different participants in the network have different motivations, and we can therefore expect them to think differently.

For example I imagine that most regular users of bitcoin, running a wallet like Bitcoin-QT, would like the blockchain to be as small as possible, while still permitting them to send and receive bitcoins quickly and safely. Nonetheless they might be willing to live with a small amount of data inflation, if the result is a product which is significantly better for them. For argument's sake, let's say it allowed them to use bitcoin to pay their utility bills.

On the other hand I imagine that most miners don't care what transactions contain, so long as (a) there are as many transactions as possible, (b) they are paid well per transaction byte and (c) the price of bitcoin (in fiat terms) remains as high as possible.

Just like there is no single entity which controls which transactions are confirmed, there is no single entity which controls which version of the bitcoin software should be run by the network. And since bitcoin is open source, there is always the risk of a fork in the software, which I think we can all agree would be fairly disastrous. In order to avoid this possibility, we must seek compromise between the interests of different participants, so that everyone is more-or-less happy enough to stay within the consensus.

And so we come to the question of transaction metadata. Let's say miners love it and everyone else hates it. In that case, this idea of OP_RETURN + 40 (or 80) bytes seems like a reasonable compromise. It allows transactions to become a lot more valuable per byte (good for miners), but it doesn't impose too much of a storage cost on other participants. We still have the mechanism of transaction fees to ensure that people making use of the compromise have to pay their way in the market.

Compared to this compromise approach, I don't see the sense in trying to dictate to the network what it should think and how it should behave. In any event the network's participants have the freedom to go in whichever direction they choose.

@luke-jr
Copy link
Member

luke-jr commented Feb 28, 2014

Forcing other people to do things is infringing on their freedom. Things like storing data in the blockchain require everyone with a stake in Bitcoin to consent, for it to cease being abuse.

@gidgreen
Copy link

I'm afraid I don't understand your logic because no one is being forced to be a part of the bitcoin network. First, people have the option of not using bitcoin. Second, they can send/receive bitcoin using lightweight wallets which don't store the blockchain on the computer.

@gavinandresen
Copy link
Contributor

I agree with @gidgreen .

Words like "abuse" and "rape" aren't helpful. If you think the size of the blockchain is a big issue, then please help optimize it. Because even without OP_RETURN, the blockchain will get bigger. Forever.

@luke-jr
Copy link
Member

luke-jr commented Feb 28, 2014

@gidgreen The terms of participation in the bitcoin network has from the start been to store financial data. Changing the terms to require storage of other data after the fact, is indeed forcing it on existing participants. Just because we can opt to stop using bitcoin does not change the fact that you are trying to change the terms we bought in with. "Lightweight wallets" don't count, because they are essentially just trusting other nodes (which is also a change in terms).

@rebroad
Copy link
Contributor

rebroad commented Feb 28, 2014

I've seen a number of pulls over the years which seem to benefit miners more than non-miners. I would like to see these sorts of changes implemented in ways that make them more optional, and cater for the majority (which I suspect are non-miners) rather than the small minority of miners.

@gidgreen I'm inclined to suggest that a fork would actually be a good thing - after all, as you say, buitcoin should not be centralized. A variety of bitcoind daemons would make this truer.

@gidgreen
Copy link

gidgreen commented Mar 1, 2014

@rebroad - you make an interesting observation. I think the fundamental reason could be that miners are in a sense the backbone of the network, and so people recognize the need to keep them happy. When the blockchain is 2 TB in size and the UTXO memory pool reaches 500 GB, who else is going to have an economic incentive (and be able to afford) to run full bitcoin nodes?

@luke-jr - I guess we are just going to have to agree to disagree about this. Under your thinking, no significant changes are permitted to the bitcoin software, unless they are agreed upon by every single current participant in the bitcoin network. Nothing illogical about that, but it seems to me like an ultra-conservative position that will hobble the network's development.

@gavinandresen gavinandresen removed this from the 0.9.0 milestone Mar 12, 2014
@jgarzik jgarzik deleted the op-return-size branch August 24, 2014 04:20
@ryanhuh
Copy link

ryanhuh commented May 31, 2018

ACk

@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.