walletrpc+lnwallet: possibility to define an address type for changes in FundPsbt

By default, P2TR addresses are used for changes. However, some users might encounter some problems with this change. We add the possibility to define a custom address type in FundPsbt for default/imported accounts (only P2TR for now). If no address type is specified for these accounts, we will use P2WKH by default.
2023-02-08 12:35:44 +01:00
1 parent 3bb68661e7
commit cdb015223b
@@ -928,26 +928,37 @@ func runFundAndSignPsbt(ht *lntemp.HarnessTest, alice *node.HarnessNode) {
 		lnrpc.AddressType_WITNESS_PUBKEY_HASH,
 		lnrpc.AddressType_TAPROOT_PUBKEY,
 	}
+	changeAddrTypes := []walletrpc.ChangeAddressType{
+		walletrpc.ChangeAddressType_CHANGE_ADDRESS_TYPE_UNSPECIFIED,
+		walletrpc.ChangeAddressType_CHANGE_ADDRESS_TYPE_P2TR,
+	}

 	for _, addrType := range spendAddrTypes {
-		ht.Logf("testing with address type %s", addrType)
+		for _, changeType := range changeAddrTypes {
+			ht.Logf("testing with address type %s and"+
+				"change address type %s", addrType, changeType)

-		// First, spend all the coins in the wallet to an address of
-		// the given type so that UTXO will be picked when funding a
-		// PSBT.
-		sendAllCoinsToAddrType(ht, alice, addrType)
+			// First, spend all the coins in the wallet to an
+			// address of the given type so that UTXO will be picked
+			// when funding a PSBT.
+			sendAllCoinsToAddrType(ht, alice, addrType)

-		// Let's fund a PSBT now where we want to send a few sats to
-		// our main address.
-		assertPsbtFundSignSpend(ht, alice, fundOutputs, false)
+			// Let's fund a PSBT now where we want to send a few
+			// sats to our main address.
+			assertPsbtFundSignSpend(
+				ht, alice, fundOutputs, changeType, false,
+			)

-		// Send all coins back to a single address once again.
-		sendAllCoinsToAddrType(ht, alice, addrType)
+			// Send all coins back to a single address once again.
+			sendAllCoinsToAddrType(ht, alice, addrType)

-		// And now make sure the alternate way of signing a PSBT, which
-		// is calling FinalizePsbt directly, also works for this
-		// address type.
-		assertPsbtFundSignSpend(ht, alice, fundOutputs, true)
+			// And now make sure the alternate way of signing a
+			// PSBT, which is calling FinalizePsbt directly, also
+			// works for this address type.
+			assertPsbtFundSignSpend(
+				ht, alice, fundOutputs, changeType, true,
+			)
+		}
 	}
 }

