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

BLAKE2b `F` Compression Function Precompile #152

Open
tjade273 opened this issue Oct 4, 2016 · 104 comments

Comments

@tjade273
Copy link

commented Oct 4, 2016

Title

Title: Add RFC 7693 compression function `F` contract
Author: Tjaden Hess <tah83@cornell.edu>
Status: Draft
Type: Standard Track
Layer: Consensus (hard-fork)
Created 2016-10-04

Abstract

This EIP introduces a new precompiled contract which implements the compression function F used in the BLAKE2b cryptographic hashing algorithm, for the purpose of allowing interoperability between the Zcash blockchain and the EVM, and introducing more flexible cryptographic hash primitives to the EVM.

Parameters

  • METROPOLIS_FORK_BLKNUM: TBD
  • GFROUND: TBD

Specification

Adds a precompile at address 0x0000....0d which accepts ABI encoded arguments corresponding to the function signature

F(bytes32[2] h, bytes32[4] m, uint t , bool f, uint rounds) returns (bytes32[2] h_new);

where h, m, t and f are the current state, the new message, the byte counter and a finalization flag, as defined in RFC 7693, and rounds is the number of rounds of mixing to perform (BLAKE2b uses 12, BLAKE2s uses 10). h_new is the updated state of the hash.

Each operation will cost GFROUND * rounds.

Motivation

Besides being a useful cryptographic hash function and SHA3 finalist, BLAKE2b allows for efficient verification of the Equihash PoW used in Zcash, making a BTC Relay - style SPV client possible on Ethereum. A single verification of an Equihash PoW verification requires 512 iterations of the hash function, making verification of Zcash block headers prohibitively expensive if a Solidity implementation of BLAKE2b is used.

The BLAKE2b algorithm is highly optimized for 64-bit CPUs, and is faster than MD5 on modern processors.

Interoperability with Zcash could enable contracts like trustless atomic swaps between the chains, which could provide a much needed aspect of privacy to the very public Ethereum blockchain.

Rationale

The most frequent concern with EIPs of this type is that the addition of specific functions at the protocol level is an infringement on Ethereum's "featureless" design. It is true that a more elegant solution to the issue is to simply improve the scalability characteristics of the network so as to make calculating functions requiring millions of gas practical for everyday use. In the meantime, however, I believe that certain operations are worth subsidising via precompiled contracts and there is significant precedent for this, most notably the inclusion of the SHA256 prcompiled contract, which is included largely to allow inter-operation with the Bitcoin blockchain.

Additionally, BLAKE2b is an excellent candidate for precompilation because of the extremely asymetric efficiency which it exhibits. BLAKE2b is heavily optimized for modern 64-bit CPUs, specifically utilizing 24 and 63-bit rotations to allow parallelism through SIMD instructions and little-endian arithmetic. These characteristics provide exceptional speed on native CPUs: 3.08 cycles per byte, or 1 gibibyte per second on an Intel i5.

In contrast, the big-endian 32 byte semantics of the EVM are not conducive to efficient implementation of BLAKE2, and thus the gas cost associated with computing the hash on the EVM is disproportionate to the true cost of computing the function natively.

Implementation of only the core F compression function allows substantial flexibility and extensibility while keeping changes at the protocol level to a minimum. This will allow functions like tree hashing, incremental hashing, and keyed, salted, and personalized hashing as well as variable length digests, none of which are currently available on the EVM.

There is very little risk of breaking backwards-compatibility with this EIP, the sole issue being if someone were to build a contract relying on the address at 0x000....0000d being empty. Te likelihood of this is low, and should specific instances arise, the address could be chosen to be any arbitrary value, with negligible risk of collision.

@zookozcash

This comment has been minimized.

Copy link

commented Oct 4, 2016

Great! I think this is a good feature to add. It will allow "Project Alchemy" — BTCRelay-style interoperation between the Zcash and Ethereum blockchains, and it may also be useful as a general purpose secure hash function which is more efficient than SHA3.

@chriseth

This comment has been minimized.

Copy link
Contributor

commented Oct 4, 2016

Please add some more details about the gas costs. I guess they should at least depend on rounds.

@zookozcash

This comment has been minimized.

Copy link

commented Oct 4, 2016

In the earlier EIP about a precompiled full BLAKE2b, the following conversation happened about gas costs:

