Skip to content

Comments

feat: upload long vote description to IPFS. Add new vote as example.#183

Merged
BATMAH69 merged 21 commits intolidofinance:masterfrom
BATMAH69:feature/ipfs-vote-description-v2
Sep 5, 2023
Merged

feat: upload long vote description to IPFS. Add new vote as example.#183
BATMAH69 merged 21 commits intolidofinance:masterfrom
BATMAH69:feature/ipfs-vote-description-v2

Conversation

@BATMAH69
Copy link
Contributor

@BATMAH69 BATMAH69 commented Aug 6, 2023

Description

Goal

We want to have long-text free-form description for the onchain votes. Don't want to fit it into transaction, so would rather use IPFS for storage.

Proposed result

Tweak the vote.lido.fi UI code (next.js typescript app) so that if the vote data has ipfs hash (among other text), UI shows separate field with the text loaded from that IPFS hash (links in this text should be regular links in the UI).

The description should be uploaded to the IPFS upon the vote creation in script from https://github.com/lidofinance/scripts. The utils should have the function uploading the text to ipfs & adding the link to it to vote description.

Demo

case examples (with ipfs description | without):

CLI messages in sllent mode:

image

CLI messages in ineraction mode:

image
image

Markdown examples:

clear markdown in grid view:

image

Code review notes

New env variable should be added WEB3_STORAGE_TOKEN api key form web3.storage

Testing notes

I created vote only as example.

Checklist:

  • Checked the changes locally.

@BATMAH69 BATMAH69 requested a review from a team as a code owner August 7, 2023 12:32
@TheDZhon TheDZhon self-requested a review August 10, 2023 12:20
@TheDZhon
Copy link
Contributor

Hey-hey,

You've made a lot of here, awesome 👍

will have a look till Monday's EOD 👀

Copy link
Contributor

@TheDZhon TheDZhon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done the first pass of a review 👍

The idea is neat and essential for the vote process transparency. I am all for it!
Thank you for driving the change here.

Left a few considerations and suggestions. To sum up, two points arise from my side:

  • maybe we can improve cross-validation of description and overall sanity checks (not only for format but for the content itself)
  • maybe we can detect what is the preferred IPFS service provider based on env vars set and give up only if no one was provided

Copy link
Contributor

@TheDZhon TheDZhon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done the second pass 👀

@TheDZhon
Copy link
Contributor

The merge conflict has been appeared with the latest master branch updates 👀

Copy link
Contributor

@TheDZhon TheDZhon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👀 Added a small set of updates

Copy link
Contributor

@TheDZhon TheDZhon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have my approval for the outstanding work here 👍

Please consider making a final decision about the description format enforcement to merge it into the master branch.

cc: @zuzueeka

Co-authored-by: Eugene Mamin <TheDZhon@gmail.com>
Copy link
Contributor

@zuzueeka zuzueeka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job on this! This should really simplify understanding the vote for voters.

I do have some minor concerns about the necessity of the vote_desc_items at the moment. Will there be any duplication of text from the general description? I've started a discussion in the Gov team chat about this.
upd. I don’t have any more questions about vote_desc_items and duplication of the items, Jenya K. made it clear to me.

Also, I was wondering if it's possible to include links in the description, such as "Item 1. Onboard 2 NOs. Discussion." I'd love to see how this would look on the test network.

@TheDZhon TheDZhon requested a review from kadmil August 15, 2023 16:54
Copy link
Contributor

@zuzueeka zuzueeka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like everything! Thank you so much for your hard work.
The only small comment I have is that I would like to see the CID displayed as a link after the voting points are displayed (before start).
Once this is done, you have my thumbs up 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants