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

Uint256 hints integration test #458

Open
wants to merge 7 commits into
base: Finalize-segments
Choose a base branch
from

Conversation

Sh0g0-1758
Copy link
Contributor

@Sh0g0-1758 Sh0g0-1758 commented Jun 12, 2024

This PR is aims to do the tasks mentioned here : #413

Current Progress :

  • Uint256Add
  • Split64
  • Uint256Sqrt
  • Uint256SignedNN
  • Uint256UnsignedDivRem
  • Uint256MulDivMod

Copy link
Contributor

@TAdev0 TAdev0 left a comment

Choose a reason for hiding this comment

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

looks good @Sh0g0-1758 ! why did you add "small" for all cairo files? they can be run with any layout except plain, i think you can remove that?

@Sh0g0-1758
Copy link
Contributor Author

@TAdev0 have you tried it locally ? It throws an error for me when I remove the specific layout. I believe @har777 told me about it.

@TAdev0
Copy link
Contributor

TAdev0 commented Jun 13, 2024

Oh yeah I guess this is how the file is detected as using layout small, my bad

@TAdev0
Copy link
Contributor

TAdev0 commented Jun 13, 2024

indeed:

	if strings.Contains(path, ".small") {
		layout = "small"

@Sh0g0-1758
Copy link
Contributor Author

Sh0g0-1758 commented Jun 13, 2024

@TAdev0 , So should I wait for another approval or can I merge right away ? I don't think the 2 approval github action is valid for branches other than main.

@TAdev0
Copy link
Contributor

TAdev0 commented Jun 13, 2024

@Sh0g0-1758 please wait for another approval so we stick to 2

@Sh0g0-1758
Copy link
Contributor Author

Sh0g0-1758 commented Jun 18, 2024

@TAdev0 , it seems like the code that you merged has a bug, the TestPoseidon is failing.

=== RUN   TestPoseidon
    cairozero_test.go:333: 
        	Error Trace:	/home/runner/work/cairo-vm-go/cairo-vm-go/integration_tests/cairozero_test.go:333
        	Error:      	Received unexpected error:
        	            	cairo-vm run builtin_tests/poseidon_test.starknet_with_keccak_compiled.json: exit status 1
        	            	Loading program at builtin_tests/poseidon_test.starknet_with_keccak_compiled.json
        	            	Running....
        	            	runtime error: pc 0:17 step 12: running instruction: infer res: segment 4, offset 3: range_check: check write: 2**128 < 4426822003494896[46](https://github.com/NethermindEth/cairo-vm-go/actions/runs/9553890922/job/26333768913?pr=458#step:6:47)213731521593476982257703159825582578145778919623645026501
        	Test:       	TestPoseidon

EDIT : nvm, The fix was pushed 6 hours ago, merged it.

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.

None yet

2 participants