signStructuredData not in line with SIP018 specification

[MarvinJanssen]

Looking at signStructuredMessage, I noticed that although it signs Clarity Values, it is missing parts of the specification thus resulting in different signatures.

connect/packages/connect/src/signature/structuredData.ts

Lines 24 to 43 in 1f3e94a

	async function signPayload(payload: StructuredDataSignaturePayload, privateKey: string) {
	const tokenSigner = new TokenSigner('ES256k', privateKey);
	// eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion
	return tokenSigner.signAsync({
	...payload,
	message: serializeCV(payload.message).toString('hex'),
	} as any);
	}
	export async function signStructuredMessage(options: StructuredDataSignatureRequestOptions) {
	const { userSession, ..._options } = options;
	const { privateKey, publicKey } = getKeys(userSession);
	const payload: StructuredDataSignaturePayload = {
	..._options,
	publicKey,
	};
	return signPayload(payload, privateKey);
	}

As per the current draft:
https://github.com/stacksgov/sips/blob/48a8493f19702a14258db5730fece837cd783b7a/sips/sip-018/sip-018-signed-structured-data.md#formal-specification

• signStructuredData(D, K) = sign(messageHash(D), K), where:
• messageHash(D) = sha256(structuredDataPrefix || domainHash || structuredDataHash(D)).

The domain tuple CV and payload CV should therefore be supplied separately. The prefix is 0x534950303138.

The domain tuple CV at the bare minimum requires:

{
	name: (string-ascii len)
	version: (string-ascii len)
	chain-id: uint
}

[aulneau]

This is actually more of an issue for the wallet -- if the goal of the connect library is to pass values to the wallet to sign, we'd want to change the signature of the function in the provider to accept multiple params (domain object, and clarity message to sign). We cannot pass the message prefix + domain hash + message hash to the wallet using the current sign structured data methods, but we could pass it just as a standard "signed message". cc @beguene

[MarvinJanssen]

Yep. I was a bit confused to have found it here.

[beguene]

I agree @aulneau, the main changes are to be made in the web wallet, but we will also have to update connect to accommodate the changes.

@markmhx I will create a new one maybe for the web wallet and link this one to it.

[janniks]

I will create a new one maybe for the web wallet and link this one to it.

Should/will the wallet also switch to RSV formatted signatures as a part of this?

[beguene]

@janniks Yes I think we should stay consistent. We can do that as part of this task. cc: @markmhx @andresgalante

[MarvinJanssen]

FYI: stacksgov/sips#57 (comment)

The structured data prefix has been changed to make sure that SIP018 messages can absolutely never be valid transactions. The OP has been edited.