Venus Protocol Analysis for PR #146#201
Venus Protocol Analysis for PR #146#201GiantDole wants to merge 53 commits intodeficollective:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
emduc
left a comment
There was a problem hiding this comment.
Thanks a lot for your review! I can tell you put a lot of work into it and there are some really good points in there. Here are some general comments
-
Could you make sure it compiles properly and renders on the preview? You can run
npm run devto run it locally and make sure it appears, it will warn you of potential compilation issues -
Here are some general style guideline we try to follow:
- use "onchain" instead of "on-chain"
- use code style for tokens and contracts:
BEP20instead of "BEP20" - we use Italic when mentioning specific protocol modules or categories in the conclusion (eg. Upgradeability)
- We group all permissions in the table at the end. We realized that since there are diagrams, it should be easier to refer to those than to the permissions table.
I also left several direct comments as you can read below. Once those are addressed we would be quite close to a final version. I'm looking forward to reading your updates ! :)
emduc
left a comment
There was a problem hiding this comment.
Some further small changes to make. I made a suggestion for the Diamond proxy in the diagram. We know this is not strictly speaking accurate since the implementation contracts are separate contracts, but with proxies in general, we made the decision to represent them inside the Proxy's box, to avoid any confusion as to which contract users should interact with. I also changed the Multisig on the governance diagram, please also push the updated version (as this is your fork, I cannot push to it).
Some of the comments are about the ## Key permissions sections, overall we try to avoid bullet points, as those are often AI coded and may leave some information out.
Thanks again for your work! It's great stuff :)
Co-authored-by: Emilien Duc <56789637+emduc@users.noreply.github.com>
Co-authored-by: Emilien Duc <56789637+emduc@users.noreply.github.com>
Co-authored-by: Emilien Duc <56789637+emduc@users.noreply.github.com>
Co-authored-by: Emilien Duc <56789637+emduc@users.noreply.github.com>
Co-authored-by: Emilien Duc <56789637+emduc@users.noreply.github.com>
|
@yvesbou is attempting to deploy a commit to the defiscan Team on Vercel. A member of the Team first needs to authorize it. |
Completed analysis of Venus Protocol in
venus-protocol.md. Added two architecture diagrams for the lending core protocol and governance.