Fix: Taproot PSBT's outputs were not including internal key and taproot hd information (#495)

Author: NicolasDorier

NBXplorer.Tests/UnitTest1.cs

@@ -365,9 +365,9 @@ public async Task CanCreatePSBT()
 				Assert.NotNull(spendingPSBT.Inputs[0].WitnessUtxo);
 				///////////////////////////
 
-				//CanCreatePSBTCore(tester, ScriptPubKeyType.SegwitP2SH);
-				//CanCreatePSBTCore(tester, ScriptPubKeyType.Segwit);
-				//CanCreatePSBTCore(tester, ScriptPubKeyType.Legacy);
+				CanCreatePSBTCore(tester, ScriptPubKeyType.SegwitP2SH);
+				CanCreatePSBTCore(tester, ScriptPubKeyType.Segwit);
+				CanCreatePSBTCore(tester, ScriptPubKeyType.Legacy);
 				CanCreatePSBTCore(tester, ScriptPubKeyType.TaprootBIP86);
 
 				// If we build a list of unconf transaction which is too long, the CreatePSBT should
@@ -471,6 +471,11 @@ private static void CanCreatePSBTCore(ServerTester tester, ScriptPubKeyType type
 					Assert.Empty(input.HDKeyPaths);
 					Assert.Single(input.HDTaprootKeyPaths);
 					Assert.NotNull(input.TaprootInternalKey);
+
+					var output = Assert.Single(minimumInputs.PSBT.Outputs, o => o.ScriptPubKey == minimumInputs.ChangeAddress.ScriptPubKey);
+					Assert.NotNull(output.TaprootInternalKey);
+					Assert.Empty(output.HDKeyPaths);
+					Assert.NotEmpty(output.HDTaprootKeyPaths);
 				}
 				Assert.Equal(2, spendAllOutpoints.PSBT.Inputs.Count);
 			}
@@ -849,8 +854,8 @@ private static void CanCreatePSBTCore(ServerTester tester, ScriptPubKeyType type
 				ReserveChangeAddress = false
 			});
 			Assert.Equal(3, psbt2.PSBT.Outputs.Count);
