From 6075997ebccd7386d8404897a6b1cf49526a8120 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Wed, 1 Jul 2020 17:45:59 -0700 Subject: [PATCH] multi: add relative thaw height interpretation This is useful when we wish to have a channel frozen for a specific amount of blocks after its confirmation. This could also be done with an absolute thaw height, but it does not suit cases where a strict block delta needs to be enforced, as it's not possible to know for certain when a channel will be included in the chain. To work around this, we add a relative interpretation of the field, where if its value is below 500,000, then it's interpreted as a relative height. This approach allows us to prevent further database modifications to account for a relative thaw height. --- channeldb/channel.go | 32 ++++++++++++++++++++++++++++++- lnrpc/rpc.pb.go | 13 ++++++++----- lnrpc/rpc.proto | 13 ++++++++----- lnrpc/rpc.swagger.json | 4 ++-- lntest/itest/lnd_test.go | 7 +------ lnwallet/chancloser/chancloser.go | 28 ++++++++++++++++----------- rpcserver.go | 21 ++++++++++++-------- 7 files changed, 80 insertions(+), 38 deletions(-) diff --git a/channeldb/channel.go b/channeldb/channel.go index 8b0b2bff8..4f5cd4471 100644 --- a/channeldb/channel.go +++ b/channeldb/channel.go @@ -23,6 +23,13 @@ import ( "github.com/lightningnetwork/lnd/shachain" ) +const ( + // AbsoluteThawHeightThreshold is the threshold at which a thaw height + // begins to be interpreted as an absolute block height, rather than a + // relative one. + AbsoluteThawHeightThreshold uint32 = 500000 +) + var ( // closedChannelBucket stores summarization information concerning // previously open, but now closed channels. @@ -654,7 +661,8 @@ type OpenChannel struct { // ThawHeight is the height when a frozen channel once again becomes a // normal channel. If this is zero, then there're no restrictions on - // this channel. + // this channel. If the value is lower than 500,000, then it's + // interpreted as a relative height, or an absolute height otherwise. ThawHeight uint32 // TODO(roasbeef): eww @@ -2766,6 +2774,28 @@ func (c *OpenChannel) RemoteRevocationStore() (shachain.Store, error) { return c.RevocationStore, nil } +// AbsoluteThawHeight determines a frozen channel's absolute thaw height. If the +// channel is not frozen, then 0 is returned. +func (c *OpenChannel) AbsoluteThawHeight() (uint32, error) { + // Only frozen channels have a thaw height. + if !c.ChanType.IsFrozen() { + return 0, nil + } + + // If the channel's thaw height is below the absolute threshold, then + // it's interpreted as a relative height to the chain's current height. + if c.ThawHeight < AbsoluteThawHeightThreshold { + // We'll only known of the channel's short ID once it's + // confirmed. + if c.IsPending { + return 0, errors.New("cannot use relative thaw " + + "height for unconfirmed channel") + } + return c.ShortChannelID.BlockHeight + c.ThawHeight, nil + } + return c.ThawHeight, nil +} + func putChannelCloseSummary(tx kvdb.RwTx, chanID []byte, summary *ChannelCloseSummary, lastChanState *OpenChannel) error { diff --git a/lnrpc/rpc.pb.go b/lnrpc/rpc.pb.go index 088bcdead..e48aaec95 100644 --- a/lnrpc/rpc.pb.go +++ b/lnrpc/rpc.pb.go @@ -2891,7 +2891,9 @@ type Channel struct { //frozen channel doest not allow a cooperative channel close by the //initiator. The thaw_height is the height that this restriction stops //applying to the channel. This field is optional, not setting it or using a - //value of zero will mean the channel has no additional restrictions. + //value of zero will mean the channel has no additional restrictions. The + //height can be interpreted in two ways: as a relative height if the value is + //less than 500,000, or as an absolute height otherwise. ThawHeight uint32 `protobuf:"varint,28,opt,name=thaw_height,json=thawHeight,proto3" json:"thaw_height,omitempty"` // List constraints for the local node. LocalConstraints *ChannelConstraints `protobuf:"bytes,29,opt,name=local_constraints,json=localConstraints,proto3" json:"local_constraints,omitempty"` @@ -5051,10 +5053,11 @@ type ChanPointShim struct { //channel ID. PendingChanId []byte `protobuf:"bytes,5,opt,name=pending_chan_id,json=pendingChanId,proto3" json:"pending_chan_id,omitempty"` // - //This uint32 indicates if this channel is to be considered 'frozen'. A - //frozen channel does not allow a cooperative channel close by the - //initiator. The thaw_height is the height that this restriction stops - //applying to the channel. + //This uint32 indicates if this channel is to be considered 'frozen'. A frozen + //channel does not allow a cooperative channel close by the initiator. The + //thaw_height is the height that this restriction stops applying to the + //channel. The height can be interpreted in two ways: as a relative height if + //the value is less than 500,000, or as an absolute height otherwise. ThawHeight uint32 `protobuf:"varint,6,opt,name=thaw_height,json=thawHeight,proto3" json:"thaw_height,omitempty"` XXX_NoUnkeyedLiteral struct{} `json:"-"` XXX_unrecognized []byte `json:"-"` diff --git a/lnrpc/rpc.proto b/lnrpc/rpc.proto index 131ac2f62..c3096c1fc 100644 --- a/lnrpc/rpc.proto +++ b/lnrpc/rpc.proto @@ -1157,7 +1157,9 @@ message Channel { frozen channel doest not allow a cooperative channel close by the initiator. The thaw_height is the height that this restriction stops applying to the channel. This field is optional, not setting it or using a - value of zero will mean the channel has no additional restrictions. + value of zero will mean the channel has no additional restrictions. The + height can be interpreted in two ways: as a relative height if the value is + less than 500,000, or as an absolute height otherwise. */ uint32 thaw_height = 28; @@ -1665,10 +1667,11 @@ message ChanPointShim { bytes pending_chan_id = 5; /* - This uint32 indicates if this channel is to be considered 'frozen'. A - frozen channel does not allow a cooperative channel close by the - initiator. The thaw_height is the height that this restriction stops - applying to the channel. + This uint32 indicates if this channel is to be considered 'frozen'. A frozen + channel does not allow a cooperative channel close by the initiator. The + thaw_height is the height that this restriction stops applying to the + channel. The height can be interpreted in two ways: as a relative height if + the value is less than 500,000, or as an absolute height otherwise. */ uint32 thaw_height = 6; } diff --git a/lnrpc/rpc.swagger.json b/lnrpc/rpc.swagger.json index d90a5e626..8d49188cd 100644 --- a/lnrpc/rpc.swagger.json +++ b/lnrpc/rpc.swagger.json @@ -2466,7 +2466,7 @@ "thaw_height": { "type": "integer", "format": "int64", - "description": "This uint32 indicates if this channel is to be considered 'frozen'. A\nfrozen channel does not allow a cooperative channel close by the\ninitiator. The thaw_height is the height that this restriction stops\napplying to the channel." + "description": "This uint32 indicates if this channel is to be considered 'frozen'. A frozen\nchannel does not allow a cooperative channel close by the initiator. The\nthaw_height is the height that this restriction stops applying to the\nchannel. The height can be interpreted in two ways: as a relative height if\nthe value is less than 500,000, or as an absolute height otherwise." } } }, @@ -2608,7 +2608,7 @@ "thaw_height": { "type": "integer", "format": "int64", - "description": "This uint32 indicates if this channel is to be considered 'frozen'. A\nfrozen channel doest not allow a cooperative channel close by the\ninitiator. The thaw_height is the height that this restriction stops\napplying to the channel. This field is optional, not setting it or using a\nvalue of zero will mean the channel has no additional restrictions." + "description": "This uint32 indicates if this channel is to be considered 'frozen'. A\nfrozen channel doest not allow a cooperative channel close by the\ninitiator. The thaw_height is the height that this restriction stops\napplying to the channel. This field is optional, not setting it or using a\nvalue of zero will mean the channel has no additional restrictions. The\nheight can be interpreted in two ways: as a relative height if the value is\nless than 500,000, or as an absolute height otherwise." }, "local_constraints": { "$ref": "#/definitions/lnrpcChannelConstraints", diff --git a/lntest/itest/lnd_test.go b/lntest/itest/lnd_test.go index f9939acc8..5940a90a1 100644 --- a/lntest/itest/lnd_test.go +++ b/lntest/itest/lnd_test.go @@ -14501,11 +14501,6 @@ func testExternalFundingChanPoint(net *lntest.NetworkHarness, t *harnessTest) { t.Fatalf("unable to gen pending chan ID: %v", err) } - _, currentHeight, err := net.Miner.Node.GetBestBlock() - if err != nil { - t.Fatalf("unable to get current blockheight %v", err) - } - // Now that we have the pending channel ID, Dave (our responder) will // register the intent to receive a new channel funding workflow using // the pending channel ID. @@ -14514,7 +14509,7 @@ func testExternalFundingChanPoint(net *lntest.NetworkHarness, t *harnessTest) { FundingTxidBytes: txid[:], }, } - thawHeight := uint32(currentHeight + 10) + thawHeight := uint32(10) chanPointShim := &lnrpc.ChanPointShim{ Amt: int64(chanSize), ChanPoint: chanPoint, diff --git a/lnwallet/chancloser/chancloser.go b/lnwallet/chancloser/chancloser.go index 0691d3519..f1380ef7d 100644 --- a/lnwallet/chancloser/chancloser.go +++ b/lnwallet/chancloser/chancloser.go @@ -355,19 +355,25 @@ func (c *ChanCloser) ProcessCloseMsg(msg lnwire.Message) ([]lnwire.Message, "have %v", spew.Sdump(msg)) } - // As we're the responder to this shutdown (the other party wants to - // close), we'll check if this is a frozen channel or not. If the - // channel is frozen and we were not also the initiator of the channel - // opening, then we'll deny their close attempt. + // As we're the responder to this shutdown (the other party + // wants to close), we'll check if this is a frozen channel or + // not. If the channel is frozen and we were not also the + // initiator of the channel opening, then we'll deny their close + // attempt. chanInitiator := c.cfg.Channel.IsInitiator() chanState := c.cfg.Channel.State() - if !chanInitiator && chanState.ChanType.IsFrozen() && - c.negotiationHeight < chanState.ThawHeight { - - return nil, false, fmt.Errorf("initiator attempting to co-op "+ - "close frozen ChannelPoint(%v) (current_height=%v, "+ - "thaw_height=%v)", c.chanPoint, c.negotiationHeight, - chanState.ThawHeight) + if !chanInitiator { + absoluteThawHeight, err := chanState.AbsoluteThawHeight() + if err != nil { + return nil, false, err + } + if c.negotiationHeight < absoluteThawHeight { + return nil, false, fmt.Errorf("initiator "+ + "attempting to co-op close frozen "+ + "ChannelPoint(%v) (current_height=%v, "+ + "thaw_height=%v)", c.chanPoint, + c.negotiationHeight, absoluteThawHeight) + } } // If the remote node opened the channel with option upfront shutdown diff --git a/rpcserver.go b/rpcserver.go index bf0e20718..2b161f7bd 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -2122,14 +2122,19 @@ func (r *rpcServer) CloseChannel(in *lnrpc.CloseChannelRequest, }) } else { // If this is a frozen channel, then we only allow the co-op - // close to proceed if we were the responder to this channel. - if channel.ChanType.IsFrozen() && channel.IsInitiator && - uint32(bestHeight) < channel.ThawHeight { - - return fmt.Errorf("cannot co-op close frozen channel "+ - "as initiator until height=%v, "+ - "(current_height=%v)", channel.ThawHeight, - bestHeight) + // close to proceed if we were the responder to this channel if + // the absolute thaw height has not been met. + if channel.IsInitiator { + absoluteThawHeight, err := channel.AbsoluteThawHeight() + if err != nil { + return err + } + if uint32(bestHeight) < absoluteThawHeight { + return fmt.Errorf("cannot co-op close frozen "+ + "channel as initiator until height=%v, "+ + "(current_height=%v)", + absoluteThawHeight, bestHeight) + } } // If the link is not known by the switch, we cannot gracefully close