-
Notifications
You must be signed in to change notification settings - Fork 184
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
feat(cli): generate system ABI's from World address #2849
Conversation
🦋 Changeset detectedLatest commit: a142476 The changes in this PR will be included in the next version bump. This PR includes changesets to release 23 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
packages/cli/src/commands/index.ts
Outdated
@@ -14,6 +14,7 @@ import test from "./test"; | |||
import trace from "./trace"; | |||
import devContracts from "./dev-contracts"; | |||
import verify from "./verify"; | |||
import abigen from "./abigen"; |
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.
not a huge fan of continuing the tablegen, worldgen, etc. naming pattern
I could see us eventually combining all of these features under a codegen
command, maybe with sub commands like codegen tables
), but in the meantime, something like "codegen abi" or "generate abi" for the CLI command and "world to abi" for the helper methods?
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.
hmm I like this line of thinking, lemme marinate on it
It's interesting that abigen
and worldgen
have different inputs but basically the same output
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.
The command is now generate-abi
and the helper is just systemToAbi
await fs.writeFile(fullOutputPath, JSON.stringify(abi)); | ||
|
||
// prepare IWorld ABI | ||
worldAbi.push(...abi); |
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.
is this accurate?
systems will have a function name like function increment()
but world may have it as function app__increment()
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.
wowww good point! i think we will have to prepare the world ABI seperately
}, | ||
}; | ||
|
||
export async function generateAbiHandler(opts: Options) { |
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.
thoughts on putting this into a separate dir like packages/cli/src/init
or something to prepare for more of these types of files that can eventually be combined into one or more CLI commands?
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.
What do you mean by "combining" the CLI commands? Like just putting some of this logic in a helper function? I imagine if we have one command that combines the logic of other commands, they all still live in the commands
directory but share helpers
import type { CommandModule, InferredOptionTypes } from "yargs"; | ||
import { Hex, createWalletClient, http } from "viem"; | ||
import { getSystems } from "../deploy/getSystems"; | ||
import { getWorldDeploy } from "../deploy/getWorldDeploy"; |
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.
we should consider "lifting" these out of deploy
dir if we're sharing them across different commands/utils
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.
Yup, and I think having separate directories for each command makes less sense with these new tools. We'll be reusing everything from deploy
except for the ensureX
functions that actually deploy the contracts
we're now writing these to the chain via #3050 |
Adds a new MUD
generate-abi
command that generates an ABI for a given World by reading the onchain system registrationsfixes #2845