-			Assert.Equal(2, psbt2.PSBT.Outputs.Where(o => o.HDKeyPaths.Any()).Count());
-			Assert.Single(psbt2.PSBT.Outputs, o => o.HDKeyPaths.Any(h => h.Value.KeyPath == newAddress.KeyPath));
+
+			AssertHasOutput(type, newAddress.KeyPath, psbt2);
 			foreach (var input in psbt2.PSBT.GetGlobalTransaction().Inputs)
 			{
 				Assert.Equal(Sequence.Final, input.Sequence);
@@ -914,8 +919,9 @@ private static void CanCreatePSBTCore(ServerTester tester, ScriptPubKeyType type
 			Assert.Equal(new KeyPath("49'/0'"), globalXPub.KeyPath);
 
 			Assert.Equal(3, psbt2.PSBT.Outputs.Count);
-			Assert.Equal(2, psbt2.PSBT.Outputs.Where(o => o.HDKeyPaths.Any()).Count());
-			var selfchange = Assert.Single(psbt2.PSBT.Outputs, o => o.HDKeyPaths.Any(h => h.Key.GetAddress(type, tester.Network).ScriptPubKey == newAddress.ScriptPubKey));
+
+			var selfchange = AssertHasOutput(type, new KeyPath("49'/0'").Derive(newAddress.KeyPath), psbt2);
+
 			Assert.All(psbt2.PSBT.Inputs.Concat<PSBTCoin>(new[] { selfchange }).SelectMany(i => i.HDKeyPaths), i =>
 			{
 				Assert.Equal(rootHD, i.Value.MasterFingerprint);
@@ -1022,6 +1028,20 @@ private static void CanCreatePSBTCore(ServerTester tester, ScriptPubKeyType type
 			}
 		}
 
+		private static PSBTOutput AssertHasOutput(ScriptPubKeyType type, KeyPath keyPath, CreatePSBTResponse psbt2)
+		{
+			if (type == ScriptPubKeyType.TaprootBIP86)
+			{
+				Assert.Equal(2, psbt2.PSBT.Outputs.Where(o => o.HDTaprootKeyPaths.Any()).Count());
+				return Assert.Single(psbt2.PSBT.Outputs, o => o.HDTaprootKeyPaths.Any(h => h.Value.RootedKeyPath.KeyPath == keyPath));
+			}
+			else
+			{
+				Assert.Equal(2, psbt2.PSBT.Outputs.Where(o => o.HDKeyPaths.Any()).Count());
+				return Assert.Single(psbt2.PSBT.Outputs, o => o.HDKeyPaths.Any(h => h.Value.KeyPath == keyPath));
+			}
+		}
+
 		[TheoryWithTimeout]
 		[InlineData(true)]
 		[InlineData(false)]
@@ -2563,16 +2583,6 @@ public async Task CanTrackAddress()
 			}
 		}
 
-		[Fact]
-		public async Task Test()
-		{
-			var rpc = new RPCClient(new RPCCredentialString()
-			{
-				UserPassword = new NetworkCredential("dashrpc", "PQQgOzs1jN7q2SWQ6TpBNLm9j"),
-			}, "https://dash-testnet.nodes.m3t4c0.xyz", AltNetworkSets.Dash.Testnet);
-			var b1 = await rpc.GetBlockAsync(new uint256("000001f02c1623e0bb12b54ac505cefdfca3f0f664bf333fc73ae5eafe34b830"));
-		}
-
 		[FactWithTimeout]
 		public async Task CanTrack2()
 		{

NBXplorer/Controllers/MainController.PSBT.cs

@@ -450,21 +450,22 @@ private async Task UpdatePSBTCore(UpdatePSBTRequest update, NBXplorerNetwork net
 				// * HDTaprootKeyPaths is used instead of HDKeyPaths
 				// * TaprootSighashType is explicitely set to default
 				// * Fill up TaprootInternalKey
-				foreach (var input in update.PSBT.Inputs)
+				foreach (var c in update.PSBT.Inputs.OfType<PSBTCoin>().Concat(update.PSBT.Outputs))
 				{
-					input.TaprootSighashType = TaprootSigHash.Default;
-					if (input.HDKeyPaths.Count != 1)
+					if (c is PSBTInput input)
+						input.TaprootSighashType = TaprootSigHash.Default;
+					if (c.HDKeyPaths.Count != 1)
 						continue;
-					foreach (var keypath in input.HDKeyPaths)
+					foreach (var keypath in c.HDKeyPaths)
 					{
 						var taprootPubKey = keypath.Key.GetTaprootFullPubKey();
-						input.TaprootInternalKey = taprootPubKey.InternalKey;
+						c.TaprootInternalKey = taprootPubKey.InternalKey;
 						// Some consumers expect the internal key to be in the HDTaprootKeyPaths
-						if (!TaprootPubKey.TryCreate(input.TaprootInternalKey.ToBytes(), out var pk))
+						if (!TaprootPubKey.TryCreate(c.TaprootInternalKey.ToBytes(), out var pk))
 							continue;
-						input.HDTaprootKeyPaths.AddOrReplace(pk, new TaprootKeyPath(keypath.Value));
+						c.HDTaprootKeyPaths.AddOrReplace(pk, new TaprootKeyPath(keypath.Value));
 					}
-					input.HDKeyPaths.Clear();
+					c.HDKeyPaths.Clear();
 				}
 			}
 		}

NBXplorer/NBXplorer.csproj

@@ -3,7 +3,7 @@
     <OutputType>Exe</OutputType>
     <TargetFramework Condition="'$(TargetFrameworkOverride)' == ''">net8.0</TargetFramework>
     <TargetFramework Condition="'$(TargetFrameworkOverride)' != ''">$(TargetFrameworkOverride)</TargetFramework>
-    <Version>2.5.15</Version>
+    <Version>2.5.16</Version>
 	  <DocumentationFile>bin\$(Configuration)\$(TargetFramework)\NBXplorer.xml</DocumentationFile>
 	  <NoWarn>1701;1702;1705;1591;CS1591</NoWarn>
 	  <LangVersion>12</LangVersion>