lightningnetwork / lnd

Lightning Network Daemon ⚡️
MIT License
7.64k stars 2.07k forks source link

[feature]: Append the listchannels command with the short_chan_id representation (XXXX:XX:X) besides the 8 byte integer one. #8650

Open ziggie1984 opened 5 months ago

ziggie1984 commented 5 months ago

Currently we only have the 8 byte representation in the listchannels output, but we have several parts in the logs where we would only log the short channel id in a different format (block:transaction:output) for an easier analysis it makes sense to also show this representation when querying the channels.

Moreover I think we can add the Channel ID as well, so we have all possible representations of the channel. This means we should depict the real chan_id in the listchannels output (chan_id : https://github.com/lightning/bolts/blob/master/02-peer-protocol.md#definition-of-channel_id) and two short_chan_id representations (integer64 and block:transaction:output) next to it.

andiz2 commented 5 months ago

Hi @ziggie1984 !

I can have a look at the listchannels and do the asked changes. Would come back with updates soon 😄

andiz2 commented 5 months ago

At the moment struggleing to install the dependencies... I run 'make install' after changed the go version in go.mod to 1.21 ..One of them are: chainntnfs/best_block_view.go:30:25: undefined: atomic.Pointer chainntnfs/mempool.go:27:18: undefined: atomic.Uint64 note: module requires Go 1.21

google.golang.org/grpc

/home/beast/go/pkg/mod/google.golang.org/grpc@v1.59.0/server.go:2082:14: undefined: atomic.Int64 note: module requires Go 1.19

github.com/lightningnetwork/lnd/lnwire

lnwire/accept_channel.go:269:33: cannot use fn.a (type tlv.RecordT[github.com/lightningnetwork/lnd/tlv.tlvType4,"".Musig2Nonce]) as type go.shape.struct { github.com/lightningnetwork/lnd/tlv.recordType uint8; Val [66]uint8 }_0 in assignment lnwire/channel_ready.go:95:37: cannot use fn.a (type tlv.RecordT[github.com/lightningnetwork/lnd/tlv.tlvType4,"".Musig2Nonce]) as type go.shape.struct { github.com/lightningnetwork/lnd/tlv.recordType uint8; Val [66]uint8 }_0 in assignment lnwire/channel_reestablish.go:217:33: cannot use fn.a (type tlv.RecordT[github.com/lightningnetwork/lnd/tlv.tlvType4,"".Musig2Nonce]) as type go.shape.struct { github.com/lightningnetwork/lnd/tlv.recordType uint8; Val [66]uint8 }_0 in assignment lnwire/closing_complete.go:66:37: cannot use fn.a (type tlv.RecordT[github.com/lightningnetwork/lnd/tlv.tlvType1,"".Sig]) as type go.shape.struct { github.com/lightningnetwork/lnd/tlv.recordType uint8; Val struct { "".bytes [64]uint8; "".sigType "".sigType } }_0 in assignment lnwire/closing_complete.go:69:37: cannot use fn.a (type tlv.RecordT[github.com/lightningnetwork/lnd/tlv.tlvType2,"".Sig]) as type go.shape.struct { github.com/lightningnetwork/lnd/tlv.recordType uint8; Val struct { "".bytes [64]uint8; "".sigType "".sigType } }_0 in assignment lnwire/closing_complete.go:72:38: cannot use fn.a (type tlv.RecordT[github.com/lightningnetwork/lnd/tlv.tlvType3,"".Sig]) as type go.shape.struct { github.com/lightningnetwork/lnd/tlv.recordType uint8; Val struct { "".bytes [64]uint8; "".sigType "".sigType } }_0 in assignment lnwire/closing_signed.go:87:33: cannot use fn.a (type tlv.RecordT[github.com/lightningnetwork/lnd/tlv.tlvType6,"".PartialSig]) as type go.shape.struct { github.com/lightningnetwork/lnd/tlv.recordType uint8; Val struct { Sig github.com/decred/dcrd/dcrec/secp256k1/v4.ModNScalar } }_0 in assignment lnwire/commit_sig.go:92:33: cannot use fn.a (type tlv.RecordT[github.com/lightningnetwork/lnd/tlv.tlvType2,"".PartialSigWithNonce]) as type go.shape.struct { github.com/lightningnetwork/lnd/tlv.recordType uint8; Val struct { "".PartialSig; Nonce "".Musig2Nonce } }_0 in assignment lnwire/funding_created.go:103:33: cannot use fn.a (type tlv.RecordT[github.com/lightningnetwork/lnd/tlv.tlvType2,"".PartialSigWithNonce]) as type go.shape.struct { github.com/lightningnetwork/lnd/tlv.recordType uint8; Val struct { "".PartialSig; Nonce "".Musig2Nonce } }_0 in assignment lnwire/funding_signed.go:89:33: cannot use fn.a (type tlv.RecordT[github.com/lightningnetwork/lnd/tlv.tlvType2,"".PartialSigWithNonce]) as type go.shape.struct { github.com/lightningnetwork/lnd/tlv.recordType uint8; Val struct { "".PartialSig; Nonce "".Musig2Nonce } }_0 in assignment lnwire/funding_signed.go:89:33: too many errors note: module requires Go 1.21

modernc.org/sqlite/lib

/home/beast/go/pkg/mod/modernc.org/sqlite@v1.29.5/lib/sqlite_linux_amd64.go:24762:51: undefined: libc.Xpread note: module requires Go 1.20

Does anyone encountered some of these before?

guggero commented 5 months ago

That sounds like you didn't install Go 1.21 or later correctly. Make sure you uninstalled any previous versions fully.

andiz2 commented 5 months ago

I will uninstall it and add it again.

andiz2 commented 5 months ago

sudo make install [sudo] password for beast: fatal: No names found, cannot describe anything. /bin/sh: 1: go: not found expr: syntax error: unexpected argument ‘21’ /bin/sh: 1: go: not found Installing lnd and lncli. go install -v -tags="" -ldflags=" -s -w -buildid= -X github.com/lightningnetwork/lnd/build.Commit=" github.com/lightningnetwork/lnd/cmd/lnd /bin/sh: 1: go: not found make: *** [Makefile:130: install-binaries] Error 127 beast@DESKTOP-TFSG0II:/mnt/c/Users/admin/Documents/code go/lighningNNNN/lnd$ go version go version go1.21.9 linux/amd64 beast@DESKTOP-TFSG0II:/mnt/c/Users/admin/Documents/code go/lighningNNNN/lnd$

I've reinstalled correct version for go but I still have some issues..

mohamedawnallah commented 5 months ago

The use of sudo typically resets environment variables for security reasons. This behavior can be adjusted with the env_keep directive in the sudoers file. Alternatively, you could try the following command to include the current PATH or the path where your go binary located:

sudo -E env "PATH=$PATH" make install
andiz2 commented 5 months ago

I've installed correctly now. Ran tests and still got some tests failing --- FAIL: TestLightningWalletBitcoindRPCPolling (13.20s) --- FAIL: TestLightningWalletBitcoindRPCPolling/btcwallet/bitcoind-rpc-polling:change_output_spend_confirmation (0.05s) test_interface.go:2222: Error Trace: /mnt/c/Users/admin/Documents/code go/lighningNNNN/lnd/lnwallet/test/test_interface.go:175 /mnt/c/Users/admin/Documents/code go/lighningNNNN/lnd/lnwallet/test/test_interface.go:2222 /mnt/c/Users/admin/Documents/code go/lighningNNNN/lnd/lnwallet/test/test_interface.go:3378 Error: Received unexpected error: undefined: -8: Fee rates larger than or equal to 1BTC/kvB are not accepted Test: TestLightningWalletBitcoindRPCPolling/btcwallet/bitcoind-rpc-polling:change_output_spend_confirmation Messages: unable to send transaction --- FAIL: TestLightningWalletBitcoindZMQ (13.63s) --- FAIL: TestLightningWalletBitcoindZMQ/btcwallet/bitcoind:change_output_spend_confirmation (0.03s) test_interface.go:2222: Error Trace: /mnt/c/Users/admin/Documents/code go/lighningNNNN/lnd/lnwallet/test/test_interface.go:175 /mnt/c/Users/admin/Documents/code go/lighningNNNN/lnd/lnwallet/test/test_interface.go:2222 /mnt/c/Users/admin/Documents/code go/lighningNNNN/lnd/lnwallet/test/test_interface.go:3378 Error: Received unexpected error: undefined: -8: Fee rates larger than or equal to 1BTC/kvB are not accepted Test: TestLightningWalletBitcoindZMQ/btcwallet/bitcoind:change_output_spend_confirmation Messages: unable to send transaction FAIL FAIL github.com/lightningnetwork/lnd/lnwallet/test/bitcoind 13.664s FAIL --- FAIL: TestInvoiceRegistry (0.41s) postgres_fixture.go:70: Error Trace: /root/go/pkg/mod/github.com/lightningnetwork/lnd/sqldb@v1.0.1/postgres_fixture.go:70 /mnt/c/Users/admin/Documents/code go/lighningNNNN/lnd/invoices/invoiceregistry_test.go:121 Error: Received unexpected error: API error (400): failed to resolve image name: short-name "postgres:11" did not resolve to an alias and no unqualified-search registries are defined in "/etc/containers/registries.conf" Test: TestInvoiceRegistry Messages: Could not start resource --- FAIL: TestInvoices (0.00s) postgres_fixture.go:70: Error Trace: /root/go/pkg/mod/github.com/lightningnetwork/lnd/sqldb@v1.0.1/postgres_fixture.go:70 /mnt/c/Users/admin/Documents/code go/lighningNNNN/lnd/invoices/invoices_test.go:227 Error: Received unexpected error: API error (400): failed to resolve image name: short-name "postgres:11" did not resolve to an alias and no unqualified-search registries are defined in "/etc/containers/registries.conf" Test: TestInvoices Messages: Could not start resource FAIL FAIL github.com/lightningnetwork/lnd/invoices 0.444s make: *** [Makefile:221: unit] Error 123

I don't understand why looking for image/container since I didn't used

guggero commented 5 months ago

Fee rates larger than or equal to 1BTC/kvB are not accepted

bitcoind check again after rebasing on current master, this was just fixed (or don't use bitcoind v27.0).

The Postgres image is used for some unit tests as a test database backend.

andiz2 commented 5 months ago

What should I use instead of this? 'or don't use bitcoind v27.0'

guggero commented 5 months ago

An older version, e.g. v26.

andiz2 commented 5 months ago

Thats's good news! I will downgrade

andiz2 commented 5 months ago

-- FAIL: TestInvoiceRegistry (0.07s) postgres_fixture.go:70: Error Trace: /root/go/pkg/mod/github.com/lightningnetwork/lnd/sqldb@v1.0.1/postgres_fixture.go:70 /mnt/c/Users/admin/Documents/code go/lighningNNNN/lnd/invoices/invoiceregistry_test.go:121 Error: Received unexpected error: API error (400): failed to resolve image name: short-name "postgres:11" did not resolve to an alias and no unqualified-search registries are defined in "/etc/containers/registries.conf" Test: TestInvoiceRegistry Messages: Could not start resource --- FAIL: TestInvoices (0.00s) postgres_fixture.go:70: Error Trace: /root/go/pkg/mod/github.com/lightningnetwork/lnd/sqldb@v1.0.1/postgres_fixture.go:70 /mnt/c/Users/admin/Documents/code go/lighningNNNN/lnd/invoices/invoices_test.go:227 Error: Received unexpected error: API error (400): failed to resolve image name: short-name "postgres:11" did not resolve to an alias and no unqualified-search registries are defined in "/etc/containers/registries.conf" Test: TestInvoices Messages: Could not start resource FAIL FAIL github.com/lightningnetwork/lnd/invoices 0.114s

make: *** [Makefile:221: unit] Error 123

After downgrade make test looks like this..is there any way to resolve these?

mohamedawnallah commented 5 months ago

By the way, @andiz2, you could still contribute even if some of your test cases fail. They will run in the CI anyway. I still have some failed test cases locally, and some of them are flaky.

andiz2 commented 5 months ago

Looks more clear now! I would add 2 new fileds in the lnrpc.Channel struct: ShortChannelIdInt64 and ShortChannelIdTx I would use these fields in

func createRPCOpenChannel(r *rpcServer, dbChannel *channeldb.OpenChannel,
    isActive, peerAliasLookup bool) (*lnrpc.Channel, error) {

and populate with proper values like these ones are

channel := &lnrpc.Channel{
        Active:                isActive,
        Private:               isPrivate(dbChannel),
        RemotePubkey:          nodeID,
        ....

Then it will be good to go :) Correct me if I am wrong

mohamedawnallah commented 5 months ago

Yes and also update the release notes 👍

alexbosworth commented 5 months ago

Another representation is height x index x vout like https://github.com/lightningnetwork/lnd/pull/2432

ziggie1984 commented 5 months ago

Another representation is height x index x vout like https://github.com/lightningnetwork/lnd/pull/2432

Very good idea, I think using x instead of : makes sense because its seems to be the right format also mentioned in the BOLT Spec

Maybe a separate PR could implement this comment as well, so make sure we accept both formats in our RPC commands. https://github.com/lightningnetwork/lnd/pull/2432#issuecomment-800716282

andiz2 commented 5 months ago

On Mon, 22 Apr 2024 at 12:31, ziggieXXX @.***> wrote:

Another representation is height x index x vout like #2432 https://github.com/lightningnetwork/lnd/pull/2432

Very good idea, I think using x instead of : makes sense because its seems to be the right format also mentioned in the BOLT Spec https://github.com/lightning/bolts/blob/master/07-routing-gossip.md#definition-of-short_channel_id

Maybe a separate PR could implement this comment as well, so make sure we accept both formats in our RPC commands.

2432 (comment)

https://github.com/lightningnetwork/lnd/pull/2432#issuecomment-800716282

— Reply to this email directly, view it on GitHub https://github.com/lightningnetwork/lnd/issues/8650#issuecomment-2068925972, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEAVOTSKXN4WRRWOHFXDPWDY6TKFTAVCNFSM6AAAAABGHGSI4GVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANRYHEZDKOJXGI . You are receiving this because you were mentioned.Message ID: @.***>

Very good idea! I can implement the version with x if this is the desired one. Is not much to change. Just let me know which one should be the final version so I can commit with that

ziggie1984 commented 5 months ago

Yes, use x instead of :, as long as we present both version of the short_chan_id (integer and height x index x vout) we are fine.

andiz2 commented 5 months ago

So from what I've noticed the new command will have this output (chan_id x short_chan_id_integer64 x block x transaction x output) please confirm

andiz2 commented 5 months ago

Regarding lnrpc.Channel I see

// The unique channel ID for the channel.
    ChanId uint64 `protobuf:"varint,2,opt,name=chan_id,json=chanId,proto3" json:"chan_id,omitempty"`

that is used as ShortChannelID from what i see in

func createRPCOpenChannel...
          dbScid := dbChannel.ShortChannelID
          ChanId:                dbScid.ToUint64(),

shouldn't this be used to store real channel_id as name suggests?

andiz2 commented 5 months ago

I've added these to lnrpc.Channel:

// ChannelID is a series of 32-bytes that uniquely identifies all channels
ChannelID ChannelID `protobuf:"bytes,37,opt,name=channel_id,json=channelId,proto3" json:"channel_id,omitempty"`
// The short channel id as a string with x between the block height and tx index
ShortChannelIdStringX string `protobuf:"bytes,38,opt,name=short_channel_id_string_x,json=shortChannelIdStringX,proto3" json:"short_channel_id_string_x,omitempty"` 
}

along with methods:

func (x *Channel) GetRealChanId() uint64 {
    if x != nil {
        return x.ChannelID
    }
    return 0
}

func (x *Channel) GetShortChannelIdStringX() string {
    if x != nil {
        return x.ShortChannelIdStringX
    }
    return ""
}

and these 2 lines in the channel init in method createRPCOpenChannel

ChannelID:             chanID,
ShortChannelIdStringX: dbScid.StringX(),

I saw that the short chan integer64 is already done via

ChanId:                dbScid.ToUint64(),

I will update if there was misunderstoods 😄

ziggie1984 commented 5 months ago

When working on an issue, create a PR set it into draft and from there ask for questions when you are stuck. Otherwise it costs too much bandwidth for reviewers. Always bundle questions so that it's not a constant back and forth. Thank you in advance.

andiz2 commented 5 months ago

@ziggie1984 you might be right... Maybe I need to lurk around for some time until I have a deeper understanding of the project and then try to contribuite again... I still have one curiosity if you can make me understand.. I saw that the short chan integer64 is already done via

ChanId:                dbScid.ToUint64(),

is this var ChanId or shortChanId as the name sugest it should be chanId.. Thanks in advance.

ziggie1984 commented 5 months ago

Yes there is an inaccuracy see the description of the issue, we label the short_chan_id as chan_id in the current listchannels output and we should change that and depict the real chan_id here as well.

Moreover I think we can add the Channel ID as well, so we have all possible representations of the channel. This means we should depict the real chan_id in the listchannels output (chan_id : https://github.com/lightning/bolts/blob/master/02-peer-protocol.md#definition-of-channel_id) and two short_chan_id representations (integer64 and block:transaction:output) next to it.

Maybe I need to lurk around for some time until I have a deeper understanding of the project and then try to contribute again...

You learn the most by coding, so I would recommend to push through, learn to persist I am sure you will be able to solve this issue.