# lnprototest Evolution
This document discusses the problems with lnprototest and proposes future simplifications.
## Table of Contents
- Problems
- Possible Simplifications
- Idea
- State of the Art
## Problems
### Bitcoin State Machine Problem
The main issue with lnprototest is the lack of tooling to be independent of hard-coded transactions within the code itself. [Here](https://github.com/rustyrussell/lnprototest/blob/master/lnprototest/utils/bitcoin_utils.py#L63) is an example of this. In addition, this is one of the problem that the dual funding tests are still disabled. understand all the 3k line of code required a lot of time
It would be beneficial to use an updated and well-maintained library like rust-bitcoin (to support Taproot). However, this would require creating some Python bindings.
### Lightning Crypto
Another issue we encountered is the hardcoding of the signature sent by the lightning implementation. We currently face difficulties in easily creating and sending back these signatures, particularly in the case of `announcement_signatures`.
During the supervision of an LDK Summer of Bitcoin project and while assisting with lnprototest tests, we observed that generating a signature for an ln message using lnprototest is challenging.
For example, signing the `announcement_signatures` message appears non-trivial without encoding it.
It would be beneficial to have a library like `lightning-crypto` (possibly in Rust with C FFI bindings) that provides a convenient syntax for signing the message.
```rust
let msg = AnnouncementSignatures::default()
.channel_id(channel_id())
.short_channel_id(short_channel_id)
.on_signature(|raw_msg| lightning_crypto::sign(raw_msg))
.on_bitcoin_signature(|raw_msg| lightning_crypto::btc_signature(raw_msg));
let msg = runner.send(connection, msg);
assert!(msg.is_msg("announcement_signatures"), "Expected `announcement_signatures` message but received the `{:?}`", msg);
```
P.S: The language used here is for simplicity, and the same code can be written in Python or any other programming language.
## Possible Simplifications
The current implementation of lnprototest is based on event handling, and there are a couple of areas where simplifications can be made:
- Significant simplifications can be achieved within the lnprototest event handling mechanism.
- Code duplication is prevalent throughout lnprototest. Even if we were to write tests in a different manner, we might still encounter code duplication. To mitigate this, we should explore injecting the correct sequence workflow. Examples of this can be found in the following utils: [ln_spec_utils.py](https://github.com/rustyrussell/lnprototest/blob/master/lnprototest/utils/ln_spec_utils.py).
```python
def test_open_channel_announce_features(runner: Runner) -> None:
"""Check that the announce features works correctly"""
# Note that the `connect_to_node_helper` is returning the
# flow of event to be connected to the node helper.
connections_events = connect_to_node_helper(
runner=runner, tx_spendable=tx_spendable, conn_privkey="02"
)
test_events = [
TryAll(
# BOLT-a12da24dd0102c170365124782b46d9710950ac1 #9:
# | 20/21 | `option_anchor_outputs` | Anchor outputs
Msg("init", globalfeatures="", features=bitfield(13, 21)),
# BOLT #9:
# | 12/13 | `option_static_remotekey` | Static key for remote output
Msg("init", globalfeatures="", features=bitfield(13)),
# And not.
Msg("init", globalfeatures="", features=""),
),
]
# We merge the events and then we run lnprototest
run_runner(runner, merge_events_sequences(connections_events, test_events))
```
- Another approach we can explore is writing tests in a different manner using the same runner, where assertions are made at runtime. Rusty has proposed a proof of concept for this approach in the following repository: [lnprotest](https://github.com/rustyrussell/lnprotest). It appears promising, but we should carefully consider how we can assert on a single message.
```python
def test_init_advertize_option_static_remotekey(runner: Runner):
conn = runner.connect(connprivkey="03")
assert type(conn.recv()) is pyln.proto.Init
conn.send(pyln.proto.Init(globalfeatures="", features="")
# optionally disconnect that first one
if runner.choose([True, False]):
conn.disconnect()
conn = runner.connect(connprivkey="02")
assert type(conn.recv()) is pyln.proto.Init
conn.send(pyln.proto.Init(globalfeatures="", features="")
# BOLT-a12da24dd0102c170365124782b46d9710950ac1 #9:
# | Bits | Name | ... | Dependencies
# ...
# | 12/13 | `option_static_remotekey` |
# ...
# | 20/21 | `option_anchor_outputs` | ... | `option_static_remotekey` |
# If you support `option_anchor_outputs`, you will
# advertize option_static_remotekey.
initmsg = conn.recv()
assert initmsg is pyln.proto.Init
# If you support option_anchor_outputs you must offer it
if runner.has_option("option_anchor_outputs"):
assert initmsg.features.offers(20):
# And if you offer it, you must offer static_remotekey
if initmsg.features.offers(20):
assert initmsg.features.offers(12)
```
My opinion on this is that we can remove all the `if` statements because an `if` statement appears to be a separate test to me. This approach will make the code more readable, especially when it becomes complex.
I also propose changing the way assertions are made by returning a `Msg` object from `conn.recv()`. We can then use `assert initmsg.is("init"), "We did not receive the init message"` to validate the received message.
Regarding the code at [lnprotest/runner.py#L83](https://github.com/rustyrussell/lnprotest/blob/master/lnprotest/runner.py#L83), I have an open question: Do we really need to assert on the DAG path if we write the test using the last method mentioned? I believe we can avoid this by running the test and checking each message individually. In this case, the runner serves as a way to execute commands in a lightning implementation with a common interface.
- I believe that with the current implementation of the runner, it is possible to write assert-like tests without re-implementing the test framework from scratch. Some small hacks may be required, but this approach allows us to maintain compatibility with existing code and implementations like ldk.
## Ideas
- We can consider removing the existing python package and creating a new one that offers a more ergonomic interface. The code in the pyln-proto package is difficult to read, which makes operations like adding a tlv type challenging. I still haven't grasped how to add it myself.
- Is there a way to eliminate the requirements for custom channel keys? Alternatively, we could improve our documentation on this topic. Rewriting all the logic in ldk to inject custom channel keys was a cumbersome process. However, it might be one of the easier tasks to address.
- Can we find a way to move away from hardcoding the logic of bitcoin core? With an assert-like test approach, we could manage it differently outside of the runner.
## Conclusion
I don't believe that rewriting the entire test workflow should be the top priority for lnprototest. However, we should invest time in creating better tooling to manage bitcoin transactions and lightning cryptography.
Lnprototest is currently challenging to work with. The event-based framework makes the code difficult to read, and the rest of the codebase is complex, especially for those unfamiliar with the pyln package. Once we address these tooling issues, we can transition to a more asserting-based approach. Furthermore, we need more users of lnprototest to provide feedback. While the event workflow may deter some users, I haven't received any requests for assistance. Additionally, ldk has implemented the lnprototest runner as part of the Summer of Bitcoin, which indicates that the framework is not so bad that it needs to be rewritten from scratch.
## State of the Art
- [lnprototest as a library](https://app.radicle.xyz/seeds/rad.hedwing.dev/rad:zLEvMXwoqiUV9GdZfFZDhDWamPsx/tree/main/tests/lnprototest/lampo_lnprototest/runner.py)
- [Rusty prototype](https://github.com/rustyrussell/lnprotest)
- [Cristian prototype](https://github.com/cdecker/lnpt)
Sign in with Wallet
Connect another wallet