-
Notifications
You must be signed in to change notification settings - Fork 1
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 precompile #4
Changes from 9 commits
bc39c31
6e2b26a
4d5287c
110a5ab
214f72f
e7fafe2
fc1426f
5e9c041
c2bdb96
0124f1f
cbd54af
b4e46b1
b667e1c
97b4056
f4b25c5
d5ff1ab
d907056
a8a06f1
251a800
f245982
4877166
0e51e86
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -18,6 +18,7 @@ package vm | |||||
|
||||||
import ( | ||||||
"crypto/sha256" | ||||||
"encoding/binary" | ||||||
"errors" | ||||||
"math/big" | ||||||
|
||||||
|
@@ -26,6 +27,7 @@ import ( | |||||
"github.com/ethereum/go-ethereum/crypto" | ||||||
"github.com/ethereum/go-ethereum/crypto/bn256" | ||||||
"github.com/ethereum/go-ethereum/params" | ||||||
"github.com/keep-network/blake2" | ||||||
"golang.org/x/crypto/ripemd160" | ||||||
) | ||||||
|
||||||
|
@@ -59,6 +61,20 @@ var PrecompiledContractsByzantium = map[common.Address]PrecompiledContract{ | |||||
common.BytesToAddress([]byte{8}): &bn256Pairing{}, | ||||||
} | ||||||
|
||||||
// PrecompiledContractsIstanbul contains the default set of pre-compiled Ethereum | ||||||
// contracts used in the Istanbul release. | ||||||
var PrecompiledContractsIstanbul = map[common.Address]PrecompiledContract{ | ||||||
common.BytesToAddress([]byte{1}): &ecrecover{}, | ||||||
common.BytesToAddress([]byte{2}): &sha256hash{}, | ||||||
common.BytesToAddress([]byte{3}): &ripemd160hash{}, | ||||||
common.BytesToAddress([]byte{4}): &dataCopy{}, | ||||||
common.BytesToAddress([]byte{5}): &bigModExp{}, | ||||||
common.BytesToAddress([]byte{6}): &bn256Add{}, | ||||||
common.BytesToAddress([]byte{7}): &bn256ScalarMul{}, | ||||||
common.BytesToAddress([]byte{8}): &bn256Pairing{}, | ||||||
common.BytesToAddress([]byte{9}): &blake2F{}, | ||||||
} | ||||||
|
||||||
// RunPrecompiledContract runs and evaluates the output of a precompiled contract. | ||||||
func RunPrecompiledContract(p PrecompiledContract, input []byte, contract *Contract) (ret []byte, err error) { | ||||||
gas := p.RequiredGas(input) | ||||||
|
@@ -358,3 +374,68 @@ func (c *bn256Pairing) Run(input []byte) ([]byte, error) { | |||||
} | ||||||
return false32Byte, nil | ||||||
} | ||||||
|
||||||
type blake2F struct{} | ||||||
|
||||||
func (c *blake2F) RequiredGas(input []byte) uint64 { | ||||||
if len(input) != 213 { | ||||||
pdyraga marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
// Input is malformed, we can't read the number of rounds. | ||||||
// Precompile can't be executed so we set its price to 0. | ||||||
return 0 | ||||||
} | ||||||
|
||||||
rounds := binary.BigEndian.Uint32(input[209:213]) | ||||||
return uint64(rounds) | ||||||
} | ||||||
|
||||||
var ( | ||||||
blake2FInputLength = 213 | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this should be a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, it could be:
but thought it's better to initialize them in one block, similarly how it's done above the Please let me know what's the preference here and I'll alter it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point, updated. |
||||||
|
||||||
errBlake2FIncorrectInputLength = errors.New( | ||||||
"input length for Blake2 F precompile should be exactly 213 bytes", | ||||||
) | ||||||
) | ||||||
|
||||||
func (c *blake2F) Run(input []byte) ([]byte, error) { | ||||||
pdyraga marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
if len(input) != 213 { | ||||||
pdyraga marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
return nil, errBlake2FIncorrectInputLength | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If it's only used in this place, might as well define the error inline
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's also used in tests, see |
||||||
} | ||||||
|
||||||
var h [8]uint64 | ||||||
h[0] = binary.BigEndian.Uint64(input[0:8]) | ||||||
h[1] = binary.BigEndian.Uint64(input[8:16]) | ||||||
h[2] = binary.BigEndian.Uint64(input[16:24]) | ||||||
h[3] = binary.BigEndian.Uint64(input[24:32]) | ||||||
h[4] = binary.BigEndian.Uint64(input[32:40]) | ||||||
h[5] = binary.BigEndian.Uint64(input[40:48]) | ||||||
h[6] = binary.BigEndian.Uint64(input[48:56]) | ||||||
h[7] = binary.BigEndian.Uint64(input[56:64]) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The EIP needs to specify how to handle cases where the input data is too short. Other precompiles typially return an error in that case. It's very important that such decisions are not left "as implementation details", since it's consensus-critical behaviour. In this case, the client woud There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll add it to the EIP spec, waiting for the draft and competing PRs to be resolved There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We now expect the input to be exactly 213 bytes long. The PR was opened as an initial implementation before we figure out some details. I should probably open it in a Draft mode. |
||||||
|
||||||
var m [blake2.BlockSize]byte | ||||||
copy(m[:], input[64:192]) | ||||||
|
||||||
var t [2]uint64 | ||||||
t[0] = binary.BigEndian.Uint64(input[192:200]) | ||||||
t[1] = binary.BigEndian.Uint64(input[200:208]) | ||||||
|
||||||
var f bool | ||||||
if input[208] == 0x00000001 { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will be checked in a different way, just |
||||||
f = true | ||||||
} | ||||||
|
||||||
rounds := binary.BigEndian.Uint32(input[209:213]) | ||||||
|
||||||
blake2.F(&h, m, t, f, int(rounds)) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it really safe to cast a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We no longer cast to |
||||||
|
||||||
var output [64]byte | ||||||
binary.BigEndian.PutUint64(output[0:8], h[0]) | ||||||
binary.BigEndian.PutUint64(output[8:16], h[1]) | ||||||
binary.BigEndian.PutUint64(output[16:24], h[2]) | ||||||
binary.BigEndian.PutUint64(output[24:32], h[3]) | ||||||
binary.BigEndian.PutUint64(output[32:40], h[4]) | ||||||
binary.BigEndian.PutUint64(output[40:48], h[5]) | ||||||
binary.BigEndian.PutUint64(output[48:56], h[6]) | ||||||
binary.BigEndian.PutUint64(output[56:64], h[7]) | ||||||
|
||||||
return output[:], nil | ||||||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If input larger than
213
is used, this will throw. If that's the intended behaviour, then the EIP must also be updated to explicitly state that only exactly213
bytes is accepted.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, my intention was to update the EIP to state it explicitly that exactly
213
bytes is accepted. I am not sure if there is any use case for passing more bytes and ignoring some of them. I personally think it's less error-prone to expect the exact length and fail when the length does not match.