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

VM: Blake2 precompiled contract gives incorrect result on some inputs #1482

Closed
convexman opened this issue Sep 20, 2021 · 4 comments · Fixed by #1486
Closed

VM: Blake2 precompiled contract gives incorrect result on some inputs #1482

convexman opened this issue Sep 20, 2021 · 4 comments · Fixed by #1486

Comments

@convexman
Copy link

Version: release v4.1.3 (commit feef85f)
Node: v10.24.1
Steps to reproduce:

  1. go to tests/api/istanbul/eip-152.js and add following testcase to const testCases = [...
  {
    input: '0000000c48c9bdf267e6096a3ba7ca8485ae67bb2bf894fe72f36e3cf1361d5f3af54fa5d182e6ad7f520e511f6c3e2b8c68059b6bbd41fbabd9831f79217e1319cde05b61626364650000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000500000000000000000000000000000001',
    expected: 'f3e89a60ec4b0b1854744984e421d22b82f181bd4601fb9b1726b2662da61c29dff09e75814acb2639fd79e56616e55fc135f8476f0302b3dc8d44e082eb83a8',
    name: 'vector 9'
  }
  1. run npm run test:API

One can verify that expected value is correct by querying mainnet node

curl https://mainnet.infura.io/v3/<infura token> \
    -X POST \
    -d '{
    "jsonrpc": "2.0",
    "method": "eth_call",
    "params": [
        {
            "from": "0x0000000000000000000000000000000000000000",
            "to": "0x0000000000000000000000000000000000000009",
            "data": "0x0000000c48c9bdf267e6096a3ba7ca8485ae67bb2bf894fe72f36e3cf1361d5f3af54fa5d182e6ad7f520e511f6c3e2b8c68059b6bbd41fbabd9831f79217e1319cde05b61626364650000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000500000000000000000000000000000001"
        },
        "latest"
    ],
    "id": 0
}' \
    -H "Content-Type: application/json"
{"jsonrpc":"2.0","id":0,"result":"0xf3e89a60ec4b0b1854744984e421d22b82f181bd4601fb9b1726b2662da61c29dff09e75814acb2639fd79e56616e55fc135f8476f0302b3dc8d44e082eb83a8"}

Hashing string abcde using some online hash calculator confirms this result (https://www.toolkitbay.com/tkb/tool/BLAKE2b_512)

One can also write similar test in go-ethereum repo (which I initially did), but this requires building go-ethereum.

It seems that all strings of length less or equal 4 are hashed correctly (thus all mentioned test vectors work), problem occurs whenever length of string is at least 5 (and it so happened all current test vectors are hashing string abc)

I was not able to figure out what's causing this issue, js implementation seems quite different from golang in go-ethereum

@holgerd77
Copy link
Member

@convexman hi there, thanks a lot for reporting and for this detailed write-up. We'll try to investigate soonish. 🙂

@holgerd77 holgerd77 changed the title Blake2 precompiled contract gives incorrect result on some inputs VM: Blake2 precompiled contract gives incorrect result on some inputs Sep 21, 2021
@holgerd77
Copy link
Member

Some context for reference: the associated precompile implementation in the VM can be found here, precompile has been implemented along PR #584 .

@holgerd77
Copy link
Member

Test vectors added to ethereum/tests: ethereum/tests#619

@jochem-brouwer jochem-brouwer self-assigned this Sep 21, 2021
@jochem-brouwer
Copy link
Member

This line is wrong, the copied code is actually here . EIP states we should implement blake2b, not blake2s.

I checked the original source code which indeed reports the values @convexman has supplied here.

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

Successfully merging a pull request may close this issue.

3 participants