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

interop: add keccak256 implementation #3301

Merged
merged 3 commits into from
Mar 23, 2024
Merged

interop: add keccak256 implementation #3301

merged 3 commits into from
Mar 23, 2024

Conversation

AliceInHunterland
Copy link
Contributor

@roman-khimov
Copy link
Member

Should be a draft, we're not merging it before C# 3.7.

@AliceInHunterland
Copy link
Contributor Author

problems with TestKeccak256_Compat

Copy link
Member

@AnnaShaleva AnnaShaleva left a comment

Choose a reason for hiding this comment

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

Just for the record: once PR is finished and ready to be merged, we need to update interop deps and move it under Cockatrice hardfork. Depends on #3213.

pkg/core/native/crypto.go Outdated Show resolved Hide resolved
pkg/core/native/crypto.go Show resolved Hide resolved
pkg/core/native/crypto_test.go Outdated Show resolved Hide resolved
pkg/core/native/crypto_test.go Outdated Show resolved Hide resolved
pkg/interop/native/crypto/crypto.go Show resolved Hide resolved
pkg/core/native/crypto.go Outdated Show resolved Hide resolved
pkg/interop/native/crypto/crypto.go Show resolved Hide resolved
pkg/core/native/crypto_test.go Outdated Show resolved Hide resolved
pkg/core/native/crypto_test.go Outdated Show resolved Hide resolved
Copy link

codecov bot commented Feb 8, 2024

Codecov Report

Attention: Patch coverage is 84.61538% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 84.88%. Comparing base (fa1c07e) to head (c090628).

Files Patch % Lines
pkg/core/native/crypto.go 84.61% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3301      +/-   ##
==========================================
+ Coverage   84.80%   84.88%   +0.07%     
==========================================
  Files         331      331              
  Lines       44969    44982      +13     
==========================================
+ Hits        38138    38183      +45     
+ Misses       5318     5284      -34     
- Partials     1513     1515       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@AnnaShaleva AnnaShaleva left a comment

Choose a reason for hiding this comment

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

Need to update interop deps one more time after PR is about to be merged to master.

pkg/core/native/crypto_test.go Outdated Show resolved Hide resolved
pkg/core/native/crypto_test.go Outdated Show resolved Hide resolved
pkg/core/native/crypto_test.go Outdated Show resolved Hide resolved
pkg/core/native/crypto_test.go Outdated Show resolved Hide resolved
pkg/core/native/crypto_test.go Outdated Show resolved Hide resolved
pkg/core/native/crypto_test.go Outdated Show resolved Hide resolved
pkg/core/native/crypto_test.go Outdated Show resolved Hide resolved
pkg/core/native/native_test/cryptolib_test.go Outdated Show resolved Hide resolved
pkg/core/native/native_test/cryptolib_test.go Outdated Show resolved Hide resolved
pkg/core/native/native_test/cryptolib_test.go Outdated Show resolved Hide resolved
Copy link
Member

@AnnaShaleva AnnaShaleva left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM, will keep this until some real proximity to v3.7.0.

@@ -92,3 +92,8 @@ func Bls12381Mul(x Bls12381Point, mul []byte, neg bool) Bls12381Point {
func Bls12381Pairing(g1, g2 Bls12381Point) Bls12381Point {
return neogointernal.CallWithToken(Hash, "bls12381Pairing", int(contract.NoneFlag), g1, g2).(Bls12381Point)
}

// Keccak256 calls `keccak256` method of native CryptoLib contract and computes Keccak256 hash of b.
Copy link
Member

Choose a reason for hiding this comment

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

Preserve the line width.

@AnnaShaleva
Copy link
Member

@AliceInHunterland, please, rebase this PR onto fresh master and update go.mod files in a separate commit. We'll merge this PR after update, 3.7 is coming.

@AnnaShaleva
Copy link
Member

@AliceInHunterland, tests are failing. Please, convert from draft when ready.

@AnnaShaleva
Copy link
Member

It's block 20 stateroot hash, please, rebase onto fresh master:

24-03-20T09:58:55.3392468Z     logger.go:146: 2024-03-20T09:57:57.516Z	DEBUG	processing rpc request	{"method": "getnativecontracts", "params": "[]"}
2024-03-20T09:58:55.3393760Z     logger.go:146: 2024-03-20T09:57:57.523Z	DEBUG	processing rpc request	{"method": "findstoragehistoric", "params": "[858c873539d6d24a70f2be13f9dafc61aef2b63c2aa16bb440676de6e44e3cf1 565cff9508ebc75aadd7fe59f38dac610ab6093c YWE=]"}
2024-03-20T09:58:55.3395532Z     logger.go:146: 2024-03-20T09:57:57.523Z	INFO	Error encountered with rpc request	{"code": -102, "cause": "Failed to get historical contract state: item not found", "method": "findstoragehistoric", "params": "[858c873539d6d24a70f2be13f9dafc61aef2b63c2aa16bb440676de6e44e3cf1 565cff9508ebc75aadd7fe59f38dac610ab6093c YWE=]"}
2024-03-20T09:58:55.3395746Z     client_test.go:2350: 
2024-03-20T09:58:55.3396329Z         	Error Trace:	/home/runner/work/neo-go/neo-go/pkg/services/rpcsrv/client_test.go:2350
2024-03-20T09:58:55.3396548Z         	Error:      	Received unexpected error:
2024-03-20T09:58:55.3397297Z         	            	Unknown contract (-102) - Failed to get historical contract state: item not found
2024-03-20T09:58:55.3397540Z         	Test:       	TestClient_FindStorageHistoric
2024-03-20T09:58:55.3398082Z     logger.go:146: 2024-03-20T09:57:57.523Z	INFO	shutting down RPC server	{"endpoint": "127.0.0.1:34691"}
2024-03-20T09:58:55.3398863Z     logger.go:146: 2024-03-20T09:57:57.524Z	INFO	persisted to disk	{"blocks": 23, "keys": 1265, "headerHeight": 23, "blockHeight": 23, "took": "473.497µs"}
2024-03-20T09:58:55.3399074Z --- FAIL: TestClient_FindStorageHistoric (0.07s)

@AnnaShaleva
Copy link
Member

@AliceInHunterland, ditto, block 20 stateroot from RPC tests needs to be updated because you change native Management's state by adding new method to native CryptoLib's manifest.

Port neo-project/neo#2925.

Close #3295

Signed-off-by: Ekaterina Pavlova <ekt@morphbits.io>
Signed-off-by: Ekaterina Pavlova <ekt@morphbits.io>
Signed-off-by: Ekaterina Pavlova <ekt@morphbits.io>
@AliceInHunterland AliceInHunterland marked this pull request as ready for review March 22, 2024 14:20
@AnnaShaleva AnnaShaleva merged commit 37d7a3a into master Mar 23, 2024
19 of 20 checks passed
@AnnaShaleva AnnaShaleva deleted the keccak256 branch March 23, 2024 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement Keccak256 functionality
3 participants