Document tx relay policy?

MarcoFalke commented at 4:29 pm on August 26, 2021: member

Since there are other projects relying on transaction relay policy, it was suggested to document the policy. However, this comes with questions and obstacles. For example, documentation might misrepresent what is done in the code, due to:

misunderstanding of the author
it becoming stale/outdated
it being incomplete

My recommendation for users is to test all of their use cases on each upgrade of Bitcoin Core, including all edge cases they rely on, ideally with an automated test suite to minimize their repeated work. Thus they should already be aware of the policy rules they rely on, and also be notified of changes by their test suite.

If there are plans to nonetheless document the relay policy, it raises questions what to include in what form and how to keep it up-to-date/consistent.

MarcoFalke added the label TX fees and policy on Aug 26, 2021

MarcoFalke added the label Brainstorming on Aug 26, 2021

glozow commented at 11:09 am on August 27, 2021: member

This sounds like it could help clarify expectations around mempool policy and tx relay for users and application devs. I’ve heard people express confusion and fear about it, though I haven’t heard anyone request a doc specifically.

I agree that devs should be writing tests for the assumptions they’re making. I also think having a markdown doc would be appropriate for guiding the process, as code comments might not be clear/communicative enough to people not familiar with the code, and BIPs are too static. I would frame it as “don’t just broadcast any transaction and expect it to propagate, here are some restrictions enforced by Bitcoin Core policy (not necessarily exhaustive)”

wrt maintainability, not to discount the burden of maintaining docs, but policy doesn’t really change that much… maybe we can give it a shot? :)

darosior commented at 11:19 am on August 27, 2021: member

As an interface used by downstream projects, like the RPC interface I believe it makes sense for us to document (and deprecating / avoid breaking) the behaviour of Bitcoin Core for the relay policy.

A document in the source tree updated as the code is modified seems to me to be a good first step in this direction.

Pinging @ariard who’s long been advocating for such a documentation.

MarcoFalke commented at 12:08 pm on August 27, 2021: member

like the RPC interface

The RPC documentation is affected by the same issues. (plain wrong, outdated, incomplete, …)

I think compiling the documentation or otherwise making it machine-readable and checking it for every commit by the CI is the only way to make it as reliable as possible.

ariard commented at 0:46 am on August 28, 2021: member

misunderstanding of the author it becoming stale/outdated it being incomplete

Yes that’s a risk with any documentation and yes the IETF RFCs have multiple errata/addendums! Though we have 50y of history proving that’s a not so bad engineering practice in the area of computer networking :)

Documentation is a formidable tool to formalize thinking and actually interrogate the well-foundedness and correctness of any code. It would also ease the understanding of new contributors and help them to review muzzy part of the codebase like the mempool.

My recommendation for users is to test all of their use cases on each upgrade of Bitcoin Core, including all edge cases they rely on, ideally with an automated test suite to minimize their repeated work. Thus they should already be aware of the policy rules they rely on, and also be notified of changes by their test suite.

I agree with this.

It’s unrealistic to expect Core developers to track and understand any new use case. Ultimately it’s the responsibility of the application developers to think about the assumptions they’re doing on Core policy and be fully-aware it might to change to their detriment.

