Skip to content

Commit

Permalink
multi: update linter, fix new issues
Browse files Browse the repository at this point in the history
  • Loading branch information
guggero committed Aug 20, 2024
1 parent 9eef428 commit e99e666
Show file tree
Hide file tree
Showing 21 changed files with 453 additions and 441 deletions.
20 changes: 17 additions & 3 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -195,12 +195,26 @@ linters:
# The linter is too aggressive and doesn't add much value since reviewers
# will also catch magic numbers that make sense to extract.
- gomnd
- mnd

# Some of the tests cannot be parallelized. On the other hand, we don't
# gain much performance with this check so we disable it for now until
# unit tests become our CI bottleneck.
# Some of the tests cannot be parallelized. On the other hand, we don't
# gain much performance with this check so we disable it for now until
# unit tests become our CI bottleneck.
- paralleltest

# New linters that we haven't had time to address yet.
- testifylint
- perfsprint
- inamedparam
- copyloopvar
- tagalign
- protogetter
- revive
- depguard
- gosmopolitan
- intrange


issues:
# Only show newly introduced problems.
new-from-rev: 8c66353e4c02329abdacb5a8df29998035ec2e24
Expand Down
2 changes: 1 addition & 1 deletion channeldb/graph_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2382,7 +2382,7 @@ func TestStressTestChannelGraphAPI(t *testing.T) {
methodsMu.Unlock()

err := fn()
require.NoErrorf(t, err, fmt.Sprintf(name))
require.NoErrorf(t, err, name)
}
})
}
Expand Down
8 changes: 5 additions & 3 deletions contractcourt/channel_arbitrator.go
Original file line number Diff line number Diff line change
Expand Up @@ -1982,9 +1982,11 @@ func (c *ChannelArbitrator) isPreimageAvailable(hash lntypes.Hash) (bool,
// have the incoming contest resolver decide that we don't want to
// settle this invoice.
invoice, err := c.cfg.Registry.LookupInvoice(context.Background(), hash)
switch err {
case nil:
case invoices.ErrInvoiceNotFound, invoices.ErrNoInvoicesCreated:
switch {
case err == nil:
case errors.Is(err, invoices.ErrInvoiceNotFound) ||
errors.Is(err, invoices.ErrNoInvoicesCreated):

return false, nil
default:
return false, err
Expand Down
3 changes: 2 additions & 1 deletion contractcourt/mock_htlcnotifier_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,6 @@ type mockHTLCNotifier struct {
}

func (m *mockHTLCNotifier) NotifyFinalHtlcEvent(key models.CircuitKey,
info channeldb.FinalHtlcInfo) { //nolint:whitespace
info channeldb.FinalHtlcInfo) {

}
2 changes: 1 addition & 1 deletion graph/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -681,7 +681,7 @@ func (b *Builder) handleNetworkUpdate(vb *ValidationBarrier,
update.err <- err

case IsError(err, ErrParentValidationFailed):
update.err <- newErrf(ErrIgnored, err.Error())
update.err <- newErrf(ErrIgnored, err.Error()) //nolint

default:
log.Warnf("unexpected error during validation "+
Expand Down
90 changes: 46 additions & 44 deletions htlcswitch/link.go
Original file line number Diff line number Diff line change
Expand Up @@ -1154,10 +1154,11 @@ func (l *channelLink) htlcManager() {
// be updated when the close transaction is
// ready to avoid that we go down before
// storing the transaction in the db.
l.fail(
l.failf(
//nolint:lll
LinkFailureError{
code: ErrSyncError,
FailureAction: LinkFailureForceClose, //nolint:lll
FailureAction: LinkFailureForceClose,
},
"unable to synchronize channel "+
"states: %v", err,
Expand Down Expand Up @@ -1195,7 +1196,7 @@ func (l *channelLink) htlcManager() {
default:
}

l.fail(
l.failf(
LinkFailureError{
code: ErrRecoveryError,
FailureAction: LinkFailureForceNone,
Expand Down Expand Up @@ -1259,14 +1260,14 @@ func (l *channelLink) htlcManager() {
// If the duplicate keystone error was encountered, we'll fail
// without sending an Error message to the peer.
case ErrDuplicateKeystone:
l.fail(LinkFailureError{code: ErrCircuitError},
l.failf(LinkFailureError{code: ErrCircuitError},
"temporary circuit error: %v", err)
return

// A non-nil error was encountered, send an Error message to
// the peer.
default:
l.fail(LinkFailureError{code: ErrInternalError},
l.failf(LinkFailureError{code: ErrInternalError},
"unable to resolve fwd pkgs: %v", err)
return
}
Expand Down Expand Up @@ -1405,7 +1406,7 @@ func (l *channelLink) htlcManager() {
}

case <-l.cfg.PendingCommitTicker.Ticks():
l.fail(
l.failf(
LinkFailureError{
code: ErrRemoteUnresponsive,
FailureAction: LinkFailureDisconnect,
Expand Down Expand Up @@ -1438,19 +1439,19 @@ func (l *channelLink) htlcManager() {
// If the duplicate keystone error was encountered,
// fail back gracefully.
case ErrDuplicateKeystone:
l.fail(LinkFailureError{code: ErrCircuitError},
fmt.Sprintf("process hodl queue: "+
"temporary circuit error: %v",
err,
),
l.failf(LinkFailureError{
code: ErrCircuitError,
}, "process hodl queue: "+
"temporary circuit error: %v",
err,
)

// Send an Error message to the peer.
default:
l.fail(LinkFailureError{code: ErrInternalError},
fmt.Sprintf("process hodl queue: "+
"unable to update commitment:"+
" %v", err),
l.failf(LinkFailureError{
code: ErrInternalError,
}, "process hodl queue: unable to update "+
"commitment: %v", err,
)
}

Expand Down Expand Up @@ -1960,7 +1961,7 @@ func (l *channelLink) handleUpstreamMsg(msg lnwire.Message) {
// handle message ordering due to concurrency choices.
// An issue has been filed to address this here:
// https://github.com/lightningnetwork/lnd/issues/8393
l.fail(
l.failf(
LinkFailureError{
code: ErrInvalidUpdate,
FailureAction: LinkFailureDisconnect,
Expand All @@ -1979,7 +1980,7 @@ func (l *channelLink) handleUpstreamMsg(msg lnwire.Message) {
// where we are a relaying node (as the blinding point will
// be in the payload when we're the introduction node).
if msg.BlindingPoint.IsSome() && l.cfg.DisallowRouteBlinding {
l.fail(LinkFailureError{code: ErrInvalidUpdate},
l.failf(LinkFailureError{code: ErrInvalidUpdate},
"blinding point included when route blinding "+
"is disabled")

Expand All @@ -1991,7 +1992,7 @@ func (l *channelLink) handleUpstreamMsg(msg lnwire.Message) {
// without sending a revoke. This would mean that the switch
// check would only occur later.
if l.isOverexposedWithHtlc(msg, true) {
l.fail(LinkFailureError{code: ErrInternalError},
l.failf(LinkFailureError{code: ErrInternalError},
"peer sent us an HTLC that exceeded our max "+
"fee exposure")

Expand All @@ -2003,7 +2004,7 @@ func (l *channelLink) handleUpstreamMsg(msg lnwire.Message) {
// "settle" list in the event that we know the preimage.
index, err := l.channel.ReceiveHTLC(msg)
if err != nil {
l.fail(LinkFailureError{code: ErrInvalidUpdate},
l.failf(LinkFailureError{code: ErrInvalidUpdate},
"unable to handle upstream add HTLC: %v", err)
return
}
Expand All @@ -2029,15 +2030,15 @@ func (l *channelLink) handleUpstreamMsg(msg lnwire.Message) {
}

if !lockedin {
l.fail(
l.failf(
LinkFailureError{code: ErrInvalidUpdate},
"unable to handle upstream settle",
)
return
}

if err := l.channel.ReceiveHTLCSettle(pre, idx); err != nil {
l.fail(
l.failf(
LinkFailureError{
code: ErrInvalidUpdate,
FailureAction: LinkFailureForceClose,
Expand Down Expand Up @@ -2126,7 +2127,7 @@ func (l *channelLink) handleUpstreamMsg(msg lnwire.Message) {
// message to the usual HTLC fail message.
err := l.channel.ReceiveFailHTLC(msg.ID, b.Bytes())
if err != nil {
l.fail(LinkFailureError{code: ErrInvalidUpdate},
l.failf(LinkFailureError{code: ErrInvalidUpdate},
"unable to handle upstream fail HTLC: %v", err)
return
}
Expand Down Expand Up @@ -2164,7 +2165,7 @@ func (l *channelLink) handleUpstreamMsg(msg lnwire.Message) {
idx := msg.ID
err := l.channel.ReceiveFailHTLC(idx, msg.Reason[:])
if err != nil {
l.fail(LinkFailureError{code: ErrInvalidUpdate},
l.failf(LinkFailureError{code: ErrInvalidUpdate},
"unable to handle upstream fail HTLC: %v", err)
return
}
Expand All @@ -2183,7 +2184,7 @@ func (l *channelLink) handleUpstreamMsg(msg lnwire.Message) {
l.uncommittedPreimages...,
)
if err != nil {
l.fail(
l.failf(
LinkFailureError{code: ErrInternalError},
"unable to add preimages=%v to cache: %v",
l.uncommittedPreimages, err,
Expand Down Expand Up @@ -2219,7 +2220,7 @@ func (l *channelLink) handleUpstreamMsg(msg lnwire.Message) {
case *lnwallet.InvalidHtlcSigError:
sendData = []byte(err.Error())
}
l.fail(
l.failf(
LinkFailureError{
code: ErrInvalidCommitment,
FailureAction: LinkFailureForceClose,
Expand Down Expand Up @@ -2248,7 +2249,7 @@ func (l *channelLink) handleUpstreamMsg(msg lnwire.Message) {
// NOTE: We do not trigger a force close because this
// could resolve itself in case our db was just busy
// not accepting new transactions.
l.fail(
l.failf(
LinkFailureError{
code: ErrInternalError,
Warning: true,
Expand Down Expand Up @@ -2336,7 +2337,7 @@ func (l *channelLink) handleUpstreamMsg(msg lnwire.Message) {
ReceiveRevocation(msg)
if err != nil {
// TODO(halseth): force close?
l.fail(
l.failf(
LinkFailureError{
code: ErrInvalidRevocation,
FailureAction: LinkFailureDisconnect,
Expand Down Expand Up @@ -2376,9 +2377,9 @@ func (l *channelLink) handleUpstreamMsg(msg lnwire.Message) {
&chanID, state.RemoteCommitment.CommitHeight-1,
)
if err != nil {
l.fail(LinkFailureError{code: ErrInternalError},
"unable to queue breach backup: %v",
err)
l.failf(LinkFailureError{
code: ErrInternalError,
}, "unable to queue breach backup: %v", err)
return
}
}
Expand Down Expand Up @@ -2425,7 +2426,7 @@ func (l *channelLink) handleUpstreamMsg(msg lnwire.Message) {
// indicates something is wrong with our channel state.
l.log.Errorf("Unable to determine if fee threshold " +
"exceeded")
l.fail(LinkFailureError{code: ErrInternalError},
l.failf(LinkFailureError{code: ErrInternalError},
"error calculating fee exposure: %v", err)

return
Expand All @@ -2434,15 +2435,15 @@ func (l *channelLink) handleUpstreamMsg(msg lnwire.Message) {
if isDust {
// The proposed fee-rate makes us exceed the fee
// threshold.
l.fail(LinkFailureError{code: ErrInternalError},
l.failf(LinkFailureError{code: ErrInternalError},
"fee threshold exceeded: %v", err)
return
}

// We received fee update from peer. If we are the initiator we
// will fail the channel, if not we will apply the update.
if err := l.channel.ReceiveUpdateFee(fee); err != nil {
l.fail(LinkFailureError{code: ErrInvalidUpdate},
l.failf(LinkFailureError{code: ErrInvalidUpdate},
"error receiving fee update: %v", err)
return
}
Expand All @@ -2461,7 +2462,7 @@ func (l *channelLink) handleUpstreamMsg(msg lnwire.Message) {
// Error received from remote, MUST fail channel, but should
// only print the contents of the error message if all
// characters are printable ASCII.
l.fail(
l.failf(
LinkFailureError{
code: ErrRemoteError,

Expand Down Expand Up @@ -2550,14 +2551,14 @@ func (l *channelLink) updateCommitTxOrFail() bool {
// A duplicate keystone error should be resolved and is not fatal, so
// we won't send an Error message to the peer.
case ErrDuplicateKeystone:
l.fail(LinkFailureError{code: ErrCircuitError},
l.failf(LinkFailureError{code: ErrCircuitError},
"temporary circuit error: %v", err)
return false

// Any other error is treated results in an Error message being sent to
// the peer.
default:
l.fail(LinkFailureError{code: ErrInternalError},
l.failf(LinkFailureError{code: ErrInternalError},
"unable to update commitment: %v", err)
return false
}
Expand Down Expand Up @@ -3440,7 +3441,7 @@ func (l *channelLink) processRemoteAdds(fwdPkg *channeldb.FwdPkg,
fwdPkg.ID(), decodeReqs,
)
if sphinxErr != nil {
l.fail(LinkFailureError{code: ErrInternalError},
l.failf(LinkFailureError{code: ErrInternalError},
"unable to decode hop iterators: %v", sphinxErr)
return
}
Expand Down Expand Up @@ -3599,9 +3600,9 @@ func (l *channelLink) processRemoteAdds(fwdPkg *channeldb.FwdPkg,
pd, obfuscator, fwdInfo, heightNow, pld,
)
if err != nil {
l.fail(LinkFailureError{code: ErrInternalError},
err.Error(),
)
l.failf(LinkFailureError{
code: ErrInternalError,
}, err.Error()) //nolint

return
}
Expand Down Expand Up @@ -3745,7 +3746,7 @@ func (l *channelLink) processRemoteAdds(fwdPkg *channeldb.FwdPkg,
if fwdPkg.State == channeldb.FwdStateLockedIn {
err := l.channel.SetFwdFilter(fwdPkg.Height, fwdPkg.FwdFilter)
if err != nil {
l.fail(LinkFailureError{code: ErrInternalError},
l.failf(LinkFailureError{code: ErrInternalError},
"unable to set fwd filter: %v", err)
return
}
Expand Down Expand Up @@ -4077,13 +4078,14 @@ func (l *channelLink) sendMalformedHTLCError(htlcIndex uint64,
})
}

// fail is a function which is used to encapsulate the action necessary for
// failf is a function which is used to encapsulate the action necessary for
// properly failing the link. It takes a LinkFailureError, which will be passed
// to the OnChannelFailure closure, in order for it to determine if we should
// force close the channel, and if we should send an error message to the
// remote peer.
func (l *channelLink) fail(linkErr LinkFailureError,
format string, a ...interface{}) {
func (l *channelLink) failf(linkErr LinkFailureError, format string,
a ...interface{}) {

reason := fmt.Errorf(format, a...)

// Return if we have already notified about a failure.
Expand Down
7 changes: 4 additions & 3 deletions htlcswitch/link_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4879,7 +4879,8 @@ func (h *persistentLinkHarness) restartLink(
FetchLastChannelUpdate: mockGetChanUpdateMessage,
PreimageCache: pCache,
OnChannelFailure: func(lnwire.ChannelID,
lnwire.ShortChannelID, LinkFailureError) { // nolint:whitespace
lnwire.ShortChannelID, LinkFailureError) {

},
UpdateContractSignals: func(*contractcourt.ContractSignals) error {
return nil
Expand Down Expand Up @@ -5834,7 +5835,7 @@ func TestChannelLinkFail(t *testing.T) {
c.cfg.Peer.(*mockPeer).disconnected = true
},
func(*testing.T, *Switch, *channelLink,
*lnwallet.LightningChannel) { //nolint:whitespace,lll
*lnwallet.LightningChannel) {

// Should fail at startup.
},
Expand All @@ -5854,7 +5855,7 @@ func TestChannelLinkFail(t *testing.T) {
c.channel.State().Packager = pkg
},
func(*testing.T, *Switch, *channelLink,
*lnwallet.LightningChannel) { //nolint:whitespace,lll
*lnwallet.LightningChannel) {

// Should fail at startup.
},
Expand Down
Loading

0 comments on commit e99e666

Please sign in to comment.