add secp256k1 + keccak256 to signature verification.

[Jim8y]

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)
• 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:

• 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

[Hecate2]

(This may not be the best place to put this dictionary)

[roman-khimov]

Why?

[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.

[roman-khimov]

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).

[Jim8y]

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]

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

[cschuchardt88]

Suggested change

	return Keccak256(value.ToArray());
	return Keccak256([.. value]);

[cschuchardt88]

Suggested change

	return Keccak256(value.ToArray());
	return Keccak256([.. value]);

[cschuchardt88]

Dont add ? nullable until project is convert to nullable project.

[cschuchardt88]

Suggested change

	var pubkey = isV2.Value ? witnesses[i].VerificationScript.Span[4..37] : witnesses[i].VerificationScript.Span[2..35];
	var pubkey = isV2.Value ? witnesses[i].VerificationScript[4..37] : witnesses[i].VerificationScript[2..35];

[cschuchardt88]

Suggested change

	if (!Crypto.VerifySignature(this.GetSignData(settings.Network), witnesses[i].InvocationScript.Span[2..], pubkey, curve, hasher.Value))
	if (!Crypto.VerifySignature(this.GetSignData(settings.Network), witnesses[i].InvocationScript[2..], pubkey, curve, hasher.Value))

[cschuchardt88]

Suggested change

	else if (IsMultiSigContract(witnesses[i].VerificationScript.Span, out var m, out ECPoint[] points, out isV2, out curve, out hasher))
	else if (IsMultiSigContract(witnesses[i].VerificationScript, out var m, out ECPoint[] points, out isV2, out curve, out hasher))

[AnnaShaleva]

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 nspcc-dev/neo-go#3425 (it is an updated version) and don't hesitate to comment, I'll port it to the core in the nearest future.

[cschuchardt88]

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

[shargon]

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

[NGDAdmin]

Close because of #3209