fix: post zero priority fee to avoid gas price bump failure in zetacore#3933
fix: post zero priority fee to avoid gas price bump failure in zetacore#3933
Conversation
…re (#3930) * fix gas price bump failure by posting zero priority fee * directly use 0 of uint64 for priority fee
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThe changes hardcode the priority fee to zero in the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Observer
participant PostVoteGasPrice
Client->>Observer: PostGasPrice()
Note right of Observer: Priority fee set to zero (legacy tx)
Observer->>PostVoteGasPrice: PostVoteGasPrice(gasPrice, 0)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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)
Other keywords and placeholders
Documentation and Community
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #3933 +/- ##
===========================================
- Coverage 64.92% 64.84% -0.08%
===========================================
Files 471 471
Lines 34413 34417 +4
===========================================
- Hits 22343 22319 -24
- Misses 11036 11069 +33
+ Partials 1034 1029 -5
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
zetaclient/chains/evm/observer/observer_gas.go (1)
25-28: Consider removing commented code to reduce technical debt.The commented implementation preserves the original logic but creates maintenance overhead. Consider either removing this code entirely or documenting a clear timeline for restoration if this change is intended to be temporary.
If this is a permanent change, apply this diff:
- // PRIORITY FEE (EIP-1559) - // priorityFee, err := ob.determinePriorityFee(ctx) - // if err != nil { - // return errors.Wrap(err, "unable to determine priority fee") - // } + // PRIORITY FEE (EIP-1559) - Disabled per issue #3221
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
zetaclient/chains/evm/observer/observer_gas.go(3 hunks)zetaclient/chains/evm/observer/observer_gas_test.go(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.go`: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
zetaclient/chains/evm/observer/observer_gas.gozetaclient/chains/evm/observer/observer_gas_test.go
🪛 GitHub Check: codecov/patch
zetaclient/chains/evm/observer/observer_gas.go
[warning] 48-48: zetaclient/chains/evm/observer/observer_gas.go#L48
Added line #L48 was not covered by tests
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: start-e2e-test / e2e
🔇 Additional comments (2)
zetaclient/chains/evm/observer/observer_gas.go (1)
18-22: Architectural decision documented with clear rationale.The hardcoded priority fee approach is well-documented with relevant issue links and context. This represents a deliberate shift from dynamic EIP-1559 transaction support to legacy transactions to prevent gas price bump failures.
zetaclient/chains/evm/observer/observer_gas_test.go (1)
27-31: Mock expectations appropriately relaxed for simplified gas price logic.The addition of
.Maybe()to the mock calls correctly reflects thatPostGasPriceno longer deterministically calls these methods since priority fee determination is now hardcoded.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
zetaclient/chains/evm/observer/observer_gas.go(3 hunks)zetaclient/chains/evm/observer/observer_gas_test.go(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.go`: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
zetaclient/chains/evm/observer/observer_gas_test.gozetaclient/chains/evm/observer/observer_gas.go
🪛 GitHub Check: codecov/patch
zetaclient/chains/evm/observer/observer_gas.go
[warning] 48-48: zetaclient/chains/evm/observer/observer_gas.go#L48
Added line #L48 was not covered by tests
🔇 Additional comments (3)
zetaclient/chains/evm/observer/observer_gas_test.go (1)
27-27: LGTM: Mock expectations appropriately made optional.The addition of
.Maybe()to the mock expectations correctly reflects that these RPC calls may no longer be invoked due to the hardcoded priority fee implementation.Also applies to: 30-31
zetaclient/chains/evm/observer/observer_gas.go (2)
18-22: LGTM: Clear documentation of the hardcoded approach.The comments effectively explain the rationale for hardcoding the priority fee to zero, with appropriate references to the underlying issue and related code.
37-37: LGTM: Correctly updated method call parameter.The change from
priorityFee.Uint64()topriorityFeecorrectly reflects the new type (uint64 instead of *big.Int).
3 participants
#3931
Update the change on develop as well.
Summary by CodeRabbit