EcRecover by shargon · Pull Request #3633 · neo-project/neo

shargon · 2024-12-18T12:06:50Z

Description

Close #3628

Type of change

Optimization (the change is only an optimization)
Style (the change is only a code style for better maintenance or standard purpose)
Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

How Has This Been Tested?

Crypto tests

Test Configuration:

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes
Any dependent changes have been merged and published in downstream modules

* Add entries to Designation event * Change to HF_Echidna * Add UT * Add count

* add base64url * active in * update placehold hf height * fix hf issue and move methods to proper place. * fix test * use identifymodel instead.

* Add entries to Designation event * Change to HF_Echidna * Add UT * Add count

* add base64url * active in * update placehold hf height * fix hf issue and move methods to proper place. * fix test * use identifymodel instead.

# Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.

* Add entries to Designation event * Change to HF_Echidna * Add UT * Add count

* add base64url * active in * update placehold hf height * fix hf issue and move methods to proper place. * fix test * use identifymodel instead.

# Conflicts: # src/Neo/Neo.csproj # src/Neo/SmartContract/Native/RoleManagement.cs

tests/Neo.UnitTests/Cryptography/UT_Crypto.cs

src/Neo/SmartContract/Native/CryptoLib.cs

OT-rebel · 2024-12-30T17:08:43Z

tests/Neo.UnitTests/Cryptography/UT_Crypto.cs

+            // Test from https://eips.ethereum.org/EIPS/eip-2098
+            var privateKey = "1234567890123456789012345678901234567890123456789012345678901234".HexToBytes();
+
+            var expectedPubKey1 = (Neo.Cryptography.ECC.ECCurve.Secp256k1.G * privateKey).ToArray();


Expected values
pubKeyX - 0xe90c7d3640a1568839c31b70a893ab6714ef8415b9de90cedfc1c8f353a6983e
pubKeyY - 0x625529392df7fa514bdd65a2003f6619567d79bee89830e63e932dbd42362d34

eth address for reference - 0x2e988A386a799F506693793c6A5AF6B54dfAaBfB

OT-rebel · 2024-12-30T17:22:46Z

tests/Neo.UnitTests/Cryptography/UT_Crypto.cs

+
+            Console.WriteLine($"Expected PubKey: {expectedPubKey1.ToHexString()}");
+            var message1 = Encoding.UTF8.GetBytes("Hello World");
+            var messageHash1 = message1.Keccak256();


This is supposed to be an Ethereum signed message which is not explicitly stated in ERC-2098

function toEthSignedMessageHash(bytes memory message) internal pure returns (bytes32) { return keccak256(bytes.concat("\x19Ethereum Signed Message:\n", bytes(Strings.toString(message.length)), message)); }

Expected value: 0xa1de988600a42c4b4ab089b619297c17d53cffae5d5120d82d8a92d0bb3b78f2

* Fix UT * Update src/Neo/ProtocolSettings.cs Co-authored-by: nan01ab <yjcc201374@outlook.com> * Update src/Neo/ProtocolSettings.cs Co-authored-by: nan01ab <yjcc201374@outlook.com> * Update src/Neo/ProtocolSettings.cs Co-authored-by: Christopher Schuchardt <cschuchardt88@gmail.com> --------- Co-authored-by: Jimmy <jinghui@wayne.edu> Co-authored-by: nan01ab <yjcc201374@outlook.com> Co-authored-by: Christopher Schuchardt <cschuchardt88@gmail.com>

Hecate2 · 2024-12-31T03:19:11Z

Fixed EIP-2098 tests, but failed in tests creating wallet in neo-cli. Cannot recognize the reason.

src/Neo/SmartContract/Native/CryptoLib.cs

Jim8y · 2024-12-31T07:32:24Z

final version:

==Specification==

===Native Contract Interface===

Three methods will be added to CryptoLib in HF_Echidna:

====Method 1: Recovery from Message and Signature====

{
    "name": "secp256k1Recover",
    "safe": true,
    "parameters": [
        {
            "name": "message",
            "type": "ByteArray"
        },
        {
            "name": "hasher",
            "type": "Integer"
        },
        {
            "name": "signature",
            "type": "ByteArray"
        }
    ],
    "returntype": "ByteArray"
}

====Method 2: Recovery from Message Components====

{
    "name": "secp256k1Recover",
    "safe": true,
    "parameters": [
        {
            "name": "message",
            "type": "ByteArray"
        },
        {
            "name": "hasher",
            "type": "Integer"
        },
        {
            "name": "r",
            "type": "ByteArray"
        },
        {
            "name": "s",
            "type": "ByteArray"
        },
        {
            "name": "v",
            "type": "Integer"
        }
    ],
    "returntype": "ByteArray"
}

====Method 3: Recovery from Pre-computed Hash====

{
    "name": "secp256k1Recover",
    "safe": true,
    "parameters": [
        {
            "name": "hash",
            "type": "ByteArray"
        },
        {
            "name": "signature",
            "type": "ByteArray"
        }
    ],
    "returntype": "ByteArray"
}