Okay, so for this new proposal — the precompiled BLAKE2b Compression Function F, the computational cost is not a base plus a cost per byte. Instead, every invocation of this precompile has the same size input (192 bytes, not counting t, f, or rounds).

I suggest the gas cost for invoking BLAKE2b Compression Function F should be 440 per invocation. That's from estimating that it will take about 440 microseconds to compute it. It is within 4X of what it would cost in gas to hash a 192-byte input with SHA3.

(I still don't understand what Zaki's rationale is for making the gas cost of BLAKE2b precompile be the same as gas cost of SHA3 precompile.)

@zookozcash

This comment has been minimized.

Copy link

commented Oct 4, 2016

Oh, good point about the gas cost of rounds. I propose that BLAKE2b Compression Function F gas cost be 37 gas per round. (12 rounds to be compatible with BLAKE2b.)

@tjade273

This comment has been minimized.

Copy link
Author

commented Oct 4, 2016

I think picking hard numbers right now would be somewhat pointless, until we can actually measure compute time in different clients. You're right that the gas is now going to be proportional to the number of rounds now, as opposed to the number of bytes.

@gcolvin

This comment has been minimized.

Copy link
Collaborator

commented Oct 4, 2016

I'd rather implement 64-bit operations in the EVM.

@matthewdgreen

This comment has been minimized.

Copy link

commented Dec 6, 2016

I think this would be a useful project, and I'd like to see it get restarted!

@zookozcash

This comment has been minimized.

Copy link

commented Dec 6, 2016

Okay, to make progress on this, I propose that we temporarily table @gcolvin's alternative suggestion of 64-bit arithmetic (interesting idea, but let's move it to a separate EIP), and satisfice on the gas costs (I still want the BLAKE2b Compression Function F to cost 37 gas per round, but if agreeing to any other number will get this project moving again, then I'll agree to that number!).

@tjade273, @vbuterin: What can we do to help make progress on this. Would it help if one of the Zcash team provided an implementation in some programming language?

@gcolvin

This comment has been minimized.

Copy link
Collaborator

commented Dec 6, 2016

Agreed.

@tjade273

This comment has been minimized.

Copy link
Author

commented Dec 6, 2016

Sorry if the project has slowed down a bit, I've been quite busy with school; I have a break coming up and I hope to get a PoC with a few of the implementations done.

Right now, I think the priority should be to get a precompile implemented in the Go and Rust implementations, since those are the most widely used. To that end, if anyone has experience with FFIs in Go or Rust, that'd be really helpful.

As for gas costs, I think it will become clear what a reasonable price is based on some simple benchmarking once we have an implementation in major clients

@zookozcash

This comment has been minimized.

Copy link

commented Dec 6, 2016

Great! @gcolvin agrees to move the 64-bit-arithmetic idea to another thread (and please let me know so I can subscribe to that one), and we all agree to defer decision about precise gas costs. I'll see if I know someone who wants to take a stab at implementing it in Rust.

@tjade273

This comment has been minimized.

Copy link
Author

commented Dec 21, 2016

The Go precompile is almost complete, and the benchmarks look good: the BLAKE2 precompile is almost exactly twice as fast as the SHA3 on my laptop. That's using a pretty un-optimized, pure-go implementation.

@zmanian

This comment has been minimized.

Copy link

commented Jan 3, 2017

@tjade273 So I was looking at this and I asked Veorq about it and it looks to me like you passing in 128 bits of counter data into the compression function. I believe that is supposed to be max 64 bits. It may not matter but it requires more invasively changing an existing Blake implementation to conform to your api like I'm doing with the Rust impl.

@tjade273

This comment has been minimized.

Copy link
Author

commented Jan 3, 2017

@zmanian The specification says that the compression function takes a

2w-bit offset counter "t"

i.e. 128 bits. This is consistent with the reference implementation.

The API is not set in stone, by the way. I'm totally open to alternative ones and I'm strongly considering getting rid of ABI encoding and just serializing the data in order.

@zmanian

This comment has been minimized.

Copy link

commented Jan 4, 2017

Yeah the spec says 2 words. There are some incidental complexities to storing the value in two state variables in the rust version. The single variable allows using checked arithmetic and typecast in token state update. i'll try and take a closer look at it.

@tjade273

This comment has been minimized.

Copy link
Author

commented Jan 6, 2017

@zmanian You may be safe just ignoring the higher-order word. You would have to be hashing ~10^10 GB of data for it to make any difference. It's hacky, but not the end of the world if it saves you a lot of trouble. We just need to make sure all of the implementations behave consistently.

@zmanian

This comment has been minimized.

Copy link

commented Jan 7, 2017

It really does save a lot of trouble. I mean nightly does have 128 bit checked arithmetic now...

I don't even @vbuterin has enough eth to so 10^10GB on the blockchain :-)

@chriseth

This comment has been minimized.

Copy link
Contributor

commented Jan 11, 2017

@tjade273 What are your numbers on the gas costs of https://github.com/ConsenSys/Project-Alchemy/blob/master/contracts/BLAKE2b/BLAKE2b.sol ? It seems to me that this contract can be heavily optimized. For example the constant could be moved from storage to code / memory (50 gas some time ago, now 200 gas for sload compared to 3 gas for mload / codecopy). This could make it viable to do all that without a precompile.

@tjade273

This comment has been minimized.

Copy link
Author

commented Jan 11, 2017

@chriseth I haven't worked on the BLAKE contract since before the gas cost readjustments, so I'm not sure exactly what the costs are. When I last ran it, each run was something like 200k gas. I'm sure I could optimize it more, but the equihash PoW verification requires 2^9 BLAKE compressions, and I though it unlikely that I would be able to get the gas costs below ~6,000.

I can take another crack at the optimizations, though and see if I can get the costs significantly lower

@zookozcash

This comment has been minimized.

Copy link

commented Apr 19, 2017

Is there anything I can do to move this EIP along? I'd really like to reduce barriers to cross-chain interoperation between Ethereum and Zcash, and this would help. (There are other approaches to cross-chain interop, but it isn't yet clear to me that any of the alternatives would be strictly better than this one.)

