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

Allow several OP_RETURN in one tx and no limited size #27043

Closed
Ayms opened this issue Feb 5, 2023 · 37 comments
Closed

Allow several OP_RETURN in one tx and no limited size #27043

Ayms opened this issue Feb 5, 2023 · 37 comments

Comments

@Ayms
Copy link

Ayms commented Feb 5, 2023

The rationale is explained in this thread [bitcoin-dev] Debate: 64 bytes in OP_RETURN VS taproot OP_FALSE OP_IF OP_PUSH

And other threads around that are addressing about the same subject: bring NFTs or other systems on or off chain and easy storage to Bitcoin

As mentionned in the thread there are so many possibilities to store things in Bitcoin, including very deviant practices like storing in addresses (then burning bitcoins), that there is no rationale for the OP_RETURN limitation, this change has a minimal impact

The current limit of 80B does not even allow to store a signature and a hash

Not doing the requested change can lead people to invent very bad practices, which will still remain not expensive but will have an impact on the future of Bitcoin

@maflcko
Copy link
Member

maflcko commented Feb 6, 2023

A datacarrier output is in the non-witness part of a transaction. An inventory vector sent in a getdata message can not request to discard the datacarrier. For example, a 0x2 inv type to request a block will be answered with the datacarrier included, however it will have its witness data removed. Same for 0x1.

@Ayms
Copy link
Author

Ayms commented Feb 6, 2023

Yes, but what do you mean?

@maflcko
Copy link
Member

maflcko commented Feb 6, 2023

My comment implies that your suggested change may put more network and storage load on peers in the network that make use of the 0x1 and 0x2 inv types to strip witness data

@Ayms
Copy link
Author

Ayms commented Feb 6, 2023

Yes but then? What is the use of those nodes? And the same pb occurs with the taproot witness (if people for example decide to store big images in it)

It's not really my proposal but the conclusion of the discussion for the thread mentionned above where at the end @petertodd proposed to let people free to decide whether they want to use several OP_RETURN and no size limit, and everybody agreed so far, my proposal was more to increase the size limit but I think that what is proposed here is the right way to go

I don't think that the threat is storage/load limit but more that people invent things that would harm bitcoin (in my specific proposals the witness trick or others do not work very well)

@apoelstra
Copy link
Contributor

Yes but then? What is the use of those nodes?

Any node that wants to learn the state of the network while trusting that witnesses are legit (for example a tool which monitors for unexpected spends of specific coins, or a node doing IBD and [reasonably IMHO] assuming that any blocks more than 1000 deep are legit, without bothering to validate the witnesses). This sort of use case is major benefit of segwit's design -- that, and allowing witnesses to be partially constructed without causing unpredictable txid changes.

In particular, this category of use is one reason that the Segwit rules allow more witness space than non-witness space.

And the same pb occurs with the taproot witness (if people for example decide to store big images in it)

It doesn't, because nodes that don't download witnesses won't see these images at all.

It's not really my proposal but the conclusion of the discussion for the thread

Well, the thread is still active, but my opinion (and I believe that of Peter Todd) is that (a) because users have found ways to store data on the blockchain no matter what, limiting OP_RETURN is technically complicated, may undermine some "legitimate" proof-of-publication protocols, and fails to achieve its goal of preventing blockchain spam; and (b) to the extent that non-witness data is worse for the p2p network than witness data (e.g. non-witness-validating nodes being forced to download the data), the economics of the "witness discount" will encourage people to do the least bad thing.

@petertodd
Copy link
Contributor

petertodd commented Feb 6, 2023 via email

@Ayms
Copy link
Author

Ayms commented Feb 6, 2023

Yes, the non witness transmission, OK, then witness taproot will affect other witness nodes anyway, then... (do we have more non witness nodes than witness ones?)

Anyway again, I think also that we can trust people not to do stupid things with this change (which will never be a spam stuff unlike others, if people want to misuse it and pay plenty of fees, that's their problem)

@Ayms
Copy link
Author

Ayms commented Feb 6, 2023

And @apoelstra if I were to monitor bitcoin to track dubious/illegal spending or secret messages, then I will for sure look into witness fields, so at the end what is proposed here does not make any difference

@earonesty
Copy link

earonesty commented Feb 7, 2023

just let people continue to encode whatever they want into witness data. no need to make it easier. its already easy enough. decoding multisig-as-data or tapscript-as-data is trivial. since 2017 we have had very large and discounted witness data.

my only concern is that the witness discount was a mistake. the real cost of witness data is nearly as high as the rest of the block.
full nodes need to store it, full-syncing nodes need to full-sync & validate it. only pruning nodes have a lower cost, and we dont really need to encourage prune vs pull using a discount. that doesn't seem to foment decentralization. fortunately, removing or curtailing the discount can be done with a soft fork.

@Ayms
Copy link
Author

Ayms commented Feb 7, 2023

Not sure if you are for the change or against, it seems like you are against while advocating that it does not nearly change anything at the end, "no need to make it easier", no, you spare useless txs with op_return and is more straight forward (and yes decoding whatever you like is "trivial" for bitcoin experts, not for usual people, op_return remains more visible), and since witness allows no size limit (and you estimate that the discount is minor) then op_return should do the same

@ChristopherA
Copy link

  • Concept ACK for no limit on OP_RETURN size.
  • Concept ACK for no limit on the number of OP_RETURNs in a transaction.

@ChristopherA
Copy link

Even though I know I could get a discount by using witness data, I prefer the single transaction nature of using OP_RETURN and the ability to use TxREF BIP-136 to point to it.

@Ayms
Copy link
Author

Ayms commented Feb 12, 2023

What is the process to have someone do the PR for this? Or I do it and most likely it will be a very shxtty one since I am not a C/C++ expert, then wasting the time of everybody

It's urgently required, I did consider OP_RETURN as a dart in the past but changed my mind, it's adapted to the current evolutions, not flooding bitcoin with 2 txs while only 1 is needed

If not the best 1 tx solution is super simple: store in addresses, and super bad at the end because burning bitcoins, while still not expensive if you don't need to store big things

@maflcko
Copy link
Member

maflcko commented Feb 12, 2023

See https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md

@Semisol
Copy link

Semisol commented Feb 13, 2023

Concept NACK, at least SegWit allows for this data to be not downloaded. Alternative would be to make 256-512 byte OP_RETURNs.
Also, OP_FALSE OP_IF OP_PUSH is an unintended way to store data and should not be used as rationale for doing protocol changes.

@Ayms
Copy link
Author

Ayms commented Feb 14, 2023

@MarcoFalke super simple...

@Semisol as mentionned in the thread there are so many ways to store things that there is no rationale to impose a limit in OP_RETURN, this is cleary a handicap for bitcoin and future uses/evolutions, bitcoin can clearly do far better what ethereum and all the messy stuff around pretend to do but we need this change

@glozow glozow closed this as not planned Won't fix, can't repro, duplicate, stale Feb 25, 2023
@Ayms
Copy link
Author

Ayms commented Feb 26, 2023

Why do you close it "as not planned"? Of course a modification request is "not planned", or who decided that it is "not planned"

@petertodd
Copy link
Contributor

petertodd commented Feb 26, 2023 via email

@ghost
Copy link

ghost commented Feb 26, 2023

The fact that Core devs feel like they can close this with no explanation at all suggests that protocols should use the backup mechanism of polluting the UTXO set. The dust limit is low enough that for a lot of use cases a few extra outputs isn't a big deal.

On February 26, 2023 12:29:49 PM GMT+01:00, Aymeric Vitte @.> wrote: Why do you close it "as not planned"? Of course a modification request is "not planned", or who decided that it is "not planned" -- Reply to this email directly or view it on GitHub: #27043 (comment) You are receiving this because you were mentioned. Message ID: @.>

Agree

@glozow glozow reopened this Feb 26, 2023
@Ayms
Copy link
Author

Ayms commented Feb 26, 2023

@petertodd If I understand correctly your comment, you mean that without this change (as I think too) people will invent stupid things like I am myself proposing here (ie storing in addresses, with uncompressed keys, worse method on earth, but not expensive as you say and as I wrote, let's say that we will survive it and don't care for the future) : https://gist.github.com/Ayms/01dbfebf219965054b4a3beed1bfeba7#workaround-to-the-80b-op_return-limitation, then indeed polluting the UTXO set with not spendable txs

@petertodd
Copy link
Contributor

petertodd commented Feb 26, 2023 via email

@Ayms
Copy link
Author

Ayms commented Feb 26, 2023

You can invent whatever non standard solution you like as long as the tx is valid and then work with miners, but that's not the easy/standard way to go

What other standard alternatives to OP_RETURN uses Opentimestamps? As far as I know we have only two for now as standard: taproot IF but 2tx, taproot annex 1tx (not available right now), that's the other mechanisms you are refering to?

Opentimestamps looks to be working the very same way for some parts than what I am proposing for NFTs, but not for the same use cases, that's probably another discussion, I read the docs but did not find the specs, obviously a third party is needed to provide the merkle proof (and store the timestamped thing), which is very exactly what I am proposing also, only the proof is stored in bitcoin

Unless someone makes a very clear point, I don't see the issue with OP_RETURN (which again I did consider in the past as shxtty stuff), and the argument based on nodes not downloading witness does not work very well, those nodes are of no use, or please prove me the contrary

@petertodd
Copy link
Contributor

petertodd commented Feb 26, 2023 via email

@Ayms
Copy link
Author

Ayms commented Feb 26, 2023

Rephrasing then since you misunderstood my comment also, the question was in fact: what are the (standard) "multiple different ways of putting data in Bitcoin transactions" for opentimestamps ?

@glozow
Copy link
Member

glozow commented Feb 27, 2023

Apologies for the mistaken close and thank you for the ping to reopen.

@Ayms
Copy link
Author

Ayms commented Feb 27, 2023

@glozow Thanks, @1440000bytes I saw your comments that you have deleted apparently (maybe a misunderstanding about my intentions), for my information who are the current 4 maintainers?

I don't see where I could be dishonest in this thread and why I would be trying to force/bypass some processes, I am a bit used to this since many years here and elsewhere, my only concerns are: what is the issue with this trivial change compared to other standard storage methods? And not doing it might lead people to invent stupid things/pollute the utxo set

@petertodd
Copy link
Contributor

So the next step here for people who want to actually see this implemented is code, and perhaps more importantly, tests.

The code change is fairly simple: change the datacarrierlimit to default to max-int, and make the rule that checks if there is more than one OP_Return only take action if the limit isn't the default. I don't think there is any reason to implement a separate rule to check if there is more than one OP_Return if you aren't limiting the size.

Second, tests. You need to check that:

  • The previous limit is still enforced when datacarrierlimit is set.
  • Handling of OP_Returns of any size works correctly.

ScriptPubKeys that fill up entire blocks already exist in testnet, eg from the time I rick-rolled testnet. But OP_Return outputs are handled very slightly, so better if you can at least find one/mine one in testnet to prove they cause no consensus problems. An explicit test for this case wouldn't hurt either. As for large numbers of OP_Returns, again, we already have transactions with large numbers of outputs and OP_Return should not be any different. But it doesn't hurt to test this explicitly.

@Ayms
Copy link
Author

Ayms commented Mar 6, 2023

@petertodd then could someone please do it? (you?)

I am good with js/nodejs/browsers but can't pretend to be a C/C++ expert, even if the change is simple it would be a waste of time/not serious I believe that I issue a PR for this, and approximation can't work with Bitcoin code

@ChristopherA did open the debate, which did intersect some discussions that I started, that's why I opened this issue myself, you proposed a solution but at the end it just must be done

@junderw
Copy link
Contributor

junderw commented Mar 7, 2023

then could someone please do it? (you?)

There are many C++ devs you could pay to have this PR done for you. That would be much quicker, especially considering how simple the change is. The only hard part might be writing tests, since the testing harness etc. does take a bit of getting used to... (that said, looking at the other tests usually can give one a good head start)

There are also many bounty websites where you can lock BTC into a vault that is only released to the person who does the bounty.

I doubt many people that are sympathetic with this issue will see your comment unless you advertise it somewhere and/or offer a cash reward...

or take a crack and learning some C++, maybe? (it's actually not as hard as it sounds)

@junderw
Copy link
Contributor

junderw commented Mar 7, 2023

@MarcoFalke perhaps this could also use a "good first issue" tag? Seems pretty straight forward.

@ChristopherA
Copy link

Blockchain Commons would be glad to contribute US$1K (paid in BTC) towards a joint bounty with others to see this as a PR with the core changes and a testing harness. Who else will contribute to the pot?

@Ayms
Copy link
Author

Ayms commented Mar 8, 2023

@junderw thanks for the tip, I know C/C++ in fact, but I don't consider to be an expert, probably you can yourself fix the wasm stuff for Tor Rust

@ChristopherA the change is trivial as you know, would take 5mn for an expert if I were to do it in my area, and for free, good to see that there is some funding proposal, but, really, in a worldwide important project like bitcoin, nobody can do this without adding funding to funding?

@ChristopherA
Copy link

@Ayms The change to the C++ code core is fairly trivial, but for the PR to be acceptable, it must have the test frameworks changed for it, which is less so as they are in Python. I've not touched that for years, and I don't have a current build environment.

It should, however, be a relatively easy first contribution for someone who wants to learn how to do bitcoin core contributions.

@petertodd
Copy link
Contributor

Alright, I'm going to take a crack at this.

I'll ask that anyone else who hasn't already implemented this patch wait until next Monday before starting work on it. :)

@Ayms
Copy link
Author

Ayms commented Mar 8, 2023

Cool, I am a bit like @ChristopherA probably , even if still operating a bitcoin node that I did compile myself (+ some slight modifications for testing), I did not work on it since years

@petertodd
Copy link
Contributor

FYI, first bit of work on this, a minor cleanup to the logic: #27261

@willcl-ark
Copy link
Member

This feature seems to be controversial and so unlikely to be implemented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

11 participants