Updates secp256k1 signature format to raw r||s #3
Conversation
Modifies the secp256k1 signature format to utilize a 64-byte raw (r||s) format. This change affects both the signing and verification processes. Ensures consistency in signature handling and resolves compatibility issues related to the previous DER-encoded format.
|
Let me review the changes, and if everything looks good, it will be merged this week. Thanks for your contribution! |
There was a problem hiding this comment.
Pull Request Overview
This PR updates the secp256k1 signature format from DER-encoded to raw 64-byte r||s format to resolve compatibility issues. The change affects both signing and verification processes.
- Updates signature encoding from
sigencode_der_canonizeto raw r||s format - Updates signature decoding from
sigdecode_dertosigdecode_string - Updates docstring comments to reflect the new 64-byte raw signature format
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| msg, | ||
| hashfunc=hashlib.sha256, | ||
| sigencode=ecdsa_util.sigencode_der_canonize, # enforce low-S | ||
| sigencode=ecdsa_util.sigencode_string_canonize, # 64-byte raw r||s with canonical low-S |
There was a problem hiding this comment.
The function sigencode_string_canonize does not exist in the ecdsa library (v0.19.1). The ecdsa library provides:
sigencode_stringfor raw r||s encoding (without canonicalization)sigencode_der_canonizefor DER encoding with canonical low-S
To achieve raw r||s format with canonical low-S, you may need to either:
- Use
sigencode_string(which may already enforce low-S depending on the library version), or - Implement a custom canonization wrapper function
This will cause a runtime AttributeError when attempting to sign with secp256k1.
| return self.vk.verify( | ||
| sig, msg, hashfunc=hashlib.sha256, sigdecode=ecdsa_util.sigdecode_der | ||
| sig, msg, hashfunc=hashlib.sha256, sigdecode=ecdsa_util.sigdecode_string |
There was a problem hiding this comment.
[nitpick] This change introduces a breaking change: existing DER-encoded secp256k1 signatures will fail verification with the new sigdecode_string decoder. If there are existing signatures in the system that need to remain valid, consider:
- Adding backwards compatibility by trying both decoders
- Providing a migration path for existing signatures
- Documenting this as a breaking change in the release notes
2 participants
Fix for: #2
Modifies the secp256k1 signature format to utilize a 64-byte raw (r||s) format. This change affects both the signing and verification processes.
Ensures consistency in signature handling and resolves compatibility issues related to the previous DER-encoded format.