Private keys are not properly passed to @aztec/sdk

[TomAFrench]

I've been experimenting with making something which requires me to calculate grumpkin addresses from a private key.

In order to check correctness I've been comparing against a key pair as generated using azteccli and could never get it to match. I ended up testing with a private key of 0000000000000000000000000000000000000000000000000000000000000001 using the accountKey flag to narrow down where the issue was (as we'd expect to get the grumpkin curve's generator as the public key) and found that I didn't get this from azteccli.

// expected (produced by my code)
0x00000000000000000000000000000000000000000000000000000000000000010000000000000002CF135E7506A45D632D270D45F1181294833FC48D823F272C
// actual (produced by azteccli)
0x01cb173c186c2980e47465852c42a81f6cad7ba529bf8db1eadc9975419b4922008ce53fd8aee2d3169c720ced4a7f2318835a6a5155aa89257269ca46e95324

It looks like the issues stem from these lines where we take the CLI input and convert it to a buffer:

azteccli/src/utils.ts

Lines 43 to 46 in eea1ee7

	const privateKey = Buffer.from(flags.accountKey);
	const publicKey = await sdk.derivePublicKey(
	Buffer.from(flags.accountKey)
	);

The problem is that Buffer.from by default takes the input as the utf8 encoding whereas we want the hex encoding. This means that rather than using the actual bytes of the private key, we're using the bytes of the string representation of the private key.

> Buffer.from("0000000000000000000000000000000000000000000000000000000000000001")
<Buffer 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 ... 14 more bytes>
// The bytes in the buffer don't match up with the hex string and the buffer is 64 bytes long so a 256 bit key has suddenly doubled in size. 

> Buffer.from("0000000000000000000000000000000000000000000000000000000000000001", "hex")
<Buffer 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 01>
// This now matches the bytes in the hex string.

azteccli doesn't do any validation on the resulting buffer so we don't notice that the Buffer is the wrong length and just take the first 32 bytes and ignore the rest.

This also seems to be an issue when deriving your aztec keys from a metamask signature (but I haven't tested this flow against zk.money to test it).

azteccli/src/utils.ts

Line 80 in eea1ee7

let privateKey = Buffer.from(signature.slice(0, 32));

Suggested fix

To fix this, we should change any instances of Buffer.from(x) to Buffer.from(x, "hex"). We should also trim any 0x prefix from the input as in this case we just get an empty buffer rather than throwing an error (we probably throw an error anyway when reading past the end of the buffer but it's not very user friendly). We can wrap this into a bufferFromHexString function which performs this validation.

To help catch these issues, it would be good if the sdk performed length checks on any buffers it receives to make sure they're of the expected length.

[critesjosh]

Thanks for flagging this tom. As you can tell I hadn't tested this.

Here is a PR that might fix it. I also haven't tested this yet. 😝

#10

Lmk if you test it. Feedback is welcome 🙏