diff --git a/config_builder.go b/config_builder.go index f5f3376df0..8212c486e5 100644 --- a/config_builder.go +++ b/config_builder.go @@ -55,6 +55,7 @@ import ( paymentsdb "github.com/lightningnetwork/lnd/payments/db" paymentsmig1 "github.com/lightningnetwork/lnd/payments/db/migration1" paymentsmig1sqlc "github.com/lightningnetwork/lnd/payments/db/migration1/sqlc" + "github.com/lightningnetwork/lnd/reputation" "github.com/lightningnetwork/lnd/rpcperms" "github.com/lightningnetwork/lnd/signal" "github.com/lightningnetwork/lnd/sqldb" @@ -1028,6 +1029,11 @@ type DatabaseInstances struct { // be used for native SQL queries for tables that already support it. // This may be nil if the use-native-sql flag was not set. NativeSQLStore sqldb.DB + + // ReputationStore is the database that stores the local reputation + // state of channels. It is only set when the native SQL store is in + // use; otherwise reputation is kept in memory only. + ReputationStore reputation.Store } // DefaultDatabaseBuilder is a type that builds the default database backends @@ -1360,6 +1366,16 @@ func (d *DefaultDatabaseBuilder) BuildDatabase( dbs.InvoiceDB = sqlInvoiceDB + // Create the reputation store. + reputationExecutor := sqldb.NewTransactionExecutor( + baseDB, func(tx *sql.Tx) reputation.SQLQueries { + return baseDB.WithTx(tx) + }, + ) + dbs.ReputationStore = reputation.NewSQLStore( + reputationExecutor, + ) + // Create the graph store. graphExecutor := sqldb.NewTransactionExecutor( baseDB, func(tx *sql.Tx) graphdb.SQLQueries { diff --git a/docs/release-notes/unreleased/11266.md b/docs/release-notes/unreleased/11266.md new file mode 100644 index 0000000000..6f4be7a29f --- /dev/null +++ b/docs/release-notes/unreleased/11266.md @@ -0,0 +1,20 @@ +# New Features + +## Functional Enhancements + +* The local reputation subsystem now [persists its channel + state](https://github.com/lightningnetwork/lnd/pull/11266) when the native SQL + store is in use (`db.use-native-sql`), so peers keep the reputation they have + built across a restart, with the downtime decaying it as if the node had + stayed online. HTLCs that were in flight during the restart are picked up + again from the switch on startup, and closed channels are dropped from the + reputation state. Nodes without the native SQL store keep the previous + in-memory behaviour. + +# Technical and Architectural Updates + +## Database + +* A new native SQL table, `reputation_channels`, [stores the local reputation + state of channels](https://github.com/lightningnetwork/lnd/pull/11266). It is + created by schema migration 16 for nodes running with `db.use-native-sql`. diff --git a/htlcswitch/circuit_map.go b/htlcswitch/circuit_map.go index fb94d5f4d3..73e27b7e95 100644 --- a/htlcswitch/circuit_map.go +++ b/htlcswitch/circuit_map.go @@ -123,6 +123,10 @@ type CircuitMap interface { // circuits that use the given payment hash. LookupByPaymentHash(hash [32]byte) []*PaymentCircuit + // ActiveCircuits returns all open circuits, i.e. those whose HTLC has + // been forwarded on an outgoing link and is awaiting resolution. + ActiveCircuits() []*PaymentCircuit + // NumPending returns the total number of active circuits added by // CommitCircuits. NumPending() int @@ -1248,3 +1252,19 @@ func (cm *circuitMap) NumOpen() int { return len(cm.opened) } + +// ActiveCircuits returns a snapshot of all open circuits, i.e. those whose +// HTLC has been forwarded on an outgoing link and is awaiting a settle or fail +// from the remote peer. The circuit map never mutates an opened circuit in +// place, so the returned circuits are safe to read without the lock. +func (cm *circuitMap) ActiveCircuits() []*PaymentCircuit { + cm.mtx.RLock() + defer cm.mtx.RUnlock() + + circuits := make([]*PaymentCircuit, 0, len(cm.opened)) + for _, c := range cm.opened { + circuits = append(circuits, c) + } + + return circuits +} diff --git a/htlcswitch/circuit_test.go b/htlcswitch/circuit_test.go index c71087a094..7bd675557b 100644 --- a/htlcswitch/circuit_test.go +++ b/htlcswitch/circuit_test.go @@ -1359,3 +1359,74 @@ func TestCircuitMapDeleteOpenCircuit(t *testing.T) { circuit2, nil) } } + +// TestCircuitMapActiveCircuits checks that ActiveCircuits returns exactly the +// opened circuits, that they survive a restart of the circuit map, and that a +// circuit is no longer reported once it is closed. +func TestCircuitMapActiveCircuits(t *testing.T) { + t.Parallel() + + cfg, circuitMap := newCircuitMap(t, false) + + // Nothing tracked yet. + require.Empty(t, circuitMap.ActiveCircuits()) + + // One circuit that is only committed, never opened, and two that are + // opened on outgoing channels. + pending := htlcswitch.Keystone{ + InKey: htlcswitch.CircuitKey{ + ChanID: lnwire.NewShortChanIDFromInt(1), HtlcID: 0, + }, + } + openOne := htlcswitch.Keystone{ + InKey: htlcswitch.CircuitKey{ + ChanID: lnwire.NewShortChanIDFromInt(1), HtlcID: 1, + }, + OutKey: htlcswitch.CircuitKey{ + ChanID: lnwire.NewShortChanIDFromInt(2), HtlcID: 0, + }, + } + openTwo := htlcswitch.Keystone{ + InKey: htlcswitch.CircuitKey{ + ChanID: lnwire.NewShortChanIDFromInt(3), HtlcID: 0, + }, + OutKey: htlcswitch.CircuitKey{ + ChanID: lnwire.NewShortChanIDFromInt(2), HtlcID: 1, + }, + } + for _, ks := range []htlcswitch.Keystone{pending, openOne, openTwo} { + require.NoError(t, createTestCircuit(ks, circuitMap)) + } + + // activeKeys reduces the snapshot to (incoming, outgoing) key pairs + // for order independent comparison. + activeKeys := func(cm htlcswitch.CircuitMap) map[htlcswitch.CircuitKey]htlcswitch.CircuitKey { //nolint:ll + keys := make(map[htlcswitch.CircuitKey]htlcswitch.CircuitKey) + for _, c := range cm.ActiveCircuits() { + require.NotNil(t, c.Outgoing, "open circuit has no "+ + "outgoing key") + keys[c.Incoming] = *c.Outgoing + } + + return keys + } + + want := map[htlcswitch.CircuitKey]htlcswitch.CircuitKey{ + openOne.InKey: openOne.OutKey, + openTwo.InKey: openTwo.OutKey, + } + require.Equal(t, want, activeKeys(circuitMap)) + + // Opened circuits are persisted, so they are still active after the + // circuit map is rebuilt from disk. + _, circuitMap = restartCircuitMap(t, cfg) + require.Equal(t, want, activeKeys(circuitMap)) + + // Closing one drops it from the snapshot. + _, err := circuitMap.FailCircuit(openOne.InKey) + require.NoError(t, err) + require.NoError(t, circuitMap.DeleteCircuits(openOne.InKey)) + + delete(want, openOne.InKey) + require.Equal(t, want, activeKeys(circuitMap)) +} diff --git a/htlcswitch/mock.go b/htlcswitch/mock.go index d480a70386..e57184a1b4 100644 --- a/htlcswitch/mock.go +++ b/htlcswitch/mock.go @@ -1150,6 +1150,10 @@ func (m *mockCircuitMap) LookupByPaymentHash(hash [32]byte) []*PaymentCircuit { return nil } +func (m *mockCircuitMap) ActiveCircuits() []*PaymentCircuit { + return nil +} + func (m *mockCircuitMap) NumPending() int { return 0 } diff --git a/htlcswitch/switch.go b/htlcswitch/switch.go index 4d5cbf9e93..7ae87af14a 100644 --- a/htlcswitch/switch.go +++ b/htlcswitch/switch.go @@ -2624,6 +2624,12 @@ func (s *Switch) CircuitLookup() CircuitLookup { return s.circuits } +// ActiveCircuits returns a snapshot of the open circuits tracked by the switch: +// the HTLCs it has forwarded on an outgoing link that are awaiting resolution. +func (s *Switch) ActiveCircuits() []*PaymentCircuit { + return s.circuits.ActiveCircuits() +} + // commitCircuits persistently adds a circuit to the switch's circuit map. func (s *Switch) commitCircuits(circuits ...*PaymentCircuit) ( *CircuitFwdActions, error) { @@ -3233,10 +3239,16 @@ func (s *Switch) handlePacketAdd(packet *htlcPacket, } // htlcAccountable extracts the experimental accountable signal from an -// incoming update_add_htlc's custom records (TLV 106823). +// incoming update_add_htlc's custom records. func htlcAccountable(htlc *lnwire.UpdateAddHTLC) bool { + return AccountableFromRecords(htlc.CustomRecords) +} + +// AccountableFromRecords reports whether the given custom records carry the +// experimental accountable signal (TLV 106823). +func AccountableFromRecords(records lnwire.CustomRecords) bool { key := uint64(lnwire.ExperimentalAccountableType) - rec, ok := htlc.CustomRecords[key] + rec, ok := records[key] return ok && len(rec) > 0 && rec[0] == lnwire.ExperimentalAccountable } diff --git a/itest/list_on_test.go b/itest/list_on_test.go index fd236c25a1..db4bc7b10f 100644 --- a/itest/list_on_test.go +++ b/itest/list_on_test.go @@ -14,6 +14,10 @@ var allTestCases = []*lntest.TestCase{ Name: "local reputation log only", TestFunc: testLocalReputationLogOnly, }, + { + Name: "local reputation persistence", + TestFunc: testLocalReputationPersistence, + }, { Name: "update channel status", TestFunc: testUpdateChanStatus, diff --git a/itest/lnd_reputation_persistence_test.go b/itest/lnd_reputation_persistence_test.go new file mode 100644 index 0000000000..3b29bd4745 --- /dev/null +++ b/itest/lnd_reputation_persistence_test.go @@ -0,0 +1,223 @@ +package itest + +import ( + "fmt" + "regexp" + "strconv" + + "github.com/btcsuite/btcd/btcutil/v2" + "github.com/lightningnetwork/lnd/lnrpc" + "github.com/lightningnetwork/lnd/lnrpc/invoicesrpc" + "github.com/lightningnetwork/lnd/lnrpc/routerrpc" + "github.com/lightningnetwork/lnd/lntest" + "github.com/lightningnetwork/lnd/lntest/node" + "github.com/lightningnetwork/lnd/lntest/wait" + "github.com/lightningnetwork/lnd/lntypes" + "github.com/lightningnetwork/lnd/lnwire" + "github.com/stretchr/testify/require" +) + +var ( + // reputationValueRe extracts the new outgoing reputation from the + // reputation change line the subsystem logs on every resolution. + reputationValueRe = regexp.MustCompile( + `new_outgoing_reputation=(-?\d+)`, + ) + + // reputationSettledRe extracts whether each logged reputation change + // came from a settled or a failed HTLC. + reputationSettledRe = regexp.MustCompile( + `Reputation change: .* settled=(true|false)`, + ) +) + +// lastOutgoingReputation returns the outgoing reputation reported by the most +// recent reputation change line in the node's log. +func lastOutgoingReputation(ht *lntest.HarnessTest, + hn *node.HarnessNode) int64 { + + matches := ht.NodeLogSubmatches(hn, reputationValueRe) + require.NotEmpty(ht, matches, "no reputation change logged") + + value, err := strconv.ParseInt(matches[len(matches)-1][1], 10, 64) + require.NoError(ht, err) + + return value +} + +// countReputationChanges returns how many reputation changes the node has +// logged for settled and for failed HTLCs. +func countReputationChanges(ht *lntest.HarnessTest, + hn *node.HarnessNode) (int, int) { + + var settled, failed int + for _, m := range ht.NodeLogSubmatches(hn, reputationSettledRe) { + if m[1] == "true" { + settled++ + } else { + failed++ + } + } + + return settled, failed +} + +// addHoldInvoice creates a hold invoice at the node for the given amount and +// returns its preimage and payment request. +func addHoldInvoice(ht *lntest.HarnessTest, hn *node.HarnessNode, + amt int64) (lntypes.Preimage, string) { + + preimage := ht.RandomPreimage() + hash := preimage.Hash() + invoice := hn.RPC.AddHoldInvoice(&invoicesrpc.AddHoldInvoiceRequest{ + Value: amt, + Hash: hash[:], + }) + + return preimage, invoice.PaymentRequest +} + +// testLocalReputationPersistence verifies that the local reputation subsystem +// carries its state across a restart of the forwarding node: channel +// reputation is restored from the store when the native SQL backend is in use, +// HTLCs that were in flight during the restart are picked up again with their +// accountable signal and scored when they settle or fail, and closed channels +// are dropped. +func testLocalReputationPersistence(ht *lntest.HarnessTest) { + const chanAmt = btcutil.Amount(100_000) + const paymentAmt = 1000 + + // Alice -> Bob -> Carol, with Bob as the forwarding node whose + // reputation subsystem is under test. + alice := ht.NewNodeWithCoins("Alice", nil) + bob := ht.NewNodeWithCoins("Bob", nil) + carol := ht.NewNode("Carol", nil) + + ht.ConnectNodes(alice, bob) + ht.ConnectNodes(bob, carol) + + chanPointAB := ht.OpenChannel( + alice, bob, lntest.OpenChannelParams{Amt: chanAmt}, + ) + chanPointBC := ht.OpenChannel( + bob, carol, lntest.OpenChannelParams{Amt: chanAmt}, + ) + ht.AssertChannelInGraph(alice, chanPointBC) + + // 1. A settled forward gives the Bob -> Carol channel its first + // reputation. Record the value Bob reports for it. + payReqs, _, _ := ht.CreatePayReqs(carol, paymentAmt, 1) + ht.CompletePaymentRequests(alice, payReqs) + ht.AssertNodeLogContains(bob, reputationChangeLog) + + repBefore := lastOutgoingReputation(ht, bob) + require.Positive(ht, repBefore, "settled forward earned nothing") + + // 2. Two more forwards are held at Carol with hold invoices, so they + // are in flight on both of Bob's channels when he restarts. The + // second one is sent with the experimental accountable signal, which + // Bob must recover from his commitment state after the restart. + settlePreimage, settleReq := addHoldInvoice(ht, carol, paymentAmt) + cancelPreimage, cancelReq := addHoldInvoice(ht, carol, paymentAmt) + + ht.SendPaymentAndAssertStatus(alice, &routerrpc.SendPaymentRequest{ + PaymentRequest: settleReq, + FeeLimitMsat: noFeeLimitMsat, + }, lnrpc.Payment_IN_FLIGHT) + + ht.SendPaymentAndAssertStatus(alice, &routerrpc.SendPaymentRequest{ + PaymentRequest: cancelReq, + FeeLimitMsat: noFeeLimitMsat, + FirstHopCustomRecords: map[uint64][]byte{ + uint64(lnwire.ExperimentalAccountableType): { + lnwire.ExperimentalAccountable, + }, + }, + }, lnrpc.Payment_IN_FLIGHT) + + // Bob has the incoming and the outgoing HTLC of both forwards. + ht.AssertNumActiveHtlcs(bob, 4) + + settledBefore, failedBefore := countReputationChanges(ht, bob) + + // 3. Restart Bob with both forwards still in flight. + ht.RestartNode(bob) + ht.EnsureConnected(alice, bob) + ht.EnsureConnected(bob, carol) + ht.AssertChannelActive(bob, chanPointAB) + ht.AssertChannelActive(bob, chanPointBC) + + // Both in-flight forwards are rebuilt from the switch's open circuits, + // with the accountable signal of the second one recovered. + ht.AssertNodeLogContains( + bob, "Reputation replayed 2 of 2 in-flight HTLCs "+ + "(1 accountable)", + ) + + // With the native SQL store both of Bob's channels come back from the + // store: one earned reputation as the outgoing link and the other + // revenue as the incoming link. Without it nothing is persisted. + loadedLog := "Reputation loaded 0 channels from store" + if *nativeSQLFlag { + loadedLog = "Reputation loaded 2 channels from store" + } + ht.AssertNodeLogContains(bob, loadedLog) + + // 4. Carol settles the first held invoice. Bob must match the + // resolution to the replayed HTLC and score it as settled. + carol.RPC.SettleInvoice(settlePreimage[:]) + ht.AssertPaymentStatus( + alice, settlePreimage.Hash(), lnrpc.Payment_SUCCEEDED, + ) + + err := waitForReputationChanges( + ht, bob, settledBefore+1, failedBefore, + ) + require.NoError(ht, err, "replayed settle not scored") + + // With persistence the fee earned by this settle is added on top of + // the reputation from before the restart, so the channel ends up above + // where it was. Without persistence it starts from zero again and the + // single fee cannot exceed the value a single fee produced before. + repAfter := lastOutgoingReputation(ht, bob) + if *nativeSQLFlag { + require.Greater(ht, repAfter, repBefore, + "reputation not carried across the restart") + } else { + require.LessOrEqual(ht, repAfter, repBefore, + "reputation carried across restart without a store") + } + + // 5. Carol cancels the second held invoice. The failure must be + // matched to the replayed HTLC and scored as failed. + cancelHash := cancelPreimage.Hash() + carol.RPC.CancelInvoice(cancelHash[:]) + ht.AssertPaymentStatus(alice, cancelHash, lnrpc.Payment_FAILED) + + err = waitForReputationChanges( + ht, bob, settledBefore+1, failedBefore+1, + ) + require.NoError(ht, err, "replayed fail not scored") + + // 6. Closing Bob's channels drops their reputation state. + ht.CloseChannel(alice, chanPointAB) + ht.CloseChannel(bob, chanPointBC) + ht.AssertNodeLogCountAtLeast(bob, "Reputation removed channel", 2) +} + +// waitForReputationChanges waits until the node has logged exactly the given +// number of settled and failed reputation changes. +func waitForReputationChanges(ht *lntest.HarnessTest, hn *node.HarnessNode, + wantSettled, wantFailed int) error { + + return wait.NoError(func() error { + settled, failed := countReputationChanges(ht, hn) + if settled == wantSettled && failed == wantFailed { + return nil + } + + return fmt.Errorf("reputation changes: settled=%d failed=%d, "+ + "want settled=%d failed=%d", settled, failed, + wantSettled, wantFailed) + }, lntest.DefaultTimeout) +} diff --git a/lncfg/db.go b/lncfg/db.go index 4a8680b386..82530cf51d 100644 --- a/lncfg/db.go +++ b/lncfg/db.go @@ -83,7 +83,7 @@ type DB struct { Sqlite *sqldb.SqliteConfig `group:"sqlite" namespace:"sqlite" description:"Sqlite settings."` - UseNativeSQL bool `long:"use-native-sql" description:"If set to true, native SQL will be used instead of KV emulation for tables that support it. Subsystems which support native SQL tables: Invoices, Graph."` + UseNativeSQL bool `long:"use-native-sql" description:"If set to true, native SQL will be used instead of KV emulation for tables that support it. Subsystems which support native SQL tables: Invoices, Graph, Reputation."` SkipNativeSQLMigration bool `long:"skip-native-sql-migration" description:"If set to true, the KV to native SQL migration will be skipped. Note that this option is intended for users who experience non-resolvable migration errors. Enabling after there is a non-resolvable migration error that resulted in an incomplete migration will cause that partial migration to be abandoned and ignored and an empty database will be used instead. Since invoices are currently the only native SQL database used, our channels will still work but the invoice history will be forgotten. This option has no effect if native SQL is not in use (db.use-native-sql=false)."` diff --git a/lntest/harness_assertion.go b/lntest/harness_assertion.go index a996e52a98..adbfc8c9f6 100644 --- a/lntest/harness_assertion.go +++ b/lntest/harness_assertion.go @@ -10,6 +10,7 @@ import ( "math" "os" "path/filepath" + "regexp" "sort" "strings" "time" @@ -102,6 +103,30 @@ func (h *HarnessTest) CountNodeLogOccurrences(hn *node.HarnessNode, substr string) int { var total int + for _, data := range h.readNodeLogs(hn) { + total += strings.Count(data, substr) + } + + return total +} + +// NodeLogSubmatches returns the submatches of every match of the given regular +// expression across the node's lnd.log files, in the order they were logged. +// It is used to read values out of log-only subsystem output. +func (h *HarnessTest) NodeLogSubmatches(hn *node.HarnessNode, + re *regexp.Regexp) [][]string { + + var matches [][]string + for _, data := range h.readNodeLogs(hn) { + matches = append(matches, re.FindAllStringSubmatch(data, -1)...) + } + + return matches +} + +// readNodeLogs returns the contents of the node's lnd.log files. +func (h *HarnessTest) readNodeLogs(hn *node.HarnessNode) []string { + var logs []string _ = filepath.WalkDir(hn.Cfg.LogDir, func(path string, d os.DirEntry, err error) error { @@ -115,13 +140,13 @@ func (h *HarnessTest) CountNodeLogOccurrences(hn *node.HarnessNode, data, readErr := os.ReadFile(path) if readErr == nil { - total += strings.Count(string(data), substr) + logs = append(logs, string(data)) } return nil }) - return total + return logs } // WaitForBlockchainSyncTo waits until the node is synced to bestBlock. diff --git a/reputation/README.md b/reputation/README.md index 2a7742a3d7..df1ba498d2 100644 --- a/reputation/README.md +++ b/reputation/README.md @@ -14,7 +14,7 @@ peers that are likely being used to jam its channels from those that are not. The package is **observational only**: it watches the HTLCs the node forwards, maintains a per-channel reputation score, and logs the decision it would make -for each HTLC. It never affects forwarding, alters the wire, or writes to disk. +for each HTLC. It never affects forwarding or alters the wire. Channels are identified by their `channel_id` rather than their short channel id, so a zero-conf channel confirming (or, in future, a splice) does not split @@ -71,11 +71,32 @@ the worst case time it could be held for means a resolution was never reported to us. Such entries are logged as a warning and deliberately left in place rather than swept away, so the underlying bug stays visible. +## Persistence + +Channel state is persisted through the `Store` interface so that peers keep +the reputation they built when the node restarts. The SQL implementation is +used when the node runs with `db.use-native-sql`; otherwise a no-op store is +used, reputation lives in memory only and re-accrues from live traffic after +a restart. + +Only the two averages are stored, one row per `channel_id`, each as its +running value and the time it was last updated, plus the revenue start time +for the warm-up factor. Decay is applied lazily on read, so restoring those +verbatim is all that is needed for the downtime to be accounted for: the first +read after a restart decays the value over the whole gap. Timestamps that lie +in the future on load (the clock went backwards) are clamped to the load time. + +Channels whose state changed are written every minute and on shutdown, so an +unclean shutdown loses at most the last minute of changes. When a channel +closes its state is removed from memory and from the store. + +Pending HTLCs are not persisted. On startup they are rebuilt from the switch's +in-flight circuits, so in-flight risk survives a restart too. + ## Operational notes The subsystem is enabled by default and can be disabled with the -`routing.no-reputation` configuration flag. It holds no persisted state, so -reputation resets on restart and re-accrues from live forwarding traffic. +`routing.no-reputation` configuration flag. ## Installation and Updating diff --git a/reputation/bench_test.go b/reputation/bench_test.go index 0fa93a1470..305d470dfd 100644 --- a/reputation/bench_test.go +++ b/reputation/bench_test.go @@ -18,6 +18,7 @@ import ( func BenchmarkForwardResolve(b *testing.B) { m, err := NewManager( DefaultConfig(), clock.NewTestClock(time.Unix(1_000_000, 0)), + nil, ) if err != nil { b.Fatalf("NewManager: %v", err) @@ -43,6 +44,7 @@ func BenchmarkForwardResolve(b *testing.B) { func BenchmarkOnForward(b *testing.B) { m, err := NewManager( DefaultConfig(), clock.NewTestClock(time.Unix(1_000_000, 0)), + nil, ) if err != nil { b.Fatalf("NewManager: %v", err) @@ -89,6 +91,7 @@ func BenchmarkForwardResolveManyChannels(b *testing.B) { m, err := NewManager( DefaultConfig(), clock.NewTestClock(time.Unix(1_000_000, 0)), + nil, ) if err != nil { b.Fatalf("NewManager: %v", err) diff --git a/reputation/channel.go b/reputation/channel.go index b417ca4131..d7daf8459a 100644 --- a/reputation/channel.go +++ b/reputation/channel.go @@ -1,6 +1,10 @@ package reputation -import "time" +import ( + "time" + + "github.com/lightningnetwork/lnd/lnwire" +) // channelReputation holds all per-channel reputation state. A single channel // plays both roles: as an outgoing link it accrues reputation and holds the @@ -38,6 +42,41 @@ func newChannelReputation(cfg Config, } } +// restoreChannelReputation rebuilds channel state from its persisted form, with +// no pending HTLCs: those are replayed separately from the switch's in-flight +// circuits. +func restoreChannelReputation(cfg Config, + state ChannelState) *channelReputation { + + return &channelReputation{ + outgoingReputation: restoreDecayingAverage( + state.OutgoingReputation, + state.OutgoingReputationUpdatedAt, + cfg.reputationWindow(), + ), + incomingRevenue: restoreAggregatedWindowAverage( + cfg.RevenueWindow, cfg.RevenueWindowCount, + state.IncomingRevenueStartedAt, state.IncomingRevenue, + state.IncomingRevenueUpdatedAt, + ), + pendingHTLCs: make(map[htlcRef]*pendingHTLC), + } +} + +// state returns the persistable form of this channel's averages. +func (c *channelReputation) state(chanID lnwire.ChannelID) ChannelState { + rep, rev := c.outgoingReputation, c.incomingRevenue + + return ChannelState{ + ChanID: chanID, + OutgoingReputation: rep.value.Int64(), + OutgoingReputationUpdatedAt: rep.lastUpdated, + IncomingRevenue: rev.inner.value.Int64(), + IncomingRevenueUpdatedAt: rev.inner.lastUpdated, + IncomingRevenueStartedAt: rev.start, + } +} + // inFlightRisk returns the total worst-case opportunity cost of the HTLCs // already in flight on this channel as an outgoing link. Per BOLT #1280 only // accountable HTLCs contribute: an unaccountable HTLC was never told it would diff --git a/reputation/decaying_average.go b/reputation/decaying_average.go index d0b3e43303..358b0f6095 100644 --- a/reputation/decaying_average.go +++ b/reputation/decaying_average.go @@ -24,6 +24,21 @@ func newDecayingAverage(start time.Time, } } +// restoreDecayingAverage rebuilds a decaying average from persisted state. The +// value and its timestamp are taken verbatim: decay is applied lazily on read, +// so the first read decays the value over the whole time the state was not +// live, including any downtime. Re-stamping the timestamp to the load time +// would silently skip that decay. +func restoreDecayingAverage(value int64, lastUpdated time.Time, + window time.Duration) *decayingAverage { + + return &decayingAverage{ + value: satFromInt(value), + lastUpdated: lastUpdated, + decayRate: decayRateForWindow(window), + } +} + // decayRateForWindow computes the per-second decay rate for the given window. // BOLT #1280 defines decay_rate = (1/2)^(1/(ln2 * window)); raised to elapsed // seconds this is e^(-elapsed/window), so the value decays to 1/e of itself diff --git a/reputation/manager.go b/reputation/manager.go index 7190c8b299..1e65cb3c95 100644 --- a/reputation/manager.go +++ b/reputation/manager.go @@ -10,9 +10,18 @@ import ( "github.com/lightningnetwork/lnd/lnwire" ) -// staleCheckInterval is how often the manager checks for pending HTLCs that -// have outlived the worst case time they could be held for. -const staleCheckInterval = 5 * time.Minute +const ( + // staleCheckInterval is how often the manager checks for pending HTLCs + // that have outlived the worst case time they could be held for. + staleCheckInterval = 5 * time.Minute + + // flushInterval is how often channels whose state changed are written + // to the store. + flushInterval = time.Minute + + // storeTimeout bounds each store operation. + storeTimeout = 30 * time.Second +) // Manager is the local reputation subsystem. It observes forwarded HTLCs via // its OnForward/OnSettle/OnFail hooks, maintains per-channel reputation state, @@ -24,15 +33,31 @@ const staleCheckInterval = 5 * time.Minute // is a handful of map lookups and floating-point operations, so it is cheap // enough to sit on the switch's forwarding path, and computing the decision // inline (rather than on a background worker) is what a future enforcement step -// will require. Nothing is persisted, so reputation is re-accrued from live -// traffic after a restart. +// will require. +// +// Channel state is loaded from the store on Start and channels whose state +// changed are written back periodically and on Stop. Pending HTLCs are not +// persisted: they are replayed from the switch's in-flight circuits. type Manager struct { cfg Config clock clock.Clock + store Store - // mu guards channels. It is held for the duration of each hook. + // mu guards channels, htlcIndex and dirty. It is held for the duration + // of each hook. mu sync.Mutex + // storeMu serialises the operations that move state between memory + // and the store (load, flush and channel removal) so that none of + // them can interleave with another and, for example, a flush write a + // snapshot taken before a removal back after it. The hooks never take + // it. When both are held, storeMu is taken before mu. + storeMu sync.Mutex + + // dirty holds the channel_ids of channels whose averages changed since + // they were last written to the store. + dirty map[lnwire.ChannelID]struct{} + // channels holds per-channel reputation state, keyed by the channel's // channel_id and created lazily on the first HTLC event for a channel. // The channel_id is used rather than the short channel id because it @@ -54,10 +79,11 @@ type Manager struct { stopOnce sync.Once } -// NewManager constructs a reputation Manager with the given config and clock. -// The clock is mandatory (production passes clock.NewDefaultClock; tests pass a -// test clock). -func NewManager(cfg Config, clk clock.Clock) (*Manager, error) { +// NewManager constructs a reputation Manager with the given config, clock and +// store. The clock is mandatory (production passes clock.NewDefaultClock; tests +// pass a test clock). A nil store means nothing is persisted and reputation is +// re-accrued from live traffic after a restart. +func NewManager(cfg Config, clk clock.Clock, store Store) (*Manager, error) { if err := cfg.Validate(); err != nil { return nil, fmt.Errorf("invalid reputation config: %w", err) } @@ -66,18 +92,25 @@ func NewManager(cfg Config, clk clock.Clock) (*Manager, error) { return nil, fmt.Errorf("reputation manager requires a clock") } + if store == nil { + store = NewNoopStore() + } + return &Manager{ cfg: cfg, clock: clk, + store: store, channels: make(map[lnwire.ChannelID]*channelReputation), htlcIndex: make(map[models.CircuitKey]lnwire.ChannelID), + dirty: make(map[lnwire.ChannelID]struct{}), quit: make(chan struct{}), }, nil } -// Start launches the periodic stale-pending check. Per-channel state is created -// lazily on the first HTLC event, so there is nothing to load. +// Start loads the persisted channel state and launches the periodic +// stale-pending check and store flush. It fails if the store cannot be read. func (m *Manager) Start() error { + var startErr error m.startOnce.Do(func() { log.Infof("Reputation manager starting (log-only): "+ "resolution_period=%v revenue_window=%v "+ @@ -85,23 +118,37 @@ func (m *Manager) Start() error { m.cfg.ResolutionPeriod, m.cfg.RevenueWindow, m.cfg.ReputationMultiplier, m.cfg.RevenueWindowCount) + if err := m.load(); err != nil { + startErr = fmt.Errorf("unable to load reputation "+ + "state: %w", err) + + return + } + m.wg.Add(1) - go m.staleCheckLoop() + go m.run() }) - return nil + return startErr } -// Stop tears down the subsystem. +// Stop tears down the subsystem, writing any unsaved channel state to the +// store first. func (m *Manager) Stop() error { + var stopErr error m.stopOnce.Do(func() { close(m.quit) m.wg.Wait() + if err := m.flush(); err != nil { + stopErr = fmt.Errorf("unable to flush reputation "+ + "state: %w", err) + } + log.Infof("Reputation manager stopped") }) - return nil + return stopErr } // OnForward observes a forwarded HTLC at the point the switch commits to @@ -304,6 +351,7 @@ func (m *Manager) resolveHTLC(incoming models.CircuitKey, settled bool, if settled { inChan := m.getOrCreateChannel(pending.incomingChan, at) inChan.incomingRevenue.add(satFromUint(pending.fee).Int64(), at) + m.dirty[pending.incomingChan] = struct{}{} } effFee := m.cfg.effectiveFee( @@ -311,6 +359,7 @@ func (m *Manager) resolveHTLC(incoming models.CircuitKey, settled bool, ) newRep := outChan.outgoingReputation.add(effFee, at) + m.dirty[outgoingChan] = struct{}{} // Log a single greppable line per resolution reporting the reputation // change, so it can be tracked without matching several phrasings. The @@ -322,23 +371,34 @@ func (m *Manager) resolveHTLC(incoming models.CircuitKey, settled bool, return nil } -// staleCheckLoop runs the periodic stale-pending check until the manager is -// stopped. -func (m *Manager) staleCheckLoop() { +// run drives the periodic stale-pending check and store flush until the +// manager is stopped. +func (m *Manager) run() { defer m.wg.Done() - ticker := time.NewTicker(staleCheckInterval) - defer ticker.Stop() + staleTicker := time.NewTicker(staleCheckInterval) + defer staleTicker.Stop() + + flushTicker := time.NewTicker(flushInterval) + defer flushTicker.Stop() for { select { case <-m.quit: return - case <-ticker.C: + case <-staleTicker.C: guarded("stale check", func() { m.reportStalePendings() }) + + case <-flushTicker.C: + guarded("flush", func() { + if err := m.flush(); err != nil { + log.Errorf("Reputation flush failed: "+ + "%v", err) + } + }) } } } diff --git a/reputation/manager_test.go b/reputation/manager_test.go index 4ee6007a10..7b06a642cd 100644 --- a/reputation/manager_test.go +++ b/reputation/manager_test.go @@ -20,7 +20,7 @@ func buildManager(t *testing.T, start int64) (*Manager, *clock.TestClock) { clk := clock.NewTestClock(time.Unix(start, 0)) - m, err := NewManager(DefaultConfig(), clk) + m, err := NewManager(DefaultConfig(), clk, nil) require.NoError(t, err, "NewManager") require.NoError(t, m.Start(), "Start") t.Cleanup(func() { _ = m.Stop() }) @@ -33,7 +33,7 @@ func TestManagerStartStop(t *testing.T) { t.Parallel() m, err := NewManager( - DefaultConfig(), clock.NewTestClock(time.Unix(1000, 0)), + DefaultConfig(), clock.NewTestClock(time.Unix(1000, 0)), nil, ) require.NoError(t, err) require.NoError(t, m.Start()) @@ -51,7 +51,7 @@ func TestManagerStartStop(t *testing.T) { func TestManagerRequiresClock(t *testing.T) { t.Parallel() - _, err := NewManager(DefaultConfig(), nil) + _, err := NewManager(DefaultConfig(), nil, nil) require.Error(t, err) } diff --git a/reputation/persist.go b/reputation/persist.go new file mode 100644 index 0000000000..092491065e --- /dev/null +++ b/reputation/persist.go @@ -0,0 +1,174 @@ +package reputation + +import ( + "context" + "time" + + "github.com/lightningnetwork/lnd/lnwire" +) + +// load restores the persisted channel state into the manager. Averages are +// restored with their timestamps verbatim so the first read decays them over +// the node's downtime. A timestamp in the future means the clock went +// backwards across the restart; it is clamped to now, so the value is kept +// without decay rather than every later read failing as backwards time. +// +// Rows whose averages have decayed to zero are restored too: the revenue start +// time still drives the warm-up factor, and the row is removed when the +// channel closes. +func (m *Manager) load() error { + m.storeMu.Lock() + defer m.storeMu.Unlock() + + ctx, cancel := context.WithTimeout(context.Background(), storeTimeout) + defer cancel() + + states, err := m.store.FetchChannels(ctx) + if err != nil { + return err + } + + now := m.clock.Now() + + var clamped int + + m.mu.Lock() + for _, state := range states { + if clampFutureTimestamps(&state, now) { + clamped++ + } + + m.channels[state.ChanID] = restoreChannelReputation( + m.cfg, state, + ) + } + m.mu.Unlock() + + if clamped > 0 { + log.Warnf("Reputation clamped %d channel timestamps that were "+ + "in the future (clock went backwards?)", clamped) + } + + // The phrasing is stable: integration tests match on it. + log.Infof("Reputation loaded %d channels from store", len(states)) + + return nil +} + +// clampFutureTimestamps clamps any timestamp of the state that lies after now +// to now, reporting whether it had to. +func clampFutureTimestamps(state *ChannelState, now time.Time) bool { + var clamped bool + + clamp := func(ts *time.Time) { + if ts.After(now) { + *ts = now + clamped = true + } + } + + clamp(&state.OutgoingReputationUpdatedAt) + clamp(&state.IncomingRevenueUpdatedAt) + clamp(&state.IncomingRevenueStartedAt) + + return clamped +} + +// flush writes every channel whose state changed since the last flush to the +// store. If the write fails the channels stay marked so the next flush retries +// them. +func (m *Manager) flush() error { + m.storeMu.Lock() + defer m.storeMu.Unlock() + + m.mu.Lock() + if len(m.dirty) == 0 { + m.mu.Unlock() + + return nil + } + + states := make([]ChannelState, 0, len(m.dirty)) + for chanID := range m.dirty { + c, ok := m.channels[chanID] + if !ok { + // Removed since it was marked; nothing to write. + delete(m.dirty, chanID) + + continue + } + + states = append(states, c.state(chanID)) + } + m.dirty = make(map[lnwire.ChannelID]struct{}) + m.mu.Unlock() + + ctx, cancel := context.WithTimeout(context.Background(), storeTimeout) + defer cancel() + + if err := m.store.UpsertChannels(ctx, states); err != nil { + m.mu.Lock() + for _, state := range states { + m.dirty[state.ChanID] = struct{}{} + } + m.mu.Unlock() + + return err + } + + log.Debugf("Reputation flushed %d channels to store", len(states)) + + return nil +} + +// RemoveChannel drops all state held for a channel, in memory and in the store. +// It is called when the channel closes. Any HTLC still pending with the channel +// as its outgoing link is forgotten with it, since the channel has no +// reputation left to score it against, and so is any HTLC that arrived on the +// channel, since there is no channel left to credit its fee to. Removing a +// channel that is not tracked only clears any stale row in the store. +func (m *Manager) RemoveChannel(chanID lnwire.ChannelID) error { + m.storeMu.Lock() + defer m.storeMu.Unlock() + + m.mu.Lock() + c, tracked := m.channels[chanID] + if tracked { + for ref := range c.pendingHTLCs { + delete(m.htlcIndex, ref) + } + } + for outChanID, outChan := range m.channels { + if outChanID == chanID { + continue + } + for ref, p := range outChan.pendingHTLCs { + if p.incomingChan != chanID { + continue + } + + delete(outChan.pendingHTLCs, ref) + delete(m.htlcIndex, ref) + } + } + delete(m.channels, chanID) + delete(m.dirty, chanID) + m.mu.Unlock() + + ctx, cancel := context.WithTimeout(context.Background(), storeTimeout) + defer cancel() + + if err := m.store.DeleteChannel(ctx, chanID); err != nil { + return err + } + + if tracked { + // The phrasing is stable: integration tests match on it. + log.Infof("Reputation removed channel %v", chanID) + } else { + log.Debugf("Reputation removal of untracked channel %v", + chanID) + } + + return nil +} diff --git a/reputation/persist_bench_test.go b/reputation/persist_bench_test.go new file mode 100644 index 0000000000..db1ba48c68 --- /dev/null +++ b/reputation/persist_bench_test.go @@ -0,0 +1,146 @@ +package reputation + +import ( + "fmt" + "math/rand" + "testing" + "time" + + "github.com/lightningnetwork/lnd/clock" +) + +// benchChannelCounts are the channel counts the persistence benchmarks run +// at: a small node, a mid-sized routing node and a large one. +var benchChannelCounts = []int{10, 100, 1000} + +// forwardsPerFlush is how many forwards the amortised benchmark drives between +// two flushes, standing in for a minute of traffic on a busy node. +const forwardsPerFlush = 10_000 + +// newBenchSQLManager builds a manager on a fresh SQL store with numChannels +// channels that each forwarded and settled one HTLC, so all of them hold state +// and are dirty. +func newBenchSQLManager(b *testing.B, numChannels int) (*Manager, Store) { + b.Helper() + + store, _ := newTestSQLStore(b) + m, err := NewManager( + DefaultConfig(), clock.NewTestClock(time.Unix(1_000_000, 0)), + store, + ) + if err != nil { + b.Fatalf("NewManager: %v", err) + } + + for i := range numChannels { + in := circuit(uint64(i), 0) + m.OnForward( + in, chanID(uint64(i)), chanID(uint64(i+1)), 2000, 1000, + 1000, 200, 100, false, + ) + m.OnSettle(in) + } + + return m, store +} + +// BenchmarkFlush measures one flush of the manager with every channel dirty: +// a single SQL transaction upserting one row per channel. This is the +// periodic write the persistence adds, and it runs off the forwarding path. +func BenchmarkFlush(b *testing.B) { + for _, numChannels := range benchChannelCounts { + name := fmt.Sprintf("channels=%d", numChannels) + b.Run(name, func(b *testing.B) { + m, _ := newBenchSQLManager(b, numChannels) + + b.ReportAllocs() + b.ResetTimer() + for range b.N { + b.StopTimer() + m.mu.Lock() + for id := range m.channels { + m.dirty[id] = struct{}{} + } + m.mu.Unlock() + b.StartTimer() + + if err := m.flush(); err != nil { + b.Fatalf("flush: %v", err) + } + } + }) + } +} + +// BenchmarkLoad measures restoring every channel from the store on startup. +func BenchmarkLoad(b *testing.B) { + for _, numChannels := range benchChannelCounts { + name := fmt.Sprintf("channels=%d", numChannels) + b.Run(name, func(b *testing.B) { + seed, store := newBenchSQLManager(b, numChannels) + if err := seed.flush(); err != nil { + b.Fatalf("flush: %v", err) + } + + b.ReportAllocs() + b.ResetTimer() + for range b.N { + clk := clock.NewTestClock(time.Unix(2e6, 0)) + m, err := NewManager( + DefaultConfig(), clk, store, + ) + if err != nil { + b.Fatalf("NewManager: %v", err) + } + if err := m.load(); err != nil { + b.Fatalf("load: %v", err) + } + if len(m.channels) != numChannels+1 { + b.Fatalf("loaded %d channels, want %d", + len(m.channels), numChannels+1) + } + } + }) + } +} + +// BenchmarkForwardResolvePersisted measures the per-forward cost with the SQL +// store attached and a flush every forwardsPerFlush forwards, so the reported +// time is the hot-path cost plus the write cost amortised over the forwards +// that caused it. Forwards are spread over random channel pairs so each flush +// writes the whole channel set. +func BenchmarkForwardResolvePersisted(b *testing.B) { + for _, numChannels := range benchChannelCounts { + name := fmt.Sprintf("channels=%d", numChannels) + b.Run(name, func(b *testing.B) { + m, _ := newBenchSQLManager(b, numChannels) + if err := m.flush(); err != nil { + b.Fatalf("flush: %v", err) + } + + rng := rand.New(rand.NewSource(1)) + + b.ReportAllocs() + b.ResetTimer() + for i := range b.N { + inIdx := uint64(rng.Intn(numChannels)) + outIdx := (inIdx + 1 + + uint64(rng.Intn(numChannels-1))) % + uint64(numChannels) + + in := circuit(inIdx, uint64(i)) + m.OnForward( + in, chanID(inIdx), chanID(outIdx), 2000, + 1000, 1000, 200, 100, false, + ) + m.OnSettle(in) + + if (i+1)%forwardsPerFlush == 0 { + if err := m.flush(); err != nil { + b.Fatalf("flush: %v", err) + } + } + } + }) + } +} diff --git a/reputation/persist_test.go b/reputation/persist_test.go new file mode 100644 index 0000000000..0d572f74ce --- /dev/null +++ b/reputation/persist_test.go @@ -0,0 +1,535 @@ +package reputation + +import ( + "context" + "database/sql" + "sync" + "testing" + "time" + + "github.com/lightningnetwork/lnd/clock" + "github.com/lightningnetwork/lnd/lnwire" + "github.com/lightningnetwork/lnd/sqldb" + "github.com/stretchr/testify/require" +) + +// memStore is an in-memory Store with injectable failures, used to drive the +// manager's persistence paths without a database. +type memStore struct { + mu sync.Mutex + channels map[lnwire.ChannelID]ChannelState + + upserts int + deletes int + + failFetch error + failUpsert error + failDelete error + + // upsertStarted and upsertRelease, when set, make UpsertChannels + // signal that it has started and then wait to be released, so a test + // can interleave other calls with an in-progress write. + upsertStarted chan struct{} + upsertRelease chan struct{} +} + +func newMemStore() *memStore { + return &memStore{channels: make(map[lnwire.ChannelID]ChannelState)} +} + +func (s *memStore) FetchChannels(context.Context) ([]ChannelState, error) { + s.mu.Lock() + defer s.mu.Unlock() + + if s.failFetch != nil { + return nil, s.failFetch + } + + states := make([]ChannelState, 0, len(s.channels)) + for _, c := range s.channels { + states = append(states, c) + } + + return states, nil +} + +func (s *memStore) UpsertChannels(_ context.Context, + channels []ChannelState) error { + + if s.upsertStarted != nil { + s.upsertStarted <- struct{}{} + <-s.upsertRelease + } + + s.mu.Lock() + defer s.mu.Unlock() + + if s.failUpsert != nil { + return s.failUpsert + } + + s.upserts++ + for _, c := range channels { + s.channels[c.ChanID] = c + } + + return nil +} + +func (s *memStore) DeleteChannel(_ context.Context, + chanID lnwire.ChannelID) error { + + s.mu.Lock() + defer s.mu.Unlock() + + if s.failDelete != nil { + return s.failDelete + } + + s.deletes++ + delete(s.channels, chanID) + + return nil +} + +func (s *memStore) get(t *testing.T, chanID lnwire.ChannelID) ChannelState { + t.Helper() + + s.mu.Lock() + defer s.mu.Unlock() + + c, ok := s.channels[chanID] + require.True(t, ok, "channel %v not in store", chanID) + + return c +} + +func (s *memStore) has(chanID lnwire.ChannelID) bool { + s.mu.Lock() + defer s.mu.Unlock() + + _, ok := s.channels[chanID] + + return ok +} + +// shortWindowConfig returns a config with windows short enough for the decay +// to be visible within a few minutes of test clock time: a 100s reputation +// window and a 600s (6 x 100s) revenue window. +func shortWindowConfig() Config { + return Config{ + ResolutionPeriod: 90 * time.Second, + RevenueWindow: 100 * time.Second, + ReputationMultiplier: 1, + RevenueWindowCount: 6, + } +} + +// startManager builds and starts a manager on the given clock and store. +func startManager(t *testing.T, cfg Config, clk clock.Clock, + store Store) *Manager { + + t.Helper() + + m, err := NewManager(cfg, clk, store) + require.NoError(t, err, "NewManager") + require.NoError(t, m.Start(), "Start") + t.Cleanup(func() { _ = m.Stop() }) + + return m +} + +// settleForward drives one forward and settle of a 1000 msat fee HTLC from +// channel 1 to channel 2 through the manager, resolving hold seconds later. +func settleForward(t *testing.T, m *Manager, clk *clock.TestClock, + htlcID uint64, hold time.Duration) { + + t.Helper() + + in := circuit(1, htlcID) + m.OnForward( + in, chanID(1), chanID(2), 2000, 1000, 1000, 200, testHeight, + false, + ) + require.Contains(t, m.htlcIndex, in, "forward not recorded") + + advance(clk, hold) + m.OnSettle(in) + require.NotContains(t, m.htlcIndex, in, "settle not applied") +} + +// averages reads both averages of the manager's channels 1 and 2 at now. +func averages(t *testing.T, m *Manager, now time.Time) (int64, int64) { + t.Helper() + + rep := m.channels[chanID(2)].outgoingReputation.valueAt(now) + + rev := m.channels[chanID(1)].incomingRevenue.valueAt(now) + + return rep, rev +} + +// testRestartIsTransparent checks that a manager restarted from the store after +// a period of downtime reads exactly the same averages as one that never +// stopped: the persisted timestamps are restored verbatim, so the downtime +// decays the values instead of being skipped, and the warm-up factor keeps +// advancing from the original start. +func testRestartIsTransparent(t *testing.T, store Store) { + t.Helper() + + cfg := shortWindowConfig() + start := time.Unix(1_000_000, 0) + + // The reference manager keeps running across the whole test. + refClk := clock.NewTestClock(start) + ref := startManager(t, cfg, refClk, nil) + settleForward(t, ref, refClk, 0, 30*time.Second) + + // The persisted manager does the same forward, then stops, which + // flushes its state to the store. + clk := clock.NewTestClock(start) + m1 := startManager(t, cfg, clk, store) + settleForward(t, m1, clk, 0, 30*time.Second) + require.NoError(t, m1.Stop(), "Stop") + + // Both channels were written: 2 earned reputation, 1 earned revenue. + states, err := store.FetchChannels(t.Context()) + require.NoError(t, err) + require.Len(t, states, 2) + + // A full reputation window of downtime, then a restart from the store. + const downtime = 100 * time.Second + advance(clk, downtime) + advance(refClk, downtime) + + m2 := startManager(t, cfg, clk, store) + require.Len(t, m2.channels, 2, "channels not restored") + + // The timestamps must be the persisted ones, not the load time. At + // 1000 * e^(-1) = 368 the reputation is visibly decayed. + settledAt := start.Add(30 * time.Second) + restoredRep := m2.channels[chanID(2)].outgoingReputation + require.WithinDuration(t, settledAt, restoredRep.lastUpdated, 0, + "reputation timestamp re-stamped on load") + restoredRev := m2.channels[chanID(1)].incomingRevenue + require.WithinDuration(t, start, restoredRev.start, 0, + "revenue start re-stamped on load") + + wantRep, wantRev := averages(t, ref, refClk.Now()) + gotRep, gotRev := averages(t, m2, clk.Now()) + require.EqualValues(t, 368, gotRep, "reputation after downtime") + require.Equal(t, wantRep, gotRep, "reputation differs from reference") + require.Equal(t, wantRev, gotRev, "revenue differs from reference") + + // Forwarding continues on top of the restored state, in step with the + // reference: 368 decayed further, plus the new fee. + settleForward(t, ref, refClk, 1, 30*time.Second) + settleForward(t, m2, clk, 1, 30*time.Second) + + wantRep, wantRev = averages(t, ref, refClk.Now()) + gotRep, gotRev = averages(t, m2, clk.Now()) + require.Equal(t, wantRep, gotRep, "reputation after restart") + require.Equal(t, wantRev, gotRev, "revenue after restart") + require.Greater(t, gotRep, int64(1000), "new fee not added on top") +} + +// TestRestartIsTransparent runs the restart check against the in-memory store. +func TestRestartIsTransparent(t *testing.T) { + t.Parallel() + + testRestartIsTransparent(t, newMemStore()) +} + +// TestRestartIsTransparentSQL runs the restart check against the SQL store, so +// the timestamp round trip through the database is covered too. +func TestRestartIsTransparentSQL(t *testing.T) { + t.Parallel() + + db := newTestDB(t) + executor := sqldb.NewTransactionExecutor( + db, func(tx *sql.Tx) SQLQueries { + return db.WithTx(tx) + }, + ) + + testRestartIsTransparent(t, NewSQLStore(executor)) +} + +// TestLoadClampsFutureTimestamps checks that persisted timestamps lying in the +// future, which happens when the clock went backwards across a restart, are +// clamped to now on load so the state stays usable. +func TestLoadClampsFutureTimestamps(t *testing.T) { + t.Parallel() + + now := time.Unix(1_000_000, 0) + future := now.Add(time.Hour) + + store := newMemStore() + store.channels[chanID(2)] = ChannelState{ + ChanID: chanID(2), + OutgoingReputation: 5000, + OutgoingReputationUpdatedAt: future, + IncomingRevenue: 700, + IncomingRevenueUpdatedAt: future, + IncomingRevenueStartedAt: future, + } + + m := startManager( + t, shortWindowConfig(), clock.NewTestClock(now), store, + ) + c, ok := m.channels[chanID(2)] + require.True(t, ok, "channel not loaded") + + // All three timestamps were clamped, so no decay is applied and reads + // at now succeed instead of failing as backwards time. + require.Equal(t, now, c.outgoingReputation.lastUpdated) + require.Equal(t, now, c.incomingRevenue.inner.lastUpdated) + require.Equal(t, now, c.incomingRevenue.start) + + rep := c.outgoingReputation.valueAt(now) + require.EqualValues(t, 5000, rep) + + rev := c.incomingRevenue.valueAt(now) + require.EqualValues(t, 700, rev) +} + +// TestLoadKeepsDecayedChannels checks that a channel whose reputation and +// revenue have both decayed to zero is still restored, with its revenue start +// time intact, since that start time keeps driving the warm-up factor. +func TestLoadKeepsDecayedChannels(t *testing.T) { + t.Parallel() + + now := time.Unix(1_000_000, 0) + cfg := shortWindowConfig() + + store := newMemStore() + + // Channel 2 last earned a tiny amount a hundred windows ago. + longAgo := now.Add(-100 * cfg.reputationWindow()) + store.channels[chanID(2)] = ChannelState{ + ChanID: chanID(2), + OutgoingReputation: 10, + OutgoingReputationUpdatedAt: longAgo, + IncomingRevenue: 10, + IncomingRevenueUpdatedAt: longAgo, + IncomingRevenueStartedAt: longAgo, + } + + m := startManager(t, cfg, clock.NewTestClock(now), store) + + c, ok := m.channels[chanID(2)] + require.True(t, ok, "decayed channel not loaded") + require.Zero(t, c.outgoingReputation.valueAt(now)) + require.Zero(t, c.incomingRevenue.valueAt(now)) + require.Equal(t, longAgo, c.incomingRevenue.start, "start re-stamped") + + require.True(t, store.has(chanID(2)), "row dropped from store") + require.Zero(t, store.deletes) +} + +// TestFlushDoesNotResurrectRemovedChannel checks that a flush whose snapshot +// was taken before a channel was removed cannot write that channel back to the +// store after the removal: the two are serialised, so the removal waits for +// the write and then deletes the row. +func TestFlushDoesNotResurrectRemovedChannel(t *testing.T) { + t.Parallel() + + store := newMemStore() + store.upsertStarted = make(chan struct{}) + store.upsertRelease = make(chan struct{}) + + clk := clock.NewTestClock(time.Unix(1_000_000, 0)) + m := startManager(t, shortWindowConfig(), clk, store) + settleForward(t, m, clk, 0, 30*time.Second) + + // Start a flush and hold it inside the store write, after it took its + // snapshot of channel 2. + flushDone := make(chan error, 1) + go func() { + flushDone <- m.flush() + }() + <-store.upsertStarted + + // Remove channel 2 while the write is in progress. + removeDone := make(chan error, 1) + go func() { + removeDone <- m.RemoveChannel(chanID(2)) + }() + + // The removal must not complete before the write does. + select { + case err := <-removeDone: + t.Fatalf("removal ran during flush: %v", err) + + case <-time.After(50 * time.Millisecond): + } + + store.upsertRelease <- struct{}{} + require.NoError(t, <-flushDone) + require.NoError(t, <-removeDone) + + require.False(t, store.has(chanID(2)), "removed channel resurrected") + require.True(t, store.has(chanID(1)), "unrelated channel lost") +} + +// TestStartFailsWhenStoreUnreadable checks that a store that cannot be read +// fails Start rather than silently starting from empty state. +func TestStartFailsWhenStoreUnreadable(t *testing.T) { + t.Parallel() + + store := newMemStore() + store.failFetch = errTest + + m, err := NewManager( + shortWindowConfig(), clock.NewTestClock(time.Unix(1, 0)), store, + ) + require.NoError(t, err) + require.ErrorIs(t, m.Start(), errTest) +} + +// TestFlushWritesOnlyChangedChannels checks that a flush writes exactly the +// channels whose averages changed since the previous flush. +func TestFlushWritesOnlyChangedChannels(t *testing.T) { + t.Parallel() + + store := newMemStore() + clk := clock.NewTestClock(time.Unix(1_000_000, 0)) + m := startManager(t, shortWindowConfig(), clk, store) + + // Nothing has changed yet: no write. + require.NoError(t, m.flush()) + require.Equal(t, 0, store.upserts) + + // A settle changes both the outgoing channel's reputation and the + // incoming channel's revenue. + settleForward(t, m, clk, 0, 30*time.Second) + require.Len(t, m.dirty, 2) + + require.NoError(t, m.flush()) + require.Equal(t, 1, store.upserts) + require.Empty(t, m.dirty, "dirty set not cleared") + require.EqualValues(t, 1000, store.get(t, chanID(2)).OutgoingReputation) + require.EqualValues(t, 1000, store.get(t, chanID(1)).IncomingRevenue) + require.Equal( + t, clk.Now(), + store.get(t, chanID(2)).OutgoingReputationUpdatedAt, + ) + + // Nothing changed since: no write. + require.NoError(t, m.flush()) + require.Equal(t, 1, store.upserts) + + // A failed HTLC only touches the outgoing channel. + in := circuit(1, 1) + m.OnForward( + in, chanID(1), chanID(2), 2000, 1000, 1000, 200, testHeight, + false, + ) + m.OnFail(in) + require.Len(t, m.dirty, 1) + require.Contains(t, m.dirty, chanID(2)) +} + +// TestFlushRetriesAfterStoreError checks that a failed write keeps the channels +// marked so the next flush retries them, and that Stop reports the failure. +func TestFlushRetriesAfterStoreError(t *testing.T) { + t.Parallel() + + store := newMemStore() + clk := clock.NewTestClock(time.Unix(1_000_000, 0)) + m := startManager(t, shortWindowConfig(), clk, store) + + settleForward(t, m, clk, 0, 30*time.Second) + + store.failUpsert = errTest + require.ErrorIs(t, m.flush(), errTest) + require.Len(t, m.dirty, 2, "channels dropped from dirty set on error") + require.False(t, store.has(chanID(2))) + + store.failUpsert = nil + require.NoError(t, m.flush()) + require.Empty(t, m.dirty) + require.True(t, store.has(chanID(2))) + require.True(t, store.has(chanID(1))) + + // Stop surfaces a failing final flush. + settleForward(t, m, clk, 1, 30*time.Second) + store.failUpsert = errTest + require.ErrorIs(t, m.Stop(), errTest) +} + +// TestRemoveChannel checks that removing a channel drops it from memory and the +// store, forgets the HTLCs pending on it, and leaves other channels alone. +func TestRemoveChannel(t *testing.T) { + t.Parallel() + + store := newMemStore() + clk := clock.NewTestClock(time.Unix(1_000_000, 0)) + m := startManager(t, shortWindowConfig(), clk, store) + + // Channel 2 has reputation in the store and an HTLC pending on it. + settleForward(t, m, clk, 0, 30*time.Second) + require.NoError(t, m.flush()) + + pending := circuit(1, 1) + m.OnForward( + pending, chanID(1), chanID(2), 2000, 1000, 1000, 200, + testHeight, true, + ) + require.Contains(t, m.htlcIndex, pending) + + // Channel 2 is also the incoming channel of an HTLC pending on + // channel 4, which must be forgotten too: settling it later would + // otherwise recreate channel 2 to credit its fee. + arrived := circuit(2, 0) + m.OnForward( + arrived, chanID(2), chanID(4), 2000, 1000, 1000, 200, + testHeight, true, + ) + + // Channel 3 is unrelated and must be untouched. + other := circuit(3, 0) + m.OnForward( + other, chanID(3), chanID(4), 2000, 1000, 1000, 200, testHeight, + true, + ) + + require.NoError(t, m.RemoveChannel(chanID(2))) + + require.NotContains(t, m.channels, chanID(2)) + require.NotContains(t, m.htlcIndex, pending, "pending not forgotten") + require.NotContains(t, m.htlcIndex, arrived, "arrived not forgotten") + require.NotContains(t, m.dirty, chanID(2)) + require.False(t, store.has(chanID(2)), "store row not deleted") + + require.Contains(t, m.channels, chanID(4)) + require.Len(t, m.channels[chanID(4)].pendingHTLCs, 1) + require.Contains(t, m.htlcIndex, other) + require.True(t, store.has(chanID(1))) + + // A late resolution for either forgotten HTLC is a harmless no-op. + m.OnSettle(pending) + m.OnSettle(arrived) + require.NotContains(t, m.channels, chanID(2), "channel recreated") + + // Removing a channel we know nothing about is fine, and a failing + // store delete is reported. + require.NoError(t, m.RemoveChannel(chanID(99))) + + store.failDelete = errTest + require.ErrorIs(t, m.RemoveChannel(chanID(4)), errTest) +} + +// TestNoopStoreManager checks the manager works unchanged without a store: a +// restart starts from empty state. +func TestNoopStoreManager(t *testing.T) { + t.Parallel() + + clk := clock.NewTestClock(time.Unix(1_000_000, 0)) + m1 := startManager(t, shortWindowConfig(), clk, nil) + settleForward(t, m1, clk, 0, 30*time.Second) + require.NoError(t, m1.Stop()) + + m2 := startManager(t, shortWindowConfig(), clk, nil) + require.Empty(t, m2.channels) +} diff --git a/reputation/replay.go b/reputation/replay.go new file mode 100644 index 0000000000..59f772102a --- /dev/null +++ b/reputation/replay.go @@ -0,0 +1,72 @@ +package reputation + +import ( + "github.com/lightningnetwork/lnd/graph/db/models" + "github.com/lightningnetwork/lnd/lnwire" +) + +// InFlightHTLC describes a forwarded HTLC that was still in flight when the +// node started, reconstructed from the switch's open circuits. +type InFlightHTLC struct { + // Incoming identifies the HTLC by its incoming circuit key. + Incoming models.CircuitKey + + // IncomingChan is the channel_id of the channel the HTLC arrived on. + IncomingChan lnwire.ChannelID + + // OutgoingChan is the channel_id of the channel the HTLC was forwarded + // on. + OutgoingChan lnwire.ChannelID + + // Fee is the fee this node charges for the forward. + Fee lnwire.MilliSatoshi + + // IncomingCltv is the cltv expiry of the incoming HTLC. + IncomingCltv uint32 + + // Accountable is the accountable signal the HTLC was forwarded with. + Accountable bool +} + +// ReplayInFlight rebuilds the pending state for HTLCs that were in flight +// across a restart, so that their in-flight risk is counted again and their +// eventual resolution is scored rather than ignored as unmatched. It returns +// the number of HTLCs replayed. +// +// The original forward time is not recoverable, so replayed HTLCs are stamped +// with the current time and height: the hold time charged when they resolve +// starts at the restart, and their remaining worst case hold is measured from +// the current height. HTLCs that cannot be tracked (expired or already known) +// are skipped with a warning. +func (m *Manager) ReplayInFlight(htlcs []InFlightHTLC, height uint32) int { + at := m.clock.Now() + + m.mu.Lock() + defer m.mu.Unlock() + + var replayed, accountable int + for _, h := range htlcs { + _, err := m.addHTLC( + h.Incoming, h.IncomingChan, h.OutgoingChan, h.Fee, + h.IncomingCltv, height, h.Accountable, at, + ) + if err != nil { + log.Warnf("Reputation could not replay in-flight htlc "+ + "%v on outgoing channel %v: %v", h.Incoming, + h.OutgoingChan, err) + + continue + } + + replayed++ + if h.Accountable { + accountable++ + } + } + + // The phrasing is stable: integration tests match on it. + log.Infof("Reputation replayed %d of %d in-flight HTLCs "+ + "(%d accountable)", replayed, len(htlcs), accountable) + + return replayed +} diff --git a/reputation/replay_test.go b/reputation/replay_test.go new file mode 100644 index 0000000000..b998983c7e --- /dev/null +++ b/reputation/replay_test.go @@ -0,0 +1,184 @@ +package reputation + +import ( + "testing" + "time" + + "github.com/lightningnetwork/lnd/clock" + "github.com/stretchr/testify/require" +) + +// TestReplayInFlight checks that replayed HTLCs are tracked exactly like live +// forwards: they are pending on their outgoing channel, indexed by circuit +// key, stamped with the replay time, and only accountable ones add risk. +func TestReplayInFlight(t *testing.T) { + t.Parallel() + + clk := clock.NewTestClock(time.Unix(1_000_000, 0)) + m := startManager(t, shortWindowConfig(), clk, nil) + + // Advance so the replay time differs from the manager start time. + advance(clk, time.Hour) + now := clk.Now() + + accountable := InFlightHTLC{ + Incoming: circuit(1, 0), IncomingChan: chanID(1), + OutgoingChan: chanID(2), Fee: 1000, + IncomingCltv: testHeight + 1, Accountable: true, + } + unaccountable := InFlightHTLC{ + Incoming: circuit(1, 1), IncomingChan: chanID(1), + OutgoingChan: chanID(2), Fee: 1000, + IncomingCltv: testHeight + 1, Accountable: false, + } + otherChan := InFlightHTLC{ + Incoming: circuit(3, 0), IncomingChan: chanID(3), + OutgoingChan: chanID(4), Fee: 500, + IncomingCltv: testHeight + 10, Accountable: true, + } + + n := m.ReplayInFlight( + []InFlightHTLC{accountable, unaccountable, otherChan}, + testHeight, + ) + require.Equal(t, 3, n) + + require.Len(t, m.channels[chanID(2)].pendingHTLCs, 2) + require.Len(t, m.channels[chanID(4)].pendingHTLCs, 1) + require.Len(t, m.htlcIndex, 3) + require.Equal(t, chanID(2), m.htlcIndex[accountable.Incoming]) + require.Equal(t, chanID(4), m.htlcIndex[otherChan.Incoming]) + + // Stamped with the replay time, and the hold measured from the replay + // height: cltv delta 1 gives a 600s worst case hold. + p := m.channels[chanID(2)].pendingHTLCs[accountable.Incoming] + require.Equal(t, now, p.addedAt) + require.Equal(t, 600*time.Second, p.maxHold) + require.True(t, p.accountable) + require.EqualValues(t, 1000, p.fee) + + // Only the accountable HTLC contributes risk: round((600-90)/90 * + // 1000) = 5667. + risk := m.channels[chanID(2)].inFlightRisk().Int64() + require.EqualValues(t, 5667, risk) + + // A replayed HTLC resolves like any other. Settling the accountable one + // 30s after the replay earns its full fee, since the hold is measured + // from the replay, not from some unknown original forward time. + advance(clk, 30*time.Second) + m.OnSettle(accountable.Incoming) + + require.NotContains(t, m.htlcIndex, accountable.Incoming) + require.Len(t, m.channels[chanID(2)].pendingHTLCs, 1) + + rep := m.channels[chanID(2)].outgoingReputation.valueAt(clk.Now()) + require.EqualValues(t, 1000, rep) + + rev := m.channels[chanID(1)].incomingRevenue.valueAt(clk.Now()) + require.EqualValues(t, 1000, rev) + + // A replayed HTLC that fails is scored too. The unaccountable one + // contributes nothing on failure and is cleared. + m.OnFail(unaccountable.Incoming) + require.NotContains(t, m.htlcIndex, unaccountable.Incoming) + require.Empty(t, m.channels[chanID(2)].pendingHTLCs) + + rep = m.channels[chanID(2)].outgoingReputation.valueAt(clk.Now()) + require.EqualValues(t, 1000, rep, "unaccountable fail changed rep") + + // The accountable one on channel 4 fails after being held past the + // resolution period since the replay, so it is charged the + // opportunity cost of the hold measured from the replay: 5 minutes + // held on a 90s period with a 500 msat fee is round((300-90)/90 * + // 500) = 1167 docked. + advance(clk, 270*time.Second) + m.OnFail(otherChan.Incoming) + require.Empty(t, m.htlcIndex) + + rep = m.channels[chanID(4)].outgoingReputation.valueAt(clk.Now()) + require.EqualValues(t, -1167, rep, "accountable fail not charged") +} + +// TestReplayInFlightSkipsUntrackable checks that HTLCs which cannot be tracked +// are skipped without disturbing the rest: an expired incoming cltv, and a +// circuit that is already pending. +func TestReplayInFlightSkipsUntrackable(t *testing.T) { + t.Parallel() + + clk := clock.NewTestClock(time.Unix(1_000_000, 0)) + m := startManager(t, shortWindowConfig(), clk, nil) + + // Already known from live traffic. + live := circuit(1, 0) + m.OnForward( + live, chanID(1), chanID(2), 2000, 1000, 1000, 200, testHeight, + true, + ) + + n := m.ReplayInFlight([]InFlightHTLC{ + { + // Duplicate of the live forward, on a different channel + // even: must not move the index. + Incoming: live, IncomingChan: chanID(1), + OutgoingChan: chanID(3), Fee: 1000, + IncomingCltv: 200, Accountable: true, + }, + { + // Expired: cltv not beyond the current height. + Incoming: circuit(1, 1), IncomingChan: chanID(1), + OutgoingChan: chanID(2), Fee: 1000, + IncomingCltv: testHeight, Accountable: true, + }, + { + // Fine. + Incoming: circuit(1, 2), IncomingChan: chanID(1), + OutgoingChan: chanID(2), Fee: 1000, + IncomingCltv: 200, Accountable: true, + }, + }, testHeight) + require.Equal(t, 1, n) + + require.Len(t, m.htlcIndex, 2) + require.Equal(t, chanID(2), m.htlcIndex[live], "index moved") + require.Contains(t, m.htlcIndex, circuit(1, 2)) + require.NotContains(t, m.htlcIndex, circuit(1, 1)) + require.Len(t, m.channels[chanID(2)].pendingHTLCs, 2) + + // Replaying nothing is fine. + require.Zero(t, m.ReplayInFlight(nil, testHeight)) +} + +// TestReplayOntoRestoredChannel checks that replayed HTLCs attach to channel +// state restored from the store rather than resetting it, so a restart keeps +// both the reputation and the in-flight risk. +func TestReplayOntoRestoredChannel(t *testing.T) { + t.Parallel() + + now := time.Unix(1_000_000, 0) + store := newMemStore() + store.channels[chanID(2)] = ChannelState{ + ChanID: chanID(2), + OutgoingReputation: 50_000, + OutgoingReputationUpdatedAt: now, + IncomingRevenueUpdatedAt: now, + IncomingRevenueStartedAt: now, + } + + m := startManager( + t, shortWindowConfig(), clock.NewTestClock(now), store, + ) + + n := m.ReplayInFlight([]InFlightHTLC{{ + Incoming: circuit(1, 0), IncomingChan: chanID(1), + OutgoingChan: chanID(2), Fee: 1000, + IncomingCltv: testHeight + 1, Accountable: true, + }}, testHeight) + require.Equal(t, 1, n) + + c := m.channels[chanID(2)] + require.Len(t, c.pendingHTLCs, 1) + + rep := c.outgoingReputation.valueAt(now) + require.EqualValues(t, 50_000, rep, "restored reputation lost") + require.EqualValues(t, 5667, c.inFlightRisk().Int64()) +} diff --git a/reputation/revenue.go b/reputation/revenue.go index e93b9cc941..92b764f412 100644 --- a/reputation/revenue.go +++ b/reputation/revenue.go @@ -35,6 +35,23 @@ func newAggregatedWindowAverage(window time.Duration, windowCount uint8, } } +// restoreAggregatedWindowAverage rebuilds an aggregated average from persisted +// state. The start time is restored too, so the warm-up factor keeps advancing +// from where it was rather than restarting from zero periods. +func restoreAggregatedWindowAverage(window time.Duration, windowCount uint8, + start time.Time, value int64, + lastUpdated time.Time) *aggregatedWindowAverage { + + return &aggregatedWindowAverage{ + start: start, + windowCount: windowCount, + windowDuration: window, + inner: restoreDecayingAverage( + value, lastUpdated, window*time.Duration(windowCount), + ), + } +} + // add records a value at the given time. func (a *aggregatedWindowAverage) add(value int64, ts time.Time) int64 { return a.inner.add(value, ts) diff --git a/reputation/sql_store.go b/reputation/sql_store.go new file mode 100644 index 0000000000..63199cf206 --- /dev/null +++ b/reputation/sql_store.go @@ -0,0 +1,167 @@ +package reputation + +import ( + "context" + "fmt" + + "github.com/lightningnetwork/lnd/lnwire" + "github.com/lightningnetwork/lnd/sqldb" + "github.com/lightningnetwork/lnd/sqldb/sqlc" +) + +// SQLQueries is the set of queries the SQL store runs against the reputation +// tables. +type SQLQueries interface { + UpsertReputationChannel(ctx context.Context, + arg sqlc.UpsertReputationChannelParams) error + + FetchReputationChannels(ctx context.Context) ([]sqlc.ReputationChannel, + error) + + DeleteReputationChannel(ctx context.Context, channelID []byte) (int64, + error) +} + +// BatchedSQLQueries combines the reputation queries with the ability to run +// them in a single transaction. +type BatchedSQLQueries interface { + SQLQueries + + sqldb.BatchedTx[SQLQueries] +} + +// SQLStore is the native SQL implementation of Store. +type SQLStore struct { + db BatchedSQLQueries +} + +// A compile time check that SQLStore implements Store. +var _ Store = (*SQLStore)(nil) + +// NewSQLStore creates a Store backed by the given SQL queries. +func NewSQLStore(db BatchedSQLQueries) *SQLStore { + return &SQLStore{db: db} +} + +// FetchChannels returns the persisted state of every channel. +func (s *SQLStore) FetchChannels(ctx context.Context) ([]ChannelState, error) { + var channels []ChannelState + + err := s.db.ExecTx(ctx, sqldb.ReadTxOpt(), func(db SQLQueries) error { + rows, err := db.FetchReputationChannels(ctx) + if err != nil { + return err + } + + channels = make([]ChannelState, 0, len(rows)) + for _, row := range rows { + state, err := channelStateFromRow(row) + if err != nil { + return err + } + + channels = append(channels, state) + } + + return nil + }, func() { + channels = nil + }) + if err != nil { + return nil, fmt.Errorf("unable to fetch reputation channels: "+ + "%w", err) + } + + return channels, nil +} + +// UpsertChannels writes the given channel states in a single transaction, +// replacing any existing state for the same channels. +func (s *SQLStore) UpsertChannels(ctx context.Context, + channels []ChannelState) error { + + if len(channels) == 0 { + return nil + } + + err := s.db.ExecTx(ctx, sqldb.WriteTxOpt(), func(db SQLQueries) error { + for _, c := range channels { + err := db.UpsertReputationChannel(ctx, upsertParams(c)) + if err != nil { + return fmt.Errorf("channel %v: %w", c.ChanID, + err) + } + } + + return nil + }, sqldb.NoOpReset) + if err != nil { + return fmt.Errorf("unable to upsert reputation channels: %w", + err) + } + + return nil +} + +// DeleteChannel removes the persisted state of a channel. Deleting a channel +// that has no persisted state is not an error. +func (s *SQLStore) DeleteChannel(ctx context.Context, + chanID lnwire.ChannelID) error { + + err := s.db.ExecTx(ctx, sqldb.WriteTxOpt(), func(db SQLQueries) error { + _, err := db.DeleteReputationChannel(ctx, chanID[:]) + + return err + }, sqldb.NoOpReset) + if err != nil { + return fmt.Errorf("unable to delete reputation channel %v: %w", + chanID, err) + } + + return nil +} + +// upsertParams converts a ChannelState into the parameters of the upsert +// query. Timestamps are stored in UTC. +func upsertParams(c ChannelState) sqlc.UpsertReputationChannelParams { + return sqlc.UpsertReputationChannelParams{ + ChannelID: c.ChanID[:], + OutgoingReputation: c.OutgoingReputation, + IncomingRevenue: c.IncomingRevenue, + IncomingRevenueUpdatedAt: c.IncomingRevenueUpdatedAt.UTC(), + IncomingRevenueStartedAt: c.IncomingRevenueStartedAt.UTC(), + OutgoingReputationUpdatedAt: c.OutgoingReputationUpdatedAt. + UTC(), + } +} + +// channelStateFromRow converts a database row into a ChannelState. +func channelStateFromRow(row sqlc.ReputationChannel) (ChannelState, error) { + chanID, err := decodeChannelID(row.ChannelID) + if err != nil { + return ChannelState{}, err + } + + return ChannelState{ + ChanID: chanID, + OutgoingReputation: row.OutgoingReputation, + IncomingRevenue: row.IncomingRevenue, + IncomingRevenueUpdatedAt: row.IncomingRevenueUpdatedAt.UTC(), + IncomingRevenueStartedAt: row.IncomingRevenueStartedAt.UTC(), + OutgoingReputationUpdatedAt: row.OutgoingReputationUpdatedAt. + UTC(), + }, nil +} + +// decodeChannelID decodes a stored channel_id, which must be exactly 32 bytes. +func decodeChannelID(b []byte) (lnwire.ChannelID, error) { + var chanID lnwire.ChannelID + if len(b) != len(chanID) { + return chanID, fmt.Errorf("invalid channel_id length %d, "+ + "want %d", len(b), len(chanID)) + } + + copy(chanID[:], b) + + return chanID, nil +} diff --git a/reputation/sql_store_postgres_test.go b/reputation/sql_store_postgres_test.go new file mode 100644 index 0000000000..385cacd683 --- /dev/null +++ b/reputation/sql_store_postgres_test.go @@ -0,0 +1,21 @@ +//go:build test_db_postgres + +package reputation + +import ( + "testing" + + "github.com/lightningnetwork/lnd/sqldb" +) + +// newTestDB creates a fresh Postgres database with all migrations applied. +func newTestDB(t testing.TB) *sqldb.BaseDB { + fixture := sqldb.NewTestPgFixture( + t, sqldb.DefaultPostgresFixtureLifetime, + ) + t.Cleanup(func() { + fixture.TearDown(t) + }) + + return sqldb.NewTestPostgresDB(t, fixture).BaseDB +} diff --git a/reputation/sql_store_sqlite_test.go b/reputation/sql_store_sqlite_test.go new file mode 100644 index 0000000000..08cb74981a --- /dev/null +++ b/reputation/sql_store_sqlite_test.go @@ -0,0 +1,14 @@ +//go:build !test_db_postgres + +package reputation + +import ( + "testing" + + "github.com/lightningnetwork/lnd/sqldb" +) + +// newTestDB creates a fresh SQLite database with all migrations applied. +func newTestDB(t testing.TB) *sqldb.BaseDB { + return sqldb.NewTestSqliteDB(t).BaseDB +} diff --git a/reputation/sql_store_test.go b/reputation/sql_store_test.go new file mode 100644 index 0000000000..b8094cd029 --- /dev/null +++ b/reputation/sql_store_test.go @@ -0,0 +1,226 @@ +package reputation + +import ( + "database/sql" + "testing" + "time" + + "github.com/lightningnetwork/lnd/sqldb" + "github.com/lightningnetwork/lnd/sqldb/sqlc" + "github.com/stretchr/testify/require" +) + +// newTestSQLStore creates an SQLStore on a fresh test database, returning the +// underlying queries as well so tests can bypass the store. +func newTestSQLStore(t testing.TB) (*SQLStore, BatchedSQLQueries) { + t.Helper() + + db := newTestDB(t) + executor := sqldb.NewTransactionExecutor( + db, func(tx *sql.Tx) SQLQueries { + return db.WithTx(tx) + }, + ) + + return NewSQLStore(executor), executor +} + +// testChannelState builds a channel state with distinct, second precision +// timestamps derived from n so round trips can be compared exactly. The +// channel_id is chanID(n), which orders rows by n. +func testChannelState(n uint64) ChannelState { + base := time.Unix(1_700_000_000+int64(n), 0).UTC() + + return ChannelState{ + ChanID: chanID(n), + OutgoingReputation: int64(n) * 1_000, + OutgoingReputationUpdatedAt: base, + IncomingRevenue: int64(n) * 10, + IncomingRevenueUpdatedAt: base.Add(time.Minute), + IncomingRevenueStartedAt: base.Add(-time.Hour), + } +} + +// TestSQLStoreEmpty checks that a fresh store holds no channels and that +// deleting an unknown channel is not an error. +func TestSQLStoreEmpty(t *testing.T) { + t.Parallel() + + store, _ := newTestSQLStore(t) + ctx := t.Context() + + channels, err := store.FetchChannels(ctx) + require.NoError(t, err) + require.Empty(t, channels) + + require.NoError(t, store.DeleteChannel(ctx, chanID(42))) + + // Upserting nothing is a no-op. + require.NoError(t, store.UpsertChannels(ctx, nil)) +} + +// TestSQLStoreRoundTrip checks that channel states survive a write and read +// unchanged, including negative reputation, and come back ordered by +// channel_id. +func TestSQLStoreRoundTrip(t *testing.T) { + t.Parallel() + + store, _ := newTestSQLStore(t) + ctx := t.Context() + + negative := testChannelState(7) + negative.OutgoingReputation = -12_345 + + want := []ChannelState{ + testChannelState(3), negative, testChannelState(1), + } + require.NoError(t, store.UpsertChannels(ctx, want)) + + got, err := store.FetchChannels(ctx) + require.NoError(t, err) + + // Rows come back ordered by channel_id. + require.Equal(t, []ChannelState{want[2], want[0], want[1]}, got) +} + +// TestSQLStoreTimestampPrecision checks how precisely timestamps survive the +// round trip. The manager stamps state with the wall clock, so nanosecond +// timestamps must come back within a microsecond, which is what the backends +// store, and never shifted by more than that. +func TestSQLStoreTimestampPrecision(t *testing.T) { + t.Parallel() + + store, _ := newTestSQLStore(t) + ctx := t.Context() + + want := testChannelState(1) + ts := time.Unix(1_700_000_000, 123_456_789).UTC() + want.OutgoingReputationUpdatedAt = ts + want.IncomingRevenueUpdatedAt = ts.Add(time.Nanosecond) + want.IncomingRevenueStartedAt = ts.Add(-time.Nanosecond) + require.NoError(t, store.UpsertChannels(ctx, []ChannelState{want})) + + got, err := store.FetchChannels(ctx) + require.NoError(t, err) + require.Len(t, got, 1) + + require.WithinDuration( + t, want.OutgoingReputationUpdatedAt, + got[0].OutgoingReputationUpdatedAt, time.Microsecond, + ) + require.WithinDuration( + t, want.IncomingRevenueUpdatedAt, + got[0].IncomingRevenueUpdatedAt, time.Microsecond, + ) + require.WithinDuration( + t, want.IncomingRevenueStartedAt, + got[0].IncomingRevenueStartedAt, time.Microsecond, + ) +} + +// TestSQLStoreUpsertReplaces checks that writing a channel again replaces its +// previous state rather than adding a second row. +func TestSQLStoreUpsertReplaces(t *testing.T) { + t.Parallel() + + store, _ := newTestSQLStore(t) + ctx := t.Context() + + first := testChannelState(5) + require.NoError(t, store.UpsertChannels(ctx, []ChannelState{first})) + + second := first + second.OutgoingReputation = 999 + second.OutgoingReputationUpdatedAt = first.OutgoingReputationUpdatedAt. + Add(time.Hour) + second.IncomingRevenue = 1 + require.NoError(t, store.UpsertChannels(ctx, []ChannelState{second})) + + got, err := store.FetchChannels(ctx) + require.NoError(t, err) + require.Equal(t, []ChannelState{second}, got) +} + +// TestSQLStoreDelete checks that deleting a channel removes only that channel. +func TestSQLStoreDelete(t *testing.T) { + t.Parallel() + + store, _ := newTestSQLStore(t) + ctx := t.Context() + + keep, drop := testChannelState(1), testChannelState(2) + require.NoError(t, store.UpsertChannels( + ctx, []ChannelState{keep, drop}, + )) + + require.NoError(t, store.DeleteChannel(ctx, drop.ChanID)) + + got, err := store.FetchChannels(ctx) + require.NoError(t, err) + require.Equal(t, []ChannelState{keep}, got) + + // Deleting it again is not an error. + require.NoError(t, store.DeleteChannel(ctx, drop.ChanID)) +} + +// TestSQLStoreAllBitsChannelID checks that a channel_id with every bit set +// survives the round trip unchanged. +func TestSQLStoreAllBitsChannelID(t *testing.T) { + t.Parallel() + + store, _ := newTestSQLStore(t) + ctx := t.Context() + + state := testChannelState(1) + for i := range state.ChanID { + state.ChanID[i] = 0xff + } + require.NoError(t, store.UpsertChannels(ctx, []ChannelState{state})) + + got, err := store.FetchChannels(ctx) + require.NoError(t, err) + require.Equal(t, []ChannelState{state}, got) +} + +// TestSQLStoreCorruptChannelID checks that a row whose channel_id is not 32 +// bytes is reported as an error instead of being decoded into a bogus channel. +func TestSQLStoreCorruptChannelID(t *testing.T) { + t.Parallel() + + store, db := newTestSQLStore(t) + ctx := t.Context() + + // Insert a malformed row directly, bypassing the store's encoding. + err := db.ExecTx(ctx, sqldb.WriteTxOpt(), func(q SQLQueries) error { + return q.UpsertReputationChannel( + ctx, sqlc.UpsertReputationChannelParams{ + ChannelID: []byte{1, 2, 3}, + OutgoingReputationUpdatedAt: time.Now().UTC(), + IncomingRevenueUpdatedAt: time.Now().UTC(), + IncomingRevenueStartedAt: time.Now().UTC(), + }, + ) + }, sqldb.NoOpReset) + require.NoError(t, err) + + _, err = store.FetchChannels(ctx) + require.ErrorContains(t, err, "invalid channel_id length 3") +} + +// TestNoopStore checks that the no-op store holds nothing and never errors. +func TestNoopStore(t *testing.T) { + t.Parallel() + + store := NewNoopStore() + ctx := t.Context() + + require.NoError(t, store.UpsertChannels( + ctx, []ChannelState{testChannelState(1)}, + )) + + channels, err := store.FetchChannels(ctx) + require.NoError(t, err) + require.Empty(t, channels) + + require.NoError(t, store.DeleteChannel(ctx, chanID(1))) +} diff --git a/reputation/store.go b/reputation/store.go new file mode 100644 index 0000000000..9f95c805a1 --- /dev/null +++ b/reputation/store.go @@ -0,0 +1,80 @@ +package reputation + +import ( + "context" + "time" + + "github.com/lightningnetwork/lnd/lnwire" +) + +// ChannelState is the persisted reputation state of a single channel. It holds +// the two decaying averages exactly as they are kept in memory, the running +// value and the time it was last updated at, so that decay over a node's +// downtime is applied lazily on the first read after a restart. +type ChannelState struct { + // ChanID is the channel_id of the channel, which is stable for the + // channel's lifetime (a short channel id is not: it changes when a + // zero-conf channel confirms or a channel is spliced). + ChanID lnwire.ChannelID + + // OutgoingReputation is the reputation the channel has accrued as an + // outgoing link, in millisatoshis. + OutgoingReputation int64 + + // OutgoingReputationUpdatedAt is the time the outgoing reputation + // average was last updated. + OutgoingReputationUpdatedAt time.Time + + // IncomingRevenue is the revenue the channel has earned as an incoming + // link, in millisatoshis, aggregated over several windows. + IncomingRevenue int64 + + // IncomingRevenueUpdatedAt is the time the incoming revenue average + // was last updated. + IncomingRevenueUpdatedAt time.Time + + // IncomingRevenueStartedAt is the time the incoming revenue average + // started tracking, which drives its warm-up factor. + IncomingRevenueStartedAt time.Time +} + +// Store persists channel reputation state across restarts. Pending HTLCs are +// deliberately not part of it: they are rebuilt from the switch's in-flight +// circuits on startup. +type Store interface { + // FetchChannels returns the persisted state of every channel. + FetchChannels(ctx context.Context) ([]ChannelState, error) + + // UpsertChannels writes the given channel states, replacing any + // existing state for the same channels. + UpsertChannels(ctx context.Context, channels []ChannelState) error + + // DeleteChannel removes the persisted state of a channel. Deleting a + // channel that has no persisted state is not an error. + DeleteChannel(ctx context.Context, chanID lnwire.ChannelID) error +} + +// noopStore is the Store used when no persistence backend is configured. It +// holds nothing, so reputation lives in memory only and is re-accrued from +// live traffic after a restart. +type noopStore struct{} + +// NewNoopStore returns a Store that persists nothing. +func NewNoopStore() Store { + return noopStore{} +} + +// FetchChannels returns no channels. +func (noopStore) FetchChannels(context.Context) ([]ChannelState, error) { + return nil, nil +} + +// UpsertChannels discards the given channels. +func (noopStore) UpsertChannels(context.Context, []ChannelState) error { + return nil +} + +// DeleteChannel does nothing. +func (noopStore) DeleteChannel(context.Context, lnwire.ChannelID) error { + return nil +} diff --git a/reputation/testutil_test.go b/reputation/testutil_test.go index 393d298832..2227607524 100644 --- a/reputation/testutil_test.go +++ b/reputation/testutil_test.go @@ -2,6 +2,7 @@ package reputation import ( "encoding/binary" + "errors" "time" "github.com/lightningnetwork/lnd/clock" @@ -29,3 +30,6 @@ func chanID(v uint64) lnwire.ChannelID { func advance(c *clock.TestClock, d time.Duration) { c.SetTime(c.Now().Add(d)) } + +// errTest is a sentinel error injected into test doubles. +var errTest = errors.New("test error") diff --git a/reputation_wiring.go b/reputation_wiring.go new file mode 100644 index 0000000000..003d538b5c --- /dev/null +++ b/reputation_wiring.go @@ -0,0 +1,387 @@ +package lnd + +import ( + "bytes" + "context" + "errors" + "fmt" + + "github.com/btcsuite/btcd/wire/v2" + "github.com/lightningnetwork/lnd/channelnotifier" + "github.com/lightningnetwork/lnd/chanstate" + graphdb "github.com/lightningnetwork/lnd/graph/db" + "github.com/lightningnetwork/lnd/graph/db/models" + "github.com/lightningnetwork/lnd/htlcswitch" + "github.com/lightningnetwork/lnd/htlcswitch/hop" + "github.com/lightningnetwork/lnd/lnwire" + "github.com/lightningnetwork/lnd/reputation" + "github.com/lightningnetwork/lnd/subscribe" +) + +// circuitSource exposes the switch's open circuits. It is implemented by the +// switch and kept as a seam so the reconstruction can be tested with a fake. +type circuitSource interface { + // ActiveCircuits returns a snapshot of the open circuits. + ActiveCircuits() []*htlcswitch.PaymentCircuit +} + +// openChannelSource exposes the node's open channels with their commitment +// HTLC sets. It is implemented by the channel state db and kept as a seam so +// the reconstruction can be tested with a fake. +type openChannelSource interface { + // FetchAllOpenChannels returns all currently open channels. + FetchAllOpenChannels() ([]*chanstate.OpenChannel, error) +} + +// policySource exposes the channel policies in the graph. It is implemented +// by the channel graph and kept as a seam so the reconstruction can be tested +// with a fake. +type policySource interface { + // FetchChannelEdgesByOutpoint returns the channel's edge info and + // both directional policies, or graphdb.ErrEdgeNotFound. + FetchChannelEdgesByOutpoint(ctx context.Context, + op *wire.OutPoint) (*models.ChannelEdgeInfo, + *models.ChannelEdgePolicy, *models.ChannelEdgePolicy, error) +} + +// fetchSelfPolicies returns this node's own forwarding policy for each of the +// given channels, keyed by funding outpoint. A channel without a policy in the +// graph is left out, the same way the link falls back to defaults for it. +func fetchSelfPolicies(ctx context.Context, graph policySource, + selfKey [33]byte, chans []*chanstate.OpenChannel) ( + map[wire.OutPoint]*models.ChannelEdgePolicy, error) { + + policies := make( + map[wire.OutPoint]*models.ChannelEdgePolicy, len(chans), + ) + for _, c := range chans { + op := c.FundingOutpoint + info, p1, p2, err := graph.FetchChannelEdgesByOutpoint(ctx, &op) + if errors.Is(err, graphdb.ErrEdgeNotFound) { + continue + } + if err != nil { + return nil, err + } + + // The policy whose edge connects to the other node is ours. + policy := p2 + if info != nil && + bytes.Equal(info.NodeKey1Bytes[:], selfKey[:]) { + + policy = p1 + } + if policy != nil { + policies[op] = policy + } + } + + return policies, nil +} + +// incomingHTLCInfo carries what the circuit map does not retain about an +// in-flight HTLC: the channel_id of the incoming channel, the incoming cltv +// expiry and the accountable signal, the latter two read back from the live +// incoming commitment HTLC. +type incomingHTLCInfo struct { + chanID lnwire.ChannelID + cltv uint32 + accountable bool +} + +// channelIndex joins the open channels to the circuit map. Circuits key both +// of their ends by the short channel id the link reports, which is the one the +// channel state holds (for a zero-conf channel that is its alias, before and +// after confirmation), while the reputation manager keys channels by their +// channel_id. +type channelIndex struct { + // incoming maps the incoming circuit key of every live incoming + // commitment HTLC to what the replay needs to know about it. An + // incoming HTLC is keyed by (short channel id, htlc index), which is + // exactly a circuit's incoming key. + incoming map[models.CircuitKey]incomingHTLCInfo + + // chanIDs maps each open channel's short channel id to its channel_id. + chanIDs map[lnwire.ShortChannelID]lnwire.ChannelID + + // nextHtlcIndex maps each open channel's short channel id to the + // index its next outgoing HTLC will get. Outgoing HTLCs at or above it + // never made it into a commitment. + nextHtlcIndex map[lnwire.ShortChannelID]uint64 + + // policies maps each open channel's short channel id to this node's + // forwarding policy on it, for the channels that have one. + policies map[lnwire.ShortChannelID]*models.ChannelEdgePolicy +} + +// channelInfo is what the reconstruction needs to know about an open channel +// beyond its channel state: this node's policy on it, if any, and the index of +// its next outgoing HTLC. Both are keyed by funding outpoint. +type channelInfo struct { + policies map[wire.OutPoint]*models.ChannelEdgePolicy + nextHtlcIndex map[wire.OutPoint]uint64 +} + +// indexChannels builds the channelIndex of the given open channels. +func indexChannels(chans []*chanstate.OpenChannel, + info channelInfo) channelIndex { + + index := channelIndex{ + incoming: make(map[models.CircuitKey]incomingHTLCInfo), + chanIDs: make( + map[lnwire.ShortChannelID]lnwire.ChannelID, len(chans), + ), + nextHtlcIndex: make( + map[lnwire.ShortChannelID]uint64, len(chans), + ), + policies: make( + map[lnwire.ShortChannelID]*models.ChannelEdgePolicy, + len(info.policies), + ), + } + + for _, c := range chans { + scid := c.ShortChanID() + chanID := lnwire.NewChanIDFromOutPoint(c.FundingOutpoint) + index.chanIDs[scid] = chanID + index.nextHtlcIndex[scid] = + info.nextHtlcIndex[c.FundingOutpoint] + + if policy, ok := info.policies[c.FundingOutpoint]; ok { + index.policies[scid] = policy + } + + for _, htlc := range c.ActiveHtlcs() { + // Circuits reference the incoming add; HTLCs we offered + // on this channel are the outgoing side of some other + // circuit. + if !htlc.Incoming { + continue + } + + key := models.CircuitKey{ + ChanID: scid, + HtlcID: htlc.HtlcIndex, + } + index.incoming[key] = incomingHTLCInfo{ + chanID: chanID, + cltv: htlc.RefundTimeout, + accountable: htlcswitch.AccountableFromRecords( + htlc.CustomRecords, + ), + } + } + } + + return index +} + +// requiredFee returns the fee this node requires to forward amtToForward from +// the incoming to the outgoing channel under its current policies, computed +// the same way the switch does for a live forward: the outgoing channel's +// outbound fee plus the incoming channel's inbound fee, clamped at zero. The +// second return value is false if either channel has no policy in the graph. +func (i channelIndex) requiredFee(incoming, outgoing lnwire.ShortChannelID, + amtToForward lnwire.MilliSatoshi) (lnwire.MilliSatoshi, bool) { + + inPolicy, ok := i.policies[incoming] + if !ok { + return 0, false + } + outPolicy, ok := i.policies[outgoing] + if !ok { + return 0, false + } + + outFee := htlcswitch.ExpectedFee(models.ForwardingPolicy{ + BaseFee: outPolicy.FeeBaseMSat, + FeeRate: outPolicy.FeeProportionalMillionths, + }, amtToForward) + + var inboundFee models.InboundFee + inPolicy.InboundFee.WhenSome(func(fee lnwire.Fee) { + inboundFee = models.NewInboundFeeFromWire(fee) + }) + + total := htlcswitch.TotalForwardingFee(amtToForward, outFee, inboundFee) + if total < 0 { + total = 0 + } + + return lnwire.MilliSatoshi(total), true +} + +// assembleInFlightHTLCs joins the open circuits with the live incoming +// commitment HTLCs into the in-flight HTLCs the reputation manager replays on +// startup. Only forwards are included: the incoming side must not be this +// node's own switch, and the circuit must have been opened on an outgoing +// link. A circuit whose incoming HTLC is no longer live on the commitment is +// skipped, since its cltv and accountable signal cannot be recovered, as is a +// circuit whose outgoing channel is no longer open. A circuit whose outgoing +// HTLC never made it into a commitment is skipped as well: the switch trims +// such circuits when the outgoing link starts and forwards the HTLC again, +// which reports it through the live hook. +// +// The circuit map does not retain the fee the node required for the forward, +// only the amounts. Like the live forward path, the replayed HTLC is scored on +// the fee the node's own policy requires rather than on the fee the sender +// offered, so that over-paying cannot inflate reputation. The policy may have +// changed since the forward was accepted, so the required fee is capped at the +// offered fee, which the node can never earn more than. If either channel has +// no policy in the graph, the offered fee is used. +func assembleInFlightHTLCs(circuits []*htlcswitch.PaymentCircuit, + chans []*chanstate.OpenChannel, + info channelInfo) []reputation.InFlightHTLC { + + index := indexChannels(chans, info) + + htlcs := make([]reputation.InFlightHTLC, 0, len(circuits)) + for _, c := range circuits { + if c.Incoming.ChanID == hop.Source || c.Outgoing == nil { + continue + } + + info, ok := index.incoming[c.Incoming] + if !ok { + srvrLog.Debugf("Skipping in-flight htlc %v for "+ + "reputation replay: incoming htlc no longer "+ + "live", c.Incoming) + + continue + } + + outgoingChan, ok := index.chanIDs[c.Outgoing.ChanID] + if !ok { + srvrLog.Debugf("Skipping in-flight htlc %v for "+ + "reputation replay: outgoing channel %v no "+ + "longer open", c.Incoming, c.Outgoing.ChanID) + + continue + } + + if c.Outgoing.HtlcID >= index.nextHtlcIndex[c.Outgoing.ChanID] { + srvrLog.Debugf("Skipping in-flight htlc %v for "+ + "reputation replay: outgoing htlc %v not "+ + "committed, will be forwarded again", + c.Incoming, c.Outgoing) + + continue + } + + var offered lnwire.MilliSatoshi + if c.IncomingAmount > c.OutgoingAmount { + offered = c.IncomingAmount - c.OutgoingAmount + } + + fee := offered + required, ok := index.requiredFee( + c.Incoming.ChanID, c.Outgoing.ChanID, c.OutgoingAmount, + ) + switch { + case !ok: + srvrLog.Debugf("No policy for in-flight htlc %v, "+ + "replaying with the offered fee %v", + c.Incoming, offered) + + case required < offered: + fee = required + } + + htlcs = append(htlcs, reputation.InFlightHTLC{ + Incoming: c.Incoming, + IncomingChan: info.chanID, + OutgoingChan: outgoingChan, + Fee: fee, + IncomingCltv: info.cltv, + Accountable: info.accountable, + }) + } + + return htlcs +} + +// nextHtlcIndexFunc returns the index an open channel's next outgoing HTLC +// will get. It is chanstate.OpenChannel.NextLocalHtlcIndex in production and +// kept as a seam since that reads the channel database. +type nextHtlcIndexFunc func(*chanstate.OpenChannel) (uint64, error) + +// reconstructInFlightHTLCs assembles the forwarded HTLCs that are in flight +// from the switch's open circuits, the live channel commitments and this +// node's policies on the channels. +func reconstructInFlightHTLCs(ctx context.Context, circuits circuitSource, + chans openChannelSource, graph policySource, selfKey [33]byte, + nextHtlcIndex nextHtlcIndexFunc) ([]reputation.InFlightHTLC, error) { + + openChans, err := chans.FetchAllOpenChannels() + if err != nil { + return nil, err + } + + policies, err := fetchSelfPolicies(ctx, graph, selfKey, openChans) + if err != nil { + return nil, err + } + + nextIndexes := make(map[wire.OutPoint]uint64, len(openChans)) + for _, c := range openChans { + next, err := nextHtlcIndex(c) + if err != nil { + return nil, fmt.Errorf("next htlc index of channel "+ + "%v: %w", c.FundingOutpoint, err) + } + + nextIndexes[c.FundingOutpoint] = next + } + + return assembleInFlightHTLCs( + circuits.ActiveCircuits(), openChans, channelInfo{ + policies: policies, + nextHtlcIndex: nextIndexes, + }, + ), nil +} + +// channelRemover removes a channel's state from the reputation manager. It is +// implemented by the manager and kept as a seam for tests. +type channelRemover interface { + RemoveChannel(chanID lnwire.ChannelID) error +} + +// removeClosedChannel drops the channel of a closed channel event from the +// reputation manager. Any other channel event is ignored, as is a closed +// channel event without a summary, which the notifier sends when it cannot +// load the close from the database. A failure to remove is logged: the +// manager is log-only and must not disturb the close. +func removeClosedChannel(mgr channelRemover, event any) { + closed, ok := event.(channelnotifier.ClosedChannelEvent) + if !ok || closed.CloseSummary == nil { + return + } + + chanID := lnwire.NewChanIDFromOutPoint(closed.CloseSummary.ChanPoint) + if err := mgr.RemoveChannel(chanID); err != nil { + srvrLog.Warnf("Unable to remove channel %v from reputation "+ + "manager: %v", chanID, err) + } +} + +// forwardChannelClosesToReputation removes closed channels from the reputation +// manager until the server shuts down. The subscription is taken by the +// caller, before the subsystems that emit close events start, so that none is +// missed. +func (s *server) forwardChannelClosesToReputation( + client *subscribe.Client) { + + defer s.wg.Done() + defer client.Cancel() + + for { + select { + case event := <-client.Updates(): + removeClosedChannel(s.reputationMgr, event) + + case <-s.quit: + return + } + } +} diff --git a/reputation_wiring_test.go b/reputation_wiring_test.go new file mode 100644 index 0000000000..353b226ba8 --- /dev/null +++ b/reputation_wiring_test.go @@ -0,0 +1,552 @@ +package lnd + +import ( + "context" + "errors" + "testing" + + "github.com/btcsuite/btcd/wire/v2" + "github.com/lightningnetwork/lnd/channelnotifier" + "github.com/lightningnetwork/lnd/chanstate" + "github.com/lightningnetwork/lnd/fn/v2" + graphdb "github.com/lightningnetwork/lnd/graph/db" + "github.com/lightningnetwork/lnd/graph/db/models" + "github.com/lightningnetwork/lnd/htlcswitch" + "github.com/lightningnetwork/lnd/htlcswitch/hop" + "github.com/lightningnetwork/lnd/lnwire" + "github.com/lightningnetwork/lnd/reputation" + "github.com/stretchr/testify/require" +) + +// fakeCircuitSource is a test double for the switch's open circuits. +type fakeCircuitSource struct { + circuits []*htlcswitch.PaymentCircuit +} + +func (f *fakeCircuitSource) ActiveCircuits() []*htlcswitch.PaymentCircuit { + return f.circuits +} + +// fakeChannelSource is a test double for the open channel source. +type fakeChannelSource struct { + chans []*chanstate.OpenChannel + err error +} + +func (f *fakeChannelSource) FetchAllOpenChannels() ([]*chanstate.OpenChannel, + error) { + + return f.chans, f.err +} + +// fakeEdge is one channel's graph entry as the fake policy source serves it. +type fakeEdge struct { + info *models.ChannelEdgeInfo + p1, p2 *models.ChannelEdgePolicy +} + +// fakePolicySource is a test double for the channel graph. +type fakePolicySource struct { + edges map[wire.OutPoint]fakeEdge + err error +} + +func (f *fakePolicySource) FetchChannelEdgesByOutpoint(_ context.Context, + op *wire.OutPoint) (*models.ChannelEdgeInfo, + *models.ChannelEdgePolicy, *models.ChannelEdgePolicy, error) { + + if f.err != nil { + return nil, nil, nil, f.err + } + + edge, ok := f.edges[*op] + if !ok { + return nil, nil, nil, graphdb.ErrEdgeNotFound + } + + return edge.info, edge.p1, edge.p2, nil +} + +// fakeRemover records the channels it was asked to remove. +type fakeRemover struct { + removed []lnwire.ChannelID + err error +} + +func (f *fakeRemover) RemoveChannel(chanID lnwire.ChannelID) error { + f.removed = append(f.removed, chanID) + + return f.err +} + +// policy builds a channel policy with the given outbound base fee and rate, +// and an inbound fee if any is given. +func policy(base, rate lnwire.MilliSatoshi, + inbound ...lnwire.Fee) *models.ChannelEdgePolicy { + + p := &models.ChannelEdgePolicy{ + FeeBaseMSat: base, + FeeProportionalMillionths: rate, + } + if len(inbound) > 0 { + p.InboundFee = fn.Some(inbound[0]) + } + + return p +} + +// makeHTLC builds a commitment HTLC with the given direction, index, expiry +// and accountable signal. +func makeHTLC(incoming bool, htlcIndex uint64, cltv uint32, + accountable bool) chanstate.HTLC { + + records := lnwire.CustomRecords{} + if accountable { + records[uint64(lnwire.ExperimentalAccountableType)] = + []byte{lnwire.ExperimentalAccountable} + } + + return chanstate.HTLC{ + Incoming: incoming, + HtlcIndex: htlcIndex, + RefundTimeout: cltv, + CustomRecords: records, + } +} + +// makeChannel builds an open channel with the given scid whose HTLCs are +// locked in on both commitments, so ActiveHtlcs returns them. Its funding +// outpoint, and so its channel_id, is derived from the scid. +func makeChannel(scid uint64, htlcs ...chanstate.HTLC) *chanstate.OpenChannel { + c := &chanstate.OpenChannel{ + ShortChannelID: lnwire.NewShortChanIDFromInt(scid), + FundingOutpoint: wire.OutPoint{Index: uint32(scid)}, + } + c.LocalCommitment.Htlcs = htlcs + c.RemoteCommitment.Htlcs = htlcs + + return c +} + +// chanIDOf returns the channel_id of the channel makeChannel builds for scid. +func chanIDOf(scid uint64) lnwire.ChannelID { + return lnwire.NewChanIDFromOutPoint(makeChannel(scid).FundingOutpoint) +} + +func inKey(scid, htlcID uint64) models.CircuitKey { + return models.CircuitKey{ + ChanID: lnwire.NewShortChanIDFromInt(scid), + HtlcID: htlcID, + } +} + +func outKey(scid, htlcID uint64) *models.CircuitKey { + k := inKey(scid, htlcID) + + return &k +} + +// nextIndexes builds the channelInfo nextHtlcIndex map for makeChannel +// channels, from scid to next outgoing htlc index pairs. +func nextIndexes(pairs ...uint64) map[wire.OutPoint]uint64 { + m := make(map[wire.OutPoint]uint64, len(pairs)/2) + for i := 0; i+1 < len(pairs); i += 2 { + m[makeChannel(pairs[i]).FundingOutpoint] = pairs[i+1] + } + + return m +} + +// fakeNextHtlcIndex is a nextHtlcIndexFunc serving the given map, failing for +// channels it does not know. +func fakeNextHtlcIndex(m map[wire.OutPoint]uint64) nextHtlcIndexFunc { + return func(c *chanstate.OpenChannel) (uint64, error) { + next, ok := m[c.FundingOutpoint] + if !ok { + return 0, errors.New("unknown channel") + } + + return next, nil + } +} + +// TestAssembleInFlightHTLCs checks that open circuits are joined with the live +// incoming commitment HTLCs to recover the cltv expiry and accountable signal, +// that both ends are mapped from the circuit's short channel ids to the +// channels' channel_ids, that the fee is the offered fee, and that everything +// which is not a live forward is filtered out. +func TestAssembleInFlightHTLCs(t *testing.T) { + t.Parallel() + + chans := []*chanstate.OpenChannel{ + // Channel 1 has two incoming HTLCs and one we offered, which + // must not be mistaken for an incoming one with the same index. + makeChannel( + 1, + makeHTLC(true, 0, 500, true), + makeHTLC(true, 1, 600, false), + makeHTLC(false, 0, 700, true), + ), + // Channel 2 is the outgoing channel of the forwards. + makeChannel(2, makeHTLC(false, 7, 400, true)), + // Channel 3 has an incoming HTLC only locked in on one + // commitment, so it is not active. + func() *chanstate.OpenChannel { + c := makeChannel(3) + c.LocalCommitment.Htlcs = []chanstate.HTLC{ + makeHTLC(true, 0, 800, true), + } + + return c + }(), + } + + circuits := []*htlcswitch.PaymentCircuit{ + // Accountable forward with a 100 msat fee. + { + Incoming: inKey(1, 0), Outgoing: outKey(2, 7), + IncomingAmount: 1100, OutgoingAmount: 1000, + }, + // Unaccountable forward whose sender underpaid (impossible for + // an accepted forward, but the fee must not go negative). + { + Incoming: inKey(1, 1), Outgoing: outKey(2, 8), + IncomingAmount: 900, OutgoingAmount: 1000, + }, + // Locally initiated payment: not a forward. + { + Incoming: models.CircuitKey{ + ChanID: hop.Source, HtlcID: 3, + }, + Outgoing: outKey(2, 9), + IncomingAmount: 1000, OutgoingAmount: 1000, + }, + // Committed but never opened on an outgoing link. + { + Incoming: inKey(1, 5), IncomingAmount: 1000, + OutgoingAmount: 1000, + }, + // Incoming HTLC not active on the commitment any more. + { + Incoming: inKey(3, 0), Outgoing: outKey(2, 10), + IncomingAmount: 1000, OutgoingAmount: 1000, + }, + // Incoming channel unknown altogether. + { + Incoming: inKey(9, 0), Outgoing: outKey(2, 11), + IncomingAmount: 1000, OutgoingAmount: 1000, + }, + // Outgoing channel not open any more. Its incoming HTLC is + // live, so only the outgoing lookup can reject it. + { + Incoming: inKey(1, 1), Outgoing: outKey(8, 0), + IncomingAmount: 1100, OutgoingAmount: 1000, + }, + // Outgoing HTLC at the channel's next index: it never made it + // into a commitment, so the switch will forward it again. + { + Incoming: inKey(1, 1), Outgoing: outKey(2, 12), + IncomingAmount: 1100, OutgoingAmount: 1000, + }, + } + + // Without any policies the offered fee is used. Channel 2's next + // outgoing htlc index is 12, so indexes up to 11 are committed. + got := assembleInFlightHTLCs(circuits, chans, channelInfo{ + nextHtlcIndex: nextIndexes(2, 12), + }) + require.Equal(t, []reputation.InFlightHTLC{ + { + Incoming: inKey(1, 0), + IncomingChan: chanIDOf(1), + OutgoingChan: chanIDOf(2), + Fee: 100, + IncomingCltv: 500, + Accountable: true, + }, + { + Incoming: inKey(1, 1), + IncomingChan: chanIDOf(1), + OutgoingChan: chanIDOf(2), + Fee: 0, + IncomingCltv: 600, + Accountable: false, + }, + }, got) + + // The channel_ids come from the funding outpoints, not the scids: two + // channels must not collide just because makeChannel derives one from + // the other. + require.NotEqual(t, chanIDOf(1), chanIDOf(2)) +} + +// TestAssembleInFlightHTLCsRequiredFee checks that a replayed HTLC is scored +// on the fee the node's own policies require, computed like the live forward +// path (outbound fee of the outgoing channel plus inbound fee of the incoming +// one), capped at the fee the sender offered and clamped at zero. +func TestAssembleInFlightHTLCsRequiredFee(t *testing.T) { + t.Parallel() + + // Both channels of a 1 -> 2 forward of 100_000 msat, with the sender + // offering a 1000 msat fee. + chans := []*chanstate.OpenChannel{ + makeChannel(1, makeHTLC(true, 0, 500, false)), + makeChannel(2), + } + circuit := func() *htlcswitch.PaymentCircuit { + return &htlcswitch.PaymentCircuit{ + Incoming: inKey(1, 0), Outgoing: outKey(2, 0), + IncomingAmount: 101_000, OutgoingAmount: 100_000, + } + } + + tests := []struct { + name string + policies map[wire.OutPoint]*models.ChannelEdgePolicy + wantFee lnwire.MilliSatoshi + }{{ + // 100 base + 100_000 * 1000 / 1e6 = 200 outbound, no inbound. + name: "required below offered", + policies: map[wire.OutPoint]*models.ChannelEdgePolicy{ + chans[0].FundingOutpoint: policy(0, 0), + chans[1].FundingOutpoint: policy(100, 1000), + }, + wantFee: 200, + }, { + // Inbound fee of 50 base + 1000 ppm on (100_000 + 200) is + // charged on top: 200 + 50 + 100 = 350. + name: "inbound fee included", + policies: map[wire.OutPoint]*models.ChannelEdgePolicy{ + chans[0].FundingOutpoint: policy( + 0, 0, lnwire.Fee{BaseFee: 50, FeeRate: 1000}, + ), + chans[1].FundingOutpoint: policy(100, 1000), + }, + wantFee: 350, + }, { + // An inbound discount larger than the outbound fee clamps at + // zero rather than going negative. + name: "inbound discount clamps at zero", + policies: map[wire.OutPoint]*models.ChannelEdgePolicy{ + chans[0].FundingOutpoint: policy( + 0, 0, lnwire.Fee{BaseFee: -1000}, + ), + chans[1].FundingOutpoint: policy(100, 1000), + }, + wantFee: 0, + }, { + // The policy was raised since the forward was accepted: the + // node still cannot earn more than the sender offered. + name: "required capped at offered", + policies: map[wire.OutPoint]*models.ChannelEdgePolicy{ + chans[0].FundingOutpoint: policy(0, 0), + chans[1].FundingOutpoint: policy(5000, 0), + }, + wantFee: 1000, + }, { + // Only the outgoing policy is known: fall back to offered. + name: "incoming policy missing", + policies: map[wire.OutPoint]*models.ChannelEdgePolicy{ + chans[1].FundingOutpoint: policy(100, 1000), + }, + wantFee: 1000, + }, { + // Only the incoming policy is known: fall back to offered. + name: "outgoing policy missing", + policies: map[wire.OutPoint]*models.ChannelEdgePolicy{ + chans[0].FundingOutpoint: policy(0, 0), + }, + wantFee: 1000, + }} + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + got := assembleInFlightHTLCs( + []*htlcswitch.PaymentCircuit{circuit()}, chans, + channelInfo{ + policies: test.policies, + nextHtlcIndex: nextIndexes(2, 1), + }, + ) + require.Len(t, got, 1) + require.Equal(t, test.wantFee, got[0].Fee) + }) + } +} + +// TestFetchSelfPolicies checks that the node's own policy is picked from the +// two directional policies of each channel, that channels without a graph +// entry or without our policy are left out, and that other graph errors are +// surfaced. +func TestFetchSelfPolicies(t *testing.T) { + t.Parallel() + + var self, other [33]byte + self[0], other[0] = 2, 3 + + chans := []*chanstate.OpenChannel{ + makeChannel(1), makeChannel(2), makeChannel(3), makeChannel(4), + } + ours1, ours2 := policy(1, 0), policy(2, 0) + theirs := policy(99, 0) + + graph := &fakePolicySource{ + edges: map[wire.OutPoint]fakeEdge{ + // We are node 1, so ours is the first policy. + chans[0].FundingOutpoint: { + info: &models.ChannelEdgeInfo{ + NodeKey1Bytes: self, + NodeKey2Bytes: other, + }, + p1: ours1, p2: theirs, + }, + // We are node 2, so ours is the second policy. + chans[1].FundingOutpoint: { + info: &models.ChannelEdgeInfo{ + NodeKey1Bytes: other, + NodeKey2Bytes: self, + }, + p1: theirs, p2: ours2, + }, + // Our policy was never advertised. + chans[2].FundingOutpoint: { + info: &models.ChannelEdgeInfo{ + NodeKey1Bytes: self, + NodeKey2Bytes: other, + }, + p2: theirs, + }, + // Channel 4 is not in the graph at all. + }, + } + + got, err := fetchSelfPolicies(t.Context(), graph, self, chans) + require.NoError(t, err) + require.Equal(t, map[wire.OutPoint]*models.ChannelEdgePolicy{ + chans[0].FundingOutpoint: ours1, + chans[1].FundingOutpoint: ours2, + }, got) + + // Any error other than a missing edge must not be mistaken for "no + // policies". + graph.err = errors.New("db closed") + _, err = fetchSelfPolicies(t.Context(), graph, self, chans) + require.ErrorIs(t, err, graph.err) +} + +// TestRemoveClosedChannel checks that a closed channel event removes the +// channel by the channel_id of its channel point, that other channel events +// and a closed channel event without a summary are ignored, and that a +// failure to remove is not propagated. +func TestRemoveClosedChannel(t *testing.T) { + t.Parallel() + + mgr := &fakeRemover{} + chanPoint := wire.OutPoint{Index: 7} + + removeClosedChannel(mgr, channelnotifier.OpenChannelEvent{}) + removeClosedChannel(mgr, channelnotifier.ClosedChannelEvent{}) + require.Empty(t, mgr.removed) + + removeClosedChannel(mgr, channelnotifier.ClosedChannelEvent{ + CloseSummary: &chanstate.ChannelCloseSummary{ + ChanPoint: chanPoint, + }, + }) + require.Equal(t, []lnwire.ChannelID{ + lnwire.NewChanIDFromOutPoint(chanPoint), + }, mgr.removed) + + mgr.err = errors.New("store closed") + removeClosedChannel(mgr, channelnotifier.ClosedChannelEvent{ + CloseSummary: &chanstate.ChannelCloseSummary{ + ChanPoint: chanPoint, + }, + }) + require.Len(t, mgr.removed, 2) +} + +// TestReconstructInFlightHTLCs checks the sources are read through, and that a +// channel source error is surfaced rather than treated as no channels. +func TestReconstructInFlightHTLCs(t *testing.T) { + t.Parallel() + + circuits := &fakeCircuitSource{ + circuits: []*htlcswitch.PaymentCircuit{{ + Incoming: inKey(1, 0), Outgoing: outKey(2, 0), + IncomingAmount: 1010, OutgoingAmount: 1000, + }}, + } + chans := &fakeChannelSource{ + chans: []*chanstate.OpenChannel{ + makeChannel(1, makeHTLC(true, 0, 500, true)), + makeChannel(2), + }, + } + + var self [33]byte + self[0] = 2 + graph := &fakePolicySource{ + edges: map[wire.OutPoint]fakeEdge{ + // Our policy on the outgoing channel requires 5 msat, + // half of the 10 offered. + chans.chans[1].FundingOutpoint: { + info: &models.ChannelEdgeInfo{ + NodeKey1Bytes: self, + }, + p1: policy(5, 0), + }, + chans.chans[0].FundingOutpoint: { + info: &models.ChannelEdgeInfo{ + NodeKey1Bytes: self, + }, + p1: policy(0, 0), + }, + }, + } + + nextIndex := fakeNextHtlcIndex(nextIndexes(1, 0, 2, 1)) + + got, err := reconstructInFlightHTLCs( + t.Context(), circuits, chans, graph, self, nextIndex, + ) + require.NoError(t, err) + require.Len(t, got, 1) + require.EqualValues(t, 5, got[0].Fee) + require.True(t, got[0].Accountable) + require.Equal(t, chanIDOf(1), got[0].IncomingChan) + require.Equal(t, chanIDOf(2), got[0].OutgoingChan) + + // An error reading the channels or the policies must not be mistaken + // for "no HTLCs in flight". + chans.err = errors.New("db closed") + _, err = reconstructInFlightHTLCs( + t.Context(), circuits, chans, graph, self, nextIndex, + ) + require.ErrorIs(t, err, chans.err) + + chans.err = nil + graph.err = errors.New("graph closed") + _, err = reconstructInFlightHTLCs( + t.Context(), circuits, chans, graph, self, nextIndex, + ) + require.ErrorIs(t, err, graph.err) + + // A channel whose next htlc index cannot be read is an error too. + graph.err = nil + _, err = reconstructInFlightHTLCs( + t.Context(), circuits, chans, graph, self, + fakeNextHtlcIndex(nil), + ) + require.ErrorContains(t, err, "next htlc index") + + // No open circuits means nothing to replay. + circuits.circuits = nil + got, err = reconstructInFlightHTLCs( + t.Context(), circuits, chans, graph, self, nextIndex, + ) + require.NoError(t, err) + require.Empty(t, got) +} diff --git a/sample-lnd.conf b/sample-lnd.conf index 6e7fe71f6a..3fc3fc44a1 100644 --- a/sample-lnd.conf +++ b/sample-lnd.conf @@ -1602,6 +1602,7 @@ ; Subsystems which support native SQL tables: ; - Invoices ; - Graph +; - Reputation ; db.use-native-sql=false ; If set to true, the KV to native SQL migration will be skipped. Note that diff --git a/server.go b/server.go index 21817f569d..a58eb5f088 100644 --- a/server.go +++ b/server.go @@ -909,12 +909,14 @@ func newServer(ctx context.Context, cfg *Config, listenAddrs []net.Addr, // by default. It only observes HTLC forwarding to compute and log // reputation; it never affects routing (log-only). When disabled via // no-reputation, repMgrIface stays a nil interface so the switch skips - // the hooks entirely. Nothing is persisted, so reputation is re-accrued - // from live traffic on restart. + // the hooks entirely. Channel state is persisted through the + // reputation store when the native SQL store is in use; otherwise it + // is kept in memory only and re-accrued from live traffic on restart. var repMgrIface htlcswitch.ReputationManager if !cfg.Routing.NoReputation { s.reputationMgr, err = reputation.NewManager( reputation.DefaultConfig(), clock.NewDefaultClock(), + dbs.ReputationStore, ) if err != nil { return nil, err @@ -2451,6 +2453,45 @@ func (s *server) Start(ctx context.Context) error { startErr = err return } + + // Rebuild the manager's view of the HTLCs that were in + // flight across the restart. This has to happen before + // the switch starts, since it re-forwards pending + // resolutions on start and those must find their HTLCs. + // The subsystem is log-only, so a failure to rebuild + // them is logged rather than failing the start: the + // HTLCs are then scored as unknown when they resolve. + var selfKey [33]byte + copy( + selfKey[:], + s.identityECDH.PubKey().SerializeCompressed(), + ) + inFlight, err := reconstructInFlightHTLCs( + ctx, s.htlcSwitch, s.chanStateDB, s.graphDB, + selfKey, + (*chanstate.OpenChannel).NextLocalHtlcIndex, + ) + if err != nil { + srvrLog.Errorf("Unable to reconstruct "+ + "in-flight HTLCs for reputation "+ + "replay: %v", err) + } else { + s.reputationMgr.ReplayInFlight( + inFlight, s.htlcSwitch.BestHeight(), + ) + } + + // Subscribe to channel events before the subsystems + // that emit closes start, so no close is missed. + closeEvents, err := s.channelNotifier. + SubscribeChannelEvents() + if err != nil { + startErr = err + return + } + + s.wg.Add(1) + go s.forwardChannelClosesToReputation(closeEvents) } if s.towerClientMgr != nil { diff --git a/sqldb/migrations.go b/sqldb/migrations.go index 241e5c0d68..e531b186c1 100644 --- a/sqldb/migrations.go +++ b/sqldb/migrations.go @@ -136,6 +136,11 @@ var ( Version: 18, SchemaVersion: 15, }, + { + Name: "000016_reputation", + Version: 19, + SchemaVersion: 16, + }, }, migrationAdditions...) // ErrMigrationMismatch is returned when a migrated record does not diff --git a/sqldb/migrations_dev.go b/sqldb/migrations_dev.go index 3716d5271b..0a4c919b29 100644 --- a/sqldb/migrations_dev.go +++ b/sqldb/migrations_dev.go @@ -10,8 +10,8 @@ package sqldb // migrations.go) with the next free versions when its schema is final. var migrationAdditions = []MigrationConfig{ { - Name: "000016_offers", - Version: 19, - SchemaVersion: 16, + Name: "000017_offers", + Version: 20, + SchemaVersion: 17, }, } diff --git a/sqldb/sqlc/migrations/000016_reputation.down.sql b/sqldb/sqlc/migrations/000016_reputation.down.sql new file mode 100644 index 0000000000..db20dd35d0 --- /dev/null +++ b/sqldb/sqlc/migrations/000016_reputation.down.sql @@ -0,0 +1 @@ +DROP TABLE IF EXISTS reputation_channels; diff --git a/sqldb/sqlc/migrations/000016_reputation.up.sql b/sqldb/sqlc/migrations/000016_reputation.up.sql new file mode 100644 index 0000000000..e56a2f1539 --- /dev/null +++ b/sqldb/sqlc/migrations/000016_reputation.up.sql @@ -0,0 +1,33 @@ +-- reputation_channels holds the persisted local reputation state of each +-- channel, so that a node does not lose the forwarding history its peers have +-- built when it restarts. One row per channel, keyed by channel_id, which +-- unlike the short channel id is stable for the channel's lifetime. +-- +-- The two decaying averages are stored exactly as they are held in memory: the +-- running value together with the timestamp it was last updated at. Decay is +-- applied lazily on read, so restoring these verbatim means the first read +-- after a restart decays the value over the full downtime. +CREATE TABLE IF NOT EXISTS reputation_channels ( + -- channel_id is the 32 byte channel_id of the channel. + channel_id BLOB PRIMARY KEY, + + -- outgoing_reputation is the reputation the channel has accrued as an + -- outgoing link, in millisatoshis. + outgoing_reputation BIGINT NOT NULL, + + -- outgoing_reputation_updated_at is the time the outgoing reputation + -- average was last updated. + outgoing_reputation_updated_at TIMESTAMP NOT NULL, + + -- incoming_revenue is the revenue the channel has earned as an incoming + -- link, in millisatoshis, aggregated over several windows. + incoming_revenue BIGINT NOT NULL, + + -- incoming_revenue_updated_at is the time the incoming revenue average + -- was last updated. + incoming_revenue_updated_at TIMESTAMP NOT NULL, + + -- incoming_revenue_started_at is the time the incoming revenue average + -- started tracking, used for its warm-up factor. + incoming_revenue_started_at TIMESTAMP NOT NULL +); diff --git a/sqldb/sqlc/migrations/000016_offers.down.sql b/sqldb/sqlc/migrations/000017_offers.down.sql similarity index 100% rename from sqldb/sqlc/migrations/000016_offers.down.sql rename to sqldb/sqlc/migrations/000017_offers.down.sql diff --git a/sqldb/sqlc/migrations/000016_offers.up.sql b/sqldb/sqlc/migrations/000017_offers.up.sql similarity index 100% rename from sqldb/sqlc/migrations/000016_offers.up.sql rename to sqldb/sqlc/migrations/000017_offers.up.sql diff --git a/sqldb/sqlc/models.go b/sqldb/sqlc/models.go index 159c041aee..e163094809 100644 --- a/sqldb/sqlc/models.go +++ b/sqldb/sqlc/models.go @@ -321,3 +321,12 @@ type PaymentRouteHopMpp struct { PaymentAddr []byte TotalMsat int64 } + +type ReputationChannel struct { + ChannelID []byte + OutgoingReputation int64 + OutgoingReputationUpdatedAt time.Time + IncomingRevenue int64 + IncomingRevenueUpdatedAt time.Time + IncomingRevenueStartedAt time.Time +} diff --git a/sqldb/sqlc/querier.go b/sqldb/sqlc/querier.go index f3655d7b61..d340a8e7d4 100644 --- a/sqldb/sqlc/querier.go +++ b/sqldb/sqlc/querier.go @@ -34,6 +34,7 @@ type Querier interface { DeleteNodeFeature(ctx context.Context, arg DeleteNodeFeatureParams) error DeletePayment(ctx context.Context, id int64) error DeletePruneLogEntriesInRange(ctx context.Context, arg DeletePruneLogEntriesInRangeParams) error + DeleteReputationChannel(ctx context.Context, channelID []byte) (int64, error) DeleteUnconnectedNodes(ctx context.Context) ([][]byte, error) DeleteZombieChannel(ctx context.Context, arg DeleteZombieChannelParams) (sql.Result, error) FailAttempt(ctx context.Context, arg FailAttemptParams) error @@ -69,6 +70,7 @@ type Querier interface { // lower bound on the primary key used for cursor-based pagination; the caller // must supply 0 when starting from the beginning. FetchPendingInvoices(ctx context.Context, arg FetchPendingInvoicesParams) ([]Invoice, error) + FetchReputationChannels(ctx context.Context) ([]ReputationChannel, error) FetchRouteLevelFirstHopCustomRecords(ctx context.Context, htlcAttemptIndices []int64) ([]PaymentAttemptFirstHopCustomRecord, error) FetchSettledAMPSubInvoices(ctx context.Context, arg FetchSettledAMPSubInvoicesParams) ([]FetchSettledAMPSubInvoicesRow, error) // FilterInvoicesByAddIndex returns invoices whose add_index (primary key id) @@ -258,6 +260,7 @@ type Querier interface { UpsertNodeAddress(ctx context.Context, arg UpsertNodeAddressParams) error UpsertNodeExtraType(ctx context.Context, arg UpsertNodeExtraTypeParams) error UpsertPruneLogEntry(ctx context.Context, arg UpsertPruneLogEntryParams) error + UpsertReputationChannel(ctx context.Context, arg UpsertReputationChannelParams) error // We use a separate upsert for our own node since we want to be less strict // about the last_update field. For our own node, we always want to // update the record even if the last_update is the same as what we have. diff --git a/sqldb/sqlc/queries/reputation.sql b/sqldb/sqlc/queries/reputation.sql new file mode 100644 index 0000000000..82e729098d --- /dev/null +++ b/sqldb/sqlc/queries/reputation.sql @@ -0,0 +1,21 @@ +-- name: UpsertReputationChannel :exec +INSERT INTO reputation_channels ( + channel_id, outgoing_reputation, outgoing_reputation_updated_at, + incoming_revenue, incoming_revenue_updated_at, incoming_revenue_started_at +) VALUES ( + $1, $2, $3, $4, $5, $6 +) +ON CONFLICT (channel_id) DO UPDATE SET + outgoing_reputation = EXCLUDED.outgoing_reputation, + outgoing_reputation_updated_at = EXCLUDED.outgoing_reputation_updated_at, + incoming_revenue = EXCLUDED.incoming_revenue, + incoming_revenue_updated_at = EXCLUDED.incoming_revenue_updated_at, + incoming_revenue_started_at = EXCLUDED.incoming_revenue_started_at; + +-- name: FetchReputationChannels :many +SELECT * FROM reputation_channels +ORDER BY channel_id; + +-- name: DeleteReputationChannel :execrows +DELETE FROM reputation_channels +WHERE channel_id = $1; diff --git a/sqldb/sqlc/reputation.sql.go b/sqldb/sqlc/reputation.sql.go new file mode 100644 index 0000000000..382ccb8b89 --- /dev/null +++ b/sqldb/sqlc/reputation.sql.go @@ -0,0 +1,95 @@ +// Code generated by sqlc. DO NOT EDIT. +// versions: +// sqlc v1.29.0 +// source: reputation.sql + +package sqlc + +import ( + "context" + "time" +) + +const deleteReputationChannel = `-- name: DeleteReputationChannel :execrows +DELETE FROM reputation_channels +WHERE channel_id = $1 +` + +func (q *Queries) DeleteReputationChannel(ctx context.Context, channelID []byte) (int64, error) { + result, err := q.db.ExecContext(ctx, deleteReputationChannel, channelID) + if err != nil { + return 0, err + } + return result.RowsAffected() +} + +const fetchReputationChannels = `-- name: FetchReputationChannels :many +SELECT channel_id, outgoing_reputation, outgoing_reputation_updated_at, incoming_revenue, incoming_revenue_updated_at, incoming_revenue_started_at FROM reputation_channels +ORDER BY channel_id +` + +func (q *Queries) FetchReputationChannels(ctx context.Context) ([]ReputationChannel, error) { + rows, err := q.db.QueryContext(ctx, fetchReputationChannels) + if err != nil { + return nil, err + } + defer rows.Close() + var items []ReputationChannel + for rows.Next() { + var i ReputationChannel + if err := rows.Scan( + &i.ChannelID, + &i.OutgoingReputation, + &i.OutgoingReputationUpdatedAt, + &i.IncomingRevenue, + &i.IncomingRevenueUpdatedAt, + &i.IncomingRevenueStartedAt, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + +const upsertReputationChannel = `-- name: UpsertReputationChannel :exec +INSERT INTO reputation_channels ( + channel_id, outgoing_reputation, outgoing_reputation_updated_at, + incoming_revenue, incoming_revenue_updated_at, incoming_revenue_started_at +) VALUES ( + $1, $2, $3, $4, $5, $6 +) +ON CONFLICT (channel_id) DO UPDATE SET + outgoing_reputation = EXCLUDED.outgoing_reputation, + outgoing_reputation_updated_at = EXCLUDED.outgoing_reputation_updated_at, + incoming_revenue = EXCLUDED.incoming_revenue, + incoming_revenue_updated_at = EXCLUDED.incoming_revenue_updated_at, + incoming_revenue_started_at = EXCLUDED.incoming_revenue_started_at +` + +type UpsertReputationChannelParams struct { + ChannelID []byte + OutgoingReputation int64 + OutgoingReputationUpdatedAt time.Time + IncomingRevenue int64 + IncomingRevenueUpdatedAt time.Time + IncomingRevenueStartedAt time.Time +} + +func (q *Queries) UpsertReputationChannel(ctx context.Context, arg UpsertReputationChannelParams) error { + _, err := q.db.ExecContext(ctx, upsertReputationChannel, + arg.ChannelID, + arg.OutgoingReputation, + arg.OutgoingReputationUpdatedAt, + arg.IncomingRevenue, + arg.IncomingRevenueUpdatedAt, + arg.IncomingRevenueStartedAt, + ) + return err +}