===Method Specification===

The methods MUST follow these rules:

Input Requirements for Method 1 (secp256k1Recover with message):
- message: Original message bytes
- hasher: Hash function ID
  - 1: SHA256
  - 2: Keccak256
  - Others: Reserved for future use
- signature: 65 bytes (32 bytes r + 32 bytes s + 1 byte v) or 64 bytes for EIP-2098 format
Input Requirements for Method 2 (secp256k1Recover with components):
- message: Original message bytes
- hasher: Hash function ID (same as above)
- r: 32 bytes
- s: 32 bytes
- v: Recovery ID (27 or 28)
Input Requirements for Method 3 (secp256k1Recover with hash):
- hash: 32-byte hash of the original message
- signature: 65 bytes (32 bytes r + 32 bytes s + 1 byte v) or 64 bytes for EIP-2098 format
Return Value (all methods):
- Success: 33-byte compressed public key in SEC format
- Failure: Returns null if:
  - Invalid signature/component length
  - Invalid recovery value (v)
  - Invalid signature format
  - Recovery failure
  - Null input parameters

Co-authored-by: Hecate2 <2474101468@qq.com>

Hecate2 · 2024-12-31T07:37:22Z

I think the name hasher can be confusing, between a function and a person who executes the hash. hashFunc can be better, but I do not mind leaving it as is.

Jim8y · 2024-12-31T07:39:03Z

I think the name hasher can be confusing, between a function and a person who executes the hash. hashFunc can be better, but I do not minding leaving it as is.

I remember the hasher name is from the builtin Hasher enum.

public enum Hasher : byte

cschuchardt88 · 2024-12-31T07:41:18Z

I think the name hasher can be confusing, between a function and a person who executes the hash. hashFunc can be better, but I do not minding leaving it as is.

I remember the hasher name is from the builtin Hasher enum.
public enum Hasher : byte

Well I told you all so, once again!!! 😃 When it comes to standards and naming conventions no one listens to me. You dont care -- I dont care. Maybe bad english is the problem.

Jim8y · 2024-12-31T07:43:59Z

I think the name hasher can be confusing, between a function and a person who executes the hash. hashFunc can be better, but I do not minding leaving it as is.

I remember the hasher name is from the builtin Hasher enum.
public enum Hasher : byte
Well I told you all so, once again!!! 😃 When it comes to standards and naming conventions no one listens to me. You dont care -- I dont care. Maybe bad english is the problem.

Bro, i would care for sure, but as you said, maybe english is the problem, but once you point it out and fix it in a pr, i promise you i will support you.

shargon · 2025-01-02T14:02:14Z

We can change the name

I think the name hasher can be confusing, between a function and a person who executes the hash. hashFunc can be better, but I do not minding leaving it as is.

I remember the hasher name is from the builtin Hasher enum.
public enum Hasher : byte
Well I told you all so, once again!!! 😃 When it comes to standards and naming conventions no one listens to me. You dont care -- I dont care. Maybe bad english is the problem.

src/Neo/SmartContract/Native/CryptoLib.cs

Jim8y · 2025-01-11T13:03:27Z

@OT-kraftchain coredev team prefers to keep only one method in the native contract, can you please pick one that works for you the best.

OT-rebel · 2025-01-23T13:22:17Z

@OT-kraftchain coredev team prefers to keep only one method in the native contract, can you please pick one that works for you the best.

Hey @Jim8y, I think
public static byte[] Secp256K1Recover(byte[] hash, byte[] signature)
makes the most sense since it gives the user the most freedom to use whatever kind of hash and signature they want

shargon · 2025-01-23T13:25:25Z

@OT-kraftchain coredev team prefers to keep only one method in the native contract, can you please pick one that works for you the best.

Hey @Jim8y, I think public static byte[] Secp256K1Recover(byte[] hash, byte[] signature) makes the most sense since it gives the user the most freedom to use whatever kind of hash and signature they want

signature format (32,32,1) ?

src/Neo/SmartContract/Native/CryptoLib.cs

OT-rebel · 2025-01-23T18:13:11Z

signature format (32,32,1) ?

Yeah, sounds about right

shargon · 2025-01-24T08:50:50Z

Moved to #3696

Jimmy and others added 26 commits August 8, 2024 00:24

add hardofork HF_Echidna

bcc5fd0

