From 0899cee987f9e5109e6768b2df2942a9da9cb020 Mon Sep 17 00:00:00 2001 From: Eng Zer Jun Date: Mon, 27 Jan 2025 00:30:45 +0800 Subject: [PATCH 1/3] refactor: replace min/max helpers with built-in min/max We can use the built-in `min` and `max` functions since Go 1.21. Reference: https://go.dev/ref/spec#Min_and_max Signed-off-by: Eng Zer Jun --- autopilot/simple_graph.go | 4 +--- lntypes/comparison.go | 26 ----------------------- lntypes/comparison_test.go | 25 ---------------------- routing/unified_edges.go | 13 +++++------- watchtower/wtdb/migration4/range_index.go | 18 ---------------- watchtower/wtdb/migration8/range_index.go | 18 ---------------- watchtower/wtdb/range_index.go | 18 ---------------- 7 files changed, 6 insertions(+), 116 deletions(-) delete mode 100644 lntypes/comparison.go delete mode 100644 lntypes/comparison_test.go diff --git a/autopilot/simple_graph.go b/autopilot/simple_graph.go index c2dc7448f..4d294b3f2 100644 --- a/autopilot/simple_graph.go +++ b/autopilot/simple_graph.go @@ -85,9 +85,7 @@ func NewSimpleGraph(g ChannelGraph) (*SimpleGraph, error) { func maxVal(mapping map[int]uint32) uint32 { maxValue := uint32(0) for _, value := range mapping { - if maxValue < value { - maxValue = value - } + maxValue = max(maxValue, value) } return maxValue } diff --git a/lntypes/comparison.go b/lntypes/comparison.go deleted file mode 100644 index bffb1a55b..000000000 --- a/lntypes/comparison.go +++ /dev/null @@ -1,26 +0,0 @@ -package lntypes - -import "golang.org/x/exp/constraints" - -// Number defines a type constraint for numbers. -type Number interface { - constraints.Integer | constraints.Float -} - -// Max returns the greater of the two inputs. -func Max[N Number](op1 N, op2 N) N { - if op1 > op2 { - return op1 - } - - return op2 -} - -// Min returns the lesser of the two inputs. -func Min[N Number](op1 N, op2 N) N { - if op1 < op2 { - return op1 - } - - return op2 -} diff --git a/lntypes/comparison_test.go b/lntypes/comparison_test.go deleted file mode 100644 index 61bf35893..000000000 --- a/lntypes/comparison_test.go +++ /dev/null @@ -1,25 +0,0 @@ -package lntypes - -import ( - "testing" - - "github.com/stretchr/testify/require" -) - -// TestMin tests getting correct minimal numbers. -func TestMin(t *testing.T) { - t.Parallel() - - require.Equal(t, 1, Min(1, 1)) - require.Equal(t, 1, Min(1, 2)) - require.Equal(t, 1.5, Min(1.5, 2.5)) -} - -// TestMax tests getting correct maximal numbers. -func TestMax(t *testing.T) { - t.Parallel() - - require.Equal(t, 1, Max(1, 1)) - require.Equal(t, 2, Max(1, 2)) - require.Equal(t, 2.5, Max(1.5, 2.5)) -} diff --git a/routing/unified_edges.go b/routing/unified_edges.go index c0e1c1044..b92010da0 100644 --- a/routing/unified_edges.go +++ b/routing/unified_edges.go @@ -6,7 +6,6 @@ import ( "github.com/btcsuite/btcd/btcutil" graphdb "github.com/lightningnetwork/lnd/graph/db" "github.com/lightningnetwork/lnd/graph/db/models" - "github.com/lightningnetwork/lnd/lntypes" "github.com/lightningnetwork/lnd/lnwire" "github.com/lightningnetwork/lnd/routing/route" ) @@ -379,13 +378,11 @@ func (u *edgeUnifier) getEdgeNetwork(netAmtReceived lnwire.MilliSatoshi, capMsat = edge.policy.MaxHTLC } - maxCapMsat = lntypes.Max(capMsat, maxCapMsat) + maxCapMsat = max(capMsat, maxCapMsat) // Track the maximum time lock of all channels that are // candidate for non-strict forwarding at the routing node. - maxTimelock = lntypes.Max( - maxTimelock, edge.policy.TimeLockDelta, - ) + maxTimelock = max(maxTimelock, edge.policy.TimeLockDelta) outboundFee := int64(edge.policy.ComputeFee(amt)) fee := outboundFee + inboundFee @@ -440,10 +437,10 @@ func (u *edgeUnifier) getEdgeNetwork(netAmtReceived lnwire.MilliSatoshi, // minAmt returns the minimum amount that can be forwarded on this connection. func (u *edgeUnifier) minAmt() lnwire.MilliSatoshi { - min := lnwire.MaxMilliSatoshi + minAmount := lnwire.MaxMilliSatoshi for _, edge := range u.edges { - min = lntypes.Min(min, edge.policy.MinHTLC) + minAmount = min(minAmount, edge.policy.MinHTLC) } - return min + return minAmount } diff --git a/watchtower/wtdb/migration4/range_index.go b/watchtower/wtdb/migration4/range_index.go index 9386834e0..afeb98242 100644 --- a/watchtower/wtdb/migration4/range_index.go +++ b/watchtower/wtdb/migration4/range_index.go @@ -74,24 +74,6 @@ func (a *RangeIndex) addRange(start, end uint64) error { "than end height %d", start, end) } - // min is a helper closure that will return the minimum of two uint64s. - min := func(a, b uint64) uint64 { - if a < b { - return a - } - - return b - } - - // max is a helper closure that will return the maximum of two uint64s. - max := func(a, b uint64) uint64 { - if a > b { - return a - } - - return b - } - // Collect the ranges that fall before and after the new range along // with the start and end values of the new range. var before, after []rangeItem diff --git a/watchtower/wtdb/migration8/range_index.go b/watchtower/wtdb/migration8/range_index.go index 94f0e2030..4f870c211 100644 --- a/watchtower/wtdb/migration8/range_index.go +++ b/watchtower/wtdb/migration8/range_index.go @@ -74,24 +74,6 @@ func (a *RangeIndex) addRange(start, end uint64) error { "than end height %d", start, end) } - // min is a helper closure that will return the minimum of two uint64s. - min := func(a, b uint64) uint64 { - if a < b { - return a - } - - return b - } - - // max is a helper closure that will return the maximum of two uint64s. - max := func(a, b uint64) uint64 { - if a > b { - return a - } - - return b - } - // Collect the ranges that fall before and after the new range along // with the start and end values of the new range. var before, after []rangeItem diff --git a/watchtower/wtdb/range_index.go b/watchtower/wtdb/range_index.go index 268b603c7..3a063c7dc 100644 --- a/watchtower/wtdb/range_index.go +++ b/watchtower/wtdb/range_index.go @@ -74,24 +74,6 @@ func (a *RangeIndex) addRange(start, end uint64) error { "than end height %d", start, end) } - // min is a helper closure that will return the minimum of two uint64s. - min := func(a, b uint64) uint64 { - if a < b { - return a - } - - return b - } - - // max is a helper closure that will return the maximum of two uint64s. - max := func(a, b uint64) uint64 { - if a > b { - return a - } - - return b - } - // Collect the ranges that fall before and after the new range along // with the start and end values of the new range. var before, after []rangeItem From e56a7945bea7bc2a6250a6c152de603a04efce06 Mon Sep 17 00:00:00 2001 From: Eng Zer Jun Date: Thu, 30 Jan 2025 23:55:23 +0800 Subject: [PATCH 2/3] refactor: replace math.Min and math.Max with built-in min/max Reference: https://github.com/lightningnetwork/lnd/pull/9451#pullrequestreview-2580942691 Signed-off-by: Eng Zer Jun --- contractcourt/commit_sweep_resolver.go | 5 +---- contractcourt/htlc_lease_resolver.go | 6 +----- lnwallet/channel.go | 14 +++----------- sqldb/interfaces.go | 2 +- 4 files changed, 6 insertions(+), 21 deletions(-) diff --git a/contractcourt/commit_sweep_resolver.go b/contractcourt/commit_sweep_resolver.go index 55ee08e5d..612e293ea 100644 --- a/contractcourt/commit_sweep_resolver.go +++ b/contractcourt/commit_sweep_resolver.go @@ -4,7 +4,6 @@ import ( "encoding/binary" "fmt" "io" - "math" "sync" "github.com/btcsuite/btcd/btcutil" @@ -391,9 +390,7 @@ func (c *commitSweepResolver) Launch() error { // expires after. unlockHeight := confHeight + c.commitResolution.MaturityDelay if c.hasCLTV() { - unlockHeight = uint32(math.Max( - float64(unlockHeight), float64(c.leaseExpiry), - )) + unlockHeight = max(unlockHeight, c.leaseExpiry) } // Update report now that we learned the confirmation height. diff --git a/contractcourt/htlc_lease_resolver.go b/contractcourt/htlc_lease_resolver.go index 6230f9677..8f97b6804 100644 --- a/contractcourt/htlc_lease_resolver.go +++ b/contractcourt/htlc_lease_resolver.go @@ -1,8 +1,6 @@ package contractcourt import ( - "math" - "github.com/btcsuite/btcd/wire" "github.com/lightningnetwork/lnd/chainntnfs" "github.com/lightningnetwork/lnd/channeldb" @@ -42,9 +40,7 @@ func (h *htlcLeaseResolver) deriveWaitHeight(csvDelay uint32, waitHeight := uint32(commitSpend.SpendingHeight) + csvDelay - 1 if h.hasCLTV() { - waitHeight = uint32(math.Max( - float64(waitHeight), float64(h.leaseExpiry), - )) + waitHeight = max(waitHeight, h.leaseExpiry) } return waitHeight diff --git a/lnwallet/channel.go b/lnwallet/channel.go index 6486464d0..345eef3be 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -7,7 +7,6 @@ import ( "crypto/sha256" "errors" "fmt" - "math" "slices" "sync" @@ -9512,7 +9511,7 @@ func (lc *LightningChannel) MaxFeeRate( // rather than us decreasing in local balance. The max fee rate is // always floored by the current fee rate of the channel. idealMaxFee := float64(baseBalance) * maxAllocation - maxFee := math.Max(float64(currentFee), idealMaxFee) + maxFee := max(float64(currentFee), idealMaxFee) maxFeeAllocation := maxFee / float64(baseBalance) maxFeeRate := chainfee.SatPerKWeight(maxFee / (float64(weight) / 1000)) @@ -9538,17 +9537,10 @@ func (lc *LightningChannel) IdealCommitFeeRate(netFeeRate, minRelayFeeRate, switch lc.channelState.ChanType.HasAnchors() && maxFeeRate > maxAnchorCommitFeeRate { case true: - commitFeeRate = chainfee.SatPerKWeight( - math.Min( - float64(netFeeRate), - float64(maxAnchorCommitFeeRate), - ), - ) + commitFeeRate = min(netFeeRate, maxAnchorCommitFeeRate) case false: - commitFeeRate = chainfee.SatPerKWeight( - math.Min(float64(netFeeRate), float64(maxFeeRate)), - ) + commitFeeRate = min(netFeeRate, maxFeeRate) } if commitFeeRate >= minRelayFeeRate { diff --git a/sqldb/interfaces.go b/sqldb/interfaces.go index 1c5b4878f..3197d7f09 100644 --- a/sqldb/interfaces.go +++ b/sqldb/interfaces.go @@ -183,7 +183,7 @@ func randRetryDelay(initialRetryDelay, maxRetryDelay time.Duration, // attempt. If we double something n times, that's the same as // multiplying the value with 2^n. We limit the power to 32 to avoid // overflows. - factor := time.Duration(math.Pow(2, math.Min(float64(attempt), 32))) + factor := time.Duration(math.Pow(2, min(float64(attempt), 32))) actualDelay := initialDelay * factor // Cap the delay at the maximum configured value. From 82a221be13125f13c64df64c1a3655d2f3f977dc Mon Sep 17 00:00:00 2001 From: Eng Zer Jun Date: Mon, 27 Jan 2025 00:48:18 +0800 Subject: [PATCH 3/3] docs: add release note for #9451 Signed-off-by: Eng Zer Jun --- docs/release-notes/release-notes-0.19.0.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/release-notes/release-notes-0.19.0.md b/docs/release-notes/release-notes-0.19.0.md index 6979d796b..0968e00cd 100644 --- a/docs/release-notes/release-notes-0.19.0.md +++ b/docs/release-notes/release-notes-0.19.0.md @@ -322,6 +322,9 @@ The underlying functionality between those two options remain the same. to the sweeper to improve code quality, with a renaming of the internal state (`Failed` -> `Fatal`) used by the inputs tracked in the sweeper. +* A code refactor that [replaces min/max helpers with built-in min/max + functions](https://github.com/lightningnetwork/lnd/pull/9451). + ## Tooling and Documentation * [Improved `lncli create` command help text](https://github.com/lightningnetwork/lnd/pull/9077)