Support for legacy world contracts in migration + support for pre-manifest migration

[wraitii]

Describe the bug
Still experiencing some issues with the briq contract migration from a Dojo v0.3 to Dojo v0.7

Namely:

1. the ModelRegistered struct was updated with new fields and the deserialisation of my events fails.
2. The executor argument to the constructor was removed, but I have it in my deployment TX, thus the generated world address differs from the actual world address (my TX is https://starkscan.co/contract/0x059f31686991d7cac25a7d4844225b9647c89e3e1e2d03460dbc61e3fbfafc59#overview)
3. The original_class_hash is set by default to the local class hash. This should probably be the remote class hash, if there is one, but it is furthermore broken: I have actually upgraded the world, so I would need to manually specify it.
4. the base class hash suffers from the same problem, and thus is also broken.
5. Because the contracts are unknown, they also have no registered base class, and so they default to the new base class, which is incorrect, so they are deployed to new addresses instead of upgrading in-place.
6. The contract salt is incorrect: it uses the fully qualified name when my contracts used the short-name originally.
7. The model comparison logic relies on conversion from snake_case (local models) to Pascal-case, for remote events. However, this fails with e.g. a model named ERC1155Balance, the snake-case is erc_1155_balance which gets converted to Erc1155Balance, and so the comparison fails. I think the comparison should be done in lowercase.

Points (1), (2), (6), (7) are going to remain problematic going forward, the rest should be fixed once I have a manifest set-up locally. It would be convenient to be able to "pull" the manifest first though.

To Reproduce

• Compile the briq repo @ branch dojo_update_07 (updated to 0.7)
• call sozo migrate plan --world 0x1ea16366a82e211a9b9045725309a5080c0260d5caf45c58836fc61b42501f5 --name briq_protocol
• Note the problems

Suggestion of fixes
Point (1): Should probably handle both in-code somehow. Dojo doesn't actually use addresses for now.
Point (2): I think there's no good solution, IMO just add an explicit executor argument to the manifest, and if present use it - this is legacy support. Alternatively, "trust" the user world-address.
Points (3/4/5): not sure.
Point (6): Needs a parameter of some kind (at some point we discussed use-short-name?) or a parameter in the manifest of some kind.
Point (7): I suggest adding to_lowercase to WorldDiff L48 (model comparison)

[wraitii]

I have a very dirty commit with the changes I've done to make it appear to work here: https://github.com/wraitii/dojo/tree/briq_upgrade_07_test

[lambda-0x]

Point 1: is this for sozo events subcommand?
Point 3:

The original_class_hash is set by default to the local class hash.

by default this is expected behaviour, user need to override the original_class_hash using overlays since there isn't a generalized way to figure this out automatically.

Point 5: similar to point 3

It would be convenient to be able to "pull" the manifest first though.

we have opened an issue (#1921) for this but since the class_hash used during the first deployment isn't stored anywhere i am not sure how to fetch it automatically

Point 7: we noticed this before #1820 (comment) and it should be solved with #1629. But as a quick fix we can also compare them in lowercase, wdyt @glihm ?

[wraitii]

Point 1: is this for sozo events subcommand?

No, solo migrate recreates the remote manifest using events.

Point 3:

The original_class_hash is set by default to the local class hash.
by default this is expected behaviour, user need to override the original_class_hash using overlays since there isn't a generalized way to figure this out automatically.

Will look into that, is there documentation already ?

It would be convenient to be able to "pull" the manifest first though.
we have opened an issue (#1921) for this but since the class_hash used during the first deployment isn't stored anywhere i am not sure how to fetch it automatically

I'm guessing events as well then, unless you plan on changing that?

[lambda-0x]

Will look into that, is there documentation already ?

we recently merged this: dojoengine/book#264, let us know if its not clear or you have any other questions.

I'm guessing events as well then, unless you plan on changing that?

yeah, we where thinking to add that info in the world contract itself but its still understand discussion

[glihm]

With rc we are not able anymore to support legacy contract of those versions. Too much has changed in the contract and having backward compatibility would have been too much compromise on the new features.

Currently the best migration possible is migrating data with specific systems.

In the new world, the migrations will break less easily because:

1. No more base contract, which avoid having to keep a mapping for addresses computation.
2. Sozo no more requires local state to be saved to be consistent. Everything is available on-chain and can be reconstructed anytime without any local prior knowledge.