dojo icon indicating copy to clipboard operation
dojo copied to clipboard

[Katana] Allow choosing the class in the genesis file without affecting the actual class hash

Open kariy opened this issue 1 year ago • 9 comments

An improvement to how one can specify a class to be used by a contract in the genesis file as suggested in #1483.

Currently, when writing the genesis file, you can specify the allocated accounts/fee token/udc to use a user-defined class instead of the default class provided by Katana. This is done by specifying exactly the class hash of the class you want to use in the class field of the contract entry.

Example of a genesis file, where the fee token contract is set to use a custom contract class.

{
  ..
 
  "feeToken": {
 "address": "0x55",
 "name": "ETHER",
 "symbol": "ETH",
 "decimals": 18,
        // This field is optional, if not provided the default class will be used.
        // If specified, the value must be one the class hash defined in the `classes` field below. Otherwise, an error will be returned.
 "class": "0xdeadbeef"
  },
  "classes": [
    {
      "path": "../path/to/class/file",
      // This field is optional, is used to override the class hash value of the class itself.
      // By default, the class hash will be computed from the class at `path`.
      "classHash": "0xdeadbeef"
    }
  ]
 
  ..
}

This method requires the user to input the exact class hash. For instance, if a class at "../path/to/class/file" computes to a hash of 0x1234, but 0xdeadbeef is specified in the genesis file, the latter becomes the canonical class hash. So, ideally we should favour an approach that doesn't involve affecting the resultant class hash.

To improve flexibility and avoid affecting the resultant class hash, a suggestion from the aforementioned PR

Trying to play around with classes, when I don't define the class hash of a class entry inside the genesis classes, it means I have to provider the exact class hash into the allocations. With the current implem, it may be possible to add a name field to the GenesisClassJson and allow a lookup by name. Does it make sense to you too?

recommends introducing a name field in [GenesisClassJson](https://github.com/dojoengine/dojo/blob/042a2c0d3659d76e08091919971573805c503f70/crates/katana/primitives/src/genesis/json.rs#L40). This field would allow class identification by a unique name rather than by hash, specifically for lookup purposes within the classes section of the genesis file. The scope of the field should only be limited to the lookup functionality in the classes section of the JSON file and should not have any significant outside of that.

{
  "feeToken": {
	"address": "0x55",
	"name": "ETHER",
	"symbol": "ETH",
	"decimals": 18,
        // This field is optional, if not provided the default class will be used.
        // If specified, the value must be one the class hash defined in the `classes` field below. Otherwise, an error will be returned.
	"class": "MyFeeTokenContract"
  },
  "classes": [
    {
      "path": "../path/to/class/file",
      // This field is optional, is used to override the class hash value of the class itself.
      // By default, the class hash will be computed from the class at `path`.
      "name": "MyFeeTokenContract"
    }
  ]
}

kariy avatar Feb 01 '24 05:02 kariy

I would like to give it a try please.

Yogalholic avatar Feb 01 '24 16:02 Yogalholic

I would like to give it a try please.

Assigned!

kariy avatar Feb 01 '24 17:02 kariy

@Yogalholic don't hesitate to ask question/help about the implementation

kariy avatar Feb 01 '24 17:02 kariy

@Yogalholic don't hesitate to ask question/help about the implementation

Ok, I'm used to contribute on Madara where we use cargo to compile/test but on dojo we have to compile with scarb instead?

Yogalholic avatar Feb 03 '24 05:02 Yogalholic

@Yogalholic don't hesitate to ask question/help about the implementation

Ok, I'm used to contribute on Madara where we use cargo to compile/test but on dojo we have to compile with scarb instead?

Nope, it's like you're used to do with madara, to compile the project and run the tests dojo also uses cargo.

# All the project
cargo build

# Running katana for instance
cargo run --bin katana

# Run the tests
cargo nextest run --all-features

Scarb is used internally by Sozo, so usually you don't use scarb explicitly when working with dojo.

glihm avatar Feb 03 '24 15:02 glihm

Hey, @Yogalholic. Any updates on this? Let me know if there's something blocking you.

kariy avatar Feb 18 '24 09:02 kariy

It took me some time to get accustomed to the Dojo/Katana stack. And my server needs one hour to compile dojo, that's why Im so long. I've almost finished updating the json.rs file then I will have to update the tests. Do you wish me to open a draft PR to take a look at my work?

Yogalholic avatar Feb 18 '24 10:02 Yogalholic

It took me some time to get accustomed to the Dojo/Katana stack. And my server needs one hour to compile dojo, that's why Im so long.

I see. Are you compiling the entire project? The scope of the issue doesn't expand outside of the katana-primitives crate, so you only ever need to compile that particular crate to make the compile time faster.

I've almost finished updating the json.rs file then I will have to update the tests. Do you wish me to open a draft PR to take a look at my work?

Yes, that would certainly be very helpful.

kariy avatar Feb 18 '24 10:02 kariy

I've created the draft PR here

Yogalholic avatar Feb 18 '24 15:02 Yogalholic

I've created the final PR here

Yogalholic avatar May 20 '24 15:05 Yogalholic

resolved in #1975

kariy avatar May 22 '24 19:05 kariy