dojo icon indicating copy to clipboard operation
dojo copied to clipboard

Migrate plan init_calldata validation

Open rsodre opened this issue 1 year ago • 1 comments
trafficstars

Describe the bug

sozo migrate plan does not validate init_calldata arguments count

To Reproduce

  • Remove one argument from a contract's init_calldata it's overlay file
  • Run sozo migrate plan, no errors will appear
  • Run sozo migrate apply, it will fail
# 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

  • Any error that can be detected on apply should be detected on plan.
  • Resuming a valid migration should always succeed

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
]

rsodre avatar Jul 26 '24 16:07 rsodre

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.

itzlambda avatar Jul 28 '24 05:07 itzlambda

We can't validate anything about the calldata regarding the cairo serialization since we can't first simulate the transaction for the contract registration. The registration will deploy it, the init is called after.

Doing a dry run on katana is the best way to test the init functions.

Adding the support for the prefixes will already help to avoid serializing manually. But for more complex functions, the best if to try it locally on katana. 👍

glihm avatar Nov 03 '24 04:11 glihm