From 5660a26b602da096bd103a453ddebeb301679f9d Mon Sep 17 00:00:00 2001 From: Conner Fromknecht Date: Mon, 30 Mar 2020 16:55:16 -0700 Subject: [PATCH 1/5] rpcserver: add wallet sync check to OpenChannel --- lntest/itest/lnd_test.go | 12 +++++++++--- rpcserver.go | 11 +++++++++++ 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/lntest/itest/lnd_test.go b/lntest/itest/lnd_test.go index f0ccc407..1a113399 100644 --- a/lntest/itest/lnd_test.go +++ b/lntest/itest/lnd_test.go @@ -214,9 +214,15 @@ func openChannelAndAssert(ctx context.Context, t *harnessTest, net *lntest.NetworkHarness, alice, bob *lntest.HarnessNode, p lntest.OpenChannelParams) *lnrpc.ChannelPoint { - chanOpenUpdate, err := net.OpenChannel( - ctx, alice, bob, p, - ) + // Wait until we are able to fund a channel successfully. This wait + // prevents us from erroring out when trying to create a channel while + // the node is starting up. + var chanOpenUpdate lnrpc.Lightning_OpenChannelClient + err := wait.NoError(func() error { + var err error + chanOpenUpdate, err = net.OpenChannel(ctx, alice, bob, p) + return err + }, defaultTimeout) if err != nil { t.Fatalf("unable to open channel: %v", err) } diff --git a/rpcserver.go b/rpcserver.go index e68e0ffd..4b5f56bc 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -1616,6 +1616,17 @@ func (r *rpcServer) OpenChannel(in *lnrpc.OpenChannelRequest, return ErrServerNotActive } + // Creation of channels before the wallet syncs up is currently + // disallowed. + isSynced, _, err := r.server.cc.wallet.IsSynced() + if err != nil { + return err + } + if !isSynced { + return errors.New("channels cannot be created before the " + + "wallet is fully synced") + } + localFundingAmt := btcutil.Amount(in.LocalFundingAmount) remoteInitialBalance := btcutil.Amount(in.PushSat) minHtlcIn := lnwire.MilliSatoshi(in.MinHtlcMsat) From 1823840ed6830d1f30ec86296f7e5ec9bdb605c3 Mon Sep 17 00:00:00 2001 From: Conner Fromknecht Date: Mon, 30 Mar 2020 16:55:31 -0700 Subject: [PATCH 2/5] rpcserver: add missing channel limit check to OpenChannelSync --- rpcserver.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/rpcserver.go b/rpcserver.go index 4b5f56bc..01faa0d3 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -1870,6 +1870,14 @@ func (r *rpcServer) OpenChannelSync(ctx context.Context, "initial state must be below the local funding amount") } + // Ensure that the user doesn't exceed the current soft-limit for + // channel size. If the funding amount is above the soft-limit, then + // we'll reject the request. + if localFundingAmt > MaxFundingAmount { + return nil, fmt.Errorf("funding amount is too large, the max "+ + "channel size is: %v", MaxFundingAmount) + } + // Restrict the size of the channel we'll actually open. At a later // level, we'll ensure that the output we create after accounting for // fees that a dust output isn't created. From 303d441d4d7cd5b256070f18c81ac4bd3513df81 Mon Sep 17 00:00:00 2001 From: Conner Fromknecht Date: Mon, 30 Mar 2020 16:56:12 -0700 Subject: [PATCH 3/5] rpcserver: ensure OpenChannel+OpenChannelSync parse NodePubkey --- rpcserver.go | 52 +++++++++++++++++++++++++++++++--------------------- 1 file changed, 31 insertions(+), 21 deletions(-) diff --git a/rpcserver.go b/rpcserver.go index 01faa0d3..f9753bbb 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -1666,13 +1666,10 @@ func (r *rpcServer) OpenChannel(in *lnrpc.OpenChannelRequest, return err } - var ( - nodePubKey *btcec.PublicKey - nodePubKeyBytes []byte - ) - // TODO(roasbeef): also return channel ID? + var nodePubKey *btcec.PublicKey + // Ensure that the NodePubKey is set before attempting to use it if len(in.NodePubkey) == 0 { return fmt.Errorf("NodePubKey is not set") @@ -1691,8 +1688,6 @@ func (r *rpcServer) OpenChannel(in *lnrpc.OpenChannelRequest, return fmt.Errorf("cannot open channel to self") } - nodePubKeyBytes = nodePubKey.SerializeCompressed() - // Based on the passed fee related parameters, we'll determine an // appropriate fee rate for the funding transaction. satPerKw := chainfee.SatPerKVByte(in.SatPerByte * 1000).FeePerKWeight() @@ -1780,7 +1775,7 @@ out: select { case err := <-errChan: rpcsLog.Errorf("unable to open channel to NodeKey(%x): %v", - nodePubKeyBytes, err) + nodePubKey.SerializeCompressed(), err) return err case fundingUpdate := <-updateChan: rpcsLog.Tracef("[openchannel] sending update: %v", @@ -1812,7 +1807,7 @@ out: } rpcsLog.Tracef("[openchannel] success NodeKey(%x), ChannelPoint(%v)", - nodePubKeyBytes, outpoint) + nodePubKey.SerializeCompressed(), outpoint) return nil } @@ -1845,16 +1840,31 @@ func (r *rpcServer) OpenChannelSync(ctx context.Context, "wallet is fully synced") } - // Decode the provided target node's public key, parsing it into a pub - // key object. For all sync call, byte slices are expected to be - // encoded as hex strings. - keyBytes, err := hex.DecodeString(in.NodePubkeyString) - if err != nil { - return nil, err - } - nodepubKey, err := btcec.ParsePubKey(keyBytes, btcec.S256()) - if err != nil { - return nil, err + var nodePubKey *btcec.PublicKey + + // Parse the remote pubkey the NodePubkey field of the request. If it's + // not present, we'll fallback to the deprecated version that parses the + // key from a hex string. + if len(in.NodePubkey) > 0 { + // Parse the raw bytes of the node key into a pubkey object so we + // can easily manipulate it. + nodePubKey, err = btcec.ParsePubKey(in.NodePubkey, btcec.S256()) + if err != nil { + return nil, err + } + } else { + // Decode the provided target node's public key, parsing it into + // a pub key object. For all sync call, byte slices are expected + // to be encoded as hex strings. + keyBytes, err := hex.DecodeString(in.NodePubkeyString) + if err != nil { + return nil, err + } + + nodePubKey, err = btcec.ParsePubKey(keyBytes, btcec.S256()) + if err != nil { + return nil, err + } } localFundingAmt := btcutil.Amount(in.LocalFundingAmount) @@ -1916,7 +1926,7 @@ func (r *rpcServer) OpenChannelSync(ctx context.Context, } req := &openChanReq{ - targetPubkey: nodepubKey, + targetPubkey: nodePubKey, chainHash: *activeNetParams.GenesisHash, localFundingAmt: localFundingAmt, pushAmt: lnwire.NewMSatFromSatoshis(remoteInitialBalance), @@ -1933,7 +1943,7 @@ func (r *rpcServer) OpenChannelSync(ctx context.Context, // If an error occurs them immediately return the error to the client. case err := <-errChan: rpcsLog.Errorf("unable to open channel to NodeKey(%x): %v", - nodepubKey, err) + nodePubKey.SerializeCompressed(), err) return nil, err // Otherwise, wait for the first channel update. The first update sent From c1f9c56e5b5a2ebfb6013b4dd0644db97472103e Mon Sep 17 00:00:00 2001 From: Conner Fromknecht Date: Mon, 30 Mar 2020 15:15:15 -0700 Subject: [PATCH 4/5] rpcserver: add self-node check in OpenChannelSync --- rpcserver.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/rpcserver.go b/rpcserver.go index f9753bbb..5ff15de2 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -1867,6 +1867,12 @@ func (r *rpcServer) OpenChannelSync(ctx context.Context, } } + // Making a channel to ourselves wouldn't be of any use, so we + // explicitly disallow them. + if nodePubKey.IsEqual(r.server.identityPriv.PubKey()) { + return nil, fmt.Errorf("cannot open channel to self") + } + localFundingAmt := btcutil.Amount(in.LocalFundingAmount) remoteInitialBalance := btcutil.Amount(in.PushSat) minHtlcIn := lnwire.MilliSatoshi(in.MinHtlcMsat) From 66d02504f9246ec3bd83308a2f325b066491ce42 Mon Sep 17 00:00:00 2001 From: Conner Fromknecht Date: Mon, 30 Mar 2020 16:59:53 -0700 Subject: [PATCH 5/5] rpcserver: align OpenChannel+OpenChannelSync req parsing This commit consolidates our request parsing for OpenChannel and OpenChannelSync which removes a good deal of code duplication. --- rpcserver.go | 228 +++++++++++++++++---------------------------------- 1 file changed, 77 insertions(+), 151 deletions(-) diff --git a/rpcserver.go b/rpcserver.go index 5ff15de2..fff3a18e 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -1603,15 +1603,10 @@ func newPsbtAssembler(req *lnrpc.OpenChannelRequest, normalizedMinConfs int32, ), nil } -// OpenChannel attempts to open a singly funded channel specified in the -// request to a remote peer. -func (r *rpcServer) OpenChannel(in *lnrpc.OpenChannelRequest, - updateStream lnrpc.Lightning_OpenChannelServer) error { - - rpcsLog.Tracef("[openchannel] request to NodeKey(%v) "+ - "allocation(us=%v, them=%v)", in.NodePubkeyString, - in.LocalFundingAmount, in.PushSat) - +// canOpenChannel returns an error if the necessary subsystems for channel +// funding are not ready. +func (r *rpcServer) canOpenChannel() error { + // We can't open a channel until the main server has started. if !r.server.Started() { return ErrServerNotActive } @@ -1627,6 +1622,19 @@ func (r *rpcServer) OpenChannel(in *lnrpc.OpenChannelRequest, "wallet is fully synced") } + return nil +} + +// praseOpenChannelReq parses an OpenChannelRequest message into the server's +// native openChanReq struct. The logic is abstracted so that it can be shared +// between OpenChannel and OpenChannelSync. +func (r *rpcServer) parseOpenChannelReq(in *lnrpc.OpenChannelRequest, + isSync bool) (*openChanReq, error) { + + rpcsLog.Debugf("[openchannel] request to NodeKey(%x) "+ + "allocation(us=%v, them=%v)", in.NodePubkey, + in.LocalFundingAmount, in.PushSat) + localFundingAmt := btcutil.Amount(in.LocalFundingAmount) remoteInitialBalance := btcutil.Amount(in.PushSat) minHtlcIn := lnwire.MilliSatoshi(in.MinHtlcMsat) @@ -1638,15 +1646,15 @@ func (r *rpcServer) OpenChannel(in *lnrpc.OpenChannelRequest, // // TODO(roasbeef): incorporate base fee? if remoteInitialBalance >= localFundingAmt { - return fmt.Errorf("amount pushed to remote peer for initial " + - "state must be below the local funding amount") + return nil, fmt.Errorf("amount pushed to remote peer for " + + "initial state must be below the local funding amount") } // Ensure that the user doesn't exceed the current soft-limit for // channel size. If the funding amount is above the soft-limit, then // we'll reject the request. if localFundingAmt > MaxFundingAmount { - return fmt.Errorf("funding amount is too large, the max "+ + return nil, fmt.Errorf("funding amount is too large, the max "+ "channel size is: %v", MaxFundingAmount) } @@ -1654,8 +1662,8 @@ func (r *rpcServer) OpenChannel(in *lnrpc.OpenChannelRequest, // level, we'll ensure that the output we create after accounting for // fees that a dust output isn't created. if localFundingAmt < minChanFundingSize { - return fmt.Errorf("channel is too small, the minimum channel "+ - "size is: %v SAT", int64(minChanFundingSize)) + return nil, fmt.Errorf("channel is too small, the minimum "+ + "channel size is: %v SAT", int64(minChanFundingSize)) } // Then, we'll extract the minimum number of confirmations that each @@ -1663,29 +1671,48 @@ func (r *rpcServer) OpenChannel(in *lnrpc.OpenChannelRequest, // satisfy. minConfs, err := extractOpenChannelMinConfs(in) if err != nil { - return err + return nil, err } // TODO(roasbeef): also return channel ID? var nodePubKey *btcec.PublicKey - // Ensure that the NodePubKey is set before attempting to use it - if len(in.NodePubkey) == 0 { - return fmt.Errorf("NodePubKey is not set") - } + // Parse the remote pubkey the NodePubkey field of the request. If it's + // not present, we'll fallback to the deprecated version that parses the + // key from a hex string if this is for REST for backwards compatibility. + switch { - // Parse the raw bytes of the node key into a pubkey object so we - // can easily manipulate it. - nodePubKey, err = btcec.ParsePubKey(in.NodePubkey, btcec.S256()) - if err != nil { - return err + // Parse the raw bytes of the node key into a pubkey object so we can + // easily manipulate it. + case len(in.NodePubkey) > 0: + nodePubKey, err = btcec.ParsePubKey(in.NodePubkey, btcec.S256()) + if err != nil { + return nil, err + } + + // Decode the provided target node's public key, parsing it into a pub + // key object. For all sync call, byte slices are expected to be encoded + // as hex strings. + case isSync: + keyBytes, err := hex.DecodeString(in.NodePubkeyString) + if err != nil { + return nil, err + } + + nodePubKey, err = btcec.ParsePubKey(keyBytes, btcec.S256()) + if err != nil { + return nil, err + } + + default: + return nil, fmt.Errorf("NodePubkey is not set") } // Making a channel to ourselves wouldn't be of any use, so we // explicitly disallow them. if nodePubKey.IsEqual(r.server.identityPriv.PubKey()) { - return fmt.Errorf("cannot open channel to self") + return nil, fmt.Errorf("cannot open channel to self") } // Based on the passed fee related parameters, we'll determine an @@ -1698,7 +1725,7 @@ func (r *rpcServer) OpenChannel(in *lnrpc.OpenChannelRequest, }, ) if err != nil { - return err + return nil, err } rpcsLog.Debugf("[openchannel]: using fee of %v sat/kw for funding tx", @@ -1706,13 +1733,14 @@ func (r *rpcServer) OpenChannel(in *lnrpc.OpenChannelRequest, script, err := parseUpfrontShutdownAddress(in.CloseAddress) if err != nil { - return fmt.Errorf("error parsing upfront shutdown: %v", err) + return nil, fmt.Errorf("error parsing upfront shutdown: %v", + err) } // Instruct the server to trigger the necessary events to attempt to // open a new channel. A stream is returned in place, this stream will // be used to consume updates of the state of the pending channel. - req := &openChanReq{ + return &openChanReq{ targetPubkey: nodePubKey, chainHash: *activeNetParams.GenesisHash, localFundingAmt: localFundingAmt, @@ -1723,6 +1751,21 @@ func (r *rpcServer) OpenChannel(in *lnrpc.OpenChannelRequest, remoteCsvDelay: remoteCsvDelay, minConfs: minConfs, shutdownScript: script, + }, nil +} + +// OpenChannel attempts to open a singly funded channel specified in the +// request to a remote peer. +func (r *rpcServer) OpenChannel(in *lnrpc.OpenChannelRequest, + updateStream lnrpc.Lightning_OpenChannelServer) error { + + if err := r.canOpenChannel(); err != nil { + return err + } + + req, err := r.parseOpenChannelReq(in, false) + if err != nil { + return err } // If the user has provided a shim, then we'll now augment the based @@ -1758,7 +1801,7 @@ func (r *rpcServer) OpenChannel(in *lnrpc.OpenChannelRequest, // transaction. copy(req.pendingChanID[:], psbtShim.PendingChanId) req.chanFunder, err = newPsbtAssembler( - in, minConfs, psbtShim, + in, req.minConfs, psbtShim, &r.server.cc.wallet.Cfg.NetParams, ) if err != nil { @@ -1775,7 +1818,7 @@ out: select { case err := <-errChan: rpcsLog.Errorf("unable to open channel to NodeKey(%x): %v", - nodePubKey.SerializeCompressed(), err) + req.targetPubkey.SerializeCompressed(), err) return err case fundingUpdate := <-updateChan: rpcsLog.Tracef("[openchannel] sending update: %v", @@ -1807,7 +1850,7 @@ out: } rpcsLog.Tracef("[openchannel] success NodeKey(%x), ChannelPoint(%v)", - nodePubKey.SerializeCompressed(), outpoint) + req.targetPubkey.SerializeCompressed(), outpoint) return nil } @@ -1818,138 +1861,21 @@ out: func (r *rpcServer) OpenChannelSync(ctx context.Context, in *lnrpc.OpenChannelRequest) (*lnrpc.ChannelPoint, error) { - rpcsLog.Tracef("[openchannel] request to NodeKey(%v) "+ - "allocation(us=%v, them=%v)", in.NodePubkeyString, - in.LocalFundingAmount, in.PushSat) - - // We don't allow new channels to be open while the server is still - // syncing, as otherwise we may not be able to obtain the relevant - // notifications. - if !r.server.Started() { - return nil, ErrServerNotActive - } - - // Creation of channels before the wallet syncs up is currently - // disallowed. - isSynced, _, err := r.server.cc.wallet.IsSynced() - if err != nil { - return nil, err - } - if !isSynced { - return nil, errors.New("channels cannot be created before the " + - "wallet is fully synced") - } - - var nodePubKey *btcec.PublicKey - - // Parse the remote pubkey the NodePubkey field of the request. If it's - // not present, we'll fallback to the deprecated version that parses the - // key from a hex string. - if len(in.NodePubkey) > 0 { - // Parse the raw bytes of the node key into a pubkey object so we - // can easily manipulate it. - nodePubKey, err = btcec.ParsePubKey(in.NodePubkey, btcec.S256()) - if err != nil { - return nil, err - } - } else { - // Decode the provided target node's public key, parsing it into - // a pub key object. For all sync call, byte slices are expected - // to be encoded as hex strings. - keyBytes, err := hex.DecodeString(in.NodePubkeyString) - if err != nil { - return nil, err - } - - nodePubKey, err = btcec.ParsePubKey(keyBytes, btcec.S256()) - if err != nil { - return nil, err - } - } - - // Making a channel to ourselves wouldn't be of any use, so we - // explicitly disallow them. - if nodePubKey.IsEqual(r.server.identityPriv.PubKey()) { - return nil, fmt.Errorf("cannot open channel to self") - } - - localFundingAmt := btcutil.Amount(in.LocalFundingAmount) - remoteInitialBalance := btcutil.Amount(in.PushSat) - minHtlcIn := lnwire.MilliSatoshi(in.MinHtlcMsat) - remoteCsvDelay := uint16(in.RemoteCsvDelay) - - // Ensure that the initial balance of the remote party (if pushing - // satoshis) does not exceed the amount the local party has requested - // for funding. - if remoteInitialBalance >= localFundingAmt { - return nil, fmt.Errorf("amount pushed to remote peer for " + - "initial state must be below the local funding amount") - } - - // Ensure that the user doesn't exceed the current soft-limit for - // channel size. If the funding amount is above the soft-limit, then - // we'll reject the request. - if localFundingAmt > MaxFundingAmount { - return nil, fmt.Errorf("funding amount is too large, the max "+ - "channel size is: %v", MaxFundingAmount) - } - - // Restrict the size of the channel we'll actually open. At a later - // level, we'll ensure that the output we create after accounting for - // fees that a dust output isn't created. - if localFundingAmt < minChanFundingSize { - return nil, fmt.Errorf("channel is too small, the minimum channel "+ - "size is: %v SAT", int64(minChanFundingSize)) - } - - // Then, we'll extract the minimum number of confirmations that each - // output we use to fund the channel's funding transaction should - // satisfy. - minConfs, err := extractOpenChannelMinConfs(in) - if err != nil { + if err := r.canOpenChannel(); err != nil { return nil, err } - // Based on the passed fee related parameters, we'll determine an - // appropriate fee rate for the funding transaction. - satPerKw := chainfee.SatPerKVByte(in.SatPerByte * 1000).FeePerKWeight() - feeRate, err := sweep.DetermineFeePerKw( - r.server.cc.feeEstimator, sweep.FeePreference{ - ConfTarget: uint32(in.TargetConf), - FeeRate: satPerKw, - }, - ) + req, err := r.parseOpenChannelReq(in, true) if err != nil { return nil, err } - rpcsLog.Tracef("[openchannel] target sat/kw for funding tx: %v", - int64(feeRate)) - - script, err := parseUpfrontShutdownAddress(in.CloseAddress) - if err != nil { - return nil, fmt.Errorf("error parsing upfront shutdown: %v", err) - } - - req := &openChanReq{ - targetPubkey: nodePubKey, - chainHash: *activeNetParams.GenesisHash, - localFundingAmt: localFundingAmt, - pushAmt: lnwire.NewMSatFromSatoshis(remoteInitialBalance), - minHtlcIn: minHtlcIn, - fundingFeePerKw: feeRate, - private: in.Private, - remoteCsvDelay: remoteCsvDelay, - minConfs: minConfs, - shutdownScript: script, - } - updateChan, errChan := r.server.OpenChannel(req) select { // If an error occurs them immediately return the error to the client. case err := <-errChan: rpcsLog.Errorf("unable to open channel to NodeKey(%x): %v", - nodePubKey.SerializeCompressed(), err) + req.targetPubkey.SerializeCompressed(), err) return nil, err // Otherwise, wait for the first channel update. The first update sent