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

Detect and manage embedded go-ipfs in Brave #947

Closed
lidel opened this issue Nov 30, 2020 · 7 comments · Fixed by #956
Closed

Detect and manage embedded go-ipfs in Brave #947

lidel opened this issue Nov 30, 2020 · 7 comments · Fixed by #956
Assignees
Labels
area/brave Issues related to Brave Browser effort/days Estimated to take multiple days, but less than a week exp/wizard Extensive knowledge (implications, ramifications) required P0 Critical: Tackled by core team ASAP

Comments

@lidel
Copy link
Member

lidel commented Nov 30, 2020

This supersedes embedded js-ipfs (#716)

Brave Nightly will ship embedded go-ipfs soon and will expose some API endpoints for ipfs-companion to facilitate end-to-end integration. Custom WebExtension APIs will enable ipfs-companion to:

  • read user preference on how to load IPFS resources (public gw, local gw, embedded one, disabled)
  • read $IPFS_PATH/config of embedded go-ipfs node managed by Brave
  • start/stop the node

We should at the very minimum:

  • add embedded go-ipfs to the list, if user enabled it in brave
  • on initial run, use embedded node as a fallback
    • detect when embedded go-ipfs running, and save it as the default instead of 5001/8080 one
    • this should be fallback, we dont want to break Brave users who already use ipfs-companion+ipfs-desktop

TBD:

  • start (and stop?) embedded node when user switches Node Type in Companion's Preferences
  • hide/migrate embedded js-ipfs to embedded go-ipfs

Details

Excerpt from brave/brave-browser#10220:

// Checks if a feature flag is enabled
chrome.ipfs.getIPFSEnabled((enabled: boolean) => {})

// Obtains a string representation of the resolve method
// method is one of the following strings:
// "ask" uses a gateway but also prompts them to install a local node
// "gateway" uses a gateway but also prompts them to install a local node
// "local" uses a gateway but also prompts them to install a local node
// "disabled" uses a gateway but also prompts them to install a local node
// Throws an error if IPFS feature flag is not enabled
chrome.ipfs.getResolveMethodType((method: string) => {})

// Launches a daemon
// Throws an error if IPFS feature flag is not enabled
// Throws an error if a local node is not installed
chrome.ipfs.launch((success: boolean) => {})

// Shutsdown a daemon
// Throws an error if IPFS feature flag is not enabled
// Throws an error if a local node is not installed
chrome.ipfs.shutdown((success: boolean) => {})

// Obtains the config contents of the local IPFS node
// Throws an error if IPFS feature flag is not enabled
// Throws an error if a local node is not installed
chrome.ipfs.getConfig((success: boolean, config: string) => {})

// Checks if the local node is installed
chrome.ipfs.getExecutableAvailable((available: boolean) => {})

chrome.ipfs.* behavior in Brave Nightly @ v1.20.36

  • getIPFSEnabled is enabled by default in Nightly, but we should check it as a formality
  • getResolveMethodType is the actual hint of user's preference
    • ask is the default on an empty profile
    • local – after "Enable IPFS" is clicked and go-ipfs daemon got installed
  • launch
  • shutdown always returns true (even if node was already off)
  • getExecutableAvailableget returns true after install

References

@lidel lidel added P0 Critical: Tackled by core team ASAP area/brave Issues related to Brave Browser exp/wizard Extensive knowledge (implications, ramifications) required effort/days Estimated to take multiple days, but less than a week labels Nov 30, 2020
@lidel lidel self-assigned this Nov 30, 2020
@lidel lidel pinned this issue Nov 30, 2020
@lidel
Copy link
Member Author

lidel commented Dec 18, 2020

Quick mockup + temperature check on leveraging Brave colors (https://brave.com/brave-branding-assets/) to indicate when Node is controlled by the browser itself:

2020-12-18--17-20-42

Is there a more subtle way?

@jessicaschilling
Copy link
Contributor

How about a tiny Brave lion to the left of the box(es) in question? Otherwise the color looks a bit like an error state.

Plus if we wanted to be really fancy the lion could be orange or purple, depending on if regular or nightly Brave.

Would add a hover state for the icon, too: "Managed by Brave".

@lidel
Copy link
Member Author

lidel commented Dec 18, 2020

@jessicaschilling I was playing with various versions and ended up with the lion inside the input (background-image) on the right – feels the least intrusive and does not require any special-casing in existing layout:

border → no border → muted background to indicate read-only
2020-12-18--21-03-48 2020-12-18--21-10-01 2020-12-18--21-16-05

Final version on the right.

Thoughts? Should we keep the orange border, or is the lion enough?

@jessicaschilling
Copy link
Contributor

Muted background is great - and no need to keep the orange border.
I might move the lion to the left, though - parses better on wide screens, and is in line with the "icon indicator + value" convention in the browser URI bar.

lidel added a commit that referenced this issue Jan 5, 2021
@lidel lidel closed this as completed in #956 Jan 7, 2021
lidel added a commit that referenced this issue Jan 7, 2021
* feat: leverage IPFS node from Brave

Adds "Provided by Brave" node type and closes #947

* feat: trigger Brave with proper onboarding page

This replaces ipfs:// URI used for triggering activation dropbar
with a page that can be updated by Companion to reflect the stage
of activation.

Co-authored-by: Jessica Schilling <[email protected]>
@RubenKelevra
Copy link

@jessicaschilling I've activated this in my brave browser and was a bit surprised about the low change in color between "installing" and "install completed" pages. And there was like no link to further information on what IPFS actually is or to the mentioned settings. Is there a chance to change this page a bit? :)

@jessicaschilling
Copy link
Contributor

@RubenKelevra That's already being worked on Brave's end - the page you're referring to is really intended as an interim measure. See brave/brave-browser#13655.

@RubenKelevra
Copy link

Ah, wonderful - thanks!

Wasn't sure where to put such a question so I didn't created a ticket. Just was reminded by your discussion here about this.

If it's a temporary solution anyway its fine for me. It was just a little hard to catch that something has finished (:

@lidel lidel unpinned this issue May 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/brave Issues related to Brave Browser effort/days Estimated to take multiple days, but less than a week exp/wizard Extensive knowledge (implications, ramifications) required P0 Critical: Tackled by core team ASAP
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants