Skip to content

feat(cli): add initialization libs to deploy#361

Merged
alvrs merged 1 commit into
latticexyz:mainfrom
dk1a:dk1a/deploy-init-separate
Feb 2, 2023
Merged

feat(cli): add initialization libs to deploy#361
alvrs merged 1 commit into
latticexyz:mainfrom
dk1a:dk1a/deploy-init-separate

Conversation

@dk1a

@dk1a dk1a commented Jan 31, 2023

Copy link
Copy Markdown
Contributor

deploy.json already has "init" for systems.
But systems are contracts. Scripts are best run through libraries.

Example:
An expensive addItem function, which adds a game item prototype with all its properties. Say it costs 1mil gas per item. I have an initializer which adds 100 of those. 100mil gas exceeds the limit.

  • If the 100 adds are in a system contract, it will revert. And making a bunch of wrapper init systems is a really bad idea.
  • If the 100 adds are in a library, forge script can at least split it into several transactions. But also the real contract calls (component sets) use only a small portion of all that gas.

(this is a simplified edge case, my real use case is unfinished and more convoluted, but similar enough, e.g. InitEquipmentAffixSystem uses way more than the transaction gas limit)

I tested that it minimally works via a slightly modified emojimon, if you wanna just clone it.

Some concerns:

@dk1a dk1a requested a review from alvrs as a code owner January 31, 2023 20:27
@frolic

frolic commented Jan 31, 2023

Copy link
Copy Markdown
Member

Love this idea.

I wonder if it's worth adding a scripts directory (and follow the foundry *.s.sol naming scheme) to keep the initialization scripts/logic separate from the long-running logic (libs).

@dk1a

dk1a commented Feb 1, 2023

Copy link
Copy Markdown
Contributor Author

I wonder if it's worth adding a scripts directory (and follow the foundry *.s.sol naming scheme) to keep the initialization scripts/logic separate from the long-running logic (libs).

scripts is kinda like src, test, out, it should exist in the root (according to the defaults of foundry book, and my preference).
Something like Deploy.sol is a script (#311 is related), and should probably be Deploy.s.sol.

But I would prefer libraries that are used by scripts to still be inside src. They can be however kept in a separate dir from the other libs. This is mostly because #309 needs libs in src, but also allows them to be in arbitrary subfolders (for example src/init for init libs).

@dk1a dk1a force-pushed the dk1a/deploy-init-separate branch from faa8a33 to 32c00b0 Compare February 1, 2023 12:56

@alvrs alvrs left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Great addition!

@alvrs alvrs merged commit 3999ca0 into latticexyz:main Feb 2, 2023
@frolic

frolic commented Feb 2, 2023

Copy link
Copy Markdown
Member

Just so I understand, this is meant to replace the current pattern of InitSystems, right?

Can we add docs for this? Maybe here: https://mud.dev/guides/getting_started/#deployment

@dk1a

dk1a commented Feb 2, 2023

Copy link
Copy Markdown
Contributor Author

Just so I understand, this is meant to replace the current pattern of InitSystems, right?

Yep

Can we add docs for this? Maybe here: https://mud.dev/guides/getting_started/#deployment

I'll make a PR

@dk1a dk1a deleted the dk1a/deploy-init-separate branch February 5, 2023 10:28
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