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

feat(cli,world): register system ABI onchain #3050

Merged
merged 26 commits into from
Aug 27, 2024
Merged

Conversation

holic
Copy link
Member

@holic holic commented Aug 20, 2024

  • fix ABI race condition in world (world uses build script to generate ABIs but that same build script needs ABIs now)
    • maybe easiest to fix my moving to parsing AST instead of ABI
  • register ABIs in metadata table during deploy
  • add .mud to .gitignore in templates, etc.
  • add namespaceLabel to config's system output

Copy link

changeset-bot bot commented Aug 20, 2024

🦋 Changeset detected

Latest commit: 5ebb331

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 26 packages
Name Type
@latticexyz/cli Patch
@latticexyz/world-modules Patch
mock-game-contracts Patch
@latticexyz/abi-ts Patch
@latticexyz/block-logs-stream Patch
@latticexyz/common Patch
@latticexyz/config Patch
create-mud Patch
@latticexyz/dev-tools Patch
@latticexyz/explorer Patch
@latticexyz/faucet Patch
@latticexyz/gas-report Patch
@latticexyz/protocol-parser Patch
@latticexyz/query Patch
@latticexyz/react Patch
@latticexyz/recs Patch
@latticexyz/schema-type Patch
solhint-config-mud Patch
solhint-plugin-mud Patch
@latticexyz/store-indexer Patch
@latticexyz/store-sync Patch
@latticexyz/store Patch
@latticexyz/utils Patch
@latticexyz/world-module-metadata Patch
@latticexyz/world Patch
ts-benchmarks Patch

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

@holic
Copy link
Member Author

holic commented Aug 21, 2024

okay so after two attempts, I understand this a bit better

we currently have a constraint/order of operations

  • parse solidity AST to codegen system interfaces + world interface
  • import/use those codegen interfaces
  • compile
  • register ABI from compile artifact

I tried to simplify by just

  • compile
  • register ABI from compile artifact
  • (eventually codegen interface from ABI)

but we lean on just ABI for a number of reasons:

  • source won't compile if it references codegen interfaces that don't exist yet (chicken and egg problem)
  • ABI is a subset of interface and is missing things like param modifiers (string memory vs string calldata, address vs address payable)

so then I went the other way

  • parse solidity AST to codegen ABI
  • register ABI
  • (eventually codegen interface from ABI)

but that is also fraught because just parsing solidity skips parts of the compiler we need, like resolving imports with remappings, resolving underlying types of enums/user types, converting structs to tuples

so we can't actually create pure/true representation of the ABI and reimplementing all that in the "parse solidity" step seems like a bad idea

all this to figure out where the right "entry point" is for generating the local system manifest and I think it's in mud build after forge build

to recap:

  • we need to keep our order-of-operations (parse source, codegen interfaces, compile, extract ABI)
  • generate system manifest from ABI, after forge build but before abi-ts (in the future we may want these JSON manifests to be strongly typed)

there's still the problem of the ABI not including enough data for a 1:1 interface (e.g. memory vs calldata) but maybe we can just make some assumptions for now and fill that in later when we have some time to combine the ABI with AST (both available in forge output, which is nice, but still a pain to unwind)

@holic holic marked this pull request as ready for review August 27, 2024 15:37
@holic holic requested a review from alvrs as a code owner August 27, 2024 15:37
Copy link
Member

@alvrs alvrs left a comment

Choose a reason for hiding this comment

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

🔥🔥🔥

@holic holic merged commit 31caecc into main Aug 27, 2024
14 checks passed
@holic holic deleted the holic/worldgen-system-abis branch August 27, 2024 16:00
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