Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix effective gas price for rpc getTransactionReceipt #4844

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from

Conversation

Frozen
Copy link
Contributor

@Frozen Frozen commented Feb 7, 2025

Fixed issue with effective gas price panic. Covered wth tests.

@@ -390,6 +417,11 @@ func NewStakingTxReceipt(
return nil, err
}

effectiveGasPrice := *big.NewInt(0)
if receipt.EffectiveGasPrice != nil {
Copy link
Contributor

@sophoah sophoah Feb 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add an else clause to prevent the RPC to returns an effective gas price of 0. Not sure what edge case we are going to have in this case.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @Frozen I've tested against old and new transactions against testnet RPC and found that rn we still have 2 behaviors:

  • old transactions have "effectiveGasPrice": 0
  • new transactions have effectiveGasPrice==gasUsed

Can we make the same behavior effectiveGasPrice==gasUsed for the old transactions?

@sophoah
Copy link
Contributor

sophoah commented Feb 18, 2025

@Frozen

@Frozen Frozen force-pushed the fix/rpc-effetive-gas-price branch from 460c1a6 to 6f126d8 Compare February 19, 2025 20:19
@Frozen
Copy link
Contributor Author

Frozen commented Feb 25, 2025

returns Nil by default

@sophoah sophoah changed the title Fix effetive gas price for rpc getTransactionReceipt Fix effective gas price for rpc getTransactionReceipt Feb 26, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants