multi: initialize block height in New, not in Run

If Run sets the current height field, it is technically a race between Run and
other methods reading the field. Setting in New is a safer option.

Removed a check that height is not 0 from static address manager.

Author: starius

instantout/manager.go

@@ -41,18 +41,17 @@ type Manager struct {
 }
 
 // NewInstantOutManager creates a new instantout manager.
-func NewInstantOutManager(cfg *Config) *Manager {
+func NewInstantOutManager(cfg *Config, height int32) *Manager {
 	return &Manager{
 		cfg:               cfg,
 		activeInstantOuts: make(map[lntypes.Hash]*FSM),
 		blockEpochChan:    make(chan int32),
+		currentHeight:     height,
 	}
 }
 
 // Run runs the instantout manager.
-func (m *Manager) Run(ctx context.Context, initChan chan struct{},
-	height int32) error {
-
+func (m *Manager) Run(ctx context.Context, initChan chan struct{}) error {
 	log.Debugf("Starting instantout manager")
 	defer func() {
 		log.Debugf("Stopping instantout manager")
@@ -62,7 +61,6 @@ func (m *Manager) Run(ctx context.Context, initChan chan struct{},
 	defer cancel()
 
 	m.runCtx = runCtx
-	m.currentHeight = height
 
 	err := m.recoverInstantOuts(runCtx)
 	if err != nil {

loopd/daemon.go

@@ -446,6 +446,14 @@ func (d *Daemon) initialize(withMacaroonService bool) error {
 		chainParams,
 	)
 
+	// We need to know the current block height to properly initialize
+	// managers.
+	getInfo, err := d.lnd.Client.GetInfo(d.mainCtx)
+	if err != nil {
+		return fmt.Errorf("failed to get current block height: %w", err)
+	}
+	blockHeight := getInfo.BlockHeight
+
 	// If we're running an asset client, we'll log something here.
 	if d.assetClient != nil {
 		getInfo, err := d.assetClient.GetInfo(
@@ -580,7 +588,7 @@ func (d *Daemon) initialize(withMacaroonService bool) error {
 		ChainParams:   d.lnd.ChainParams,
 		ChainNotifier: d.lnd.ChainNotifier,
 	}
-	staticAddressManager = address.NewManager(addrCfg)
+	staticAddressManager = address.NewManager(addrCfg, int32(blockHeight))
 
 	// Static address deposit manager setup.
 	depositStore := deposit.NewSqlStore(baseDb)
@@ -606,7 +614,7 @@ func (d *Daemon) initialize(withMacaroonService bool) error {
 		ChainNotifier:             d.lnd.ChainNotifier,
 		Signer:                    d.lnd.Signer,
 	}
-	withdrawalManager = withdraw.NewManager(withdrawalCfg)
+	withdrawalManager = withdraw.NewManager(withdrawalCfg, blockHeight)
 
 	// Static address loop-in manager setup.
 	staticAddressLoopInStore := loopin.NewSqlStore(
@@ -631,7 +639,7 @@ func (d *Daemon) initialize(withMacaroonService bool) error {
 		ValidateLoopInContract:               loop.ValidateLoopInContract,
 		MaxStaticAddrHtlcFeePercentage:       d.cfg.MaxStaticAddrHtlcFeePercentage,
 		MaxStaticAddrHtlcBackupFeePercentage: d.cfg.MaxStaticAddrHtlcBackupFeePercentage,
-	})
+	}, blockHeight)
 
 	var (
 		reservationManager *reservation.Manager
@@ -674,7 +682,7 @@ func (d *Daemon) initialize(withMacaroonService bool) error {
 		}
 
 		instantOutManager = instantout.NewInstantOutManager(
-			instantOutConfig,
+			instantOutConfig, int32(blockHeight),
 		)
 	}
 
@@ -769,19 +777,11 @@ func (d *Daemon) initialize(withMacaroonService bool) error {
 		go func() {
 			defer d.wg.Done()
 
-			// We need to know the current block height to properly
-			// initialize the reservation manager.
-			getInfo, err := d.lnd.Client.GetInfo(d.mainCtx)
-			if err != nil {
-				d.internalErrChan <- err
-				return
-			}
-
 			infof("Starting reservation manager")
 			defer infof("Reservation manager stopped")
 
-			err = d.reservationManager.Run(
-				d.mainCtx, int32(getInfo.BlockHeight), initChan,
+			err := d.reservationManager.Run(
+				d.mainCtx, int32(blockHeight), initChan,
 			)
 			if err != nil && !errors.Is(err, context.Canceled) {
 				d.internalErrChan <- err
@@ -809,18 +809,10 @@ func (d *Daemon) initialize(withMacaroonService bool) error {
 		go func() {
 			defer d.wg.Done()
 
-			getInfo, err := d.lnd.Client.GetInfo(d.mainCtx)
-			if err != nil {
-				d.internalErrChan <- err
-				return
-			}
-
 			infof("Starting instantout manager")
 			defer infof("Instantout manager stopped")
 
-			err = d.instantOutManager.Run(
-				d.mainCtx, initChan, int32(getInfo.BlockHeight),
-			)
+			err := d.instantOutManager.Run(d.mainCtx, initChan)
 			if err != nil && !errors.Is(err, context.Canceled) {
 				d.internalErrChan <- err
 			}
@@ -847,7 +839,7 @@ func (d *Daemon) initialize(withMacaroonService bool) error {
 			defer d.wg.Done()
 
 			infof("Starting static address manager...")
-			err = staticAddressManager.Run(d.mainCtx)
+			err := staticAddressManager.Run(d.mainCtx)
 			if err != nil && !errors.Is(context.Canceled, err) {
 				d.internalErrChan <- err
 			}
@@ -862,7 +854,7 @@ func (d *Daemon) initialize(withMacaroonService bool) error {
 			defer d.wg.Done()
 
 			infof("Starting static address deposit manager...")
-			err = depositManager.Run(d.mainCtx)
+			err := depositManager.Run(d.mainCtx)
 			if err != nil && !errors.Is(context.Canceled, err) {
 				d.internalErrChan <- err
 			}
@@ -877,17 +869,9 @@ func (d *Daemon) initialize(withMacaroonService bool) error {
 		go func() {
 			defer d.wg.Done()
 
-			// Lnd's GetInfo call supplies us with the current block
-			// height.
-			info, err := d.lnd.Client.GetInfo(d.mainCtx)
-			if err != nil {
-				d.internalErrChan <- err
-				return
-			}
-
 			infof("Starting static address deposit withdrawal " +
 				"manager...")
-			err = withdrawalManager.Run(d.mainCtx, info.BlockHeight)
+			err := withdrawalManager.Run(d.mainCtx)
 			if err != nil && !errors.Is(context.Canceled, err) {
 				d.internalErrChan <- err
 			}
@@ -903,19 +887,8 @@ func (d *Daemon) initialize(withMacaroonService bool) error {
 		go func() {
 			defer d.wg.Done()
 
-			// Lnd's GetInfo call supplies us with the current block
-			// height.
-			info, err := d.lnd.Client.GetInfo(d.mainCtx)
-			if err != nil {
-				d.internalErrChan <- err
-
-				return
-			}
-
 			infof("Starting static address loop-in manager...")
-			err = staticLoopInManager.Run(
-				d.mainCtx, info.BlockHeight,
-			)
+			err := staticLoopInManager.Run(d.mainCtx)
 			if err != nil && !errors.Is(context.Canceled, err) {
 				d.internalErrChan <- err
 			}

staticaddr/address/manager.go

@@ -57,10 +57,13 @@ type Manager struct {
 }
 
 // NewManager creates a new address manager.
-func NewManager(cfg *ManagerConfig) *Manager {
-	return &Manager{
+func NewManager(cfg *ManagerConfig, currentHeight int32) *Manager {
+	m := &Manager{
 		cfg: cfg,
 	}
+	m.currentHeight.Store(currentHeight)
+
+	return m
 }
 
 // Run runs the address manager.
@@ -111,11 +114,6 @@ func (m *Manager) NewAddress(ctx context.Context) (*btcutil.AddressTaproot,
 	}
 	m.Unlock()
 
-	// Ensure that we have that we have a sane current block height.
-	if m.currentHeight.Load() == 0 {
-		return nil, fmt.Errorf("current block height is unknown")
-	}
-
 	// We are fetching a new L402 token from the server. There is one static
 	// address per L402 token allowed.
 	err = m.cfg.FetchL402(ctx)

staticaddr/address/manager_test.go

@@ -154,6 +154,9 @@ type ManagerTestContext struct {
 // NewAddressManagerTestContext creates a new test context for the static
 // address manager.
 func NewAddressManagerTestContext(t *testing.T) *ManagerTestContext {
+	ctxb, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
 	mockLnd := test.NewMockLnd()
 	lndContext := test.NewContext(t, mockLnd)
 
@@ -183,7 +186,10 @@ func NewAddressManagerTestContext(t *testing.T) *ManagerTestContext {
 		FetchL402:     func(context.Context) error { return nil },
 	}
 
-	manager := NewManager(cfg)
+	getInfo, err := mockLnd.Client.GetInfo(ctxb)
+	require.NoError(t, err)
+
+	manager := NewManager(cfg, int32(getInfo.BlockHeight))
 
 	return &ManagerTestContext{
 		manager:                 manager,

staticaddr/loopin/manager.go

@@ -139,21 +139,22 @@ type Manager struct {
 }
 
 // NewManager creates a new deposit withdrawal manager.
-func NewManager(cfg *Config) *Manager {
-	return &Manager{
+func NewManager(cfg *Config, currentHeight uint32) *Manager {
+	m := &Manager{
 		cfg:           cfg,
 		initChan:      make(chan struct{}),
 		newLoopInChan: make(chan *newSwapRequest),
 		exitChan:      make(chan struct{}),
 		errChan:       make(chan error),
 		activeLoopIns: make(map[lntypes.Hash]*FSM),
 	}
+	m.currentHeight.Store(currentHeight)
+
+	return m
 }
 
 // Run runs the static address loop-in manager.
-func (m *Manager) Run(ctx context.Context, currentHeight uint32) error {
-	m.currentHeight.Store(currentHeight)
-
+func (m *Manager) Run(ctx context.Context) error {
 	registerBlockNtfn := m.cfg.ChainNotifier.RegisterBlockEpochNtfn
 	newBlockChan, newBlockErrChan, err := registerBlockNtfn(ctx)
 	if err != nil {

staticaddr/withdraw/manager.go

@@ -115,21 +115,22 @@ type Manager struct {
 }
 
 // NewManager creates a new deposit withdrawal manager.
-func NewManager(cfg *ManagerConfig) *Manager {
-	return &Manager{
+func NewManager(cfg *ManagerConfig, currentHeight uint32) *Manager {
+	m := &Manager{
 		cfg:                      cfg,
 		initChan:                 make(chan struct{}),
 		finalizedWithdrawalTxns:  make(map[chainhash.Hash]*wire.MsgTx),
 		exitChan:                 make(chan struct{}),
 		newWithdrawalRequestChan: make(chan newWithdrawalRequest),
 		errChan:                  make(chan error),
 	}
+	m.initiationHeight.Store(currentHeight)
+
+	return m
 }
 
 // Run runs the deposit withdrawal manager.
-func (m *Manager) Run(ctx context.Context, currentHeight uint32) error {
-	m.initiationHeight.Store(currentHeight)
-
+func (m *Manager) Run(ctx context.Context) error {
 	newBlockChan, newBlockErrChan, err :=
 		m.cfg.ChainNotifier.RegisterBlockEpochNtfn(ctx)