UniversityDAO / udao

The official UniversityDAO DApp repository.
GNU Lesser General Public License v2.1
4 stars 4 forks source link

Reading Integration with Ethers #41

Closed austin-davis1 closed 2 years ago

austin-davis1 commented 2 years ago

Two main things in this PR:

  1. Testing environment for Ethers (this was supposed to be original API but was scrapped). Location in /test
  2. Production API for Ethers (not completed). Location in frontend/src/data.

Please feel free to leave any comments if there are any questions

austin-davis1 commented 2 years ago

To try out the reading capabilities, do the following:

  1. Start hardhat node: npx hardhat node (in root)
  2. Run python deployment script: python auto-deploy-script.py (in /test)
  3. Run the EthersTesting.js file: npx hardhat run --network localhost EthersTesting.js (in /test)
  4. Spin up the site: npm start (in /frontend)
oslfmt commented 2 years ago

@CptAstro Ping me or request a review from anyone else when this PR is ready for review.

austin-davis1 commented 2 years ago

@CptAstro Ping me or request a review from anyone else when this PR is ready for review.

Reading functionality is all completed so it can be merged whenever. No proposals/grants load on the site at the moment since the writing part is not complete, but the functionality itself is written and ready to be integrated.

oslfmt commented 2 years ago

Awesome, taking a look now. Make sure you change the title to reflect it's not a WIP anymore too.

austin-davis1 commented 2 years ago

Looks like most of the essential functionality is there. I'm a bit worried about the efficiency of the implementation though. Right now, it seems like you're fetching the appropriate data in each component. e.g. fetching grants & proposals in dashboard component, proposals in proposals component, and grants in the grants component. Since each call to these getX() functions involves a call to getProposalIDsWithCID(), there's a lot of duplicate work going on here. I'm wondering if we can do everything in the root component, ie, App.js. Here, we can fetch the contract abi's + set provider + getGrants + getProposals. Then we can just pass the data (in a react prop) to the respective component, ie, grants to grants, grants & proposals to dashboard, etc. This would require just one blockchain read operation, compared to the (at least) 4 reads in the current implementation.

Other than that, it looks good besides some smaller issues.. We'll probably need to do more thinking about the efficiency, I haven't fully thought through the above strategy and don't want to jump to it if it ends up causing different problems. Efficiency may be something we want to worry about later, if it ends up being an major issue.

That was actually my next PR, and I'm working on that right now. I originally loaded everything separate on each page because I was constantly having to refresh and what not when I was first getting functionality set up. I definitely agree that loading on each page is not ideal in production, which I why my next PR I'll have it so that when App launches (when launch app is clicked) we will grab all the data then and pass it down as props to each of the sub components. I'm not super worried about the Ethers functions time efficiency, more so the IPFS stuff since the loading speed is so unpredictable.