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.

Sign up for GitHub

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

Jump to bottom

Implementation of SHA3-256 FIPS 202 hash precompile and EcRecover Uncompressed public key precompile to integrate with ICON blockchain #118

Closed
wants to merge 1 commit into from
Closed

Implementation of SHA3-256 FIPS 202 hash precompile and EcRecover Uncompressed public key precompile to integrate with ICON blockchain #118

wants to merge 1 commit into from

Conversation

simsonraj
Copy link

@simsonraj simsonraj commented Mar 18, 2021 •
edited
Loading

Description

  • Implementation of SHA3-256 FIPS 202 hash precompile to integrate with ICON blockchain
  • EcRecover Uncompressed public key precompile to integrate with ICON blockchain

Rationale

  • In order to validate the merkle tree data from ICON, the SHA3-256 hashing function is required because all of ICON blockchain's hash and transaction hash are created using the SHA3-256 hashing function
  • It is needed to validate signatures of validators in ICON. Currently ICON uses a similar address building scheme to ethereum. ICON uses a part of the hash value of the public key. But we are using a different hash function, SHA3-256 FIPS.

Example

Test cases done to check the input and output of the functions for the given parameters below

sha3fips
params
data : ‘0x0448250ebe88d77e0a12bcf530fe6a2cf1ac176945638d309b840d631940c93b78c2bd6d16f227a8877e3f1604cd75b9c5a8ab0cac95174a8a0a0f8ea9e4c10bca’
returns : ‘0xc7647f7e251bf1bd70863c8693e93a4e77dd0c9a689073e987d51254317dc704’

ecrecoverPublicKey
params
hash : ‘0xc5d6c454e4d7a8e8a654f5ef96e8efe41d21a65b171b298925414aa3dc061e37’
v : ‘0x00’
r : ‘0x4011de30c04302a2352400df3d1459d6d8799580dceb259f45db1d99243a8d0c’
s : ‘0x64f548b7776cb93e37579b830fc3efce41e12e0958cda9f8c5fcad682c610795’
returns : ‘0x0448250ebe88d77e0a12bcf530fe6a2cf1ac176945638d309b840d631940c93b78c2bd6d16f227a8877e3f1604cd75b9c5a8ab0cac95174a8a0a0f8ea9e4c10bca’

Changes

Notable changes:

  • Added the new methods on to the contracts.go file
  • And the related testcases on to the contracts_test.go file

Preflight checks

  • build passed (make build) - Passed
  • tests passed (make test) - Passed
  • manual transaction test passed - Passed

Already reviewed by

Related issues

@simsonraj
Implementation of SHA3-256 FIPS 202 hash precompile and EcRecover Unc…
8c1245a 
…ompressed public key precompile to integrate with ICON blockchain
@simsonraj
Copy link
Author

Please find the attached (as Precompiles.txt file) solidity code for testing.
Precompiles.txt

HaoyangLiu
HaoyangLiu reviewed Mar 26, 2021
View reviewed changes
core/vm/contracts.go

//Needed for SHA3-256 FIPS202 implementation
"encoding/hex"
"golang.org/x/crypto/sha3"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

go format imports

@HaoyangLiu
Copy link
Contributor

Could you submit a BEP to https://github.com/binance-chain/BEPs

@mohamedelshami
Copy link

hi @HaoyangLiu just checking on the status PR of this please? what is plan for marging this, and what's required from us to for the hardfork?

@iakisme
Copy link
Contributor

iakisme commented Aug 6, 2021

@simsonraj: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

3 similar comments
@iakisme
Copy link
Contributor

iakisme commented Aug 6, 2021

@simsonraj: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@iakisme
Copy link
Contributor

iakisme commented Aug 7, 2021

@simsonraj: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@iakisme
Copy link
Contributor

iakisme commented Aug 8, 2021

@simsonraj: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@simsonraj
Copy link
Author

simsonraj commented Aug 9, 2021 •
edited
Loading

@iakisme we have created a new PR as a continuation of this.
Please refer
#357

@iakisme
Copy link
Contributor

iakisme commented Aug 9, 2021

@simsonraj: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

6 similar comments
@iakisme
Copy link
Contributor

iakisme commented Aug 11, 2021

@simsonraj: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@iakisme
Copy link
Contributor

iakisme commented Aug 12, 2021

@simsonraj: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@iakisme
Copy link
Contributor

iakisme commented Aug 13, 2021

@simsonraj: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@iakisme
Copy link
Contributor

iakisme commented Aug 14, 2021

@simsonraj: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@iakisme
Copy link
Contributor

iakisme commented Aug 15, 2021

@simsonraj: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@iakisme
Copy link
Contributor

iakisme commented Aug 16, 2021

@simsonraj: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@unclezoro
Copy link
Collaborator

Will kindly close this PR. since there is a duplicated one.

@unclezoro unclezoro closed this Dec 3, 2021
galaio pushed a commit to galaio/bsc that referenced this pull request Jul 31, 2024
@trianglesphere
Merge pull request bnb-chain#118 from mdehoog/disable-pending-txpool-…
88c9fa3 
…access

Disable access to pending txs from filters + subscriptions
galaio pushed a commit to galaio/bsc that referenced this pull request Jul 31, 2024
@mdehoog
Revert "Merge pull request bnb-chain#118 from mdehoog/disable-pending…
20753a1 
…-txpool-access"

This reverts commit 88c9fa3, reversing
changes made to bb04c5f.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@HaoyangLiu HaoyangLiu HaoyangLiu left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@simsonraj @HaoyangLiu @mohamedelshami @iakisme @unclezoro