Ask ipfs.config for gateway address where possible

[olizilla]

Hi @lidel 👋

This PR aims to make a small improvement to the out-o-the-box experience, by asking the ipfs api what it's config and using that where the user hasn't provided a non default override. This PR is deliberately the smallest change that could work, as I'm new here, and wanted to get your thoughts on it.

I'm working on integrating ipfs into brave, building on top of the work that @diasdavid has done. I'd like to get ipfs-companion working in brave, but the first issue I hit was I had to run my local ipfs gateway on a port other than 8080 as the brave dev process uses 8080, so I figured I could learn a thing by getting ipfs-companion to find the gateway port from the api.

[olizilla]

One thing that was tricky with the current set up was I couldn't see a way to know if the user had explicitly set an option, or if it had be added in by storeMissingOptions. What's the intention behind that step? Would it be reasonable to only store options the user has set explicitly?

[lidel]

Hello @olizilla! Getting Gateway address from API is indeed a neat idea! 👌
This PR is a very good PoC, thank you!

I've come up with some UX edge cases that need to be addressed before we merge it:
(my apologies if I sound a bit chaotic, its 1am here 💤 )

1. There is no way to tell what was user's intention by looking at the value of customGatewayUrl alone. It is possible that user is ok with a default value and do not want to change it to one returned from API. This means we need to have a new checkbox that toggles this feature on the Preferences screen (relevant codepaths: src/options/options.html + initStates + onStorageChange), so that users can opt-out from this feature:

   • If the feature is enabled, we should make "Custom Local Gateway" read-only (as it will be set to a value fetched from API )
   • If the feature is disabled, "Custom Local Gateway" should be editable by user (current behavior)
   • Not sure how to name this new option -- any ideas? 🤔
   • If we want to be extra thorough, we probably should update the value of customGatewayUrl every time API changes state from offline to online and when user changes URL of API itself

2. https://unpkg.com/multiaddr@3.0.1/dist/index.min.js is 100KB in size -- I feel we could avoid including it by writing code and host extraction manually. In general we want to avoid external dependencies (Reproducible Build #306).

If you have time, feel free to work on it here 👍
If not, I can pick it up after more pressing issues are resolved and my bandwidth frees up.

---

Would it be reasonable to only store options the user has set explicitly?

It sounds good on paper, but my experience is that you quickly run into layers of checks and orchestration that increases complexity and degrades performance.

I feel some background is due here:
Our extension is inspecting every request made by the browser in a blocking manner, so performance is our top priority. I strongly believe we need to respect users and do our best to minimize impact on browsing experience, including memory and battery use. This often means we can't take "the easy route" that other extensions take.

To avoid reading various options multiple times on every request, background script maintains a local state object with normalized URLs and other heavily used flags.
Functions initStates and onStorageChange are responsible for keeping browser.storage.local and state in sync. It is not 1:1, as we do some pre-processing of things like URL object (do it once here instead on every request).
The storeMissingOptions simplifies a lot of things by ensuring browser.storage.local always has all the required keys. New/missing keys are loaded from src/lib/option-defaults.js and saved to browser.storage.local during startup. It guarantees everything is in sync and there are no missing values. One less thing to worry about.

You may think that is a problem, as we are unable to tell if an option "was missing, so we used the default" or "was manually set by the user to a value that just happens to be the default". In reality if you need to differentiate between the two you run into a risk of false-positives, which is a sign of a deeper UX problem. In case of customGatewayUrl we should just assume user wants the value fetched from API (with an option to opt-out). No need for comparing current value with default one.

I hope I did not make things worse with this elaboration, wanted to be thorough 🙂

I am aware some code patterns were ported from legacy add-on and may not feel "idiomatic" in webextension, but keep in mind when we started rewrite there were no webextensions around.
Let me know if something is unclear or could be simplified.

[olizilla]

@lidel this is really helpful, thank you for taking the time to write it up. I'll pin down the use-cases and figure out a plan.

Our extension is inspecting every request made by the browser in a blocking manner, so performance is our top priority.

👍 💯 ✨ strongly agree! I really like that this add-on is focused it is on giving a good user-experience for ipfs.

am aware some code patterns were ported from legacy add-on and may not feel "idiomatic" in webextension, but keep in mind when we started rewrite there were no webextensions around.

Don't worry, I totally know that feeling. It's all good! I've jumped in here because I think this add-on is doing a great job and the brave integration is best achieved by helping out with ipfs-companion rather than making another bespoke thing!

[alanshaw]

typo: IPSF

[alanshaw]

Can you just fetch the Addresses.Gateway value?

$ ipfs config 'Addresses.Gateway'
/ip4/127.0.0.1/tcp/8080

[olizilla]

Sure can. I thought that getting the entire ipfs config might provide other useful info. Perhaps we won't ever need it, but I don't think it's a bad gamble.

[alanshaw]

All the rest are pinned

[lidel]

This feature will be delivered as a part of #491 (comment) so I am closing this PR in favor of addressing it there.