From 2cd7ec083378dcc42fa0f95f518f17dec4238326 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Tue, 27 Feb 2018 18:31:05 +0100 Subject: [PATCH 1/6] lnwallet: add errors.go, errors sent to peer during funding --- lnwallet/errors.go | 108 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 108 insertions(+) create mode 100644 lnwallet/errors.go diff --git a/lnwallet/errors.go b/lnwallet/errors.go new file mode 100644 index 00000000..396cdd8a --- /dev/null +++ b/lnwallet/errors.go @@ -0,0 +1,108 @@ +package lnwallet + +import ( + "errors" + "fmt" + + "github.com/lightningnetwork/lnd/lnwire" + "github.com/roasbeef/btcd/chaincfg/chainhash" + "github.com/roasbeef/btcutil" +) + +// ReservationError wraps certain errors returned during channel reservation +// that can be sent across the wire to the remote peer. Errors not being +// ReservationErrors will not be sent to the remote in case of a failed channel +// reservation, as they may contain private information. +type ReservationError struct { + error +} + +// A compile time check to ensure ReservationError implements the error +// interface. +var _ error = (*ReservationError)(nil) + +// ErrZeroCapacity returns an error indicating the funder attempted to put zero +// funds into the channel. +func ErrZeroCapacity() ReservationError { + return ReservationError{ + errors.New("zero channel funds"), + } +} + +// ErrChainMismatch returns an error indicating that the initiator tried to +// open a channel for an unknown chain. +func ErrChainMismatch(knownChain, + unknownChain *chainhash.Hash) ReservationError { + return ReservationError{ + fmt.Errorf("Unknown chain=%v. Supported chain=%v", + unknownChain, knownChain), + } +} + +// ErrFunderBalanceDust returns an error indicating the initial balance of the +// funder is considered dust at the current commitment fee. +func ErrFunderBalanceDust(commitFee, funderBalance, + minBalance int64) ReservationError { + return ReservationError{ + fmt.Errorf("Funder balance too small (%v) with fee=%v sat, "+ + "minimum=%v sat required", funderBalance, + commitFee, minBalance), + } +} + +// ErrCsvDelayTooLarge returns an error indicating that the CSV delay was to +// large to be accepted, along with the current max. +func ErrCsvDelayTooLarge(remoteDelay, maxDelay uint16) ReservationError { + return ReservationError{ + fmt.Errorf("CSV delay too large: %v, max is %v", + remoteDelay, maxDelay), + } +} + +// ErrChanReserveTooLarge returns an error indicating that the chan reserve the +// remote is requiring, is too large to be accepted. +func ErrChanReserveTooLarge(reserve, + maxReserve btcutil.Amount) ReservationError { + return ReservationError{ + fmt.Errorf("Channel reserve is too large: %v sat, max "+ + "is %v sat", int64(reserve), int64(maxReserve)), + } +} + +// ErrMinHtlcTooLarge returns an error indicating that the MinHTLC value the +// remote required is too large to be accepted. +func ErrMinHtlcTooLarge(minHtlc, + maxMinHtlc lnwire.MilliSatoshi) ReservationError { + return ReservationError{ + fmt.Errorf("Minimum HTLC value is too large: %v, max is %v", + minHtlc, maxMinHtlc), + } +} + +// ErrMaxHtlcNumTooLarge returns an error indicating that the 'max HTLCs in +// flight' value the remote required is too large to be accepted. +func ErrMaxHtlcNumTooLarge(maxHtlc, maxMaxHtlc uint16) ReservationError { + return ReservationError{ + fmt.Errorf("maxHtlcs is too large: %d, max is %d", + maxHtlc, maxMaxHtlc), + } +} + +// ErrMaxHtlcNumTooSmall returns an error indicating that the 'max HTLCs in +// flight' value the remote required is too small to be accepted. +func ErrMaxHtlcNumTooSmall(maxHtlc, minMaxHtlc uint16) ReservationError { + return ReservationError{ + fmt.Errorf("maxHtlcs is too small: %d, min is %d", + maxHtlc, minMaxHtlc), + } +} + +// ErrMaxValueInFlightTooSmall returns an error indicating that the 'max HTLC +// value in flight' the remote required is too small to be accepted. +func ErrMaxValueInFlightTooSmall(maxValInFlight, + minMaxValInFlight lnwire.MilliSatoshi) ReservationError { + return ReservationError{ + fmt.Errorf("maxValueInFlight too small: %v, min is %v", + maxValInFlight, minMaxValInFlight), + } +} From bb63ad7da63426b94ec934e1d6b44ce7bc82af04 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Tue, 27 Feb 2018 18:32:54 +0100 Subject: [PATCH 2/6] lnwallet: send ReservationError for unacceptable constraints --- lnwallet/reservation.go | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/lnwallet/reservation.go b/lnwallet/reservation.go index 1ea28015..ad0b6456 100644 --- a/lnwallet/reservation.go +++ b/lnwallet/reservation.go @@ -1,7 +1,6 @@ package lnwallet import ( - "fmt" "net" "sync" @@ -186,9 +185,11 @@ func NewChannelReservation(capacity, fundingAmt btcutil.Amount, // // TODO(roasbeef): reject if 30% goes to fees? dust channel if initiator && ourBalance.ToSatoshis() <= 2*DefaultDustLimit() { - return nil, fmt.Errorf("unable to init reservation, with "+ - "fee=%v sat/kw, local output is too small: %v sat", - int64(commitFee), int64(ourBalance.ToSatoshis())) + return nil, ErrFunderBalanceDust( + int64(commitFee), + int64(ourBalance.ToSatoshis()), + int64(2*DefaultDustLimit()), + ) } // Next we'll set the channel type based on what we can ascertain about @@ -282,8 +283,9 @@ func (r *ChannelReservation) CommitConstraints(csvDelay, maxHtlcs uint16, // Fail if we consider csvDelay excessively large. // TODO(halseth): find a more scientific choice of value. - if csvDelay > 10000 { - return fmt.Errorf("csvDelay is too large: %d", csvDelay) + const maxDelay = 10000 + if csvDelay > maxDelay { + return ErrCsvDelayTooLarge(csvDelay, maxDelay) } // Fail if we consider the channel reserve to be too large. @@ -291,8 +293,7 @@ func (r *ChannelReservation) CommitConstraints(csvDelay, maxHtlcs uint16, // channel capacity. maxChanReserve := r.partialState.Capacity / 5 if chanReserve > maxChanReserve { - return fmt.Errorf("chanReserve is too large: %g", - chanReserve.ToBTC()) + return ErrChanReserveTooLarge(chanReserve, maxChanReserve) } // Fail if the minimum HTLC value is too large. If this is @@ -302,29 +303,28 @@ func (r *ChannelReservation) CommitConstraints(csvDelay, maxHtlcs uint16, // it wants. // TODO(halseth): set a reasonable/dynamic value. if minHtlc > maxValueInFlight { - return fmt.Errorf("minimum HTLC value is too large: %g", - r.ourContribution.MinHTLC.ToBTC()) + return ErrMinHtlcTooLarge(minHtlc, maxValueInFlight) } // Fail if maxHtlcs is above the maximum allowed number of 483. // This number is specified in BOLT-02. if maxHtlcs > uint16(MaxHTLCNumber/2) { - return fmt.Errorf("maxHtlcs is too large: %d", maxHtlcs) + return ErrMaxHtlcNumTooLarge(maxHtlcs, uint16(MaxHTLCNumber/2)) } // Fail if we consider maxHtlcs too small. If this is too small // we cannot offer many HTLCs to the remote. const minNumHtlc = 5 if maxHtlcs < minNumHtlc { - return fmt.Errorf("maxHtlcs is too small: %d", maxHtlcs) + return ErrMaxHtlcNumTooSmall(maxHtlcs, minNumHtlc) } // Fail if we consider maxValueInFlight too small. We currently // require the remote to at least allow minNumHtlc * minHtlc // in flight. if maxValueInFlight < minNumHtlc*minHtlc { - return fmt.Errorf("maxValueInFlight is too small: %g", - maxValueInFlight.ToBTC()) + return ErrMaxValueInFlightTooSmall(maxValueInFlight, + minNumHtlc*minHtlc) } r.ourContribution.ChannelConfig.CsvDelay = csvDelay From 0cbb759d0a681c603fbda56fe71ff359eb6411b4 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Tue, 27 Feb 2018 18:35:56 +0100 Subject: [PATCH 3/6] lnwallet: send ReservationError for invalid/incompatible reservation --- lnwallet/wallet.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lnwallet/wallet.go b/lnwallet/wallet.go index c394ff2d..5719a1ee 100644 --- a/lnwallet/wallet.go +++ b/lnwallet/wallet.go @@ -479,8 +479,8 @@ func (l *LightningWallet) InitChannelReservation( func (l *LightningWallet) handleFundingReserveRequest(req *initFundingReserveMsg) { // It isn't possible to create a channel with zero funds committed. if req.fundingAmount+req.capacity == 0 { - req.err <- fmt.Errorf("cannot have channel with zero " + - "satoshis funded") + err := ErrZeroCapacity() + req.err <- err req.resp <- nil return } @@ -488,9 +488,9 @@ func (l *LightningWallet) handleFundingReserveRequest(req *initFundingReserveMsg // If the funding request is for a different chain than the one the // wallet is aware of, then we'll reject the request. if !bytes.Equal(l.Cfg.NetParams.GenesisHash[:], req.chainHash[:]) { - req.err <- fmt.Errorf("unable to create channel reservation "+ - "for chain=%v, wallet is on chain=%v", - req.chainHash, l.Cfg.NetParams.GenesisHash) + err := ErrChainMismatch(l.Cfg.NetParams.GenesisHash, + req.chainHash) + req.err <- err req.resp <- nil return } From b3441561cfca0b0101aae532b1534477f5c1360a Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Tue, 27 Feb 2018 18:36:30 +0100 Subject: [PATCH 4/6] lnwire: make ErrorCode satisfy error interface --- lnwire/error.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lnwire/error.go b/lnwire/error.go index fb338c43..9f47bfef 100644 --- a/lnwire/error.go +++ b/lnwire/error.go @@ -48,6 +48,13 @@ func (e ErrorCode) String() string { } } +// Error returns the human redable version of the target ErrorCode. +// +// Satisfies the Error interface. +func (e ErrorCode) Error() string { + return e.String() +} + // ErrorData is a set of bytes associated with a particular sent error. A // receiving node SHOULD only print out data verbatim if the string is composed // solely of printable ASCII characters. For reference, the printable character From 6942e1479d3ef4dbe48986c9cfa9f2a9b3121e72 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Tue, 27 Feb 2018 18:37:21 +0100 Subject: [PATCH 5/6] funding: only send know errors across wire This commit changes the failFundingFlow to accept an error, which will only be sent to the remote the peer if it is among the ReservationErrors or ErrorCode. This is done to ensure we don't send errors that contain information we don't want to leak. --- fundingmanager.go | 84 +++++++++++++++++++++++++++-------------------- 1 file changed, 49 insertions(+), 35 deletions(-) diff --git a/fundingmanager.go b/fundingmanager.go index 4614c18b..d3440b69 100644 --- a/fundingmanager.go +++ b/fundingmanager.go @@ -689,21 +689,40 @@ func (f *fundingManager) CancelPeerReservations(nodePub [33]byte) { } // failFundingFlow will fail the active funding flow with the target peer, -// identified by its unique temporary channel ID. This method is send an error -// to the remote peer, and also remove the reservation from our set of pending -// reservations. +// identified by its unique temporary channel ID. This method will send an +// error to the remote peer, and also remove the reservation from our set of +// pending reservations. // // TODO(roasbeef): if peer disconnects, and haven't yet broadcast funding // transaction, then all reservations should be cleared. func (f *fundingManager) failFundingFlow(peer *btcec.PublicKey, - tempChanID [32]byte, msg []byte) { + tempChanID [32]byte, fundingErr error) { + + // We only send the exact error if it is part of out whitelisted set of + // errors (lnwire.ErrorCode or lnwallet.ReservationError). + var msg lnwire.ErrorData + switch e := fundingErr.(type) { + + // Let the actual error message be sent to the remote. + case lnwallet.ReservationError: + msg = lnwire.ErrorData(e.Error()) + + // Send the status code. + case lnwire.ErrorCode: + msg = lnwire.ErrorData{byte(e)} + + // We just send a generic error. + default: + msg = lnwire.ErrorData("funding failed due to internal error") + } errMsg := &lnwire.Error{ ChanID: tempChanID, Data: msg, } - fndgLog.Errorf("Failing funding flow: %v", spew.Sdump(errMsg)) + fndgLog.Errorf("Failing funding flow: %v (%v)", fundingErr, + spew.Sdump(errMsg)) if _, err := f.cancelReservationCtx(peer, tempChanID); err != nil { fndgLog.Errorf("unable to cancel reservation: %v", err) @@ -819,8 +838,7 @@ func (f *fundingManager) handleFundingOpen(fmsg *fundingOpenMsg) { if len(f.activeReservations[peerIDKey]) >= cfg.MaxPendingChannels { f.failFundingFlow( fmsg.peerAddress.IdentityKey, fmsg.msg.PendingChannelID, - lnwire.ErrorData{byte(lnwire.ErrMaxPendingChannels)}, - ) + lnwire.ErrMaxPendingChannels) return } @@ -834,8 +852,7 @@ func (f *fundingManager) handleFundingOpen(fmsg *fundingOpenMsg) { } f.failFundingFlow( fmsg.peerAddress.IdentityKey, fmsg.msg.PendingChannelID, - lnwire.ErrorData{byte(lnwire.ErrSynchronizingChain)}, - ) + lnwire.ErrSynchronizingChain) return } @@ -844,8 +861,7 @@ func (f *fundingManager) handleFundingOpen(fmsg *fundingOpenMsg) { if msg.FundingAmount > maxFundingAmount { f.failFundingFlow( fmsg.peerAddress.IdentityKey, fmsg.msg.PendingChannelID, - lnwire.ErrorData{byte(lnwire.ErrChanTooLarge)}, - ) + lnwire.ErrChanTooLarge) return } @@ -871,7 +887,7 @@ func (f *fundingManager) handleFundingOpen(fmsg *fundingOpenMsg) { if err != nil { fndgLog.Errorf("Unable to initialize reservation: %v", err) f.failFundingFlow(fmsg.peerAddress.IdentityKey, - msg.PendingChannelID, []byte(err.Error())) + msg.PendingChannelID, err) return } @@ -890,10 +906,9 @@ func (f *fundingManager) handleFundingOpen(fmsg *fundingOpenMsg) { msg.MaxValueInFlight, msg.HtlcMinimum, msg.ChannelReserve, ) if err != nil { - f.failFundingFlow( - fmsg.peerAddress.IdentityKey, fmsg.msg.PendingChannelID, - []byte(fmt.Sprintf("Unacceptable channel "+ - "constraints: %v", err)), + fndgLog.Errorf("Unaccaptable channel constraints: %v", err) + f.failFundingFlow(fmsg.peerAddress.IdentityKey, + fmsg.msg.PendingChannelID, err, ) return } @@ -951,9 +966,8 @@ func (f *fundingManager) handleFundingOpen(fmsg *fundingOpenMsg) { err = reservation.ProcessSingleContribution(remoteContribution) if err != nil { fndgLog.Errorf("unable to add contribution reservation: %v", err) - // TODO(roasbeef): verify only sending sane info over f.failFundingFlow(fmsg.peerAddress.IdentityKey, - msg.PendingChannelID, []byte(err.Error())) + msg.PendingChannelID, err) return } @@ -985,7 +999,7 @@ func (f *fundingManager) handleFundingOpen(fmsg *fundingOpenMsg) { if err != nil { fndgLog.Errorf("unable to send funding response to peer: %v", err) f.failFundingFlow(fmsg.peerAddress.IdentityKey, - msg.PendingChannelID, []byte(err.Error())) + msg.PendingChannelID, err) return } } @@ -1028,11 +1042,9 @@ func (f *fundingManager) handleFundingAccept(fmsg *fundingAcceptMsg) { msg.MaxValueInFlight, msg.HtlcMinimum, msg.ChannelReserve, ) if err != nil { - f.failFundingFlow( - fmsg.peerAddress.IdentityKey, fmsg.msg.PendingChannelID, - []byte(fmt.Sprintf("Unacceptable channel "+ - "constraints: %v", err)), - ) + fndgLog.Warnf("Unacceptable channel constraints: %v", err) + f.failFundingFlow(fmsg.peerAddress.IdentityKey, + fmsg.msg.PendingChannelID, err) return } @@ -1070,7 +1082,7 @@ func (f *fundingManager) handleFundingAccept(fmsg *fundingAcceptMsg) { fndgLog.Errorf("Unable to process contribution from %v: %v", fmsg.peerAddress.IdentityKey, err) f.failFundingFlow(fmsg.peerAddress.IdentityKey, - msg.PendingChannelID, []byte(err.Error())) + msg.PendingChannelID, err) resCtx.err <- err return } @@ -1115,7 +1127,7 @@ func (f *fundingManager) handleFundingAccept(fmsg *fundingAcceptMsg) { if err != nil { fndgLog.Errorf("Unable to parse signature: %v", err) f.failFundingFlow(fmsg.peerAddress.IdentityKey, - msg.PendingChannelID, []byte(err.Error())) + msg.PendingChannelID, err) resCtx.err <- err return } @@ -1123,7 +1135,7 @@ func (f *fundingManager) handleFundingAccept(fmsg *fundingAcceptMsg) { if err != nil { fndgLog.Errorf("Unable to send funding complete message: %v", err) f.failFundingFlow(fmsg.peerAddress.IdentityKey, - msg.PendingChannelID, []byte(err.Error())) + msg.PendingChannelID, err) resCtx.err <- err return } @@ -1177,7 +1189,7 @@ func (f *fundingManager) handleFundingCreated(fmsg *fundingCreatedMsg) { // TODO(roasbeef): better error logging: peerID, channelID, etc. fndgLog.Errorf("unable to complete single reservation: %v", err) f.failFundingFlow(fmsg.peerAddress.IdentityKey, - pendingChanID, []byte(err.Error())) + pendingChanID, err) return } @@ -1218,7 +1230,7 @@ func (f *fundingManager) handleFundingCreated(fmsg *fundingCreatedMsg) { if err != nil { fndgLog.Errorf("unable to parse signature: %v", err) f.failFundingFlow(fmsg.peerAddress.IdentityKey, - pendingChanID, []byte(err.Error())) + pendingChanID, err) deleteFromDatabase() return } @@ -1230,7 +1242,7 @@ func (f *fundingManager) handleFundingCreated(fmsg *fundingCreatedMsg) { if err := f.cfg.SendToPeer(peerKey, fundingSigned); err != nil { fndgLog.Errorf("unable to send FundingSigned message: %v", err) f.failFundingFlow(fmsg.peerAddress.IdentityKey, - pendingChanID, []byte(err.Error())) + pendingChanID, err) deleteFromDatabase() return } @@ -1332,11 +1344,12 @@ func (f *fundingManager) handleFundingSigned(fmsg *fundingSignedMsg) { delete(f.signedReservations, fmsg.msg.ChanID) f.resMtx.Unlock() if !ok { - err := fmt.Sprintf("Unable to find signed reservation for "+ + err := fmt.Errorf("Unable to find signed reservation for "+ "chan_id=%x", fmsg.msg.ChanID) - fndgLog.Warnf(err) + fndgLog.Warnf(err.Error()) + // TODO: add ErrChanNotFound? f.failFundingFlow(fmsg.peerAddress.IdentityKey, - pendingChanID, []byte(err)) + pendingChanID, err) return } @@ -1346,8 +1359,9 @@ func (f *fundingManager) handleFundingSigned(fmsg *fundingSignedMsg) { if err != nil { fndgLog.Warnf("Unable to find reservation (peerID:%v, chanID:%x)", peerKey, pendingChanID[:]) + // TODO: add ErrChanNotFound? f.failFundingFlow(fmsg.peerAddress.IdentityKey, - pendingChanID, []byte(err.Error())) + pendingChanID, err) return } @@ -1369,7 +1383,7 @@ func (f *fundingManager) handleFundingSigned(fmsg *fundingSignedMsg) { fndgLog.Errorf("Unable to complete reservation sign complete: %v", err) resCtx.err <- err f.failFundingFlow(fmsg.peerAddress.IdentityKey, - pendingChanID, []byte(err.Error())) + pendingChanID, err) return } From 781c2a6eb45dd6aaf40e6eb50958db75757ccd0b Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 28 Feb 2018 23:07:13 +0100 Subject: [PATCH 6/6] lnwallet test: update test to new error message --- lnwallet/interface_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lnwallet/interface_test.go b/lnwallet/interface_test.go index ed15c8d7..864e3f28 100644 --- a/lnwallet/interface_test.go +++ b/lnwallet/interface_test.go @@ -581,7 +581,7 @@ func testReservationInitiatorBalanceBelowDustCancel(miner *rpctest.Harness, t.Fatalf("initialization should have failed due to " + "insufficient local amount") - case !strings.Contains(err.Error(), "local output is too small"): + case !strings.Contains(err.Error(), "Funder balance too small"): t.Fatalf("incorrect error: %v", err) } }