search

onflow / flow-evm-gateway

FlowEVM Gateway implements an Ethereum-equivalent JSON-RPC API for EVM clients to use
https://developers.flow.com/evm/about
Apache License 2.0
11 stars 9 forks source link

Update the `storage` solidity contract used in testing #536

Closed m-Peter closed 2 weeks ago

m-Peter commented 2 weeks ago

Description

This will allow us to test stuff about precompiled calls and build a full-fledged EVM state.

For contributor use:

Summary by CodeRabbit

coderabbitai[bot] commented 2 weeks ago

Walkthrough

The pull request introduces significant modifications to a smart contract and its associated tests. Key changes include an increased gas limit for contract deployment, updates to the bytecode and Solidity contract to enhance functionality, and adjustments in expected values for various tests related to gas consumption and transaction fees. Additionally, new functions and events are added to the contract, improving its interactivity and logging capabilities.

Changes

Files Change Summary
tests/e2e_web3js_test.go Increased gas limit for evmSign function in TestWeb3_E2E from 350,000 to 1,250,000.
tests/fixtures/storage.byte Significant modifications to smart contract bytecode, including altered function implementations and signatures, indicating changes in logic and interface.
tests/fixtures/storage.sol Added new constant, events, and functions to the Storage contract, enhancing logging and interactivity, including functions for retrieving blockchain information and verifying external contract interactions.
tests/fixtures/storageABI.json Updated ABI with new events and functions reflecting changes in the Storage contract, including NewStore, blockNumber, blockTime, and several verification functions.
tests/web3js/eth_deploy_contract_and_interact_test.js Adjusted expected values for gas usage in contract deployment tests, reflecting increased gas consumption.
tests/web3js/eth_filter_endpoints_test.js Updated expected transaction hash and modified transaction object properties in filter endpoint tests, indicating changes in expected outputs.
tests/web3js/eth_transaction_type_fees_test.js Revised expected values for balance assertions in transaction fee calculations, indicating changes in gas usage and balance outcomes across multiple test cases.

Possibly related PRs

🐰 In fields of code, we hop and play,
New functions and events brighten the day.
With gas limits raised, we leap so high,
Through smart contracts, we swiftly fly!
A joyful test, with values anew,
In the world of blockchain, we dance and pursue! 🌟

[!TIP]