That said, I think it would help application quality audit if policy changes happen in the beginning or mid release schedule. Further it’s really hard to ensure your test suite is reflecting Core behavior, straightforward toolchain might not exist yet (see https://github.com/rust-bitcoin/rust-bitcoinconsensus/pull/15).

If there are plans to nonetheless document the relay policy, it raises questions what to include in what form and how to keep it up-to-date/consistent.

Yes let’s avoid the BIP process it’s too much static, maybe we can try adoc/policy/*.md ?

Happy to try the experiment with my full-rbf proposal for 0.24 and write an updated documentation of our replacement logic. Then documenting PreChecks’s list of checks could be following steps if we think it’s good practice.

(Maybe the new package acceptance checks could be documented there too?)

Lastly, yes it’s a maintenance burden though if we don’t I think we’ll let proliferate more safety risks for our downstream users and find us in thorny situations where we have to arbitrarily favor use-case A over use-case B. E.g “zero-conf transactions” vs “dual-funded channels” with the deprecation of opt-in : https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2021-June/019074.html

ariard commented at 0:55 am on August 28, 2021: member

And also documentation would help spot on inconsistencies/oddities in the policy. E.g a scriptPubKey superior to MAX_SCRIPT_SIZE is considered as an always valid output by GetDustThresholddue to IsUnspendable but in fact it’s not a standard scriptpubkey following IsStandard because nMaxDatacarrierBytes is 83 bytes.

michaelfolkson commented at 11:39 am on August 30, 2021: contributor

I agree with most of the above (definitely worth documenting Core transaction relay policy given how Layer 2 depends on it and documentation should provide clarity and perhaps even stumble on more things we didn’t realize).

Yes let’s avoid the BIP process it’s too much static, maybe we can try a doc/policy/*.md ?

But I think avoiding the BIP process entirely on this is suboptimal. Firstly, we have to consider alternative implementations to Core e.g. @pinheadmz was the first to discover the BIP 125 issue while reimplementing it for Bcoin. Reimplementing from scratch a spec/standard seems superior to just copying the code in Core.

I think the better approach would be to document this in the Core repo under the expectation that significant step changes to tx relay policy should be BIPed. The Core docs can be a moving target while changes that deserve more scrutiny should be BIPed. I really don’t like this attitude that because Core’s consensus code is the ultimate authority on consensus that Core’s policy code should also be the ultimate authority on policy. The former can’t be avoided, the latter can be.

edit: My understanding is that Layer 2 wants transaction relay policy to be as static as possible (or at least when changes are made they are well communicated in advance to Layer 2). To this end the extra scrutiny of a BIP for a significant change should be beneficial.

ariard commented at 0:24 am on September 9, 2021: member

The Core docs can be a moving target while changes that deserve more scrutiny should be BIPed.

Ofc Core transaction relay policy documentation could be always duplicated as a BIP, with additional information such as applications relying on specific behaviors and even references to test vectors . Now hoping it would attract more eyes because it’s BIPed… maybe it gives a chance to third-party developers to object in case of annoying/harmful changes for them but I’m the opinion it would be better for the Core project to be more proactive on its policy changes, like “best practice” ML announcement for tightening with an open consultation period or flag day activation.

On the other side, not systematically proposing Core’s policy in the BIP repo make it less authoritative IMO. Other full-node implementations or Bitcoin software should feel free to document their own policies as examples, and in fact that’s an area where they could easily do far better than Core. Less legacy codes and they can experiment faster because they have less risks to break application software (though on the long term, I hope we’ll be able to deploy policies in a more sandboxed approach, once the work on multiprocess/interfaces is more mature)

JeremyRubin commented at 1:19 am on September 9, 2021: contributor

I think it’d be worthwhile to have this, and also to make clear that anything that is relayable but is not specified by the spec may become un-relayable in the future – so if you plan to make an application out of something that is a gap in our standardness definitions, your mileage may vary.

jnewbery commented at 12:23 pm on September 28, 2021: member

Strong concept ACK.

My preference would be to document Bitcoin Core’s relay policy in https://github.com/bitcoin-core/bitcoin-devwiki/wiki. I think a wiki is a much more suitable platform for documentation than this git repository. We already have more activity in this repo than we can manage (400+ PRs currently), and people should be able to make incremental improvements to documentation (eg fix spelling/formatting/typos) without having to go through the very heavy PR process and adding to contributors’ and maintainers’ burdens.

I also think we should have documentation for relay policy for all versions of Bitcoin Core (not just latest/master). We can easily do that using multiple wiki pages.

MarcoFalke referenced this in commit 23afc5f47b on Dec 20, 2021

sidhujag referenced this in commit 2a913acfbb on Dec 20, 2021

MarcoFalke closed this on Jan 17, 2022

DrahtBot locked this on Jan 17, 2023

Document tx relay policy? #22806