Post audit tweaks

[lightclient]

We've had a few really nice audits done on this code and things have generally been positive. There are a few pieces of feedback that I think are worth addressing before we settle on the final code though. Here is a list, ordered by priority:

1. Verify timestamp is non-zero -- currently the implementation allows an input of zero to return a zero root. This is because the current mainnet timestamp is not perfectly divisible by our modulus. Although no proof could be authenticated against the zero root, it's still an ugly correctness edge case that I think should be addressed.
2. Update buflen to 93600 -- this value will share 500-1000¹ more remainders if the slot time were to change versus the current modulus, decreasing the overall storage use of the contract (h/t @adietrichs).
3. Load calldata once -- minor nit, but in case the cost of retrieving calldata changes in the future, it would make more sense to only access it via calldatacopy a single time, instead of paying for that cost at each load. This require a bit of dup'ing and swap'ing, but still extremely manageable. As a part of this I was also able to delete the get_input macro (h/t @adietrichs).

Footnotes

1. https://gist.github.com/lightclient/820a0d5c5861ed09c5cae5e384e9779e ↩

[holiman]

Don't forget to update the readme (and any other reference) with the new code, so the repo is consistent.

[holiman]

I have a few more nits. I don't like that revert is a label, revert() is a macro, and, well, revert is also an opcode. I tried to change this earlier, but I think it has crept back. What I would want is

• lbl_revert for the label (and lbl_-prefix for all labels)
• macro_revert for the macro

I'll make a PR

[holiman]

This change is fine, IMO, but if one were to complain, it would be that the flow is a bit awkward.

1. Check-length, if ok: goto loadtime,
2. fail revert
3. loadtime: check input == zero, if bad, goto revert
4. verify stored==load, if ok, goto loadroot
5. fail revert
6. loadroot: ...

The goto revert is used exactly once, and the inlined macro revert is used twice (three times if we count the usage inside the goto revert). A more consistent control-flow would be

1. Check-length, if bad: goto revert,
2. Check input == zero, if bad, goto revert
3. loads root..
4. Check stored==load, if bad, goto revert
5. loadroot: ...
   7.revert: reverts..

Then we would have one label revert, but no macro revert. This would be a somewhat larger change, since we need to reverse some of the checks.