Skip to content

StaticAddr: fix rc review comments #868

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Dec 16, 2024
+108 −93
Merged

StaticAddr: fix rc review comments #868

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
249a423
staticaddr: fix waring message formatting
hieblmi Dec 13, 2024
10b970c
staticaddr: no experimental flag for static
hieblmi Dec 13, 2024
acea303
staticaddr: check dust limits before withdrawing
hieblmi Dec 14, 2024
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
6 changes: 3 additions & 3 deletions cmd/loop/staticaddr.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -641,10 +641,10 @@ func depositsToOutpoints(deposits []*looprpc.Deposit) []string {

func displayNewAddressWarning() error {
fmt.Printf("\nWARNING: Be aware that loosing your l402.token file in " +
".loop under your home directory \nwill take your ability to " +
".loop under your home directory will take your ability to " +
"spend funds sent to the static address via loop-ins or " +
"\nwithdrawals. You will have to wait until the deposit " +
"expires and your loop client \nsweeps the funds back to your " +
"withdrawals. You will have to wait until the deposit " +
"expires and your loop client sweeps the funds back to your " +
"lnd wallet. The deposit expiry could be months in the " +
"future.\n")

Expand Down
134 changes: 68 additions & 66 deletions loopd/daemon.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -536,15 +536,80 @@ func (d *Daemon) initialize(withMacaroonService bool) error {
}()

var (
reservationManager *reservation.Manager
instantOutManager *instantout.Manager

staticAddressManager *address.Manager
depositManager *deposit.Manager
withdrawalManager *withdraw.Manager
staticLoopInManager *loopin.Manager
)

// Static address manager setup.
staticAddressStore := address.NewSqlStore(baseDb)
addrCfg := &address.ManagerConfig{
AddressClient: staticAddressClient,
FetchL402: swapClient.Server.FetchL402,
Store: staticAddressStore,
WalletKit: d.lnd.WalletKit,
ChainParams: d.lnd.ChainParams,
ChainNotifier: d.lnd.ChainNotifier,
}
staticAddressManager = address.NewManager(addrCfg)

// Static address deposit manager setup.
depositStore := deposit.NewSqlStore(baseDb)
depoCfg := &deposit.ManagerConfig{
AddressClient: staticAddressClient,
AddressManager: staticAddressManager,
SwapClient: swapClient,
Store: depositStore,
WalletKit: d.lnd.WalletKit,
ChainParams: d.lnd.ChainParams,
ChainNotifier: d.lnd.ChainNotifier,
Signer: d.lnd.Signer,
}
depositManager = deposit.NewManager(depoCfg)

// Static address deposit withdrawal manager setup.
withdrawalCfg := &withdraw.ManagerConfig{
StaticAddressServerClient: staticAddressClient,
AddressManager: staticAddressManager,
DepositManager: depositManager,
WalletKit: d.lnd.WalletKit,
ChainParams: d.lnd.ChainParams,
ChainNotifier: d.lnd.ChainNotifier,
Signer: d.lnd.Signer,
}
withdrawalManager = withdraw.NewManager(withdrawalCfg)

// Static address loop-in manager setup.
staticAddressLoopInStore := loopin.NewSqlStore(
loopdb.NewTypedStore[loopin.Querier](baseDb),
clock.NewDefaultClock(), d.lnd.ChainParams,
)

staticLoopInManager = loopin.NewManager(&loopin.Config{
Server: staticAddressClient,
QuoteGetter: swapClient.Server,
LndClient: d.lnd.Client,
InvoicesClient: d.lnd.Invoices,
NodePubkey: d.lnd.NodePubkey,
AddressManager: staticAddressManager,
DepositManager: depositManager,
Store: staticAddressLoopInStore,
WalletKit: d.lnd.WalletKit,
ChainNotifier: d.lnd.ChainNotifier,
NotificationManager: notificationManager,
ChainParams: d.lnd.ChainParams,
Signer: d.lnd.Signer,
ValidateLoopInContract: loop.ValidateLoopInContract,
MaxStaticAddrHtlcFeePercentage: d.cfg.MaxStaticAddrHtlcFeePercentage,
MaxStaticAddrHtlcBackupFeePercentage: d.cfg.MaxStaticAddrHtlcBackupFeePercentage,
})

var (
reservationManager *reservation.Manager
instantOutManager *instantout.Manager
)

// Create the reservation and instantout managers.
if d.cfg.EnableExperimental {
reservationStore := reservation.NewSQLStore(
Expand Down Expand Up @@ -583,69 +648,6 @@ func (d *Daemon) initialize(withMacaroonService bool) error {
instantOutManager = instantout.NewInstantOutManager(
instantOutConfig,
)

// Static address manager setup.
staticAddressStore := address.NewSqlStore(baseDb)
addrCfg := &address.ManagerConfig{
AddressClient: staticAddressClient,
FetchL402: swapClient.Server.FetchL402,
Store: staticAddressStore,
WalletKit: d.lnd.WalletKit,
ChainParams: d.lnd.ChainParams,
ChainNotifier: d.lnd.ChainNotifier,
}
staticAddressManager = address.NewManager(addrCfg)

// Static address deposit manager setup.
depositStore := deposit.NewSqlStore(baseDb)
depoCfg := &deposit.ManagerConfig{
AddressClient: staticAddressClient,
AddressManager: staticAddressManager,
SwapClient: swapClient,
Store: depositStore,
WalletKit: d.lnd.WalletKit,
ChainParams: d.lnd.ChainParams,
ChainNotifier: d.lnd.ChainNotifier,
Signer: d.lnd.Signer,
}
depositManager = deposit.NewManager(depoCfg)

// Static address deposit withdrawal manager setup.
withdrawalCfg := &withdraw.ManagerConfig{
StaticAddressServerClient: staticAddressClient,
AddressManager: staticAddressManager,
DepositManager: depositManager,
WalletKit: d.lnd.WalletKit,
ChainParams: d.lnd.ChainParams,
ChainNotifier: d.lnd.ChainNotifier,
Signer: d.lnd.Signer,
}
withdrawalManager = withdraw.NewManager(withdrawalCfg)

// Static address loop-in manager setup.
staticAddressLoopInStore := loopin.NewSqlStore(
loopdb.NewTypedStore[loopin.Querier](baseDb),
clock.NewDefaultClock(), d.lnd.ChainParams,
)

staticLoopInManager = loopin.NewManager(&loopin.Config{
Server: staticAddressClient,
QuoteGetter: swapClient.Server,
LndClient: d.lnd.Client,
InvoicesClient: d.lnd.Invoices,
NodePubkey: d.lnd.NodePubkey,
AddressManager: staticAddressManager,
DepositManager: depositManager,
Store: staticAddressLoopInStore,
WalletKit: d.lnd.WalletKit,
ChainNotifier: d.lnd.ChainNotifier,
NotificationManager: notificationManager,
ChainParams: d.lnd.ChainParams,
Signer: d.lnd.Signer,
ValidateLoopInContract: loop.ValidateLoopInContract,
MaxStaticAddrHtlcFeePercentage: d.cfg.MaxStaticAddrHtlcFeePercentage,
MaxStaticAddrHtlcBackupFeePercentage: d.cfg.MaxStaticAddrHtlcBackupFeePercentage,
})
}

// Now finally fully initialize the swap client RPC server instance.
Expand Down
61 changes: 37 additions & 24 deletions staticaddr/withdraw/manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,39 @@ func (m *Manager) createFinalizedWithdrawalTx(ctx context.Context,
).FeePerKWeight()
}

// We'll now check the selected fee rate leaves a withdrawal output that
Copy link
Collaborator

@starius starius Dec 15, 2024

Choose a reason for hiding this comment

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

I propose to cover the logic and various sub-cases in this method with unit tests. Maybe factor out a pure function to make it unit tests friendly.

// is above the dust limit. If not we cancel the withdrawal instead of
// requesting a signature from the server.
addressParams, err := m.cfg.AddressManager.GetStaticAddressParameters(
ctx,
)
if err != nil {
return nil, fmt.Errorf("couldn't get confirmation height for "+
"deposit, %w", err)
}

// Calculate the fee value in satoshis.
outpoints := toOutpoints(deposits)
weight, err := withdrawalFee(len(outpoints), withdrawalAddress)
if err != nil {
return nil, err
}
feeValue := withdrawalSweepFeeRate.FeeForWeight(weight)

var (
prevOuts = m.toPrevOuts(deposits, addressParams.PkScript)
totalValue = withdrawalValue(prevOuts)
outputValue = int64(totalValue) - int64(feeValue)
// P2TRSize calculates a dust limit based on a 40 byte maximum
// size witness output.
dustLimit = lnwallet.DustLimitForSize(input.P2TRSize)
)

if outputValue < int64(dustLimit) {
return nil, fmt.Errorf("withdrawal output value %d sats "+
"below dust limit %d sats", outputValue, dustLimit)
}

resp, err := m.cfg.StaticAddressServerClient.ServerWithdrawDeposits(
ctx, &staticaddressrpc.ServerWithdrawRequest{
Outpoints: toPrevoutInfo(outpoints),
Expand All @@ -393,19 +425,9 @@ func (m *Manager) createFinalizedWithdrawalTx(ctx context.Context,
return nil, err
}

addressParams, err := m.cfg.AddressManager.GetStaticAddressParameters(
ctx,
)
if err != nil {
return nil, fmt.Errorf("couldn't get confirmation height for "+
"deposit, %w", err)
}

prevOuts := m.toPrevOuts(deposits, addressParams.PkScript)
totalValue := withdrawalValue(prevOuts)
withdrawalOutputValue := int64(totalValue - feeValue)
withdrawalTx, err := m.createWithdrawalTx(
outpoints, totalValue, withdrawalAddress,
withdrawalSweepFeeRate,
outpoints, withdrawalOutputValue, withdrawalAddress,
)
if err != nil {
return nil, err
Expand Down Expand Up @@ -613,9 +635,8 @@ func byteSliceTo66ByteSlice(b []byte) ([musig2.PubNonceSize]byte, error) {
}

func (m *Manager) createWithdrawalTx(outpoints []wire.OutPoint,
withdrawlAmount btcutil.Amount, clientSweepAddress btcutil.Address,
feeRate chainfee.SatPerKWeight) (*wire.MsgTx,
error) {
withdrawlOutputValue int64, clientSweepAddress btcutil.Address) (
*wire.MsgTx, error) {

// First Create the tx.
msgTx := wire.NewMsgTx(2)
Expand All @@ -628,22 +649,14 @@ func (m *Manager) createWithdrawalTx(outpoints []wire.OutPoint,
})
}

// Estimate the fee.
weight, err := withdrawalFee(len(outpoints), clientSweepAddress)
if err != nil {
return nil, err
}

pkscript, err := txscript.PayToAddrScript(clientSweepAddress)
if err != nil {
return nil, err
}

fee := feeRate.FeeForWeight(weight)

// Create the sweep output
sweepOutput := &wire.TxOut{
Value: int64(withdrawlAmount) - int64(fee),
Value: withdrawlOutputValue,
PkScript: pkscript,
}

Expand Down
Loading