Calculate the `CumulativeGasUsed` field for transaction receipts

m-Peter commented 2 months ago

Closes: https://github.com/onflow/flow-evm-gateway/issues/357

Description

We calculate this field when consuming the Cadence events, as it is not included in the EVM.TransactionExecuted event. I am not sure if it is easy to add it there.

For contributor use:

[x] Targeted PR against master branch
[x] Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
[x] Code follows the standards mentioned here.
[x] Updated relevant documentation
[x] Re-reviewed Files changed in the Github PR explorer
[x] Added appropriate labels

Summary by CodeRabbit

New Features
- Added logic to calculate cumulative gas usage for transaction receipts.
Tests
- Enhanced tests to validate gas usage metrics for contract deployment and batch transactions.
- Updated assertions to verify gas usage and ensure consistency with cumulative values.

coderabbitai[bot] commented 2 months ago

[!NOTE] Currently processing new changes in this PR. This may take a few minutes, please wait...

Commits
Files that changed from the base of the PR and between d6628b3f0354c6ec051a2a6aa0fb188f54459f59 and eaa4956a28b59fae92b9a9f46b38a1c64ba5c3f9.

Files selected for processing (1)
* tests/web3js/eth_non_interactive_test.js (2 hunks)
 _______________________________________________________________________________
< Why do we never have time to do it right, but always have time to do it over? >
 -------------------------------------------------------------------------------
  \
   \   (\__/)
       (•ㅅ•)
       / 　 づ

Walkthrough

The changes enhance the CadenceEvents class by implementing logic to accurately compute cumulativeGasUsed for transaction receipts within the Transactions method. Additionally, the test suite was updated to verify this calculation, ensuring precise tracking of gas usage across batch transactions.

Changes

File	Change Summary
models/events.go	Enhanced the `Transactions` method to calculate `cumulativeGasUsed` for transaction receipts.
models/transaction.go	Removed `CumulativeGasUsed` from `gethReceipt` struct for simplification.
tests/web3js/eth_batch_retrieval_test.js	Updated test function for clarity and added logic to validate `cumulativeGasUsed` for each transaction.
tests/web3js/eth_deploy_contract_and_interact_test.js	Added assertions to verify `gasUsed` and `cumulativeGasUsed` during contract deployment.
tests/web3js/eth_non_interactive_test.js	Modified assertions to ensure gas usage consistency based on transaction receipt details.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant CadenceEvents
    participant Web3Provider

    User->>CadenceEvents: Call Transactions method
    CadenceEvents->>Web3Provider: Fetch batch transactions
    Web3Provider-->>CadenceEvents: Return transactions and receipts
    loop Calculate cumulativeGasUsed
        CadenceEvents->>CadenceEvents: Calculate cumulativeGasUsed for each receipt
    end
    CadenceEvents-->>User: Return transactions with cumulativeGasUsed

Assessment against linked issues

Objective	Addressed	Explanation
Calculate properly the `CumulativeGasUsed` field for transaction receipts (#357)	✅

Poem

In the world of code, where gas is tall,
Transactions gather, big and small.
With gas calculated, cumulative and wise,
Our tests now see through clearer eyes.
Batch by batch, receipts align,
In CadenceEvents, all is fine. 🚀

[!TIP]

CodeRabbit can use your project's `golangci-lint` configuration to improve the quality of Go code reviews.
Add a [configuration file](https://golangci-lint.run/usage/configuration/) to your project to customize how CodeRabbit runs `golangci-lint`.

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 as 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. Additionally, you can add `@coderabbitai ignore` anywhere in the PR description to prevent this PR from being reviewed. ### 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.

sideninja commented 2 months ago

Nice work, but we don't need this anymore since it was added to the receipt in flow-go latest PRs.

sideninja commented 2 months ago

Will convert to draft until we update flow-go with those PRs

m-Peter commented 2 months ago

Nice work, but we don't need this anymore since it was added to the receipt in flow-go latest PRs.

Thanks for the flag @sideninja . I didn't know this was being worked on the flow-go side :innocent: I did found the addition of CumulativeGasUsed field in the Receipt (https://github.com/onflow/flow-go/blob/08e28b9bd69e2fdcb0e21fa2cb58b441bc075e9d/fvm/evm/types/result.go#L138), but we can't make use of that in the Gateway. It has to be included in the EVM.TransactionExecuted event as well: https://github.com/onflow/flow-go/blob/master/fvm/evm/stdlib/contract.cdc#L39-L75. Or am I missing something? :thinking:

sideninja commented 2 months ago

Nice work, but we don't need this anymore since it was added to the receipt in flow-go latest PRs.

Thanks for the flag @sideninja . I didn't know this was being worked on the flow-go side 😇 I did found the addition of CumulativeGasUsed field in the Receipt (https://github.com/onflow/flow-go/blob/08e28b9bd69e2fdcb0e21fa2cb58b441bc075e9d/fvm/evm/types/result.go#L138), but we can't make use of that in the Gateway. It has to be included in the EVM.TransactionExecuted event as well: https://github.com/onflow/flow-go/blob/master/fvm/evm/stdlib/contract.cdc#L39-L75. Or am I missing something? 🤔

Yeah it was just added so hard for you to know it. If it's not in the execution result it should be I guess. Maybe we can add it there.

sideninja commented 2 months ago

Can I merge @m-Peter ?

m-Peter commented 2 months ago

@sideninja Yeah, it's good to go. I will follow-up with a PR, for the leftover comments :pray:

onflow / flow-evm-gateway