Lightning Service Provider Specification #206
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ErikDeSmedt
force-pushed
the
2023_07_12/lsps_logic
branch
from
7cf6ddd
to
9ae0fba
Compare
cdecker
reviewed
Jul 12, 2023
|
I think if you have a small test that sends and receives messages from one node to the other we can already merge this, so we can pipeline changes in and avoid large PRs :-) |
ErikDeSmedt
force-pushed
the
2023_07_12/lsps_logic
branch
from
2da3fbc
to
11433fa
Compare
The code to serialize and deserialize messages in the JSON-rpc. I have decided not to include a separate json_rpc crate. The first reason is that we will be using noise_xk as a transport layer. I did not want to include unneeded dependencies that allow us to do json_rpc over https, tcp, web-sockets. The second reason is that LSPS-1 depends on a subset of json_rpc 2.0. Batching isn't allowed, a request id MUST be provided, etc... It is quite straight-forward to implement the logic using these constraints. The third reason is that we only need to implement at the client for a specific situation. I would like to enforce some parts in the type-system which some json_rpc crates seem to enforce at run-time.
LSPS0 describes the transport-layer used by nodes should interact with their Lightning Service Provider. https://github.com/BitcoinAndLightningLayerSpecs/lsp/tree/main/LSPS0 The general idea is that it is a JSON-RPC 2.0 which is implemented over BOLT-8 messages.
cdecker suggested to automatically derive the From-trait using this_error during code-review.
The goal is to achieve language bindings (foreign function interface).
The JSON-rpc code is strongly typed and relies heavily on generics.
This is tricky to write language bindings. Using `py03` we cannot use
the `#[pyclass]` macro to generate the bindings. (This is fundamentally
broken, it is not a pyo3 limitation).
To get the language bindings working we must ensure that all the
required types are instantiated at compile-time (See `crate::lsps::message`).
The other-challenge is that at run-time only the method-name is
available. And each method-name translates to a `JsonRpcMethod<'a, I, O,
E>` of which we do not know the type.
We cannot write a handle-request function because we do not know the
types.
```
fn handle_request(&self, method_name : &str, params : ????) {
}
```
We solve this trick using a trait that does type-erasure.
The trait `JsonRpcMethodErased` implements the same functionality.
But instead of working with the concrete type it always uses
`serde_json::Value` as representation.
The `JsonRpcMethodUnerased` takes it one step further.
It allows us to write library components that can either use the generic
or erased-methods.
I also removed the life-time from JSonRPCMethod. I do not see any
use-case for creating new RpcMethods at run-time and managing all the
life-times is really annoying.
I've implemented an integration test for LSPS0 which replicates the list-protocols behavior. To ensure our test-case is reproducible I have removed the dependency on `generate_random_rpc_id`. When writing tests you can use `rpc_call_with_id`. In production the use of `rpc_call` is recommended. This split is present in multiple methods for the creation of JsonRpcRequests. Previously, I made the mistake of misnaming the `jsonrpc` field. The spurious underscore is removed everywhere. I used the ```serde_json::from_value()``` in transport.rs when deserializing a JSON-rpc response. This did not work as expected when the type has been erased. I replaced this to ``` method.parse_json_response_value(,,.) ``` See transport.rs line 139. This ensures the underlying types are checked even when a type-erased version is used.
The test was flakey. The key problem is that the LSPS-server send the list-protocols message before the LSP-client could send the message
cdecker
force-pushed
the
2023_07_12/lsps_logic
branch
from
d8b5cfe
to
fd97160
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Draft on how to implement the LSPS-specification in greenlight.
Todo: