fix(UTXO): remove extra opcode check in scriptSig signature parsing function #2591
Conversation
… extract_signature()
dimxy
added
priority: urgent
mariocynicys
left a comment
mariocynicys
left a comment
There was a problem hiding this comment.
Thanks for the fix!
I'd rather lean for keeping things restrictive and only ease the rules when we have an example at hand that doesn't fit (like this one).
|
before trying this fix, i tried the exact same swap again (same taker, same maker) and this time it worked fine |
signatures are 64 bytes but variable in size when der encoded. the encoded sig size varies between 70 and 72 bytes (but seems like 69 is also valid too given this PR). |
cipig
left a comment
cipig
left a comment
There was a problem hiding this comment.
did a bunch of swaps AXE/KMD, back and forth, all working fine
other swaps also still working fine
| }, | ||
| opcode => Err(format!("Unexpected opcode {opcode:?}")), | ||
| Some(Ok(instruction)) => match instruction.data { | ||
| Some(bytes) if !bytes.is_empty() => Ok(bytes.to_vec()), |
There was a problem hiding this comment.
nit: let's not do the if !is_empty() check to be consistent with the error messages like the pubkey extraction counterpart.
There was a problem hiding this comment.
We need to have at least one byte in this data because we will send a slice of len-1 of it to SecpSignature::from_der fn.
There was a problem hiding this comment.
ooh. nice catch. this -1 seems a lil bit dangerous. maybe we could use .saturating_sub in this situation.
There was a problem hiding this comment.
changed to saturating_sub, thanks
|
I don't think this |
fixed to return a structured error fbe9854 instead converting string to TxDeserializationError |
…unction (#2591) This change relaxes scriptSig parsing and P2PK verification to handle real‑world P2PK/P2PKH variants across UTXO chains. Signatures are accepted from any non‑empty push, and pubkeys are parsed from any push and validated as compressed or uncompressed secp256k1 keys. This improves compatibility with chain‑specific encodings (e.g., 69‑byte AXE signatures) without loosening validation, and includes tests for AXE‑like sizes and empty scriptSig cases.
When KDF validates that all inputs signed with a certain pubkey, an internal extract_signature function is used.
This function has too restrictive opcode check for the signature in the scriptSig so some signatures did not pass this check.
This PR relaxes this signature check and relies on the third-party library function to parse and validate DER signatures.
Fixes: #2592