Skip to content
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

simple cross-chain hub demo #46

Merged
merged 3 commits into from
Jul 15, 2024
Merged

Conversation

doitian
Copy link
Member

@doitian doitian commented Jun 7, 2024

Specification: #85

Future Works: #88

@doitian doitian force-pushed the feature/cch branch 16 times, most recently from 67d40ea to 10b175b Compare June 18, 2024 11:56
@doitian doitian marked this pull request as ready for review June 18, 2024 12:01
@doitian doitian requested a review from quake June 18, 2024 12:02
This was referenced Jun 19, 2024
@doitian doitian marked this pull request as draft June 27, 2024 00:59
src/rpc/cch.rs Show resolved Hide resolved
src/cch/actor.rs Outdated Show resolved Hide resolved
async fn create_invoices_client(
&self,
) -> Result<InvoicesClient, lnd_grpc_tonic_client::channel::Error> {
create_invoices_client(
Copy link
Collaborator

@contrun contrun Jun 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that we need to create a new gRPC client for each tlc settled. It would be better if we can somehow reuse the same connection. I don't know if tonic can automatically do that for us. This is not an urgent or important feature for now. But I think we can document that.

src/cch/config.rs Show resolved Hide resolved
src/cch/actor.rs Outdated Show resolved Hide resolved
src/cch/actor.rs Show resolved Hide resolved
src/cch/config.rs Outdated Show resolved Hide resolved
src/cch/error.rs Outdated Show resolved Hide resolved

let mut client = state.lnd_connection.create_router_client().await?;
// TODO: set a fee
let mut stream = client.send_payment_v2(req).await?.into_inner();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will block the actor from processing other messages while issuing gRPC request. I think we can spawn a subtask and make this non-blocking. The problem is that after received gRPC response, we need to update the state. We can do that by add a message to the cch actor which can then process state update by handling this message.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is aync. Awaiting on a inner future will not block the actor.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Async function means that this code snippet will not block current thread, but this will still make the actor unable to process any other messages unrelated to this one, e.g. other SendBtc messages. This is because the function handle of this actor has still not returned, thus we have no chance to handle any other messages queued in the mailbox.

src/cch/actor.rs Show resolved Hide resolved
Copy link
Collaborator

@contrun contrun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is a little bit centered at BTC<->wBTC (an xDUT token with 1-1 pegging to BTC). I am wondering how easy it is to tweak code in this PR to support other assets. To be clear, we currently are only interested in BTC<->wBTC exchange. But the code can be make more generic.

tests/bruno/e2e/cross-chain-hub/07-node1-add-tlc.bru Outdated Show resolved Hide resolved
src/cch/orders_db.rs Show resolved Hide resolved
@doitian doitian force-pushed the feature/cch branch 2 times, most recently from 0fa868e to 6434c53 Compare June 28, 2024 08:34
@doitian
Copy link
Member Author

doitian commented Jun 28, 2024

This PR is a little bit centered at BTC<->wBTC (an xDUT token with 1-1 pegging to BTC). I am wondering how easy it is to tweak code in this PR to support other assets. To be clear, we currently are only interested in BTC<->wBTC exchange. But the code can be make more generic.

The code base before this PR supports CKB <-> BTC by configuring a ratio. A similar change can be applied to support other xUDT.

@contrun

This comment was marked as resolved.

@doitian

This comment was marked as resolved.

@doitian
Copy link
Member Author

doitian commented Jul 5, 2024

While trying to demo the functionality of cross chain hub, I also added a few requests to close the channels in both lightning network and fiber network. Can you also add these to current PR? I think it would make the e2e test more complete. Here is my additional bruno files. https://github.com/nervosnetwork/cfn-node/tree/afbfc1e5a1c2dc2f900dae57f00ba1ac86f4927c/tests/bruno/e2e/cross-chain-hub

You can send a PR to https://github.com/doitian/cfn-node/tree/feature/cch

doitian#2

@doitian
Copy link
Member Author

doitian commented Jul 5, 2024

Blocking, see #123

@doitian doitian force-pushed the feature/cch branch 2 times, most recently from 1cdea9b to 821a3df Compare July 5, 2024 06:19
@quake quake merged commit 3a367ed into nervosnetwork:main Jul 15, 2024
12 checks passed
@doitian doitian deleted the feature/cch branch July 18, 2024 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants