add secp256k1 + keccak256 to signature verification.
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
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.
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).
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..];
}
Note: Still need to add it to the hard fork.
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.
Can't they use NeoX to bridge to N3? Isnt that the point of NeoX?
It's possible to use his verifyWithECDsa as verification script without store the contract in the blockchain, isn't it?
Close because of https://github.com/neo-project/neo/pull/3209