Add return length assert

[olehmisar]

To prevent fools (like me) from shooting themselves in the foot.
Add this assert:

assert(InputElements == div_ceil(OutputBytes, 3) * 4, "base64 length mismatch");

Add this check for encode and decode. You can grab dev_ceil implementation here.

https://github.com/grjte/noir_base64/blob/eb933f20b8175132f8e965e519b98c41ad805af1/src/lib.nr#L100

[grjte]

@olehmisar good point! I think that this exact fix won't work for the non-padding case, only for padding (which is the standard setting), but I will update it in the code to make padding optional that I mentioned in #15 , since it adds support for padding and addresses a similar error, and we can put in a slightly different assert for the non-padding case. (I'm just waiting for #18 to be merged to be merged so I can open a PR for the padding fix and then the follow-up for the base64url)

[grjte]

the ideal will be to compute OutputBytes from InputElements, which I get the impression from the inline comments will be possible at some point in the future (once arithmetic ops are supported on generics), but it would be cool to hear if the Noir team has a timeline for this

[olehmisar]

@grjte arithmetic ops are already supported on generics. But only them. You need div_ceil to calculate base64 encoding length: out = ceil(input / 4) * 3. It requires comptime fns in generics. Is it something that is on the roadmap @TomAFrench?

[TomAFrench]

Having support for arbitrary comptime functions in generics is definitely not on the roadmap. ceil(input/4) can just be done as (input + 3) / 4.

[olehmisar]

No, you have to ceil it

[olehmisar]

@TomAFrench if conditions were supported in generics, it would be written as:

fn decode<let N: u32>(input: N) -> [(N / 3 + if N % 3 == 0 { 0 } else { 1 }) * 4)] { ... }

[grjte]

helpful clarifications, thanks. Sounds like that inline comment should go away

[grjte]

In this case @olehmisar I'm just going to push the change to add padding and then alphabet support as they're already written to avoid making the PRs more complex & I'll leave this bug open

[olehmisar]

@TomAFrench oh yeah, you are absolutely right, ceil(a / b) can be rewritten as (a + b - 1) / b

[saleel]

Just saw this - should be fixed for both pad/non-pad once this is merged

[saleel]

Fixed in #21. Will be released in #20