Cairo1 runner fixes #675

MaksymMalicki · 2024-11-15T15:30:28Z

No description provided.

…arguments and initial gas to the memory

MaksymMalicki · 2024-12-10T14:32:11Z

Current changes in the existing code feature:

General:

Bump of version of go from 1.20 -> 1.21
Updates of the Makefile make integration command, so that it builds the nesseccary binaries for integration tests

cmd/cli/main.go:

Reading and parsing entrypoint arguments and gas
Loading gas and entrypoint arguments to the execution segment after builtins pointer
Shifting the keys in the hintmap by the size of entry code instructions

integration_tests/cairo_vm_test.go:

Fixes in the names of few of integration tests, so that they point to the correct layout
Writing the missing integration tests to the file
Restructuring of the file, so that the python VM is being run only for zero integration tests

pkg/hintrunner/core/cairo_hintparser.go, pkg/hintrunner/core/hint.go, pkg/hintrunner/core/hint_test.go, pkg/parsers/starknet/hint.go:

Corrections in parsing of Uint256InvModN
Introduction of ExternalWriteArgsToMemoryHint, which allows to easily write the input params to memory at given step, without if statements

pkg/hintrunner/hintrunner.go:

Writing userArgs to the scope of the ExternalWriteArgsToMemoryHint, so that they can be retrieved by the hint in the future

pkg/parsers/starknet/args.go, pkg/parsers/starknet/args_test.go:

Parsing of user input args and tests (only single values and arrays, does not support nesting)

pkg/runner/gas.go:

Introduces the additional initialization steps to the VM, including adding a pointer to the gas costs segment to the end of the program segment and writing gas costs of listed tokens to that segment (necessary for proper working of gas)

pkg/runner/runner.go:

