Problem: push0 opcode is not tested #1273
Conversation
- shanghai hardfork introduces push0 opcode, can be enabled in solc
yihuang
requested review from
devashishdxt and
thomas-nguy
and removed request for
a team
WalkthroughThe updates across the various files primarily involve a shift in the Solidity version from 0.8.10 to 0.8.21, suggesting an alignment with the latest compiler features and optimizations. There's also a new Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on X ? TipsChat with CodeRabbit Bot (
|
Signed-off-by: yihuang <[email protected]>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (11)
- CHANGELOG.md (1 hunks)
- integration_tests/contracts/contracts/CosmosToken.sol (1 hunks)
- integration_tests/contracts/contracts/Gravity.sol (1 hunks)
- integration_tests/contracts/contracts/ICosmosToken.sol (1 hunks)
- integration_tests/contracts/contracts/TestBlackListERC20.sol (1 hunks)
- integration_tests/contracts/contracts/TestCRC20.sol (1 hunks)
- integration_tests/contracts/contracts/TestERC20A.sol (1 hunks)
- integration_tests/contracts/contracts/TestERC20Utility.sol (1 hunks)
- integration_tests/contracts/contracts/TestERC21Source.sol (1 hunks)
- integration_tests/contracts/contracts/TestMaliciousSupply.sol (1 hunks)
- integration_tests/contracts/hardhat.config.ts (1 hunks)
Files skipped from review due to trivial changes (3)
- integration_tests/contracts/contracts/ICosmosToken.sol
- integration_tests/contracts/contracts/TestBlackListERC20.sol
- integration_tests/contracts/contracts/TestERC20Utility.sol
Additional comments: 7
integration_tests/contracts/contracts/TestERC20A.sol (1)
- 1-1: Ensure that the contract
TestERC20Ahas been thoroughly tested with the updated Solidity version0.8.21to prevent any unexpected behavior due to changes in the compiler.integration_tests/contracts/hardhat.config.ts (1)
- 7-12: Confirm that the Hardhat configuration changes, including the Solidity version upgrade to
0.8.21and theevmVersionset to "shanghai", are compatible with the entire project and do not introduce any breaking changes.integration_tests/contracts/contracts/TestMaliciousSupply.sol (1)
- 1-1: Ensure that the contract
TestMaliciousSupplyhas been thoroughly tested with the updated Solidity version0.8.21to prevent any unexpected behavior due to changes in the compiler, especially given the contract's purpose for testing edge cases.integration_tests/contracts/contracts/TestCRC20.sol (1)
- 1-1: Ensure that the contract
TestCRC20has been thoroughly tested with the updated Solidity version0.8.21to prevent any unexpected behavior due to changes in the compiler.integration_tests/contracts/contracts/CosmosToken.sol (1)
- 1-1: Ensure that the contract
CosmosTokenhas been thoroughly tested with the updated Solidity version0.8.21to prevent any unexpected behavior due to changes in the compiler.integration_tests/contracts/contracts/TestERC21Source.sol (1)
- 1-1: Ensure that the contract
TestERC21Sourcehas been thoroughly tested with the updated Solidity version0.8.21to prevent any unexpected behavior due to changes in the compiler.integration_tests/contracts/contracts/Gravity.sol (1)
- 1-4: Ensure that the contract
Gravityhas been thoroughly tested with the updated Solidity version0.8.21and the new imports from the "@openzeppelin/contracts" library to prevent any unexpected behavior due to changes in the compiler and new dependencies.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (5)
- integration_tests/conftest.py (1 hunks)
- integration_tests/test_basic.py (2 hunks)
- integration_tests/test_ibc_rly_gas.py (1 hunks)
- integration_tests/test_pruned_node.py (1 hunks)
- integration_tests/utils.py (2 hunks)
Additional comments: 4
integration_tests/conftest.py (1)
- 14-14: The addition of the "unmarked" marker to the pytest configuration is a good practice to handle tests without specific markers. Ensure that this new configuration is documented and that the team is aware of its usage.
integration_tests/test_pruned_node.py (1)
- 49-49: The decrement of
exp_gas_usedby 1 should reflect an accurate expectation of gas usage for the transaction. Verify that this change is consistent with the gas costs observed after thepush0opcode update.integration_tests/test_basic.py (2)
54-54: The addition of
assert receipt.gasUsed == 21000assumes that the gas used for the transaction will always be exactly 21000. This is a common amount for a simple ETH transfer, but it may not hold true for all environments or future updates. Consider verifying if this value should be hardcoded or if it should allow for some variance.63-63: The addition of
exp_gas_used=633810as an argument to thedeploy_contractfunction call seems to be a specific expectation for the gas used during contract deployment. Ensure that this value is accurate and consider if it should be a constant or derived from some configuration, especially since gas usage can change with contract complexity or EVM changes.
Signed-off-by: yihuang <[email protected]>
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (3)
- CHANGELOG.md (1 hunks)
- integration_tests/test_basic.py (2 hunks)
- integration_tests/utils.py (2 hunks)
Files skipped from review due to trivial changes (1)
- CHANGELOG.md
Files skipped from review as they are similar to previous changes (2)
- integration_tests/test_basic.py
- integration_tests/utils.py
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
* Problem: there are flaky integration tests wait for first block * add missing markers
- shanghai hardfork introduces push0 opcode, can be enabled in solc Update CHANGELOG.md Signed-off-by: yihuang <[email protected]> enlarge gas gap assert some exact gas number fix python lint test upgrade test push0 opcode in upgrade
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (5)
- integration_tests/conftest.py (2 hunks)
- integration_tests/contracts/contracts/Greeter.sol (1 hunks)
- integration_tests/contracts/hardhat.config.ts (1 hunks)
- integration_tests/test_min_gas_price.py (4 hunks)
- integration_tests/test_upgrade.py (2 hunks)
Files skipped from review due to trivial changes (2)
- integration_tests/contracts/contracts/Greeter.sol
- integration_tests/contracts/hardhat.config.ts
Files skipped from review as they are similar to previous changes (1)
- integration_tests/conftest.py
Additional comments: 7
integration_tests/test_min_gas_price.py (4)
6-6: The addition of
wait_for_new_blocksto the import statement is noted. Ensure that this function is used appropriately within the test cases.29-29: The change in the directory name from "min-gas-price-gt" to "min-gas-price-lte" is observed. Confirm that this change aligns with the intended test environment setup and that no references to the old directory name remain in the codebase or documentation.
68-68: The addition of
wait_for_new_blocksbefore thetest_dynamic_fee_txfunction is appropriate to ensure the blockchain state is ready for the test. This is a good practice to avoid flaky tests due to timing issues.106-106: Similarly, the addition of
wait_for_new_blocksbefore thetest_base_fee_adjustmentfunction is a good practice to ensure the blockchain state is consistent before running the test.integration_tests/test_upgrade.py (3)
127-131: The conditional block checking for
testnetand asserting the absence of support for thePUSH0opcode before the upgrade is a good addition. It ensures that the test environment correctly reflects the capabilities of the system before and after the upgrade.191-193: The conditional block checking for
testnetand deploying a new contract to verify support for thePUSH0opcode after the upgrade is a good validation step. It confirms that the upgrade has taken effect and the new opcode is now supported.195-197: The JSON-RPC query assertions are correct and ensure that the state of the blockchain before the upgrade is still accessible and consistent after the upgrade.
Signed-off-by: yihuang <[email protected]>
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- integration_tests/test_upgrade.py (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- integration_tests/test_upgrade.py
Signed-off-by: yihuang <[email protected]>
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- integration_tests/test_basic.py (2 hunks)
- integration_tests/test_upgrade.py (2 hunks)
Files skipped from review as they are similar to previous changes (2)
- integration_tests/test_basic.py
- integration_tests/test_upgrade.py
👮🏻👮🏻👮🏻 !!!! REFERENCE THE PROBLEM YOUR ARE SOLVING IN THE PR TITLE AND DESCRIBE YOUR SOLUTION HERE !!!! DO NOT FORGET !!!! 👮🏻👮🏻👮🏻
PR Checklist:
make)make test)go fmt)golangci-lint run)go list -json -m all | nancy sleuth)Thank you for your code, it's appreciated! :)
Summary by CodeRabbit
Documentation
New Features
push0opcode in integration tests for enhanced smart contract capabilities.Bug Fixes
Tests