zksecurity / wasmati

Write low-level WebAssembly, from JavaScript
MIT License
219 stars 6 forks source link

Integer constant validation #9

Closed Mike-Dax closed 3 weeks ago

Mike-Dax commented 3 weeks ago

I had a bug in my code where I was passing a float to i32.const intermittently.

Would you consider validating that the passed in value is a valid integer at the call site of i32.const(123) (and other integers)? Otherwise it errors at the BigInt conversion which is hard to correlate with the actual code that generated the call.

(I'd be happy to open a PR with direction of a good place to do this check, I was doing it in baseInstruction and just checking the i32 and i64 const instructions, but this could be done more generally.)

mitschabaude commented 3 weeks ago

Ah that's good feedback, thanks @Mike-Dax!

I think I want instruction-specific validation to live locally next to the instruction definitions. So what I'm going to do is add another instruction factory, say baseInstructionWithValidation(), which takes an additional validation callback that runs before creating an instruction. And then define i32.const and i64.const using that factory

mitschabaude commented 3 weeks ago

fix merged and released with 0.2.4!

Mike-Dax commented 3 weeks ago

Amazing, thank you!