-
Notifications
You must be signed in to change notification settings - Fork 118
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
sweepbatcher: add mode with presigned transactions #891
base: master
Are you sure you want to change the base?
Changes from 5 commits
025b81b
6a2e147
7428443
323617c
418bd2d
1d4fd56
334b5f9
c551410
d49a2ff
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,7 @@ import ( | |
"github.com/btcsuite/btcd/btcec/v2/schnorr/musig2" | ||
"github.com/btcsuite/btcd/btcutil" | ||
"github.com/btcsuite/btcd/btcutil/psbt" | ||
"github.com/btcsuite/btcd/chaincfg" | ||
"github.com/btcsuite/btcd/chaincfg/chainhash" | ||
"github.com/btcsuite/btcd/txscript" | ||
"github.com/btcsuite/btcd/wire" | ||
|
@@ -120,6 +121,9 @@ type sweep struct { | |
// but it failed. We try to spend a sweep cooperatively only once. This | ||
// status is not persisted in the DB. | ||
coopFailed bool | ||
|
||
// presigned is set, if the sweep should be handled in presigned mode. | ||
presigned bool | ||
} | ||
|
||
// batchState is the state of the batch. | ||
|
@@ -173,6 +177,14 @@ type batchConfig struct { | |
// Note that musig2SignSweep must be nil in this case, however signer | ||
// client must still be provided, as it is used for non-coop spendings. | ||
customMuSig2Signer SignMuSig2 | ||
|
||
// presignedHelper provides methods used when presigned batches are | ||
// enabled. | ||
presignedHelper PresignedHelper | ||
|
||
// chainParams are the chain parameters of the chain that is used by | ||
// batches. | ||
chainParams *chaincfg.Params | ||
} | ||
|
||
// rbfCache stores data related to our last fee bump. | ||
|
@@ -460,7 +472,9 @@ func (b *batch) Errorf(format string, params ...interface{}) { | |
} | ||
|
||
// addSweep tries to add a sweep to the batch. If this is the first sweep being | ||
// added to the batch then it also sets the primary sweep ID. | ||
// added to the batch then it also sets the primary sweep ID. If presigned mode | ||
// is enabled, the result depends on the outcome of presignedHelper.Presign for | ||
// a non-empty batch. For an empty batch, the input needs to pass PresignSweep. | ||
func (b *batch) addSweep(ctx context.Context, sweep *sweep) (bool, error) { | ||
done, err := b.scheduleNextCall() | ||
defer done() | ||
|
@@ -566,6 +580,54 @@ func (b *batch) addSweep(ctx context.Context, sweep *sweep) (bool, error) { | |
} | ||
} | ||
|
||
// If presigned mode is enabled, we should first presign the new version | ||
// of batch transaction. Also ensure that all the sweeps in the batch | ||
// use the same mode (presigned or regular). | ||
if sweep.presigned { | ||
// Ensure that all the sweeps in the batch use presigned mode. | ||
for _, s := range b.sweeps { | ||
if !s.presigned { | ||
b.log().Infof("failed to add sweep %x to the "+ | ||
"batch, because the batch has "+ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: maybe clarify "failed to add presigned sweep %x..." There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed |
||
"non-presigned sweep %x", | ||
sweep.swapHash[:6], s.swapHash[:6]) | ||
|
||
return false, nil | ||
} | ||
} | ||
|
||
if len(b.sweeps) != 0 { | ||
if err := b.presign(ctx, sweep); err != nil { | ||
b.log().Infof("failed to add sweep %x to the "+ | ||
"batch, because failed to presign new "+ | ||
"version of batch tx: %v", | ||
sweep.swapHash[:6], err) | ||
|
||
return false, nil | ||
} | ||
} else { | ||
if err := b.ensurePresigned(ctx, sweep); err != nil { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If |
||
return false, fmt.Errorf("failed to check "+ | ||
"signing of input %x, this means that "+ | ||
"batcher.PresignSweep was not called "+ | ||
"prior to AddSweep for this input: %w", | ||
sweep.swapHash[:6], err) | ||
} | ||
} | ||
} else { | ||
// Ensure that all the sweeps in the batch don't use presigned. | ||
for _, s := range b.sweeps { | ||
if s.presigned { | ||
b.log().Infof("failed to add sweep %x to the "+ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: maybe clarify "failed to add non-presigned sweep %x..." There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed |
||
"batch, because the batch has "+ | ||
"presigned sweep %x", | ||
sweep.swapHash[:6], s.swapHash[:6]) | ||
|
||
return false, nil | ||
} | ||
} | ||
} | ||
|
||
// Past this point we know that a new incoming sweep passes the | ||
// acceptance criteria and is now ready to be added to this batch. | ||
|
||
|
@@ -863,6 +925,23 @@ func (b *batch) isUrgent(skipBefore time.Time) bool { | |
return true | ||
} | ||
|
||
// isPresigned returns if the batch uses presigned mode. Currently presigned and | ||
// non-presigned sweeps never appear in the same batch. Fails if the batch is | ||
// empty. | ||
func (b *batch) isPresigned() (bool, error) { | ||
if len(b.sweeps) == 0 { | ||
return false, fmt.Errorf("the batch is empty") | ||
} | ||
|
||
for _, sweep := range b.sweeps { | ||
if sweep.presigned { | ||
return true, nil | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think if all sweeps are presigned we return There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I rewrote the function to ensure that all the sweeps have the same presigned status. If there is a mix, the function returns an error. This would be a bug. |
||
} | ||
} | ||
|
||
return false, nil | ||
} | ||
|
||
// publish creates and publishes the latest batch transaction to the network. | ||
func (b *batch) publish(ctx context.Context) error { | ||
var ( | ||
|
@@ -888,7 +967,19 @@ func (b *batch) publish(ctx context.Context) error { | |
b.publishErrorHandler(err, errMsg, b.log()) | ||
} | ||
|
||
fee, err, signSuccess = b.publishMixedBatch(ctx) | ||
// Determine if we should use presigned mode for the batch. | ||
presigned, err := b.isPresigned() | ||
if err != nil { | ||
return fmt.Errorf("failed to determine if the batch %d uses "+ | ||
"presigned mode: %w", b.id, err) | ||
} | ||
|
||
if presigned { | ||
fee, err, signSuccess = b.publishPresigned(ctx) | ||
} else { | ||
fee, err, signSuccess = b.publishMixedBatch(ctx) | ||
} | ||
|
||
if err != nil { | ||
if signSuccess { | ||
logPublishError("publish error", err) | ||
|
@@ -959,9 +1050,9 @@ func (b *batch) createPsbt(unsignedTx *wire.MsgTx, sweeps []sweep) ([]byte, | |
|
||
// constructUnsignedTx creates unsigned tx from the sweeps, paying to the addr. | ||
// It also returns absolute fee (from weight and clamped). | ||
func (b *batch) constructUnsignedTx(sweeps []sweep, | ||
address btcutil.Address) (*wire.MsgTx, lntypes.WeightUnit, | ||
btcutil.Amount, btcutil.Amount, error) { | ||
func constructUnsignedTx(sweeps []sweep, address btcutil.Address, | ||
currentHeight int32, feeRate chainfee.SatPerKWeight) (*wire.MsgTx, | ||
lntypes.WeightUnit, btcutil.Amount, btcutil.Amount, error) { | ||
|
||
// Sanity check, there should be at least 1 sweep in this batch. | ||
if len(sweeps) == 0 { | ||
|
@@ -971,7 +1062,7 @@ func (b *batch) constructUnsignedTx(sweeps []sweep, | |
// Create the batch transaction. | ||
batchTx := &wire.MsgTx{ | ||
Version: 2, | ||
LockTime: uint32(b.currentHeight), | ||
LockTime: uint32(currentHeight), | ||
} | ||
|
||
// Add transaction inputs and estimate its weight. | ||
|
@@ -1023,7 +1114,7 @@ func (b *batch) constructUnsignedTx(sweeps []sweep, | |
|
||
// Find weight and fee. | ||
weight := weightEstimate.Weight() | ||
feeForWeight := b.rbfCache.FeeRate.FeeForWeight(weight) | ||
feeForWeight := feeRate.FeeForWeight(weight) | ||
|
||
// Clamp the calculated fee to the max allowed fee amount for the batch. | ||
fee := clampBatchFee(feeForWeight, batchAmt) | ||
|
@@ -1108,8 +1199,8 @@ func (b *batch) publishMixedBatch(ctx context.Context) (btcutil.Amount, error, | |
|
||
// Construct unsigned batch transaction. | ||
var err error | ||
tx, weight, feeForWeight, fee, err = b.constructUnsignedTx( | ||
sweeps, address, | ||
tx, weight, feeForWeight, fee, err = constructUnsignedTx( | ||
sweeps, address, b.currentHeight, b.rbfCache.FeeRate, | ||
) | ||
if err != nil { | ||
return 0, fmt.Errorf("failed to construct tx: %w", err), | ||
|
@@ -1769,6 +1860,28 @@ func (b *batch) handleSpend(ctx context.Context, spendTx *wire.MsgTx) error { | |
// handleConf handles a confirmation notification. This is the final step of the | ||
// batch. Here we signal to the batcher that this batch was completed. | ||
func (b *batch) handleConf(ctx context.Context) error { | ||
// If the batch is in presigned mode, cleanup presignedHelper. | ||
presigned, err := b.isPresigned() | ||
if err != nil { | ||
return fmt.Errorf("failed to determine if the batch %d uses "+ | ||
"presigned mode: %w", b.id, err) | ||
} | ||
|
||
if presigned { | ||
b.Infof("Cleaning up presigned store") | ||
|
||
inputs := make([]wire.OutPoint, 0, len(b.sweeps)) | ||
for _, sweep := range b.sweeps { | ||
inputs = append(inputs, sweep.outpoint) | ||
} | ||
|
||
err := b.cfg.presignedHelper.CleanupTransactions(ctx, inputs) | ||
if err != nil { | ||
return fmt.Errorf("failed to clean up store for "+ | ||
"batch %d, inputs %v: %w", b.id, inputs, err) | ||
} | ||
} | ||
|
||
b.Infof("confirmed in txid %s", b.batchTxid) | ||
b.state = Confirmed | ||
|
||
|
@@ -1811,7 +1924,21 @@ func (b *batch) persist(ctx context.Context) error { | |
|
||
// getBatchDestAddr returns the batch's destination address. If the batch | ||
// has already generated an address then the same one will be returned. | ||
// The method must not be used in presigned mode. Use getSweepsDestAddr instead. | ||
func (b *batch) getBatchDestAddr(ctx context.Context) (btcutil.Address, error) { | ||
// Determine if we should use presigned mode for the batch. | ||
presigned, err := b.isPresigned() | ||
if err != nil { | ||
return nil, fmt.Errorf("failed to determine if the batch %d "+ | ||
"uses presigned mode: %w", b.id, err) | ||
} | ||
|
||
// Make sure that the method is not used for presigned batches. | ||
if presigned { | ||
return nil, fmt.Errorf("getBatchDestAddr used in presigned " + | ||
"mode") | ||
} | ||
|
||
var address btcutil.Address | ||
|
||
// If a batch address is set, use that. Otherwise, generate a | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instead of iterating, do you think it makes sense to add a
presigned
flag to the batch struct that we can check before adding a sweep to a batch?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I plan to allow half-presigned batches in the future to use a regular sweep an an anchor and such a flag would be removed. So I decided to keep this calculated not to forget to remove it in the future :)