Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

iOS Safari Compatibility #823

Closed
bkase opened this issue Mar 30, 2023 · 18 comments · Fixed by #933
Closed

iOS Safari Compatibility #823

bkase opened this issue Mar 30, 2023 · 18 comments · Fixed by #933
Assignees
Labels
bug Something isn't working good first issue Good for newcomers to-research

Comments

@bkase
Copy link
Member

bkase commented Mar 30, 2023

The new iOS Safari release supports nested web worker spawning, and SnarkyJS with Safari on desktop seems to work; however, iOS Safari SnarkyJS does not seem to according to some internal testing by @shimkiv

Observed behavior is that something is breaking during SnarkyJS load-time, isReady never resolves and nothing prints.

Note that this does work on an iOS simulator.

We believe this is just a matter of working through the errors one by one until it works! It would be a great first issue for a contributor as you'll step through different parts of SnarkyJS trying to dig through errors.

@bkase bkase added the bug Something isn't working label Mar 30, 2023
@mitschabaude mitschabaude added the good first issue Good for newcomers label Apr 3, 2023
@barriebyron
Copy link
Contributor

I thought of this bug when I read the last paragraph in
https://book.leadthe.dev/people/diversity-is-key.html

an analysis on the bug my teams had worked on in the past few months. After some crunching, an obvious number stood out. More than 85% of our frontend bugs happened on Windows browsers. I was puzzled at first but then something really obvious hit me. My whole organisation used mac laptops. All our frontend developers used Chrome. We were a monoculture! You know why I use this story all the time? It's because it happened in the team I just mentioned. We thought we were a diverse team. Well, we weren't if we had considered the OS/Browser dimensions!

@shimkiv
Copy link
Member

shimkiv commented Apr 11, 2023

When it comes to UI we actually doing diverse testing, which includes different desktop operating systems like macOS/Windows/Linux, different mobile OS like iOS/iPadOS/Android, different browsers (Chrome, Firefox, Safari), etc..

Speaking of issue itself, an OOM might be happening on iOS devices as it was discovered by running project generated by zkApp-CLI + JS/TS Framework like SvelteKit.

@ryancwalsh
Copy link

Hi @bkase ! Could you (or anyone who knows) please add steps to reproduce?

(and any updates / insights / hypotheses from the past month)

Which combos of iOS / Safari have problems? Which don't?

Which simulators work, and which don't, and what exactly happens?

Will anyone be assigned to this issue soon?

Thanks!

@mitschabaude
Copy link
Member

I think @shimkiv has found these issues and might be able to help with reproducing?

@jdsteinhauser
Copy link

@ryancwalsh, I was able to create it the following way, for my test project wat:

  • Install zkApp CLI (version 0.7.6)
  • Create a new project zk project wat
  • Created accompanying UI project, using svelte
  • Created a SvelteKit skeleton project
  • Added type checking with TypeScript syntax
  • Selected no additional options
    After creating the project, I modified just the ui/src/routes/+page.svelte source file
  • Replaced L10 with alert("Let's try this");
  • Replaced L12 with alert("Hey it worked");
  • Added Berkeley Add zkApp address to L16: const zkAppAddress = "B62qkwohsqTBPsvhYE8cPZSpzJMgoKn4i1LQRuBAtVXWpaT4dgH6WoA";
  • Ran npm run dev

When accessing localhost with Chrome and Safari (on macOS), both alerts will fire. However, with Safari on an iOS emulator, only the first alert is seen

@stevenpack
Copy link

@nicc have pushed this up the backlog for next sprint.

@shimkiv
Copy link
Member

shimkiv commented May 24, 2023

@jdsteinhauser are you sure that Safari on iOS emulator won't do the same for any simple html+js you mentioned? Like they might prevent consequent alerts spawning by default for instance.

@shimkiv
Copy link
Member

shimkiv commented May 24, 2023

Another interesting finding is that SnarkyJS won't work on WebKit built from sources. It was discovered by using Playwright's webkit installed by npx playwright install --with-deps then ~/Library/Caches/ms-playwright and launch browser with script. It reflects same (most likely) issue as iOS browsers.
It is interesting because at least we have this issue on something that can be built from sources and hence, help with debugging.

@jdsteinhauser
Copy link

@jdsteinhauser are you sure that Safari on iOS emulator won't do the same for any simple html+js you mentioned? Like they might prevent consequent alerts spawning by default for instance.

I haven't tried SnarkyJS alone without a framework in Safari on iOS

@shimkiv
Copy link
Member

shimkiv commented May 24, 2023

@jasongitmail I actually talking about any simple html + js alerts.

@mitschabaude
Copy link
Member

@jasongitmail I actually talking about any simple html + js alerts.

you tagged the wrong Jason :P

@shimkiv
Copy link
Member

shimkiv commented May 24, 2023

hehehe sorry for that :)

@jdsteinhauser
Copy link

@shimkiv Ah, I gotcha now. Sorry for the confusion! Yes, I can get alert modals to show up with plain html/js on an iPhone emulator

@MartinMinkov
Copy link
Contributor

MartinMinkov commented May 28, 2023

After some exploration, I successfully made a SnarkyJS build operate with Safari on iOS.

The critical adjustment involves modifying the requested WebAssembly memory within a particular function, which can be found at this link: https://github.com/o1-labs/snarkyjs-bindings/blob/9948427f992545da1f6919fa5d4445437da751d9/compiled/web_bindings/plonk_wasm.js#LL7746C77-L7746C90

Through experimentation, I discovered that the memory error ceases to occur by reducing the maximum memory value to a smaller amount, for instance, 16384 (each page is 64KiB, which equates to 1 GiB of memory).

Here is an example:

imports.wbg.memory = maybe_memory || new WebAssembly.Memory({initial:19,maximum:16384,shared:true});

The current memory size is 4 GiB (65536 pages). This seems to be fine for Chrome but for iOS, there might be issues with that size. It's worth noting that doubling the memory to 32768 results in a failure due to memory exhaustion. Perhaps that could be some browser limit set for iOS devices? Trying to find any documentation related to memory usage is difficult, this doesn't seem to be easily accessible information :(

The implications of reducing the memory are unclear to me now - it could lead to downstream issues. One possible solution could be to perform a browser check and exclusively reduce memory for iOS agents if there is no issue of lowering memory.

@mitschabaude, what kind of errors (if any) could occur from lowering the maximum memory requested from 4GiB to 1GiB?

Finally, the requested memory is used to load the plonk_wasm.js file to provide additional context. Another potential solution would be lowering its size, but I'm unsure how feasible that is.

@mitschabaude
Copy link
Member

Awesome find @MartinMinkov! The Wasm memory limit should be as large as possible - it's the constraining factor for circuit size. So I'd be in favor of detecting iOS browsers, maybe we can look at 'navigator.userAgent'

@mitschabaude
Copy link
Member

@MartinMinkov I think we can create the memory in 'initSnarkyJS()', with limits depending on the user agent, and then pass it to the init() function ourselves

@MartinMinkov
Copy link
Contributor

Another potential issue I considered is that there might also be memory differences between iOS devices.

I was testing an iPad Pro running iOS version 16.3.1. I haven't confirmed the memory fix to work on iPhones as well, but I could imagine that smaller devices could have even a lower maximum memory parameter. I'll do some testing with an iPhone to make sure :D

@MartinMinkov
Copy link
Contributor

MartinMinkov commented May 28, 2023

I confirmed that 1GiB of memory also works for an iPhone SE running 16.1.1. 🎉 This should be a safe number for mobile iOS devices.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers to-research
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants