Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ethereum Core Devs Meeting 70 Agenda #123

Closed
Souptacular opened this issue Sep 4, 2019 · 16 comments
Closed

Ethereum Core Devs Meeting 70 Agenda #123

Souptacular opened this issue Sep 4, 2019 · 16 comments

Comments

@Souptacular
Copy link
Contributor

Souptacular commented Sep 4, 2019

Ethereum Core Devs Meeting 70 Agenda

Agenda

  1. Istanbul related client updates.
  2. "Patch proposals" for Istanbul
  3. Decide block number for Istanbul testnet/mainnet
  4. Least Authority ProgPoW Initial Audit Report (LA team in attendance)
  5. Review previous decisions made and action items
  6. Client Updates (only if they are posted in the comments below)
    a) Geth
    b) Parity Ethereum
    c) Aleth/eth
    d) Trinity/PyEVM
    e) EthereumJS
    f) EthereumJ/Harmony
    g) Pantheon
    h) Turbo Geth
    i) Nimbus
    j) web3j
    k) Mana/Exthereum
    l) Mantis
    m) Nethermind
  7. EWASM & Research Updates (only if they are posted in the comments below)
@carver
Copy link

carver commented Sep 5, 2019

How about we add in a step 1.5, talking about patch proposals for the Istanbul EIPs?

Here are some examples that we could cover:

  • Blake2 - hopefully quick check-in about
  • Should we increase the cost of EXTCODECOPY? After the SLOAD repricing, it will be cheaper to load a word out of a contract than to load a single storage slot. This disparity grows as you load more words in series. (Perhaps this is okay, because loading a linear series of bytes actually is faster than loading a bunch of values out of the storage trie)
  • How to deal with contracts that break by increasing SLOAD costs (eg~ Aragon). Some soft proposals I've heard floated:
    • Reduce log costs
    • Reduce SLOAD cost to 700
    • Add a STATICCALLWITHLOGSANDVALUE (obviously with a better name) and interpret call with default stipend as this new SCWLAV
    • Keep a second gas counter with the old gas costs until hitting the default call stipend

@Souptacular
Copy link
Contributor Author

@carver I'll add it to the agenda and let you lead the part of the discussion that addresses these things.

@holgerd77
Copy link

holgerd77 commented Sep 6, 2019

Short Istanbul progress update for the EthereumJS VM:

We are done with all EIP implementations (many thanks to @s1na who did some amazing catch-up within 2 weeks time), for an overview see ethereumjs/ethereumjs-monorepo#501.

There is one EIP still waiting for a review (EIP-2200, SSTORE net gas metering). After that we can do an Istanbul v4.1 release - this will likely be mid next week. We'll mark this (the Istanbul support, so not the VM version release itself) as beta since this is only basically tested and there might be last minute changes to some EIPs.

@karalabe
Copy link
Member

karalabe commented Sep 6, 2019

Some mental notes (waiting for plane, won't be able to attend the call):

  • Re Blake 2 vs. 4 byte round: Is there a legit use case for anything other than 12? I mean it's nice that we want to be flexible, but do we meaningfully need to? How many rounds are there in real use cases? I really think the number should be adjusted to the use case. 2 vs. 4 byte doesn't seem to matter to me really. If there's a reason for large rounds, 4 seems safer since it can cover any weirdo scenario. If there's absolutely no reason for large rounds, why not 1 byte or why not hard code 12 rounds altogether?
  • Re Blake t fields. @axic mentioned that the old fields are 128 bits and the new suggestion is 32. That does not compute because he suggested halving them. That said, 32 bits can already index 4GB, so I kind of agree that there's no point in supporting something that is impossible to get into the EVM in the first place.
  • Re Blake m_len field. This seems a weird addition to avoid Solidity doing proper work :P I kind of get where we're coming from, but still this seems like a weirdo ugly hack to save some loose gas.
  • Re EXCODECOPY pricing, I wouldn't try to add new features at the 11th hour. We can analyze what can be cone with EXTCODECOPY and consider updating it after Istanbul, but this seems a random idea that has not yet been really analyzed, so it would just prolong Istanbul.
  • Re SLOAD, my only emphasis is to take Net SSTORE into account too, since some rules of that EIP were adjusted to SLOAD being 700. If sload is capped to 700, the SSTORE eip might need to be tweaked too. As for the actual solution, I leave that to @holiman, but I'd appreciate if we didn't start to add dirty hacks all over the place.

@jochem-brouwer
Copy link
Member

The SLOAD "issue" might lead to things happening on chain equivalent to GasToken. I see no harm in this, but it might just lead to unintended practices like that and especially "weird" patterns to store data.

@mhluongo
Copy link
Contributor

mhluongo commented Sep 6, 2019

Re Blake 2 vs. 4 byte round: Is there a legit use case for anything other than 12? I mean it's nice that we want to be flexible, but do we meaningfully need to? How many rounds are there in real use cases? I really think the number should be adjusted to the use case. 2 vs. 4 byte doesn't seem to matter to me really. If there's a reason for large rounds, 4 seems safer since it can cover any weirdo scenario. If there's absolutely no reason for large rounds, why not 1 byte or why not hard code 12 rounds altogether?

There are potential future use cases that our team has considered for larger rounds, but they're early and I'd rather not speculate on creative applications just yet.

@zookozcash has also suggested similarly

I do think restricting the rounds to something more reasonable considering gas limits, etc is fine- as the gas pricing is per round, this is priced in regardless. As @axic pointed out the round limit doesn't come from the RFC. The question for me is whether we want clients to need to update to an EIP revision before Istanbul.

Re Blake t fields. @axic mentioned that the old fields are 128 bits and the new suggestion is 32. That does not compute because he suggested halving them. That said, 32 bits can already index 4GB, so I kind of agree that there's no point in supporting something that is impossible to get into the EVM in the first place.

Note @axic withdrew the t field modification proposal.

@axic
Copy link
Member

axic commented Sep 6, 2019

There are potential future use cases that our team has considered for larger rounds, but they're early and I'd rather not speculate on creative applications just yet.

I think the confusion is that if you change the rounds, it is not BLAKE2b anymore. It is a different configuration.

And if the precompile is BLAKE2b specific then the rounds has no place in it.

@mhluongo
Copy link
Contributor

mhluongo commented Sep 6, 2019

And if the precompile is BLAKE2b specific then the rounds has no place in it.

It's unfortunate that this function doesn't have a better name, but one of the RFC spec authors is also suggesting this flexibility. Leaving it to future developers means this precompile is more likely to handle Sia-style PoW changes as well as future innovation.

Perhaps this is "the BLAKE2 F compression function, supporting 64-bit BLAKE2 variants"? I think that's a worthwhile clarification in the EIP.

@axic
Copy link
Member

axic commented Sep 6, 2019

It's unfortunate that this function doesn't have a better name, but one of the RFC authors is also suggesting this flexibility..

Can you link to that suggestion please? Can't see it in the RFC.

@pdyraga
Copy link

pdyraga commented Sep 6, 2019

It's unfortunate that this function doesn't have a better name, but one of the RFC authors is also suggesting this flexibility..

Can you link to that suggestion please? Can't see it in the RFC.

reduced or increased rounds: not currently in use but it is quite possible that in the coming years reduced-round variants may come into use, because that might be necessary to meet performance constraints that the full hash function (and other full hash functions like SHA2 and SHA3) can't meet.

Reference

@axic
Copy link
Member

axic commented Sep 6, 2019

I may be misreading it but to me that paragraph refers to BLAKE2 and not BLAKE2b.

Nitpick: Zooko is not listed as an author on the RFC 😉 (He is on the spec.)

@axic
Copy link
Member

axic commented Sep 6, 2019

Here's my brief summary of the call: ethereum/EIPs#152 (comment)

Let me know if I’ve missed anything or if anything is incorrect.

@mhluongo
Copy link
Contributor

mhluongo commented Sep 6, 2019

Beat me to it, thanks @axic!

@Souptacular
Copy link
Contributor Author

Closing in favor of #125.

@vogelito
Copy link
Contributor

From the call notes:

I have seen a list of contracts that could be affected and the list is long.

Where can we find the list of contracts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests