Merge pull request #288 from lightninglabs/issue-191

Add sats_per_byte option to newAccounts cmd

account/manager.go

@@ -188,16 +188,26 @@ func (m *Manager) start() error {
 	if err != nil {
 		return fmt.Errorf("unable to retrieve accounts: %v", err)
 	}
+
+	// We calculate the default fee rate that will be used
+	// for resuming accounts for which we haven't created and broadcast
+	// a transaction yet
+	feeRate, err := m.cfg.Wallet.EstimateFee(
+		ctx, int32(DefaultFundingConfTarget),
+	)
+	if err != nil {
+		return fmt.Errorf("unable to estimate default fees %w", err)
+	}
+
 	for _, account := range accounts {
 		// Try to resume the account now.
 		//
 		// TODO(guggero): Refactor this to extract the init/funding
 		// part so we properly abandon the account if it fails before
 		// publishing the TX instead of trying to re-fund on startup.
-		err := m.resumeAccount(
-			ctx, account, true, false, DefaultFundingConfTarget,
-		)
-		if err != nil {
+		if err := m.resumeAccount(
+			ctx, account, true, false, feeRate,
+		); err != nil {
 			return fmt.Errorf("unable to resume account %x: %v",
 				account.TraderKey.PubKey.SerializeCompressed(),
 				err)
@@ -258,7 +268,8 @@ func (m *Manager) QuoteAccount(ctx context.Context, value btcutil.Amount,
 // InitAccount handles a request to create a new account with the provided
 // parameters.
 func (m *Manager) InitAccount(ctx context.Context, value btcutil.Amount,
-	expiry, bestHeight, confTarget uint32) (*Account, error) {
+	feeRate chainfee.SatPerKWeight, expiry,
+	bestHeight uint32) (*Account, error) {
 
 	// We'll make sure to acquire the reservation lock throughout the
 	// account funding process to ensure we use the same reservation, as
@@ -281,26 +292,6 @@ func (m *Manager) InitAccount(ctx context.Context, value btcutil.Amount,
 		return nil, err
 	}
 
-	// Let's make sure our wallet contains enough coins to fund the account
-	// before we reserve any resources. We ask lnd to create a transaction
-	// to send the given account value in dry-run mode. This makes sure we
-	// actually have some UTXOs to fund the account as this method returns
-	// an error on insufficient balance. Unfortunately it currently only
-	// supports estimating the total fee for a transaction using a
-	// confirmation target. We'll want to add sats/vByte as well as soon as
-	// the API allows it.
-	totalMinerFee, err := m.cfg.TxFeeEstimator.EstimateFeeToP2WSH(
-		ctx, value, int32(confTarget),
-	)
-	if err != nil {
-		return nil, fmt.Errorf("error estimating on-chain fee: %v", err)
-	}
-
-	// A by-product of the balance check is the total fee we'd need to pay
-	// so we might as well log it here.
-	log.Infof("Estimated total chain fee of %v for new account with "+
-		"value=%v, conf_target=%v", totalMinerFee, value, confTarget)
-
 	// We'll start by deriving a key for ourselves that we'll use in our
 	// 2-of-2 multi-sig construction.
 	keyDesc, err := m.cfg.Wallet.DeriveNextKey(
@@ -350,7 +341,7 @@ func (m *Manager) InitAccount(ctx context.Context, value btcutil.Amount,
 	log.Infof("Creating new account %x of %v that expires at height %v",
 		keyDesc.PubKey.SerializeCompressed(), value, expiry)
 
-	err = m.resumeAccount(ctx, account, false, false, confTarget)
+	err = m.resumeAccount(ctx, account, false, false, feeRate)
 	if err != nil {
 		return nil, err
 	}
@@ -389,6 +380,8 @@ func (m *Manager) WatchMatchedAccounts(ctx context.Context,
 		// closed because it was used up or pending batch update because
 		// it was recreated. Either way, let's resume it now by creating
 		// the appropriate watchers again.
+		// We set feerate to 0 because we know that we won't need to
+		// create a new transaction for resuming the account.
 		err = m.resumeAccount(ctx, acct, false, false, 0)
 		if err != nil {
 			return fmt.Errorf("error resuming account %x: %v",
@@ -418,7 +411,7 @@ func (m *Manager) maybeBroadcastTx(ctx context.Context, tx *wire.MsgTx,
 // This method serves as a way to consolidate the logic of resuming accounts on
 // startup and during normal operation.
 func (m *Manager) resumeAccount(ctx context.Context, account *Account, // nolint
-	onRestart bool, onRecovery bool, fundingConfTarget uint32) error {
+	onRestart bool, onRecovery bool, feeRate chainfee.SatPerKWeight) error {
 
 	accountOutput, err := account.Output()
 	if err != nil {
@@ -480,26 +473,24 @@ func (m *Manager) resumeAccount(ctx context.Context, account *Account, // nolint
 		}
 
 		if createTx {
-			// We need a static sat/vByte value for the fee in
-			// SendOutputs, so we use the stored targetConf of the
-			// account to estimate it.
-			feeSatPerKw, err := m.cfg.Wallet.EstimateFee(
-				ctx, int32(fundingConfTarget),
-			)
-			if err != nil {
-				return err
+			acctKey := account.TraderKey.PubKey.SerializeCompressed()
+
+			if feeRate == 0 {
+				return fmt.Errorf("unable to create "+
+					" transaction for account with "+
+					"  trader key %x, feeRate should "+
+					"be greater than 0", acctKey)
 			}
 
 			// If we have a label prefix, then we'll apply that now
 			// and also attach some additional meta data.
-			acctKey := account.TraderKey.PubKey.SerializeCompressed()
 			contextLabel := fmt.Sprintf(" poold -- "+
 				"AccountCreation(acct_key=%x)", acctKey)
 			label := makeTxnLabel(m.cfg.TxLabelPrefix, contextLabel)
 
 			// TODO(wilmer): Expose manual controls to bump fees.
 			tx, err := m.cfg.Wallet.SendOutputs(
-				ctx, []*wire.TxOut{accountOutput}, feeSatPerKw,
+				ctx, []*wire.TxOut{accountOutput}, feeRate,
 				label,
 			)
 			if err != nil {
@@ -969,7 +960,7 @@ func (m *Manager) handleAccountSpend(traderKey *btcec.PublicKey,
 		if ok {
 			// Proceed with the rest of the flow. We won't send to
 			// the account output again, so we don't need to set
-			// a valid conf target.
+			// a valid feeRate.
 			return m.resumeAccount(
 				context.Background(), account, false, false, 0,
 			)
@@ -1464,7 +1455,7 @@ func (m *Manager) RecoverAccount(ctx context.Context, account *Account) error {
 	// the `onRestart` flag to false because that would try to re-publish
 	// the opening transaction in some cases which we don't want. Instead we
 	// set the `onRecovery` flag to true. We won't send to the account
-	// output again, so we don't need to set a valid funding conf target.
+	// output again, so we don't need to set a valid funding freeRate.
 	return m.resumeAccount(ctx, account, false, true, 0)
 }
 

account/manager_test.go

@@ -151,7 +151,7 @@ func (h *testHarness) openAccount(value btcutil.Amount, expiry uint32, // nolint
 	// Create a new account. Its initial state should be StatePendingOpen.
 	ctx := context.Background()
 	account, err := h.manager.InitAccount(
-		ctx, value, expiry, bestHeight, DefaultFundingConfTarget,
+		ctx, value, chainfee.FeePerKwFloor, expiry, bestHeight,
 	)
 	if err != nil {
 		h.t.Fatalf("unable to create new account: %v", err)
@@ -531,8 +531,8 @@ func TestResumeAccountAfterRestart(t *testing.T) {
 	// persisted intent to create the account.
 	go func() {
 		_, _ = h.manager.InitAccount(
-			context.Background(), value, expiry, bestHeight,
-			DefaultFundingConfTarget,
+			context.Background(), value, chainfee.FeePerKwFloor, expiry,
+			bestHeight,
 		)
 	}()
 

cmd/pool/account.go

@@ -101,6 +101,11 @@ var newAccountCommand = cli.Command{
 				"within",
 			Value: defaultFundingConfTarget,
 		},
+		cli.Uint64Flag{
+			Name: "sat_per_vbyte",
+			Usage: "the fee rate expressed in sat/vbyte that " +
+				"should be used for the account creation transaction",
+		},
 		cli.BoolFlag{
 			Name:  "force",
 			Usage: "skip account fee confirmation",
@@ -119,10 +124,37 @@ func newAccount(ctx *cli.Context) error {
 
 	req := &poolrpc.InitAccountRequest{
 		AccountValue: amt,
-		Fees: &poolrpc.InitAccountRequest_ConfTarget{
+		Initiator:    defaultInitiator,
+	}
+
+	satPerVByte := ctx.Uint64("sat_per_vbyte")
+	confTarget := ctx.Uint64("conf_target")
+
+	switch {
+	case satPerVByte > 0:
+		// Enforce a minimum fee rate of 253 sat/kw by rounding up if 1
+		// sat/byte is used.
+		feeRate := chainfee.SatPerKVByte(satPerVByte * 1000).FeePerKWeight()
+		if feeRate < chainfee.FeePerKwFloor {
+			feeRate = chainfee.FeePerKwFloor
+		}
+		req.Fees = &poolrpc.InitAccountRequest_FeeRateSatPerKw{
+			FeeRateSatPerKw: uint64(feeRate),
+		}
+
+	case ctx.IsSet("conf_target"):
+		if confTarget == 0 {
+			return fmt.Errorf("specified confirmation target must be " +
+				"greater than 0")
+		}
+
+		req.Fees = &poolrpc.InitAccountRequest_ConfTarget{
 			ConfTarget: uint32(ctx.Uint64("conf_target")),
-		},
-		Initiator: defaultInitiator,
+		}
+
+	default:
+		return fmt.Errorf("either sat/vbyte or confirmation target " +
+			"must be specified")
 	}
 
 	// Parse the expiry in either of its forms. We'll always prefer the
@@ -150,8 +182,10 @@ func newAccount(ctx *cli.Context) error {
 	// present it to the user.
 	if !ctx.Bool("force") {
 		if err := printAccountFees(
-			client, btcutil.Amount(amt),
-			uint32(ctx.Uint64("conf_target")),
+			client,
+			btcutil.Amount(amt),
+			satPerVByte,
+			uint32(confTarget),
 		); err != nil {
 			return err
 		}
@@ -173,27 +207,36 @@ func newAccount(ctx *cli.Context) error {
 }
 
 func printAccountFees(client poolrpc.TraderClient, amt btcutil.Amount,
-	confTarget uint32) error {
+	satPerVByte uint64, confTarget uint32) error {
 
-	req := &poolrpc.QuoteAccountRequest{
-		AccountValue: uint64(amt),
-		Fees: &poolrpc.QuoteAccountRequest_ConfTarget{
-			ConfTarget: confTarget,
-		},
+	if confTarget == 0 {
+		fmt.Println("-- Account Funding Details --")
+		fmt.Printf("Amount: %v\n", amt)
+		fmt.Printf("Fee rate (estimated): %d sat/vByte\n", satPerVByte)
+
+		return nil
 	}
-	resp, err := client.QuoteAccount(context.Background(), req)
+
+	resp, err := client.QuoteAccount(
+		context.Background(),
+		&poolrpc.QuoteAccountRequest{
+			AccountValue: uint64(amt),
+			Fees: &poolrpc.QuoteAccountRequest_ConfTarget{
+				ConfTarget: confTarget,
+			},
+		},
+	)
 	if err != nil {
 		return fmt.Errorf("unable to estimate on-chain fees: "+
 			"%v", err)
 	}
 
 	feeRate := chainfee.SatPerKWeight(resp.MinerFeeRateSatPerKw)
-	satPerVByte := float64(feeRate.FeePerKVByte()) / 1000
-
+	feePerVByte := float64(feeRate.FeePerKVByte()) / 1000
 	fmt.Println("-- Account Funding Details --")
 	fmt.Printf("Amount: %v\n", amt)
 	fmt.Printf("Confirmation target: %v blocks\n", confTarget)
-	fmt.Printf("Fee rate (estimated): %.1f sat/vByte\n", satPerVByte)
+	fmt.Printf("Fee rate (estimated): %.1f sat/vByte\n", feePerVByte)
 	fmt.Printf("Total miner fee (estimated): %v\n",
 		btcutil.Amount(resp.MinerFeeTotal))
 

poolrpc/trader.proto

@@ -221,8 +221,11 @@ message InitAccountRequest {
         */
         uint32 conf_target = 4;
 
-        // TODO(guggero): Add sat_per_vbyte as soon as lnd has a parameter for
-        // that in rpcServer.EstimateFee.
+        /*
+        The fee rate, in satoshis per kw, to use for the initial funding
+        transaction.
+        */
+        uint64 fee_rate_sat_per_kw = 6;
     }
 
     /*

poolrpc/trader.swagger.json

@@ -1495,6 +1495,11 @@
           "format": "int64",
           "description": "The target number of blocks that the transaction should be confirmed in."
         },
+        "fee_rate_sat_per_kw": {
+          "type": "string",
+          "format": "uint64",
+          "description": "The fee rate, in satoshis per kw, to use for the initial funding\ntransaction."
+        },
         "initiator": {
           "type": "string",
           "description": "An optional identification string that will be appended to the user agent\nstring sent to the server to give information about the usage of pool. This\ninitiator part is meant for user interfaces to add their name to give the\nfull picture of the binary used (poold, LiT) and the method used for opening\nthe account (pool CLI, LiT UI, other 3rd party UI)."