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

c-lightning backend #6

Closed
wants to merge 4 commits into from
Closed

c-lightning backend #6

wants to merge 4 commits into from

Conversation

fiatjaf
Copy link

@fiatjaf fiatjaf commented Apr 13, 2019

No description provided.

@fiatjaf
Copy link
Author

fiatjaf commented Apr 13, 2019

I can run this with a regtest lightningd and see my only channel in the list. I can select it and see more info. Should lntop have more options (I didn't write the invoice generation and payment stuff yet) besides these (I didn't try it with lnd)?

However, I tried to run the same binaries with a mainnet lightningd that has about 20 channels and it crashes upon start (actually I don't know if it crashes, it just flashes the screen and returns to the terminal, nothing is printed, but it doesn't exit, just stays there, doing nothing). The logs don't show anything (I don't know what should I write in the logs config, currently it says "production" because I tried other values and that caused errors).

I need some help!

@edouardparis
Copy link
Owner

edouardparis commented Apr 13, 2019

Thank you for your wrok
lntop list only channels balance and current information. For log please, change logger type value in config for development and add app.logger.Info(...) everywhere for debug.

I would be very happy to add this backend !
I just need some time to try lightningd

@fiatjaf
Copy link
Author

fiatjaf commented Apr 13, 2019

So what are these methods for generating, watching and paying invoices in the backend interface? Do you plan on implementing these things in the future?

@edouardparis
Copy link
Owner

Yes, it was mostly for testing, but they may be needed in the future for rebalancing channels. You can implement these methods with a return nil, nil for the moment. I will add a switch in pubsub to use only some methods according to backend.

@fiatjaf
Copy link
Author

fiatjaf commented Apr 14, 2019

Pardon the digression, but isn't it a good idea to implement that basic functionality and so people could use lntop as their wallet instead of Zap or Spark? Maybe there could be a separate screen for paying, receiving and listing transactions, accessible through F5 and F5 or something like that, I don't know.

@edouardparis
Copy link
Owner

Yes it would be great, I was thinking about a sidebar menu toggled by F2 or another key with different link to different panels, but I am trying to have a stable and minimal version of the software first.
I you want to implement invoice creation etc, in this PR go ahead. It is possible to add temporary cli commands for test.

@fiatjaf
Copy link
Author

fiatjaf commented Apr 15, 2019

Last request: can you comment or document somehow where the methods expect satoshis, millisatoshis or bitcoin units? Or is it satoshis everywhere? c-lightning deals with millisatoshis in most places, satoshis in some places. It can be confusing.

@edouardparis
Copy link
Owner

For the moment it is satoshis everywhere. I will add a CONTRIBUTING.md soon 👍

@fiatjaf
Copy link
Author

fiatjaf commented Apr 17, 2019

As far as I could grasp, GetChannelInfo() is called with the same structs that were previously returned from ListChannels(), is this right?

I'm setting Policy1 and Policy2 with the fields I have available in the ListChannels() call, and then filling them with more info on GetChannelsInfo(), but the prefilled data is showing as zero on the UI. What am I missing?

network/backend/lightningd/lightningd.go Show resolved Hide resolved
network/backend/lightningd/lightningd.go Outdated Show resolved Hide resolved
network/backend/lightningd/lightningd.go Outdated Show resolved Hide resolved
network/network.go Outdated Show resolved Hide resolved
@edouardparis
Copy link
Owner

I think this check does not pass
https://github.com/edouardparis/lntop/blob/master/ui/models/models.go#L60
maybe I should add a clause || channel.LastUpdate.Before(channels[i].LastUpdate)

@fiatjaf
Copy link
Author

fiatjaf commented Apr 20, 2019

