tapchannel: add an extra unit of tolerance to invoice acceptor

When converting an mSAT amount to units and vice versa we frequently get
an off-by-1-unit result. This is already accounted for in our invoice
acceptor by adding the value of as many asset units as HTLCs in the
invoice. What we didn't account for is that the UnitsToMilliSatoshi
calculation for the margin can also be off by 1 msat.
Allowing for an extra unit of tolerance should fix those off-by-one
errors.

Author: GeorgeTsagk

tapchannel/aux_invoice_manager.go

@@ -264,8 +264,19 @@ func (s *AuxInvoiceManager) handleInvoiceAccept(ctx context.Context,
 
 	// We assume that each shard can have a rounding error of up to 1 asset
 	// unit. So we allow the final amount to be off by up to 1 asset unit
-	// per accepted HTLC (plus the one we're currently processing).
+	// per accepted HTLC (plus the one we're currently processing). The
+	// plus one here is because the invoice only has previously accepted
+	// HTLCs committed to it, but we're processing the current HTLC which
+	// is not yet in that list.
 	allowedMarginAssetUnits := uint64(len(req.Invoice.Htlcs) + 1)
+
+	// Because we do a round trip of units -> milli-sats, then split into
+	// shards, then back to units and then back to milli-sats again, we lose
+	// up to numHTLCs+1 asset units in the process. So we allow for an
+	// additional unit here.
+	allowedMarginAssetUnits++
+
+	// Convert the allowed margin asset units to milli-satoshis.
 	marginAssetUnits := rfqmath.NewBigIntFixedPoint(
 		allowedMarginAssetUnits, 0,
 	)

tapchannel/aux_traffic_shaper_test.go

@@ -0,0 +1,114 @@
+package tapchannel
+
+import (
+	"testing"
+
+	"github.com/lightninglabs/taproot-assets/rfqmath"
+	"github.com/lightningnetwork/lnd/lnwire"
+	"github.com/stretchr/testify/require"
+	"pgregory.net/rapid"
+)
+
+// TestUnitConversionTolerance demonstrates the unit conversion loss of
+// precision issue that lead to the increased tolerance value in the invoice
+// manager.
+func TestUnitConversionTolerance(t *testing.T) {
+	const (
+		invoiceAmtUnits = 6_000_000
+		numHTLCs        = 2
+	)
+	var (
+		rate = rfqmath.BigIntFixedPoint{
+			Coefficient: rfqmath.NewBigIntFromUint64(9852216748),
+			Scale:       0,
+		}
+	)
+
+	t.Logf("Initial amount in asset units: %d", invoiceAmtUnits)
+
+	assetAmountFP := rfqmath.NewBigIntFixedPoint(invoiceAmtUnits, 0)
+	invoiceAmtMsat := rfqmath.UnitsToMilliSatoshi(assetAmountFP, rate)
+
+	t.Logf("Invoice amount in msat: %d", invoiceAmtMsat)
+
+	numAssetUnitsFp := rfqmath.MilliSatoshiToUnits(invoiceAmtMsat, rate)
+	numAssetUnits := numAssetUnitsFp.ScaleTo(0).ToUint64()
+
+	msatPerUnit := float64(invoiceAmtMsat) / float64(numAssetUnits)
+	t.Logf("Calculated msat per asset unit: %.2f", msatPerUnit)
+
+	t.Logf("Number of asset units after round trip: %d", numAssetUnits)
+
+	shardSizeMSat := invoiceAmtMsat / numHTLCs
+
+	shardSizeFP := rfqmath.MilliSatoshiToUnits(shardSizeMSat, rate)
+	shardSizeUnit := shardSizeFP.ScaleTo(0).ToUint64()
+	t.Logf("Sum of %d shards in asset units: %d", numHTLCs,
+		shardSizeUnit*numHTLCs)
+
+	shardSumFP := rfqmath.NewBigIntFixedPoint(shardSizeUnit*numHTLCs, 0)
+	inboundAmountMSat := rfqmath.UnitsToMilliSatoshi(shardSumFP, rate)
+
+	t.Logf("Inbound amount in msat: %d", inboundAmountMSat)
+	t.Logf("Total tolerance required in msat: %d",
+		invoiceAmtMsat-inboundAmountMSat)
+
+	marginAssetUnits := rfqmath.NewBigIntFixedPoint(numHTLCs, 0)
+	allowedMarginMSat := rfqmath.UnitsToMilliSatoshi(marginAssetUnits, rate)
+
+	newMarginAssetUnits := rfqmath.NewBigIntFixedPoint(numHTLCs+1, 0)
+	newAllowedMarginMSat := rfqmath.UnitsToMilliSatoshi(
+		newMarginAssetUnits, rate,
+	)
+
+	t.Logf("Old tolerance allowed in msat: %d", allowedMarginMSat)
+	t.Logf("New tolerance allowed in msat: %d", newAllowedMarginMSat)
+}
+
+// TestUnitConversionToleranceRapid uses rapid to randomly draw invoice amounts,
+// HTLC counts, and coefficients to test unit conversion tolerance. This ensures
+// the conversion logic is robust against a wide range of values.
+func TestUnitConversionToleranceRapid(t *testing.T) {
+	rapid.Check(t, func(t *rapid.T) {
+		invoiceAmtUnits := rapid.Uint64Range(1, 10_000_000).
+			Draw(t, "invoiceAmtUnits")
+		numHTLCs := rapid.Uint64Range(1, 16).
+			Draw(t, "numHTLCs")
+		coefficient := rapid.Uint64Range(1, 20_000_000_000).
+			Draw(t, "coefficient")
+
+		rate := rfqmath.BigIntFixedPoint{
+			Coefficient: rfqmath.NewBigIntFromUint64(coefficient),
+			Scale:       0,
+		}
+
+		assetAmountFP := rfqmath.NewBigIntFixedPoint(invoiceAmtUnits, 0)
+		invoiceAmtMsat := rfqmath.UnitsToMilliSatoshi(
+			assetAmountFP, rate,
+		)
+
+		shardSizeMSat := invoiceAmtMsat / lnwire.MilliSatoshi(numHTLCs)
+		shardSizeFP := rfqmath.MilliSatoshiToUnits(shardSizeMSat, rate)
+		shardSizeUnit := shardSizeFP.ScaleTo(0).ToUint64()
+
+		shardSumFP := rfqmath.NewBigIntFixedPoint(
+			shardSizeUnit*numHTLCs, 0,
+		)
+		inboundAmountMSat := rfqmath.UnitsToMilliSatoshi(
+			shardSumFP, rate,
+		)
+
+		newMarginAssetUnits := rfqmath.NewBigIntFixedPoint(
+			numHTLCs+1, 0,
+		)
+		newAllowedMarginMSat := rfqmath.UnitsToMilliSatoshi(
+			newMarginAssetUnits, rate,
+		)
+
+		// The difference should be within the newly allowed margin.
+		require.LessOrEqual(
+			t,
+			invoiceAmtMsat-inboundAmountMSat, newAllowedMarginMSat,
+		)
+	})
+}