-
Notifications
You must be signed in to change notification settings - Fork 35.5k
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
Libstandardness (edition 2023) #28220
Libstandardness (edition 2023) #28220
Conversation
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
Concept NACK.
This is not and should never become a thing. Every node has its own policies, and relying on any specific policy is broken by design. |
While each node can set its own transaction relay policy, economically rational nodes are expected to follow the set of rules yielding the highest feerate traffic, at the very least for the ones partaking in block template construction if they wish to stay competitive mining entities. If the default set of transaction relay rules do not yield the highest feerate traffic modulo the restrictions for DoS robustness, this is certainly a defect or imperfections that should be fixed, and this in fact the work pursued by BIP331 / nversion3 and its implementation in Bitcoin Core to the best of my understanding. In light of this economical rationality design goal, second-layers protocols can be designed accordingly and issue transactions ensuring they’re understood and relayed by the economically rational nodes on the transaction relay network. If they assume uneconomic transaction-relay behavior in their design and requirements, they should be fixed in consequence. If there is a wish to define further “economically rational” I can propose the lineaments of a game-theory model of Bitcoin transaction-relay, including miners strategy on the mailing list. In the meantime, this work is only exposing our mempool validation interface than one could leverage by direct p2p message to a full-node. |
Better to use the |
I don’t think you’re explaining how I’m supposed to configure an “economically rational" node on an embedded or mobile device where computing resources are limited neither are answering the limitations of current testmempoolaccept w.r.t to pre-signed lightning transactions where the feerates / timelocks should be disregarded from the validation model (e.g when you receive counterparty witnesses). I think rpc is just an interface like another one (zmq / shared library) and application / protocol dev building on top of Bitcoin Core should be constrained in one interface at the detriment of another already existent one (each interface is coming with timing / throughput limitations). If your concern is about sensible code maintenance, thanks to make it explicit. Considering mempool policy a per-node decision is a limited view in a modern world where you might have redundant transaction rebroadcast and a heterogeneity of time-sensitive second-layers protocols, it’s more a question of transaction-propagation standardness. This position deserves a more in-depth explanation on the mailing list and I’ll do so. During the meantime, still looking for approach ACK on the proposed libstandardness interface. |
Look at this a different way - this provides Policy may be node-local, but "minimum viable policy", or something which lightning nodes can generally rely on existing at least on one or two miners and in a broad enough subset of the network that we can rely on such transactions getting mined is a critical API of Bitcoin Core for many applications. Making that something easy for applications to test against is a pretty important thing for the ecosystem. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
spawning hundreds of Bitcoin Core instances in tests
A single instance per chain tip should be enough, no? If there are many chain tips, you'd also have to keep the state with this proposed lib, no?
const auto chainman = std::make_unique<ChainstateManager>(chainman_opts, blockman_opts); | ||
|
||
const auto dummy_chainstate = std::make_unique<Chainstate>(dummy_mempool.get(), chainman->m_blockman, *chainman); | ||
//TODO: initialize dummy_chainstate with chain tip as seen by the caller and spent_UTXO |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How is that different from libbitcoinkernel? Seems odd to add a library for something that can be done with libbitcoinkernel?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As of adc0921ea1
, I think this is very unclear what validation API is offered by libbitcoinkernel. There is no doc/libbitcoinerkernel
so far and bitcoin-chainstate.cpp
is just for demo-only purpose so far.
Verifying the propagation of transactions for use-cases like Lightning and other contracting protocols require more API flexibility than “validation-at-the-tip” as feerates might have to be disregard (i.e they can be added latter with a CPFP) and absolute / relative timelocks ignored (user A funds safety is defined by the ability to broadcast a valid transaction after X though strictly not before X to avoid jeopardizing user B funds safety e.g the htlc-claim / htlc-timeout flow).
Additionally, resources-constrained clients like LN mobile might still validate the propagation validity of their transactions while relying on SPV servers (bip157, electrum) for the chainstate, and therefore uncoupled validation libraries give them more flexibility.
Let’s say you wanna do parallel-fuzzing of all the transactions received from your lightning counterparty to verify that your state machine reject invalid transactions, a library is easier for testing purpose, as you can give the chainstate as a fixed point, I think. |
I think it's unlikely that we'll introduce a new library for this. More ideally, seeing what falls out of, or, building on top of the work being done for the kernel might be a better approach (I think this was also discussed recently). A minimal, kernel-based application, built specifically for testing may also alleviate some (not all) of the complexity concerns. |
I presume the goal here is to provide a library that accepts the list of utxos, as opposed to managing it internally, which libbitcoinkernel is expected to do (to minimize the footgun potential for users). Though, I wonder if this can be a build option, or an API marked "dangerous", within libbitcoinkernel. |
Effectively after chatting with the libbitcoinkernel folks, it sounds possible to build this already existent library as a shared one and then add on top the wished standardness validation function. I expect such API to be more footgunish as you’re validating a transaction (or chain of transactions) and not simply managing UTXO, yet with good design and user documentation this concern can be alleviated. I think such API can be hidden behind a build / config flag as I see standardness sanitization API being used by downstream project developers, not the average lambda user. Closing this PR for now and I’ll propose the same logic hacked on top of libbitcoinkernel soon. To the best of my understanding for now mempool validation is expected to be scoped in the kernel. If it turns out there is some unseen issue integrating with it, we can reconsider approach in this PR. I’m still planning to answer on the mailing list the exposed shortcoming by some reviewers of thinking of mempool policy rules design only in term of your local node, and why it matters for lightning / second-layers. |
This PR proposes to introduce a new interface to allow applications and second layers protocols to verify that their unconfirmed and non-propagated transactions are valid under Bitcoin Core transaction relay policy.
This new
libstandardness
interface is designed at the image of thebitcoinconsensus
library, which already exposes some of the script verification internals to other applications. A new method is introducedlibstandard_verify_transaction
which indicate to the caller if the transaction is valid, i.e can propagate at current chain tip. For now, the policy rules as considered as a "black box", there is no detail (i.eTxValidationState::m_reject_reason
) on thepolicy rule violated.This interface allows second layers like contracting protocols relying on pre-signed transactions and efficient propagation over the network mempools for the security of user funds. E.g for the Lightning Network, counterparties are exchanging signatures for the commitment transaction and second-stage HTLCs during the BOLT2's
commitment_signed
message dance. A policy invalid commitment transaction with pending HTLC outputs not propagating on the network can be source of failure, a serious vulnerability for a Lightning implementation as CVE-2020-26895 showed it. Beyond being source of loss of funds, a policy rule violation can be a source of liquidity griefing for a Lightning node participating in a collaborative transaction flow (dual-funding / splicing), where inputs/outputs are freely added by the counterparties.For some contracting protocols, where the pre-signed txn are malleable by the counterparty it doesn't seem computationally plausible to
testmempoolaccept
all the combinationd valid under the protocol template and this would still assume some changes in our current transaction validation interface to bypass some stateful checks such as timelocks and mempool min fee.As of today, there is no straightforward software tooling for applications and second-layers to verify the validity of the policy rules of their transactions, and most of implementations to the best of my knwoledge are re-implementing the policy rules in their backend, or delegate this verification to their bitcoin libraries. Such re-implementation is sometimes imperfect, must be updated at each Bitcoin Core policy rules changes (e.g packages policy rules) and be adapted for each protocol transaction templates (e.g for Lightning, commitment tx, second-stage tx, closing_tx, collaborative tx, legacy/anchor/taproot).
The proposed interface is introduced as a shared library rather than a RPC interface, as ideally policy rules could be enforced on embedded / resource constrained platforms such as L2 signers enforcing security rule validation of the second layer state machine (and from where one should be able to extract propagating pre-signed transactions in case of emergency recovery) and generally allow for more flexibility for applications and second layers, e.g on mobile phone where a full-node is not assumed (all caveats reserved on the lower security model in that latter case).
This interface has been proposed in the past, see previous PRs
Opening the PR for now to collect conceptual and implementation-approach feedbacks. If the new interface is judged as relevant and useful, I'll add bindings in rust-bitcoin to test the interface end-to-end with adequate Lightning software.
TODO:
doc/shared-libraries.md
and maybe indoc/policy/
BUILD_BITCOIN_LIBSTANDARDNESS
insrc/Makefile.am
andconfigure.ac
)