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

Implementation of eth_get_transaction_count function #983

Merged
merged 7 commits into from
Sep 30, 2024

Conversation

Gerson2102
Copy link
Contributor

@Gerson2102 Gerson2102 commented Sep 26, 2024

This PR include changes on the following file crates/contracts/src/kakarot_core/eth_rpc.cairo. I'm doing the implementation of the eth_get_transaction_count function. The issue is not done yet.

Pull Request type

Normal

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no API changes)
  • Build-related changes
  • Documentation content changes
  • Other (please describe):

Resolves: #942

Does this introduce a breaking change?

  • Yes
  • No

This change is Reviewable

@Gerson2102
Copy link
Contributor Author

I was checking in the codebase for more places where we can now use the eth_get_transaction_count but I didn't find any to be honest. Let me know if there is any that is missing. @enitrat

@Gerson2102
Copy link
Contributor Author

Gerson2102 commented Sep 29, 2024

I added one test. But I was thinking about change the nonce and check it again. But I dont really know how to do it... Maybe using set_nonce()?

@enitrat
Copy link
Contributor

enitrat commented Sep 29, 2024

  1. don't deploy contracts with setup_contracts_for_testing, just do a local, internal test with
    https://github.com/kkrt-labs/kakarot-ssj/pull/983/files#diff-843e16e046c7baf8fcae8e8d2c2bfc5f8aecfed6d81af61768352c08e6ac83f7R231

  2. you can use mock_call from starknet foundry to mock the account call to get_nonce to return an arbitrary value - you can check existing usages in the codebase

@Gerson2102
Copy link
Contributor Author

Gerson2102 commented Sep 30, 2024

This is the test that I'm doing now:

#[test]
    fn test_eth_get_transaction_count() {
        let kakarot_state = set_up();
        // Deployed eoa should return a zero nonce
        let starknet_address = compute_starknet_address(
            test_address(), evm_address(), uninitialized_account()
        );
        start_mock_call::<u256>(starknet_address, selector!("get_nonce"), 0);
        assert_eq!(kakarot_state.eth_get_transaction_count(evm_address()), 0);
    }

And I'm getting this error:

[FAIL] contracts::kakarot_core::eth_rpc::tests::test_eth_get_transaction_count

Failure data:
    "Contract not deployed at address: 0x0"

That was the reason that I was using deploy_eoa() function before.

@enitrat
Copy link
Contributor

enitrat commented Sep 30, 2024

the issue was that the setup function does not intitialize anyu storage var, thus the class used for address computation was 0

enitrat
enitrat previously approved these changes Sep 30, 2024
@enitrat enitrat merged commit 81dc3c7 into kkrt-labs:main Sep 30, 2024
3 of 4 checks passed
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.

feat: implement eth_get_transaction_count
2 participants