Additionally, making an efficient implementation of BLAKE2 available to Ethereum contracts could help with completely unrelated uses, such as if an Ethereum contract wanted to somehow interoperate with other systems like https://github.com/bramcohen/MerkleSet, or just because BLAKE2 is a highly efficient and secure hash function.

I see that the "editor-needs-to-review" label was added. Who is the editor? Thanks!

@MicahZoltu

This comment has been minimized.

Copy link
Contributor

commented Apr 19, 2017

@zookozcash I believe your best bet is probably to create a PR of this EIP. Issues are intended for discussion, which it seems there isn't much of at this point. Once there is no longer any "debate" then a PR that adds the EIP using the template is the next step in the process.

I believe the contributing guide was recently updated: https://github.com/ethereum/EIPs#contributing

Of course, I could be totally wrong about all this as I'm just a random guy on the internet with no authority, but if I were you the above is what I would do. :)

@zookozcash

This comment has been minimized.

Copy link

commented Jun 4, 2017

By the way, there are two different motivations for why this EIP should go into Ethereum. The first (dear to my heart) is to facilitate Zcash↔Ethereum interoperation using the "BTCRelay" technique. Basically allowing Ethereum contracts to "see into" the Zcash blockchain! This is already possible, but it is very slow and expensive without this precompile.

But I want to emphasize that there is another independent reason for this EIP, which is that BLAKE2 is just a more efficient and more widely used hash function than SHA-3, and this EIP could allow Ethereum contracts to efficiently interoperate with data generated by the growing number of BLAKE2-using data generators out there.

I was reminded of this latter fact when reading this blog post by the estimable Adam Langley “Maybe Skip SHA-3” and the ensuing discussions on hackernews and reddit.

There appears to be a growing consensus among cryptographers that SHA-3 is not a great fit for most purposes and is not likely to become widely used. Even the authors of SHA-3 seem to be advocating not for adoption of SHA-3, but for adoption of newer, more efficient variants of SHA-3.

But everyone agrees that BLAKE2 is efficient, secure, and widely used.

By the way, you might enjoy this nice history of hash functions:

https://z.cash/technology/history-of-hash-function-attacks.html
screenshot 2017-06-03 at 19 30 50

@vbuterin

This comment has been minimized.

Copy link
Contributor

commented Jun 4, 2017

Thanks a lot for the great feedback. This is definitely making me consider BLAKE2 support much more strongly.

@whyrusleeping

This comment has been minimized.

Copy link

commented Jul 5, 2017

The ipfs team is working towards switching our default hash function to blake2b-256, on the grounds of making future integrations much easier, i'm definitely in support of this

@axic

This comment has been minimized.

Copy link
Member

commented Sep 22, 2017

@tjade273 if it is already ABI encoded, why not have two functions: hash, hash_final? That would remove the need for the bool f (F(bytes32[2] h, bytes32[4] m, uint t , bool f, uint rounds) returns (bytes32[2] h_new);).

It may also make sense considering to make the rounds part of the name too.

I assume the code using this would either go for Blake2b or Blake2s and wouldn't really switch between them, it would mean cheaper execution as the number of parameters for ABI encoding is lower and uses less memory as well. The downside is of course if the rounds parameter is actually free to be anything based on the use case.

@holiman

This comment has been minimized.

Copy link
Contributor

commented Sep 25, 2017

I'm tentatively in favour of this, but I'm not so sure about having a precompile accept ABI-encoded data. Reasons being:

  • ABI-encoding (and the ABI-format) is currently not part of consensus, it's not implemented on the evm layer.
  • No other precompiles use that format
  • I believe it adds overhead when determining gascosts.

Furthermore, ABI-encoding is imo not fool-proof. It's possible to e.g. point two dynamic arrays to use the same data, or point the data-part to wrap around back to the start of the argument-field.

TLDR; I'd rather not have ABI-parsing as part of consensus.

@axic

This comment has been minimized.

Copy link
Member

commented Sep 25, 2017

@holiman I think it is certainly possible defining a restricted set of ABI encoding supported by this precompile the same way as the data format is described for every other precompile.

@axic

This comment has been minimized.

Copy link
Member

commented Sep 4, 2019

with two rows repeated at the end

Without having looked at the sources, the two rows repeated can be explained. BLAKE2b uses 12 rounds, BLAKE2s uses 10 rounds. The RFC uses a mod 10 on the round counter for looking up in the sigma table. E.g. repeating the last two lines would make no such mod 10 is needed and the same table can used by both BLAKE2b and BLAKE2s.

However, there are other issues out there relating to rounds and configurations, see my previous messages.

@dvdplm

This comment has been minimized.

Copy link

commented Sep 4, 2019

the two rows repeated can be explained. BLAKE2b uses 12 rounds, BLAKE2s uses 10 rounds.

Yeah, the C reference impl follows the spec. We should do the same.

@dvdplm

This comment has been minimized.

Copy link

commented Sep 4, 2019

However, there are other issues out there relating to rounds and configurations, see my previous messages.

Indeed. Somewhat related, here's an interesting comment from the author of a well-regarded blake2 implementation: oconnor663/blake2_simd#13 (comment)

@pdyraga

This comment has been minimized.

Copy link
Contributor

commented Sep 4, 2019

Why do we want to support 4 billion rounds in the precompile? Would making the rounds fields a 8-bit value be a good compromise? Or should it be an 8-bit restricted to the values of 10 and 12 only?

For BLAKE2b we need 12 rounds as you say @axic. Limiting to just hardcoded 12 could be enough for now but the assumption was that we are adding something generic and we can't predict all the use cases and since the gas depends on the number of rounds, it's fine to allow more.

It seems everyone (including me) was focused on BLAKE2b and may have omitted support for BLAKE2s or any other configuration with a different word size.

I think the assumption from the very beginning was that we go with BLAKE2b (optimized for 64-bit). If we want to add support for BLAKE2s (32-bit), we probably need to add another precompile.

@axic

This comment has been minimized.

Copy link
Member

commented Sep 4, 2019

Limiting to just hardcoded 12 could be enough for now but the assumption was that we are adding something generic

The current specification is not generic. In order to be generic, the word size and the IV must also be passed (and perhaps even the sigma table). That however may turn this into fairly suboptimal, because for each block one would need to transmit a lot more data.

since the gas depends on the number of rounds, it's fine to allow more.

Yes, from a users' perspective it is fine, but from a client developer's side it means adding tests for unrealistic and useless edge cases.

I think the assumption from the very beginning was that we go with BLAKE2b (optimized for 64-bit). If we want to add support for BLAKE2s (32-bit), we probably need to add another precompile.

If this is a blake2b precompile, then I see no reason to expose the F function. Just make it into a proper blake2b precompile taking the input.

@mhluongo

This comment has been minimized.

Copy link
Contributor

commented Sep 4, 2019

If this is a blake2b precompile, then I see no reason to expose the F function. Just make it into a proper blake2b precompile taking the input.

Keying, incremental hashing, tree hashing- there are a few applications you can't accomplish without access to the F function or a really robust and stateful precompile.

This was a long discussion - I'd actually prefer to see both or even all four (two F precompiles, a BLAKE2b and a BLAKE2s precompile) but we couldn't commit the time and were confident we could get this approach in time for Istanbul.

@pdyraga

This comment has been minimized.

Copy link
Contributor

commented Sep 4, 2019

Using a different SIGMA means using a different sequence of operations in the G mixing function as well. This is confusing and makes auditing the code really tricky and time consuming and could lead the paranoid to suspicion.
Was this a mistake? Are there alternative specs out there? Is there a technical reason for using one SIGMA rather than the other?

It would be good if all implementations used the same constants.

For geth implementation we decided to reuse code from golang.org/x/crypto implementation which seems to be a more trustworthy choice than implementing it from scratch. There is always some tradeoff.

G is used internally by F, so as long as all F test vectors give expected results, I think it should be fine.

@pdyraga

This comment has been minimized.

Copy link
Contributor

commented Sep 4, 2019

To follow-up the gitter thread and discuss some misunderstanding about 2s and 2b: as explained here by @zookozcash, F precompile for BLAKE2b is the minimal building block needed for the relay and something we could commit to deliver for Istanbul. BLAKE2s is only a nice-to-have for the relay.

Some other interesting use cases for F were also mentioned earlier.

@pdyraga

This comment has been minimized.

Copy link
Contributor

commented Sep 4, 2019

Regarding the number of rounds, I do agree with @axic that 2^32-1 rounds is not realistic now. On the other hand, I do not know if 2^8-1 will be enough for all the future use cases.

If we consider 4 bytes round field a blocker, I can commit to update geth implementation to reflect the proposed change.

@axic

This comment has been minimized.

Copy link
Member

commented Sep 4, 2019

The EIP is misleading/confusing, it uses the term "blake2" and "blake2b" interchangeably, e.g. "Add Blake2 compression function F precompile", "BLAkE2b F compression function", etc.

Keying, incremental hashing, tree hashing- there are a few applications you can't accomplish without access to the F function or a really robust and stateful precompile.

You don't need a stateful precompile or access to the F function in order to accomplish these. The precompile just need to return the state. It is a design choice to expose the F function, but it needs to be exposed well.

If this is a BLAKE2b only precompile then I don't see the reason for a rounds parameter as that is fixed at 12 for BLAKE2b. BLAKE2b is very well defined. Are there any use cases?

I have a feeling that nobody has written any real code utilising this precompile during the writing of the specification, hence why these observations are brought up this late in the process.

@chfast

This comment has been minimized.

Copy link
Contributor

commented Sep 4, 2019

Some other interesting use cases for F were also mentioned earlier.

There seems not to be any example there that would use more than 12 rounds.

@axic

This comment has been minimized.

Copy link
Member

commented Sep 4, 2019

To follow-up the gitter thread and discuss some misunderstanding about 2s and 2b: as explained here by @zookozcash, F precompile for BLAKE2b is the minimal building block needed for the relay and something we could commit to deliver for Istanbul. BLAKE2s is only a nice-to-have for the relay.

It is still unclear why BLAKE2s is a nice to have. What is it used for?

@mhluongo

This comment has been minimized.

Copy link
Contributor

commented Sep 4, 2019

It is still unclear why BLAKE2s is a nice to have. What is it used for?

We're relying on @zookozcash and @daira here...

BLAKE2s — whether the Compression Function F or the full hash — is only a nice-to-have for the purposes of a ZEC-ETH relay.

... where I read "nice to have" as "unnecessary". At least that's been the case for the relay design our project has been considering. cc @prestwich

You don't need a stateful precompile or access to the F function in order to accomplish these. The precompile just need to return the state. It is a design choice to expose the F function, but it needs to be exposed well.

Again, we decided that that attack surface was too high to make it into this hard fork. All of these design decisions have been driven by trying to make this a straightforward candidate for Istanbul, and they're all above in this thread and in the EIP.

We didn't decide the HF schedule- we tried to scope the work in a way that would lead to safe implementations against a solid deadline.

I have a feeling that nobody has written any real code utilising this precompile during the writing of the specification, hence why these observations are brought up this late in the process.

