Libstandardness (edition 2023)

ariard commented at 3:34 am on August 5, 2023: member

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 the bitcoinconsensus library, which already exposes some of the script verification internals to other applications. A new method is introduced libstandard_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.e TxValidationState::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:

write documentation for language bindings write in doc/shared-libraries.md and maybe in doc/policy/
integrate as a config flag only feature in build system (e.g BUILD_BITCOIN_LIBSTANDARDNESS in src/Makefile.am and configure.ac)
initialize the chainstate and its associated mempool with the spent utxos
some other things

Extract deserialize for share libs usage 32e52aa604

Add libstandardness shared library 2774e082f4

DrahtBot commented at 3:34 am on August 5, 2023: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept NACK	luke-jr

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#28539 (lib: add taproot support to libconsensus by brunoerg)
#28438 (Use serialization parameters for CTransaction by ajtowns)

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.

ariard marked this as a draft on Aug 5, 2023

DrahtBot added the label CI failed on Aug 5, 2023

luke-jr commented at 4:57 pm on August 5, 2023: member

Concept NACK.

Bitcoin Core transaction relay policy.

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.

ariard commented at 0:42 am on August 7, 2023: member

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.

ajtowns commented at 0:26 am on August 11, 2023: contributor

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.

Better to use the testmempoolaccept rpc for this than introduce a new api: mempool policy is a per-node decision, so having an actual node to ask is fairly natural. If you have a particular expectation on what “economically rational” nodes do, then configure that node be an “economically rational” one.

ariard commented at 1:24 am on August 30, 2023: member

Better to use the testmempoolaccept rpc for this than introduce a new api: mempool policy is a per-node decision, so having an actual node to ask is fairly natural. If you have a particular expectation on what “economically rational” nodes do, then configure that node be an “economically rational” one.

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.

TheBlueMatt commented at 8:09 pm on August 30, 2023: contributor

Better to use the testmempoolaccept rpc for this than introduce a new api: mempool policy is a per-node decision, so having an actual node to ask is fairly natural. If you have a particular expectation on what “economically rational” nodes do, then configure that node be an “economically rational” one.

Look at this a different way - this provides testmempoolaccept as a library. libbitcoinconsensus is incredibly useful in tests, as it avoids all the complexity of actually spawning hundreds of Bitcoin Core instances in tests.

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.

in src/policy/libstandardness.cpp:64 in 2774e082f4

59+            .notifications = notifications,
60+        };
61+        const auto chainman = std::make_unique<ChainstateManager>(chainman_opts, blockman_opts);
62+
63+        const auto dummy_chainstate = std::make_unique<Chainstate>(dummy_mempool.get(), chainman->m_blockman, *chainman);
64+        //TODO: initialize dummy_chainstate with chain tip as seen by the caller and spent_UTXO

MarcoFalke commented at 10:29 am on September 3, 2023:

How is that different from libbitcoinkernel? Seems odd to add a library for something that can be done with libbitcoinkernel?

ariard commented at 7:14 pm on September 12, 2023:

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.

MarcoFalke commented at 10:32 am on September 3, 2023: member

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?

ariard commented at 7:17 pm on September 12, 2023: member

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?

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.

fanquake commented at 8:37 am on September 27, 2023: member

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.

MarcoFalke commented at 8:48 am on September 27, 2023: member

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.

ariard commented at 4:27 pm on September 27, 2023: member

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.

ariard closed this on Sep 27, 2023

Libstandardness (edition 2023) #28220

Reviews

Conflicts