starknet-io / starknet.js

JavaScript library for StarkNet
https://www.starknetjs.com
MIT License
1.21k stars 739 forks source link

orderPropsByAbi #762

Closed tabaktoni closed 10 months ago

tabaktoni commented 12 months ago

Describe the bug Populate lose one parameter, also check codebase for other potential issues.

  const contract = new Contract(TestAbi1.abi, '0x123', account);

  let myCall = contract.populate("setAddressLen", { chain_id: 1, address_len: 20 });

  console.log("myCall", myCall);

This result in calldata [ '1' ] instead of ['1', '20']

To Reproduce Minimal test ABI

{
    "abi": [
        {
            "type": "function",
            "name": "setAddressLen",
            "inputs": [
                {
                    "name": "chain_id",
                    "type": "core::felt252"
                },
                {
                    "name": "address_len",
                    "type": "core::felt252"
                }
            ],
            "outputs": [],
            "state_mutability": "external"
        }
    ]
}

Additional context last version of StarknetJS

PhilippeR26 commented 12 months ago

address_len is considered as part of an array. Valid for Cairo 0 contract, but not for Cairo 1. I will see how to discriminate both cases.

PhilippeR26 commented 12 months ago

I would like to use cairo.isCairo1type() to identify a Cairo 1 contract, but I am not sure that type.includes('core::') is covering 100% of cases. In the new ArgentX account contract, I have found :

{
          "type": "function",
          "name": "execute_from_outside",
          "inputs": [
            {
              "name": "outside_execution",
              "type": "lib::outside_execution::OutsideExecution"
            },
            {
              "name": "signature",
              "type": "core::array::Array::<core::felt252>"
            }
          ],
          "outputs": [
            {
              "type": "core::array::Array::<core::array::Span::<core::felt252>>"
            }
          ],
          "state_mutability": "external"
        }

outside_execution hasn't a type including core:: Should we change to include only ::?

tabaktoni commented 12 months ago

Yea it is edge case with _len reserved namind for cairo0 but should be used in Cairo1 Yes i think matcher for ``::*`` will be good

tabaktoni commented 10 months ago

We can close this one as it should be resolved