Add entries to Designation event (#3397)

e63f10b

* Add entries to Designation event * Change to HF_Echidna * Add UT * Add count

[Neo Core StdLib] Add Base64url (#3453)

9d16c53

* add base64url * active in * update placehold hf height * fix hf issue and move methods to proper place. * fix test * use identifymodel instead.

add hardofork HF_Echidna

f746f8d

Add entries to Designation event (#3397)

8d7f9e8

* Add entries to Designation event * Change to HF_Echidna * Add UT * Add count

[Neo Core StdLib] Add Base64url (#3453)

3fc8077

* add base64url * active in * update placehold hf height * fix hf issue and move methods to proper place. * fix test * use identifymodel instead.

add hardofork HF_Echidna

7e31d0c

Add entries to Designation event (#3397)

993e3fe

* Add entries to Designation event * Change to HF_Echidna * Add UT * Add count

[Neo Core StdLib] Add Base64url (#3453)

37bf0cb

* add base64url * active in * update placehold hf height * fix hf issue and move methods to proper place. * fix test * use identifymodel instead.

format

7dba130

Merge branch 'HF_Echidna' of github.com:neo-project/neo into HF_Echidna

0457ccd

# Conflicts: # src/Neo/Neo.csproj # src/Neo/SmartContract/Native/RoleManagement.cs

Merge Master

d7a291e

Fixed typo

6e780c0

Added back #3397

02d6ab9

Fixed tests

8fb30df

fixed global.json

f9244eb

Merge branch 'master' into HF_Echidna

650c9e9

Merge branch 'master' into HF_Echidna

331541a

Merge branch 'master' into HF_Echidna

eb96d14

Merge branch 'master' into HF_Echidna

74498e5

Merge branch 'master' into HF_Echidna

577c431

Update src/Neo/Neo.csproj

1c82ed9

Update src/Neo/Neo.csproj

b81b127

Merge branch 'master' into HF_Echidna

a3a6fd5

EC Recover

9e28008

shargon requested a review from Jim8y December 18, 2024 12:06

shargon changed the base branch from master to HF_Echidna December 18, 2024 12:07

shargon commented Dec 18, 2024

View reviewed changes

tests/Neo.UnitTests/Cryptography/UT_Crypto.cs Outdated Show resolved Hide resolved

src/Neo/SmartContract/Native/CryptoLib.cs Outdated Show resolved Hide resolved

shargon marked this pull request as ready for review December 18, 2024 12:09

OT-rebel reviewed Dec 30, 2024

View reviewed changes

Hecate2 and others added 2 commits December 31, 2024 10:58

fix eip-2098 tests

6fc94aa

Jimmy added 2 commits December 31, 2024 15:19

Merge branch 'HF_Echidna' into ec-recover

fb28f36

add a new Recover interface

c60ba6f

Hecate2 reviewed Dec 31, 2024

View reviewed changes

src/Neo/SmartContract/Native/CryptoLib.cs Outdated Show resolved Hide resolved

Jimmy and others added 2 commits December 31, 2024 15:33

update manifest

7b22712

Update src/Neo/SmartContract/Native/CryptoLib.cs

9112507

Co-authored-by: Hecate2 <2474101468@qq.com>

Jim8y mentioned this pull request Jan 1, 2025

[Core] rename hasher #3651

Merged

14 tasks

Jim8y mentioned this pull request Jan 7, 2025

[Jan.9] Core Dev Regular Meeting Proposal Discussion #3653

Closed

shargon commented Jan 7, 2025

View reviewed changes

src/Neo/SmartContract/Native/CryptoLib.cs Show resolved Hide resolved

shargon mentioned this pull request Jan 22, 2025

[Jan.23] Core Dev Regular Meeting Proposal Discussion #3690

Closed

Base automatically changed from HF_Echidna to master January 23, 2025 09:52

cschuchardt88 reviewed Jan 23, 2025

View reviewed changes

src/Neo/SmartContract/Native/CryptoLib.cs Show resolved Hide resolved

shargon mentioned this pull request Jan 24, 2025

EC Recover #3696

Merged

15 tasks

shargon closed this Jan 24, 2025

shargon deleted the ec-recover branch January 24, 2025 08:50

Conversation

shargon commented Dec 18, 2024

Description

Type of change

How Has This Been Tested?

Checklist:

Uh oh!

Uh oh!

Uh oh!

OT-rebel Dec 30, 2024

Choose a reason for hiding this comment

Uh oh!

OT-rebel Dec 30, 2024

Choose a reason for hiding this comment

Uh oh!

Hecate2 commented Dec 31, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Jim8y commented Dec 31, 2024

Uh oh!

Hecate2 commented Dec 31, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Jim8y commented Dec 31, 2024

Uh oh!

cschuchardt88 commented Dec 31, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Jim8y commented Dec 31, 2024

Uh oh!

shargon commented Jan 2, 2025

Uh oh!

Uh oh!

Jim8y commented Jan 11, 2025

Uh oh!

OT-rebel commented Jan 23, 2025

Uh oh!

shargon commented Jan 23, 2025

Uh oh!

Uh oh!

OT-rebel commented Jan 23, 2025

Uh oh!

shargon commented Jan 24, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Hecate2 commented Dec 31, 2024 •

edited

Loading

Hecate2 commented Dec 31, 2024 •

edited

Loading

cschuchardt88 commented Dec 31, 2024 •

edited

Loading