Conversation
There was a problem hiding this comment.
Pull request overview
This PR extends network tests by adding comprehensive account operation tests for the Sepolia testnet. The changes introduce tests for core account functionalities including balance queries, transaction execution, account deployment, contract declaration, and contract deployment.
- Adds a new test file with 6 tests covering account operations on Sepolia testnet
- Introduces a "SaltedContract" that generates unique class hashes per compilation through dynamic function naming
- Updates the compilation script to handle salted contracts by temporarily replacing placeholders during builds
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| starknet_py/tests/e2e/tests_on_networks/account_test.py | New test file with 6 network tests for account operations: get_nonce, get_balance, execute_v3, deploy_account_v3, declare_v3, and deploy_v3 |
| starknet_py/tests/e2e/mock/contracts_v2/src/salted_contract.cairo | Minimal Cairo contract with a placeholder function name that gets replaced during compilation to ensure unique class hashes |
| starknet_py/tests/e2e/mock/contracts_v2/src/lib.cairo | Adds module reference for the new salted_contract |
| starknet_py/tests/e2e/mock/compile_contracts.sh | Adds functions to update and restore salted contracts during compilation, integrating salt replacement into the build process |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @pytest.mark.asyncio | ||
| async def test_estimate_fee_for_declare_transaction( | ||
| account, map_compiled_contract_and_class_hash | ||
| ): | ||
| (compiled_contract, class_hash) = map_compiled_contract_and_class_hash | ||
| declare_tx = await account.sign_declare_v3( | ||
| compiled_contract=compiled_contract, | ||
| compiled_class_hash=class_hash, | ||
| resource_bounds=MAX_RESOURCE_BOUNDS, | ||
| ) | ||
|
|
||
| estimated_fee = await account.client.estimate_fee(tx=declare_tx) | ||
|
|
||
| assert isinstance(estimated_fee.overall_fee, int) | ||
| assert estimated_fee.overall_fee > 0 | ||
| assert estimated_fee.calculate_overall_fee() == estimated_fee.overall_fee | ||
|
|
||
|
|
There was a problem hiding this comment.
note: Duplicate of test_account_estimate_fee_for_declare_transaction
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - name: Install dependencies needed for HIDAPI and Pillow | ||
| run: | | ||
| sudo apt install python3-dev libusb-1.0-0-dev libudev-dev libjpeg-dev | ||
| sudo apt-get update && sudo apt install --fix-missing python3-dev libusb-1.0-0-dev libudev-dev libjpeg-dev |
There was a problem hiding this comment.
note: I experienced that this step fails indeterministically, hence this change.
| # Prefund account with 5 * deployment fee | ||
| invocation = await contract.functions["transfer"].invoke_v3( | ||
| address, deploy_account_fee.overall_fee * 5, auto_estimate=True | ||
| ) |
There was a problem hiding this comment.
note: 5 is arbitrary, but wanted to have a safe margin for the deployment fee.
…e-mansion/starknet.py into extend-network-tests # Conflicts: # .github/actions/compile-contracts/action.yml # .github/workflows/checks.yml
This reverts commit ee75a81.
…e-mansion/starknet.py into extend-network-tests
There was a problem hiding this comment.
note: Sleeps are added to lower risk of tests failure due to old nonce, tests like to fails sometimes because of it. If you see a better way I'm happy to change it.
…rknet.py into refactor-contracts-utils
There was a problem hiding this comment.
Are we sure we want to run this test suite on every CI run?
There was a problem hiding this comment.
We can surely run them only in the compatibility suite, but I’m wondering whether keeping these 4 tests in the regular CI does more good than harm - especially since we don't have any basic network-backed coverage for these methods (on regular run). WDYT?
There was a problem hiding this comment.
Running interactions with the network hundreds of times with each change doesn’t seem ideal, maybe only run them on merge?
…ion/starknet.py into extend-network-tests # Conflicts: # starknet_py/tests/e2e/mock/contracts_v2/.tool-versions # starknet_py/tests/e2e/mock/contracts_v2/Scarb.toml
…ion/starknet.py into extend-network-tests
2 participants
Stack
scarbto2.15.0#1701Towards #1681
Introduced changes
Extend network tests with account tests