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

Add test and support for negative Signed integers smaller than 252 bits #1014

Open
tabaktoni opened this issue Mar 14, 2024 · 13 comments
Open
Assignees
Labels
ODHack Issue to assign for the ODHack event Type: feature New feature or request

Comments

@tabaktoni
Copy link
Collaborator

tabaktoni commented Mar 14, 2024

Is your feature request related to a problem? Please describe.
https://docs.starknet.io/documentation/architecture_and_concepts/Smart_Contracts/serialization_of_Cairo_types/#data_types_of_252_bits_or_less

Describe the solution you'd like
Test current behavior and support required field el calc
i8, i16, i32, i64, and i128

Negativ value should be 2^{251} + 172^{192} + 1 + (negative value)
ex -5 should serialise to 2^{251} + 17
2^{192} + 1 - 5

@tabaktoni tabaktoni added the Type: feature New feature or request label Mar 14, 2024
@AryanGodara
Copy link

@tabaktoni Can I work on this one after completing #1018 ? :)

@tabaktoni
Copy link
Collaborator Author

Yes ofc, maybe the task will need to be expanded to multiple sub tasks we will need to check how to implement all Cairo int types in the manner of CairoInt.
But the initial step (Test current behavior ) can be immediately tackled.

@AryanGodara
Copy link

Yes ofc, maybe the task will need to be expanded to multiple sub tasks we will need to check how to implement all Cairo int types in the manner of CairoInt. But the initial step (Test current behavior ) can be immediately tackled.

Got it! I'll start looking into the tests first! Then see how to break this down into subtasks :)

@ivpavici ivpavici added ODHack Issue to assign for the ODHack event OnlyDust Open for OnlyDust contributors labels Apr 19, 2024
@ivpavici
Copy link
Collaborator

Task will be offered again on the ODHack event
https://onlydust.notion.site/ODHack-Common-Guidelines-b9c6b6a4ac4146d087185568aca38a3f

@AryanGodara
Copy link

@ivpavici I'm still working on the other issue. So please don't unassign there 😅
I had left a doubt regarding testing earlier. And I'd like to complete it during this week

@ivpavici ivpavici assigned AryanGodara and unassigned AryanGodara Apr 22, 2024
@ivpavici
Copy link
Collaborator

ok when you are done there we can assign you here if no one else wants to pick it up

@lucilapastore
Copy link

Hey! I am from Argentina. I am at the mu. I want to contribute and make this my first issue. Regards!

@ivpavici
Copy link
Collaborator

@lucilapastore good luck!

@fmmesen
Copy link

fmmesen commented May 22, 2024

Could I work on this? @ivpavici

@ivpavici
Copy link
Collaborator

good luck!

@fmmesen
Copy link

fmmesen commented Jun 11, 2024

Hey @ivpavici , sorry for the delay! I just have a doubt

Doing the serialization manually:

2^{251} + 172^{192} + 1 - 5

I'll receive:

3618502788666131113263695016908177884250476444008934042335404944711319814140
result that will be returned as a felt252

So the idea is to create a unit test that will expect(result).ToBe(361850278866613111......) for i8, i16, i32, i64 and i128?

@tabaktoni
Copy link
Collaborator Author

tabaktoni commented Jun 18, 2024

Implementation needs to be the similar to CairoUint256

In addition to this you need to define and store:

  1. LImits:
    For each signed variant can store numbers from:
    $−(2^{n−1})$ to $2^{n−1}−1$ inclusive, where n is the number of bits that variant uses.
    Example: So an i8 can store numbers from $−(2^7)$ to $2^7−1$, which equals -128 to 127

  2. felt252 representation in field math notation (one you mentioning)

  3. JSC Representation in ''standard'' math example you provided will return -5

The class will contain all these properties + existing ones as in the uint256 example.
Class needs to encode for request -5 to 3618502788... , and decode on response to -5
This needs to be a public class method.
From an end-user perspective, it would be used as a normal number -X ... +X, and lib will based on ABI compile it to field math representation.

So to finally answer the question, yes for unit testing you need to test all Class methods as you described, but in e2e test you will expect -5.

@fmmesen
Copy link

fmmesen commented Jun 26, 2024

Understood, I'll update you soon!

fmmesen added a commit to fmmesen/starknet.js that referenced this issue Jul 5, 2024
fmmesen added a commit to fmmesen/starknet.js that referenced this issue Aug 9, 2024
@ivpavici ivpavici removed the OnlyDust Open for OnlyDust contributors label Sep 13, 2024
fmmesen added a commit to fmmesen/starknet.js that referenced this issue Sep 22, 2024
fmmesen added a commit to fmmesen/starknet.js that referenced this issue Oct 7, 2024
fmmesen added a commit to fmmesen/starknet.js that referenced this issue Oct 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ODHack Issue to assign for the ODHack event Type: feature New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants