dojoengine / dojo

Dojo is a toolchain for building provable games and applications
https://dojoengine.org
Apache License 2.0
414 stars 168 forks source link

[BUG] sozo apply erasing manifest contract addresses #1820

Closed rsodre closed 6 months ago

rsodre commented 6 months ago

Describe the bug Runing sozo apply twice replaces the manifest addresses to null

To Reproduce

Using Dojo 0.6.0

$ sozo --version
sozo 0.6.0
scarb: 2.5.4
cairo: 2.5.4
sierra: 1.4.0

$ katana --disable-fee
$ sozo clean
$ sozo build
$ sozo -P dev migrate apply --world 0x10923ed11da790f94efd6f68c5558f5f604df0ff2ff776bd5209b7ebb8985b8
$ grep '"address"' manifests/dev/manifest.json
$ sozo -P dev migrate apply --world 0x10923ed11da790f94efd6f68c5558f5f604df0ff2ff776bd5209b7ebb8985b8
$ grep '"address"' manifests/dev/manifest.json

Expected behavior if sozo migrate apply is updated, it should not write to manifests

Screenshots

Console outputs running from this repo: https://github.com/funDAOmental/pistols/tree/wallet

Look at the greps from manifest.json

Terminal 1:

$ cd dojo
$ ./run_katana

Terminal 2:

$ sozo --version
sozo 0.6.0
scarb: 2.5.4
cairo: 2.5.4
sierra: 1.4.0

$ cd dojo
$ sozo clean
$ sozo build --typescript
Model token::components::introspection::src5::src_5_model not found in target.
Model token::components::token::erc20::erc20_bridgeable::erc_20_bridgeable_model not found in target.
Model token::erc20::models::erc_20_balance not found in target.
Model token::erc20::models::erc_20_allowance not found in target.
Model token::erc20::models::erc_20_meta not found in target.
Model token::erc721::models::erc_721_meta not found in target.
Model token::erc721::models::erc_721_operator_approval not found in target.
Model token::erc721::models::erc_721_owner not found in target.
Model token::erc721::models::erc_721_balance not found in target.
Model token::erc721::models::erc_721_token_approval not found in target.
Model token::erc1155::models::erc_1155_meta not found in target.
Model token::erc1155::models::erc_1155_operator_approval not found in target.
Model token::erc1155::models::erc_1155_balance not found in target.

$ sozo -P dev migrate apply --world 0x10923ed11da790f94efd6f68c5558f5f604df0ff2ff776bd5209b7ebb8985b8

Migration account: 0xb3ff441a68610b30fd5e2abbf3a1548eb6ba6f3559f2862bf2dc757e5828ca

World name: pistols

Chain ID: KATANA

[1] 🌎 Building World state....
  > No remote World found
[2] 🧰 Evaluating Worlds diff....
  > Total diffs found: 15
[3] 📦 Preparing for migration....
  > Total items to be migrated (15): New 15 Update 0

# Base Contract
^C
$ sozo -P dev migrate apply --world 0x10923ed11da790f94efd6f68c5558f5f604df0ff2ff776bd5209b7ebb8985b8

Migration account: 0xb3ff441a68610b30fd5e2abbf3a1548eb6ba6f3559f2862bf2dc757e5828ca

World name: pistols

Chain ID: KATANA

[1] 🌎 Building World state....
  > No remote World found
[2] 🧰 Evaluating Worlds diff....
  > Total diffs found: 15
[3] 📦 Preparing for migration....
  > Total items to be migrated (15): New 15 Update 0

# Base Contract
  > Class Hash: 0x679177a2cb757694ac4f326d01052ff0963eac0bc2a17116a2b87badcdf6f76
# World
  > Contract address: 0x10923ed11da790f94efd6f68c5558f5f604df0ff2ff776bd5209b7ebb8985b8
# Models (11)
pistols::models::coins::coin
  > Class hash: 0x733ce322dd5d77fdb68ab997891a0a46834003fb3e5f12052f8f6227083ff72
pistols::models::config::config
  > Class hash: 0x3599fa3e07e926a11894221ee80929455bb55687029cc530a0ee40c098ffc1f
pistols::models::models::challenge
  > Class hash: 0x40a8c89324fd3f2eaf33f0484e0b29a99dc8791ca73e47d111ec72e386b2c53
pistols::models::models::duelist
  > Class hash: 0x59e9c67b9f7ce3c6b9af6cecb76a29ab88d7a292aeea33171cd6573d8562179
pistols::models::models::pact
  > Class hash: 0x3860eb2dfca07d6cffd069ed8a12937f69830c89ca410a6798d0643b940ede3
pistols::models::models::round
  > Class hash: 0x2da5c69c0fb27bea4d38b1a0cb8c3427d81f7bed15415842dda19c6eba4df35
pistols::models::models::wager
  > Class hash: 0x50668cca4fde9d2bc5a65daca7bf51ff6e38159be49541322b06dfd4d479dce
token::components::security::initializable::initializable_model
  > Class hash: 0x203cf018080b21fc921033ee35af2e1b8811cc7c73e9256fb7a86f794c556ce
token::components::token::erc20::erc20_allowance::erc_20_allowance_model
  > Class hash: 0x194d40513d3428318f01b1cd74380d3dc67e361a3e6f31c2c1297b1099fb9a5
