openwallet-foundation-labs / sd-jwt-js

A JavaScript implementation of the Selective Disclosure JWT (SD-JWT) spec.
https://sdjwt.js.org/
Apache License 2.0
36 stars 11 forks source link

Should we require the status list entries to be a multiple of the statuses per byte? #235

Open TimoGlastra opened 3 months ago

TimoGlastra commented 3 months ago

e.g. if you have a status list with 4 status, and bit size 1, you encode this and then decode it, the decoded status list will have 8 entries, as you encode the status list per byte.

So I think we should maybe do a check that the <totalStatus> % (8 / <bitsPerStatus>) equals 0

E.g. a status list of length 256, with 1 bit per status = 256 % (8 / 1) = 0

However a status list of length 4 with 1 bit per status = 4 % (8 / 1) = 4

This will ensure the initial status list will always equal the decoded status list

cre8 commented 3 months ago

Mhh, I would not prefer to limit the user that he needs to think about what should be n so n % 8 = 0. I understand that the result of encoding and decoding in the future will not produce the same output for the first run.

Here we are calculating the size based on the byteArray, there is no hardcoded reference how many elements should be managed. And since we have to set a value for the bit in the byte, it is 0.

TimoGlastra commented 3 months ago

As having implemented this functionality in Credo i spent quite a chunk of time debugging why my status list was longer than I configured it.

I thought it was a bug in the library, and finally i found that it goes per byte.

So passing 4 is the same as 8, i think it's useful to throw an error and say that you should pass a size of multiple of 8 / bitSize.

We can also implement this on the Credo layer, but i think unless there's a good reason why you would ever want to create a list of size 4 that ends up at 8 anyway, we should guide users to put in the correct length in the first place

cre8 commented 3 months ago

I would prefer a longer status list with more entries than intended, rather than forcing the use user to pick a specific value. Pick a random number has also no impact on the efficiency of the algorithm. When he says "I need a list of 799" elements, it will not break any use case when there are 800.

From the functionality aspect, a longer list will break no test or system. Because you do not now the final length because of the compression after this.

TimoGlastra commented 2 months ago

Okay would like to hear some input of others on this (@lukasjhan, @berendsliedrecht). If everyone/most of us think it's ok to have that misalignment between input and actual created list I will close this issue.

lukasjhan commented 2 months ago

If input a number that is not a multiple of 8, get a result that is different from the intended number of bits as @TimoGlastra mentioned. But I think it would be complicated to use if it throws an error.

I think it would be better to handle it gracefully rather than throwing an error. So I'd like to suggest keeping the current direction and marking it clearly in the docs.