Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make MaxWaitNumBlocksFundingConf Configurable for itest #9562

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions docs/release-notes/release-notes-0.19.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,11 @@
* [Add ability](https://github.com/lightningnetwork/lnd/pull/8998) to paginate
wallet transactions.

* [Make](https://github.com/lightningnetwork/lnd/pull/9562)
`MaxWaitNumBlocksFundingConf` configurable, allowing integration/development
tests to set a lower value for faster funding confirmation timeout while
keeping the default of 2016 blocks for production stability.

## RPC Additions

* [Add a new rpc endpoint](https://github.com/lightningnetwork/lnd/pull/8843)
Expand Down
26 changes: 19 additions & 7 deletions funding/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"github.com/lightningnetwork/lnd/input"
"github.com/lightningnetwork/lnd/keychain"
"github.com/lightningnetwork/lnd/labels"
"github.com/lightningnetwork/lnd/lncfg"
"github.com/lightningnetwork/lnd/lnpeer"
"github.com/lightningnetwork/lnd/lnrpc"
"github.com/lightningnetwork/lnd/lnutils"
Expand Down Expand Up @@ -101,11 +102,6 @@ const (

msgBufferSize = 50

// MaxWaitNumBlocksFundingConf is the maximum number of blocks to wait
// for the funding transaction to be confirmed before forgetting
// channels that aren't initiated by us. 2016 blocks is ~2 weeks.
MaxWaitNumBlocksFundingConf = 2016

// pendingChansLimit is the maximum number of pending channels that we
// can have. After this point, pending channel opens will start to be
// rejected.
Expand Down Expand Up @@ -339,6 +335,11 @@ type DevConfig struct {
// remote node's channel ready message once the channel as been marked
// as `channelReadySent`.
ProcessChannelReadyWait time.Duration

// MaxWaitNumBlocksFundingConf is the maximum number of blocks to wait
// for the funding transaction to be confirmed before forgetting
// channels that aren't initiated by us.
MaxWaitNumBlocksFundingConf uint32
}

// Config defines the configuration for the FundingManager. All elements
Expand Down Expand Up @@ -3164,9 +3165,20 @@ func (f *Manager) waitForTimeout(completeChan *channeldb.OpenChannel,

defer epochClient.Cancel()

// For the waitBlocksForFundingConf different values are set in case we
// are in a dev environment so enhance test capabilities.
var waitBlocksForFundingConf uint32 = lncfg.
DefaultMaxWaitNumBlocksFundingConf

// Get the waitBlocksForFundingConf. If we are not in development mode,
// this would be DefaultMaxWaitNumBlocksFundingConf.
if lncfg.IsDevBuild() {
waitBlocksForFundingConf = f.cfg.Dev.MaxWaitNumBlocksFundingConf
}

// On block maxHeight we will cancel the funding confirmation wait.
broadcastHeight := completeChan.BroadcastHeight()
maxHeight := broadcastHeight + MaxWaitNumBlocksFundingConf
maxHeight := broadcastHeight + waitBlocksForFundingConf
for {
select {
case epoch, ok := <-epochClient.Epochs:
Expand All @@ -3182,7 +3194,7 @@ func (f *Manager) waitForTimeout(completeChan *channeldb.OpenChannel,
log.Warnf("Waited for %v blocks without "+
"seeing funding transaction confirmed,"+
" cancelling.",
MaxWaitNumBlocksFundingConf)
waitBlocksForFundingConf)

// Notify the caller of the timeout.
close(timeoutChan)
Expand Down
20 changes: 12 additions & 8 deletions funding/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2334,14 +2334,15 @@ func TestFundingManagerFundingTimeout(t *testing.T) {
// mine 2016-1, and check that it is still pending.
bob.mockNotifier.epochChan <- &chainntnfs.BlockEpoch{
Height: fundingBroadcastHeight +
MaxWaitNumBlocksFundingConf - 1,
lncfg.DefaultMaxWaitNumBlocksFundingConf - 1,
}

// Bob should still be waiting for the channel to open.
assertNumPendingChannelsRemains(t, bob, 1)

bob.mockNotifier.epochChan <- &chainntnfs.BlockEpoch{
Height: fundingBroadcastHeight + MaxWaitNumBlocksFundingConf,
Height: fundingBroadcastHeight +
lncfg.DefaultMaxWaitNumBlocksFundingConf,
}

// Bob should have sent an Error message to Alice.
Expand Down Expand Up @@ -2387,30 +2388,33 @@ func TestFundingManagerFundingNotTimeoutInitiator(t *testing.T) {
t.Fatalf("alice did not publish funding tx")
}

// Increase the height to 1 minus the MaxWaitNumBlocksFundingConf
// Increase the height to 1 minus the DefaultMaxWaitNumBlocksFundingConf
// height.
alice.mockNotifier.epochChan <- &chainntnfs.BlockEpoch{
Height: fundingBroadcastHeight +
MaxWaitNumBlocksFundingConf - 1,
lncfg.DefaultMaxWaitNumBlocksFundingConf - 1,
}

bob.mockNotifier.epochChan <- &chainntnfs.BlockEpoch{
Height: fundingBroadcastHeight +
MaxWaitNumBlocksFundingConf - 1,
lncfg.DefaultMaxWaitNumBlocksFundingConf - 1,
}

// Assert both and Alice and Bob still have 1 pending channels.
assertNumPendingChannelsRemains(t, alice, 1)

assertNumPendingChannelsRemains(t, bob, 1)

// Increase both Alice and Bob to MaxWaitNumBlocksFundingConf height.
// Increase both Alice and Bob to DefaultMaxWaitNumBlocksFundingConf
// height.
alice.mockNotifier.epochChan <- &chainntnfs.BlockEpoch{
Height: fundingBroadcastHeight + MaxWaitNumBlocksFundingConf,
Height: fundingBroadcastHeight +
lncfg.DefaultMaxWaitNumBlocksFundingConf,
}

bob.mockNotifier.epochChan <- &chainntnfs.BlockEpoch{
Height: fundingBroadcastHeight + MaxWaitNumBlocksFundingConf,
Height: fundingBroadcastHeight +
lncfg.DefaultMaxWaitNumBlocksFundingConf,
}

// Since Alice was the initiator, the channel should not have timed out.
Expand Down
4 changes: 4 additions & 0 deletions itest/list_on_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -670,6 +670,10 @@ var allTestCases = []*lntest.TestCase{
Name: "fee replacement",
TestFunc: testFeeReplacement,
},
{
Name: "funding manager funding timeout",
TestFunc: testFundingManagerFundingTimeout,
},
}

// appendPrefixed is used to add a prefix to each test name in the subtests
Expand Down
55 changes: 54 additions & 1 deletion itest/lnd_open_channel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/btcsuite/btcd/chaincfg/chainhash"
"github.com/lightningnetwork/lnd/chainreg"
"github.com/lightningnetwork/lnd/funding"
"github.com/lightningnetwork/lnd/lncfg"
"github.com/lightningnetwork/lnd/lnrpc"
"github.com/lightningnetwork/lnd/lnrpc/routerrpc"
"github.com/lightningnetwork/lnd/lntest"
Expand Down Expand Up @@ -844,7 +845,7 @@ func testFundingExpiryBlocksOnPending(ht *lntest.HarnessTest) {
// blocks and verify the value of FundingExpiryBlock at each step.
const numEmptyBlocks = 3
for i := int32(0); i < numEmptyBlocks; i++ {
expectedVal := funding.MaxWaitNumBlocksFundingConf - i
expectedVal := lncfg.DefaultMaxWaitNumBlocksFundingConf - i
pending := ht.AssertNumPendingOpenChannels(alice, 1)
require.Equal(ht, expectedVal, pending[0].FundingExpiryBlocks)
pending = ht.AssertNumPendingOpenChannels(bob, 1)
Expand Down Expand Up @@ -967,3 +968,55 @@ func testOpenChannelLockedBalance(ht *lntest.HarnessTest) {
// Finally, we check to make sure the balance is unlocked again.
ht.AssertWalletLockedBalance(alice, 0)
}

// testFundingManagerFundingTimeout tests that after an OpenChannel, and before
// the funding transaction is confirmed, if a user is not the channel initiator,
// the channel is forgotten after waitBlocksForFundingConf.
func testFundingManagerFundingTimeout(ht *lntest.HarnessTest) {
// Set the maximum wait blocks for funding confirmation.
waitBlocksForFundingConf := 10

// Create nodes for testing, ensuring Alice has sufficient initial
// funds.
alice := ht.NewNodeWithCoins("Alice", nil)
bob := ht.NewNode("Bob", nil)

// Restart Bob with the custom configuration for funding confirmation
// timeout.
ht.RestartNodeWithExtraArgs(bob, []string{
"--dev.maxwaitnumblocksfundingconf=10",
})

// Ensure Alice and Bob are connected.
ht.EnsureConnected(alice, bob)

// Open the channel between Alice and Bob. This runs through the process
// up until the funding transaction is broadcasted.
ht.OpenChannelAssertPending(alice, bob, lntest.OpenChannelParams{
Amt: 500000,
PushAmt: 0,
})

// At this point, both nodes have a pending channel waiting for the
// funding transaction to be confirmed.
ht.AssertNumPendingOpenChannels(alice, 1)
ht.AssertNumPendingOpenChannels(bob, 1)

// We expect Bob to forget the channel after waitBlocksForFundingConf
// blocks, so mine waitBlocksForFundingConf-1, and check that it is
// still pending.
ht.MineEmptyBlocks(waitBlocksForFundingConf - 1)
ht.AssertNumPendingOpenChannels(bob, 1)

// Now mine one additional block to reach waitBlocksForFundingConf.
ht.MineEmptyBlocks(1)

// Bob should now have forgotten the channel.
ht.AssertNumPendingOpenChannels(bob, 0)

// Since Alice was the initiator, her pending channel should remain.
ht.AssertNumPendingOpenChannels(alice, 1)

// Cleanup the mempool by mining blocks.
ht.MineBlocksAndAssertNumTxes(6, 1)
}
5 changes: 5 additions & 0 deletions lncfg/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,11 @@ const (
// DefaultZombieSweeperInterval is the default time interval at which
// unfinished (zombiestate) open channel flows are purged from memory.
DefaultZombieSweeperInterval = 1 * time.Minute

// DefaultMaxWaitNumBlocksFundingConf is the maximum number of blocks to
// wait for the funding transaction to confirm before forgetting
// channels that aren't initiated by us. 2016 blocks is ~2 weeks.
DefaultMaxWaitNumBlocksFundingConf = 2016
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this defined twice now? I think we should just use this value everywhere now and remove the one in funding. Unless that will cause a circular package dependency, in which case we need to find a good place for it.

)

// CleanAndExpandPath expands environment variables and leading ~ in the
Expand Down
6 changes: 6 additions & 0 deletions lncfg/dev.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,9 @@ func (d *DevConfig) GetReservationTimeout() time.Duration {
func (d *DevConfig) GetZombieSweeperInterval() time.Duration {
return DefaultZombieSweeperInterval
}

// GetMaxWaitNumBlocksFundingConf returns the config value for
// `MaxWaitNumBlocksFundingConf`.
func (d *DevConfig) GetMaxWaitNumBlocksFundingConf() uint32 {
return DefaultMaxWaitNumBlocksFundingConf
}
19 changes: 15 additions & 4 deletions lncfg/dev_integration.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,11 @@ func IsDevBuild() bool {
//
//nolint:ll
type DevConfig struct {
ProcessChannelReadyWait time.Duration `long:"processchannelreadywait" description:"Time to sleep before processing remote node's channel_ready message."`
ReservationTimeout time.Duration `long:"reservationtimeout" description:"The maximum time we keep a pending channel open flow in memory."`
ZombieSweeperInterval time.Duration `long:"zombiesweeperinterval" description:"The time interval at which channel opening flows are evaluated for zombie status."`
UnsafeDisconnect bool `long:"unsafedisconnect" description:"Allows the rpcserver to intentionally disconnect from peers with open channels."`
ProcessChannelReadyWait time.Duration `long:"processchannelreadywait" description:"Time to sleep before processing remote node's channel_ready message."`
ReservationTimeout time.Duration `long:"reservationtimeout" description:"The maximum time we keep a pending channel open flow in memory."`
ZombieSweeperInterval time.Duration `long:"zombiesweeperinterval" description:"The time interval at which channel opening flows are evaluated for zombie status."`
UnsafeDisconnect bool `long:"unsafedisconnect" description:"Allows the rpcserver to intentionally disconnect from peers with open channels."`
MaxWaitNumBlocksFundingConf uint32 `long:"maxwaitnumblocksfundingconf" description:"Maximum blocks to wait for funding confirmation before discarding non-initiated channels."`
}

// ChannelReadyWait returns the config value `ProcessChannelReadyWait`.
Expand Down Expand Up @@ -54,3 +55,13 @@ func (d *DevConfig) GetZombieSweeperInterval() time.Duration {
func (d *DevConfig) GetUnsafeDisconnect() bool {
return d.UnsafeDisconnect
}

// GetMaxWaitNumBlocksFundingConf returns the config value for
// `MaxWaitNumBlocksFundingConf`.
func (d *DevConfig) GetMaxWaitNumBlocksFundingConf() uint32 {
if d.MaxWaitNumBlocksFundingConf == 0 {
return DefaultMaxWaitNumBlocksFundingConf
}

return d.MaxWaitNumBlocksFundingConf
}
16 changes: 14 additions & 2 deletions rpcserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -3850,9 +3850,21 @@ func (r *rpcServer) fetchPendingOpenChannels() (pendingOpenChannels, error) {
commitBaseWeight := blockchain.GetTransactionWeight(utx)
commitWeight := commitBaseWeight + witnessWeight

// For the waitBlocksForFundingConf different values are set in
// case we are in dev environment so enhance test capabilities.
var waitBlocksForFundingConf uint32 = lncfg.
DefaultMaxWaitNumBlocksFundingConf

// Get the waitBlocksForFundingConf. If we are not in
// development mode, this would be nil.
if lncfg.IsDevBuild() {
waitBlocksForFundingConf = r.cfg.Dev.
GetMaxWaitNumBlocksFundingConf()
}

// FundingExpiryBlocks is the distance from the current block
// height to the broadcast height + MaxWaitNumBlocksFundingConf.
maxFundingHeight := funding.MaxWaitNumBlocksFundingConf +
// height to the broadcast height + waitBlocksForFundingConf.
maxFundingHeight := waitBlocksForFundingConf +
pendingChan.BroadcastHeight()
fundingExpiryBlocks := int32(maxFundingHeight) - currentHeight

Expand Down
2 changes: 2 additions & 0 deletions server.go
Original file line number Diff line number Diff line change
Expand Up @@ -1449,6 +1449,8 @@ func newServer(cfg *Config, listenAddrs []net.Addr,
if lncfg.IsDevBuild() {
devCfg = &funding.DevConfig{
ProcessChannelReadyWait: cfg.Dev.ChannelReadyWait(),
MaxWaitNumBlocksFundingConf: cfg.Dev.
GetMaxWaitNumBlocksFundingConf(),
}

reservationTimeout = cfg.Dev.GetReservationTimeout()
Expand Down
Loading