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

simsonraj · 2021-06-24T06:22:40Z

Issue

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

Test

Unit Test Coverage

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’

Before:

After:

Test/Run Logs

The Run logs are from the truffle test project

 Using network 'testnet'.


Compiling your contracts...
===========================
> Everything is up to date, there is nothing to compile.



  Contract: Precompiles
    ✓ should execute sha3fips Precompiled fucntion
    ✓ should execute ecrecoverPublicKey Precompiled fucntion


  2 passing (115ms)

Operational Checklist

Does this PR introduce backward-incompatible changes to the on-disk data structure and/or the over-the-wire protocol?. (If no, skip to question 8.)

NO
Describe the migration plan.. For each flag epoch, describe what changes take place at the flag epoch, the anticipated interactions between upgraded/non-upgraded nodes, and any special operational considerations for the migration.
Describe how the plan was tested.
How much minimum baking period after the last flag epoch should we allow on Pangaea before promotion onto mainnet?
What are the planned flag epoch numbers and their ETAs on Pangaea?
What are the planned flag epoch numbers and their ETAs on mainnet?

Note that this must be enough to cover baking period on Pangaea.
What should node operators know about this planned change?
Does this PR introduce backward-incompatible changes NOT related to on-disk data structure and/or over-the-wire protocol? (If no, continue to question 11.)

NO
Does the existing node.sh continue to work with this change?
What should node operators know about this change?
Does this PR introduce significant changes to the operational requirements of the node software, such as >20% increase in CPU, memory, and/or disk usage?
No

TODO

gupadhyaya · 2021-06-25T18:12:14Z

@simsonraj thanks for this PR. could you fix the build failure: https://travis-ci.com/github/harmony-one/harmony/jobs/518515627 ? one of your test is throwing nil pointer error.

simsonraj · 2021-06-26T10:29:14Z

@gupadhyaya Hi, I fixed the nil pointer issue, my tests doesn't fail anymore, still the build fails. Can you please let me know if i missed something?

gupadhyaya · 2021-07-08T04:01:08Z

@gupadhyaya Hi, I fixed the nil pointer issue, my tests doesn't fail anymore, still the build fails. Can you please let me know if i missed something?

thanks @simsonraj looks like goimports failing

goimports FAILED!
1222--- BEGIN goimports ---
1223diff -u core/vm/contracts.go.orig core/vm/contracts.go
1224--- core/vm/contracts.go.orig	2021-06-26 10:03:14.121332438 +0000
1225+++ core/vm/contracts.go	2021-06-26 10:03:14.121332438 +0000
1226@@ -33,6 +33,7 @@
1227 
1228 	//Needed for SHA3-256 FIPS202
1229 	"encoding/hex"
1230+
1231 	"golang.org/x/crypto/sha3"
1232 )
1233 
1234@@ -96,7 +97,7 @@
1235 	common.BytesToAddress([]byte{8}):   &bn256PairingIstanbul{},
1236 	common.BytesToAddress([]byte{9}):   &blake2F{},
1237 	common.BytesToAddress([]byte{255}): &vrf{},
1238-	
1239+
1240 	common.BytesToAddress([]byte{102}): &sha3fip{},
1241 	common.BytesToAddress([]byte{103}): &ecrecoverPublicKey{},
1242 }
1243--- END goimports ---

simsonraj · 2021-07-08T06:04:14Z

@gupadhyaya I checked this error, couldn't make sense of it fully, I can run the local tests and can test the changes with an integration tests too.
when I went through the travis build script, the build script fails whenever there is any line added to the imports.

can you please give me more inputs on how to fix this? I don't necessarily think this is a coding issue, thanks.

gupadhyaya · 2021-07-10T00:30:16Z

@gupadhyaya I checked this error, couldn't make sense of it fully, I can run the local tests and can test the changes with an integration tests too.
when I went through the travis build script, the build script fails whenever there is any line added to the imports.

can you please give me more inputs on how to fix this? I don't necessarily think this is a coding issue, thanks.

small one. so fixed it directly. will review soon.

simsonraj · 2021-07-12T10:09:38Z

I see its a spacing issue, thanks for fixing it @gupadhyaya.

rlan35 · 2021-08-10T21:15:03Z

core/vm/contracts.go

