search

coronanet / rn-coronanet

React native interface of the Corona Network
12 stars 4 forks source link

Ghostbridge RN API Integration V1 #12

Closed daodesigner closed 4 years ago

daodesigner commented 4 years ago

resolves part of https://github.com/coronanet/rn-coronanet/issues/11

77244717-e107e180-6bee-11ea-9381-31278e48b939

https://www.figma.com/proto/y3VSBxXOrX6f6tCeHFF7HB/Corona-Net?node-id=1%3A4438&viewport=-346%2C2816%2C0.4470480978488922&scaling=min-zoom

daodesigner commented 4 years ago

set up assets, clean up component styles, wrap up for the day. assets

karalabe commented 4 years ago

I'm wondering if we could break this up into smaller pieces so that independent things can get merged, while not-quite-ready stuff can remain open? I think the image assets could easily be merged, the API wrappers are fine-ish, with some tweaks. It would make things flow faster if all of them didn't need to wait for each other.

daodesigner commented 4 years ago

Will clean up and publish a working version as well as implement suggestions today after work, agree don't want to be the bottle neck, just needed to get some quick and dirty code down. Will try to add some docs as well.

karalabe commented 4 years ago

I've played a bit with the demo on Figma, a few thoughts:

My original idea for the 3 main "pages" of the dashboard after a signup was:

I'll think about events a bit and how this whole thing would look like on the REST API side. That might clear it up a bit for me + maybe I can clear up my threat model.

daodesigner commented 4 years ago

** forgive my pseudo code I know these aren't real endpoints/not commenting on the API at all

  • I'm wondering if we could turn the first one into the splash screen that just shows up while things are loading and then switch over to the second for signup?

Hard to Mock loading screen in Figma but consider it done.

  • On the second page, I'd replace the generate private key with simply create new user. All the crypto and P2P and decentralized mumbo jumbo are irrelevant for the user actually in front of the screen.

Done.

  • For signing up, I think a subset of the profile update screen would be needed. The only really important thing to set initially is a single full-name string. Everything else can be done later (especially virus status and message and whatnot), definitely not when creating a user.

Done.

  • The location is not something I'd add. It complicates things a lot (GPS access or even rough location access requires permissions, a lot of nasty java code); and we won't be using it anything (at least not for now), so going without it seems saner.

