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

[artwyman]

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.

[f3r10]

hi @cedoor could I take this issue? I am part of the PSE 2024 core program

[cedoor]

@f3r10 sure! Is there enough information?

[f3r10]

@f3r10 sure! Is there enough information?

thanks @cedoor. for now, I think I understand the issue. I will let you know if something is missing.

[f3r10]

@cedoor the only type supported for the message parameter should be bitint or bigint | number | Buffer | Uint8Array?

[cedoor]

@cedoor the only type supported for the message parameter should be bitint or bigint | number | Buffer | Uint8Array?

message is limited to 32 bytes and is converted to bigint internally, so one simple option (which would make it more consistent with the other types) would be to have BigNumber imo. In case we need to add documentation to explain how devs can convert text or buffers to bigint tho (should happen rarely).

Tagging @artwyman here. Wdyt?

[f3r10]

oh ok, using a BigNumber I created an additional unit test with

        const message = Buffer.from(crypto.getRandomValues(31))

with 31 as an example, the test passes but with 34 the test fails. Would be that ok?

[cedoor]

Would be that ok?

Yes, please make sure to update/add tests to check the correct type. BigNumber is either bigint or string (string must still be a stringified bigint).

[f3r10]

hi! @cedoor I just opened a PR about this issue, please let me know if that would be ok or if something else has to be done.