Skip to content
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

Docs CRs #17

Closed
steve0xp opened this issue Mar 25, 2024 · 10 comments
Closed

Docs CRs #17

steve0xp opened this issue Mar 25, 2024 · 10 comments
Assignees
Labels
enhancement New feature or request

Comments

@steve0xp
Copy link
Contributor

steve0xp commented Mar 25, 2024

Scope

This issue encompasses possible docs change requests (CRs). It is a live issue. It is also a referencable note to help create said CRs to the docs when we have time. If the ideas here are worthwhile, we will open issues and respective PR(s) to make the changes in the docs.

Ref: BalancerV3 Docs

@steve0xp steve0xp added the enhancement New feature or request label Mar 25, 2024
@steve0xp steve0xp self-assigned this Mar 25, 2024
@steve0xp
Copy link
Contributor Author

  1. The smart contract example within Create a custom AMM with a novel invariant page needs to be corrected.
  2. Additional context on calling IVault(vault).registerPool() and IVault(vault).initialize() would be nice. It would go hand in hand with the explanation on the proxy architecture.
  3. Proxy architecture with diagrams outlining how things work would be nice too (just explaining the inheritance visually is easier I find versus tracking things down in the code itself). Once one tracks it in the code itself, it is easy to forget too :O

@steve0xp steve0xp mentioned this issue Apr 19, 2024
@steve0xp
Copy link
Contributor Author

Testing Sections

IMO, it would be cool to see a testing section in the docs outlining how Balancer provides a test suite that a developer has to ensure works with their pool (custom or not). This is similar to how Yearn strategies are tested against the YearnV2 architecture.

Questions:

  1. Is this of interest to Balancer? Does Balancer already provide this kind of test suite? If not, how difficult is it to make the a custom pool test file that would cover what is necessary on this?

@steve0xp
Copy link
Contributor Author

Custom Pool Factory

  • Use of custom factory for pool deployments, and aspects related to it.
    • Key Gotchas include: Sequence of things: creating your pool, then your factory that inheritsBasePoolFactory, then deployment.
    • Possible nice to haves: test suite ensuring custom pool factory fits key testing coverage in Balancer architecture (if applicable).

@johngrantuk
Copy link
Member

  • Smart contract example corrections: Yes please :) PRs correcting any mistakes are very welcome.
  • Additional context on register/initialize: If you found this tricky to understand I imagine others would too so please feel free to open PRs with suggestions on how to improve. Worth covering in call too.
  • Proxy architecture diagrams: Not quite clear what this covers, can discuss in call.
  • Testing section: Interesting. Would definitely be worth discussing further. Can you point to some good examples please? Also how would this differ to what we're trying to achieve with scaffold-balancer with testing templates/examples?
  • Pool factory: I imagine we should just add some docs about this as its not covered at all?

@MattPereira
Copy link
Member

MattPereira commented Apr 23, 2024

TokenConfig Notes

@MattPereira
Copy link
Member

MattPereira commented Apr 28, 2024

On the "Create a custom AMM with a novel invariant" page of docs, in the "Deploy your pool" section, the third step says "Call vault.intialize with a link leading to IVaultExtension.sol code.
https://docs-v3.balancer.fi/build-a-custom-amm/build-an-amm/create-custom-amm-with-novel-invariant.html#deploy-your-pool

This is confusing because the only way to initialize a pool is through the router so maybe referencing router.initialize would make more sense?

@steve0xp
Copy link
Contributor Author

steve0xp commented May 2, 2024

Additional Questions / Areas that May Need Details in the Docs (if not already in updated versions):

  • If pools are permissionless, then if I created a random pool contract, what would it have to do to be able to be registered with the vault (aside from the tokens having proper rateProvider info perhaps (which may not be needed actually)?)
  • How does building off of the baseFactory help Balancer subgraph pick up necessary info from children pools?

TokenConfig - more info on it as requested before.

  • Can we have clarity on the token config reversion scenarios? This LoCmakes me wonder, and we troubleshot scenarios using the same tokens and different configs and it didn't revert (though we can test more extensively).
    • For further context: John Grants had stated the following: "...I believe there is a check at Vault level that makes sure you cannot register a Pool with DAI/DAI or BAL/BAL."
  • Perhaps point to appropriate smart contract files in the monorepo to help elaborate (since there is conditional logic and probably natspec for it already): VaultExtension.sol and function registerPool()

@t-phoenix
Copy link

t-phoenix commented Aug 22, 2024

Deployment steps for Core System Contract

Scaffold-balancer-v3 is seamless in providing playground dev experience with balancer v3 pools, but it lacks in providing steps to replicate the necessary deployment process for core system contracts like vault, Router, permit2, extensions, etc.

Specially with vaultFactory overexceeding smart-contract size limit, it becomes really difficult to deploy core smart-contracts.

@MattPereira
Copy link
Member

Hey @t-phoenix
The scope for scaffold is to focus on helping devs create v3 pools and hooks

Not sure what plan is for supporting deployment of v3 core contracts, but feel free to ask on our discord dev support channel

@MattPereira
Copy link
Member

Cleaning up outdated issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants