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

allow empty maxFeePerBlobGas for eth_call #6731

Merged
merged 16 commits into from
Mar 19, 2024

Conversation

macfarla
Copy link
Contributor

@macfarla macfarla commented Mar 14, 2024

PR description

  • allow for maxFeePerBlobGas to be empty even if versioned hashes are present
  • if maxFeePerBlobGas is empty or zero, allow user's balance to be exceeded (same behaviour as other gas params)

Fixed Issue(s)

fixes #6709

Thanks for sending a pull request! Have you done the following?

  • Checked out our contribution guidelines?
  • Considered documentation and added the doc-change-required label to this PR if updates are required.
  • Considered the changelog and included an update if required.
  • For database changes (e.g. KeyValueSegmentIdentifier) considered compatibility and performed forwards and backwards compatibility tests

Most advanced CI tests are deferred until PR approval, but you could:

  • locally run all unit tests via: ./gradlew build
  • locally run all acceptance tests via: ./gradlew acceptanceTest
  • locally run all integration tests via: ./gradlew integrationTest
  • locally run all reference tests via: ./gradlew ethereum:referenceTests:referenceTests
  • locally run hive tests engine-cancun suite

@macfarla macfarla marked this pull request as draft March 14, 2024 23:58
Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
@macfarla macfarla requested a review from fab-10 March 15, 2024 05:28
@macfarla
Copy link
Contributor Author

@fab-10 does it makes sense to add the blob gas checks under the isAllowExceedingBalance flag - so don't check blobGasPrice or maxFeePerBlobGas if isAllowExceedingBalance is true

@macfarla macfarla marked this pull request as ready for review March 15, 2024 05:56
@fab-10
Copy link
Contributor

fab-10 commented Mar 15, 2024

@fab-10 does it makes sense to add the blob gas checks under the isAllowExceedingBalance flag - so don't check blobGasPrice or maxFeePerBlobGas if isAllowExceedingBalance is true

yes it makes sense to me

Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
@macfarla
Copy link
Contributor Author

@fab-10 does it makes sense to add the blob gas checks under the isAllowExceedingBalance flag - so don't check blobGasPrice or maxFeePerBlobGas if isAllowExceedingBalance is true

yes it makes sense to me

I have made this change. could you review @fab-10 - thanks!

@macfarla macfarla requested review from jflo and fab-10 March 18, 2024 07:31
@macfarla
Copy link
Contributor Author

engine-cancun hive tests passing locally except for occasional known flakiness from Invalid Missing Ancestor Syncing ReOrg, StateRoot, EmptyTxs=False, CanonicalReOrg=True, Invalid P9 (Cancun)

@macfarla macfarla merged commit cc20169 into hyperledger:main Mar 19, 2024
42 checks passed
@macfarla macfarla deleted the eth-call-blob-gas-optional branch March 19, 2024 06:11
amsmota pushed a commit to Citi/besu that referenced this pull request Apr 16, 2024
* allow empty blob gas for eth_call

* handle empty maxFeePerBlobGas by setting to blobBaseFee if empty

* set allowExceedingBalance if blobGas not specified

* added a test case for strict with zero blob gas

Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>

---------

Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
Co-authored-by: Justin Florentine <justin+github@florentine.us>
Signed-off-by: amsmota <antonio.mota@citi.com>
amsmota pushed a commit to Citi/besu that referenced this pull request Apr 16, 2024
* allow empty blob gas for eth_call

* handle empty maxFeePerBlobGas by setting to blobBaseFee if empty

* set allowExceedingBalance if blobGas not specified

* added a test case for strict with zero blob gas

Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>

---------

Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
Co-authored-by: Justin Florentine <justin+github@florentine.us>
Signed-off-by: amsmota <antonio.mota@citi.com>
matthew1001 pushed a commit to kaleido-io/besu that referenced this pull request Jun 7, 2024
* allow empty blob gas for eth_call

* handle empty maxFeePerBlobGas by setting to blobBaseFee if empty

* set allowExceedingBalance if blobGas not specified

* added a test case for strict with zero blob gas

Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>

---------

Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
Co-authored-by: Justin Florentine <justin+github@florentine.us>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

make maxBlobFeePerGas optional for eth_call with blob versionedHashes
3 participants