@@ -1064,7 +1075,8 @@ func assertPsbtSpend(ht *lntemp.HarnessTest, alice *node.HarnessNode,
 // assertPsbtFundSignSpend funds a PSBT from the internal wallet and then
 // attempts to sign it by using the SignPsbt or FinalizePsbt method.
 func assertPsbtFundSignSpend(ht *lntemp.HarnessTest, alice *node.HarnessNode,
-	fundOutputs map[string]uint64, useFinalize bool) {
+	fundOutputs map[string]uint64, changeType walletrpc.ChangeAddressType,
+	useFinalize bool) {

 	fundResp := alice.RPC.FundPsbt(&walletrpc.FundPsbtRequest{
 		Template: &walletrpc.FundPsbtRequest_Raw{
@@ -1075,7 +1087,8 @@ func assertPsbtFundSignSpend(ht *lntemp.HarnessTest, alice *node.HarnessNode,
 		Fees: &walletrpc.FundPsbtRequest_SatPerVbyte{
 			SatPerVbyte: 2,
 		},
-		MinConfs: 1,
+		MinConfs:   1,
+		ChangeType: changeType,
 	},
 	)
 	require.GreaterOrEqual(ht, fundResp.ChangeOutputIndex, int32(-1))
@@ -1113,6 +1126,11 @@ func assertPsbtFundSignSpend(ht *lntemp.HarnessTest, alice *node.HarnessNode,
 	finalTx, err := psbt.Extract(signedPacket)
 	require.NoError(ht, err)

+	// Check type of the change script depending on the change address
+	// type we provided in FundPsbt.
+	changeScript := finalTx.TxOut[fundResp.ChangeOutputIndex].PkScript
+	assertChangeScriptType(ht, changeScript, changeType)
+
 	var buf bytes.Buffer
 	err = finalTx.Serialize(&buf)
 	require.NoError(ht, err)
@@ -1128,6 +1146,21 @@ func assertPsbtFundSignSpend(ht *lntemp.HarnessTest, alice *node.HarnessNode,
 	ht.Miner.AssertTxInBlock(block, &finalTxHash)
 }

+// assertChangeScriptType checks if the given script has the right type given
+// the change address type we used in FundPsbt. By default, the script should
+// be a P2WPKH one.
+func assertChangeScriptType(ht *lntemp.HarnessTest, script []byte,
+	fundChangeType walletrpc.ChangeAddressType) {
+
+	switch fundChangeType {
+	case walletrpc.ChangeAddressType_CHANGE_ADDRESS_TYPE_P2TR:
+		require.True(ht, txscript.IsPayToTaproot(script))
+
+	default:
+		require.True(ht, txscript.IsPayToWitnessPubKeyHash(script))
+	}
+}
+
 // deriveInternalKey derives a signing key and returns its descriptor, full
 // derivation path and parsed public key.
 func deriveInternalKey(ht *lntemp.HarnessTest,
@@ -173,14 +173,8 @@ func psbtSendFromImportedAccount(ht *lntemp.HarnessTest, srcNode, destNode,
 	)
 	switch accountAddrType {
 	case walletrpc.AddressType_WITNESS_PUBKEY_HASH:
-		if account != defaultImportedAccount {
-			expTxFee = 141
-			expChangeScriptType = txscript.WitnessV0PubKeyHashTy
-			break
-		}
-
-		expTxFee = 153
-		expChangeScriptType = txscript.WitnessV1TaprootTy
+		expTxFee = 141
+		expChangeScriptType = txscript.WitnessV0PubKeyHashTy

 	case walletrpc.AddressType_NESTED_WITNESS_PUBKEY_HASH:
 		if account != defaultImportedAccount {
@@ -189,16 +183,26 @@ func psbtSendFromImportedAccount(ht *lntemp.HarnessTest, srcNode, destNode,
 			break
 		}

-		expTxFee = 176
-		expChangeScriptType = txscript.WitnessV1TaprootTy
+		// Spends from the default NP2WKH imported account have the same
+		// fee rate as the hybrid address type since a NP2WKH input is
+		// spent and a P2WKH change output is created.
+		fallthrough

 	case walletrpc.AddressType_HYBRID_NESTED_WITNESS_PUBKEY_HASH:
 		expTxFee = 164
 		expChangeScriptType = txscript.WitnessV0PubKeyHashTy

 	case walletrpc.AddressType_TAPROOT_PUBKEY:
-		expTxFee = 143
-		expChangeScriptType = txscript.WitnessV1TaprootTy
+		if account != defaultImportedAccount {
+			expTxFee = 143
+			expChangeScriptType = txscript.WitnessV1TaprootTy
+			break
+		}
+
+		// Spends from the default imported account fall back to a P2WKH
+		// change. We'll want to change that, but in a separate PR.
+		expTxFee = 131
+		expChangeScriptType = txscript.WitnessV0PubKeyHashTy

 	default:
 		ht.Fatalf("unsupported addr type %v", accountAddrType)
@@ -316,14 +320,8 @@ func fundChanAndCloseFromImportedAccount(ht *lntemp.HarnessTest, srcNode,
 	)
 	switch accountAddrType {
 	case walletrpc.AddressType_WITNESS_PUBKEY_HASH:
-		if account != defaultImportedAccount {
-			expChanTxFee = 153
-			expChangeScriptType = txscript.WitnessV0PubKeyHashTy
-			break
-		}
-
-		expChanTxFee = 165
-		expChangeScriptType = txscript.WitnessV1TaprootTy
+		expChanTxFee = 153
+		expChangeScriptType = txscript.WitnessV0PubKeyHashTy

 	case walletrpc.AddressType_NESTED_WITNESS_PUBKEY_HASH:
 		if account != defaultImportedAccount {
@@ -332,16 +330,26 @@ func fundChanAndCloseFromImportedAccount(ht *lntemp.HarnessTest, srcNode,
 			break
 		}

-		expChanTxFee = 188
-		expChangeScriptType = txscript.WitnessV1TaprootTy
+		// Spends from the default NP2WKH imported account have the same
+		// fee rate as the hybrid address type since a NP2WKH input is
+		// spent and a P2WKH change output is created.
+		fallthrough

 	case walletrpc.AddressType_HYBRID_NESTED_WITNESS_PUBKEY_HASH:
 		expChanTxFee = 176
 		expChangeScriptType = txscript.WitnessV0PubKeyHashTy

 	case walletrpc.AddressType_TAPROOT_PUBKEY:
-		expChanTxFee = 155
-		expChangeScriptType = txscript.WitnessV1TaprootTy
+		if account != defaultImportedAccount {
+			expChanTxFee = 155
+			expChangeScriptType = txscript.WitnessV1TaprootTy
+			break
+		}
+
+		// Spends from the default imported account fall back to a P2WKH
+		// change. We'll want to change that, but in a separate PR.
+		expChanTxFee = 143
+		expChangeScriptType = txscript.WitnessV0PubKeyHashTy

 	default:
 		ht.Fatalf("unsupported addr type %v", accountAddrType)
@@ -212,7 +212,7 @@ func (w *WalletController) ListLeasedOutputs() ([]*base.ListLeasedOutputResult,

 // FundPsbt currently does nothing.
 func (w *WalletController) FundPsbt(*psbt.Packet, int32, chainfee.SatPerKWeight,
-	string) (int32, error) {
+	string, *waddrmgr.KeyScope) (int32, error) {

 	return 0, nil
 }