Skip to content

Refactor / Organize Scripts #74

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

baxelson12
Copy link

I'm not normally one to drop large rewrites, but I had a bit of a vision for this. It may be somewhat opinionated, feel free to close if it's too extensive.

Using the scripts within the template, I'm finding myself adding a lot of code to BaseScript to make things work and it gets bloated and confusing pretty quick. What I'm suggesting in this PR is to break things up into much smaller, logical bits, which can then be combined in any manner in an "orchestrator" script to meet whatever needs a consumer may have. I feel this provides the tools to build off of this template much more easily, and it feels much less "locked in". Instead of having to modify the code we provide in the template, it enables consumers to extend it.

As an additional point, this runs much more easily on a local anvil test network, and examples run out-of-the-box with no configuration (I couldn't get anvil working with the current scripts).

What's changed

  • Remove base/
  • Create actions/ which contains libraries of the same basic functionalities (get dependent contract addresses, create mock tokens, mine / deploy hooks contract, create pool and add liquidity OR create pool only OR add liquidity only, perform swap.
  • Create examples/ which contains "orchestrator" scripts showing how to make the building blocks work together for the basic tasks a consumer may need to perform.

By doing this, a consumer can much more easily drop a new file like script/MyScript.s.sol and simply import the pieces they need to test or launch their project.

For me, personally, I did this because I use this repo for a lot of scripting done in ts or rust, and it enables me to do so without having to change as many hardcoded values. I figured if I'm doing it, there's at least one other person doing the same so maybe it'd be useful.

Let me know if you have questions or feedback. Thanks!

@akshatmittal
Copy link
Contributor

I wanted to share a little bit about why we decided to way the way it is:

  1. Anything in this repo is things that developers can and should modify to their needs. They are intended to be starting points not copy-paste stuff. Why? Simply because we want developers to have a good starting set, but allow them to do things their own way. If you abstract too much and only give them high level functions, they will either never fully understand what's happening or will not be able to customize it to their desire.
  2. We do have some abstractions, but these are things that are outside of the protocol purview, fully generalized and tackle a very specific thing. These abstractions live in either v4-periphery and (more to come) Hookmate.
  3. The things are structured in a way to avoid common pitfalls. A good example: Creating a pool and providing liquidity for it should always happen in the same transaction, otherwise an adversary can frontrun the liquidity addition and move the price before that happens. If you abstract and split everything, this becomes much less clear.

I'm open to ideas if you have any, for utilities to be included in one place or another. For example, will push the CurrencyTools library soon which combines the functionality of your TokenUtils and TokenApprover plus a few other things.

@baxelson12
Copy link
Author

baxelson12 commented Jun 26, 2025

Hey @akshatmittal thanks for the insight. Like I said feel free to close if it does not fit the spec internally, I simply shared it because I spent some time rewriting it for our own needs and it felt much better using so maybe there could at minimum be some ideas provided for future iterations. Through your rewrite, things definitely are moving in a better direction and I know a lot of work was put into it. In response to the points provided though, I do have some counter points:

  1. Offering a well-structured abstraction doesn’t necessarily prevent learning or customization. Instead, it can help onboard users more quickly by reducing the cognitive load upfront, while still giving them the option to dive deeper if they choose. What needs to be considered, too, is not every developer using this template is looking to learn or build from scratch. Some may be experienced and simply want a base they can trust and build upon.

2 is great, let me know if any help is needed in the process!

  1. I can see the intent on this one, but I also can see this being documented in the readme as opposed to locking all consumers in, what about the people who are working on pre-existing pools or far more complex systems? It needs to be more general and cover more scenarios. LP management has been my primary source of income for many years now and have several automated systems doing so. While it is really awesome that those two steps can be performed in a single transaction now, if there truly were a frontrun risk doing those steps separately I would never be able to accurately add liquidity to a pre-existing pool (and would probably be homeless lol). The real risk lies in the pool initialization itself (e.g malicious actor setting their own price), which is not feasibly avoidable. The best way the attack vector could be minimized in a new-launch scenario is through scripting. I suppose the roundabout point I'm trying to make here is that scenarios other than initializing everything from scratch should also be taken into consideration.

As this repo grows, please take into consideration how other widely-used templates in other fields are used. For example create-next-app or express-generator. While obviously much more intricate and not in the same field, they provide the tools needed to build, and a functioning example out of the box, it is up to the consumer on how they build upon what it provides - we just need to give them all of the tools we reasonably can to do so.

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.

2 participants