zk-kit icon indicating copy to clipboard operation
zk-kit copied to clipboard

Restrict types of `message` in eddsa-poseidon's `signMessage`

Open artwyman opened this issue 11 months ago • 8 comments

Describe the improvement you're thinking about

This issue is conceptually similar to #188, now applied to the message argument to eddsa-poseidon's signMessage rather than the private key. This argument is typed as BigNumberish and the docs don't say anything special about how it's interpreted. The message of the underlying Poseidon algorithm is expected to be a single field element (bigint mod r).

The checkMessage function used to check this input allows arbitrary strings. If a string is valid decimal or hex, it'll be converted to a bigint as digits. Otherwise it'll be converted to a Buffer as raw bytes. This works so long as the string isn't more than 32 characters. If it's longer, it'll trigger an exception later from leBigIntToBuffer which uses a size argument.

This might be intended to be a convenience feature for signing string messages. If so, it would need to be more clearly documented. I think the size limitation and the potential for confusion about whether a small string should be treated as digits or bytes suggests it would be better to remove this case and reject string inputs which can't be interpreted as stringified bigints.

Additional context I came across this when writing tests for handling of bad inputs in the Zupass repo. I passed a string expecting a TypeError and was surprised to get no exception. I've added my own type check to avoid the potential confusion here, so I'm not blocked by this issue.

artwyman avatar Mar 23 '24 00:03 artwyman