stacks-core icon indicating copy to clipboard operation
stacks-core copied to clipboard
verified publisher icon stacks-network

fix: allow `secp256k1-recover?` to accept high-S signatures

Open brice-stacks opened this issue 1 month ago • 2 comments

Fixes discrepancy in updated secp256k1 implementation with old version.

brice-stacks avatar Oct 31 '25 13:10 brice-stacks

We sure this fixes the chain sync test?

It definitely fixes the one I investigated, https://explorer.hiro.so/txid/0x86ef14fa87fb7de8aef4955290637a7b216d8a47378b7890a87065e41cc8e09c?chain=mainnet

brice-stacks avatar Oct 31 '25 14:10 brice-stacks

For this release (3.3.0.0.0) we are going to revert back to the old library for secp256k1 (#6645). We will come back to this fix with more time for proper testing.

brice-stacks avatar Oct 31 '25 17:10 brice-stacks

@brice-stacks Since the reversion to libsecp256k1 is complete, can this be closed?

jcnelson avatar Dec 15 '25 18:12 jcnelson

I think we may still want to switch to the new crate since it is nice that it is all Rust and it removes the need for the separate implementation for Wasm. We just need the time to properly test it.

I can check and make sure that this draft implementation doesn't have the same discrepancy that the r1 implementation had.

brice-stacks avatar Dec 16 '25 16:12 brice-stacks

I confirmed that for secp256k1 here, I did use the correct _prehash variants, avoiding the double hash situation that we saw in secp256r1.

brice-stacks avatar Dec 16 '25 17:12 brice-stacks