Prepare Parser for scripting by joemphilips · Pull Request #694 · MetacoSA/NBitcoin

joemphilips · 2019-05-22T05:50:37Z

Original PR is #684

This creates NBitcoin.Scripting.Parser namespace, and puts Monadic Parser combinator for internal usage.
It does not affect the library's public AP,I nor it has tests. Tests will be done in following PRs.

The API almost completely follows those of Sprache, the biggest difference is that it is generalized against its input type. (Sprache only allows string as an input, but this one will take arbitrary IEnumerable<T> for input.) This is crucial for Miniscript decompilation.

This might be a good candidate for using Span<T> as an input type to improve the speed. But I didn't want to do an early optimization so I didn't.

NicolasDorier · 2019-05-22T06:34:36Z

Awesome, I suggest putting all those classes internal?

NicolasDorier · 2019-05-22T06:36:33Z

NBitcoin/Scripting/Parser/IInput.cs

+
+namespace NBitcoin.Scripting.Parser
+{
+    public interface IInput<out T>


This one should be internal ?

Also, I would suggest to inherit from IEnumerator<out T> and remove Advance/GetCurrent/GetNext.
Then just make methods returning IInput<out T> before implement IEnumerable<T> and return IInput<T> .

The nice thing is that people can use foreach then.

Nice. I wonder why this didn't come up to my mind...

I've been trying to inherit from IEnumerator. But it is not working well :(
It is because that Input for Parser combinator must be immutable. (Advance() will create another instance each time it proceeds)
IEnumerator is only for mutable object.
I think inheriting from IEnumerable and delegate GetEnuemrator call to underlying Source object is enough.

NBitcoin/Scripting/Parser/ParseException.cs

knocte · 2019-05-23T06:59:48Z

IEnumerator is only for mutable object.

Not sure I agree. You could create a specific Enumerator class for this type which is mutable without making your type mutable.

joemphilips · 2019-05-23T09:24:29Z

Not sure I agree. You could create a specific Enumerator class for this type which is mutable without making your type mutable.

You mean make my class inherit from IEnumerable<T> instead of IEnumerator<T> right? That's what I have done here.

NicolasDorier · 2019-05-23T12:11:22Z

@joemphilips if my suggestion is not really used and does not make the code more visible, then you can just roll it back.

NicolasDorier · 2019-05-23T12:11:51Z

NBitcoin/Scripting/Parser/Parse.Char.cs

+		{
+			return from nothing in Return<char, string>("")
+					   // dummy so that CultureInfo.CurrentCulture is evaluated later
+				   from dot in String((ci ?? CultureInfo.CurrentCulture).NumberFormat.NumberDecimalSeparator).Text()


oh no, never use CurrentCulture!

I've drop the support for culture-aware decimal.
now it uses InvariantCulture only.

NicolasDorier · 2019-05-23T12:14:33Z

Oh actually it is used in the test, seems better.

…ption

…matException

NicolasDorier · 2019-05-25T12:57:40Z

Tests failing rebasing on top of master should fix it

joemphilips · 2019-05-25T14:23:37Z

Rebased but not fixed.

It seems that some parts of codes are using obsolete PubKey.GetAddress

BIP38\BitcoinConfirmationCode.cs(125,58): warning CS0618: 'PubKey.GetAddress(Network)' is obsolete: 'Use GetAddress(ScriptPubKeyType.Legacy, network) instead' [C:\projects\nbitcoin\NBitcoin\NBitcoin.csproj]
BIP38\BitcoinEncryptedSecret.cs(39,49): warning CS0618: 'PubKey.GetAddress(Network)' is obsolete: 'Use GetAddress(ScriptPubKeyType.Legacy, network) instead' [C:\projects\nbitcoin\NBitcoin\NBitcoin.csproj]
BIP38\BitcoinEncryptedSecret.cs(94,49): warning CS0618: 'PubKey.GetAddress(Network)' is obsolete: 'Use GetAddress(ScriptPubKeyType.Legacy, network) instead' [C:\projects\nbitcoin\NBitcoin\NBitcoin.csproj]
BIP38\BitcoinEncryptedSecret.cs(202,27): warning CS0618: 'PubKey.GetAddress(Network)' is obsolete: 'Use GetAddress(ScriptPubKeyType.Legacy, network) instead' [C:\projects\nbitcoin\NBitcoin\NBitcoin.csproj]
BIP38\BitcoinPassphraseCode.cs(236,27): warning CS0618: 'PubKey.GetAddress(Network)' is obsolete: 'Use GetAddress(ScriptPubKeyType.Legacy, network) instead' [C:\projects\nbitcoin\NBitcoin\NBitcoin.csproj]
BIP38\BitcoinConfirmationCode.cs(125,58): warning CS0618: 'PubKey.GetAddress(Network)' is obsolete: 'Use GetAddress(ScriptPubKeyType.Legacy, network) instead' [C:\projects\nbitcoin\NBitcoin\NBitcoin.csproj]
BIP38\BitcoinEncryptedSecret.cs(39,49): warning CS0618: 'PubKey.GetAddress(Network)' is obsolete: 'Use GetAddress(ScriptPubKeyType.Legacy, network) instead' [C:\projects\nbitcoin\NBitcoin\NBitcoin.csproj]
BIP38\BitcoinEncryptedSecret.cs(94,49): warning CS0618: 'PubKey.GetAddress(Network)' is obsolete: 'Use GetAddress(ScriptPubKeyType.Legacy, network) instead' [C:\projects\nbitcoin\NBitcoin\NBitcoin.csproj]
BIP38\BitcoinEncryptedSecret.cs(202,27): warning CS0618: 'PubKey.GetAddress(Network)' is obsolete: 'Use GetAddress(ScriptPubKeyType.Legacy, network) instead' [C:\projects\nbitcoin\NBitcoin\NBitcoin.csproj]
BIP38\BitcoinPassphraseCode.cs(236,27): warning CS0618: 'PubKey.GetAddress(Network)' is obsolete: 'Use GetAddress(ScriptPubKeyType.Legacy, network) instead'

Should I fix on my side?

NicolasDorier · 2019-05-25T14:34:40Z

Yes you can fix... but so strange, on master the build is passing!

NBitcoin/Scripting/Parser/StringInput.cs

* Since in netstandard1.3, string-as-enumerable should be treated bit differently.

NicolasDorier · 2019-05-27T11:52:52Z

What should I review and merge in priority after @joemphilips ?

joemphilips · 2019-05-27T13:27:24Z

I want you to take a look for #696 since the spec is well formalized than #695 , and it has some changes to an existing objects.

But honestly, I myself am not in hurry so please take your time. @NicolasDorier

joemphilips force-pushed the prepare_parser branch 2 times, most recently from c9c9e21 to 54b1b2c Compare May 22, 2019 06:05

This was referenced May 22, 2019

Support miniscript #695

Closed

Support output descriptor #696

Merged

Support Miniscript (with C#) #684

Closed

NicolasDorier reviewed May 22, 2019

View reviewed changes

NicolasDorier reviewed May 23, 2019

View reviewed changes

NBitcoin/Scripting/Parser/ParseException.cs Outdated Show resolved Hide resolved

joemphilips force-pushed the prepare_parser branch 3 times, most recently from 2fc36c9 to 493ec7f Compare May 23, 2019 06:44

NicolasDorier reviewed May 23, 2019

View reviewed changes

joemphilips added 7 commits May 25, 2019 18:47

Prepare parser for scripting

827da0f

Make everything except ParseException in Parser module internal

16d6ad1

Rename ParseException -> ParsingException and inherit from FormatExce…

e6276b9

…ption

Make IInput IEnumerable

2784fda

Remove unnecessary GetNext from IInput

b3d6411

fixup! Rename ParseException -> ParsingException and inherit from For…

75d3f0f

…matException

Support only InvariantCulture Decimal in Parser

7b28689

joemphilips force-pushed the prepare_parser branch from fd4b479 to 7b28689 Compare May 25, 2019 14:18

Fix compiler warning for using obsolete GetAddress(Network)

085634f

NicolasDorier reviewed May 25, 2019

View reviewed changes

NBitcoin/Scripting/Parser/StringInput.cs Outdated Show resolved Hide resolved

joemphilips force-pushed the prepare_parser branch from 63a7d08 to 4453369 Compare May 25, 2019 14:59

joemphilips force-pushed the prepare_parser branch from 4453369 to 323e787 Compare May 25, 2019 15:04

Cast StringInput.Source to propertype beforing getting enumerator.

961a6f4

* Since in netstandard1.3, string-as-enumerable should be treated bit differently.

joemphilips force-pushed the prepare_parser branch from 323e787 to 961a6f4 Compare May 25, 2019 23:55

NicolasDorier merged commit 7531e26 into MetacoSA:master May 27, 2019

joemphilips deleted the prepare_parser branch May 27, 2019 09:58

Conversation

joemphilips commented May 22, 2019

Uh oh!

NicolasDorier commented May 22, 2019

Uh oh!

NicolasDorier May 22, 2019

Choose a reason for hiding this comment

Uh oh!

NicolasDorier May 22, 2019

Choose a reason for hiding this comment

Uh oh!

joemphilips May 22, 2019

Choose a reason for hiding this comment

Uh oh!

joemphilips May 23, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

knocte commented May 23, 2019

Uh oh!

joemphilips commented May 23, 2019

Uh oh!

NicolasDorier commented May 23, 2019

Uh oh!

NicolasDorier May 23, 2019

Choose a reason for hiding this comment

Uh oh!

joemphilips May 23, 2019

Choose a reason for hiding this comment

Uh oh!

NicolasDorier commented May 23, 2019

Uh oh!

NicolasDorier commented May 25, 2019

Uh oh!

joemphilips commented May 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NicolasDorier commented May 25, 2019

Uh oh!

Uh oh!

NicolasDorier commented May 27, 2019

Uh oh!

joemphilips commented May 27, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Comments

joemphilips commented May 25, 2019 •

edited

Loading