token::components::token::erc20::erc20_balance::erc_20_balance_model
  > Class hash: 0x7b1ae7ba3115fabcc3a01e0046ec79bbcea285822ae3bde8501c499dcfb944f
token::components::token::erc20::erc20_metadata::erc_20_metadata_model
  > Class hash: 0x554661c0f3ba0abb585de13ad65cd95e60feee212f65e0f3d26f3c8b06d5058
All models are registered at: 0x53b8462d761f8223dc4c98056d8fe2922708b557c2c3dd34347feef80f30922
# Contracts (3)
pistols::mocks::lords_mock::lords_mock
  > Contract address: 0x775f949fd0aee5d034c3f1f1a59de273732e1b3b067b75c430d4ce33b153148
pistols::systems::actions::actions
  > Contract address: 0x702e255a3446fce9c49df1c579a62fba9e6bfefd4621a437b28c559b68e0f81
pistols::systems::admin::admin
  > Contract address: 0x281b9cb20b07e7aacbb9fcada94bd350a7b9d877755fdf08af4e0f3f2fe3bf2

🎉 Successfully migrated World on block #3 at address 0x10923ed11da790f94efd6f68c5558f5f604df0ff2ff776bd5209b7ebb8985b8

✨ Updating manifests...

✨ Done.

$ grep '"address"' manifests/dev/manifest.json
    "address": "0x10923ed11da790f94efd6f68c5558f5f604df0ff2ff776bd5209b7ebb8985b8",
      "address": "0x775f949fd0aee5d034c3f1f1a59de273732e1b3b067b75c430d4ce33b153148",
      "address": "0x702e255a3446fce9c49df1c579a62fba9e6bfefd4621a437b28c559b68e0f81",
      "address": "0x281b9cb20b07e7aacbb9fcada94bd350a7b9d877755fdf08af4e0f3f2fe3bf2",

$ sozo -P dev migrate apply --world 0x10923ed11da790f94efd6f68c5558f5f604df0ff2ff776bd5209b7ebb8985b8

Migration account: 0xb3ff441a68610b30fd5e2abbf3a1548eb6ba6f3559f2862bf2dc757e5828ca

World name: pistols

Chain ID: KATANA

[1] 🌎 Building World state....
  > Found remote World: 0x10923ed11da790f94efd6f68c5558f5f604df0ff2ff776bd5209b7ebb8985b8
[2] 🧰 Evaluating Worlds diff....
  > Total diffs found: 3
[3] 📦 Preparing for migration....
  > Total items to be migrated (3): New 3 Update 0

# Models (3)
token::components::token::erc20::erc20_allowance::erc_20_allowance_model
  > Already declared: 0x194d40513d3428318f01b1cd74380d3dc67e361a3e6f31c2c1297b1099fb9a5
token::components::token::erc20::erc20_balance::erc_20_balance_model
  > Already declared: 0x7b1ae7ba3115fabcc3a01e0046ec79bbcea285822ae3bde8501c499dcfb944f
token::components::token::erc20::erc20_metadata::erc_20_metadata_model
  > Already declared: 0x554661c0f3ba0abb585de13ad65cd95e60feee212f65e0f3d26f3c8b06d5058
All models are registered at: 0x747150d49f47dfc89c5341d292849af32f1c38206e60e37d94467dfaf144b67

🎉 Successfully migrated World at address 0x10923ed11da790f94efd6f68c5558f5f604df0ff2ff776bd5209b7ebb8985b8

✨ Updating manifests...

✨ Done.

$ grep '"address"' manifests/dev/manifest.json
    "address": "0x10923ed11da790f94efd6f68c5558f5f604df0ff2ff776bd5209b7ebb8985b8",
      "address": null,
      "address": null,
      "address": null,

$ grep 0x702e255a3446fce9c49df1c579a62fba9e6bfefd4621a437b28c559b68e0f81 manifests/dev/manifest.json
(nothing here, this is one of my deployed systems)
fabrobles92 commented 6 months ago

I've been into sozo lately, can i take this one?

lambda-0x commented 6 months ago

@rsodre thanks for the detailed report.

@fabrobles92 thanks you for interest in this issue, but i think this is related to fix done in #1790. i will confirm that and let you know.

lambda-0x commented 6 months ago

this is caused due to two issues:

but onchain they are in this format:

image

this is how sozo handles case conversion: https://github.com/dojoengine/dojo/blob/e9fab2caf970621ed8311330f3752dc9635f60d7/crates/dojo-world/src/migration/world.rs#L36-L46

@glihm how should we fix this?

glihm commented 6 months ago

@glihm how should we fix this?

We should wait the rework that will use the model's selector instead of their name actually. To directly take the name without modification and having it's selector.

Oh ok, that's why I couldn't reproduce on the spawn-and-move example without additional contracts.

lambda-0x commented 6 months ago

with merge of #1822 this issue is fixed.

for the second issue i mentioned of sozo unnecessarily trying to migrate few contracts @glihm should I create a new issue to track it? or if there is already an issue for the rework you mentioned i can comment this there.

lambda-0x commented 6 months ago

the second issue should be fixed with changes mentioned in #1629