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: Difference in (StatementIdx(97456), Bitwise): Some(2) != Some(0). #4455

Closed
enitrat opened this issue Nov 22, 2023 · 4 comments · Fixed by #4464
Closed

bug: Difference in (StatementIdx(97456), Bitwise): Some(2) != Some(0). #4455

enitrat opened this issue Nov 22, 2023 · 4 comments · Fixed by #4464
Labels
bug Something isn't working

Comments

@enitrat
Copy link
Contributor

enitrat commented Nov 22, 2023

Bug Report

Cairo version:
2.4.0-rc3

Current behavior:
Compilation of the following function fails.

fn commit(self: @Account) -> Result<(), EVMError> {
        // The account can be deployed already, but can be `should_deploy` as well if we're deploying on a previously selfdestructed account.
        let is_deployed = self.address().evm.is_deployed();

        // If a Starknet account is already deployed for this evm address, we
        // should "EVM-Deploy" only if the bytecode length is different (!=0) or the nonce is different.
        let should_deploy = if is_deployed {
            let deployed_bytecode_len = ContractAccountTrait::fetch_bytecode_length(self)?;
            let deployed_nonce = ContractAccountTrait::fetch_nonce(self)?;
            if (deployed_bytecode_len != self.bytecode().len() || deployed_nonce != *self.nonce) {
                true
            } else {
                false
            }
        } else {
            // Otherwise, the deploy condition is simply has_code_or_nonce.
            self.should_deploy()
        };

        if should_deploy {
            // If SELFDESTRUCT, deploy empty SN account
            let (initial_nonce, initial_code) = if (*self.selfdestruct == true) {
                (0, Default::default().span())
            } else {
                (*self.nonce, *self.code)
            };
            ContractAccountTrait::deploy(
                self.address().evm,
                initial_nonce,
                initial_code,
                deploy_starknet_contract: !is_deployed
            )?;
            return Result::Ok(());
        //Storage is handled outside of the account and must be commited after all accounts are commited.
        };

        // If the account was not scheduled for deployment - then update it if it's deployed.
        if is_deployed {
            // Only CAs have components commited on starknet.
            if self.is_ca() {
                if *self.selfdestruct {
                    return ContractAccountTrait::selfdestruct(self);
                }
                self.store_nonce(*self.nonce);
            };
            return Result::Ok(());
        };
        return Result::Ok(());
    }

The error message is:

Difference in (StatementIdx(97456), Bitwise): Some(2) != Some(0).
Difference in (StatementIdx(97453), Bitwise): Some(0) != Some(2).
Difference in (StatementIdx(97471), Pedersen): Some(10) != Some(0).
Difference in (StatementIdx(97444), Pedersen): Some(0) != Some(10).
Difference in (StatementIdx(97385), Pedersen): Some(0) != Some(10).
Difference in (StatementIdx(97385), Bitwise): Some(0) != Some(2).
Difference in (StatementIdx(97471), Bitwise): Some(2) != Some(0).
Difference in (StatementIdx(97444), Bitwise): Some(0) != Some(2).
Difference in (StatementIdx(97456), Pedersen): Some(10) != Some(0).
Difference in (StatementIdx(97453), Pedersen): Some(0) != Some(10).
thread 'main' panicked at /Users/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/cairo-lang-sierra-gas-2.4.0-rc3/src/gas_info.rs:73:9:
Comparison failed.
stack backtrace:
   0: _rust_begin_unwind
   1: core::panicking::panic_fmt
   2: cairo_lang_sierra_to_casm::metadata::calc_metadata
   3: cairo_lang_starknet::casm_contract_class::CasmContractClass::from_contract_class
   4: <core::iter::adapters::GenericShunt<I,R> as core::iter::traits::iterator::Iterator>::next
   5: <scarb::compiler::compilers::starknet_contract::StarknetContractCompiler as scarb::compiler::Compiler>::compile
   6: scarb::commands::build::run
   7: scarb::commands::run
   8: scarb::main

The bug manifests when compiling our contracts crate with scarb build -p contracts.
However, it disappears if I comment line 213, with the explicit return in the if should_deploy block commented.

 if should_deploy {
            // If SELFDESTRUCT, deploy empty SN account
            let (initial_nonce, initial_code) = if (*self.selfdestruct == true) {
                (0, Default::default().span())
            } else {
                (*self.nonce, *self.code)
            };
            ContractAccountTrait::deploy(
                self.address().evm,
                initial_nonce,
                initial_code,
                deploy_starknet_contract: !is_deployed
            )?;
            // return Result::Ok(()); commenting this compiles.
        };

Expected behavior:
No unexpected compilation error.

Steps to reproduce:
Clone the Kakarot repository and clone the bugreport/22-11 branch.

@enitrat enitrat added the bug Something isn't working label Nov 22, 2023
@orizi
Copy link
Collaborator

orizi commented Nov 23, 2023

Known issue - unfortunately will probably only be fixed in sierra-1.5.0.
It is because the update of gas costs is not in full effect just yet.
In general putting pedersen and bitwise usages inside a non-inlined function should solve the issue.

@enitrat
Copy link
Contributor Author

enitrat commented Nov 23, 2023

In general putting pedersen and bitwise usages inside a non-inlined function should solve the issue.

How can I take action on this?
I just introduced a second if should_deploy branch where I did the return statement, so it's not a blocker anyway.

@orizi
Copy link
Collaborator

orizi commented Nov 23, 2023

i'm trying something to see if we can fix this here - hold on a bit.

@orizi orizi linked a pull request Nov 23, 2023 that will close this issue
@orizi
Copy link
Collaborator

orizi commented Nov 23, 2023

Linked PR solves the issue.
Will release a new rc when it is merged.

@orizi orizi closed this as completed Nov 23, 2023
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.

2 participants