Support integer and decimal Nano amounts to raw conversion

[RickiNano]

This adds support for decimal numbers when converting from Nano to raw. It is also backwards compatible with integer amounts both expressed like "35" or like "35.00000" Decimals beyond 30 are cut to conform with the precision specifications Invalid numbers will still show an error message This pr fixes #3777

[clemahieu]

We should look at a real number parser rather than manually slicing the string up. https://www.boost.org/doc/libs/1_75_0/libs/spirit/doc/html/spirit/qi/reference/numeric/real.html

This also needs tests, specifically showing what it does to extreme values.

[clemahieu]

I like this strategy better than a separate RPC, we just need to make sure rounding errors can't happen.

[RickiNano]

I have added a bunch of testcases including all the edgecases I could think of. They run successfully with this implementation. I'm hesitant to use the Boost float library. I don't have the required skills and I'm also worried if the library has the precision needed to store floating values from 0.00000000000000000000000000001 (one raw in Nano) and up to 13324829600000000000000000000000000000 (max supply as raw) I feel more confident about handling the integer and integral parts as two separate numbers. This way the integer calculation is virtually unchanged compared to the current version, while only adding the integral part

[nano2dev]

@RickiNano Whats the status of this? Waiting on OK?

I need this more than ever and the bounty is still waiting to be claimed!

[RickiNano]

It's all up to @clemahieu if this gets approved. I've done a lot of testing on this and it's working correctly I've also created the following testcases that runs successfully: nano_to_raw_one_nano nano_to_raw_zero_nano nano_to_raw_one_tenth_nano nano_to_raw_one_raw nano_to_raw_value_less_than_one_raw nano_to_raw_ten_million_nano nano_to_raw_negative_value nano_to_raw_invalid_input nano_to_raw_zero_as_decimal nano_to_raw_integer_as_decimal If needed I can also code a tool that will do millions of random value requests to the old and the new api, and make sure that all integer numbers are handled the same to ensure 100% backward compatibility

[nano2dev]

Nice @RickiNano

@clemahieu Sorry for all the pings. Check this out when you get a chance.

Nano Node shipping 🚢 with Floating point conversion RPC makes for super fast prototyping w/ Nano!

[dsiganos]

I haven't looked at this closely but I agree with the idea that doing our own slicing of the string and parsing it ourselves is safer in short and medium term. Floating number parsing is such a complicated task that if we left it to a library, it would be even harder to know exactly what the parsing rules were. Let's make our own very simple and very restrictive rules about what we parse.

[RickiNano]

Hi @dsiganos I have decided to use regex to validate the input. This makes validation strict and inputs are rejected early on. The required input format is either Digits only or if we have a dot then we require digits on both sides of the dot. All other inputs are rejected The regex used is ^\d+(.\d+)?$ You can try it out with something like regex101.com if you want I have also included all the testcases that you asked for and they run successfully I've added documentation and the code should also be a lot more readable now. I have also addressed the points you've made in your review

[dsiganos]

I find the use of regex in this case over the top and a slight complication. It would be extremely easy to perform the same check with pure C/C++.

[RickiNano]

I have rolled back my attempt to convert the regex to c++ code I lack the skills to do this myself so I hope someone can do this for me.

[dsiganos]

@RickiNano , @nano2dev you could write a specification for the allowed incoming string to be converted. For example, what is the max number, what is the smallest number, etc. so it can then be coded.

[RickiNano]

@dsiganos I had already written a summary for the method but I have updated it just now. It now says:

/// <summary>
/// Converts a Nano value with a maximum of 30 decimals into the corresponding raw value.
/// Accepted inputs are either digits only, or digits seperated by one dot. Any other format is rejected
/// Minimum value is 0 and maximum is 340 million
/// </summary>
/// <returns>raw value</returns>

I think this description is concise but still addresses all input validations I have also updated the maximum amount to 340 million. Unit tests have also been updated to reflect this. Btw, the current v24 implementation has no maximum amount and will return faulty values for inputs above 340 million caused by int128 overflow

[nano2dev]

Just commenting to say Thank You everyone involved.

I understand this PR may be controversial from an implementation standpoint.

My hope is that it gets shipped sooner than later - and it gets better with time.

I suspect, that I will be the only company to adopt this new feature immediately.

As most bring their own Big Num library.

My use case is Bash. Where no Big Num library exists.

On that note. This PR will make the Node much more "self-sufficient". Specially in Low level language ecosystems, where user input is Decimals.

[nano2dev]

@RickiNano Check this out when you get a chance:

https://github.com/RickiNano/nano-node/pull/1

Thanks @marcusmmmz

[RickiNano]

@dsiganos I have tested and merged @marcusmmmz 's changes. The validation is now done in c++ instead of using regex

[dsiganos]

Guys, I still do not see parsing rules. Where are the rules? What's allowed, what is not? The rules need to answer all the questions I had posted at an earlier message. then we can check the code against the rules.

[marcusmmmz]

@dsiganos RickiNano already wrote out the parsing rules over here: https://github.com/nanocurrency/nano-node/pull/3781#issuecomment-1247105938

They're intentionally very simple:

• Minimum amount is 0
• Maximum amount is 340 million
• Maximum of 30 decimals
• And it can be either:

1. Just a number sequence
2. Number sequence + "." + Number sequence

That's all the rules, no leading dots, no negative/positive signs, no hex, no scientific notation.

[dsiganos]

How many characters are allowed in the whole number section? What is the max string length? Can we check for that explicitly before doing any other processing?

[dsiganos]

Is this legal? the current rules do not say. 000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001

[marcusmmmz]

How many characters are allowed in the whole number section? What is the max string length? Can we check for that explicitly before doing any other processing?

As 340 million is the max value (9 characters), and the maximum of decimals is 30, plus the "." character, that equals 40 characters, so that's the max string length. So yes, we could easily check that before doing other processing.

Is this legal? the current rules do not say. 000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001

Currently that's not being checked, but it can easily be done without adding too much complexity so i think we should add that check. I'll be working on this.

Edit: RickiNano's next commit supersedes this.

[RickiNano]

@dsiganos I've added these extra validations:

• The integer part is tested for a maximum length of 9
• Leading zeros are now being removed

The validation rules are:

/// 9 characters [0-9] allowed
/// Optionally followed by up to one '.' character and then 30 characters [0-9]
/// Max value is 340 million

I have also added a test case with leading zeros

[nano2dev]

@dsiganos Please check this when possible :)

No rush on getting it shipped.

I intend on paying bounties when you simply give the 👍🏽

[nano2dev]

@RickiNano Do you mind providing nano address for reward. The 100 NANO prize is split 50/50 with @marcusmmmz .

Hope this PR makes in when adequate.

[marcusmmmz]

@dsiganos Have you reviewed what's left to be reviewed yet? it's been quite some time and we'd like at least some instructions on what to do next.

[dsiganos]

This change is controversial, Colin for example doesn't want it (because he regards it to be beyond the scope of nano node), but he will reluctantly accept it, if it is demonstrated that it is safe.

To finish this PR we need:

• better specification of the parsing rules
• better C++ coding
• to demonstrate that it is safe for the node and for the programmer using the feature
• documented well

In article the below, I say: "During the process of contributing, a relationship, trust and a common understanding is being built between the core team and the contributor. This takes time but it is the most important element of the process. Many people forget this and do not give it the care and attention it deserves." https://nano.org/en/blog/how-to-make-c-contributions-to-nano-node-development--cfc11ce8 A common understanding never got established here. This (and it being a low priority) are the primary reasons this PR did not progress.

I am currently trying to clean up existing PRs. Let's revisit this on Monday. I will work on this PR Monday and get it over the line.

[dsiganos]

Apparently this was closed by mistake so I am reopening.