No, we haven't been able to write the Zcash header verification or dig into HNS interop- and we've been upfront about that throughout this process. We're volunteering our time and only have so much to give.

The spec is rigorous, and we followed the process. If we need to amend the EIP process to include a requirement for real-world code examples for all new precompiles, we should do that rather than re-opening what appears to be a settled spec and implementation (IMO). I appreciate that it's taken a while to get to this feedback, but to re-design the entire pre-compile, we needed it in June.

IMO if we want to expand BLAKE2 functionality on the EVM, the way to do that is with 1 or more additional pre-compiles post-Istanbul. Otherwise we're effectively punting on trustless Zcash interop another 6 months.

@prestwich

This comment has been minimized.

Copy link

commented Sep 4, 2019

To follow-up the gitter thread and discuss some misunderstanding about 2s and 2b: as explained here by @zookozcash, F precompile for BLAKE2b is the minimal building block needed for the relay and something we could commit to deliver for Istanbul. BLAKE2s is only a nice-to-have for the relay.

It is still unclear why BLAKE2s is a nice to have. What is it used for?

Outside of Zcash it's much less common than BLAKE2b. Within Zcash it's used for various things in the Sapling cryptosystem, including deriving view keys, creating nullifiers, etc. (see the spec here)

The reason it's a nice to have is any relay necessarily makes the SPV assumption: that miners will not extend invalid chains. In other words, cross-chain communication (via relay or any other mechanism) assumes the validity of transaction authorization and witness information. So verifying proper execution of Sapling operations adds no security.

Which is to say, this EIP was motivated by real-world use cases that BLAKE2s does not contribute to.

@dvdplm

This comment has been minimized.

Copy link

commented Sep 5, 2019

For geth implementation we decided to reuse code from golang.org/x/crypto implementation which seems to be a more trustworthy choice than implementing it from scratch. There is always some tradeoff.

That is understandable. I guess the question about why the go team choose go outside the spec is best put to them. Going back to the original sources and initial pull request reveals nothing of their thinking. They also link to the spec from blake2.net which contains a different G function than the one they actually implement.

…F test vectors give expected results, I think it should be fine.

There are not that many test vectors for F, and if the ones that are out there are generated using the non-spec G, then I think needless doubts&paranoia could arise. After all, this community is the same that choose to use keccak rather than sha3 out of fear of NSA tampering for a single constant change…

@MicahZoltu

This comment has been minimized.

Copy link
Contributor

commented Sep 5, 2019

I am pretty sure (someone correct me if wrong) that Keccak was chosen because it was expected (at the time) to become the SHA3 spec. After Ethereum had already built with Keccak, a minor modification to Keccak256 was made and then it was dubbed SHA3. I don't think the choice of Keccak over SHA3 had anything to do with conspiracy theories about the extra suffix.

@pdyraga

This comment has been minimized.

Copy link
Contributor

commented Sep 5, 2019

There seems not to be any example there that would use more than 12 rounds.

reduced or increased rounds, point 3

If this is a BLAKE2b only precompile then I don't see the reason for a rounds parameter as that is fixed at 12 for BLAKE2b. BLAKE2b is very well defined. Are there any use cases?

The whole point of this long discussion here about whether we want to ship full BLAKE2b precompile or just F compression precompile is that there are some use cases - currently existing and potential future uses - for which the full BLAKE2b precompile would not suffice.

For F, we can ship two versions: one for 32-bit word (BLAKE2s) and one for 64-bit word (BLAKE2b).

The comment from @prestwich and the comment from @zookozcash explain why we decided for 64-bit (BLAKE2b) version - it's the minimal thing for ZEC-ETH relay. Also, it's something we could commit to deliver for Istanbul.

Even if we assume we need both 32-bit and 64-bit versions, this would mean we need two separate precompiles - BLAKE2s F and BLAKE2b F.

When I said generic I did not mean one precompile being able to support both 32-bit and 64-bit version as this would be really unwieldy and suboptimal. What I meant is F precompile optimized for a 64-bit word with a configurable number of rounds and no shorter inputs than those in the RFC.

If we decided to ship F precompile instead of the full BLAKE2b precompile to let developers implement use cases we can't all predict, I think we should leave the decision to developers about what do they pass. I do not know what input length is likely and what is unlikely. If we implement BLAKE2b F, I think we should support no less than what's in the RFC.

