From 665179ece2657d95e16e10e8bb509675c88e87d7 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Mon, 17 Jul 2023 12:53:17 +0200 Subject: [PATCH 01/15] funding+server: don't use wallet DB for channel operations Since the actual wallet backends might be different between the wallet DB and the actual channel state DB, we pass in the correct struct to the funding manager. --- funding/manager.go | 23 ++++++++++++----------- funding/manager_test.go | 2 ++ server.go | 1 + 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/funding/manager.go b/funding/manager.go index 9f78aac06..2e356fb85 100644 --- a/funding/manager.go +++ b/funding/manager.go @@ -386,6 +386,9 @@ type Config struct { // so that the channel creation process can be completed. Notifier chainntnfs.ChainNotifier + // ChannelDB is the database that keeps track of all channel state. + ChannelDB *channeldb.ChannelStateDB + // SignMessage signs an arbitrary message with a given public key. The // actual digest signed is the double sha-256 of the message. In the // case that the private key corresponding to the passed public key @@ -698,7 +701,7 @@ func (f *Manager) start() error { // down. // TODO(roasbeef): store height that funding finished? // * would then replace call below - allChannels, err := f.cfg.Wallet.Cfg.Database.FetchAllChannels() + allChannels, err := f.cfg.ChannelDB.FetchAllChannels() if err != nil { return err } @@ -1396,7 +1399,7 @@ func (f *Manager) handleFundingOpen(peer lnpeer.Peer, // Also count the channels that are already pending. There we don't know // the underlying intent anymore, unfortunately. - channels, err := f.cfg.Wallet.Cfg.Database.FetchOpenChannels(peerPubKey) + channels, err := f.cfg.ChannelDB.FetchOpenChannels(peerPubKey) if err != nil { f.failFundingFlow(peer, cid, err) return @@ -1423,7 +1426,7 @@ func (f *Manager) handleFundingOpen(peer lnpeer.Peer, } // Ensure that the pendingChansLimit is respected. - pendingChans, err := f.cfg.Wallet.Cfg.Database.FetchPendingChannels() + pendingChans, err := f.cfg.ChannelDB.FetchPendingChannels() if err != nil { f.failFundingFlow(peer, cid, err) return @@ -4633,9 +4636,7 @@ func (f *Manager) saveInitialFwdingPolicy(permChanID lnwire.ChannelID, byteOrder.PutUint64(scratch[24:32], uint64(forwardingPolicy.FeeRate)) byteOrder.PutUint32(scratch[32:], forwardingPolicy.TimeLockDelta) - return f.cfg.Wallet.Cfg.Database.SaveInitialFwdingPolicy( - chanID, scratch, - ) + return f.cfg.ChannelDB.SaveInitialFwdingPolicy(chanID, scratch) } // getInitialFwdingPolicy fetches the initial forwarding policy for a given @@ -4647,7 +4648,7 @@ func (f *Manager) getInitialFwdingPolicy(permChanID lnwire.ChannelID) ( chanID := make([]byte, 32) copy(chanID, permChanID[:]) - value, err := f.cfg.Wallet.Cfg.Database.GetInitialFwdingPolicy(chanID) + value, err := f.cfg.ChannelDB.GetInitialFwdingPolicy(chanID) if err != nil { return nil, err } @@ -4676,7 +4677,7 @@ func (f *Manager) deleteInitialFwdingPolicy(permChanID lnwire.ChannelID) error { chanID := make([]byte, 32) copy(chanID, permChanID[:]) - return f.cfg.Wallet.Cfg.Database.DeleteInitialFwdingPolicy(chanID) + return f.cfg.ChannelDB.DeleteInitialFwdingPolicy(chanID) } // saveChannelOpeningState saves the channelOpeningState for the provided @@ -4694,7 +4695,7 @@ func (f *Manager) saveChannelOpeningState(chanPoint *wire.OutPoint, scratch := make([]byte, 10) byteOrder.PutUint16(scratch[:2], uint16(state)) byteOrder.PutUint64(scratch[2:], shortChanID.ToUint64()) - return f.cfg.Wallet.Cfg.Database.SaveChannelOpeningState( + return f.cfg.ChannelDB.SaveChannelOpeningState( outpointBytes.Bytes(), scratch, ) } @@ -4710,7 +4711,7 @@ func (f *Manager) getChannelOpeningState(chanPoint *wire.OutPoint) ( return 0, nil, err } - value, err := f.cfg.Wallet.Cfg.Database.GetChannelOpeningState( + value, err := f.cfg.ChannelDB.GetChannelOpeningState( outpointBytes.Bytes(), ) if err != nil { @@ -4729,7 +4730,7 @@ func (f *Manager) deleteChannelOpeningState(chanPoint *wire.OutPoint) error { return err } - return f.cfg.Wallet.Cfg.Database.DeleteChannelOpeningState( + return f.cfg.ChannelDB.DeleteChannelOpeningState( outpointBytes.Bytes(), ) } diff --git a/funding/manager_test.go b/funding/manager_test.go index df6d8cf22..3fc9dc4ba 100644 --- a/funding/manager_test.go +++ b/funding/manager_test.go @@ -432,6 +432,7 @@ func createTestFundingManager(t *testing.T, privKey *btcec.PrivateKey, IDKeyLoc: testKeyLoc, Wallet: lnw, Notifier: chainNotifier, + ChannelDB: cdb, FeeEstimator: estimator, SignMessage: func(_ keychain.KeyLocator, _ []byte, _ bool) (*ecdsa.Signature, error) { @@ -601,6 +602,7 @@ func recreateAliceFundingManager(t *testing.T, alice *testNode) { IDKeyLoc: oldCfg.IDKeyLoc, Wallet: oldCfg.Wallet, Notifier: oldCfg.Notifier, + ChannelDB: oldCfg.ChannelDB, FeeEstimator: oldCfg.FeeEstimator, SignMessage: func(_ keychain.KeyLocator, _ []byte, _ bool) (*ecdsa.Signature, error) { diff --git a/server.go b/server.go index e3fc80113..ff8738edf 100644 --- a/server.go +++ b/server.go @@ -1297,6 +1297,7 @@ func newServer(cfg *Config, listenAddrs []net.Addr, return cc.Wallet.LabelTransaction(hash, label, true) }, Notifier: cc.ChainNotifier, + ChannelDB: s.chanStateDB, FeeEstimator: cc.FeeEstimator, SignMessage: cc.MsgSigner.SignMessage, CurrentNodeAnnouncement: func() (lnwire.NodeAnnouncement, From 5b9aa63ceca6a5e23068bffc0c3dfac094f04d08 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Mon, 17 Jul 2023 12:53:18 +0200 Subject: [PATCH 02/15] funding: extract defaultForwardingPolicy --- funding/manager.go | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/funding/manager.go b/funding/manager.go index 2e356fb85..c244076ef 100644 --- a/funding/manager.go +++ b/funding/manager.go @@ -1747,13 +1747,10 @@ func (f *Manager) handleFundingOpen(peer lnpeer.Peer, minHtlc = acceptorResp.MinHtlcIn } - // If we are handling a FundingOpen request then we need to - // specify the default channel fees since they are not provided - // by the responder interactively. - forwardingPolicy := htlcswitch.ForwardingPolicy{ - BaseFee: f.cfg.DefaultRoutingPolicy.BaseFee, - FeeRate: f.cfg.DefaultRoutingPolicy.FeeRate, - } + // If we are handling a FundingOpen request then we need to specify the + // default channel fees since they are not provided by the responder + // interactively. + forwardingPolicy := f.defaultForwardingPolicy() // Once the reservation has been created successfully, we add it to // this peer's map of pending reservations to track this particular @@ -1765,7 +1762,7 @@ func (f *Manager) handleFundingOpen(peer lnpeer.Peer, resCtx := &reservationWithCtx{ reservation: reservation, chanAmt: amt, - forwardingPolicy: forwardingPolicy, + forwardingPolicy: *forwardingPolicy, remoteCsvDelay: remoteCsvDelay, remoteMinHtlc: minHtlc, remoteMaxValue: remoteMaxValue, @@ -4282,14 +4279,10 @@ func (f *Manager) handleInitFundingMsg(msg *InitFundingMsg) { maxHtlcs = f.cfg.RequiredRemoteMaxHTLCs(capacity) } - // Prepare the optional channel fee values from the initFundingMsg. - // If useBaseFee or useFeeRate are false the client did not - // provide fee values hence we assume default fee settings from - // the config. - forwardingPolicy := htlcswitch.ForwardingPolicy{ - BaseFee: f.cfg.DefaultRoutingPolicy.BaseFee, - FeeRate: f.cfg.DefaultRoutingPolicy.FeeRate, - } + // Prepare the optional channel fee values from the initFundingMsg. If + // useBaseFee or useFeeRate are false the client did not provide fee + // values hence we assume default fee settings from the config. + forwardingPolicy := f.defaultForwardingPolicy() if baseFee != nil { forwardingPolicy.BaseFee = lnwire.MilliSatoshi(*baseFee) } @@ -4327,7 +4320,7 @@ func (f *Manager) handleInitFundingMsg(msg *InitFundingMsg) { resCtx := &reservationWithCtx{ chanAmt: capacity, - forwardingPolicy: forwardingPolicy, + forwardingPolicy: *forwardingPolicy, remoteCsvDelay: remoteCsvDelay, remoteMinHtlc: minHtlcIn, remoteMaxValue: maxValue, @@ -4621,7 +4614,15 @@ func copyPubKey(pub *btcec.PublicKey) *btcec.PublicKey { return btcec.NewPublicKey(&tmp.X, &tmp.Y) } -// saveInitialFwdingPolicy saves the forwarding policy for the provided +// defaultForwardingPolicy returns the default forwarding policy. +func (f *Manager) defaultForwardingPolicy() *htlcswitch.ForwardingPolicy { + return &htlcswitch.ForwardingPolicy{ + BaseFee: f.cfg.DefaultRoutingPolicy.BaseFee, + FeeRate: f.cfg.DefaultRoutingPolicy.FeeRate, + } +} + +// saveInitialForwardingPolicy saves the forwarding policy for the provided // chanPoint in the channelOpeningStateBucket. func (f *Manager) saveInitialFwdingPolicy(permChanID lnwire.ChannelID, forwardingPolicy *htlcswitch.ForwardingPolicy) error { From 4770cb0aaf7f26c09f3e005a342b1201ff8edfb3 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Mon, 17 Jul 2023 12:53:19 +0200 Subject: [PATCH 03/15] manager: also store initial time lock delta and HTLC settings This will allow us to also set the TimeLockDelta and HTLC settings when creating the channel. We leave the actual implementation of the RPC and CLI changes to another PR, this is just to make things more consistent. --- funding/manager.go | 32 +++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/funding/manager.go b/funding/manager.go index c244076ef..9d8c300a5 100644 --- a/funding/manager.go +++ b/funding/manager.go @@ -1750,7 +1750,10 @@ func (f *Manager) handleFundingOpen(peer lnpeer.Peer, // If we are handling a FundingOpen request then we need to specify the // default channel fees since they are not provided by the responder // interactively. - forwardingPolicy := f.defaultForwardingPolicy() + ourContribution := reservation.OurContribution() + forwardingPolicy := f.defaultForwardingPolicy( + ourContribution.ChannelConstraints, + ) // Once the reservation has been created successfully, we add it to // this peer's map of pending reservations to track this particular @@ -1825,7 +1828,6 @@ func (f *Manager) handleFundingOpen(peer lnpeer.Peer, // With the initiator's contribution recorded, respond with our // contribution in the next message of the workflow. - ourContribution := reservation.OurContribution() fundingAccept := lnwire.AcceptChannel{ PendingChannelID: msg.PendingChannelID, DustLimit: ourContribution.DustLimit, @@ -4279,10 +4281,16 @@ func (f *Manager) handleInitFundingMsg(msg *InitFundingMsg) { maxHtlcs = f.cfg.RequiredRemoteMaxHTLCs(capacity) } + // Once the reservation has been created, and indexed, queue a funding + // request to the remote peer, kicking off the funding workflow. + ourContribution := reservation.OurContribution() + // Prepare the optional channel fee values from the initFundingMsg. If // useBaseFee or useFeeRate are false the client did not provide fee // values hence we assume default fee settings from the config. - forwardingPolicy := f.defaultForwardingPolicy() + forwardingPolicy := f.defaultForwardingPolicy( + ourContribution.ChannelConstraints, + ) if baseFee != nil { forwardingPolicy.BaseFee = lnwire.MilliSatoshi(*baseFee) } @@ -4291,10 +4299,6 @@ func (f *Manager) handleInitFundingMsg(msg *InitFundingMsg) { forwardingPolicy.FeeRate = lnwire.MilliSatoshi(*feeRate) } - // Once the reservation has been created, and indexed, queue a funding - // request to the remote peer, kicking off the funding workflow. - ourContribution := reservation.OurContribution() - // Fetch our dust limit which is part of the default channel // constraints, and log it. ourDustLimit := ourContribution.DustLimit @@ -4614,11 +4618,17 @@ func copyPubKey(pub *btcec.PublicKey) *btcec.PublicKey { return btcec.NewPublicKey(&tmp.X, &tmp.Y) } -// defaultForwardingPolicy returns the default forwarding policy. -func (f *Manager) defaultForwardingPolicy() *htlcswitch.ForwardingPolicy { +// defaultForwardingPolicy returns the default forwarding policy based on the +// default routing policy and our local channel constraints. +func (f *Manager) defaultForwardingPolicy( + constraints channeldb.ChannelConstraints) *htlcswitch.ForwardingPolicy { + return &htlcswitch.ForwardingPolicy{ - BaseFee: f.cfg.DefaultRoutingPolicy.BaseFee, - FeeRate: f.cfg.DefaultRoutingPolicy.FeeRate, + MinHTLCOut: constraints.MinHTLC, + MaxHTLC: constraints.MaxPendingAmount, + BaseFee: f.cfg.DefaultRoutingPolicy.BaseFee, + FeeRate: f.cfg.DefaultRoutingPolicy.FeeRate, + TimeLockDelta: f.cfg.DefaultRoutingPolicy.TimeLockDelta, } } From eda34a8d2af60f2a3e05992e1090339e519fe508 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Mon, 17 Jul 2023 12:53:21 +0200 Subject: [PATCH 04/15] channeldb: move fwding policy code to new file --- channeldb/db.go | 63 ------------------------------- channeldb/forwarding_policy.go | 68 ++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 63 deletions(-) create mode 100644 channeldb/forwarding_policy.go diff --git a/channeldb/db.go b/channeldb/db.go index 7515d1ae5..18a66c46e 100644 --- a/channeldb/db.go +++ b/channeldb/db.go @@ -312,11 +312,6 @@ var ( // channelOpeningState for each channel that is currently in the process // of being opened. channelOpeningStateBucket = []byte("channelOpeningState") - - // initialChannelFwdingPolicyBucket is the database bucket used to store - // the forwarding policy for each permanent channel that is currently - // in the process of being opened. - initialChannelFwdingPolicyBucket = []byte("initialChannelFwdingPolicy") ) // DB is the primary datastore for the lnd daemon. The database stores @@ -1433,64 +1428,6 @@ func (c *ChannelStateDB) AbandonChannel(chanPoint *wire.OutPoint, return dbChan.CloseChannel(summary, ChanStatusLocalCloseInitiator) } -// SaveInitialFwdingPolicy saves the serialized forwarding policy for the -// provided permanent channel id to the initialChannelFwdingPolicyBucket. -func (c *ChannelStateDB) SaveInitialFwdingPolicy(chanID, - forwardingPolicy []byte) error { - - return kvdb.Update(c.backend, func(tx kvdb.RwTx) error { - bucket, err := tx.CreateTopLevelBucket( - initialChannelFwdingPolicyBucket, - ) - if err != nil { - return err - } - - return bucket.Put(chanID, forwardingPolicy) - }, func() {}) -} - -// GetInitialFwdingPolicy fetches the serialized forwarding policy for the -// provided channel id from the database, or returns ErrChannelNotFound if -// a forwarding policy for this channel id is not found. -func (c *ChannelStateDB) GetInitialFwdingPolicy(chanID []byte) ([]byte, error) { - var serializedState []byte - err := kvdb.View(c.backend, func(tx kvdb.RTx) error { - bucket := tx.ReadBucket(initialChannelFwdingPolicyBucket) - if bucket == nil { - // If the bucket does not exist, it means we - // never added a channel fees to the db, so - // return ErrChannelNotFound. - return ErrChannelNotFound - } - - stateBytes := bucket.Get(chanID) - if stateBytes == nil { - return ErrChannelNotFound - } - - serializedState = append(serializedState, stateBytes...) - - return nil - }, func() { - serializedState = nil - }) - return serializedState, err -} - -// DeleteInitialFwdingPolicy removes the forwarding policy for a given channel -// from the database. -func (c *ChannelStateDB) DeleteInitialFwdingPolicy(chanID []byte) error { - return kvdb.Update(c.backend, func(tx kvdb.RwTx) error { - bucket := tx.ReadWriteBucket(initialChannelFwdingPolicyBucket) - if bucket == nil { - return ErrChannelNotFound - } - - return bucket.Delete(chanID) - }, func() {}) -} - // SaveChannelOpeningState saves the serialized channel state for the provided // chanPoint to the channelOpeningStateBucket. func (c *ChannelStateDB) SaveChannelOpeningState(outPoint, diff --git a/channeldb/forwarding_policy.go b/channeldb/forwarding_policy.go new file mode 100644 index 000000000..e158341dd --- /dev/null +++ b/channeldb/forwarding_policy.go @@ -0,0 +1,68 @@ +package channeldb + +import "github.com/lightningnetwork/lnd/kvdb" + +var ( + // initialChannelFwdingPolicyBucket is the database bucket used to store + // the forwarding policy for each permanent channel that is currently + // in the process of being opened. + initialChannelFwdingPolicyBucket = []byte("initialChannelFwdingPolicy") +) + +// SaveInitialFwdingPolicy saves the serialized forwarding policy for the +// provided permanent channel id to the initialChannelFwdingPolicyBucket. +func (c *ChannelStateDB) SaveInitialFwdingPolicy(chanID, + forwardingPolicy []byte) error { + + return kvdb.Update(c.backend, func(tx kvdb.RwTx) error { + bucket, err := tx.CreateTopLevelBucket( + initialChannelFwdingPolicyBucket, + ) + if err != nil { + return err + } + + return bucket.Put(chanID, forwardingPolicy) + }, func() {}) +} + +// GetInitialFwdingPolicy fetches the serialized forwarding policy for the +// provided channel id from the database, or returns ErrChannelNotFound if +// a forwarding policy for this channel id is not found. +func (c *ChannelStateDB) GetInitialFwdingPolicy(chanID []byte) ([]byte, error) { + var serializedState []byte + err := kvdb.View(c.backend, func(tx kvdb.RTx) error { + bucket := tx.ReadBucket(initialChannelFwdingPolicyBucket) + if bucket == nil { + // If the bucket does not exist, it means we + // never added a channel fees to the db, so + // return ErrChannelNotFound. + return ErrChannelNotFound + } + + stateBytes := bucket.Get(chanID) + if stateBytes == nil { + return ErrChannelNotFound + } + + serializedState = append(serializedState, stateBytes...) + + return nil + }, func() { + serializedState = nil + }) + return serializedState, err +} + +// DeleteInitialFwdingPolicy removes the forwarding policy for a given channel +// from the database. +func (c *ChannelStateDB) DeleteInitialFwdingPolicy(chanID []byte) error { + return kvdb.Update(c.backend, func(tx kvdb.RwTx) error { + bucket := tx.ReadWriteBucket(initialChannelFwdingPolicyBucket) + if bucket == nil { + return ErrChannelNotFound + } + + return bucket.Delete(chanID) + }, func() {}) +} From 36fcf65e97096c0f4fdb8c8a4a49286761da5663 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Mon, 17 Jul 2023 12:53:22 +0200 Subject: [PATCH 05/15] funding+channeldb: rename fwding -> forwarding --- channeldb/forwarding_policy.go | 36 +++++++++++++++----------- funding/manager.go | 46 ++++++++++++++++++---------------- funding/manager_test.go | 14 +++++++---- 3 files changed, 55 insertions(+), 41 deletions(-) diff --git a/channeldb/forwarding_policy.go b/channeldb/forwarding_policy.go index e158341dd..8d86b8624 100644 --- a/channeldb/forwarding_policy.go +++ b/channeldb/forwarding_policy.go @@ -3,20 +3,22 @@ package channeldb import "github.com/lightningnetwork/lnd/kvdb" var ( - // initialChannelFwdingPolicyBucket is the database bucket used to store - // the forwarding policy for each permanent channel that is currently - // in the process of being opened. - initialChannelFwdingPolicyBucket = []byte("initialChannelFwdingPolicy") + // initialChannelForwardingPolicyBucket is the database bucket used to + // store the forwarding policy for each permanent channel that is + // currently in the process of being opened. + initialChannelForwardingPolicyBucket = []byte( + "initialChannelFwdingPolicy", + ) ) -// SaveInitialFwdingPolicy saves the serialized forwarding policy for the -// provided permanent channel id to the initialChannelFwdingPolicyBucket. -func (c *ChannelStateDB) SaveInitialFwdingPolicy(chanID, +// SaveInitialForwardingPolicy saves the serialized forwarding policy for the +// provided permanent channel id to the initialChannelForwardingPolicyBucket. +func (c *ChannelStateDB) SaveInitialForwardingPolicy(chanID, forwardingPolicy []byte) error { return kvdb.Update(c.backend, func(tx kvdb.RwTx) error { bucket, err := tx.CreateTopLevelBucket( - initialChannelFwdingPolicyBucket, + initialChannelForwardingPolicyBucket, ) if err != nil { return err @@ -26,13 +28,15 @@ func (c *ChannelStateDB) SaveInitialFwdingPolicy(chanID, }, func() {}) } -// GetInitialFwdingPolicy fetches the serialized forwarding policy for the +// GetInitialForwardingPolicy fetches the serialized forwarding policy for the // provided channel id from the database, or returns ErrChannelNotFound if // a forwarding policy for this channel id is not found. -func (c *ChannelStateDB) GetInitialFwdingPolicy(chanID []byte) ([]byte, error) { +func (c *ChannelStateDB) GetInitialForwardingPolicy(chanID []byte) ([]byte, + error) { + var serializedState []byte err := kvdb.View(c.backend, func(tx kvdb.RTx) error { - bucket := tx.ReadBucket(initialChannelFwdingPolicyBucket) + bucket := tx.ReadBucket(initialChannelForwardingPolicyBucket) if bucket == nil { // If the bucket does not exist, it means we // never added a channel fees to the db, so @@ -54,11 +58,13 @@ func (c *ChannelStateDB) GetInitialFwdingPolicy(chanID []byte) ([]byte, error) { return serializedState, err } -// DeleteInitialFwdingPolicy removes the forwarding policy for a given channel -// from the database. -func (c *ChannelStateDB) DeleteInitialFwdingPolicy(chanID []byte) error { +// DeleteInitialForwardingPolicy removes the forwarding policy for a given +// channel from the database. +func (c *ChannelStateDB) DeleteInitialForwardingPolicy(chanID []byte) error { return kvdb.Update(c.backend, func(tx kvdb.RwTx) error { - bucket := tx.ReadWriteBucket(initialChannelFwdingPolicyBucket) + bucket := tx.ReadWriteBucket( + initialChannelForwardingPolicyBucket, + ) if bucket == nil { return ErrChannelNotFound } diff --git a/funding/manager.go b/funding/manager.go index 9d8c300a5..5469fe1f8 100644 --- a/funding/manager.go +++ b/funding/manager.go @@ -2372,7 +2372,7 @@ func (f *Manager) handleFundingCreated(peer lnpeer.Peer, // With a permanent channel id established we can save the respective // forwarding policy in the database. In the channel announcement phase // this forwarding policy is retrieved and applied. - err = f.saveInitialFwdingPolicy(cid.chanID, &forwardingPolicy) + err = f.saveInitialForwardingPolicy(cid.chanID, &forwardingPolicy) if err != nil { log.Errorf("Unable to store the forwarding policy: %v", err) } @@ -2475,7 +2475,9 @@ func (f *Manager) handleFundingSigned(peer lnpeer.Peer, // We have to store the forwardingPolicy before the reservation context // is deleted. The policy will then be read and applied in // newChanAnnouncement. - err = f.saveInitialFwdingPolicy(permChanID, &resCtx.forwardingPolicy) + err = f.saveInitialForwardingPolicy( + permChanID, &resCtx.forwardingPolicy, + ) if err != nil { log.Errorf("Unable to store the forwarding policy: %v", err) } @@ -3315,7 +3317,7 @@ func (f *Manager) annAfterSixConfs(completeChan *channeldb.OpenChannel, // For private channels we do not announce the channel policy // to the network but still need to delete them from the // database. - err = f.deleteInitialFwdingPolicy(chanID) + err = f.deleteInitialForwardingPolicy(chanID) if err != nil { log.Infof("Could not delete channel fees "+ "for chanId %x.", chanID) @@ -3818,7 +3820,7 @@ func (f *Manager) newChanAnnouncement(localPubKey, // forwarding policy to be announced. If no persisted initial policy // values are found, then we will use the default policy values in the // channel announcement. - storedFwdingPolicy, err := f.getInitialFwdingPolicy(chanID) + storedFwdingPolicy, err := f.getInitialForwardingPolicy(chanID) if err != nil && !errors.Is(err, channeldb.ErrChannelNotFound) { return nil, errors.Errorf("unable to generate channel "+ "update announcement: %v", err) @@ -3950,7 +3952,7 @@ func (f *Manager) announceChannel(localIDKey, remoteIDKey *btcec.PublicKey, // After the fee parameters have been stored in the announcement // we can delete them from the database. - err = f.deleteInitialFwdingPolicy(chanID) + err = f.deleteInitialForwardingPolicy(chanID) if err != nil { log.Infof("Could not delete channel fees for chanId %x.", chanID) @@ -4634,7 +4636,7 @@ func (f *Manager) defaultForwardingPolicy( // saveInitialForwardingPolicy saves the forwarding policy for the provided // chanPoint in the channelOpeningStateBucket. -func (f *Manager) saveInitialFwdingPolicy(permChanID lnwire.ChannelID, +func (f *Manager) saveInitialForwardingPolicy(permChanID lnwire.ChannelID, forwardingPolicy *htlcswitch.ForwardingPolicy) error { chanID := make([]byte, 32) @@ -4647,48 +4649,50 @@ func (f *Manager) saveInitialFwdingPolicy(permChanID lnwire.ChannelID, byteOrder.PutUint64(scratch[24:32], uint64(forwardingPolicy.FeeRate)) byteOrder.PutUint32(scratch[32:], forwardingPolicy.TimeLockDelta) - return f.cfg.ChannelDB.SaveInitialFwdingPolicy(chanID, scratch) + return f.cfg.ChannelDB.SaveInitialForwardingPolicy(chanID, scratch) } -// getInitialFwdingPolicy fetches the initial forwarding policy for a given +// getInitialForwardingPolicy fetches the initial forwarding policy for a given // channel id from the database which will be applied during the channel // announcement phase. -func (f *Manager) getInitialFwdingPolicy(permChanID lnwire.ChannelID) ( - *htlcswitch.ForwardingPolicy, error) { +func (f *Manager) getInitialForwardingPolicy( + permChanID lnwire.ChannelID) (*htlcswitch.ForwardingPolicy, error) { chanID := make([]byte, 32) copy(chanID, permChanID[:]) - value, err := f.cfg.ChannelDB.GetInitialFwdingPolicy(chanID) + value, err := f.cfg.ChannelDB.GetInitialForwardingPolicy(chanID) if err != nil { return nil, err } - var fwdingPolicy htlcswitch.ForwardingPolicy - fwdingPolicy.MinHTLCOut = lnwire.MilliSatoshi( + var forwardingPolicy htlcswitch.ForwardingPolicy + forwardingPolicy.MinHTLCOut = lnwire.MilliSatoshi( byteOrder.Uint64(value[:8]), ) - fwdingPolicy.MaxHTLC = lnwire.MilliSatoshi( + forwardingPolicy.MaxHTLC = lnwire.MilliSatoshi( byteOrder.Uint64(value[8:16]), ) - fwdingPolicy.BaseFee = lnwire.MilliSatoshi( + forwardingPolicy.BaseFee = lnwire.MilliSatoshi( byteOrder.Uint64(value[16:24]), ) - fwdingPolicy.FeeRate = lnwire.MilliSatoshi( + forwardingPolicy.FeeRate = lnwire.MilliSatoshi( byteOrder.Uint64(value[24:32]), ) - fwdingPolicy.TimeLockDelta = byteOrder.Uint32(value[32:36]) + forwardingPolicy.TimeLockDelta = byteOrder.Uint32(value[32:36]) - return &fwdingPolicy, nil + return &forwardingPolicy, nil } -// deleteInitialFwdingPolicy removes channel fees for this chanID from +// deleteInitialForwardingPolicy removes channel fees for this chanID from // the database. -func (f *Manager) deleteInitialFwdingPolicy(permChanID lnwire.ChannelID) error { +func (f *Manager) deleteInitialForwardingPolicy( + permChanID lnwire.ChannelID) error { + chanID := make([]byte, 32) copy(chanID, permChanID[:]) - return f.cfg.ChannelDB.DeleteInitialFwdingPolicy(chanID) + return f.cfg.ChannelDB.DeleteInitialForwardingPolicy(chanID) } // saveChannelOpeningState saves the channelOpeningState for the provided diff --git a/funding/manager_test.go b/funding/manager_test.go index 3fc9dc4ba..a12eeb796 100644 --- a/funding/manager_test.go +++ b/funding/manager_test.go @@ -1347,7 +1347,7 @@ func assertInitialFwdingPolicyNotFound(t *testing.T, node *testNode, if i > 0 { time.Sleep(testPollSleepMs * time.Millisecond) } - fwdingPolicy, err = node.fundingMgr.getInitialFwdingPolicy( + fwdingPolicy, err = node.fundingMgr.getInitialForwardingPolicy( *chanID, ) require.ErrorIs(t, err, channeldb.ErrChannelNotFound) @@ -3173,13 +3173,13 @@ func TestFundingManagerCustomChannelParameters(t *testing.T) { // After the funding is signed and before the channel announcement // we expect Alice and Bob to store their respective fees in the // database. - forwardingPolicy, err := alice.fundingMgr.getInitialFwdingPolicy( + forwardingPolicy, err := alice.fundingMgr.getInitialForwardingPolicy( fundingSigned.ChanID, ) require.NoError(t, err) require.NoError(t, assertFees(forwardingPolicy, 42, 1337)) - forwardingPolicy, err = bob.fundingMgr.getInitialFwdingPolicy( + forwardingPolicy, err = bob.fundingMgr.getInitialForwardingPolicy( fundingSigned.ChanID, ) require.NoError(t, err) @@ -3269,13 +3269,17 @@ func TestFundingManagerCustomChannelParameters(t *testing.T) { // After the announcement we expect Alice and Bob to have cleared // the fees for the channel from the database. - _, err = alice.fundingMgr.getInitialFwdingPolicy(fundingSigned.ChanID) + _, err = alice.fundingMgr.getInitialForwardingPolicy( + fundingSigned.ChanID, + ) if err != channeldb.ErrChannelNotFound { err = fmt.Errorf("channel fees were expected to be deleted" + " but were not") t.Fatal(err) } - _, err = bob.fundingMgr.getInitialFwdingPolicy(fundingSigned.ChanID) + _, err = bob.fundingMgr.getInitialForwardingPolicy( + fundingSigned.ChanID, + ) if err != channeldb.ErrChannelNotFound { err = fmt.Errorf("channel fees were expected to be deleted" + " but were not") From 59b5fb156599233edeef96f3b64c9a8d75fc4880 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Mon, 17 Jul 2023 12:53:23 +0200 Subject: [PATCH 06/15] channeldb+funding: move policy encoding into channel DB --- channeldb/forwarding_policy.go | 63 +++++++++++++++++++++++++++------- channeldb/models/channel.go | 38 ++++++++++++++++++++ funding/manager.go | 62 +++++++++++++-------------------- 3 files changed, 111 insertions(+), 52 deletions(-) diff --git a/channeldb/forwarding_policy.go b/channeldb/forwarding_policy.go index 8d86b8624..2a4123045 100644 --- a/channeldb/forwarding_policy.go +++ b/channeldb/forwarding_policy.go @@ -1,6 +1,10 @@ package channeldb -import "github.com/lightningnetwork/lnd/kvdb" +import ( + "github.com/lightningnetwork/lnd/channeldb/models" + "github.com/lightningnetwork/lnd/kvdb" + "github.com/lightningnetwork/lnd/lnwire" +) var ( // initialChannelForwardingPolicyBucket is the database bucket used to @@ -13,8 +17,18 @@ var ( // SaveInitialForwardingPolicy saves the serialized forwarding policy for the // provided permanent channel id to the initialChannelForwardingPolicyBucket. -func (c *ChannelStateDB) SaveInitialForwardingPolicy(chanID, - forwardingPolicy []byte) error { +func (c *ChannelStateDB) SaveInitialForwardingPolicy(chanID lnwire.ChannelID, + forwardingPolicy *models.ForwardingPolicy) error { + + chanIDCopy := make([]byte, 32) + copy(chanIDCopy, chanID[:]) + + scratch := make([]byte, 36) + byteOrder.PutUint64(scratch[:8], uint64(forwardingPolicy.MinHTLCOut)) + byteOrder.PutUint64(scratch[8:16], uint64(forwardingPolicy.MaxHTLC)) + byteOrder.PutUint64(scratch[16:24], uint64(forwardingPolicy.BaseFee)) + byteOrder.PutUint64(scratch[24:32], uint64(forwardingPolicy.FeeRate)) + byteOrder.PutUint32(scratch[32:], forwardingPolicy.TimeLockDelta) return kvdb.Update(c.backend, func(tx kvdb.RwTx) error { bucket, err := tx.CreateTopLevelBucket( @@ -24,17 +38,20 @@ func (c *ChannelStateDB) SaveInitialForwardingPolicy(chanID, return err } - return bucket.Put(chanID, forwardingPolicy) + return bucket.Put(chanIDCopy, scratch) }, func() {}) } // GetInitialForwardingPolicy fetches the serialized forwarding policy for the // provided channel id from the database, or returns ErrChannelNotFound if // a forwarding policy for this channel id is not found. -func (c *ChannelStateDB) GetInitialForwardingPolicy(chanID []byte) ([]byte, - error) { +func (c *ChannelStateDB) GetInitialForwardingPolicy( + chanID lnwire.ChannelID) (*models.ForwardingPolicy, error) { - var serializedState []byte + chanIDCopy := make([]byte, 32) + copy(chanIDCopy, chanID[:]) + + var forwardingPolicy *models.ForwardingPolicy err := kvdb.View(c.backend, func(tx kvdb.RTx) error { bucket := tx.ReadBucket(initialChannelForwardingPolicyBucket) if bucket == nil { @@ -44,23 +61,43 @@ func (c *ChannelStateDB) GetInitialForwardingPolicy(chanID []byte) ([]byte, return ErrChannelNotFound } - stateBytes := bucket.Get(chanID) + stateBytes := bucket.Get(chanIDCopy) if stateBytes == nil { return ErrChannelNotFound } - serializedState = append(serializedState, stateBytes...) + forwardingPolicy = &models.ForwardingPolicy{ + MinHTLCOut: lnwire.MilliSatoshi( + byteOrder.Uint64(stateBytes[:8]), + ), + MaxHTLC: lnwire.MilliSatoshi( + byteOrder.Uint64(stateBytes[8:16]), + ), + BaseFee: lnwire.MilliSatoshi( + byteOrder.Uint64(stateBytes[16:24]), + ), + FeeRate: lnwire.MilliSatoshi( + byteOrder.Uint64(stateBytes[24:32]), + ), + TimeLockDelta: byteOrder.Uint32(stateBytes[32:36]), + } return nil }, func() { - serializedState = nil + forwardingPolicy = nil }) - return serializedState, err + + return forwardingPolicy, err } // DeleteInitialForwardingPolicy removes the forwarding policy for a given // channel from the database. -func (c *ChannelStateDB) DeleteInitialForwardingPolicy(chanID []byte) error { +func (c *ChannelStateDB) DeleteInitialForwardingPolicy( + chanID lnwire.ChannelID) error { + + chanIDCopy := make([]byte, 32) + copy(chanIDCopy, chanID[:]) + return kvdb.Update(c.backend, func(tx kvdb.RwTx) error { bucket := tx.ReadWriteBucket( initialChannelForwardingPolicyBucket, @@ -69,6 +106,6 @@ func (c *ChannelStateDB) DeleteInitialForwardingPolicy(chanID []byte) error { return ErrChannelNotFound } - return bucket.Delete(chanID) + return bucket.Delete(chanIDCopy) }, func() {}) } diff --git a/channeldb/models/channel.go b/channeldb/models/channel.go index d05165309..4a65462e7 100644 --- a/channeldb/models/channel.go +++ b/channeldb/models/channel.go @@ -90,3 +90,41 @@ func (k *CircuitKey) Decode(r io.Reader) error { func (k CircuitKey) String() string { return fmt.Sprintf("(Chan ID=%s, HTLC ID=%d)", k.ChanID, k.HtlcID) } + +// ForwardingPolicy describes the set of constraints that a given ChannelLink +// is to adhere to when forwarding HTLC's. For each incoming HTLC, this set of +// constraints will be consulted in order to ensure that adequate fees are +// paid, and our time-lock parameters are respected. In the event that an +// incoming HTLC violates any of these constraints, it is to be _rejected_ with +// the error possibly carrying along a ChannelUpdate message that includes the +// latest policy. +type ForwardingPolicy struct { + // MinHTLCOut is the smallest HTLC that is to be forwarded. + MinHTLCOut lnwire.MilliSatoshi + + // MaxHTLC is the largest HTLC that is to be forwarded. + MaxHTLC lnwire.MilliSatoshi + + // BaseFee is the base fee, expressed in milli-satoshi that must be + // paid for each incoming HTLC. This field, combined with FeeRate is + // used to compute the required fee for a given HTLC. + BaseFee lnwire.MilliSatoshi + + // FeeRate is the fee rate, expressed in milli-satoshi that must be + // paid for each incoming HTLC. This field combined with BaseFee is + // used to compute the required fee for a given HTLC. + FeeRate lnwire.MilliSatoshi + + // TimeLockDelta is the absolute time-lock value, expressed in blocks, + // that will be subtracted from an incoming HTLC's timelock value to + // create the time-lock value for the forwarded outgoing HTLC. The + // following constraint MUST hold for an HTLC to be forwarded: + // + // * incomingHtlc.timeLock - timeLockDelta = fwdInfo.OutgoingCTLV + // + // where fwdInfo is the forwarding information extracted from the + // per-hop payload of the incoming HTLC's onion packet. + TimeLockDelta uint32 + + // TODO(roasbeef): add fee module inside of switch +} diff --git a/funding/manager.go b/funding/manager.go index 5469fe1f8..e9eb3ad48 100644 --- a/funding/manager.go +++ b/funding/manager.go @@ -20,6 +20,7 @@ import ( "github.com/lightningnetwork/lnd/chainreg" "github.com/lightningnetwork/lnd/chanacceptor" "github.com/lightningnetwork/lnd/channeldb" + "github.com/lightningnetwork/lnd/channeldb/models" "github.com/lightningnetwork/lnd/discovery" "github.com/lightningnetwork/lnd/htlcswitch" "github.com/lightningnetwork/lnd/input" @@ -4636,62 +4637,45 @@ func (f *Manager) defaultForwardingPolicy( // saveInitialForwardingPolicy saves the forwarding policy for the provided // chanPoint in the channelOpeningStateBucket. -func (f *Manager) saveInitialForwardingPolicy(permChanID lnwire.ChannelID, +func (f *Manager) saveInitialForwardingPolicy(chanID lnwire.ChannelID, forwardingPolicy *htlcswitch.ForwardingPolicy) error { - chanID := make([]byte, 32) - copy(chanID, permChanID[:]) - - scratch := make([]byte, 36) - byteOrder.PutUint64(scratch[:8], uint64(forwardingPolicy.MinHTLCOut)) - byteOrder.PutUint64(scratch[8:16], uint64(forwardingPolicy.MaxHTLC)) - byteOrder.PutUint64(scratch[16:24], uint64(forwardingPolicy.BaseFee)) - byteOrder.PutUint64(scratch[24:32], uint64(forwardingPolicy.FeeRate)) - byteOrder.PutUint32(scratch[32:], forwardingPolicy.TimeLockDelta) - - return f.cfg.ChannelDB.SaveInitialForwardingPolicy(chanID, scratch) + return f.cfg.ChannelDB.SaveInitialForwardingPolicy( + chanID, &models.ForwardingPolicy{ + MinHTLCOut: forwardingPolicy.MinHTLCOut, + MaxHTLC: forwardingPolicy.MaxHTLC, + BaseFee: forwardingPolicy.BaseFee, + FeeRate: forwardingPolicy.FeeRate, + TimeLockDelta: forwardingPolicy.TimeLockDelta, + }, + ) } // getInitialForwardingPolicy fetches the initial forwarding policy for a given // channel id from the database which will be applied during the channel // announcement phase. func (f *Manager) getInitialForwardingPolicy( - permChanID lnwire.ChannelID) (*htlcswitch.ForwardingPolicy, error) { + chanID lnwire.ChannelID) (*htlcswitch.ForwardingPolicy, error) { - chanID := make([]byte, 32) - copy(chanID, permChanID[:]) - - value, err := f.cfg.ChannelDB.GetInitialForwardingPolicy(chanID) + dbPolicy, err := f.cfg.ChannelDB.GetInitialForwardingPolicy( + chanID, + ) if err != nil { return nil, err } - var forwardingPolicy htlcswitch.ForwardingPolicy - forwardingPolicy.MinHTLCOut = lnwire.MilliSatoshi( - byteOrder.Uint64(value[:8]), - ) - forwardingPolicy.MaxHTLC = lnwire.MilliSatoshi( - byteOrder.Uint64(value[8:16]), - ) - forwardingPolicy.BaseFee = lnwire.MilliSatoshi( - byteOrder.Uint64(value[16:24]), - ) - forwardingPolicy.FeeRate = lnwire.MilliSatoshi( - byteOrder.Uint64(value[24:32]), - ) - forwardingPolicy.TimeLockDelta = byteOrder.Uint32(value[32:36]) - - return &forwardingPolicy, nil + return &htlcswitch.ForwardingPolicy{ + MinHTLCOut: dbPolicy.MinHTLCOut, + MaxHTLC: dbPolicy.MaxHTLC, + BaseFee: dbPolicy.BaseFee, + FeeRate: dbPolicy.FeeRate, + TimeLockDelta: dbPolicy.TimeLockDelta, + }, nil } // deleteInitialForwardingPolicy removes channel fees for this chanID from // the database. -func (f *Manager) deleteInitialForwardingPolicy( - permChanID lnwire.ChannelID) error { - - chanID := make([]byte, 32) - copy(chanID, permChanID[:]) - +func (f *Manager) deleteInitialForwardingPolicy(chanID lnwire.ChannelID) error { return f.cfg.ChannelDB.DeleteInitialForwardingPolicy(chanID) } From d5c504c8de7b7962950666710df5af1c2894251b Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Mon, 17 Jul 2023 12:53:24 +0200 Subject: [PATCH 07/15] multi: use fwding policy from models pkg --- chainreg/chainregistry.go | 8 ++--- funding/manager.go | 40 +++++++------------------ funding/manager_test.go | 18 +++++------ htlcswitch/interfaces.go | 2 +- htlcswitch/link.go | 48 ++++-------------------------- htlcswitch/link_test.go | 9 +++--- htlcswitch/mock.go | 2 +- htlcswitch/switch.go | 2 +- htlcswitch/test_utils.go | 5 ++-- peer/brontide.go | 11 +++---- routing/localchans/manager.go | 8 ++--- routing/localchans/manager_test.go | 4 +-- 12 files changed, 52 insertions(+), 105 deletions(-) diff --git a/chainreg/chainregistry.go b/chainreg/chainregistry.go index dbe374c4e..309238505 100644 --- a/chainreg/chainregistry.go +++ b/chainreg/chainregistry.go @@ -25,7 +25,7 @@ import ( "github.com/lightningnetwork/lnd/chainntnfs/btcdnotify" "github.com/lightningnetwork/lnd/chainntnfs/neutrinonotify" "github.com/lightningnetwork/lnd/channeldb" - "github.com/lightningnetwork/lnd/htlcswitch" + "github.com/lightningnetwork/lnd/channeldb/models" "github.com/lightningnetwork/lnd/input" "github.com/lightningnetwork/lnd/keychain" "github.com/lightningnetwork/lnd/kvdb" @@ -195,7 +195,7 @@ type PartialChainControl struct { ChainSource chain.Interface // RoutingPolicy is the routing policy we have decided to use. - RoutingPolicy htlcswitch.ForwardingPolicy + RoutingPolicy models.ForwardingPolicy // MinHtlcIn is the minimum HTLC we will accept. MinHtlcIn lnwire.MilliSatoshi @@ -270,7 +270,7 @@ func NewPartialChainControl(cfg *Config) (*PartialChainControl, func(), error) { switch cfg.PrimaryChain() { case BitcoinChain: - cc.RoutingPolicy = htlcswitch.ForwardingPolicy{ + cc.RoutingPolicy = models.ForwardingPolicy{ MinHTLCOut: cfg.Bitcoin.MinHTLCOut, BaseFee: cfg.Bitcoin.BaseFee, FeeRate: cfg.Bitcoin.FeeRate, @@ -282,7 +282,7 @@ func NewPartialChainControl(cfg *Config) (*PartialChainControl, func(), error) { DefaultBitcoinStaticMinRelayFeeRate, ) case LitecoinChain: - cc.RoutingPolicy = htlcswitch.ForwardingPolicy{ + cc.RoutingPolicy = models.ForwardingPolicy{ MinHTLCOut: cfg.Litecoin.MinHTLCOut, BaseFee: cfg.Litecoin.BaseFee, FeeRate: cfg.Litecoin.FeeRate, diff --git a/funding/manager.go b/funding/manager.go index e9eb3ad48..d15a2943d 100644 --- a/funding/manager.go +++ b/funding/manager.go @@ -22,7 +22,6 @@ import ( "github.com/lightningnetwork/lnd/channeldb" "github.com/lightningnetwork/lnd/channeldb/models" "github.com/lightningnetwork/lnd/discovery" - "github.com/lightningnetwork/lnd/htlcswitch" "github.com/lightningnetwork/lnd/input" "github.com/lightningnetwork/lnd/keychain" "github.com/lightningnetwork/lnd/labels" @@ -155,7 +154,7 @@ type reservationWithCtx struct { chanAmt btcutil.Amount // forwardingPolicy is the policy provided by the initFundingMsg. - forwardingPolicy htlcswitch.ForwardingPolicy + forwardingPolicy models.ForwardingPolicy // Constraints we require for the remote. remoteCsvDelay uint16 @@ -433,7 +432,7 @@ type Config struct { // DefaultRoutingPolicy is the default routing policy used when // initially announcing channels. - DefaultRoutingPolicy htlcswitch.ForwardingPolicy + DefaultRoutingPolicy models.ForwardingPolicy // DefaultMinHtlcIn is the default minimum incoming htlc value that is // set as a channel parameter. @@ -520,7 +519,7 @@ type Config struct { // UpdateForwardingPolicies is used by the manager to update active // links with a new policy. UpdateForwardingPolicies func( - chanPolicies map[wire.OutPoint]htlcswitch.ForwardingPolicy) + chanPolicies map[wire.OutPoint]models.ForwardingPolicy) // OpenChannelPredicate is a predicate on the lnwire.OpenChannel message // and on the requesting node's public key that returns a bool which @@ -3254,7 +3253,7 @@ func (f *Manager) addToRouterGraph(completeChan *channeldb.OpenChannel, // our local policy in the switch to make sure we can forward payments // with the correct fees. We can't do this when creating the link // initially as that only takes the static channel parameters. - updatedPolicy := map[wire.OutPoint]htlcswitch.ForwardingPolicy{ + updatedPolicy := map[wire.OutPoint]models.ForwardingPolicy{ completeChan.FundingOutpoint: { MinHTLCOut: ann.chanUpdateAnn.HtlcMinimumMsat, MaxHTLC: ann.chanUpdateAnn.HtlcMaximumMsat, @@ -4624,9 +4623,9 @@ func copyPubKey(pub *btcec.PublicKey) *btcec.PublicKey { // defaultForwardingPolicy returns the default forwarding policy based on the // default routing policy and our local channel constraints. func (f *Manager) defaultForwardingPolicy( - constraints channeldb.ChannelConstraints) *htlcswitch.ForwardingPolicy { + constraints channeldb.ChannelConstraints) *models.ForwardingPolicy { - return &htlcswitch.ForwardingPolicy{ + return &models.ForwardingPolicy{ MinHTLCOut: constraints.MinHTLC, MaxHTLC: constraints.MaxPendingAmount, BaseFee: f.cfg.DefaultRoutingPolicy.BaseFee, @@ -4638,16 +4637,10 @@ func (f *Manager) defaultForwardingPolicy( // saveInitialForwardingPolicy saves the forwarding policy for the provided // chanPoint in the channelOpeningStateBucket. func (f *Manager) saveInitialForwardingPolicy(chanID lnwire.ChannelID, - forwardingPolicy *htlcswitch.ForwardingPolicy) error { + forwardingPolicy *models.ForwardingPolicy) error { return f.cfg.ChannelDB.SaveInitialForwardingPolicy( - chanID, &models.ForwardingPolicy{ - MinHTLCOut: forwardingPolicy.MinHTLCOut, - MaxHTLC: forwardingPolicy.MaxHTLC, - BaseFee: forwardingPolicy.BaseFee, - FeeRate: forwardingPolicy.FeeRate, - TimeLockDelta: forwardingPolicy.TimeLockDelta, - }, + chanID, forwardingPolicy, ) } @@ -4655,22 +4648,9 @@ func (f *Manager) saveInitialForwardingPolicy(chanID lnwire.ChannelID, // channel id from the database which will be applied during the channel // announcement phase. func (f *Manager) getInitialForwardingPolicy( - chanID lnwire.ChannelID) (*htlcswitch.ForwardingPolicy, error) { + chanID lnwire.ChannelID) (*models.ForwardingPolicy, error) { - dbPolicy, err := f.cfg.ChannelDB.GetInitialForwardingPolicy( - chanID, - ) - if err != nil { - return nil, err - } - - return &htlcswitch.ForwardingPolicy{ - MinHTLCOut: dbPolicy.MinHTLCOut, - MaxHTLC: dbPolicy.MaxHTLC, - BaseFee: dbPolicy.BaseFee, - FeeRate: dbPolicy.FeeRate, - TimeLockDelta: dbPolicy.TimeLockDelta, - }, nil + return f.cfg.ChannelDB.GetInitialForwardingPolicy(chanID) } // deleteInitialForwardingPolicy removes channel fees for this chanID from diff --git a/funding/manager_test.go b/funding/manager_test.go index a12eeb796..c9288c25a 100644 --- a/funding/manager_test.go +++ b/funding/manager_test.go @@ -22,9 +22,9 @@ import ( "github.com/lightningnetwork/lnd/chainreg" acpt "github.com/lightningnetwork/lnd/chanacceptor" "github.com/lightningnetwork/lnd/channeldb" + "github.com/lightningnetwork/lnd/channeldb/models" "github.com/lightningnetwork/lnd/channelnotifier" "github.com/lightningnetwork/lnd/discovery" - "github.com/lightningnetwork/lnd/htlcswitch" "github.com/lightningnetwork/lnd/input" "github.com/lightningnetwork/lnd/keychain" "github.com/lightningnetwork/lnd/lncfg" @@ -250,7 +250,7 @@ type testNode struct { testDir string shutdownChannel chan struct{} reportScidChan chan struct{} - updatedPolicies chan map[wire.OutPoint]htlcswitch.ForwardingPolicy + updatedPolicies chan map[wire.OutPoint]models.ForwardingPolicy localFeatures []lnwire.FeatureBit remoteFeatures []lnwire.FeatureBit @@ -380,7 +380,7 @@ func createTestFundingManager(t *testing.T, privKey *btcec.PrivateKey, shutdownChan := make(chan struct{}) reportScidChan := make(chan struct{}) updatedPolicies := make( - chan map[wire.OutPoint]htlcswitch.ForwardingPolicy, 1, + chan map[wire.OutPoint]models.ForwardingPolicy, 1, ) wc := &mock.WalletController{ @@ -477,7 +477,7 @@ func createTestFundingManager(t *testing.T, privKey *btcec.PrivateKey, return nil, fmt.Errorf("unable to find channel") }, - DefaultRoutingPolicy: htlcswitch.ForwardingPolicy{ + DefaultRoutingPolicy: models.ForwardingPolicy{ MinHTLCOut: 5, BaseFee: 100, FeeRate: 1000, @@ -540,7 +540,7 @@ func createTestFundingManager(t *testing.T, privKey *btcec.PrivateKey, }, AliasManager: aliasMgr, UpdateForwardingPolicies: func( - p map[wire.OutPoint]htlcswitch.ForwardingPolicy) { + p map[wire.OutPoint]models.ForwardingPolicy) { updatedPolicies <- p }, @@ -633,7 +633,7 @@ func recreateAliceFundingManager(t *testing.T, alice *testNode) { }, TempChanIDSeed: oldCfg.TempChanIDSeed, FindChannel: oldCfg.FindChannel, - DefaultRoutingPolicy: htlcswitch.ForwardingPolicy{ + DefaultRoutingPolicy: models.ForwardingPolicy{ MinHTLCOut: 5, BaseFee: 100, FeeRate: 1000, @@ -1146,7 +1146,7 @@ func assertChannelAnnouncements(t *testing.T, alice, bob *testNode, // At this point we should also have gotten a policy update that // was sent to the switch subsystem. Make sure it contains the // same values. - var policyUpdate htlcswitch.ForwardingPolicy + var policyUpdate models.ForwardingPolicy select { case policyUpdateMap := <-node.updatedPolicies: require.Len(t, policyUpdateMap, 1) @@ -1340,7 +1340,7 @@ func assertInitialFwdingPolicyNotFound(t *testing.T, node *testNode, t.Helper() - var fwdingPolicy *htlcswitch.ForwardingPolicy + var fwdingPolicy *models.ForwardingPolicy var err error for i := 0; i < testPollNumTries; i++ { // If this is not the first try, sleep before retrying. @@ -3076,7 +3076,7 @@ func TestFundingManagerCustomChannelParameters(t *testing.T) { // Helper method for checking baseFee and feeRate stored for a // reservation. - assertFees := func(forwardingPolicy *htlcswitch.ForwardingPolicy, + assertFees := func(forwardingPolicy *models.ForwardingPolicy, baseFee, feeRate lnwire.MilliSatoshi) error { if forwardingPolicy.BaseFee != baseFee { diff --git a/htlcswitch/interfaces.go b/htlcswitch/interfaces.go index 8f418edfe..304f4a595 100644 --- a/htlcswitch/interfaces.go +++ b/htlcswitch/interfaces.go @@ -191,7 +191,7 @@ type ChannelLink interface { // UpdateForwardingPolicy updates the forwarding policy for the target // ChannelLink. Once updated, the link will use the new forwarding // policy to govern if it an incoming HTLC should be forwarded or not. - UpdateForwardingPolicy(ForwardingPolicy) + UpdateForwardingPolicy(models.ForwardingPolicy) // CheckHtlcForward should return a nil error if the passed HTLC details // satisfy the current forwarding policy fo the target link. Otherwise, diff --git a/htlcswitch/link.go b/htlcswitch/link.go index 4fbe90554..d025cdfff 100644 --- a/htlcswitch/link.go +++ b/htlcswitch/link.go @@ -64,44 +64,6 @@ const ( DefaultMaxLinkFeeAllocation float64 = 0.5 ) -// ForwardingPolicy describes the set of constraints that a given ChannelLink -// is to adhere to when forwarding HTLC's. For each incoming HTLC, this set of -// constraints will be consulted in order to ensure that adequate fees are -// paid, and our time-lock parameters are respected. In the event that an -// incoming HTLC violates any of these constraints, it is to be _rejected_ with -// the error possibly carrying along a ChannelUpdate message that includes the -// latest policy. -type ForwardingPolicy struct { - // MinHTLC is the smallest HTLC that is to be forwarded. - MinHTLCOut lnwire.MilliSatoshi - - // MaxHTLC is the largest HTLC that is to be forwarded. - MaxHTLC lnwire.MilliSatoshi - - // BaseFee is the base fee, expressed in milli-satoshi that must be - // paid for each incoming HTLC. This field, combined with FeeRate is - // used to compute the required fee for a given HTLC. - BaseFee lnwire.MilliSatoshi - - // FeeRate is the fee rate, expressed in milli-satoshi that must be - // paid for each incoming HTLC. This field combined with BaseFee is - // used to compute the required fee for a given HTLC. - FeeRate lnwire.MilliSatoshi - - // TimeLockDelta is the absolute time-lock value, expressed in blocks, - // that will be subtracted from an incoming HTLC's timelock value to - // create the time-lock value for the forwarded outgoing HTLC. The - // following constraint MUST hold for an HTLC to be forwarded: - // - // * incomingHtlc.timeLock - timeLockDelta = fwdInfo.OutgoingCTLV - // - // where fwdInfo is the forwarding information extracted from the - // per-hop payload of the incoming HTLC's onion packet. - TimeLockDelta uint32 - - // TODO(roasbeef): add fee module inside of switch -} - // ExpectedFee computes the expected fee for a given htlc amount. The value // returned from this function is to be used as a sanity check when forwarding // HTLC's to ensure that an incoming HTLC properly adheres to our propagated @@ -109,7 +71,7 @@ type ForwardingPolicy struct { // // TODO(roasbeef): also add in current available channel bandwidth, inverse // func -func ExpectedFee(f ForwardingPolicy, +func ExpectedFee(f models.ForwardingPolicy, htlcAmt lnwire.MilliSatoshi) lnwire.MilliSatoshi { return f.BaseFee + (htlcAmt*f.FeeRate)/1000000 @@ -123,7 +85,7 @@ type ChannelLinkConfig struct { // deciding whether to forwarding incoming HTLC's or not. This value // can be updated with subsequent calls to UpdateForwardingPolicy // targeted at a given ChannelLink concrete interface implementation. - FwrdingPolicy ForwardingPolicy + FwrdingPolicy models.ForwardingPolicy // Circuits provides restricted access to the switch's circuit map, // allowing the link to open and close circuits. @@ -2522,7 +2484,9 @@ func (l *channelLink) AttachMailBox(mailbox MailBox) { // update all of the link's FwrdingPolicy's values. // // NOTE: Part of the ChannelLink interface. -func (l *channelLink) UpdateForwardingPolicy(newPolicy ForwardingPolicy) { +func (l *channelLink) UpdateForwardingPolicy( + newPolicy models.ForwardingPolicy) { + l.Lock() defer l.Unlock() @@ -2627,7 +2591,7 @@ func (l *channelLink) CheckHtlcTransit(payHash [32]byte, // canSendHtlc checks whether the given htlc parameters satisfy // the channel's amount and time lock constraints. -func (l *channelLink) canSendHtlc(policy ForwardingPolicy, +func (l *channelLink) canSendHtlc(policy models.ForwardingPolicy, payHash [32]byte, amt lnwire.MilliSatoshi, timeout uint32, heightNow uint32, originalScid lnwire.ShortChannelID) *LinkError { diff --git a/htlcswitch/link_test.go b/htlcswitch/link_test.go index 35eb6a9ea..21b11c57d 100644 --- a/htlcswitch/link_test.go +++ b/htlcswitch/link_test.go @@ -23,6 +23,7 @@ import ( sphinx "github.com/lightningnetwork/lightning-onion" "github.com/lightningnetwork/lnd/build" "github.com/lightningnetwork/lnd/channeldb" + "github.com/lightningnetwork/lnd/channeldb/models" "github.com/lightningnetwork/lnd/contractcourt" "github.com/lightningnetwork/lnd/htlcswitch/hodl" "github.com/lightningnetwork/lnd/htlcswitch/hop" @@ -1925,7 +1926,7 @@ func newSingleLinkTestHarness(t *testing.T, chanAmt, chanReserve btcutil.Amount) sentMsgs: make(chan lnwire.Message, 2000), quit: make(chan struct{}), } - globalPolicy = ForwardingPolicy{ + globalPolicy = models.ForwardingPolicy{ MinHTLCOut: lnwire.NewMSatFromSatoshis(5), MaxHTLC: lnwire.NewMSatFromSatoshis(chanAmt), BaseFee: lnwire.NewMSatFromSatoshis(1), @@ -4374,7 +4375,7 @@ func (h *persistentLinkHarness) restartLink( quit: make(chan struct{}), } - globalPolicy = ForwardingPolicy{ + globalPolicy = models.ForwardingPolicy{ MinHTLCOut: lnwire.NewMSatFromSatoshis(5), BaseFee: lnwire.NewMSatFromSatoshis(1), TimeLockDelta: 6, @@ -5630,7 +5631,7 @@ func TestExpectedFee(t *testing.T) { } for _, test := range testCases { - f := ForwardingPolicy{ + f := models.ForwardingPolicy{ BaseFee: test.baseFee, FeeRate: test.feeRate, } @@ -5716,7 +5717,7 @@ func TestCheckHtlcForward(t *testing.T) { link := channelLink{ cfg: ChannelLinkConfig{ - FwrdingPolicy: ForwardingPolicy{ + FwrdingPolicy: models.ForwardingPolicy{ TimeLockDelta: 20, MinHTLCOut: 500, MaxHTLC: 1000, diff --git a/htlcswitch/mock.go b/htlcswitch/mock.go index a17045b1e..ea76549b4 100644 --- a/htlcswitch/mock.go +++ b/htlcswitch/mock.go @@ -840,7 +840,7 @@ func (f *mockChannelLink) getDustClosure() dustClosure { func (f *mockChannelLink) HandleChannelUpdate(lnwire.Message) { } -func (f *mockChannelLink) UpdateForwardingPolicy(_ ForwardingPolicy) { +func (f *mockChannelLink) UpdateForwardingPolicy(_ models.ForwardingPolicy) { } func (f *mockChannelLink) CheckHtlcForward([32]byte, lnwire.MilliSatoshi, lnwire.MilliSatoshi, uint32, uint32, uint32, diff --git a/htlcswitch/switch.go b/htlcswitch/switch.go index 6f632be35..2d20db800 100644 --- a/htlcswitch/switch.go +++ b/htlcswitch/switch.go @@ -611,7 +611,7 @@ func (s *Switch) SendHTLC(firstHop lnwire.ShortChannelID, attemptID uint64, // forwarding policies for all links have been updated, or the switch shuts // down. func (s *Switch) UpdateForwardingPolicies( - chanPolicies map[wire.OutPoint]ForwardingPolicy) { + chanPolicies map[wire.OutPoint]models.ForwardingPolicy) { log.Tracef("Updating link policies: %v", newLogClosure(func() string { return spew.Sdump(chanPolicies) diff --git a/htlcswitch/test_utils.go b/htlcswitch/test_utils.go index c1bfb1f55..b03aff417 100644 --- a/htlcswitch/test_utils.go +++ b/htlcswitch/test_utils.go @@ -23,6 +23,7 @@ import ( "github.com/go-errors/errors" sphinx "github.com/lightningnetwork/lightning-onion" "github.com/lightningnetwork/lnd/channeldb" + "github.com/lightningnetwork/lnd/channeldb/models" "github.com/lightningnetwork/lnd/contractcourt" "github.com/lightningnetwork/lnd/htlcswitch/hop" "github.com/lightningnetwork/lnd/input" @@ -1069,7 +1070,7 @@ func createTwoClusterChannels(t *testing.T, aliceToBob, // hopNetwork is the base struct for two and three hop networks type hopNetwork struct { feeEstimator *mockFeeEstimator - globalPolicy ForwardingPolicy + globalPolicy models.ForwardingPolicy obfuscator hop.ErrorEncrypter defaultDelta uint32 @@ -1078,7 +1079,7 @@ type hopNetwork struct { func newHopNetwork() *hopNetwork { defaultDelta := uint32(6) - globalPolicy := ForwardingPolicy{ + globalPolicy := models.ForwardingPolicy{ MinHTLCOut: lnwire.NewMSatFromSatoshis(5), BaseFee: lnwire.NewMSatFromSatoshis(1), TimeLockDelta: defaultDelta, diff --git a/peer/brontide.go b/peer/brontide.go index 43d4e57f3..cd328ea10 100644 --- a/peer/brontide.go +++ b/peer/brontide.go @@ -21,6 +21,7 @@ import ( "github.com/lightningnetwork/lnd/build" "github.com/lightningnetwork/lnd/chainntnfs" "github.com/lightningnetwork/lnd/channeldb" + "github.com/lightningnetwork/lnd/channeldb/models" "github.com/lightningnetwork/lnd/channelnotifier" "github.com/lightningnetwork/lnd/contractcourt" "github.com/lightningnetwork/lnd/discovery" @@ -234,7 +235,7 @@ type Config struct { // RoutingPolicy is used to set the forwarding policy for links created by // the Brontide. - RoutingPolicy htlcswitch.ForwardingPolicy + RoutingPolicy models.ForwardingPolicy // Sphinx is used when setting up ChannelLinks so they can decode sphinx // onion blobs. @@ -877,9 +878,9 @@ func (p *Brontide) loadActiveChannels(chans []*channeldb.OpenChannel) ( // If we don't yet have an advertised routing policy, then // we'll use the current default, otherwise we'll translate the // routing policy into a forwarding policy. - var forwardingPolicy *htlcswitch.ForwardingPolicy + var forwardingPolicy *models.ForwardingPolicy if selfPolicy != nil { - forwardingPolicy = &htlcswitch.ForwardingPolicy{ + forwardingPolicy = &models.ForwardingPolicy{ MinHTLCOut: selfPolicy.MinHTLC, MaxHTLC: selfPolicy.MaxHTLC, BaseFee: selfPolicy.FeeBaseMSat, @@ -934,7 +935,7 @@ func (p *Brontide) loadActiveChannels(chans []*channeldb.OpenChannel) ( // addLink creates and adds a new ChannelLink from the specified channel. func (p *Brontide) addLink(chanPoint *wire.OutPoint, lnChan *lnwallet.LightningChannel, - forwardingPolicy *htlcswitch.ForwardingPolicy, + forwardingPolicy *models.ForwardingPolicy, chainEvents *contractcourt.ChainEventSubscription, syncStates bool) error { @@ -3839,7 +3840,7 @@ func (p *Brontide) addActiveChannel(c *channeldb.OpenChannel) error { // the total value of outstanding HTLCs. fwdMinHtlc := lnChan.FwdMinHtlc() defaultPolicy := p.cfg.RoutingPolicy - forwardingPolicy := &htlcswitch.ForwardingPolicy{ + forwardingPolicy := &models.ForwardingPolicy{ MinHTLCOut: fwdMinHtlc, MaxHTLC: c.LocalChanCfg.MaxPendingAmount, BaseFee: defaultPolicy.BaseFee, diff --git a/routing/localchans/manager.go b/routing/localchans/manager.go index 2155e6285..3a4e2416a 100644 --- a/routing/localchans/manager.go +++ b/routing/localchans/manager.go @@ -7,8 +7,8 @@ import ( "github.com/btcsuite/btcd/wire" "github.com/lightningnetwork/lnd/channeldb" + "github.com/lightningnetwork/lnd/channeldb/models" "github.com/lightningnetwork/lnd/discovery" - "github.com/lightningnetwork/lnd/htlcswitch" "github.com/lightningnetwork/lnd/kvdb" "github.com/lightningnetwork/lnd/lnrpc" "github.com/lightningnetwork/lnd/lnwire" @@ -21,7 +21,7 @@ type Manager struct { // UpdateForwardingPolicies is used by the manager to update active // links with a new policy. UpdateForwardingPolicies func( - chanPolicies map[wire.OutPoint]htlcswitch.ForwardingPolicy) + chanPolicies map[wire.OutPoint]models.ForwardingPolicy) // PropagateChanPolicyUpdate is called to persist a new policy to disk // and broadcast it to the network. @@ -66,7 +66,7 @@ func (r *Manager) UpdatePolicy(newSchema routing.ChannelPolicy, var failedUpdates []*lnrpc.FailedUpdate var edgesToUpdate []discovery.EdgeWithInfo - policiesToUpdate := make(map[wire.OutPoint]htlcswitch.ForwardingPolicy) + policiesToUpdate := make(map[wire.OutPoint]models.ForwardingPolicy) // Next, we'll loop over all the outgoing channels the router knows of. // If we have a filter then we'll only collected those channels, @@ -106,7 +106,7 @@ func (r *Manager) UpdatePolicy(newSchema routing.ChannelPolicy, }) // Add updated policy to list of policies to send to switch. - policiesToUpdate[info.ChannelPoint] = htlcswitch.ForwardingPolicy{ + policiesToUpdate[info.ChannelPoint] = models.ForwardingPolicy{ BaseFee: edge.FeeBaseMSat, FeeRate: edge.FeeProportionalMillionths, TimeLockDelta: uint32(edge.TimeLockDelta), diff --git a/routing/localchans/manager_test.go b/routing/localchans/manager_test.go index 94f4255ce..b14093a01 100644 --- a/routing/localchans/manager_test.go +++ b/routing/localchans/manager_test.go @@ -7,8 +7,8 @@ import ( "github.com/btcsuite/btcd/chaincfg/chainhash" "github.com/btcsuite/btcd/wire" "github.com/lightningnetwork/lnd/channeldb" + "github.com/lightningnetwork/lnd/channeldb/models" "github.com/lightningnetwork/lnd/discovery" - "github.com/lightningnetwork/lnd/htlcswitch" "github.com/lightningnetwork/lnd/kvdb" "github.com/lightningnetwork/lnd/lnrpc" "github.com/lightningnetwork/lnd/lnwire" @@ -50,7 +50,7 @@ func TestManager(t *testing.T) { } updateForwardingPolicies := func( - chanPolicies map[wire.OutPoint]htlcswitch.ForwardingPolicy) { + chanPolicies map[wire.OutPoint]models.ForwardingPolicy) { if len(chanPolicies) == 0 { return From 7ac445caafec8fff5eb3c6b20c1179d9ef52c788 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Mon, 17 Jul 2023 12:53:25 +0200 Subject: [PATCH 08/15] funding: make sure initial policy is always set --- funding/manager.go | 55 ++++++++++++++++++++++++ funding/manager_test.go | 92 ++++++++++++++++++++++++++--------------- 2 files changed, 113 insertions(+), 34 deletions(-) diff --git a/funding/manager.go b/funding/manager.go index d15a2943d..edd6b5f5f 100644 --- a/funding/manager.go +++ b/funding/manager.go @@ -3711,6 +3711,13 @@ func (f *Manager) handleChannelReady(peer lnpeer.Peer, } }() + // Before we can add the channel to the peer, we'll need to ensure that + // we have an initial forwarding policy set. + if err := f.ensureInitialForwardingPolicy(chanID, channel); err != nil { + log.Errorf("Unable to ensure initial forwarding policy: %v", + err) + } + if err := peer.AddNewChannel(channel, f.quit); err != nil { log.Errorf("Unable to add new channel %v with peer %x: %v", channel.FundingOutpoint, @@ -3719,6 +3726,54 @@ func (f *Manager) handleChannelReady(peer lnpeer.Peer, } } +// ensureInitialForwardingPolicy ensures that we have an initial forwarding +// policy set for the given channel. If we don't, we'll fall back to the default +// values. +func (f *Manager) ensureInitialForwardingPolicy(chanID lnwire.ChannelID, + channel *channeldb.OpenChannel) error { + + // Before we can add the channel to the peer, we'll need to ensure that + // we have an initial forwarding policy set. This should always be the + // case except for a channel that was created with lnd <= 0.15.5 and + // is still pending while updating to this version. + var needDBUpdate bool + forwardingPolicy, err := f.getInitialForwardingPolicy(chanID) + if err != nil { + log.Errorf("Unable to fetch initial forwarding policy, "+ + "falling back to default values: %v", err) + + forwardingPolicy = f.defaultForwardingPolicy( + channel.LocalChanCfg.ChannelConstraints, + ) + needDBUpdate = true + } + + // We only started storing the actual values for MinHTLCOut and MaxHTLC + // after 0.16.x, so if a channel was opened with such a version and is + // still pending while updating to this version, we'll need to set the + // values to the default values. + if forwardingPolicy.MinHTLCOut == 0 { + forwardingPolicy.MinHTLCOut = channel.LocalChanCfg.MinHTLC + needDBUpdate = true + } + if forwardingPolicy.MaxHTLC == 0 { + forwardingPolicy.MaxHTLC = channel.LocalChanCfg.MaxPendingAmount + needDBUpdate = true + } + + // And finally, if we found that the values currently stored aren't + // sufficient for the link, we'll update the database. + if needDBUpdate { + err := f.saveInitialForwardingPolicy(chanID, forwardingPolicy) + if err != nil { + return fmt.Errorf("unable to update initial "+ + "forwarding policy: %v", err) + } + } + + return nil +} + // chanAnnouncement encapsulates the two authenticated announcements that we // send out to the network after a new channel has been created locally. type chanAnnouncement struct { diff --git a/funding/manager_test.go b/funding/manager_test.go index c9288c25a..c1fc17a81 100644 --- a/funding/manager_test.go +++ b/funding/manager_test.go @@ -7,6 +7,7 @@ import ( "fmt" "net" "path/filepath" + "reflect" "runtime" "strings" "testing" @@ -31,6 +32,7 @@ import ( "github.com/lightningnetwork/lnd/lnpeer" "github.com/lightningnetwork/lnd/lnrpc" "github.com/lightningnetwork/lnd/lntest/mock" + "github.com/lightningnetwork/lnd/lntest/wait" "github.com/lightningnetwork/lnd/lnwallet" "github.com/lightningnetwork/lnd/lnwallet/chainfee" "github.com/lightningnetwork/lnd/lnwire" @@ -1340,42 +1342,38 @@ func assertInitialFwdingPolicyNotFound(t *testing.T, node *testNode, t.Helper() - var fwdingPolicy *models.ForwardingPolicy - var err error - for i := 0; i < testPollNumTries; i++ { - // If this is not the first try, sleep before retrying. - if i > 0 { - time.Sleep(testPollSleepMs * time.Millisecond) - } - fwdingPolicy, err = node.fundingMgr.getInitialForwardingPolicy( - *chanID, - ) - require.ErrorIs(t, err, channeldb.ErrChannelNotFound) - - // Got expected result, return with success. - return - } - - // 10 tries without success. - t.Fatalf("expected to not find a forwarding policy, found policy %v", - fwdingPolicy) + _, err := node.fundingMgr.getInitialForwardingPolicy(*chanID) + require.ErrorIs(t, err, channeldb.ErrChannelNotFound) } -func assertHandleChannelReady(t *testing.T, alice, bob *testNode) { +func assertHandleChannelReady(t *testing.T, alice, bob *testNode, + checkChannel ...func(node *testNode, msg *newChannelMsg) bool) { + t.Helper() + const timeout = time.Second * 15 // They should both send the new channel state to their peer. select { case c := <-alice.newChannels: + for _, check := range checkChannel { + require.NoError(t, wait.Predicate(func() bool { + return check(alice, c) + }, timeout)) + } close(c.err) - case <-time.After(time.Second * 15): + case <-time.After(timeout): t.Fatalf("alice did not send new channel to peer") } select { case c := <-bob.newChannels: + for _, check := range checkChannel { + require.NoError(t, wait.Predicate(func() bool { + return check(bob, c) + }, timeout)) + } close(c.err) - case <-time.After(time.Second * 15): + case <-time.After(timeout): t.Fatalf("bob did not send new channel to peer") } } @@ -2892,12 +2890,12 @@ func TestFundingManagerCustomChannelParameters(t *testing.T) { const fundingAmt = 5000000 const chanReserve = 100000 - // Use custom channel fees. - // These will show up in the channel reservation context - var baseFee uint64 - var feeRate uint64 - baseFee = 42 - feeRate = 1337 + // Use custom channel fees. These will show up in the channel + // reservation context. + var ( + baseFee uint64 = 42 + feeRate uint64 = 1337 + ) // We will consume the channel updates as we go, so no buffering is // needed. @@ -3218,10 +3216,6 @@ func TestFundingManagerCustomChannelParameters(t *testing.T) { alice.fundingMgr.ProcessFundingMsg(channelReadyBob, bob) bob.fundingMgr.ProcessFundingMsg(channelReadyAlice, alice) - // Check that they notify the breach arbiter and peer about the new - // channel. - assertHandleChannelReady(t, alice, bob) - // Make sure both fundingManagers send the expected channel // announcements. // Alice should advertise the default MinHTLC value of @@ -3238,15 +3232,45 @@ func TestFundingManagerCustomChannelParameters(t *testing.T) { // Alice should have custom fees set whereas Bob should see his // configured default fees announced. + defaultDelta := bob.fundingMgr.cfg.DefaultRoutingPolicy.TimeLockDelta defaultBaseFee := bob.fundingMgr.cfg.DefaultRoutingPolicy.BaseFee - defaultFeerate := bob.fundingMgr.cfg.DefaultRoutingPolicy.FeeRate + defaultFeeRate := bob.fundingMgr.cfg.DefaultRoutingPolicy.FeeRate baseFees := []lnwire.MilliSatoshi{ lnwire.MilliSatoshi(baseFee), defaultBaseFee, } feeRates := []lnwire.MilliSatoshi{ - lnwire.MilliSatoshi(feeRate), defaultFeerate, + lnwire.MilliSatoshi(feeRate), defaultFeeRate, } + // Check that they notify the breach arbiter and peer about the new + // channel. + assertHandleChannelReady( + t, alice, bob, func(node *testNode, msg *newChannelMsg) bool { + aliceDB := alice.fundingMgr.cfg.ChannelDB + chanID := lnwire.NewChanIDFromOutPoint( + &msg.channel.FundingOutpoint, + ) + + if node == alice { + p, err := aliceDB.GetInitialForwardingPolicy( + chanID, + ) + require.NoError(t, err) + + return reflect.DeepEqual( + p, &models.ForwardingPolicy{ + MaxHTLC: maxHtlcArr[0], + MinHTLCOut: minHtlcArr[0], + BaseFee: baseFees[0], + FeeRate: feeRates[0], + TimeLockDelta: defaultDelta, + }, + ) + } + + return true + }, + ) assertChannelAnnouncements( t, alice, bob, capacity, minHtlcArr, maxHtlcArr, baseFees, feeRates, From fb3680672477b372daa93f32060f434208a0e39f Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Mon, 17 Jul 2023 12:53:26 +0200 Subject: [PATCH 09/15] peer: load initial forwarding policy from channel DB --- peer/brontide.go | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/peer/brontide.go b/peer/brontide.go index cd328ea10..a58a44292 100644 --- a/peer/brontide.go +++ b/peer/brontide.go @@ -3832,26 +3832,18 @@ func (p *Brontide) addActiveChannel(c *channeldb.OpenChannel) error { err) } - // We'll query the localChanCfg of the new channel to determine the - // minimum HTLC value that can be forwarded. For the maximum HTLC value - // that can be forwarded and fees we'll use the default values, as they - // currently are always set to the default values at initial channel - // creation. Note that the maximum HTLC value defaults to the cap on - // the total value of outstanding HTLCs. - fwdMinHtlc := lnChan.FwdMinHtlc() - defaultPolicy := p.cfg.RoutingPolicy - forwardingPolicy := &models.ForwardingPolicy{ - MinHTLCOut: fwdMinHtlc, - MaxHTLC: c.LocalChanCfg.MaxPendingAmount, - BaseFee: defaultPolicy.BaseFee, - FeeRate: defaultPolicy.FeeRate, - TimeLockDelta: defaultPolicy.TimeLockDelta, + // We'll query the channel DB for the new channel's initial forwarding + // policies to determine the policy we start out with. + initialPolicy, err := p.cfg.ChannelDB.GetInitialForwardingPolicy(chanID) + if err != nil { + return fmt.Errorf("unable to query for initial forwarding "+ + "policy: %v", err) } // Create the link and add it to the switch. err = p.addLink( - chanPoint, lnChan, forwardingPolicy, - chainEvents, shouldReestablish, + chanPoint, lnChan, initialPolicy, chainEvents, + shouldReestablish, ) if err != nil { return fmt.Errorf("can't register new channel link(%v) with "+ From d822d75f00512ae8fec00ab4c006ce1d3e252bfe Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Mon, 17 Jul 2023 12:53:27 +0200 Subject: [PATCH 10/15] server+funding: remove previous link update mechanism Because we now send the correct initial forwarding policy to Brontide, the correct initial values are inserted into the link and we no longer need to update the link in a later step. --- funding/manager.go | 27 ------------------------ funding/manager_test.go | 46 +++++------------------------------------ server.go | 5 ++--- 3 files changed, 7 insertions(+), 71 deletions(-) diff --git a/funding/manager.go b/funding/manager.go index edd6b5f5f..3701cff7a 100644 --- a/funding/manager.go +++ b/funding/manager.go @@ -516,11 +516,6 @@ type Config struct { // transition from pending open to open. NotifyOpenChannelEvent func(wire.OutPoint) - // UpdateForwardingPolicies is used by the manager to update active - // links with a new policy. - UpdateForwardingPolicies func( - chanPolicies map[wire.OutPoint]models.ForwardingPolicy) - // OpenChannelPredicate is a predicate on the lnwire.OpenChannel message // and on the requesting node's public key that returns a bool which // tells the funding manager whether or not to accept the channel. @@ -3246,28 +3241,6 @@ func (f *Manager) addToRouterGraph(completeChan *channeldb.OpenChannel, return ErrFundingManagerShuttingDown } - // The user can define non-default channel policies when opening a - // channel. They are stored in the database to be persisted from the - // moment of funding the channel to it being confirmed. We just - // announced those policies to the network, but we also need to update - // our local policy in the switch to make sure we can forward payments - // with the correct fees. We can't do this when creating the link - // initially as that only takes the static channel parameters. - updatedPolicy := map[wire.OutPoint]models.ForwardingPolicy{ - completeChan.FundingOutpoint: { - MinHTLCOut: ann.chanUpdateAnn.HtlcMinimumMsat, - MaxHTLC: ann.chanUpdateAnn.HtlcMaximumMsat, - BaseFee: lnwire.MilliSatoshi( - ann.chanUpdateAnn.BaseFee, - ), - FeeRate: lnwire.MilliSatoshi( - ann.chanUpdateAnn.FeeRate, - ), - TimeLockDelta: uint32(ann.chanUpdateAnn.TimeLockDelta), - }, - } - f.cfg.UpdateForwardingPolicies(updatedPolicy) - return nil } diff --git a/funding/manager_test.go b/funding/manager_test.go index c1fc17a81..685d49f20 100644 --- a/funding/manager_test.go +++ b/funding/manager_test.go @@ -252,7 +252,6 @@ type testNode struct { testDir string shutdownChannel chan struct{} reportScidChan chan struct{} - updatedPolicies chan map[wire.OutPoint]models.ForwardingPolicy localFeatures []lnwire.FeatureBit remoteFeatures []lnwire.FeatureBit @@ -381,9 +380,6 @@ func createTestFundingManager(t *testing.T, privKey *btcec.PrivateKey, publTxChan := make(chan *wire.MsgTx, 1) shutdownChan := make(chan struct{}) reportScidChan := make(chan struct{}) - updatedPolicies := make( - chan map[wire.OutPoint]models.ForwardingPolicy, 1, - ) wc := &mock.WalletController{ RootKey: alicePrivKey, @@ -541,11 +537,6 @@ func createTestFundingManager(t *testing.T, privKey *btcec.PrivateKey, return nil, nil }, AliasManager: aliasMgr, - UpdateForwardingPolicies: func( - p map[wire.OutPoint]models.ForwardingPolicy) { - - updatedPolicies <- p - }, } for _, op := range options { @@ -570,7 +561,6 @@ func createTestFundingManager(t *testing.T, privKey *btcec.PrivateKey, testDir: tempTestDir, shutdownChannel: shutdownChan, reportScidChan: reportScidChan, - updatedPolicies: updatedPolicies, addr: addr, } @@ -651,12 +641,11 @@ func recreateAliceFundingManager(t *testing.T, alice *testNode) { UpdateLabel: func(chainhash.Hash, string) error { return nil }, - ZombieSweeperInterval: oldCfg.ZombieSweeperInterval, - ReservationTimeout: oldCfg.ReservationTimeout, - OpenChannelPredicate: chainedAcceptor, - DeleteAliasEdge: oldCfg.DeleteAliasEdge, - AliasManager: oldCfg.AliasManager, - UpdateForwardingPolicies: oldCfg.UpdateForwardingPolicies, + ZombieSweeperInterval: oldCfg.ZombieSweeperInterval, + ReservationTimeout: oldCfg.ReservationTimeout, + OpenChannelPredicate: chainedAcceptor, + DeleteAliasEdge: oldCfg.DeleteAliasEdge, + AliasManager: oldCfg.AliasManager, }) require.NoError(t, err, "failed recreating aliceFundingManager") @@ -1145,21 +1134,6 @@ func assertChannelAnnouncements(t *testing.T, alice, bob *testNode, } } - // At this point we should also have gotten a policy update that - // was sent to the switch subsystem. Make sure it contains the - // same values. - var policyUpdate models.ForwardingPolicy - select { - case policyUpdateMap := <-node.updatedPolicies: - require.Len(t, policyUpdateMap, 1) - for _, policy := range policyUpdateMap { - policyUpdate = policy - } - - case <-time.After(time.Second * 5): - t.Fatalf("node didn't send policy update") - } - gotChannelAnnouncement := false gotChannelUpdate := false for _, msg := range announcements { @@ -1188,34 +1162,24 @@ func assertChannelAnnouncements(t *testing.T, alice, bob *testNode, minHtlc = customMinHtlc[j] } require.Equal(t, minHtlc, m.HtlcMinimumMsat) - require.Equal( - t, minHtlc, policyUpdate.MinHTLCOut, - ) // We might expect a custom MaxHltc value. if len(customMaxHtlc) > 0 { maxHtlc = customMaxHtlc[j] } require.Equal(t, maxHtlc, m.HtlcMaximumMsat) - require.Equal(t, maxHtlc, policyUpdate.MaxHTLC) // We might expect a custom baseFee value. if len(baseFees) > 0 { baseFee = baseFees[j] } require.EqualValues(t, baseFee, m.BaseFee) - require.EqualValues( - t, baseFee, policyUpdate.BaseFee, - ) // We might expect a custom feeRate value. if len(feeRates) > 0 { feeRate = feeRates[j] } require.EqualValues(t, feeRate, m.FeeRate) - require.EqualValues( - t, feeRate, policyUpdate.FeeRate, - ) gotChannelUpdate = true } diff --git a/server.go b/server.go index ff8738edf..206ab43f1 100644 --- a/server.go +++ b/server.go @@ -1459,9 +1459,8 @@ func newServer(cfg *Config, listenAddrs []net.Addr, RegisteredChains: cfg.registeredChains, MaxAnchorsCommitFeeRate: chainfee.SatPerKVByte( s.cfg.MaxCommitFeeRateAnchors * 1000).FeePerKWeight(), - DeleteAliasEdge: deleteAliasEdge, - AliasManager: s.aliasMgr, - UpdateForwardingPolicies: s.htlcSwitch.UpdateForwardingPolicies, + DeleteAliasEdge: deleteAliasEdge, + AliasManager: s.aliasMgr, }) if err != nil { return nil, err From 38b106ccf36834caf4516a3080f3692d526be9de Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Tue, 22 Aug 2023 07:04:17 +0800 Subject: [PATCH 11/15] funding: rename `channelReady` to `channelReadySent` for clarity --- funding/manager.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/funding/manager.go b/funding/manager.go index 3701cff7a..bd5b3a188 100644 --- a/funding/manager.go +++ b/funding/manager.go @@ -639,7 +639,7 @@ func (c channelOpeningState) String() string { case markedOpen: return "markedOpen" case channelReadySent: - return "channelReady" + return "channelReadySent" case addedToRouterGraph: return "addedToRouterGraph" default: From d9e046492932b53b41f97142bb5b4d9aeef43c98 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Tue, 22 Aug 2023 07:04:37 +0800 Subject: [PATCH 12/15] funding: defer initial forwarding policy deletion This commit moves the deletion of the initial forwarding policy to the end of `stateStep` to make sure the router has persisted it to disk before the deletion. --- funding/manager.go | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/funding/manager.go b/funding/manager.go index bd5b3a188..aa7e38a29 100644 --- a/funding/manager.go +++ b/funding/manager.go @@ -1261,6 +1261,16 @@ func (f *Manager) stateStep(channel *channeldb.OpenChannel, err) } + // After the fee parameters have been stored in the + // announcement we can delete them from the database. For + // private channels we do not announce the channel policy to + // the network but still need to delete them from the database. + err = f.deleteInitialForwardingPolicy(chanID) + if err != nil { + log.Infof("Could not delete initial policy for chanId "+ + "%x", chanID) + } + log.Debugf("Channel(%v) with ShortChanID %v: successfully "+ "announced", chanID, shortChanID) @@ -3286,15 +3296,6 @@ func (f *Manager) annAfterSixConfs(completeChan *channeldb.OpenChannel, return fmt.Errorf("unable to send node announcement "+ "to peer %x: %v", pubKey, err) } - - // For private channels we do not announce the channel policy - // to the network but still need to delete them from the - // database. - err = f.deleteInitialForwardingPolicy(chanID) - if err != nil { - log.Infof("Could not delete channel fees "+ - "for chanId %x.", chanID) - } } else { // Otherwise, we'll wait until the funding transaction has // reached 6 confirmations before announcing it. @@ -3978,14 +3979,6 @@ func (f *Manager) announceChannel(localIDKey, remoteIDKey *btcec.PublicKey, return err } - // After the fee parameters have been stored in the announcement - // we can delete them from the database. - err = f.deleteInitialForwardingPolicy(chanID) - if err != nil { - log.Infof("Could not delete channel fees for chanId %x.", - chanID) - } - // We only send the channel proof announcement and the node announcement // because addToRouterGraph previously sent the ChannelAnnouncement and // the ChannelUpdate announcement messages. The channel proof and node From 32d82082727254a5c0eeb37b758fd06d679528fa Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Tue, 22 Aug 2023 07:48:30 +0800 Subject: [PATCH 13/15] funding: add new method `handleChannelReadyReceived` This commit is a pure code move. We add a new method `handleChannelReadyReceived` to handle the channel's state change after the remote's channel ready message is received. --- funding/manager.go | 153 ++++++++++++++++++++++++--------------------- 1 file changed, 81 insertions(+), 72 deletions(-) diff --git a/funding/manager.go b/funding/manager.go index aa7e38a29..bba42642c 100644 --- a/funding/manager.go +++ b/funding/manager.go @@ -1151,79 +1151,9 @@ func (f *Manager) stateStep(channel *channeldb.OpenChannel, return nil } - var peerAlias *lnwire.ShortChannelID - if channel.IsZeroConf() { - // We'll need to wait until channel_ready has been - // received and the peer lets us know the alias they - // want to use for the channel. With this information, - // we can then construct a ChannelUpdate for them. - // If an alias does not yet exist, we'll just return, - // letting the next iteration of the loop check again. - var defaultAlias lnwire.ShortChannelID - chanID := lnwire.NewChanIDFromOutPoint( - &channel.FundingOutpoint, - ) - foundAlias, _ := f.cfg.AliasManager.GetPeerAlias( - chanID, - ) - if foundAlias == defaultAlias { - return nil - } - - peerAlias = &foundAlias - } - - err = f.addToRouterGraph(channel, shortChanID, peerAlias, nil) - if err != nil { - return fmt.Errorf("failed adding to "+ - "router graph: %v", err) - } - - // As the channel is now added to the ChannelRouter's topology, - // the channel is moved to the next state of the state machine. - // It will be moved to the last state (actually deleted from - // the database) after the channel is finally announced. - err = f.saveChannelOpeningState( - &channel.FundingOutpoint, addedToRouterGraph, - shortChanID, + return f.handleChannelReadyReceived( + channel, shortChanID, pendingChanID, updateChan, ) - if err != nil { - return fmt.Errorf("error setting channel state to"+ - " addedToRouterGraph: %v", err) - } - - log.Debugf("Channel(%v) with ShortChanID %v: successfully "+ - "added to router graph", chanID, shortChanID) - - // Give the caller a final update notifying them that - // the channel is now open. - // TODO(roasbeef): only notify after recv of channel_ready? - fundingPoint := channel.FundingOutpoint - cp := &lnrpc.ChannelPoint{ - FundingTxid: &lnrpc.ChannelPoint_FundingTxidBytes{ - FundingTxidBytes: fundingPoint.Hash[:], - }, - OutputIndex: fundingPoint.Index, - } - - if updateChan != nil { - upd := &lnrpc.OpenStatusUpdate{ - Update: &lnrpc.OpenStatusUpdate_ChanOpen{ - ChanOpen: &lnrpc.ChannelOpenUpdate{ - ChannelPoint: cp, - }, - }, - PendingChanId: pendingChanID[:], - } - - select { - case updateChan <- upd: - case <-f.quit: - return ErrFundingManagerShuttingDown - } - } - - return nil // The channel was added to the Router's topology, but the channel // announcement was not sent. @@ -3700,6 +3630,84 @@ func (f *Manager) handleChannelReady(peer lnpeer.Peer, } } +// handleChannelReadyReceived is called once the remote's channelReady message +// is received and processed. At this stage, we must have sent out our +// channelReady message, once the remote's channelReady is processed, the +// channel is now active, thus we change its state to `addedToRouterGraph` to +// let the channel start handling routing. +func (f *Manager) handleChannelReadyReceived(channel *channeldb.OpenChannel, + scid *lnwire.ShortChannelID, pendingChanID [32]byte, + updateChan chan<- *lnrpc.OpenStatusUpdate) error { + + chanID := lnwire.NewChanIDFromOutPoint(&channel.FundingOutpoint) + + var peerAlias *lnwire.ShortChannelID + if channel.IsZeroConf() { + // We'll need to wait until channel_ready has been received and + // the peer lets us know the alias they want to use for the + // channel. With this information, we can then construct a + // ChannelUpdate for them. If an alias does not yet exist, + // we'll just return, letting the next iteration of the loop + // check again. + var defaultAlias lnwire.ShortChannelID + chanID := lnwire.NewChanIDFromOutPoint(&channel.FundingOutpoint) + foundAlias, _ := f.cfg.AliasManager.GetPeerAlias(chanID) + if foundAlias == defaultAlias { + return nil + } + + peerAlias = &foundAlias + } + + err := f.addToRouterGraph(channel, scid, peerAlias, nil) + if err != nil { + return fmt.Errorf("failed adding to router graph: %w", err) + } + + // As the channel is now added to the ChannelRouter's topology, the + // channel is moved to the next state of the state machine. It will be + // moved to the last state (actually deleted from the database) after + // the channel is finally announced. + err = f.saveChannelOpeningState( + &channel.FundingOutpoint, addedToRouterGraph, scid, + ) + if err != nil { + return fmt.Errorf("error setting channel state to"+ + " addedToRouterGraph: %w", err) + } + + log.Debugf("Channel(%v) with ShortChanID %v: successfully "+ + "added to router graph", chanID, scid) + + // Give the caller a final update notifying them that the channel is + fundingPoint := channel.FundingOutpoint + cp := &lnrpc.ChannelPoint{ + FundingTxid: &lnrpc.ChannelPoint_FundingTxidBytes{ + FundingTxidBytes: fundingPoint.Hash[:], + }, + OutputIndex: fundingPoint.Index, + } + + if updateChan != nil { + upd := &lnrpc.OpenStatusUpdate{ + Update: &lnrpc.OpenStatusUpdate_ChanOpen{ + ChanOpen: &lnrpc.ChannelOpenUpdate{ + ChannelPoint: cp, + }, + }, + PendingChanId: pendingChanID[:], + } + + select { + case updateChan <- upd: + case <-f.quit: + return ErrFundingManagerShuttingDown + } + } + + return nil +} + // ensureInitialForwardingPolicy ensures that we have an initial forwarding // policy set for the given channel. If we don't, we'll fall back to the default // values. @@ -4695,6 +4703,7 @@ func (f *Manager) saveChannelOpeningState(chanPoint *wire.OutPoint, scratch := make([]byte, 10) byteOrder.PutUint16(scratch[:2], uint16(state)) byteOrder.PutUint64(scratch[2:], shortChanID.ToUint64()) + return f.cfg.ChannelDB.SaveChannelOpeningState( outpointBytes.Bytes(), scratch, ) From 0a6b05d2de5ab01334172dfb805a9b71925b9b9e Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Tue, 22 Aug 2023 08:08:58 +0800 Subject: [PATCH 14/15] funding: catching barrier signal in `receivedChannelReady` There is a race condition, - goroutine_1: performing `stateStep` under `channelReadySent`. Once `receivedChannelReady` returns true, it will mark the channel state as `addedToRouterGraph`, which will delete the initial forwarding policy for private channels. - goroutine_2: performing `handleChannelReady`, which processes the remote's channelReady message. It will ask brontide to `AddNewChannel` in the end. - goroutine_3: performing `handleNewActiveChannel` in brontide, which will query the initial forwarding policy and fail to find it because it's already deleted in goroutine_1. To fix it, we require `receivedChannelReady` to also check that the channel's barrier signal in the map `handleChannelReadyBarriers` doesn't exist, as this signal is removed once `handleChannelReady` finishes adding the channel in brontide. --- funding/manager.go | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/funding/manager.go b/funding/manager.go index bba42642c..9fa860a58 100644 --- a/funding/manager.go +++ b/funding/manager.go @@ -3070,6 +3070,8 @@ func (f *Manager) receivedChannelReady(node *btcec.PublicKey, return false, err } + // If we cannot find the channel, then we haven't processed the + // remote's channelReady message. channel, err := f.cfg.FindChannel(node, chanID) if err != nil { log.Errorf("Unable to locate ChannelID(%v) to determine if "+ @@ -3077,7 +3079,18 @@ func (f *Manager) receivedChannelReady(node *btcec.PublicKey, return false, err } - return channel.RemoteNextRevocation != nil, nil + // If we haven't insert the next revocation point, we haven't finished + // processing the channel ready message. + if channel.RemoteNextRevocation == nil { + return false, nil + } + + // Finally, the barrier signal is removed once we finish + // `handleChannelReady`. If we can still find the signal, we haven't + // finished processing it yet. + _, loaded := f.handleChannelReadyBarriers.Load(chanID) + + return !loaded, nil } // extractAnnounceParams extracts the various channel announcement and update From 6820b08b53acf09fc420eee2ed1610799161ccf2 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Tue, 22 Aug 2023 10:25:52 +0800 Subject: [PATCH 15/15] funding: put assertNoFwdingPolicy in `wait.NoError` --- funding/manager_test.go | 54 ++++++++++++++++++----------------------- 1 file changed, 23 insertions(+), 31 deletions(-) diff --git a/funding/manager_test.go b/funding/manager_test.go index 685d49f20..f9e3b2b18 100644 --- a/funding/manager_test.go +++ b/funding/manager_test.go @@ -1267,13 +1267,12 @@ func assertNoChannelState(t *testing.T, alice, bob *testNode, } func assertNoFwdingPolicy(t *testing.T, alice, bob *testNode, - fundingOutPoint *wire.OutPoint) { + chanID lnwire.ChannelID) { t.Helper() - chandID := lnwire.NewChanIDFromOutPoint(fundingOutPoint) - assertInitialFwdingPolicyNotFound(t, alice, &chandID) - assertInitialFwdingPolicyNotFound(t, bob, &chandID) + assertInitialFwdingPolicyNotFound(t, alice, &chanID) + assertInitialFwdingPolicyNotFound(t, bob, &chanID) } func assertErrChannelNotFound(t *testing.T, node *testNode, @@ -1306,8 +1305,16 @@ func assertInitialFwdingPolicyNotFound(t *testing.T, node *testNode, t.Helper() - _, err := node.fundingMgr.getInitialForwardingPolicy(*chanID) - require.ErrorIs(t, err, channeldb.ErrChannelNotFound) + err := wait.NoError(func() error { + _, err := node.fundingMgr.getInitialForwardingPolicy(*chanID) + if errors.Is(err, channeldb.ErrChannelNotFound) { + return nil + } + + return fmt.Errorf("expected ErrChannelNotFound, got %w", err) + }, wait.DefaultTimeout) + + require.NoError(t, err) } func assertHandleChannelReady(t *testing.T, alice, bob *testNode, @@ -1433,7 +1440,7 @@ func TestFundingManagerNormalWorkflow(t *testing.T) { // The forwarding policy for the channel announcement should // have been deleted from the database, as the channel is announced. - assertNoFwdingPolicy(t, alice, bob, fundingOutPoint) + assertNoFwdingPolicy(t, alice, bob, channelReadyAlice.ChanID) } // TestFundingManagerRejectCSV tests checking of local CSV values against our @@ -1746,7 +1753,7 @@ func TestFundingManagerRestartBehavior(t *testing.T) { // The forwarding policy for the channel announcement should // have been deleted from the database, as the channel is announced. - assertNoFwdingPolicy(t, alice, bob, fundingOutPoint) + assertNoFwdingPolicy(t, alice, bob, channelReadyAlice.ChanID) } // TestFundingManagerOfflinePeer checks that the fundingManager waits for the @@ -1909,7 +1916,7 @@ func TestFundingManagerOfflinePeer(t *testing.T) { // The forwarding policy for the channel announcement should // have been deleted from the database, as the channel is announced. - assertNoFwdingPolicy(t, alice, bob, fundingOutPoint) + assertNoFwdingPolicy(t, alice, bob, channelReadyAlice.ChanID) } // TestFundingManagerPeerTimeoutAfterInitFunding checks that the zombie sweeper @@ -2367,7 +2374,7 @@ func TestFundingManagerReceiveChannelReadyTwice(t *testing.T) { // The forwarding policy for the channel announcement should // have been deleted from the database, as the channel is announced. - assertNoFwdingPolicy(t, alice, bob, fundingOutPoint) + assertNoFwdingPolicy(t, alice, bob, channelReadyAlice.ChanID) } // TestFundingManagerRestartAfterChanAnn checks that the fundingManager properly @@ -2466,7 +2473,7 @@ func TestFundingManagerRestartAfterChanAnn(t *testing.T) { // The forwarding policy for the channel announcement should // have been deleted from the database, as the channel is announced. - assertNoFwdingPolicy(t, alice, bob, fundingOutPoint) + assertNoFwdingPolicy(t, alice, bob, channelReadyAlice.ChanID) } // TestFundingManagerRestartAfterReceivingChannelReady checks that the @@ -2561,7 +2568,7 @@ func TestFundingManagerRestartAfterReceivingChannelReady(t *testing.T) { // The forwarding policy for the channel announcement should // have been deleted from the database, as the channel is announced. - assertNoFwdingPolicy(t, alice, bob, fundingOutPoint) + assertNoFwdingPolicy(t, alice, bob, channelReadyAlice.ChanID) } // TestFundingManagerPrivateChannel tests that if we open a private channel @@ -2685,7 +2692,7 @@ func TestFundingManagerPrivateChannel(t *testing.T) { // The forwarding policy for the channel announcement should // have been deleted from the database. - assertNoFwdingPolicy(t, alice, bob, fundingOutPoint) + assertNoFwdingPolicy(t, alice, bob, channelReadyAlice.ChanID) } // TestFundingManagerPrivateRestart tests that the privacy guarantees granted @@ -2834,7 +2841,7 @@ func TestFundingManagerPrivateRestart(t *testing.T) { // The forwarding policy for the channel announcement should // have been deleted from the database. - assertNoFwdingPolicy(t, alice, bob, fundingOutPoint) + assertNoFwdingPolicy(t, alice, bob, channelReadyAlice.ChanID) } // TestFundingManagerCustomChannelParameters checks that custom requirements we @@ -3257,22 +3264,7 @@ func TestFundingManagerCustomChannelParameters(t *testing.T) { // After the announcement we expect Alice and Bob to have cleared // the fees for the channel from the database. - _, err = alice.fundingMgr.getInitialForwardingPolicy( - fundingSigned.ChanID, - ) - if err != channeldb.ErrChannelNotFound { - err = fmt.Errorf("channel fees were expected to be deleted" + - " but were not") - t.Fatal(err) - } - _, err = bob.fundingMgr.getInitialForwardingPolicy( - fundingSigned.ChanID, - ) - if err != channeldb.ErrChannelNotFound { - err = fmt.Errorf("channel fees were expected to be deleted" + - " but were not") - t.Fatal(err) - } + assertNoFwdingPolicy(t, alice, bob, channelReadyAlice.ChanID) } // TestFundingManagerInvalidChanReserve ensures proper validation is done on @@ -4467,7 +4459,7 @@ func TestFundingManagerZeroConf(t *testing.T) { // The forwarding policy for the channel announcement should // have been deleted from the database, as the channel is announced. - assertNoFwdingPolicy(t, alice, bob, fundingOp) + assertNoFwdingPolicy(t, alice, bob, aliceChannelReady.ChanID) } // TestCommitmentTypeFundmaxSanityCheck was introduced as a way of reminding