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

[BUG] The CairoSerializer cannot handle names starting with a underscore #260

Closed
ca11ab1e opened this issue Jul 21, 2022 · 7 comments · Fixed by #262
Closed

[BUG] The CairoSerializer cannot handle names starting with a underscore #260

ca11ab1e opened this issue Jul 21, 2022 · 7 comments · Fixed by #262
Labels
bug Something isn't working

Comments

@ca11ab1e
Copy link
Contributor

ca11ab1e commented Jul 21, 2022

I've a Cairo contract using names starting with a underscore. And using the FunctionCallSerializer for get back data from Cairo to Python (to decode transactions return data) will raise:

ValueError: Field names cannot start with an underscore: '_name`

It's a namedtuple limitation, from here:

res = NamedTuple(
"Result", [(key, type(value)) for key, value in result.items()]
)

Python version

All.

Expected Behavior

Names starting with a underscore should not be a problem.

Current Behavior

ValueError: Field names cannot start with an underscore: '_name`

Possible Solution

  1. We could use the rename kwargs from the namedtuple, but will would loose the real name then.
  2. An idea using dataclasses, but we would then loose the tuple comparison (which is mandatory to keep):
 from dataclasses import make_dataclass

 # instead of the namedtuple:
 result = make_dataclass("Result", [(k, v) for k, v in ...], order=True, frozen=True, slots=True)
  1. 💟 https://github.com/ksamuel/drecord

Steps to Reproduce

  1. Use a name starting with a underscore, in any Cairo method.
  2. Use FunctionCallSerializer.to_python().
  3. See the exception.

Context (Environment)

The issue is quite big, because we're using the FunctionCallSerializer in the Ape StarkNet plugin to decode transactions return data. The goal is to leverage all the fantastic work you've already done to decode all kind of data types (like arrays, and custom complex structs).

Detailed Description

Possible Implementation

I like the 3rd idea. If you think it's something you would like to implement, I could give a try.

@Arcticae
Copy link
Member

Drecord doesn't have equality implemented which is quite a complex feature. We have a couple of ideas we want to try - @war-in will handle this one.

@Arcticae
Copy link
Member

Or me if he's too busy ;)

@Arcticae Arcticae added the bug Something isn't working label Jul 21, 2022
@war-in
Copy link
Collaborator

war-in commented Jul 22, 2022

If you are going to decode any Cairo data you could try using CairoSerializer. It can decode all cairo structures if their types are known :) There is a section in our documentation about it: https://starknetpy.readthedocs.io/en/latest/guide.html#using-cairoserializer

@ca11ab1e
Copy link
Contributor Author

ca11ab1e commented Jul 22, 2022

I am using it :)

The problem here, is that such a Cairo function will break the serializer:

func foo(_arg : felt):
    return ()
end

_arg is not allowed in namedtuples because it starts with a underscore.

https://docs.python.org/3/library/collections.html#collections.namedtuple states:

Any valid Python identifier may be used for a fieldname except for names starting with an underscore. Valid identifiers consist of letters, digits, and underscores but do not start with a digit or underscore and cannot be a keyword such as class, for, return, global, pass, or raise.

@ca11ab1e
Copy link
Contributor Author

And reading again the namedtuple limitation, it means arguments names like "class, for, return, global, pass, raise" will be problematic too. There are quite specific though.

@war-in
Copy link
Collaborator

war-in commented Jul 22, 2022

Yeah, I know that my comment doesn't close the issue, but you mentioned FunctionCallSerializer and I thought that you didn't know about CairoSerializer because it is the relatively new one.

@ca11ab1e
Copy link
Contributor Author

Oh OK :)
I know that FunctionCallSerializer calls CairoSerializer under the wood, and as FunctionCallSerializer is the new DataTransformer name, I used it. But yes, both have the same issue at the end.

@Arcticae Arcticae linked a pull request Jul 22, 2022 that will close this issue
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants