Skip to content

Commit eb09bf6

Browse files
NicolasDorierNicolasDorier
authored
Reject ineffective Miniscript relative locktimes (#1349)
1 parent 6a1d667 commit eb09bf6

4 files changed

Lines changed: 89 additions & 4 deletions

File tree

‎NBitcoin.Tests/MiniscriptTests.cs‎

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ public void CanRoundtripMiniscript(string miniscript)
7777
[InlineData("older(01)", false)]
7878
[InlineData("after(01)", false)]
7979
[InlineData("multi(01,A,B)", false)]
80-
[InlineData("older(0)", true)]
80+
[InlineData("older(0)", false)]
8181
[InlineData("older(1000)", true)]
8282
[InlineData("after(1000)", true)]
8383
public void RejectsNonCanonicalNumbers(string miniscript, bool expected)
@@ -86,6 +86,57 @@ public void RejectsNonCanonicalNumbers(string miniscript, bool expected)
8686
Assert.Equal(expected, Miniscript.TryParse(miniscript, settings, out _));
8787
}
8888

89+
[Theory]
90+
[InlineData("older(0)")]
91+
[InlineData("older(2147483648)")]
92+
public void RejectsInvalidOlderSequence(string miniscript)
93+
{
94+
var settings = new MiniscriptParsingSettings(Network.RegTest, KeyType.Classic);
95+
Assert.False(Miniscript.TryParse(miniscript, settings, out var error, out _));
96+
Assert.IsType<MiniscriptError.LocktimeExpected>(error);
97+
}
98+
99+
[Theory]
100+
[InlineData("older(1)")]
101+
[InlineData("older(65535)")]
102+
[InlineData("older(4194304)")]
103+
[InlineData("older(4259839)")]
104+
[InlineData("older(2147483647)")]
105+
public void AcceptsValidOlderSequence(string miniscript)
106+
{
107+
var settings = new MiniscriptParsingSettings(Network.RegTest, KeyType.Classic);
108+
var parsed = Miniscript.Parse(miniscript, settings);
109+
Assert.Equal(miniscript, parsed.ToString());
110+
}
111+
112+
[Theory]
113+
[InlineData(0U)]
114+
[InlineData(0x80000000U)]
115+
public void RejectsInvalidOlderSequenceReplacement(uint sequence)
116+
{
117+
var settings = new MiniscriptParsingSettings(Network.RegTest, KeyType.Classic) { AllowedParameters = ParameterTypeFlags.NamedParameter };
118+
var parsed = Miniscript.Parse("older(A)", settings);
119+
var exception = Assert.Throws<MiniscriptReplacementException>(() => parsed.ReplaceParameters(new()
120+
{
121+
["A"] = new MiniscriptNode.Value.LockTimeValue(sequence)
122+
}));
123+
Assert.IsType<MiniscriptNode.ParameterRequirement.RelativeLocktime>(exception.Requirement);
124+
}
125+
126+
[Theory]
127+
[InlineData(1U)]
128+
[InlineData(0x00400000U)]
129+
[InlineData(0x7fffffffU)]
130+
public void AcceptsValidOlderSequenceReplacement(uint sequence)
131+
{
132+
var settings = new MiniscriptParsingSettings(Network.RegTest, KeyType.Classic) { AllowedParameters = ParameterTypeFlags.NamedParameter };
133+
var parsed = Miniscript.Parse("older(A)", settings).ReplaceParameters(new()
134+
{
135+
["A"] = new MiniscriptNode.Value.LockTimeValue(sequence)
136+
});
137+
Assert.Equal($"older({sequence})", parsed.ToString());
138+
}
139+
89140
[Theory]
90141
[InlineData("thresh(0)", KeyType.Classic)]
91142
[InlineData("multi(0)", KeyType.Classic)]

‎NBitcoin/WalletPolicies/Miniscript.cs‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,24 @@ private static bool TryParseLocktime(ParsingContext ctx, [MaybeNullWhen(true)] o
280280
error = new MiniscriptError.LocktimeExpected(ctx.Offset);
281281
return false;
282282
}
283+
private static bool TryParseSequence(ParsingContext ctx, [MaybeNullWhen(true)] out MiniscriptError error, [MaybeNullWhen(false)] out MiniscriptNode node)
284+
{
285+
var offset = ctx.Offset;
286+
if (!TryParseLocktime(ctx, out error, out node))
287+
return false;
288+
if (node is Parameter parameter)
289+
{
290+
node = parameter with { Requirement = ParameterRequirement.RelativeLocktime.Instance };
291+
return true;
292+
}
293+
if (!ParameterRequirement.RelativeLocktime.Instance.Check(node))
294+
{
295+
node = null;
296+
error = new MiniscriptError.LocktimeExpected(offset);
297+
return false;
298+
}
299+
return true;
300+
}
283301

284302
private static bool TryParseNamedParameter(ParsingContext ctx, ParameterRequirement requirement, [MaybeNullWhen(false)] out MiniscriptNode node)
285303
{
@@ -562,7 +580,7 @@ private static bool TryParseExpression(ParsingContext ctx, [MaybeNullWhen(true)]
562580
"pk_h" => TryParseParameters(ctx, 1, TryParseKey, out error, out var p) ? FragmentSingleParameter.pk_h(p[0]) : null,
563581
"pk" => TryParseParameters(ctx, 1, TryParseKey, out error, out var p) ? FragmentSingleParameter.pk(p[0]) : null,
564582
"pkh" => TryParseParameters(ctx, 1, TryParseKey, out error, out var p) ? FragmentSingleParameter.pkh(p[0]) : null,
565-
"older" => TryParseParameters(ctx, 1, TryParseLocktime, out error, out var p) ? FragmentSingleParameter.older(p[0]) : null,
583+
"older" => TryParseParameters(ctx, 1, TryParseSequence, out error, out var p) ? FragmentSingleParameter.older(p[0]) : null,
566584
"after" => TryParseParameters(ctx, 1, TryParseLocktime, out error, out var p) ? FragmentSingleParameter.after(p[0]) : null,
567585
"sha256" => TryParseParameters(ctx, 1, TryParse32Bytes, out error, out var p) ? FragmentSingleParameter.sha256(p[0]) : null,
568586
"ripemd160" => TryParseParameters(ctx, 1, TryParse20Bytes, out error, out var p) ? FragmentSingleParameter.ripemd160(p[0]) : null,

‎NBitcoin/WalletPolicies/MiniscriptNode.cs‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -868,6 +868,19 @@ public sealed override string ToString()
868868
return "A LockTimeValue is expected";
869869
}
870870
}
871+
public record RelativeLocktime : ParameterRequirement
872+
{
873+
public readonly static RelativeLocktime Instance = new();
874+
public override bool Check(MiniscriptNode node)
875+
{
876+
return node is Value.LockTimeValue locktime && locktime.LockTime.Value != 0 &&
877+
(locktime.LockTime.Value & NBitcoin.Sequence.SEQUENCE_LOCKTIME_DISABLE_FLAG) == 0;
878+
}
879+
public sealed override string ToString()
880+
{
881+
return "A relative LockTimeValue between 1 and 2147483647 is expected";
882+
}
883+
}
871884
}
872885
public record Parameter(string Name, ParameterRequirement Requirement) : MiniscriptNode
873886
{

‎NBitcoin/WalletPolicies/Visitors/MockScriptVisitor.cs‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,12 @@ private MiniscriptNode CreateDummy(Parameter p, string? paramName = null)
6363
replacements.Add((Encoders.Hex.EncodeData(bytes), $"<{p.Name}>"));
6464
modifiedParameter = new Value.HashValue(bytes);
6565
}
66-
else if (p.Requirement is MiniscriptNode.ParameterRequirement.Locktime l)
66+
else if (p.Requirement is MiniscriptNode.ParameterRequirement.Locktime or MiniscriptNode.ParameterRequirement.RelativeLocktime)
6767
{
68-
var rand = new LockTime(RandomUtils.GetUInt32());
68+
var value = RandomUtils.GetUInt32();
69+
if (p.Requirement is MiniscriptNode.ParameterRequirement.RelativeLocktime)
70+
value = (value & ~Sequence.SEQUENCE_LOCKTIME_DISABLE_FLAG) | 1U;
71+
var rand = new LockTime(value);
6972
replacements.Add((Encoders.Hex.EncodeData(Op.GetPushOp(rand.Value).PushData), $"<{p.Name}>"));
7073
modifiedParameter = new Value.LockTimeValue(rand);
7174
}

0 commit comments

Comments
 (0)