Public commit record
58/100 · Thin
What the developer wrote
Authored by ziggie
channeldb: derive channel packagers
Remove the KV forwarding packager from OpenChannel and derive a
ChannelPackager inside the channeldb store methods that need one.
This keeps the backend-specific kvdb transaction helper in channeldb,
so the OpenChannel type no longer carries that dependency toward
chanstate.
✓ Descriptive subject✓ Provides detailed explanatory context
What changed, and why it matters
This commit is a code cleanup: it removes a forwarding-package helper object from the in-memory channel state and instead creates it on demand inside database methods. There is no direct security fix or vulnerability being patched. It mainly reduces coupling between components and updates tests to match the new structure.
Recommended action
No immediate security action required. Treat as normal refactoring; review for functional regressions in forwarding-package persistence during channel state transitions if validating the change.
Security signals we found
01
No security-relevant keywords in commit title or message
02
No changes to cryptographic operations, authentication, or network parsing
03
Refactoring only: field removal and localized object construction
04
Test-only mock change from Packager to Store interface
05
No advisory, CVE, or vendor security disclosure referenced
Evidence from the diff
The change removes the Packager field from channeldb.OpenChannel and derives a NewChannelPackager(channel.ShortChannelID) inside ChannelStateDB methods that need one (AppendRemoteCommitChain, AdvanceCommitChainTail, LoadFwdPkgs, AckAddHtlcs, AckSettleFails, SetFwdFilter, RemoveFwdPkgs, closeChannelSync). It also removes Packager from OpenChannel.Copy and from fetchOpenChannel/fetchChanInfo/MarkAsOpen. Tests are updated to stop setting Packager explicitly, and a test mock is changed from mocking the Packager interface to mocking the chanstate.Store interface. The commit message frames this as keeping backend-specific kvdb helpers inside channeldb and not carrying that dependency toward chanstate.
Changed components
channeldb.OpenChannel structchanneldb.ChannelStateDB forwarding-package methodscontractcourt testshtlcswitch testslnwallet testspeer testsInspect captured patch +47 / −98
diff --git a/channeldb/channel.go b/channeldb/channel.go
index 0e1a04c..7ce0078 100644
--- a/channeldb/channel.go
+++ b/channeldb/channel.go
@@ -742,11 +742,6 @@ type OpenChannel struct {
// implementation of secret store is shachain store.
RevocationStore shachain.Store
- // Packager is used to create and update forwarding packages for this
- // channel, which encodes all necessary information to recover from
- // failures and reforward HTLCs that were not fully processed.
- Packager FwdPackager
-
// FundingTxn is the transaction containing this channel's funding
// outpoint. Upon restarts, this txn will be rebroadcast if the channel
// is found to be pending.
@@ -1457,7 +1452,6 @@ func (c *OpenChannel) MarkAsOpen(openLoc lnwire.ShortChannelID) error {
c.IsPending = false
c.ShortChannelID = openLoc
- c.Packager = NewChannelPackager(openLoc)
return nil
}
@@ -2286,8 +2280,6 @@ func fetchOpenChannel(chanBucket kvdb.RBucket,
err)
}
- channel.Packager = NewChannelPackager(channel.ShortChannelID)
-
return channel, nil
}
@@ -3018,7 +3010,9 @@ func (c *ChannelStateDB) AppendRemoteCommitChain(channel *OpenChannel,
// Mark all of these as being fully processed in our forwarding
// package, which prevents us from reprocessing them after
// startup.
- err = channel.Packager.AckAddHtlcs(tx, diff.AddAcks...)
+ packager := NewChannelPackager(channel.ShortChannelID)
+
+ err = packager.AckAddHtlcs(tx, diff.AddAcks...)
if err != nil {
return err
}
@@ -3028,7 +3022,7 @@ func (c *ChannelStateDB) AppendRemoteCommitChain(channel *OpenChannel,
// prevents the same fails and settles from being retransmitted
// after restarts. The actual fail or settle we need to
// propagate to the remote party is now in the commit diff.
- err = channel.Packager.AckSettleFails(
+ err = packager.AckSettleFails(
tx, diff.SettleFailAcks...,
)
if err != nil {
@@ -3343,7 +3337,9 @@ func (c *ChannelStateDB) AdvanceCommitChainTail(channel *OpenChannel,
// Lastly, we write the forwarding package to disk so that we
// can properly recover from failures and reforward HTLCs that
// have not received a corresponding settle/fail.
- if err := channel.Packager.AddFwdPkg(tx, fwdPkg); err != nil {
+ packager := NewChannelPackager(channel.ShortChannelID)
+ err = packager.AddFwdPkg(tx, fwdPkg)
+ if err != nil {
return err
}
@@ -3484,7 +3480,8 @@ func (c *ChannelStateDB) LoadFwdPkgs(channel *OpenChannel) ([]*FwdPkg,
var fwdPkgs []*FwdPkg
if err := kvdb.View(c.backend, func(tx kvdb.RTx) error {
var err error
- fwdPkgs, err = channel.Packager.LoadFwdPkgs(tx)
+ packager := NewChannelPackager(channel.ShortChannelID)
+ fwdPkgs, err = packager.LoadFwdPkgs(tx)
return err
}, func() {
fwdPkgs = nil
@@ -3512,7 +3509,8 @@ func (c *ChannelStateDB) AckAddHtlcs(channel *OpenChannel,
addRefs ...AddRef) error {
return kvdb.Update(c.backend, func(tx kvdb.RwTx) error {
- return channel.Packager.AckAddHtlcs(tx, addRefs...)
+ packager := NewChannelPackager(channel.ShortChannelID)
+ return packager.AckAddHtlcs(tx, addRefs...)
}, func() {})
}
@@ -3535,7 +3533,8 @@ func (c *ChannelStateDB) AckSettleFails(channel *OpenChannel,
settleFailRefs ...SettleFailRef) error {
return kvdb.Update(c.backend, func(tx kvdb.RwTx) error {
- return channel.Packager.AckSettleFails(tx, settleFailRefs...)
+ packager := NewChannelPackager(channel.ShortChannelID)
+ return packager.AckSettleFails(tx, settleFailRefs...)
}, func() {})
}
@@ -3554,7 +3553,8 @@ func (c *ChannelStateDB) SetFwdFilter(channel *OpenChannel, height uint64,
fwdFilter *PkgFilter) error {
return kvdb.Update(c.backend, func(tx kvdb.RwTx) error {
- return channel.Packager.SetFwdFilter(tx, height, fwdFilter)
+ packager := NewChannelPackager(channel.ShortChannelID)
+ return packager.SetFwdFilter(tx, height, fwdFilter)
}, func() {})
}
@@ -3579,8 +3579,10 @@ func (c *ChannelStateDB) RemoveFwdPkgs(channel *OpenChannel,
heights ...uint64) error {
return kvdb.Update(c.backend, func(tx kvdb.RwTx) error {
+ packager := NewChannelPackager(channel.ShortChannelID)
+
for _, height := range heights {
- err := channel.Packager.RemovePkg(tx, height)
+ err := packager.RemovePkg(tx, height)
if err != nil {
return err
}
@@ -3943,7 +3945,8 @@ func (c *ChannelStateDB) closeChannelSync(channel *OpenChannel,
return err
}
- if err = chanState.Packager.Wipe(tx); err != nil {
+ packager := NewChannelPackager(chanState.ShortChannelID)
+ if err = packager.Wipe(tx); err != nil {
return err
}
@@ -4116,7 +4119,6 @@ func (c *OpenChannel) Copy() *OpenChannel {
RemoteNextRevocation: c.RemoteNextRevocation,
RevocationProducer: c.RevocationProducer,
RevocationStore: c.RevocationStore,
- Packager: c.Packager,
ThawHeight: c.ThawHeight,
LastWasRevoke: c.LastWasRevoke,
RevocationKeyLocator: c.RevocationKeyLocator,
@@ -4707,8 +4709,6 @@ func fetchChanInfo(chanBucket kvdb.RBucket, channel *OpenChannel) error {
// open channel.
amendOpenChannelTlvData(channel, auxData)
- channel.Packager = NewChannelPackager(channel.ShortChannelID)
-
// Finally, read the optional shutdown scripts.
if err := getOptionalUpfrontShutdownScript(
chanBucket, localUpfrontShutdownKey, &channel.LocalShutdownScript,
diff --git a/channeldb/channel_test.go b/channeldb/channel_test.go
index 115794b..a6a870c 100644
--- a/channeldb/channel_test.go
+++ b/channeldb/channel_test.go
@@ -414,7 +414,6 @@ func createTestChannelState(t *testing.T, cdb *ChannelStateDB) *OpenChannel {
RevocationProducer: producer,
RevocationStore: store,
Db: cdb,
- Packager: NewChannelPackager(chanID),
FundingTxn: channels.TestFundingTx,
ThawHeight: uint32(defaultPendingHeight),
InitialLocalBalance: lnwire.MilliSatoshi(9000),
@@ -938,7 +937,9 @@ func TestChannelStateTransition(t *testing.T) {
}
// At this point, we should have 2 forwarding packages added.
- fwdPkgs := loadFwdPkgs(t, cdb.backend, channel.Packager)
+ fwdPkgs := loadFwdPkgs(
+ t, cdb.backend, NewChannelPackager(channel.ShortChanID()),
+ )
require.Len(t, fwdPkgs, 2, "wrong number of forwarding packages")
// Now attempt to delete the channel from the database.
@@ -973,7 +974,9 @@ func TestChannelStateTransition(t *testing.T) {
}
// All forwarding packages of this channel has been deleted too.
- fwdPkgs = loadFwdPkgs(t, cdb.backend, channel.Packager)
+ fwdPkgs = loadFwdPkgs(
+ t, cdb.backend, NewChannelPackager(channel.ShortChanID()),
+ )
require.Empty(t, fwdPkgs, "no forwarding packages should exist")
}
@@ -1417,16 +1420,6 @@ func TestRefresh(t *testing.T) {
"updated before refreshing short_chan_id")
}
- // Now that the receiver's short channel id has been updated, check to
- // ensure that the channel packager's source has been updated as well.
- // This ensures that the packager will read and write to buckets
- // corresponding to the new short chan id, instead of the prior.
- if state.Packager.(*ChannelPackager).source != chanOpenLoc {
- t.Fatalf("channel packager source was not updated: want %v, "+
- "got %v", chanOpenLoc,
- state.Packager.(*ChannelPackager).source)
- }
-
// Now, refresh the state of the pending channel.
err = pendingChannel.Refresh()
require.NoError(t, err, "unable to refresh short_chan_id")
@@ -1439,21 +1432,15 @@ func TestRefresh(t *testing.T) {
pendingChannel.ShortChanID())
}
- // Check to ensure that the _other_ OpenChannel channel packager's
- // source has also been updated after the refresh. This ensures that the
- // other packagers will read and write to buckets corresponding to the
- // updated short chan id.
- if pendingChannel.Packager.(*ChannelPackager).source != chanOpenLoc {
- t.Fatalf("channel packager source was not updated: want %v, "+
- "got %v", chanOpenLoc,
- pendingChannel.Packager.(*ChannelPackager).source)
- }
-
// Check to ensure that this channel is no longer pending and this field
// is up to date.
if pendingChannel.IsPending {
t.Fatalf("channel pending state wasn't updated: want false got true")
}
+
+ require.Equal(
+ t, chanOpenLoc, NewChannelPackager(state.ShortChanID()).source,
+ )
}
// TestCloseInitiator tests the setting of close initiator statuses for
diff --git a/contractcourt/breach_arbitrator_test.go b/contractcourt/breach_arbitrator_test.go
index a000a6c..c8fa13f 100644
--- a/contractcourt/breach_arbitrator_test.go
+++ b/contractcourt/breach_arbitrator_test.go
@@ -2330,7 +2330,6 @@ func createInitChannels(t *testing.T) (
LocalCommitment: aliceCommit,
RemoteCommitment: aliceCommit,
Db: dbAlice.ChannelStateDB(),
- Packager: channeldb.NewChannelPackager(shortChanID),
FundingTxn: channels.TestFundingTx,
}
bobChannelState := &channeldb.OpenChannel{
@@ -2348,7 +2347,6 @@ func createInitChannels(t *testing.T) (
LocalCommitment: bobCommit,
RemoteCommitment: bobCommit,
Db: dbBob.ChannelStateDB(),
- Packager: channeldb.NewChannelPackager(shortChanID),
}
aliceSigner := input.NewMockSigner(
diff --git a/htlcswitch/link_test.go b/htlcswitch/link_test.go
index 573c0bb..ecd5ea2 100644
--- a/htlcswitch/link_test.go
+++ b/htlcswitch/link_test.go
@@ -25,6 +25,7 @@ import (
sphinx "github.com/lightningnetwork/lightning-onion"
"github.com/lightningnetwork/lnd/build"
"github.com/lightningnetwork/lnd/channeldb"
+ cstate "github.com/lightningnetwork/lnd/chanstate"
"github.com/lightningnetwork/lnd/contractcourt"
"github.com/lightningnetwork/lnd/fn/v2"
"github.com/lightningnetwork/lnd/graph/db/models"
@@ -32,7 +33,6 @@ import (
"github.com/lightningnetwork/lnd/htlcswitch/hop"
"github.com/lightningnetwork/lnd/input"
invpkg "github.com/lightningnetwork/lnd/invoices"
- "github.com/lightningnetwork/lnd/kvdb"
"github.com/lightningnetwork/lnd/lnpeer"
"github.com/lightningnetwork/lnd/lntest/wait"
"github.com/lightningnetwork/lnd/lntypes"
@@ -5768,42 +5768,20 @@ func TestChannelLinkCleanupSpuriousResponses(t *testing.T) {
}
}
-type mockPackager struct {
- failLoadFwdPkgs bool
+// mockFailLoadFwdPkgStore wraps a real channel state store and overrides only
+// LoadFwdPkgs. This lets the link startup test inject a forwarding-package
+// load failure through OpenChannel.Db without replacing the rest of the store.
+type mockFailLoadFwdPkgStore struct {
+ cstate.Store[*channeldb.OpenChannel]
}
-func (*mockPackager) AddFwdPkg(tx kvdb.RwTx, fwdPkg *channeldb.FwdPkg) error {
- return nil
-}
-
-func (*mockPackager) SetFwdFilter(tx kvdb.RwTx, height uint64,
- fwdFilter *channeldb.PkgFilter) error {
- return nil
-}
-
-func (*mockPackager) AckAddHtlcs(tx kvdb.RwTx,
- addRefs ...channeldb.AddRef) error {
- return nil
-}
-
-func (m *mockPackager) LoadFwdPkgs(tx kvdb.RTx) ([]*channeldb.FwdPkg, error) {
- if m.failLoadFwdPkgs {
- return nil, fmt.Errorf("failing LoadFwdPkgs")
- }
- return nil, nil
-}
-
-func (*mockPackager) RemovePkg(tx kvdb.RwTx, height uint64) error {
- return nil
-}
+// LoadFwdPkgs fails the forwarding-package load to exercise link startup
+// failure handling while all other store methods delegate to the embedded
+// store.
+func (m *mockFailLoadFwdPkgStore) LoadFwdPkgs(
+ *channeldb.OpenChannel) ([]*channeldb.FwdPkg, error) {
-func (*mockPackager) Wipe(tx kvdb.RwTx) error {
- return nil
-}
-
-func (*mockPackager) AckSettleFails(tx kvdb.RwTx,
- settleFailRefs ...channeldb.SettleFailRef) error {
- return nil
+ return nil, fmt.Errorf("failing LoadFwdPkgs")
}
// TestChannelLinkFail tests that we will fail the channel, and force close the
@@ -5879,10 +5857,10 @@ func TestChannelLinkFail(t *testing.T) {
func(c *channelLink) {
// We make the call to resolveFwdPkgs fail by
// making the underlying forwarder fail.
- pkg := &mockPackager{
- failLoadFwdPkgs: true,
+ state := c.channel.State()
+ state.Db = &mockFailLoadFwdPkgStore{
+ Store: state.Db,
}
- c.channel.State().Packager = pkg
},
func(*testing.T, *Switch, *channelLink,
*lnwallet.LightningChannel) {
diff --git a/htlcswitch/test_utils.go b/htlcswitch/test_utils.go
index 110d97b..f0ebb5c 100644
--- a/htlcswitch/test_utils.go
+++ b/htlcswitch/test_utils.go
@@ -320,7 +320,6 @@ func createTestChannel(t *testing.T, alicePrivKey, bobPrivKey []byte,
RemoteCommitment: aliceCommit,
ShortChannelID: chanID,
Db: dbAlice.ChannelStateDB(),
- Packager: channeldb.NewChannelPackager(chanID),
FundingTxn: channels.TestFundingTx,
}
@@ -339,7 +338,6 @@ func createTestChannel(t *testing.T, alicePrivKey, bobPrivKey []byte,
RemoteCommitment: bobCommit,
ShortChannelID: chanID,
Db: dbBob.ChannelStateDB(),
- Packager: channeldb.NewChannelPackager(chanID),
}
if err := aliceChannelState.SyncPending(bobAddr, broadcastHeight); err != nil {
diff --git a/lnwallet/taproot_test_vectors_test.go b/lnwallet/taproot_test_vectors_test.go
index fcf3fd2..23c72c4 100644
--- a/lnwallet/taproot_test_vectors_test.go
+++ b/lnwallet/taproot_test_vectors_test.go
@@ -906,10 +906,7 @@ func createTaprootTestChannelsForVectors(tc *taprootTestContext,
LocalCommitment: remoteCommit,
RemoteCommitment: remoteCommit,
Db: dbRemote.ChannelStateDB(),
- Packager: channeldb.NewChannelPackager(
- shortChanID,
- ),
- FundingTxn: fundingTx,
+ FundingTxn: fundingTx,
}
localChannelState := &channeldb.OpenChannel{
LocalChanCfg: localCfg,
@@ -926,10 +923,7 @@ func createTaprootTestChannelsForVectors(tc *taprootTestContext,
LocalCommitment: localCommit,
RemoteCommitment: localCommit,
Db: dbLocal.ChannelStateDB(),
- Packager: channeldb.NewChannelPackager(
- shortChanID,
- ),
- FundingTxn: fundingTx,
+ FundingTxn: fundingTx,
}
// Create mock signers with all deterministic keys. The funding key must
diff --git a/lnwallet/test_utils.go b/lnwallet/test_utils.go
index 1edc92e..5723be7 100644
--- a/lnwallet/test_utils.go
+++ b/lnwallet/test_utils.go
@@ -323,7 +323,6 @@ func CreateTestChannels(t *testing.T, chanType channeldb.ChannelType,
LocalCommitment: aliceLocalCommit,
RemoteCommitment: aliceRemoteCommit,
Db: dbAlice.ChannelStateDB(),
- Packager: channeldb.NewChannelPackager(shortChanID),
FundingTxn: testTx,
}
bobChannelState := &channeldb.OpenChannel{
@@ -341,7 +340,6 @@ func CreateTestChannels(t *testing.T, chanType channeldb.ChannelType,
LocalCommitment: bobLocalCommit,
RemoteCommitment: bobRemoteCommit,
Db: dbBob.ChannelStateDB(),
- Packager: channeldb.NewChannelPackager(shortChanID),
}
// If the channel type has a tapscript root, then we'll also specify
diff --git a/lnwallet/transactions_test.go b/lnwallet/transactions_test.go
index 41fb23b..6f941ad 100644
--- a/lnwallet/transactions_test.go
+++ b/lnwallet/transactions_test.go
@@ -982,7 +982,6 @@ func createTestChannelsForVectors(tc *testContext, chanType channeldb.ChannelTyp
LocalCommitment: remoteCommit,
RemoteCommitment: remoteCommit,
Db: dbRemote.ChannelStateDB(),
- Packager: channeldb.NewChannelPackager(shortChanID),
FundingTxn: tc.fundingTx.MsgTx(),
}
localChannelState := &channeldb.OpenChannel{
@@ -1000,7 +999,6 @@ func createTestChannelsForVectors(tc *testContext, chanType channeldb.ChannelTyp
LocalCommitment: localCommit,
RemoteCommitment: localCommit,
Db: dbLocal.ChannelStateDB(),
- Packager: channeldb.NewChannelPackager(shortChanID),
FundingTxn: tc.fundingTx.MsgTx(),
}
diff --git a/peer/test_utils.go b/peer/test_utils.go
index a823b82..f2c0097 100644
--- a/peer/test_utils.go
+++ b/peer/test_utils.go
@@ -253,7 +253,6 @@ func createTestPeerWithChannel(t *testing.T, updateChan func(a,
LocalCommitment: aliceCommit,
RemoteCommitment: aliceCommit,
Db: dbAlice.ChannelStateDB(),
- Packager: channeldb.NewChannelPackager(shortChanID),
FundingTxn: channels.TestFundingTx,
}
bobChannelState := &channeldb.OpenChannel{
@@ -270,7 +269,6 @@ func createTestPeerWithChannel(t *testing.T, updateChan func(a,
LocalCommitment: bobCommit,
RemoteCommitment: bobCommit,
Db: dbBob.ChannelStateDB(),
- Packager: channeldb.NewChannelPackager(shortChanID),
}
// Set custom values on the channel states.
Risk score
Our methodology →Why this scored 18/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.