Was thinking just make this a text string like twitter does, as a privacy nut myself usually just leave it blank or put a Pink Floyd reference (https://twitter.com/quentinc137) not validated/GPS at all, but useful from a social network perspective.

was thinking this as the full UX interactions for contact management:

Ben hits QR/pair  --> *Display QR on screen while valid* // should agree on a timeout so people cant spam your phone?
Alissa hits QR/scan -->  *Scan QR with camera* --> Display Profile Page ---> Alissa hits Follow //Remove the contact if navigates away instead
Alissa hits profile/Ben --> Unfollow (delete contact)

*know some of this UX is missing presently from prototype will remove the Follow Button step and just display profile.

  • I'm also a bit unsure how the events come mixed in with contacts? Could you detail what your thought pattern was?

In the same way that you have a list of events that you attended/hosted you should be able to see the events others attended 

Profile:{
  contactId, // added to profile on create 
  name,
  ...
  [...eventID] 
}
Contact: {
  contactId, // added to contact on pairing
  name,
  ...
  [...eventID] // allows reduce((id)=>inMyEvents(id))
}
Event:{
  eventID, // added on event create 
  contactID, // can only sync event data from host?
  metadata,
  ...
  registered: uint // set by host when event is created
  checkedIn: [...contactId] // allows len(checkedIn), reduce((id)=>inMyContactsAndInfected(id))
}

My original idea for the 3 main "pages" of the dashboard after a signup was:

  • A timeline that is akin to a facebook feed.
    • It contains updates from your contacts; and updates from events you attended (if relevant to you).

Not included in prototype MVP for Coronanet, agree would be nice for social network - will mock

Done

  • We can also create shareable URLs that another phone could click on + auto join via the app, but I'd need to figure out how those work on Android first. That would allow making friends remotely.

Would be awesome, how many concurrent pairings can a phone do at once? might be tough being popular.

  • An list of events I attended

Currently added to your profile, but could make an overall events page.

Profile

  • Here I'd add an option to organize a new event or join an existing one through somebody. In the latter case, I'd go as far as only support joining via QR code, to enforce physical contact. Otherwise I'd worry about malicious users posting the joining code/url somewhere public and then messing with the thing.

These are kindof *already in the profile but will think of a way to make it more clear. (In your profile if you are a Host you get the "check-in guest" option; if you are browsing a Host's profile while the event is live you get "request check-in"

*some of this UX is missing presently from prototype

Guest hits QR/request checkin  --> *Display QR on screen while valid* 
Host hits QR/checkin guest-->  *Scan QR with camera* --> Update Event Info --> Broadcast

CheckIn RequestCheckIn

I'll think about events a bit and how this whole thing would look like on the REST API side. That might clear it up a bit for me + maybe I can clear up my threat model.

karalabe commented 4 years ago

was thinking this as the full UX interactions for contact management: [...]

Hmm, that could awork, but we'd need to make the pairing protocol a bit smarter. Currently during pairing (after scanning the qr code, behind the scenes) there's just a super dumb "here's my pubkey and here's my address" exchange.

Now, the current model does support connecting to someone, and then disconnecting if you don't like them "fast enough", but that entails a window where the peers can leech data from one another, that you might not want to share.

Also there's a problem once you revel your Tor onion address: even if you deauthenticate the other side, they can still check if you're online (reachable or not). The tornet package does address rotation for these cases (when you unfriend someone), but that entails a long winded process until all your contacts are updated to the new address.

An alternative would be to extend the peering protocol https://github.com/coronanet/go-coronanet/blob/master/spec/wire.md#pairing-protocol-v1-draft so that it's not just a blind exchange, rather it could also swap profiles and require a second confirm step before the actual real pubkey/address is exchanged. That's a safer approach but will need a bit of backend work. I'll thnk about it a bit more, see how hard that would be.

Would be awesome, how many concurrent pairings can a phone do at once? might be tough being popular.

Not sure honestly. Currently I limited it to 1 because you can't share more than 1 QR code at the same time. I don't really see a technical limitation to run arbitrary many. There is probably a bandwidth/battery hit to maintaining multiple onions (I think 1 onion drained 0.3%/24h battery for me + 10MB/24h bandwidth).

The catch with many pairings is that the user should not share the same pairing URL with more than one person (otherwise that URL might end up public somewhere, murdering your phone with inbound requests).

That said, if we make pairing multi-step like with the profile exchange from the previous point, we could prompt the user to explicitly accept friend "requests", so then we could even get away with running a single pairing "server" and accepting arbitrarily many inbound connects. This seems like a promising/friendly approach.

Currently added to your profile, but could make an overall events page.

That could work too, just wondering how to be able to see "events" and "updates" cleanly too. I.e. would be nice to somehow see at a glance the events I participated in (and if any of those had nasty updates) and separately a feed of stuff that happened throughout my network. I'm honestly fully open to suggestions here, just want to keep in mind that "status updates" might be numerous, so we probably don't want to mix in the events in between.

But all in all anything that is clear works for me, I'm not married to any particular design :)

daodesigner commented 4 years ago

Ready for merge after review, fully decoupled UI from ghostbridge should be easy enough to follow this pattern for the rest of the API once we agree on user stories from prototype. Will take a look at the peering protocol and update Figma tomorrow to include UX for events as well as incorporate feedback.

karalabe commented 4 years ago

Just a quick note, I haven't forgotten about this PR, just doing a crash course in RN hooks to understand all the funky constructs in it :D

daodesigner commented 4 years ago

Happy to talk you through it, if hooks are new to you this is diving in way in the deep end (not going to find a tutorial for this redux-thunk pattern alternative). Doing the same for the tor spec./go libs you're using in the coronanet back end any pointers ? That said, the whole point of writing it this way is you really only need to know how to useContext all the networking/state management is black boxed.

0mkara commented 4 years ago

@daodesigner I saw there was a DELETE API to stop the network. I feel we might need to expose that feature as well. https://github.com/coronanet/go-coronanet/blob/master/rest/gateway.go#L48

daodesigner commented 4 years ago

@daodesigner I saw there was a DELETE API to stop the network. I feel we might need to expose that feature as well. https://github.com/coronanet/go-coronanet/blob/master/rest/gateway.go#L48

100% should expose the remainder of the API, def. some low hanging fruit just want to get this merged in first to let others contribute following the software design pattern I laid out. Might just wrap it in it's own repo and turn into an npm package we can import back into this repo which should focus on UI implementation anyway.

karalabe commented 4 years ago

Happy to answer any questions, just post them on the Go repo and I'll do a braindump :)

karalabe commented 4 years ago

@daodesigner I'm not a fan of scattering things across repos really. Too many times it happens that you need to modify both places at once and then it's an annoying sync issue to keep pushing into multiple repos. We deliberately kept go-ethereum as a single repo because of this and retrospectively it was a very good decision.

karalabe commented 4 years ago

I've made some tweaks to the code, mostly as I was going along and trying to wrap my head around the RN hooks. Hope you don't mind :)

This is my current understanding of the design, please correct me if I missed something (I've never in my life seen RN hooks before, so keep that in mind if something I say is wrong or super obvious):

My commit also reindented the js file to only use a single tab, not two. I guess this might have been an editor accident? Hope I didn't mess up some standard indentation here.

Also if I understand correctly the provider in its current form is the starting point for testing and the utils.js is not used at all, so I didn't bother to clean up those as there wouldn't have been any point.

Did I understand the core correctly above? If yes, then we can merge from my part :)

Btw, sorry or going ahead and modifying the code instead of leaving review comments, but I figured I need to go through it and document it myself to really understand what's going on, so might as well push that instead of losing it.

daodesigner commented 4 years ago

I've made some tweaks to the code, mostly as I was going along and trying to wrap my head around the RN hooks. Hope you don't mind :)

totally fine, will PR a few small changes back in but overall agree with the name changes for variables, makes the code easier to read.

This is my current understanding of the design, please correct me if I missed something (I've never in my life seen RN hooks before, so keep that in mind if something I say is wrong or super obvious):

will try my best :)

The useFetch creates a callable object that can call an API endpoint and will populate (or repopulate previous) API results. The use as I understand is to create a wrapper for every possible API endpoint and whenever we want to call something, have that automatically cascade a plethora of UI updates based on the returned response.

The useFetch creates a cached async fetch call factory template, and parses responses into state variables. This keeps react from recomputing functions on each render.

I've seen that the response returned from the API is saved as plain text, not parsed as JSON. I have a hunch that you did it like this because my API errors are plain strings. Question: is this the standard way to do REST APIs fr react, or should I return a json in case of an error too (e.g. {"error": "bla bla bla"}? For me it's all the same, so whatever is the standard I can accommodate the Go code to it.

Yupp it is 100% because the API errors are plain strings, parsing as JSON fails. Would be better if we passed back a JSON object for all responses but current implementation is ok.

The useGatewayStatus, useGetProfile, useGetContactProfile are essentially factory methods that create these callable objects for very specific endpoints, that can cascade their changes/

yup, they are factory methods that create cached callable objects for very specific endpoints. They only cascade changes down to the state variables created by useFetch.

Here I've renamed a few to make them more consistent with the Go counterparts (e.g. dropped the ById (it's always by id and there's no other way), for pairing I renamed to Init/Join/WaitPairing). lgm It's not clear to me how updates work with these hooks. E.g. for useUpdateProfile, if I want to specify a specific body, that will create a new hook. Isn't that an issue that the API wrapper itself would get recreated? More specifically, the useUpdateContact would only allow a single contact to be updated at a time, because react hooks cannot be dynamically created (quantity wise)? If so, that's fine, I'm just trying to make sure I understand how these hooks work.

react hooks can be (and generally are recreated on each render unless you useCallback or useMemo) dynamically created. In the case of useUpdateProfile only the callback is recreated if the arguments to useUpdateProfile change. Hooks like this are generated on render, which is cheap and only exist while a user is updating a profile (can understand how some of this isn't clear since we haven't implemented any logic components).

The context is kind of like the "world state" for the app. We'd dump pretty much everything API accessor in there, then all UI elements can just dig up the name of the profile without having to retrieve it themselves.

The context is kind of like the world state and if done correctly automatically cascades state changes to trigger component re-renders only for components that use those pieces of state with useContext().

How does it work for multiple contacts? If I understand the hook system, I can only have one contact loader hook. I can of course load multiple contacts one after the other, but how would I be able to represent that in my context? Or would I place 1 useFetch hook for retrieving the data and N destination storage blobs to store individual results is successful?

will probably implement a custom hook to load all profiles. Generally there are two approaches here: create hooks for each contact and load data on render O(1) amortized or create a load all contacts hook and load all contact data to state before render O(n) upfront.

The provider is currently the automatically cascading things that magically does various things based on the results of API calls: Initially it starts out by trying to load the profile, which when completes triggers the profile creation hook, which is a noop if the loading was successful (ok, starts network). If the profile loading is unsuccessful, we try to create a new one and cascade back. Deletion is enabled to be reacted to, but is not called anywhere.

Yup, pretty much.

One note here: in the latest 0.0.4 release I did last week, I managed to break the dependency between user availability and network connectivity. The gateway can now be enabled and disabled independent if there's a local user available or not. As such, you don't need the connectToNetwork side effect as a reaction user creation. Currently the Java service that starts the gateway will enable networking itself already. The API is mostly if the user wants to enable/disable the network manually for some reason, but I think we might want to do that as a button on the notification itself, because it's already there so it feels more natural to support it.

sounds good will make the change.