Update to Solc 0.5

[gskapka]

Solc 0.5.0 Version of the API

Here it is, the API upgraded to the latest Solidity compiler, and (hopefully) following all the best practices. Rather than simply getting it to compile, many other changes have been made and so hence the need for a thorough review. Amongst those changes:

• Added stuff to make it compile (a million memory keywords &c.)
• More explicit castings (which broke stuff - see later) between types
• Adding a safe version of parseInt() (original remains incase anyone using its behaviour)
• Moved all global variables to the top of the contract
• Moved all modifiers together
• Standardized comment styles (/* ... */ for multilines instead of multiple //'s)
• Standardized grammar in comments
• Standardized white space between lines
• Standardized variable naming (the few examples of snake_case are now camelCased)
• Standardized use of whitespace between function declaration parens. vs. if/while/for conditionals
• Standardized the use of whitespace between closing parens. & {s
• Standardized the use of underscores before parameter names
• Fix the failing RandomDS proof (which I broke during the above)

Notes:

Re which latter bullet point: It turns out that with the new recasting rules we must first cast to types of the same size. So to go from bytes(1) to uint we first need to go via either uint8 or bytes32:

uint256(bytes32(bytes1(...)));
// or
uint256(uint8(bytes1(...)));

The two casts pad differently hence the new explicitness ruling. As such, I broke (and then fixed) the proof verification thusly:

To Do

@D-Nice & @riccardopersiani please can you go over this PR with a fine tooth comb and make sure I haven't goofed anything? Also tell me if any of the above listed changes are a step too far/pointless/terrible/should be burnt with fire &c., &c.

Also, I have a private repo elsewhere with some (unpolished but passing) tests for some of the parsing helpers we offer in this API, plus all the truffle examples in the ethereum-examples repo have been updated to als use this API and all tests therein are also passing.
So it should be all good w/r/t it working, now it's just a case of arguing over the changes I've made.

[D-Nice]

shall we also decide on a standard as to whether to return anonymous or named variables?

[gskapka]

Easy question for me - named variables everytime (unless we're talking point-free - but that's a different & more fun kettle of fish!). That whole:

naming is hard

thing is dumb. Naming is explicit and useful and enlightening.

[gskapka]

Update:

Okay so I've done anything from the above review that I can & pushed it. I've also compiled anything I couldn't do into this one post to make it easier to see & address.

Done:

:heavy_check_mark: Naming return params (and adding underscores to go along with current practices) :heavy_check_mark: Fixing the license typo. :heavy_check_mark: Removing white space from string [] type declarations.

I can confirm that all tests are passing on all examples that have test-suites per my tool on gitlab using this current iteration of the API.

---

Stuff that needs discussing/addressing:

:pager: Paging @bertani & @D-Nice, whose relevance to issues at hand are marked after the issues:

• Do we update the copyright information? (@bertani)  
• Should getPrice become view? (@bertani, @D-Nice)  
• Do we remove the coupon function? (@bertani)  
• Do we drop the safeMemoryCleaner function? (@bertani)  
• Do we revert with a message on finding an unexpectedly high price in a query? (@bertani)  
• Do we use view, pure &c., even where it results in a lot more compiler warnings? (@bertani, @D-Nice)  
• Do you hate my compulsion & aesthetic driven re-ordering of listed variables into length order enough to make me revert it? (@D-Nice)

[gskapka]

Responses to above from @bertani

• Re copyright: No, it's fine as is.  
• Re getPrice becoming view: Whatever keeps compatability with the current connector.  
• Re coupon: Can be removed.  
• Re safeMemoryCleaner: Since @D-Nice implemented it, if it's no longer patching whatever it patched it can be dropped.  
• Re revert, if we didn't throw in the first place, the return value was probably there for a reason so it's probably best to keep the current behaviour.  
• Re compiler warnings: if they don't change behaviour, it'd be best to minimise compiler warnings at the expense of being explicit with the visibility specifiers.

---

Next:

:balance_scale: Just need you to weigh in now @D-Nice please!

[D-Nice]

ok, based on those replies, let's remove coupon. Also, based on our earlier discussion, and me diving into safeMemoryCleaner, I think it's best we still leave it in, because as mentioned, it can be useful for any other assembly that works on memory, but doesn't necessarily zero it out, so the safeMemoryCleaner can be re-used there.

[gskapka]

Done & done.

[axic]

Oh wow, this is great! :)