Update EIP-4788: post audit tweaks

[lightclient]

repost from lightclient/4788asm#16

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). updated to 8191 as the number is prime and there is 100% utilization for all potential slot time values.
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 ↩

[eth-bot]

✅ All reviewers have approved.

[shemnon]

Do the r and s values have any particular meaning? I'm more curious as to why they s value had to change. Was it searched to find the first valid signature of any address from "zero" or is there some other meaning?

[lightclient]

The s value is like the "nonce" for our address generator. We keep trying new s values until we get a deploy address with beac02 prefix.

[shemnon]

Got it.

Is there a canonical definition of what a synthetic address is and best practices to grind out and S value?

In this particular case (r being 1337 in decimal, and s being 0x1337 left shifted 72 bytes, and then search upwards from there) it is obvious it should not have a known public key. But I think the industry would benefit from a best practice for showing a transaction is a synthetic address transaction with a know r producing a corresponding s.

[lightclient]

I think this post is the pinnacle of our knowledge on the subject and it does not give any explicit instructions: https://weka.medium.com/how-to-send-ether-to-11-440-people-187e332566b7

[SamWilsn]

Are you talking about Nothing-up-my-sleeve numbers?

[djrtwo]

this value will share 500-1000 if slot were to change

1. this is only if the slot time were to change to 4, 8, 16, 24, or 32. I suppose aesthetically we expect that?
2. What is the precise value of sharing these remainders? a reduced non-zero state size for this precompile in the event that the slot time is changed? what magnitude are we talking about?

[adietrichs]

this value will share 500-1000 if slot were to change

Not sure I understand this, but I created my own gist to compare: https://gist.github.com/adietrichs/4f41090c37d292a96b08c8cdbd076d90
Active means percent of slots actively used under that slot time, Inactive counts the extra slots that were written to under 12s and are now no longer used.

For many slot times, 93600 results in a lower number of active (and total, taking into account the inactive ones) slots.
For the range of slot times up to 30s, the ones where there is a difference are: 5, 9, 10, 13, 15, 18, 20, 25, 26, 30.
For all of these 93600 is more efficient. For all others, both choices of modulus are equally efficient.

[adietrichs]

I talked with @lightclient, and out of that came the idea to maybe go the other way: I updated the gist with a third option of a modulus of 8191. Because that's a prime number, that way we are always at 100% active slots, but across a much smaller total range (picked so that at 12s slot time, it still covers ~1d). I'll open a PR for that change.

[adietrichs]

I have opened the PR: lightclient#7.
To keep conversation in one place, here a copy of the description:

HISTORY_BUFFER_LENGTH determines the overall size of the history region of the 4788 contract in storage. Both current proposed modulus, as well as the replacement proposed in the branch this PR is against, share prime factors with the current slot time (12s). This means that the usage of the history region will be sparse (only 1/12 of the storage slots in the region will be used). Furthermore, if we change the slot time in the future, this will change in somewhat arbitrary ways (depending on how many factors the new slot time and the modulus share). While this does not affect the functioning of the EIP, it does add unnecessary variance to two aspects, namely the total storage requirements and the total history depth available.

This PR changes the modulus to a prime number, so that there can never be any shared factors with the slot time. This way, 100% of the history region will be utilized, allowing for simple reasoning about total storage requirements and the total history depth, even in the case of future slot time changes. The specific value (8191 == 2**13 - 1) was chosen so that in the 12s slot time case it matches the total history depth of the original modulus as closely as possible.

[lightclient]

New beacon root address found by @spencer-tb! Thanks!

[lightclient]

Final beacon root address is 0x000F3df6D732807Ef1319fB7B8bB8522d0Beac02.

[eth-bot]

All Reviewers Have Approved; Performing Automatic Merge...

[chfast]

Why was the address changed?

[chfast]

This should be push2 now.

[holiman]

yeah, this was a miss. However, not worth fixing, the runtime execution gascost is identical, the only difference is that the code is one byte larger than it would have needed to be.

[chfast]

This repeats 4x so 4 bytes wasted. Forever :)

[namiloh]

Yeah. Now there are 32 bits less left in the world.

🚻 ☮️