Announcements - The review status is no longer posted as a separate comment when there are no actionable or nitpick comments. In such cases, the review status is included in the walkthrough comment. - We have updated our review workflow to use the Anthropic's Claude family of models. Please share any feedback in the [discussion post](https://discordapp.com/channels/1134356397673414807/1279579842131787838) on our Discord. - Possibly related PRs: Walkthrough comment now includes a list of potentially related PRs to help you recall past context. Please share any feedback in the [discussion post](https://discordapp.com/channels/1134356397673414807/1282535539299323995) on our Discord. - Suggested labels: CodeRabbit can now suggest labels by learning from your past PRs in the walkthrough comment. You can also provide custom labeling instructions in the UI or configuration file. - Possibly related PRs, automatic label suggestions based on past PRs, learnings, and possibly related issues require data opt-in (enabled by default).
Recent review details **Configuration used: CodeRabbit UI** **Review profile: CHILL**
Commits Files that changed from the base of the PR and between 452d42a783e4186b32de7fd87c11b0f50a2851cd and 0e5ff982d198e17a89e84d0709ac64a1380a897b.
Files selected for processing (7) * tests/e2e_web3js_test.go (1 hunks) * tests/fixtures/storage.byte (1 hunks) * tests/fixtures/storage.sol (3 hunks) * tests/fixtures/storageABI.json (3 hunks) * tests/web3js/eth_deploy_contract_and_interact_test.js (2 hunks) * tests/web3js/eth_filter_endpoints_test.js (2 hunks) * tests/web3js/eth_transaction_type_fees_test.js (4 hunks)
Additional comments not posted (38)
tests/fixtures/storage.sol (15)
`7-7`: **LGTM!** The constant address `cadenceArch` is correctly declared and initialized. --- `8-8`: **LGTM!** The event `NewStore` is correctly declared with appropriate indexed parameters to log storage operations. --- `9-9`: **LGTM!** The event `Calculated` is correctly declared with appropriate indexed and non-indexed parameters to log calculation results. --- `23-26`: **LGTM!** The function `storeWithLog` is correctly implemented and enhances transparency by emitting the `NewStore` event when a value is stored. --- `28-31`: **LGTM!** The function `storeButRevert` is correctly implemented and can be used to demonstrate a state change followed by a revert, which could be useful for testing error handling. --- `43-45`: **LGTM!** The view function `blockNumber` is correctly implemented and provides access to the current block number. --- `47-49`: **LGTM!** The view function `blockTime` is correctly implemented and provides access to the current block timestamp. --- `51-53`: **LGTM!** The view function `blockHash` is correctly implemented and provides access to the hash of a given block number. --- `55-57`: **LGTM!** The view function `random` is correctly implemented and provides access to the previous random number. --- `59-61`: **LGTM!** The view function `chainID` is correctly implemented and provides access to the current chain ID. --- `63-65`: **LGTM!** The function `destroy` is correctly implemented and can be used to terminate the contract and clean up any remaining funds. --- `75-80`: **LGTM!** The view function `verifyArchCallToRandomSource` is correctly implemented and can be used to interact with the external contract to retrieve a random source for a given block height. --- `82-87`: **LGTM!** The view function `verifyArchCallToRevertibleRandom` is correctly implemented and can be used to interact with the external contract to retrieve a revertible random number. --- `89-95`: **LGTM!** The view function `verifyArchCallToFlowBlockHeight` is correctly implemented and can be used to interact with the external contract to retrieve the Flow block height and verify that it matches the expected value. --- `97-103`: **LGTM!** The view function `verifyArchCallToVerifyCOAOwnershipProof` is correctly implemented and can be used to interact with the external contract to verify a COA ownership proof and verify that the result matches the expected value.
tests/web3js/eth_transaction_type_fees_test.js (4)
`70-70`: **Verify the significant increase in coinbase balance.** The assertion for `coinbaseBalance` has been updated from `55585700n` to `185858350n`, indicating a substantial increase in the coinbase balance after the transaction. Ensure that this change aligns with the expected behavior and is a result of intentional modifications to the transaction fees or the gas price. --- `24-24`: **Verify the significant increase in gas used.** The assertion for `rcp.gasUsed` has been updated from `338798n` to `1207193n`, indicating a substantial increase in the gas used for the transaction. Ensure that this change aligns with the expected behavior and is a result of intentional modifications to the contract or the blockchain environment. --- `30-30`: **Verify the decrease in sender's balance.** The assertion for `senderBalance` has been updated from `4999999999949180300n` to `4999999999818921050n`, indicating a decrease in the sender's balance after the transaction. Ensure that this change aligns with the expected behavior and is a result of intentional modifications to the transaction fees or the gas price.
Verification successful --- **Verified: The decrease in sender's balance is correct.** The change in the assertion for `senderBalance` from `4999999999949180300n` to `4999999999818921050n` accurately reflects the transaction cost, calculated using the `gasUsed` and `gasPrice`. This aligns with the expected behavior of the transaction. - The `gasUsed` is `1207193n`, and the transaction cost is calculated as `res.receipt.gasUsed * gasPrice`.
Scripts executed The following scripts were executed for the analysis: Script: ```shell #!/bin/bash # Description: Verify the sender balance change for the legacy transaction. # Test: Search for the legacy transaction and check the sender's balance before and after the transaction. # Expect: The balance difference should match the transaction cost (gasUsed * gasPrice). rg --type js -A 10 $'let res = await helpers.signAndSend' ``` Length of output: 12185 --- Script: ```shell #!/bin/bash # Description: Search for the gas used in the transaction within the eth_transaction_type_fees_test.js file. # Test: Look for the gas used in the transaction to verify the balance change. rg --type js -A 5 'gasUsed' tests/web3js/eth_transaction_type_fees_test.js ``` Length of output: 1414
--- Line range hint `127-225`: **Verify the increase in coinbase balance.** The assertion for `coinbaseBalance` has been updated from `190368385n` to `200403185n`, indicating an increase in the coinbase balance after the access list transaction. Ensure that this change aligns with the expected behavior and is a result of intentional modifications to the transaction fees or the gas price.
tests/web3js/eth_deploy_contract_and_interact_test.js (2)
`236-236`: **Verify the slight increase in gas estimate.** The assertion for `gasEstimate` has been updated from `27374n` to `27442n`, indicating a slight increase in the estimated gas required for the operation. Ensure that this change aligns with the expected behavior and is a result of intentional modifications to the contract or the blockchain environment.
Verification successful --- **Verified: The increase in gas estimate is intentional.** The change in the gas estimate from `27374n` to `27442n` aligns with recent updates to the storage contract used in testing. The commit history indicates modifications that likely impacted the gas estimation, confirming that the change is expected and intentional.
Scripts executed The following scripts were executed for the analysis: Script: ```shell #!/bin/bash # Description: Verify the gas estimate change for the contract interaction. # Test: Search for the contract interaction and check the gas estimate. # Expect: The gas estimate should match the updated assertion value. rg --type js -A 5 $'gasEstimate = await web3.eth.estimateGas' ``` Length of output: 1165 --- Script: ```shell #!/bin/bash # Check for recent changes in the contract code or configuration files that might affect gas estimation. # Find the contract file(s) being deployed in the test. rg --type js 'new web3.eth.Contract' -A 5 # Check the commit history for changes in the contract file(s) or configuration files. git log -p -n 5 -- tests/web3js/eth_deploy_contract_and_interact_test.js ``` Length of output: 14682
--- `21-21`: **Verify the significant increase in gas used for contract deployment.** The assertion for `rcp.gasUsed` has been updated from `338798n` to `1207193n`, indicating a substantial increase in the gas used for the contract deployment transaction. Ensure that this change aligns with the expected behavior and is a result of intentional modifications to the contract or the blockchain environment.
tests/web3js/eth_filter_endpoints_test.js (2)
`348-350`: **LGTM!** The change in the expected value of `txHashes[1]` is approved. --- Line range hint `401-412`: **LGTM!** The changes in the expected transaction object properties are approved.
tests/fixtures/storageABI.json (14)
`54-72`: **LGTM!** The `NewStore` event is correctly defined with appropriate parameter types. --- `80-98`: **LGTM!** The `blockHash` function is correctly defined with appropriate parameter and return types. --- `99-111`: **LGTM!** The `blockNumber` function is correctly defined with an appropriate return type. --- `112-124`: **LGTM!** The `blockTime` function is correctly defined with an appropriate return type. --- `125-137`: **LGTM!** The `cadenceArch` function is correctly defined with an appropriate return type. --- `138-150`: **LGTM!** The `chainID` function is correctly defined with an appropriate return type. --- `158-164`: **LGTM!** The `destroy` function is correctly defined. --- `165-177`: **LGTM!** The `random` function is correctly defined with an appropriate return type. --- `204-216`: **LGTM!** The `storeButRevert` function is correctly defined with an appropriate parameter type. --- `217-229`: **LGTM!** The `storeWithLog` function is correctly defined with an appropriate parameter type. --- `253-272`: **LGTM!** The `verifyArchCallToFlowBlockHeight` function is correctly defined with appropriate parameter and return types. --- `273-291`: **LGTM!** The `verifyArchCallToRandomSource` function is correctly defined with appropriate parameter and return types. --- `292-304`: **LGTM!** The `verifyArchCallToRevertibleRandom` function is correctly defined with an appropriate return type. --- `305-338`: **LGTM!** The `verifyArchCallToVerifyCOAOwnershipProof` function is correctly defined with appropriate parameter and return types.
tests/e2e_web3js_test.go (1)
`153-153`: **Approve the gas limit increase, but verify the contract deployment.** The gas limit increase for the contract deployment transaction is approved. However, ensure that the contract deployment succeeds with this new gas limit. Run the following script to verify the contract deployment:
Verification successful --- **Verification successful: Contract deployment with increased gas limit is tested.** The test includes a check for errors after the contract deployment transaction, ensuring that the deployment succeeds with the new gas limit. - The line `require.NoError(t, err)` confirms that the deployment is expected to succeed, and the test will fail if it does not.
Scripts executed The following scripts were executed for the analysis: Script: ```shell #!/bin/bash # Description: Verify the contract deployment succeeds with the new gas limit. # Test: Search for the contract deployment transaction. Expect: A successful transaction. rg --type go -A 10 $'deployPayload, _, err := evmSign' ``` Length of output: 826
--- 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 your favorite social media?
Share - [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai) - [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai) - [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai) - [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code)
Tips ### Chat There are 3 ways to chat with [CodeRabbit](https://coderabbit.ai): - Review comments: Directly reply to a review comment made by CodeRabbit. Example: - `I pushed a fix in commit .` - `Generate unit testing code for this file.` - `Open a follow-up GitHub issue for this discussion.` - Files and specific lines of code (under the "Files changed" tab): Tag `@coderabbitai` in a new review comment at the desired location with your query. Examples: - `@coderabbitai generate unit testing code for this file.` - `@coderabbitai modularize this function.` - PR comments: Tag `@coderabbitai` in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples: - `@coderabbitai generate interesting stats about this repository and render them as a table.` - `@coderabbitai show all the console.log statements in this repository.` - `@coderabbitai read src/utils.ts and generate unit testing code.` - `@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.` - `@coderabbitai help me debug CodeRabbit configuration file.` Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. ### CodeRabbit Commands (Invoked using PR comments) - `@coderabbitai pause` to pause the reviews on a PR. - `@coderabbitai resume` to resume the paused reviews. - `@coderabbitai review` to trigger an incremental review. This is useful when automatic reviews are disabled for the repository. - `@coderabbitai full review` to do a full review from scratch and review all the files again. - `@coderabbitai summary` to regenerate the summary of the PR. - `@coderabbitai resolve` resolve all the CodeRabbit review comments. - `@coderabbitai configuration` to show the current CodeRabbit configuration for the repository. - `@coderabbitai help` to get help. ### Other keywords and placeholders - Add `@coderabbitai ignore` anywhere in the PR description to prevent this PR from being reviewed. - Add `@coderabbitai summary` to generate the high-level summary at a specific location in the PR description. - Add `@coderabbitai` anywhere in the PR title to generate the title automatically. ### CodeRabbit Configuration File (`.coderabbit.yaml`) - You can programmatically configure CodeRabbit by adding a `.coderabbit.yaml` file to the root of your repository. - Please see the [configuration documentation](https://docs.coderabbit.ai/guides/configure-coderabbit) for more information. - If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: `# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json` ### Documentation and Community - Visit our [Documentation](https://coderabbit.ai/docs) for detailed information on how to use CodeRabbit. - Join our [Discord Community](https://discord.com/invite/GsXnASn26c) to get help, request features, and share feedback. - Follow us on [X/Twitter](https://twitter.com/coderabbitai) for updates and announcements.