I think we need to make the same assumption about the number of rounds - @zookozcash already mentioned reduced or increased rounds.

@pdyraga

This comment has been minimized.

Copy link
Contributor

commented Sep 5, 2019

There are not that many test vectors for F, and if the ones that are out there are generated using the non-spec G, then I think needless doubts&paranoia could arise. After all, this community is the same that choose to use keccak rather than sha3 out of fear of NSA tampering for a single constant change…

The test vectors in the EIP are based on the Appendix A in the RFC and were reviewed by @ebfull from ZCash before they were approved and merged.

Test vector 5 in the EIP is one-to-one with the one from Appendix A.

As you say, @dvdplm there is just one official test vector for F available in official resources, that's why we used and modified one from appendix A instead of generating our own.

All the available full BLAKE2b test vectors pass on the golang implementation.

We had to add test vectors for 0 rounds, 1 round and the maximum number of rounds to the EIP to make sure precompile input is well interpreted but you should have the same result for it if you use SIGMA from golang implementation or SIGMA from RFC. Those test vectors were also recomputed based on the one from RFC Appendix A.

@axic

This comment has been minimized.

Copy link
Member

commented Sep 6, 2019

Here I have proposed to change the t_0 and t_1 fields. I would like to withdraw that proposal.

Reason: while it is unlikely to hash anything large on-chain, it is still possible to start with a state where a large input was already processed.

Since it is a 128-bit value for BLAKE2b I was considering to propose changing that to be just like that (and not two 64-bit values), but I am fine either way.

@axic

This comment has been minimized.

Copy link
Member

commented Sep 6, 2019

I have went through the BLAKE2X specification again and to me it seems (though I'm not 100% sure) it would be possible to implement BLAKE2Xb on top of this precompile. (And it doesn't need a change in rounds.)

@mhluongo

This comment has been minimized.

Copy link
Contributor

commented Sep 6, 2019

I think we need to rename the EIP and make it clear the EIP as it stands is for 64-bit BLAKE2 variants (rather the just BLAKE2b).

@axic

This comment has been minimized.

Copy link
Member

commented Sep 6, 2019

To recap ACD#70:

  • There seems to be some work going on in the background to make use of a BLAKE2b-variant with non-standard rounds. Unfortunately @mhluongo could not divulge further details.
  • It is unknown what would be a reasonable upper bound for rounds for such a non-standard BLAKE2b. As a result the rounds parameter seemed to be left at 32-bit.
  • It was agreed that the EIP should be updated to reflect it describes a very specific superset of BLAKE2b, but still a subset of BLAKE2. The EIP also should include the actual compression function and parameters (SIGMA, IV) – and not leave it as a reference to the RFC.
  • There was no agreement about introducing the m_len field, which is a devex improvement and gas saving. It would make sense for client implementers to judge the complexity and decide upon that.
@mhluongo

This comment has been minimized.

Copy link
Contributor

commented Sep 6, 2019

It would make sense for client implementers to judge the complexity and decide upon that.

We should follow up on Gitter to get a feel for the m_len change more broadly- strong arguments for it if implementers can rally to get it out.

@dvdplm

This comment has been minimized.

Copy link

commented Sep 6, 2019

@axic thank you for the great recap. :)

The EIP also should include the actual compression function and parameters (SIGMA, IV) – and not leave it as a reference to the RFC.

I'm not 100% sure I understand what this means but I guess implementors are free to use the ones from the RFC (like we do at Parity) or not (like geth)?

@pdyraga

This comment has been minimized.

Copy link
Contributor

commented Sep 6, 2019

@axic thank you for the great recap. :)

The EIP also should include the actual compression function and parameters (SIGMA, IV) – and not leave it as a reference to the RFC.

I'm not 100% sure I understand what this means but I guess implementors are free to use the ones from the RFC (like we do at Parity) or not (like geth)?

I have the same understanding. We have the same issue currently when we reference the RFC. I think that if we cover F, G, IV and SIGMA in the EIP as well as test vectors, it's up to the implementations to use the same or optimized version as long as all test vectors for the entire F pass.

@guthlStarkware

This comment has been minimized.

Copy link

commented Sep 8, 2019

Just read Axic comment. I'm in favour to keep the m_len as it would allow for the use case I was describing during the call. I will also bring it on Gitter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.