Fix: Incorrect Redeem for non p2sh or p2wsh in Miniscript.ToScripts

Author: NicolasDorier

NBitcoin.Tests/MiniscriptTests.cs

@@ -223,6 +223,39 @@ public void CanGenerateSegwitAndTaprootFromMiniscript(string str, ScriptPubKeyTy
 			Assert.Equal(str, policy.DescriptorTemplate.ToString());
 		}
 
+		[Fact]
+		public void CanGenerateScripts()
+		{
+			var settings = new MiniscriptParsingSettings(Network.RegTest)
+			{
+				Dialect = MiniscriptDialect.BIP388,
+				AllowedParameters = ParameterTypeFlags.All
+			};
+			var script = Miniscript.Parse("pkh([aaaaaaaa/44h/1h/0h]tpubDDV486pBqkML6Ywhznz8DS3VS95h3q4A2pUMCc6yy739QpKMg3gA8EXGrjraDBDxrhLsezepjCEfBtak5wngDH4vMh6aXKV8hPN7JsMtdEf/<0;1>/*)", settings);
+			var expectedScripts = script.Derive(AddressIntent.Deposit, 0).Miniscript.ToScripts();
+			Assert.Equal("76a914bc1bf603880f28ffaf9c888b5660e9adf8dcadac88ac", expectedScripts.ScriptPubKey.ToHex());
+			Assert.Equal("76a914bc1bf603880f28ffaf9c888b5660e9adf8dcadac88ac", expectedScripts.ScriptCode.ToHex());
+			Assert.Null(expectedScripts.RedeemScript);
+
+			script = Miniscript.Parse("sh(pkh([aaaaaaaa/44h/1h/0h]tpubDDV486pBqkML6Ywhznz8DS3VS95h3q4A2pUMCc6yy739QpKMg3gA8EXGrjraDBDxrhLsezepjCEfBtak5wngDH4vMh6aXKV8hPN7JsMtdEf/<0;1>/*))", settings);
+			expectedScripts = script.Derive(AddressIntent.Deposit, 0).Miniscript.ToScripts();
+			Assert.Equal("a91498062e6879a7b6643735a626ac129db470087a2f87", expectedScripts.ScriptPubKey.ToHex());
+			Assert.Equal("76a914bc1bf603880f28ffaf9c888b5660e9adf8dcadac88ac", expectedScripts.ScriptCode.ToHex());
+			Assert.Equal("76a914bc1bf603880f28ffaf9c888b5660e9adf8dcadac88ac", expectedScripts.RedeemScript.ToHex());
+
+			script = Miniscript.Parse("wsh(pkh([aaaaaaaa/44h/1h/0h]tpubDDV486pBqkML6Ywhznz8DS3VS95h3q4A2pUMCc6yy739QpKMg3gA8EXGrjraDBDxrhLsezepjCEfBtak5wngDH4vMh6aXKV8hPN7JsMtdEf/<0;1>/*))", settings);
+			expectedScripts = script.Derive(AddressIntent.Deposit, 0).Miniscript.ToScripts();
+			Assert.Equal("0020df3d2f624d5744aa959d0e87cfdfb33dec486a329aee2aeee8da17c98f7e999f", expectedScripts.ScriptPubKey.ToHex());
+			Assert.Equal("76a914bc1bf603880f28ffaf9c888b5660e9adf8dcadac88ac", expectedScripts.ScriptCode.ToHex());
+			Assert.Equal("76a914bc1bf603880f28ffaf9c888b5660e9adf8dcadac88ac", expectedScripts.RedeemScript.ToHex());
+
+			script = Miniscript.Parse("sh(wsh(pkh([aaaaaaaa/44h/1h/0h]tpubDDV486pBqkML6Ywhznz8DS3VS95h3q4A2pUMCc6yy739QpKMg3gA8EXGrjraDBDxrhLsezepjCEfBtak5wngDH4vMh6aXKV8hPN7JsMtdEf/<0;1>/*)))", settings);
+			expectedScripts = script.Derive(AddressIntent.Deposit, 0).Miniscript.ToScripts();
+			Assert.Equal("a914151fd2c5490f9a6488a72fad3d8fa7bd1e3fab3387", expectedScripts.ScriptPubKey.ToHex());
+			Assert.Equal("76a914bc1bf603880f28ffaf9c888b5660e9adf8dcadac88ac", expectedScripts.ScriptCode.ToHex());
+			Assert.Equal("76a914bc1bf603880f28ffaf9c888b5660e9adf8dcadac88ac", expectedScripts.RedeemScript.ToHex());
+		}
+
 		[Fact]
 		public void CanGenerateSH()
 		{

NBitcoin/WalletPolicies/Miniscript.cs

@@ -91,7 +91,13 @@ public MiniscriptParsingSettings(Network network, KeyType? defaultKeyType = null
 	/// </summary>
 	public class Miniscript
 	{
-		public record Scripts(Script ScriptPubKey, Script? RedeemScript);
+		/// <summary>
+		/// Scripts generated by the Miniscript
+		/// </summary>
+		/// <param name="ScriptPubKey">The script that appears in the <see cref="TxOut.ScriptPubKey"></see></param>
+		/// <param name="RedeemScript">The script that must be pushed in the <see cref="TxIn.WitScript"></see> for WSH outputs, or <see cref="TxIn.ScriptSig"></see> for non-segwit P2SH outputs</param>
+		/// <param name="ScriptCode">The script that must be included in the hash algorithm used for signing the input.</param>
+		public record Scripts(Script ScriptPubKey, Script? RedeemScript, Script? ScriptCode);
 		public record TaprootInfo(TaprootInternalPubKey InternalPubKey, uint256? MerkleRoot);
 		public MiniscriptNode RootNode { get; }
 		public IReadOnlyDictionary<string, IReadOnlyCollection<MiniscriptNode.Parameter>> Parameters { get; }
@@ -730,10 +736,14 @@ public Scripts ToScripts()
 			if (Parameters.Count > 0)
 				throw new InvalidOperationException("Impossible to generate a script while parameters haven't been set. Use ToScriptString() if you want a user-readable representation instead.");
 			var scriptPubKey = RootNode.GetScript();
+			Script? scriptCode = null;
 			Script? redeem = null;
 			if (GetScriptCodeNode(RootNode) is { } n)
-				redeem = n.GetScript();
-			return new(scriptPubKey, redeem);
+				scriptCode = n.GetScript();
+			if (RootNode is Fragment f &&
+				(f.Descriptor == FragmentDescriptor.wsh || f.Descriptor == FragmentDescriptor.sh))
+				redeem = scriptCode;
+			return new(scriptPubKey, redeem, scriptCode);
 		}
 
 		/// <summary>
@@ -804,7 +814,7 @@ public DerivationResult[] Derive(DeriveParameters parameters)
 		}
 
 		/// <summary>
-		/// Make the miniscript more readable by removing HDKeys (such as [fingerprint]xpub/<0;1>/*) by key placeholders (such as @0/<0;1>/*). It is the reverse operation of <see cref="ReplaceKeyPlaceholdersByHDKeys(HDKeyNode[])"/>
+		/// Make the miniscript more readable by removing HDKeys (such as [fingerprint]xpub/&lt;0;1&gt;/*) by key placeholders (such as @0/&lt;0;1&gt;/*). It is the reverse operation of <see cref="ReplaceKeyPlaceholdersByHDKeys(HDKeyNode[])"/>
 		/// </summary>
 		/// <returns>The modified <see cref="Miniscript"/></returns>
 		public Miniscript ReplaceHDKeysByKeyPlaceholders(out HDKeyNode[] keys)

NBitcoin/WalletPolicies/MiniscriptNode.cs

@@ -529,7 +529,7 @@ public PubKey GetAggregatePubKey()
 		/// <param name="DepositIndex"></param>
 		/// <param name="ChangeIndex"></param>
 		/// <param name="Target"></param>
-		/// <param name="ShortForm">If <0;1>/* path should be be noted /** instead</param>
+		/// <param name="ShortForm">If &lt;0;1&gt;/* path should be be noted /** instead</param>
 		public record MultipathNode(int DepositIndex, int ChangeIndex, MiniscriptNode Target, bool ShortForm) : MiniscriptNode
 		{
 			internal static MultipathNode Parse(string str, Network network)