Clarification on bytes unpacking logic

[bweick]

Brian from zkp2p here! Just wanted to make sure I was understanding some logic in the StringUtils function.

1. Am I right to assume that maxBytes needs to be a multiple of packSize here?
   

   email-wallet/packages/contracts/src/utils/StringUtils.sol

   Line 74 in 461782c

   function convertPackedBytesToString(uint256[] memory packedBytes, uint256 maxBytes, uint256 packSize)

2. Similarly it seems that packSize is fixed here to 7 is that right? Since that's how circom does it's packing.
   https://github.com/zkemail/email-wallet/blob/461782c9f3a9cf9ef21fbd8628414692e45223a0/packages/contracts/src/utils/StringUtils.sol#L82

3. If I can suggest a different function signature could it look something like this:
   function convertPackedBytesToBytes(uint256[] memory packedBytes, uint256 signals) internal pure returns (string memory extractedString)

We can then treat the packSize as a constant --> PACK_SIZE and calculate maxBytes as follows:
uint256 maxBytes = signals * PACK_SIZE

Not sure signals is the best name but this seems a bit cleaner. You also may be designing for different potential pack sizes. Lemme know what you think.

[Divide-By-0]

1. Not necessarily. You might imagine you have some value that in the circuit is at most 10 characters, but is packed into 1 packed byte; so in this case, the maxBytes would be 10 characters. maxBytes is used to ensure that even with offsets, your string is only a certain length. We can default that to be the same as packedBytes * pack_size if unspecified, but in some cases might want to be less.

2. Correct, that is a bug. It's fixed to 7 arbitrarily on accident (it used to be 7 a long time ago), even though we changed the packed size in circom. We should edit the 7 in the body to be pack_size.

3. Not sure PACK_SIZE should be constant, people might want to pack things other than bytes. At the same time, we do hardcode packed signals to be bytes (i.e. at most 8 bits via >> 8), so perhaps both that 8 and PACK_SIZE should be either both arguments or both constants. If 8 is hardcoded, then PACK_SIZE should be 31, since that should be set as the max in any corresponding circuits for optimal performance.

[Divide-By-0]

Here's the fix PR:
zkemail/zk-email-verify#125

I've fixed the your point 2 bug, and added functions to more or less address your 1 and 3 points. The latest version of the SDK has the revisions.