Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(parser): use starknetjs parsers to encode decode data #334

Merged

Conversation

metalboyrick
Copy link
Collaborator

@metalboyrick metalboyrick commented Oct 25, 2024

Task name here

Fixes #326
Fixes #319

Types of change

  • Feature
  • Bug
  • Enhancement

Comments (optional)

Have not cleaned up yet, but tested with bulletproof contracts, should work with previous data types

TODO:

  • Debug page
  • useScaffoldReadContract hook
  • useScafffoldWriteContract hook
  • useScaffoldMultiWriteContract hook
  • useScaffoldEventHistory hook
  • Documentation on the hooks input and output - docs: update based on new parser ss2-docs#38
  • Code cleanup of old parsers
  • Update unit tests

@metalboyrick
Copy link
Collaborator Author

Can review first, I will leave it as draft for now @jrcarlos2000 @Nadai2010

@ngjupeng
Copy link
Collaborator

image
there's a bug with tuple unsigned integer

// translate boolean input
if (isCairoBool(key)) return convertStringInputToBool(value);

if (
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should include u256 and parsed by decimal?

    if (
      isValidNumber(value) &&
      (isCairoBigInt(key) || isCairoInt(key) || isCairoFelt(key) || isCairoU256(key))
    ) {
      return parseInt(value, 10);
    }

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not necessarily, inputs can be hex as well, but i can add another check

Copy link
Collaborator

@iossocket iossocket Oct 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your reply and explanation, I have a question, correct me if I'm wrong. If we write an unit test like this, what's the expect result? 10 or pared 10 as a hex value 16?

describe("utilsContract", () => {
  it("should parse u8 form correctly", () => {
    const result = getArgsAsStringInputFromForm({ "echo_u8_input_core::integer::u8": '10' }, false, true);
    expect(result).toEqual([10]);
  })
});

For an IntegerInput component, we can not input something like 0x10. for now so how can we differentiate decimal and hexadecimal?

Im thinking to add new logic branch to differentiate it. which can check hex number or decimal number

function isValidHexNumber(input?: string): boolean {
  if ((input || "").length === 0) return false;
  return /^0x[0-9a-fA-F]+$/i.test(input || "")
}
...

    if (
      isValidHexNumber(value) &&
      (isCairoBigInt(key) || isCairoInt(key) || isCairoFelt(key) || isCairoU256(key))
    ) {
      return parseInt(value, 16);
    }

    if (
      isValidNumber(value) &&
      (isCairoBigInt(key) || isCairoInt(key) || isCairoFelt(key) || isCairoU256(key))
    ) {
      return parseInt(value, 10);
    }

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And I wrote a unit test suit, try to cover all the cases, would you mind to check it?

import { describe, expect, it } from "vitest";
import { getArgsAsStringInputFromForm } from "../utilsContract";

describe("utilsContract", () => {
  it("should parse integer form correctly", () => {
    expect(getArgsAsStringInputFromForm({ "echo_u8_input_core::integer::u8": '10' }, false, true)).toEqual([10]);
    expect(getArgsAsStringInputFromForm({ "echo_u8_input_core::integer::u16": '10' }, false, true)).toEqual([10]);
    expect(getArgsAsStringInputFromForm({ "echo_u8_input_core::integer::u32": '10' }, false, true)).toEqual([10]);
    expect(getArgsAsStringInputFromForm({ "echo_u8_input_core::integer::u64": '10' }, false, true)).toEqual([10]);
    expect(getArgsAsStringInputFromForm({ "echo_u8_input_core::integer::u128": '10' }, false, true)).toEqual([10]);
    expect(getArgsAsStringInputFromForm({ "echo_u8_input_core::integer::u256": '10' }, false, true)).toEqual([10]);
    expect(getArgsAsStringInputFromForm({ "echo_u8_input_core::felt252": '10' }, false, true)).toEqual([10]);

    expect(getArgsAsStringInputFromForm({ "echo_u8_input_core::integer::i8": '10' }, false, true)).toEqual([10]);
    expect(getArgsAsStringInputFromForm({ "echo_u8_input_core::integer::i16": '10' }, false, true)).toEqual([10]);
    expect(getArgsAsStringInputFromForm({ "echo_u8_input_core::integer::i32": '10' }, false, true)).toEqual([10]);
    expect(getArgsAsStringInputFromForm({ "echo_u8_input_core::integer::i64": '10' }, false, true)).toEqual([10]);
    expect(getArgsAsStringInputFromForm({ "echo_u8_input_core::integer::i128": '10' }, false, true)).toEqual([10]);
  });

  it("should parse bool form correctly", () => {
    expect(getArgsAsStringInputFromForm({ "echo_u8_input_core::bool": 'true' }, false, true)).toEqual([true]);
    expect(getArgsAsStringInputFromForm({ "echo_u8_input_core::bool": '1' }, false, true)).toEqual([true]);
    expect(getArgsAsStringInputFromForm({ "echo_u8_input_core::bool": '0x1' }, false, true)).toEqual([true]);
    expect(getArgsAsStringInputFromForm({ "echo_u8_input_core::bool": '0x01' }, false, true)).toEqual([true]);
    expect(getArgsAsStringInputFromForm({ "echo_u8_input_core::bool": '0x001' }, false, true)).toEqual([true]);

    expect(getArgsAsStringInputFromForm({ "echo_u8_input_core::bool": 'false' }, false, true)).toEqual([false]);
    expect(getArgsAsStringInputFromForm({ "echo_u8_input_core::bool": '0' }, false, true)).toEqual([false]);
    expect(getArgsAsStringInputFromForm({ "echo_u8_input_core::bool": '0x0' }, false, true)).toEqual([false]);
    expect(getArgsAsStringInputFromForm({ "echo_u8_input_core::bool": '0x00' }, false, true)).toEqual([false]);
    expect(getArgsAsStringInputFromForm({ "echo_u8_input_core::bool": '0x000' }, false, true)).toEqual([false]);
  });

  it("should parse struct correctly", () => {
    const form = {
      "echo_struct_input_contracts::YourModel::User": {
        "name": {
          "type": "core::byte_array::ByteArray",
          "value": "john"
        },
        "age": {
          "type": "core::integer::u8",
          "value": "24"
        },
        "msg": {
          "type": "contracts::YourModel::Message",
          "value": {
            "variant": {
              "Quit": {
                "type": "()",
                "value": ""
              },
              "Echo": {
                "type": "core::felt252",
                "value": ""
              },
              "Move": {
                "type": "(core::integer::u128, core::integer::u128)",
                "value": "(1,2)"
              }
            }
          }
        }
      }
    }
    const result = getArgsAsStringInputFromForm(form, false, true);
    expect(result).toEqual([
      {
        "name": "john",
        "age": 24,
        "msg": {
          "variant": {
            "Move": {
              "0": 1,
              "1": 2
            },
          }
        }
      }
    ])
  });

  it("should parse multi input correctly", () => {
    const form = {
      "echo_enum_u_input_u8_core::integer::u8": "1",
      "echo_enum_u_input_contracts::YourModel::Message": {
        "variant": {
          "Quit": {
            "type": "()",
            "value": ""
          },
          "Echo": {
            "type": "core::felt252",
            "value": "32"
          },
          "Move": {
            "type": "(core::integer::u128, core::integer::u128)",
            "value": ""
          }
        }
      },
      "echo_enum_u_input_u16_core::integer::u16": "3",
      "echo_enum_u_input_u32_core::integer::i32": "4"
    }
    const result = getArgsAsStringInputFromForm(form, false, true);
    expect(result).toEqual([
      1,
      {
        "variant": {
          "Echo": 32
        }
      },
      3,
      4
    ])
  });
});

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hahaha sorry ser, would be helpful if you can branch out into a separate branch that is based off this branch and work from there if the changes are alot, cuz pretty hard to read in the comments cannot show which stuff are changed. Appreciate your effort in reviewing the PRs though!.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But this UT should prove helpful!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the distinction between hex or regular int, im currently considering the idea of using radio buttons, like how starkscan does it, wdyt cc @ngjupeng @Nadai2010 @jrcarlos2000, but automatic validation like the suggested change can work, just that we have to update the placeholder text later

image

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iossocket i will incorporate your isValidHex function

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@metalboyrick Hi I have created a pr on #337

@@ -66,8 +71,7 @@ export const ReadOnlyFunctionForm = ({
});

const handleRead = () => {
const newInputValue = getParsedContractFunctionArgs(form, false, true);

const newInputValue = getArgsAsStringInputFromForm(form, false, true);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image
bug with integer type, and

export const isCairoInt = (type: string): type is CairoInt =>
  /^core::integer::(u|i)(8|16|32)$/.test(type);

never trigger as true because the form is const key = getFunctionInputKey(abiFunction.name, input, inputIndex); so its start with function name not start with core

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will fix

@ngjupeng
Copy link
Collaborator

image
ContractAddress is bigint here

@metalboyrick metalboyrick marked this pull request as ready for review October 30, 2024 15:36
@metalboyrick
Copy link
Collaborator Author

Since I've written some changes for the Address component, this should resolve issue #319 as well

@ngjupeng
Copy link
Collaborator

ngjupeng commented Nov 3, 2024

image
0x0x issue from useScaffoldEventHistory on felt type

@ngjupeng
Copy link
Collaborator

ngjupeng commented Nov 3, 2024

image
issue happen in nested enum

Sample nested enum cairo code:

#[derive(Drop, Serde)]
enum SampleEnum {
    enum1: u256,
    enum2: felt252
}

#[derive(Drop, Serde)]
enum SampleNestedEnum {
    enum1: SampleEnum,
    enum2: felt252
}

#[derive(Drop, starknet::Event)]
pub struct EventWithNestedEnum {
    element1: u256,
    element2: SampleNestedEnum,
}

 fn emit_event_with_nested_enum(
          ref self: ContractState, element1: u256, element2: SampleNestedEnum
      ) {
          self.emit(EventWithNestedEnum { element1, element2 });
      }

@iossocket
Copy link
Collaborator

@ngjupeng hi, #339 had fixed the 0x0x event issue, would you mind to check it ?

@ngjupeng
Copy link
Collaborator

ngjupeng commented Nov 5, 2024

image

image
we need handle this case?

@jrcarlos2000
Copy link
Contributor

image

image we need handle this case?

this used to be handled before @metalboyrick iirc

@metalboyrick
Copy link
Collaborator Author

@jrcarlos2000 @ngjupeng used to be, new changes may have override this, will check

@ngjupeng
Copy link
Collaborator

ngjupeng commented Nov 5, 2024

image
0x is valid input (ignore the gia, it only exist on my local for debug purpose)

@ngjupeng
Copy link
Collaborator

ngjupeng commented Nov 5, 2024

image
u256>

@ngjupeng
Copy link
Collaborator

ngjupeng commented Nov 5, 2024

image
integer type is not working

@ngjupeng
Copy link
Collaborator

ngjupeng commented Nov 5, 2024

image
expect to return as contract format

@iossocket
Copy link
Collaborator

image will this be a fix for inputting more than one variant for enum form? @metalboyrick

@metalboyrick
Copy link
Collaborator Author

@iossocket yep, enums are supposed to only have one active variant only

@metalboyrick
Copy link
Collaborator Author

yeah but that somehow doesnt work

@iossocket
Copy link
Collaborator

image does not get this hint?

@metalboyrick
Copy link
Collaborator Author

wait now it works

@metalboyrick
Copy link
Collaborator Author

@iossocket that is the expected one

@metalboyrick
Copy link
Collaborator Author

i tink this breaks down if the enum is nested
@iossocket

@iossocket
Copy link
Collaborator

is it because one form will only have one errorMessage state, other field item may change it as well?

@metalboyrick
Copy link
Collaborator Author

yep @iossocket fixed this, cc @ngjupeng

@metalboyrick
Copy link
Collaborator Author

expect to return as contract format

@ngjupeng this has been fixed but some output might loook a bit uglier haha

@iossocket
Copy link
Collaborator

for the signed integer, starknetjs does not support right now. starknet-io/starknet.js#1177

@ngjupeng
Copy link
Collaborator

ngjupeng commented Nov 6, 2024

expect to return as contract format

@ngjupeng this has been fixed but some output might loook a bit uglier haha

this issue existed in tuple and struct also, but it was fixed on array

@metalboyrick
Copy link
Collaborator Author

merging and will record outstanding issues to unblock HH usage

@metalboyrick metalboyrick merged commit cac2446 into main Nov 6, 2024
2 checks passed
@metalboyrick metalboyrick deleted the refactor/use-starknetjs-parsers-to-encode-decode-data branch November 6, 2024 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

invalid input for correct tuple input UseScaffoldEventHistory "Invalid BigInt Syntax"
6 participants