dojoengine / dojo

Dojo is a toolchain for building provable games and autonomous worlds with Cairo
https://dojoengine.org
Apache License 2.0
404 stars 162 forks source link

Migrate plan init_calldata validation #2218

Open rsodre opened 1 month ago

rsodre commented 1 month ago

Describe the bug

sozo migrate plan does not validate init_calldata arguments count

To Reproduce

# sozo log
pistols-minter
error: Failed to migrate pistols-minter: TransactionExecutionError. Please also verify init calldata is valid, if any.

# katana log
Execution failed. Failure reason: 0x4661696c656420746f20646573657269616c697a6520706172616d202334 ('Failed to deserialize param #4').

Expected behavior

Additional context

This is a valid config from my contract minter.toml:

init_calldata = [
  "$contract_address:pistols-token_duelist", # duelist token
  "10000",  # max supply
  "100",    # max per wallet
  "1",      # is_open
]

Changing to this will not raise a plan error but will fail on apply:

init_calldata = [
#  "$contract_address:pistols-token_duelist", # duelist token
  "10000",  # max supply
  "100",    # max per wallet
  "1",      # is_open
]

On the other hand, if I change the first argument to something invalid, plan will understand and raise an error:

init_calldata = [
  "$contract_address:::::::::::::pistols-token_duelist", # duelist token
  "10000",  # max supply
  "100",    # max per wallet
  "1",      # is_open
]
lambda-0x commented 1 month ago

atm in sozo migrate plan we just generate the manifest files and print the addresses that everything will have (since they can be deterministically calculated).

we don't simulate any of the transaction to actually verify the data.

it might be possible to detect obvious bad syntax as you mentioned. but i am not sure it would be possible to detect all kinds of error that could be possible during actual deployment. i.e. things like "$contract_address:::::::::::::pistols-token_duelist", # duelist token

we can try to simulating a tranction which does the whole deployment in a single transaction but i am not sure about how good the error message from that would be, and even that would require significant refactor to achieve.