@@ -74,6 +79,9 @@ var PrecompiledContractsIstanbul = map[common.Address]PrecompiledContract{
 	common.BytesToAddress([]byte{7}): &bn256ScalarMulIstanbul{},
 	common.BytesToAddress([]byte{8}): &bn256PairingIstanbul{},
 	common.BytesToAddress([]byte{9}): &blake2F{},
+
+	common.BytesToAddress([]byte{102}): &sha3fip{},


since we already have vrf at 255. How about going backward from there like 254 and 253?

@rlan35 changed the address as suggested.

Please add a new PrecompiledContractsXXXXX and add a forking epoch to enable it. You can follow the code here: https://github.com/harmony-one/harmony/pull/3703/files

hi @rlan35, thanks for the reference code, much helpful,
Can I also please ask in this file
https://github.com/harmony-one/harmony/blob/b34d06b2690395ca0ab64c34be72484ed92249d0/internal/params/config.go#L62
what would be the epoch number for the MainnetChainConfig & TestnetChainConfig and for the rest of the configs I assume it is Zero(0).

Sorry, missed the thread. You can put "EpochTBD" for mainnet and testnet for now. 0 for other configs. We will decide a number before release.

rlan35 · 2021-08-10T21:17:29Z

core/vm/contracts.go

+}
+
+func (c *ecrecoverPublicKey) Run(input []byte) ([]byte, error) {
+	const ecrecoverPublicKeyInputLength = 128


Are these code from a existing implementation or it's a new implementation by yourself? If it's existing one, can you add the link to the original code?

ecrecover public key function is similar to the "ecrecover" which retrieves the address while the former has one step lesser, converting the public key into an address.

but the sha3fips code implementation is our own using the "golang.org/x/crypto/sha3" crypto library.

ok, looks good.

…tively

rlan35 · 2021-08-13T00:18:22Z

core/vm/contracts.go

+// This method does not require any overflow checking as the input size gas costs
+// required for anything significant is so high it's impossible to pay for.
+func (c *sha3fip) RequiredGas(input []byte) uint64 {
+	return uint64(len(input)+31)/32*params.Sha256PerWordGas + params.Sha256BaseGas


How is this gas formula determined?

@rlan35 the gas price formula is kept the same as that of sha256hash precompiles. Thanks for pointing it out, I did a bit more in-depth research on the gas prices of various hashing functions, since SHA3-256 is the same family as keccak256.
the gas price should be of that as mentioned in the Ethereum yellow paper (appendix G)
.
we will change this to the values as shown above, agree?

hi, @rlan35 , waiting for your goahead for the above as well as the epoch numbers as per previous conversation.

Yes, 30 and 6 seems more reasonable here.

Hi @rlan35 , Done with these changes, please review it

…ged the gas prices for sha3

simsonraj added 3 commits June 21, 2021 21:53

SHA3-256 FIPS-202 precompiled contract implementation

180ef50

go fmt

1b2ae73

Added precompiles address under VRF

1d627f7

added sha3 prcompiles to Istanbul

f8d97a2

fix goimports error

b189da6

gupadhyaya requested a review from rlan35 August 7, 2021 19:23

rlan35 reviewed Aug 10, 2021

View reviewed changes

Updated the sha3 & ecreceover pk contract address to 253 & 254 respec…

deda93e

…tively

rlan35 reviewed Aug 13, 2021

View reviewed changes

simsonraj added 3 commits August 16, 2021 16:00

Merge branch 'harmony-one:main' into web3labs

bc426c7

Merge branch 'harmony-one:main' into web3labs

11293e6

Added a new PrecompiledContractsSHA3FIPS and add forking epoch & Chan…

0862dcb

…ged the gas prices for sha3

rlan35 approved these changes Sep 17, 2021

View reviewed changes

rlan35 merged commit 70d9583 into harmony-one:main Sep 17, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

simsonraj commented Jun 24, 2021 •

edited

Loading

gupadhyaya commented Jun 25, 2021

simsonraj commented Jun 26, 2021

gupadhyaya commented Jul 8, 2021

simsonraj commented Jul 8, 2021 •

edited

Loading

gupadhyaya commented Jul 10, 2021

simsonraj commented Jul 12, 2021

rlan35 Aug 10, 2021

simsonraj Aug 11, 2021

rlan35 Aug 13, 2021

simsonraj Aug 16, 2021

rlan35 Sep 17, 2021

rlan35 Aug 10, 2021

simsonraj Aug 11, 2021

rlan35 Sep 17, 2021

rlan35 Aug 13, 2021

simsonraj Aug 16, 2021

simsonraj Aug 24, 2021

rlan35 Sep 17, 2021

simsonraj Sep 17, 2021

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

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

Conversation

simsonraj commented Jun 24, 2021 • edited Loading

Issue

Test

Unit Test Coverage

Test/Run Logs

Operational Checklist

TODO

gupadhyaya commented Jun 25, 2021

simsonraj commented Jun 26, 2021

gupadhyaya commented Jul 8, 2021

simsonraj commented Jul 8, 2021 • edited Loading

gupadhyaya commented Jul 10, 2021

simsonraj commented Jul 12, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

simsonraj commented Jun 24, 2021 •

edited

Loading

simsonraj commented Jul 8, 2021 •

edited

Loading