neo icon indicating copy to clipboard operation
neo copied to clipboard

add secp256k1 + keccak256 to signature verification.

Open Jim8y opened this issue 1 year ago • 7 comments

Description

This pr focus on adding support to secp256k1 signed transactions and hashed with Keccak256.

Fixes # (issue)

Type of change

  • [ ] Bug fix (non-breaking change which fixes an issue)
  • [x] 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?

Test Configuration:

Checklist:

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

Jim8y avatar Apr 25 '24 03:04 Jim8y

Why?

Hey @roman-khimov, this feature is required by some exchanges that only support secp256k1 to sign transactions, its hardware restrained and can not be updated. Thus, we need to add support to verify secp256k1 signed transactions. But we only verify it, we dont construct it.

Jim8y avatar Apr 25 '24 09:04 Jim8y

How come it wasn't a problem for years of Neo existence?

This raises so many compatibility questions that I don't even know where to begin. Addresses/wallets/script parsers/NeoFS accounts/NeoX accounts.

Likely the issue can be solved with crypto.verifyWithECDsa that has support for k1 (proxy contract that does actions based on incoming payloads signed with k1 key).

roman-khimov avatar Apr 25 '24 12:04 roman-khimov

New syscall:

System.Crypto.CheckSigV2

System.Crypto.CheckMultisigV2

Data Type:

InvocationScript: Old format VerificationScript: 1 byte curve| 1 byte hash | Old format

Curve:

0x00 r1, 0x01 k1,

Hash:

0x00 SHA256, 0x01 Keccak256

Exampe:

            using ScriptBuilder verificationScriptBuilder = new();
            verificationScriptBuilder.EmitRaw([0x01]); // Push curve secp256K1
            verificationScriptBuilder.EmitRaw([0x01]); // Push Keccak256 hasher
            verificationScriptBuilder.EmitPush(pubKey); // Push pubkey
            verificationScriptBuilder.EmitSysCall(ApplicationEngine.System_Crypto_CheckSigV2);

Price:

SignatureContractCostV2: SignatureContractCost() + (isV2 ? ApplicationEngine.OpCodePriceTable[(byte)OpCode.PUSH1] * 2 : 0);

MultiSignatureContractCostV2: MultiSignatureContractCost() + (isV2 ? ApplicationEngine.OpCodePriceTable[(byte)OpCode.PUSH1] * 2 : 0);

How to check if a verification script is V2:

       private static ReadOnlySpan<byte> ParseScriptV2(ReadOnlySpan<byte> script, out bool? isV2, out ECCurve? curve, out Hasher? hasher)
        {
            if (script.Length < 40) throw new FormatException("The verification script is too short.");
            isV2 = BinaryPrimitives.ReadUInt32LittleEndian(script[^4..]) == ApplicationEngine.System_Crypto_CheckSigV2 ||
                BinaryPrimitives.ReadUInt32LittleEndian(script[^4..]) == ApplicationEngine.System_Crypto_CheckMultisigV2;
            curve = ECCurve.Secp256r1;
            hasher = Hasher.SHA256;
            if (!isV2.Value)
            {
                return script;
            }

            curve = script[0] == 0x01 ? ECCurve.Secp256k1 : ECCurve.Secp256r1;
            hasher = script[1] == 0x01 ? Hasher.Keccak256 : Hasher.SHA256;
            return script[2..];
        }

Jim8y avatar Apr 27 '24 18:04 Jim8y

Note: Still need to add it to the hard fork.

Jim8y avatar Apr 27 '24 18:04 Jim8y

This raises so many compatibility questions that I don't even know where to begin. Addresses/wallets/script parsers/NeoFS accounts/NeoX accounts.

Just to be in sync with Discord: we've discussed it with @roman-khimov and prepared an alternative concept that do not require core protocol modifications and do not require custom verification contract deployment. The solution is rather simple: use custom witness verification script with a call to native CryptoLib's verifyWithECDsa. See the https://github.com/nspcc-dev/neo-go/pull/3425 (it is an updated version) and don't hesitate to comment, I'll port it to the core in the nearest future.

AnnaShaleva avatar Apr 28 '24 10:04 AnnaShaleva

Can't they use NeoX to bridge to N3? Isnt that the point of NeoX?

cschuchardt88 avatar Apr 28 '24 23:04 cschuchardt88

It's possible to use his verifyWithECDsa as verification script without store the contract in the blockchain, isn't it?

shargon avatar Apr 29 '24 08:04 shargon

Close because of https://github.com/neo-project/neo/pull/3209

NGDAdmin avatar May 10 '24 02:05 NGDAdmin