From dc4a5641acd85f5f0a39b79c1b4901d0867240e9 Mon Sep 17 00:00:00 2001 From: Boris Nagaev Date: Fri, 14 Mar 2025 14:16:07 -0300 Subject: [PATCH] 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. --- instantout/manager.go | 8 ++-- loopd/daemon.go | 65 +++++++++--------------------- staticaddr/address/manager.go | 12 +++--- staticaddr/address/manager_test.go | 8 +++- staticaddr/loopin/manager.go | 11 ++--- staticaddr/withdraw/manager.go | 11 ++--- 6 files changed, 46 insertions(+), 69 deletions(-) diff --git a/instantout/manager.go b/instantout/manager.go index 38207c44..905874fc 100644 --- a/instantout/manager.go +++ b/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 { diff --git a/loopd/daemon.go b/loopd/daemon.go index d35d5fc5..3c52c816 100644 --- a/loopd/daemon.go +++ b/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 } diff --git a/staticaddr/address/manager.go b/staticaddr/address/manager.go index 96e9cdbf..cc601273 100644 --- a/staticaddr/address/manager.go +++ b/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) diff --git a/staticaddr/address/manager_test.go b/staticaddr/address/manager_test.go index 35a6d006..37ba83c9 100644 --- a/staticaddr/address/manager_test.go +++ b/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, diff --git a/staticaddr/loopin/manager.go b/staticaddr/loopin/manager.go index cbe6d9ce..9f31b29e 100644 --- a/staticaddr/loopin/manager.go +++ b/staticaddr/loopin/manager.go @@ -139,8 +139,8 @@ 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), @@ -148,12 +148,13 @@ func NewManager(cfg *Config) *Manager { 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 { diff --git a/staticaddr/withdraw/manager.go b/staticaddr/withdraw/manager.go index 2de21214..3a446ca1 100644 --- a/staticaddr/withdraw/manager.go +++ b/staticaddr/withdraw/manager.go @@ -115,8 +115,8 @@ 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), @@ -124,12 +124,13 @@ func NewManager(cfg *ManagerConfig) *Manager { 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)