search

f-o-a-m / purescript-web3

a purescript library for the web3 api
Apache License 2.0
127 stars 24 forks source link

Fix: Long HexStrings cause a `RangeError` (js recursion) #36

Closed XertroV closed 6 years ago

XertroV commented 6 years ago

While publishing some contract code I noticed mkHexString will fail on long strings. (E.g. mine is 7332 chars including 0x).

Particular error is: RangeError: Maximum call stack size exceeded

I've added a test showing the failing case. Implemented a solution like: https://github.com/purescript-contrib/purescript-parsing/issues/34

It works, but see my comment further down regarding speed.

XertroV commented 6 years ago

I've refactored mkHexString to use a Trampoline now - this resolves the recursion issue at the expense of speed.

It's hard to get good comparisons to the non-trampolined way of doing it since it breaks, but I did notice that a hexstring of length 1024 32 was VERY slow - to the point I ctrl+c'd the test. I've made the test smaller at 128 32 (which still fails with the old way) but works with the trampoline.

All in all I'm not sure happy since it seems like there is a lot of computation going on with the parsing but at least it works now. Seems like mkHexString maybe shouldn't use a parser and there might be much faster ways out there.

martyall commented 6 years ago

i mean, i agree with the fact that maybe it just shouldn't be a parser, but this is fine for now if it's working.

🎉

XertroV commented 6 years ago

Agreed that it's okay for the moment. If I have any performance issues I'll let you know, but I don't anticipate any.

Also, I'm pretty sure I've written some hex parsing code somewhere. Next time I come across it I'll do some comparisons and benchmarks. If the difference is significant I'll create a PR then.

PS. Thanks for the quick merge!