ecdsa signature format has side-car recovery id instead of inline

[leighmcculloch]

During the original development of the secp256k1 interface @kwantam recommended that we specify ecdsa signatures as including the recovery id inline.

I'd just declare that ECDSA signatures over secp256k1 are 65 bytes (32-byte r, 32-byte s, 1-byte rec-id), make pk-recover take 2 args rather than 3

Ref: #839 (comment)

It appears we didn't end up implementing that recommendation, but I also don't see any conversation in GitHub about it or why and so I suspect we missed the recommendation. This is the interface we have today:

This is the function exposed in the Soroban SDK with the recovery ID collected in a separate parameter:

/// Recovers the ECDSA secp256k1 public key.
///
/// The public key returned is the SEC-1-encoded ECDSA secp256k1 public key
/// that produced the 64-byte signature over a given 32-byte message digest,
/// for a given recovery_id byte.
pub fn secp256k1_recover(
    &self,
    message_digest: &BytesN<32>,
    signature: &BytesN<64>,
    recorvery_id: u32,
) -> BytesN<65>

Ref: https://github.com/stellar/rs-soroban-sdk/blob/349cd57fc7f20c67730ff89762e99e64e280e4d5/soroban-sdk/src/crypto.rs#L50-L60

These are the args of the function defined in the Soroban Environment host interface also showing the recovery ID collected separately:

rs-soroban-env/soroban-env-common/env.json

Lines 1947 to 1958 in 8e593ab

	{
	"name": "msg_digest",
	"type": "BytesObject"
	},
	{
	"name": "signature",
	"type": "BytesObject"
	},
	{
	"name": "recovery_id",
	"type": "U32Val"
	}

It may be too late to change this interface, but I wanted to surface this regardless because this is a crypto interface.

Related issue:

• ecdsa public key format uncompressed vs compressed #1234

[leighmcculloch]

The main concerns I can see with the separate recovery ID are:

• A signature is two pieces of information that developers need to pass around in some form, and it's possible folks will use different inconsistent encoded forms for that.
• In other ecosystems, like Ethereum, combining them into a single form is the norm, and we're diverging from that practice for unclear benefit. With Soroban we've tried to lean into Ethereum practices where there was no value in diverging.

[leighmcculloch]

@tomerweller pointed out to me that we did make a conscious decision on this to follow the k256 crate's interface:

• Add secp256k1 and keccak256 host functions #839 (review)