type Invoice struct {
// Index: index of this invoice.
// Each newly created invoice will increment
// this index making it monotonically increasing.
Index uint64
Amount int64
// AmountPaid: The amount that was accepted for this invoice, in satoshis.
AmountPaid int64
// AmountPaidInMSat: The amount that was accepted for this invoice, in milli satoshis.
AmountPaidInMSat int64
Description string
// RPreImage: The hex-encoded preimage (32 byte) which will allow
// settling an incoming HTLC payable to this preimage
RPreImage []byte
// RHash: The hash of the preimage.
RHash []byte
// PaymentRequest: A bare-bones invoice for a payment within the Lightning Network.
// With the details of the invoice, the sender has all the data
// necessary to send a payment to the recipient.
PaymentRequest string
DescriptionHash []byte
// FallBackAddress: Fallback on-chain address.
FallBackAddress string
Settled bool
CreationDate int64
SettleDate int64
Expiry int64
// CLTVExpiry: Delta to use for the time-lock of the CLTV extended to the final hop.
CLTVExpiry uint64
// Private: Whether this invoice should include routing hints for private channels.
Private bool
}

Some of these fields don't make sense in the context of c-lightning. For example, there isn't an "index" that monotonically increases, instead there are unique "labels" that must be manually provided. I don't know if it makes sense for lntop to keep track of these things somehow or if we could just discard this label.


func (b *Backend) GetInvoice(ctx context.Context, hash string) (*models.Invoice, error) {
invoice, ok := b.invoices[hash]
if !ok {
return nil, errors.New("unable to locate invoice")
}
return &invoice, nil
}

Seeing this now I think definitely lntop should keep track of the label. c-lightning only allows locating the invoice with its label, not with the payment hash. Maybe this function should be given a pointer to a models.Invoice with either the hash or the label and the backend would use whatever information it has to locate the invoice.

@fiatjaf
Copy link
Author

fiatjaf commented Apr 20, 2019

type Channel struct {
ID uint64
Status int
RemotePubKey string
ChannelPoint string
Capacity int64
LocalBalance int64
RemoteBalance int64
CommitFee int64
CommitWeight int64
FeePerKiloWeight int64
UnsettledBalance int64
TotalAmountSent int64
TotalAmountReceived int64
ConfirmationHeight *uint32
UpdatesCount uint64
CSVDelay uint32
Private bool
PendingHTLC []*HTLC
LastUpdate *time.Time
Node *Node
Policy1 *RoutingPolicy
Policy2 *RoutingPolicy
}

Here also there are some fields that aren't available on c-lightning, like the information about the on-chain fees for the channel. Maybe we could just ignore them, but I'm saying so you can decide.

Does Policy1 and Policy2 refer to Node1 and Node2 from lnd? I believe the assignment of 1 and 2 is made based on the lexicographical order of the node ids, which is basically arbitrary and meaningless. On c-lightning too there's a similar arbitrary thing. I'm not sure how you're using the data of Policy1 and Policy2, but perhaps we could make it more useful by naming these policies as ToUs, ToThem or something like that.

@fiatjaf
Copy link
Author

fiatjaf commented Apr 20, 2019

For the moment it is satoshis everywhere. I will add a CONTRIBUTING.md soon +1

I don't know. In some places It can't be. For example,

Fee int64
should have a millisatoshi component, but as an int64 there's no way to specify msats there. If you're just getting the data from lnd for this currently I would bet lnd is giving that piece of data as msat instead of sat, right?

@fiatjaf
Copy link
Author

fiatjaf commented Apr 20, 2019

What's the difference between an Invoice and a PayReq?

It seems to me that they should be the same, albeit when it is generated locally we should have more info about it (like the preimage), right?

@edouardparis
Copy link
Owner

You are right for the fee and the choice of millisatoshis, I thought milli-satoshis were a premature optimisation by the lnd team, but even lightningd use it. I will change fields type for float64: the basic unit will be satoshis (ex 1337.42 sats)

Yes lnd and lightningd does not have same fields for managing channels, I will try to find a easy way to converge to a same interface. Maybe all fields could be pointer and a nil value means the field is not supported by the current backend ?

Invoice and Payreq are different. Invoice include the payreq information for payment and the information of the settlement (settle date and chosen route + fee). Some fields are missing in the current Invoice model.

@edouardparis
Copy link
Owner

Hello @fiatjaf, thank you again for your commitment. I am not a big fan of gjson.Result and I was thinking about using https://github.com/niftynei/glightning for the c-lightning backend, which could help for the creation of a plugin for events subscriptions. I am on my way to try c-lightning and learn about the different fields and methods.

@fiatjaf fiatjaf closed this May 16, 2019
@fiatjaf fiatjaf deleted the lightningd branch May 16, 2019 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants