ethereum / clrfund

Eth2 CLR project built on clr.fund
https://eth2clrfund.netlify.app/
GNU General Public License v3.0
29 stars 16 forks source link

Generalize how round info is fetched on app load #431

Closed samajammin closed 2 years ago

samajammin commented 2 years ago

Currently the data fetching of the app is built specifically for the Eth2 round use case - it essentially assumes we only care about the current round.

In App.vue we fetch the current round as soon as this root component is mounted: https://github.com/ethereum/clrfund/blob/develop/vue-app/src/App.vue#L106 We do this so we can load round information as soon as possible, e.g. in the RoundStatusBanner which we display on the landing page: Image 2021-10-05 at 2 03 06 PM

This all works well but we run into potential issues with the ProjectList component if the user wants to view historical rounds, e.g. via the rounds route: https://clr.fund/#/rounds. Selecting a round adds the round address to the route param (e.g. https://clr.fund/#/round/0xf8acacfA954742Dc4eaf8Bd8498F4DFdc01B3875) which ProjectList.vue uses to load the round info: https://github.com/ethereum/clrfund/blob/develop/vue-app/src/views/ProjectList.vue#L151 This could result in clashing data - ProjectList.vue would set the round using the route param but the polling in App.vue may overwrite this if it fetches getCurrentRound.

We should refactor this to avoid any potential clashes, e.g. by ensuring that App.vue only sets the round address to the current round if there is no address in the route params.

samajammin commented 2 years ago

Hey @auryn-macmillan @daodesigner as we've dug into this issue we've hit some open questions & are looking to get your thoughts on this...

As part of this "upstream merge" effort, we're working to generalize the frontend to handle fetching & presenting info from historical rounds.

In particular, we care about being able to filter the ProjectList by the projects that were included in that round only, and to see the results of that round. This keeps feature parity with the current monorepo.

e.g. to list projects of a specific round:

By default, the ProjectList view lists projects for the current round

We have this working but noticed it leads to some open product/UX questions. e.g. when clicking on a project page, which round should it be in reference to? Right now, there's no explicit reference to a round from a project page - the route is just /project/:project-id. So if a user clicks on a project page from the project list of a specific round, it uses that round. But if a user were to refresh a project page (or load it as the first page), the app will use the current round.

I'm a bit stumped to how the monorepo solves this currently. Looking at the code, it seems to suffer from this same issue but looking at what's deployed to clr.fund, it looks like all historical rounds utilize an IPFS upload. Using that approach it appears that the entire site navigation becomes rooted in the round saved by that IPFS hash, so navigation works as expected and keeps you "within that round". I'm unclear how exactly that works or how we could reproduce this by running the app locally. Is our understanding correct that the static project page gets uploaded to IPFS when finalized? Is this process automated? Or does it require manually uploading and adding the hash to the process.env.VUE_APP_EXTRA_ROUNDS? Given that we now have the subgraph implemented, would there still be a reason to utilize IPFS here instead of querying the graph?

One potential approach (as an alternative to this IPFS magic) might be to utilize the round as a route param across the app, e.g. so instead of just /project/:project-id for the project page URL, it would include :round-address/project/:project-id. This would create a unique URL for each project on every round (which feels a bit weird) BUT would allow e.g. a project to find their claim button for a specific round directly via a URL (which seems useful, rather than having to navigate to the project page from the project list for that round). Another option might be to use query params (e.g. /project/:project-id?round=round-address). Curious to learn why you took the current approach or if you had already considered alternatives like this. OR are we overthinking this entirely & the current UX is ok?

Thanks!

auryn-macmillan commented 2 years ago

Right, so with the current app, you drop the IPFS hashes of any rounds that were run with different Funding Round Factories into VUE_APP_EXTRA_ROUNDS in your ENV. It was built this way so that the app could display historical rounds that happened on different Funding Round Factories, especially those that had different interfaces in the contract and are not compatible with the current app, along with having the app display the correct round number for the current round.

If we can get the contract interfaces to a relatively stable place, I think the deployer and the subgraph could make the the current solution redundant, and provide a more consistent UX. (the current app is really confusing because it actually routes you to an old instance of the app with no way to get back to the current app)

Two complexities that we need to account for:

  1. Upgrading to new versions of the funding round factory. Either the contacts should be upgradable or the app/subgraph should handle new funding round factories elegantly.
  2. Network migrations. What happens when instances migrate to whatever the hot new execution environment is? Can the app handle this elegantly as well?
pettinarip commented 2 years ago

Closing. Issue moved to the monorepo https://github.com/clrfund/monorepo/issues/448