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.

Sign up for GitHub

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

Jump to bottom

Use the peerswaprpc proto file for shared serialization #231

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

Use the peerswaprpc proto file for shared serialization #231

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8a1e08a
cln: Use the peerswaprpc proto file for shared serialization
YusukeShimizu Aug 31, 2023
ed75d67
rpc: Exported newScidFromString to call from cln
YusukeShimizu Aug 31, 2023
7fd8f9b
test: Changed to use peerswaprpc response
YusukeShimizu Aug 31, 2023
de30b18
multi: added short channel id for PeerSwapPeerChannel
YusukeShimizu Sep 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 20 additions & 48 deletions clightning/clightning_commands.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func (g *LiquidGetAddress) Call() (jrpc2.Result, error) {
return nil, err
}
log.Infof("[Wallet] Getting lbtc address %s", res)
return &GetAddressResponse{LiquidAddress: res}, nil
return &peerswaprpc.GetAddressResponse{Address: res}, nil
}

func (g *LiquidGetAddress) Description() string {
Expand All @@ -70,10 +70,6 @@ func (g *LiquidGetAddress) Get(client *ClightningClient) jrpc2.ServerMethod {
}
}

type GetAddressResponse struct {
LiquidAddress string `json:"lbtc_address"`
}

// GetBalance returns the liquid balance
type LiquidGetBalance struct {
cl *ClightningClient
Expand Down Expand Up @@ -101,8 +97,8 @@ func (g *LiquidGetBalance) Call() (jrpc2.Result, error) {
if err != nil {
return nil, err
}
return &GetBalanceResponse{
res,
return &peerswaprpc.GetBalanceResponse{
SatAmount: res,
}, nil
}

Expand All @@ -120,10 +116,6 @@ func (g *LiquidGetBalance) Get(client *ClightningClient) jrpc2.ServerMethod {
}
}

type GetBalanceResponse struct {
LiquidBalance uint64 `json:"lbtc_balance_sat"`
}

// LiquidSendToAddress sends
type LiquidSendToAddress struct {
Address string `json:"address"`
Expand Down Expand Up @@ -166,11 +158,7 @@ func (s *LiquidSendToAddress) Call() (jrpc2.Result, error) {
log.Infof("Error sending to address %v", err)
return nil, err
}
return &SendToAddressResponse{TxId: res}, nil
}

type SendToAddressResponse struct {
TxId string `json:"txid"`
return &peerswaprpc.SendToAddressResponse{TxId: res}, nil
}

func (s *LiquidSendToAddress) Description() string {
Expand Down Expand Up @@ -596,7 +584,7 @@ func (l *ListPeers) Call() (jrpc2.Result, error) {
return nil, err
}

peerSwappers := []*PeerSwapPeer{}
peerSwappers := []*peerswaprpc.PeerSwapPeer{}
for _, peer := range peers {
if p, ok := polls[peer.Id]; ok {
swaps, err := l.cl.swaps.ListSwapsByPeer(peer.Id)
Expand Down Expand Up @@ -632,17 +620,17 @@ func (l *ListPeers) Call() (jrpc2.Result, error) {
}
}

peerSwapPeer := &PeerSwapPeer{
peerSwapPeer := &peerswaprpc.PeerSwapPeer{
NodeId: peer.Id,
SwapsAllowed: p.PeerAllowed,
SupportedAssets: p.Assets,
AsSender: &SwapStats{
AsSender: &peerswaprpc.SwapStats{
SwapsOut: SenderSwapsOut,
SwapsIn: SenderSwapsIn,
SatsOut: SenderSatsOut,
SatsIn: SenderSatsIn,
},
AsReceiver: &SwapStats{
AsReceiver: &peerswaprpc.SwapStats{
SwapsOut: ReceiverSwapsOut,
SwapsIn: ReceiverSwapsIn,
SatsOut: ReceiverSatsOut,
Expand All @@ -651,14 +639,18 @@ func (l *ListPeers) Call() (jrpc2.Result, error) {
PaidFee: paidFees,
}

peerSwapPeerChannels := []*PeerSwapPeerChannel{}
peerSwapPeerChannels := []*peerswaprpc.PeerSwapPeerChannel{}
for _, channel := range peer.Channels {
if c, ok := fundingChannels[channel.ShortChannelId]; ok {
peerSwapPeerChannels = append(peerSwapPeerChannels, &PeerSwapPeerChannel{
ChannelId: c.ShortChannelId,
scid, err := peerswaprpc.NewScidFromString(c.ShortChannelId)
if err != nil {
return nil, err
}
peerSwapPeerChannels = append(peerSwapPeerChannels, &peerswaprpc.PeerSwapPeerChannel{
ChannelId: scid.ToUint64(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe a scid it is more intuitive and "human readable". Do you think it makes sense to update the proto message

peerswap/peerswaprpc/peerswaprpc.proto

Lines 152 to 157 in 3201594

message PeerSwapPeerChannel {
uint64 channel_id = 1;
uint64 local_balance = 2;
uint64 remote_balance = 3;
bool active = 5;
}
to take a scid (string) instead of a channel id?

Copy link
Contributor Author

@YusukeShimizu YusukeShimizu Sep 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I think there is a concern that replacing the channel id with a short channel id would be a breaking change and have a impact.
I think there will be a no problem if both channel id and short channel id are set, so I just add a short channel id.
de30b18

Copy link
Contributor

Choose a reason for hiding this comment

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

Internally we use the scid so there should no be a problem here. Changing from channel_id to scid would "break" the api that is just consumed by peerswapcli and maybe RTL so far. Vice versa for a change from scid to channel_id on the core-lightning plugin. We either way have to go with on change on the user facing api 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, this may have caused a little confusion, so I will state it again.

I considered the following two way.
I personally believe that 1 is the way that will have the least impact, which do you think better?

  1. Add string short_channel_id = 6;.
    The current change(de30b18) does this.
message PeerSwapPeerChannel {
    uint64 channel_id = 1;
    uint64 local_balance = 2;
    uint64 remote_balance = 3;
    bool active = 5;
    string short_channel_id = 6;
}
  1. Update uint64 uint channel_id = 1; to string short_channel_id = 1;
message PeerSwapPeerChannel {
    string short_channel_id = 1;
    uint64 local_balance = 2;
    uint64 remote_balance = 3;
    bool active = 5;
}

LocalBalance: c.OurAmountMilliSatoshi.MSat() / 1000,
RemoteBalance: (c.AmountMilliSatoshi.MSat() - c.OurAmountMilliSatoshi.MSat()) / 1000,
State: c.State,
Active: channelActive(c.State),
})
}
}
Expand All @@ -670,6 +662,10 @@ func (l *ListPeers) Call() (jrpc2.Result, error) {
return peerSwappers, nil
}

func channelActive(state string) bool {
return state == "CHANNELD_NORMAL"
}

type GetSwap struct {
SwapId string `json:"swap_id"`
cl *ClightningClient
Expand Down Expand Up @@ -1115,30 +1111,6 @@ func (c ListConfig) LongDescription() string {
return c.Description()
}

type PeerSwapPeerChannel struct {
ChannelId string `json:"short_channel_id"`
LocalBalance uint64 `json:"local_balance"`
RemoteBalance uint64 `json:"remote_balance"`
State string `json:"state"`
}

type SwapStats struct {
SwapsOut uint64 `json:"total_swaps_out"`
SwapsIn uint64 `json:"total_swaps_in"`
SatsOut uint64 `json:"total_sats_swapped_out"`
SatsIn uint64 `json:"total_sats_swapped_in"`
}

type PeerSwapPeer struct {
NodeId string `json:"nodeid"`
SwapsAllowed bool `json:"swaps_allowed"`
SupportedAssets []string `json:"supported_assets"`
Channels []*PeerSwapPeerChannel `json:"channels"`
AsSender *SwapStats `json:"sent,omitempty"`
AsReceiver *SwapStats `json:"received,omitempty"`
PaidFee uint64 `json:"total_fee_paid,omitempty"`
}

// checkFeatures checks if a node runs the peerswap Plugin
func checkFeatures(features []byte, featureBit int64) bool {
featuresInt := big.NewInt(0)
Expand Down
4 changes: 2 additions & 2 deletions peerswaprpc/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -546,7 +546,7 @@ func (p *PeerswapServer) AllowSwapRequests(ctx context.Context, request *AllowSw
}

func PrettyprintFromServiceSwap(swap *swap.SwapStateMachine) *PrettyPrintSwap {
scid, err := newScidFromString(swap.Data.GetScid())
scid, err := NewScidFromString(swap.Data.GetScid())
if err != nil {
log.Debugf("Could not parse scid from %s: %v", scid, err)
}
Expand Down Expand Up @@ -574,7 +574,7 @@ func PrettyprintFromServiceSwap(swap *swap.SwapStateMachine) *PrettyPrintSwap {
}
}

func newScidFromString(scid string) (*lnwire.ShortChannelID, error) {
func NewScidFromString(scid string) (*lnwire.ShortChannelID, error) {
scid = strings.ReplaceAll(scid, "x", ":")
parts := strings.Split(scid, ":")
if len(parts) != 3 {
Expand Down
12 changes: 6 additions & 6 deletions test/setup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -442,10 +442,10 @@ func clnclnElementsSetup(t *testing.T, fundAmt uint64) (*testframework.BitcoinNo

// Give liquid funds to nodes to have something to swap.
for _, lightningd := range lightningds {
var result clightning.GetAddressResponse
var result peerswaprpc.GetAddressResponse
lightningd.Rpc.Request(&clightning.LiquidGetAddress{}, &result)
_ = liquidd.GenerateBlocks(20)
_, err = liquidd.Rpc.Call("sendtoaddress", result.LiquidAddress, 10., "", "", false, false, 1, "UNSET")
_, err = liquidd.Rpc.Call("sendtoaddress", result.Address, 10., "", "", false, false, 1, "UNSET")
require.NoError(t, err)
}

Expand Down Expand Up @@ -734,10 +734,10 @@ func mixedElementsSetup(t *testing.T, fundAmt uint64, funder fundingNode) (*test
}

// Give liquid funds to nodes to have something to swap.
var lar clightning.GetAddressResponse
var lar peerswaprpc.GetAddressResponse
cln.Rpc.Request(&clightning.LiquidGetAddress{}, &lar)
_ = liquidd.GenerateBlocks(20)
_, err = liquidd.Rpc.Call("sendtoaddress", lar.LiquidAddress, 10., "", "", false, false, 1, "UNSET")
_, err = liquidd.Rpc.Call("sendtoaddress", lar.Address, 10., "", "", false, false, 1, "UNSET")
require.NoError(t, err)

r, err := peerswapd.PeerswapClient.LiquidGetAddress(context.Background(), &peerswaprpc.GetAddressRequest{})
Expand Down Expand Up @@ -778,12 +778,12 @@ type CLightningNodeWithLiquid struct {
}

func (n *CLightningNodeWithLiquid) GetBtcBalanceSat() (uint64, error) {
var response clightning.GetBalanceResponse
var response peerswaprpc.GetBalanceResponse
err := n.Rpc.Request(&clightning.LiquidGetBalance{}, &response)
if err != nil {
return 0, err
}
return response.LiquidBalance, nil
return response.GetSatAmount(), nil
}

type LndNodeWithLiquid struct {
Expand Down
Loading