feat: add support for circuits

[ilyalesokhin-starkware]

This change is 

[ilyalesokhin-starkware]

crates/blockifier/feature_contracts/cairo1/test_contract.cairo line 552 at r1 (raw file):

Previously, dorimedini-starkware wrote…

why the `(x,)` single-tuple? non-blocking

yes, it's a tuple with a single value.

A circuit is defined using a tuple of outputs. (out1, out2, out3, ...)

[ilyalesokhin-starkware]

crates/blockifier/src/execution/entry_point_test.rs line 560 at r1 (raw file):

Previously, dorimedini-starkware wrote…

is there a formula for this number? asking because upgrading the compiler and recompiling cairo1 contracts is painful because of magic numbers like these; every one needs to be manually updated to get tests to pass. if the number is a function of some other maintainable numbers / named values that would help...

I can remove it and leave just the account test.

There are other places where we have such costs: https://github.com/starkware-libs/blockifier/blob/b22fb076a7db5e0fcdd2048a6fb579b0b1d25561/crates/blockifier/src/execution/syscalls/syscalls_test.rs#L132

You do want to make sure that those costs don't change without changes to the compiler. maybe we need a consts file and a fix script?

[ilyalesokhin-starkware]

crates/blockifier/src/transaction/account_transactions_test.rs line 70 at r1 (raw file):

Previously, dorimedini-starkware wrote…

replace this with `resource_bounds`, this is a v3 transaction. test is currently passing because resource bounds are zero, i.e. fee is ignored

should I use?

Code snippet:

l1_resource_bounds(u64::from(!zero_bounds), DEFAULT_STRK_L1_GAS_PRICE)

[ilyalesokhin-starkware]

crates/blockifier/src/transaction/account_transactions_test.rs line 70 at r1 (raw file):

Previously, ilyalesokhin-starkware wrote…

should I use?

why is it called l1_resource_bounds?

[ilyalesokhin-starkware]

crates/blockifier/src/transaction/account_transactions_test.rs line 108 at r2 (raw file):

    eprint!("tx_execution_info: {:?}", tx_execution_info.revert_error);
    assert!(tx_execution_info.revert_error.is_none());

this fails with: tx_execution_info: Some("Insufficient max L1 gas: max amount: 5537, actual used: 6175.")thread 'transaction::account_transaction::test::test_circuit' panicked at crates/blockifier/src/transaction/account_transactions_test.rs:108:5:

How am I supposed to get the resource_bound?

Code quote:

assert!(tx_execution_info.revert_error.is_none());

[ilyalesokhin-starkware]

crates/blockifier/src/transaction/account_transactions_test.rs line 70 at r1 (raw file):

Previously, dorimedini-starkware wrote…

because it creates a ResourceBounds instance without L2 bounds, only L1 bounds. And please use a non-zero bound on the amount, so fee limits aren;t ignored (the average-case usage of execution layer)

What do you mean by "use a non-zero bound on the amount"?

[ilyalesokhin-starkware]

crates/blockifier/src/blockifier/config.rs line 8 at r5 (raw file):

Previously, dorimedini-starkware wrote…

why this change? @noaov1 PTAL

I was unable to compile the code without it.

[ilyalesokhin-starkware]

crates/blockifier/src/execution/entry_point_execution.rs line 167 at r1 (raw file):

Previously, noaov1 (Noa Oved) wrote…

What is the `range_check96` builtin? We only have mul and add (sub and inverse are obtained from those?)? Do you need to add it [here](https://github.com/starkware-libs/blockifier/blob/605b4bd52fa1c3d68dd3dfa9a63f1e9d4b57dece/crates/blockifier/src/bouncer.rs#L163) as well?

range_check96 is a 96bit rangecheck instead of 128. yes, the only builtins are mul and add.

I guess I should add the builtins in the place you suggested.

[ilyalesokhin-starkware]

crates/blockifier/src/transaction/account_transactions_test.rs line 54 at r5 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Why do we need to test it in a context of an account transaction? (just curious)

I don't mind removing it, @dorimedini-starkware said that this is a better testing framework.

[ilyalesokhin-starkware]

crates/blockifier/src/blockifier/config.rs line 8 at r5 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Can you please explain?

When I press run test in vscode, I get the error below.

Maybe it's a vscode only error.

Code snippet:

error[E0599]: no function or associated item named `create_for_testing` found for struct `ConcurrencyConfig` in the current scope
  --> crates/blockifier/src/blockifier/config.rs:7:55
   |
7  |         Self { concurrency_config: ConcurrencyConfig::create_for_testing() }
   |                                                       ^^^^^^^^^^^^^^^^^^ function or associated item not found in `ConcurrencyConfig`

[ilyalesokhin-starkware]

crates/blockifier/resources/versioned_constants.json line 560 at r8 (raw file):

            16,
            100
        ],

What do I put here and in the older versions?

Code quote:

        "add_mod_builtin": [
            16,
            100
        ],

[ilyalesokhin-starkware]

crates/blockifier/src/transaction/account_transactions_test.rs line 54 at r5 (raw file):

Previously, ilyalesokhin-starkware wrote…

I don't mind removing it, @dorimedini-starkware said that this is a better testing framework.

@dorimedini-starkware what test do you think we should keep?

[ilyalesokhin-starkware]

crates/blockifier/src/fee/fee_utils.rs line 50 at r11 (raw file):

Previously, dorimedini-starkware wrote…

what's this change?

debugging, I'll revet it.