-
Notifications
You must be signed in to change notification settings - Fork 43
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Initial draft of usage docs #192
Conversation
WalkthroughThe recent changes introduce comprehensive documentation and enhancements for the Sommelier network. New files delve into the UI workflows for cellar deposits and withdrawals, explain how fees and reserves are managed, outline the permissioning system, and provide guidelines on deploying and configuring cellars. Additionally, some code entities were updated to reflect the new functionalities and configurations introduced. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Cellar
User ->> Cellar: deposit(amount, receiver)
Cellar ->> Cellar: validate deposit
Cellar ->> Cellar: update user's balance
Cellar -->> User: confirmation
User ->> Cellar: withdraw(amount, receiver)
Cellar ->> Cellar: check withdrawal queue
Cellar ->> Cellar: process withdrawal
Cellar -->> User: confirmation
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 8
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- docs/cellar_ui.md (1 hunks)
- docs/fees.md (1 hunks)
- docs/permissions.md (1 hunks)
- docs/using_cellars.md (1 hunks)
Files not reviewed due to errors (3)
- docs/fees.md (no review received)
- docs/cellar_ui.md (no review received)
- docs/using_cellars.md (no review received)
Additional context used
LanguageTool
docs/permissions.md
[grammar] ~6-~6: The verb form ‘use’ does not seem to match the subject ‘contract’.
Context: ... ## Cellars The basic Cellar contract use the Auth contract from solmate from per...(SUBJECT_VERB_AGREEMENT)
[uncategorized] ~8-~8: When a number forms part of an adjectival compound, use a hyphen.
Context: ...in 1-5 min or calls that require a full 48 hour vote of the Sommelier token holders. O...(MISSING_HYPHEN)
[misspelling] ~10-~10: This expression is normally spelled as one or with a hyphen.
Context: ...en hold complex defi positions that are non trivial to unwind. Even if a Cellar is in the S...(EN_COMPOUNDS_NON_TRIVIAL)
[grammar] ~14-~14: Two determiners in a row. Choose either “the” or “a”.
Context: ...gistry contract is typically managed by the a multisig (Gnosis Safe). The registry is...(DT_DT)
[uncategorized] ~14-~14: A comma might be missing here.
Context: ...r regulates the behavior of the Cellar. Typically adding a new adapter to the cellar requ...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[uncategorized] ~14-~14: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...pprove the change. The Registry makes it so the Cellar will trust the new adapter a...(COMMA_COMPOUND_SENTENCE_2)
[uncategorized] ~14-~14: Possible missing comma found.
Context: ...kes it so the Cellar will trust the new adapter and Sommelier governance makes the new ...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~18-~18: Possible missing preposition found.
Context: ... in the Cellar. if the strategist tends keep funds in adapters that are not directly...(AI_HYDRA_LEO_MISSING_TO)
[uncategorized] ~18-~18: Possible missing comma found.
Context: ...rs that are not directly withdrawalable from then catastrophic pricing exploits are ...(AI_HYDRA_LEO_MISSING_COMMA)
[misspelling] ~18-~18: This word is normally spelled as one.
Context: ...strophic pricing exploits are unlikely. Never the less, the Price Router is responsible for ge...(EN_COMPOUNDS_NEVER_THE_LESS)
[grammar] ~18-~18: Make sure that the noun ‘setup’ is correct. Did you mean the past participle “set up”?
Context: ...urrent deployment of the PriceRouter is setup with a 7 day timelock on changes to dec...(BE_VB_OR_NN)
docs/fees.md
[typographical] ~7-~7: Consider adding a comma after ‘Generally’ for more clarity.
Context: ...adapter the hold funds from the cellar. Generally the strategist moves funds into the res...(RB_LY_COMMA)
[style] ~7-~7: ‘on a regular basis’ might be wordy. Consider a shorter alternative.
Context: ...strategist moves funds into the reserve on a regular basis from profits. The reserve gives the str...(EN_WORDINESS_PREMIUM_ON_A_REGULAR_BASIS)
[uncategorized] ~7-~7: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ... Fees must be collected from the reserve but funds in the reserve can also be return...(COMMA_COMPOUND_SENTENCE_2)
[duplication] ~11-~11: Possible typo: you repeated a word
Context: ... tracking a highwater mark of the share from from each fee collection and then computing ...(ENGLISH_WORD_REPEAT_RULE)
[uncategorized] ~11-~11: Possible missing comma found.
Context: ... are minted when the performance fee is collected and that effectively dilutes the existi...(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~49-~49: “Strategist” is a singular noun. It appears that the verb form is incorrect.
Context: ...## Platform Fees When the strategist withdraw fees from the Cellar, the top level pla...(PCT_SINGULAR_NOUN_PLURAL_VERB_AGREEMENT)
[uncategorized] ~49-~49: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...gist withdraw fees from the Cellar, the top level platform fee takes a cut for the Sommel...(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~49-~49: You might be missing the article “the” here.
Context: ...mmelier chain via the gravity bridge on Ethereum mainnet and the Axelar on other chains....(AI_EN_LECTOR_MISSING_DETERMINER_THE)
docs/cellar_ui.md
[misspelling] ~13-~13: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...rom the sender to the cellar. There is a also 'previewMint' function that allow ...(EN_A_VS_AN)
[uncategorized] ~13-~13: This verb does not appear to agree with the subject. Consider using a different form.
Context: ...e is a also 'previewMint' function that allow a UI to simulate the deposit for the us...(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
[uncategorized] ~19-~19: A comma might be missing here.
Context: ...he deposit. This fee serves a couple of purposes but most importantly it prevents exploi...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[uncategorized] ~19-~19: A comma might be missing here.
Context: ...ee serves a couple of purposes but most importantly it prevents exploiting small price arbt...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[uncategorized] ~21-~21: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...t on a cellar can set all the supported asset withsetAlternativeAssetData
. The var...(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
[style] ~26-~26: As a shorter alternative for ‘able to’, consider using “can only”.
Context: ...t er4626 implentations. These functions are only able to withdraw liquidity from positions where...(BE_ABLE_TO)
[style] ~26-~26: This phrasing could be wordy, so try replacing it with something more concise.
Context: ...ons where the_withdrawableFrom
value is greater than 0. Many adapters do not enable withdraw...(MORE_THAN_EXCEEDS)
[uncategorized] ~26-~26: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...ducing the surface for arbitrage attacks but it requires the strategist to interacti...(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~30-~30: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...eue functionality is provided to enable user to place requests for withdrawl liquidi...(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
[grammar] ~30-~30: The plural noun “users” cannot be used with the article “a”. Did you mean “a user”?
Context: ... and service requests from. The flow is a users who wishes to withdraw with call `maxRe...(A_NNS)
[grammar] ~30-~30: Did you mean “wish”? Alternatively, check whether the singular of ‘users’ should be used.
Context: ... requests from. The flow is a users who wishes to withdraw with callmaxRedeem
if th...(RELATIVE_CLAUSE_AGREEMENT)
[uncategorized] ~30-~30: ‘Amount of’ should usually only be used with uncountable or mass nouns. Consider using “number” if this is not the case.
Context: ...o withdraw with callmaxRedeem
if the amount of shares they want to redeem is greate...(AMOUNTOF_TO_NUMBEROF)
[grammar] ~30-~30: The modal verb ‘will’ requires the verb’s base form.
Context: ...greater thanmaxRedeem
. The user will calledupdateWithdrawRequest
with the vault ...(MD_BASEFORM)
[style] ~30-~30: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...drawRequest` with the vault shares they want to redeem with appropiate metdata. ```sol...(REP_WANT_TO_VB)
docs/using_cellars.md
[uncategorized] ~5-~5: Possible missing comma found.
Context: ... is located heresrc/base/Cellar.sol
. Typiccally a strategists will choose to deploy one...(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~5-~5: The plural noun “strategists” cannot be used with the article “a”. Did you mean “a strategist” or “strategists”?
Context: ... heresrc/base/Cellar.sol
. Typiccally a strategists will choose to deploy one of the permut...(A_NNS)
[grammar] ~12-~12: The plural noun “cellars” cannot be used with the article “a”. Did you mean “a cellar” or “cellars”?
Context: ...oing to go through a mock deployment of a cellars. 1. First we configure the core set of...(A_NNS)
[uncategorized] ~37-~37: Consider adding hyphens to this phrasal adjective.
Context: ...tiAssetDepositWithNativeSupport` is the state of the art cellar that supports some advanced capa...(STATE_OF_THE_ART)
[misspelling] ~42-~42: Use “An” instead of ‘A’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...r Flash Loans for leveraged staking. 2. A erc4626 Price Oracle for pricing the sh...(EN_A_VS_AN)
Markdownlint
docs/permissions.md
3-3: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
1-1: Punctuation: '.'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
docs/fees.md
47-47: null
Headings must start at the beginning of the line(MD023, heading-start-left)
docs/cellar_ui.md
3-3: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
6-6: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines(MD022, blanks-around-headings)
25-25: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines(MD022, blanks-around-headings)
docs/using_cellars.md
115-115: Expected: atx; Actual: atx_closed
Heading style(MD003, heading-style)
7-7: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
24-24: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
25-25: Expected: 1; Actual: 3
Multiple consecutive blank lines(MD012, no-multiple-blanks)
46-46: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
104-104: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
111-111: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
112-112: Expected: 1; Actual: 3
Multiple consecutive blank lines(MD012, no-multiple-blanks)
113-113: Expected: 1; Actual: 4
Multiple consecutive blank lines(MD012, no-multiple-blanks)
114-114: Expected: 1; Actual: 5
Multiple consecutive blank lines(MD012, no-multiple-blanks)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Outside diff range and nitpick comments (2)
README.md (2)
Line range hint
17-17
: Grammar Correction: Verb UsageChange "setup" to "set up" to correctly use it as a verb.
- Before attempting to setup the repo, first make sure you have Foundry installed and updated. + Before attempting to set up the repo, first make sure you have Foundry installed and updated.Tools
Markdownlint
6-6: Expected: h2; Actual: h3
Heading levels should only increment by one level at a time(MD001, heading-increment)
5-5: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
15-15: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
Line range hint
43-43
: Grammar and Article Corrections
- Add the article "the" before "test".
- Correct the verb agreement issue with "want" and "foundry".
- Before running test, rename `sample.env` to `.env`, and add your mainnet RPC. If you want foundry to verify the contracts. + Before running the test, rename `sample.env` to `.env`, and add your mainnet RPC. If you want Foundry to verify the contracts.Tools
Markdownlint
6-6: Expected: h2; Actual: h3
Heading levels should only increment by one level at a time(MD001, heading-increment)
5-5: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
15-15: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- README.md (1 hunks)
- docs/permissions.md (1 hunks)
Additional context used
LanguageTool
README.md
[grammar] ~17-~17: The word “setup” is a noun. The verb is spelled with a white space.
Context: ...Getting Started** Before attempting to setup the repo, first make sure you have Foun...(NOUN_VERB_CONFUSION)
[uncategorized] ~43-~43: You might be missing the article “the” here.
Context: ...txtfile. **Testing** Before running test, rename
sample.envto
.env`, and add...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[grammar] ~43-~43: There is an agreement error between ‘want’ and ‘foundry’. Insert ‘a(n)’ or change the noun to plural.
Context: ...rivate Key, and an Etherscan key(if you want foundry to verify the contracts). Note in order...(PRP_VB_NN)
[style] ~44-~44: Consider a shorter alternative to avoid wordiness.
Context: ... foundry to verify the contracts). Note in order to run tests against forked mainnet, your ...(IN_ORDER_TO_PREMIUM)
docs/permissions.md
[uncategorized] ~14-~14: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...pprove the change. The Registry makes it so the Cellar will trust the new adapter, ...(COMMA_COMPOUND_SENTENCE_2)
[uncategorized] ~18-~18: Possible missing preposition found.
Context: ... in the Cellar. if the strategist tends keep funds in adapters that are not directly...(AI_HYDRA_LEO_MISSING_TO)
[uncategorized] ~18-~18: Possible missing comma found.
Context: ...rs that are not directly withdrawalable from then catastrophic pricing exploits are ...(AI_HYDRA_LEO_MISSING_COMMA)
[misspelling] ~18-~18: This word is normally spelled as one.
Context: ...strophic pricing exploits are unlikely. Never the less, the Price Router is responsible for ge...(EN_COMPOUNDS_NEVER_THE_LESS)
Markdownlint
README.md
6-6: Expected: h2; Actual: h3
Heading levels should only increment by one level at a time(MD001, heading-increment)
5-5: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
15-15: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
19-19: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
41-41: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
docs/permissions.md
3-3: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
1-1: Punctuation: '.'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
Additional comments not posted (2)
README.md (2)
8-11
: Documentation Links UpdatedThe links to the new documentation files have been correctly updated and are appropriately described.
Line range hint
44-44
: Consider a Shorter AlternativeTo avoid wordiness, consider revising the sentence to be more concise.
- Note in order to run tests against forked mainnet, your RPC must be an archive node. + Note, to run tests against forked mainnet, your RPC must be an archive node.Tools
Markdownlint
6-6: Expected: h2; Actual: h3
Heading levels should only increment by one level at a time(MD001, heading-increment)
5-5: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
15-15: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
@@ -0,0 +1,18 @@ | |||
# Permississons in Cellars. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Markdown Formatting Corrections Required
Remove the trailing punctuation in the heading.
- # Permississons in Cellars.
+ # Permississons in Cellars
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
# Permississons in Cellars. | |
# Permississons in Cellars |
Tools
Markdownlint
1-1: Punctuation: '.'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
|
||
## Registry | ||
|
||
The Registry contract is typically managed by a multisig (Gnosis Safe). The registry is a contract that holds the addresses of all the adapters that the Cellar will use. Typically, adding a new adapter to the cellar requires both the multisig on the Registry and Sommelier governance to approve the change. The Registry makes it so the Cellar will trust the new adapter, and Sommelier governance makes the new adapter callable by the strategist. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Grammar Correction: Comma Usage
Use a comma before 'so' to correctly separate independent clauses.
- The Registry makes it so the Cellar will trust the new adapter and Sommelier governance makes the new adapter callable by the strategist.
+ The Registry makes it so the Cellar will trust the new adapter, and Sommelier governance makes the new adapter callable by the strategist.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
The Registry contract is typically managed by a multisig (Gnosis Safe). The registry is a contract that holds the addresses of all the adapters that the Cellar will use. Typically, adding a new adapter to the cellar requires both the multisig on the Registry and Sommelier governance to approve the change. The Registry makes it so the Cellar will trust the new adapter, and Sommelier governance makes the new adapter callable by the strategist. | |
The Registry contract is typically managed by a multisig (Gnosis Safe). The registry is a contract that holds the addresses of all the adapters that the Cellar will use. Typically, adding a new adapter to the cellar requires both the multisig on the Registry and Sommelier governance to approve the change. The Registry makes it so the Cellar will trust the new adapter, and Sommelier governance makes the new adapter callable by the strategist. |
Tools
LanguageTool
[uncategorized] ~14-~14: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...pprove the change. The Registry makes it so the Cellar will trust the new adapter, ...(COMMA_COMPOUND_SENTENCE_2)
Summary by CodeRabbit
Documentation
New Features