Various cairo runner modes: Execution and Proof for cairo0 and cairo1
AssembleProgram - prepend entry code instructions and append footer instructions to the bytecode, shift hints ids accordingly
Add proper entry point initialization for Execution and Proof mode for Cairo1
Add proper initialization of builtins for Cairo1 (creating a builtin segment only when builtin is present both in layout and program builtins list
Generation of entry code for cairo1 - code refactoring for paramTypes handling, fixes in calculation of offset for call method in the entry code, proper handling of gas builtin

pkg/vm/vm.go:

Add helper, debug PrintMemory() method

* Add parsing logic for input user args * Add flags for available gas, input user args, writing args to memory * Fix unit tests for user arguments parsing * Lint the PR * Add user args to hint context * Refactor the code * Fix unconditional append of ExternalWriteArgsToMemory, bug fixes in integration tests * Add fixes of the call size calculation and include ExternalWriteArgsToMemory hint when gas present * Add layouts for integration tests * Add error handling * Fixes in entry code generation * Address changes mentioned in a discussion * Add comment regarding writing to memory in a hint for the future reference in the integration tests with args * Changes in calculations of the initial PC offset, CALL opcode offset incremented by mainFuncOffset, writing user args to the AP in the hint * Turn back VM config to private field * Add error handling on assign of `userArgs` to the initial scope * Lint project * Bump go version from 1.20 -> 1.21 (#678) * Bump go version from 1.20 -> 1.21 * Update golangci-lint * Simplify the Makefile * Correction in the makefile

…, code cleanups

cicr99 · 2025-01-08T14:35:21Z

integration_tests/cairo_vm_test.go

@@ -182,7 +179,7 @@ func TestCairoFiles(t *testing.T) {
 		{"./cairo_zero_hint_tests/", true},
 		{"./cairo_zero_file_tests/", true},
 		{"./builtin_tests/", true},
-		// {"./cairo_1_programs/", false},
+		{"./cairo_1_programs/", false},


Probably you'll need to leave the comment here, as the tests are not passing yet

cicr99 · 2025-01-08T14:58:30Z

pkg/hintrunner/hintrunner.go

+	if userArgs != nil {
+		err := context.ScopeManager.AssignVariable("userArgs", userArgs)
+		if err != nil {
+			panic(fmt.Errorf("assign userArgs: %v", err))


Can you leave a comment here that this line should not be reachable since the context was just initialized above?

cicr99 · 2025-01-08T15:01:29Z

pkg/runner/gas.go

+	"github.com/NethermindEth/cairo-vm-go/pkg/vm/memory"
+	mem "github.com/NethermindEth/cairo-vm-go/pkg/vm/memory"


Why both? I would use the one without the alias only

cicr99 · 2025-01-08T15:22:35Z

pkg/runner/gas.go

+
+const (
+	ConstToken TokenGasCost = iota + 1
+	HoleToken


Are you using this one? The rest I understand refer to builtins but I think for this one if you are going to leave it here then you should add a comment

Also for ConstToken

yes, it is not necessary for now, removed the unused tokens

cicr99 · 2025-01-08T15:23:39Z

pkg/runner/gas.go

+	MulModToken
+)
+
+func getTokenGasCost(token TokenGasCost) (uint64, error) {


Also I know these values come from the rust vm, could you pls add a comment with a link to the source?

cicr99 · 2025-01-08T16:16:23Z

pkg/vm/builtins/builtin_runner.go

@@ -25,6 +25,7 @@ const (
 	AddModeType
 	MulModType
 	GasBuiltinType
+	SystemType


What does this one refer to? I see you only use it in emulatedBuiltins?

ah yes, this is something I thought might be useful in the future, but it turned out not

MaksymMalicki added 2 commits November 15, 2024 16:16

Fixes for the generation of entry code, fixes of hints parsing

d7b74e6

Add modifications to the runner

dfd9ec8

MaksymMalicki changed the title ~~Fixes for the generation of entry code, fixes of hints parsing~~ Entry code generation fix Nov 21, 2024

MaksymMalicki and others added 3 commits December 2, 2024 08:20

Merge branch 'main' into add_code_entry

a7814a1

Add fixes for the entrycode generation

83872d8

Refactor main CLI, offset the hints indexes by entry code size, load …

cd04099

…arguments and initial gas to the memory

MaksymMalicki changed the title ~~Entry code generation fix~~ Cairo1 runner fixes Dec 10, 2024

MaksymMalicki and others added 7 commits December 25, 2024 21:55

Fix the integration tests

8812042

Fixes in the runner

78cf788

Fixes in the runner

af47a3d

Fix the unit tests, uncomment pythonVm execution in integration tests…

73f6dd5

…, code cleanups

Add writing tokens gas cost to memory

17b17df

Proper builtins initialization for cairo mode

688b3d2

cicr99 approved these changes Jan 8, 2025

View reviewed changes

Address comments in the PR

f105506

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cairo1 runner fixes #675

Cairo1 runner fixes #675

MaksymMalicki commented Nov 15, 2024

MaksymMalicki commented Dec 10, 2024 •

edited

Loading

cicr99 Jan 8, 2025

MaksymMalicki Jan 9, 2025

cicr99 Jan 8, 2025 •

edited

Loading

MaksymMalicki Jan 9, 2025

cicr99 Jan 8, 2025

MaksymMalicki Jan 9, 2025

cicr99 Jan 8, 2025

cicr99 Jan 8, 2025

MaksymMalicki Jan 9, 2025

cicr99 Jan 8, 2025

MaksymMalicki Jan 9, 2025

cicr99 Jan 8, 2025

MaksymMalicki Jan 9, 2025

		"github.com/NethermindEth/cairo-vm-go/pkg/vm/memory"
		mem "github.com/NethermindEth/cairo-vm-go/pkg/vm/memory"

Cairo1 runner fixes #675

Are you sure you want to change the base?

Cairo1 runner fixes #675

Conversation

MaksymMalicki commented Nov 15, 2024

MaksymMalicki commented Dec 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cicr99 Jan 8, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MaksymMalicki commented Dec 10, 2024 •

edited

Loading

cicr99 Jan 8, 2025 •

edited

Loading