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

Apply deliver hook gas limit for receipt side effects #1911

Closed
wants to merge 2 commits into from

Conversation

stevenlanders
Copy link
Contributor

Describe your changes and provide context

  • set transient receipt was being governed by the transaction gas limit
  • this gas limit isn't actually charged against the transaction (receipt already rendered)
  • the change is to make this limited by the hook gas limit which is configured

Testing performed to validate your change

  • integration test passes

Copy link

codecov bot commented Oct 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 61.56%. Comparing base (32c89cd) to head (01364da).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1911      +/-   ##
==========================================
+ Coverage   61.36%   61.56%   +0.20%     
==========================================
  Files         263      263              
  Lines       23306    23306              
==========================================
+ Hits        14301    14349      +48     
+ Misses       8001     7951      -50     
- Partials     1004     1006       +2     
Files with missing lines Coverage Δ
app/receipt.go 79.53% <100.00%> (ø)

... and 3 files with indirect coverage changes

@stevenlanders
Copy link
Contributor Author

closing in favor of another option

@stevenlanders stevenlanders deleted the change-gas-ctx-for-receipt-updates branch October 29, 2024 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant