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

feat(zetaclient)!: Add support for EIP-1559 gas fees #2634

Merged
merged 15 commits into from
Aug 6, 2024

Conversation

swift1337
Copy link
Contributor

@swift1337 swift1337 commented Aug 5, 2024

This PR concludes EIP-1559 implementation for outbound transactions. The flow

  • Observers observe whether external EVMs support EIP-1559. If so, they fetch & vote for priorityFee (included in MsgVoteGasPrice)
  • Then, if priorityFee != 0, we create, sign, and broadcast ethtypes.DynamicFeeTx{} with GasTipCap instead of ethtypes.LegacyTx{}

Closes #1952. This is a rewrite of #2387

Summary by CodeRabbit

  • New Features

    • Added support for EIP-1559 gas fees, enhancing transaction fee management.
    • Improved gas management for Ethereum transactions, particularly for cross-chain operations.
  • Bug Fixes

    • Enhanced validation for gas parameters, ensuring compliance with EIP-1559 standards.
  • Tests

    • Expanded test coverage for outbound data creation and gas parameter handling, improving reliability and clarity.
  • Documentation

    • Updated changelog to reflect recent changes and enhancements.

@swift1337 swift1337 self-assigned this Aug 5, 2024
Copy link
Contributor

coderabbitai bot commented Aug 5, 2024

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

Walkthrough

The recent changes implement support for EIP-1559 gas fees within the ZetaClient, enhancing transaction fee management through a more sophisticated structure. This update includes the addition of a dedicated Gas struct for handling gas parameters and improvements to related functionalities, focusing on robust and clear processing of cross-chain transactions. The modifications signify a strategic shift towards modernizing transaction handling to comply with evolving blockchain standards.

Changes

Files Change Summary
changelog.md, zetaclient/chains/evm/signer/gas.go, zetaclient/chains/evm/signer/gas_test.go Added support for EIP-1559 gas fees, introducing a Gas struct and comprehensive validation and testing.
zetaclient/chains/evm/signer/outbound_data.go, zetaclient/chains/evm/signer/outbound_data_test.go Restructured OutboundData, integrating gas handling into new functions for improved clarity and error handling.
zetaclient/chains/evm/signer/signer.go, zetaclient/chains/evm/signer/signer_test.go Refactored transaction signing logic to utilize the new Gas struct, simplifying method signatures and enhancing tests.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant ZetaClient
    participant EVM
    User->>ZetaClient: Prepare EIP-1559 Transaction
    ZetaClient->>EVM: Sign Transaction with Gas Struct
    EVM->>ZetaClient: Transaction Signed
    ZetaClient->>User: Transaction Confirmation
Loading

Assessment against linked issues

Objective Addressed Explanation
Support EIP-1559 transactions with ZetaClient (#1952)

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
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • 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 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 for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@swift1337 swift1337 added the zetaclient Issues related to ZetaClient label Aug 5, 2024
@swift1337 swift1337 changed the title feat(zetaclient)!: Add supportf for EIP-1559 gas fees feat(zetaclient)!: Add support for EIP-1559 gas fees Aug 5, 2024
Copy link

codecov bot commented Aug 5, 2024

Codecov Report

Attention: Patch coverage is 79.43925% with 44 lines in your changes missing coverage. Please review.

Project coverage is 70.63%. Comparing base (c3a5428) to head (7870420).

Files Patch % Lines
zetaclient/chains/evm/signer/outbound_data.go 73.41% 15 Missing and 6 partials ⚠️
zetaclient/chains/evm/signer/signer.go 82.66% 10 Missing and 3 partials ⚠️
zetaclient/chains/evm/signer/gas.go 83.05% 10 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2634      +/-   ##
===========================================
+ Coverage    70.55%   70.63%   +0.07%     
===========================================
  Files          335      336       +1     
  Lines        18373    18453      +80     
===========================================
+ Hits         12963    13034      +71     
- Misses        4824     4829       +5     
- Partials       586      590       +4     
Files Coverage Δ
zetaclient/chains/evm/observer/observer_gas.go 39.47% <100.00%> (ø)
zetaclient/chains/evm/signer/gas.go 83.05% <83.05%> (ø)
zetaclient/chains/evm/signer/signer.go 52.93% <82.66%> (+1.75%) ⬆️
zetaclient/chains/evm/signer/outbound_data.go 71.76% <73.41%> (+1.39%) ⬆️

@swift1337 swift1337 marked this pull request as ready for review August 5, 2024 15:36
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 9

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 3b0431e and 93c930c.

Files selected for processing (7)
  • changelog.md (1 hunks)
  • zetaclient/chains/evm/signer/gas.go (1 hunks)
  • zetaclient/chains/evm/signer/gas_test.go (1 hunks)
  • zetaclient/chains/evm/signer/outbound_data.go (2 hunks)
  • zetaclient/chains/evm/signer/outbound_data_test.go (1 hunks)
  • zetaclient/chains/evm/signer/signer.go (23 hunks)
  • zetaclient/chains/evm/signer/signer_test.go (10 hunks)
Additional context used
Path-based instructions (6)
zetaclient/chains/evm/signer/gas.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

zetaclient/chains/evm/signer/outbound_data_test.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

zetaclient/chains/evm/signer/gas_test.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

zetaclient/chains/evm/signer/outbound_data.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

zetaclient/chains/evm/signer/signer_test.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

zetaclient/chains/evm/signer/signer.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

Learnings (1)
zetaclient/chains/evm/signer/signer_test.go (3)
Learnt from: ws4charlie
PR: zeta-chain/node#2411
File: zetaclient/orchestrator/chain_activate.go:116-181
Timestamp: 2024-07-05T00:02:36.493Z
Learning: The `CreateSignerObserverEVM` function in `zetaclient/orchestrator/chain_activate.go` is covered by unit tests in `zetaclient/orchestrator/chain_activate_test.go`.
Learnt from: ws4charlie
PR: zeta-chain/node#2411
File: zetaclient/orchestrator/orchestrator.go:192-217
Timestamp: 2024-07-04T23:46:38.428Z
Learning: The `GetUpdatedSigner` method in `zetaclient/orchestrator/orchestrator.go` is covered by unit tests in `zetaclient/orchestrator/chain_activate_test.go` and `zetaclient/orchestrator/orchestrator_test.go`.
Learnt from: ws4charlie
PR: zeta-chain/node#2411
File: zetaclient/orchestrator/chain_activate.go:184-247
Timestamp: 2024-07-05T00:02:31.446Z
Learning: The `CreateSignerObserverBTC` function in `zetaclient/orchestrator/chain_activate.go` is covered by unit tests in `zetaclient/orchestrator/chain_activate_test.go`.
GitHub Check: codecov/patch
zetaclient/chains/evm/signer/gas.go

[warning] 38-43: zetaclient/chains/evm/signer/gas.go#L38-L43
Added lines #L38 - L43 were not covered by tests

zetaclient/chains/evm/signer/outbound_data.go

[warning] 64-64: zetaclient/chains/evm/signer/outbound_data.go#L64
Added line #L64 was not covered by tests


[warning] 85-85: zetaclient/chains/evm/signer/outbound_data.go#L85
Added line #L85 was not covered by tests


[warning] 90-92: zetaclient/chains/evm/signer/outbound_data.go#L90-L92
Added lines #L90 - L92 were not covered by tests


[warning] 95-96: zetaclient/chains/evm/signer/outbound_data.go#L95-L96
Added lines #L95 - L96 were not covered by tests


[warning] 100-103: zetaclient/chains/evm/signer/outbound_data.go#L100-L103
Added lines #L100 - L103 were not covered by tests


[warning] 111-111: zetaclient/chains/evm/signer/outbound_data.go#L111
Added line #L111 was not covered by tests


[warning] 142-142: zetaclient/chains/evm/signer/outbound_data.go#L142
Added line #L142 was not covered by tests

zetaclient/chains/evm/signer/signer.go

[warning] 182-182: zetaclient/chains/evm/signer/signer.go#L182
Added line #L182 was not covered by tests


[warning] 217-217: zetaclient/chains/evm/signer/signer.go#L217
Added line #L217 was not covered by tests


[warning] 231-240: zetaclient/chains/evm/signer/signer.go#L231-L240
Added lines #L231 - L240 were not covered by tests


[warning] 426-426: zetaclient/chains/evm/signer/signer.go#L426
Added line #L426 was not covered by tests


[warning] 519-519: zetaclient/chains/evm/signer/signer.go#L519
Added line #L519 was not covered by tests


[warning] 528-528: zetaclient/chains/evm/signer/signer.go#L528
Added line #L528 was not covered by tests


[warning] 543-543: zetaclient/chains/evm/signer/signer.go#L543
Added line #L543 was not covered by tests


[warning] 554-554: zetaclient/chains/evm/signer/signer.go#L554
Added line #L554 was not covered by tests


[warning] 562-562: zetaclient/chains/evm/signer/signer.go#L562
Added line #L562 was not covered by tests


[warning] 576-576: zetaclient/chains/evm/signer/signer.go#L576
Added line #L576 was not covered by tests


[warning] 592-592: zetaclient/chains/evm/signer/signer.go#L592
Added line #L592 was not covered by tests

Additional comments not posted (40)
zetaclient/chains/evm/signer/gas.go (6)

13-16: Constants for gas limits look good.

The defined constants for minGasLimit and maxGasLimit are reasonable and appropriate for EVM transactions.


18-34: Struct definition looks good.

The Gas struct is well-defined with appropriate fields and helpful comments.


36-46: Validation function is well-implemented but lacks test coverage.

The validate function correctly checks for invalid states in the Gas struct. However, the added lines are not covered by tests.

Ensure that test cases are added to cover the validate function.

Tools
GitHub Check: codecov/patch

[warning] 38-43: zetaclient/chains/evm/signer/gas.go#L38-L43
Added lines #L38 - L43 were not covered by tests


49-55: Legacy check function is well-implemented.

The isLegacy function correctly determines if the transaction is a legacy transaction based on the PriorityFee.


57-96: Gas creation function is well-implemented but ensure comprehensive test coverage.

The makeGasFromCCTX function correctly creates a Gas struct from a cross-chain transaction with proper error handling and logging. However, ensure that test cases cover all branches of the function.

Ensure comprehensive test coverage for all branches of the makeGasFromCCTX function.


99-113: Big integer parsing function is well-implemented.

The bigIntFromString function correctly parses a string into a big.Int and handles various edge cases appropriately.

zetaclient/chains/evm/signer/outbound_data_test.go (8)

27-56: Success test case is well-implemented.

The "success" test case follows the Arrange-Act-Assert pattern and includes comprehensive assertions to verify the integrity of the created outbound data.


59-72: Pending revert test case is well-implemented.

The "pending revert" test case correctly sets up the transaction status and includes assertions to verify the expected behavior.


74-86: Pending outbound test case is well-implemented.

The "pending outbound" test case correctly sets up the transaction status and includes assertions to verify the expected behavior.


89-98: Skip inbound test case is well-implemented.

The "skip inbound" test case correctly sets up the transaction status and includes assertions to verify the expected behavior.


102-112: Skip aborted test case is well-implemented.

The "skip aborted" test case correctly sets up the transaction status and includes assertions to verify the expected behavior.


115-125: Invalid gas price test case is well-implemented.

The "invalid gas price" test case correctly sets up the invalid gas price and includes assertions to verify the expected error message.


127-136: Unknown chain test case is well-implemented.

The "unknown chain" test case correctly sets up the unknown chain and includes assertions to verify the expected error message.


138-147: No outbound params test case is well-implemented.

The "no outbound params" test case correctly sets up the missing parameters and includes assertions to verify the expected error message.

zetaclient/chains/evm/signer/gas_test.go (11)

32-41: Legacy: gas is too low test case is well-implemented.

The "legacy: gas is too low" test case correctly sets up the gas limit and includes assertions to verify the expected behavior.


44-53: London: gas is too low test case is well-implemented.

The "london: gas is too low" test case correctly sets up the gas limit and includes assertions to verify the expected behavior.


56-65: Pre London gas logic test case is well-implemented.

The "pre London gas logic" test case correctly sets up the gas parameters and includes assertions to verify the expected behavior.


68-77: Post London gas logic test case is well-implemented.

The "post London gas logic" test case correctly sets up the gas parameters and includes assertions to verify the expected behavior.


80-89: Gas is too high, force to the ceiling test case is well-implemented.

The "gas is too high, force to the ceiling" test case correctly sets up the gas limit and includes assertions to verify the expected behavior.


92-95: Priority fee is invalid test case is well-implemented.

The "priority fee is invalid" test case correctly sets up the invalid priority fee and includes assertions to verify the expected error message.


97-100: Priority fee is negative test case is well-implemented.

The "priority fee is negative" test case correctly sets up the negative priority fee and includes assertions to verify the expected error message.


102-105: Gas price is less than priority fee test case is well-implemented.

The "gasPrice is less than priorityFee" test case correctly sets up the gas price and priority fee and includes assertions to verify the expected error message.


107-110: Gas price is invalid test case is well-implemented.

The "gasPrice is invalid" test case correctly sets up the invalid gas price and includes assertions to verify the expected error message.


125-129: Utility function for asserting gas equality is well-implemented.

The assertGasEquals function correctly compares the expected and actual Gas structs, ensuring accurate comparisons of the fields.


131-134: **Utility function for Gwei

zetaclient/chains/evm/signer/outbound_data.go (2)

Line range hint 24-43:
LGTM!

The OutboundData struct has been updated with a new Gas type and reorganization of fields. These changes enhance clarity and maintainability.


44-137: Ensure comprehensive test coverage.

The NewOutboundData function has been updated with enhanced error handling and logging. Ensure that all new lines, especially error handling paths, are covered by tests.

Would you like assistance in generating additional unit tests to cover the new lines?

Verification successful

Ensure comprehensive test coverage.

The NewOutboundData function has been updated with enhanced error handling and logging. The function is well-integrated and covered by tests in signer_test.go and outbound_data_test.go, ensuring that all new lines, especially error handling paths, are tested.

  • File: zetaclient/chains/evm/signer/signer_test.go
  • File: zetaclient/chains/evm/signer/outbound_data_test.go
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of `NewOutboundData` function in the codebase.

# Test: Search for the function usage. Expect: Occurrences of the new signature.
rg --type go -A 5 $'NewOutboundData'

Length of output: 6557

Tools
GitHub Check: codecov/patch

[warning] 64-64: zetaclient/chains/evm/signer/outbound_data.go#L64
Added line #L64 was not covered by tests


[warning] 85-85: zetaclient/chains/evm/signer/outbound_data.go#L85
Added line #L85 was not covered by tests


[warning] 90-92: zetaclient/chains/evm/signer/outbound_data.go#L90-L92
Added lines #L90 - L92 were not covered by tests


[warning] 95-96: zetaclient/chains/evm/signer/outbound_data.go#L95-L96
Added lines #L95 - L96 were not covered by tests


[warning] 100-103: zetaclient/chains/evm/signer/outbound_data.go#L100-L103
Added lines #L100 - L103 were not covered by tests


[warning] 111-111: zetaclient/chains/evm/signer/outbound_data.go#L111
Added line #L111 was not covered by tests

zetaclient/chains/evm/signer/signer_test.go (10)

Line range hint 200-204:
LGTM!

The invocation of NewOutboundData in TestSigner_SignOutbound has been updated. The changes are consistent with the new function signature.


Line range hint 236-240:
LGTM!

The invocation of NewOutboundData in TestSigner_SignRevertTx has been updated. The changes are consistent with the new function signature.


Line range hint 276-280:
LGTM!

The invocation of NewOutboundData in TestSigner_SignCancelTx has been updated. The changes are consistent with the new function signature.


Line range hint 316-320:
LGTM!

The invocation of NewOutboundData in TestSigner_SignWithdrawTx has been updated. The changes are consistent with the new function signature.


Line range hint 354-358:
LGTM!

The invocation of NewOutboundData in TestSigner_SignCommandTx has been updated. The changes are consistent with the new function signature.


Line range hint 401-405:
LGTM!

The invocation of NewOutboundData in TestSigner_SignERC20WithdrawTx has been updated. The changes are consistent with the new function signature.


Line range hint 442-446:
LGTM!

The invocation of NewOutboundData in TestSigner_BroadcastOutbound has been updated. The changes are consistent with the new function signature.


579-583: LGTM!

The makeCtx function has been updated to include an additional chain in the context setup. This change enhances the testing scope.


Line range hint 499-503:
LGTM!

The invocation of NewOutboundData in TestSigner_SignWhitelistERC20Cmd has been updated. The changes are consistent with the new function signature.


Line range hint 544-548:
LGTM!

The invocation of NewOutboundData in TestSigner_SignMigrateTssFundsCmd has been updated. The changes are consistent with the new function signature.

zetaclient/chains/evm/signer/signer.go (2)

243-247: LGTM!

The Broadcast method has been renamed to broadcast. The changes are consistent with the intended usage.


763-766: Ensure comprehensive test coverage.

The SignCommandTx method has been updated to use the new Gas structure. Ensure that all new lines,

changelog.md (1)

49-49: Ensure consistency in changelog entry formatting.

The new entry follows the correct format and provides a clear description of the new feature. No issues found.

zetaclient/chains/evm/signer/outbound_data.go Outdated Show resolved Hide resolved
zetaclient/chains/evm/signer/outbound_data.go Outdated Show resolved Hide resolved
zetaclient/chains/evm/signer/outbound_data.go Show resolved Hide resolved
zetaclient/chains/evm/signer/signer.go Show resolved Hide resolved
zetaclient/chains/evm/signer/signer.go Show resolved Hide resolved
zetaclient/chains/evm/signer/signer.go Show resolved Hide resolved
zetaclient/chains/evm/signer/signer.go Show resolved Hide resolved
zetaclient/chains/evm/signer/signer.go Show resolved Hide resolved
zetaclient/chains/evm/signer/signer.go Show resolved Hide resolved
@lumtis lumtis added UPGRADE_TESTS Run make start-upgrade-tests ADMIN_TESTS Run make start-admin-tests labels Aug 5, 2024
Copy link
Member

@lumtis lumtis left a comment

Choose a reason for hiding this comment

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

Had a first look, looks good but will need to check more in details on the core logic, added upgrade and admin tests for the PR CI

changelog.md Outdated Show resolved Hide resolved
zetaclient/chains/evm/signer/gas.go Outdated Show resolved Hide resolved
zetaclient/chains/evm/signer/outbound_data.go Outdated Show resolved Hide resolved
Copy link
Member

@lumtis lumtis left a comment

Choose a reason for hiding this comment

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

The protocol still pay the gas with the formula gasPrice*gasLimit

To maintain solvency, we need to ensure the fee paid in the protocol is alway higher or equal to the actual fee paid by the TSS

Do we ensure this invarient is maintained?

zetaclient/chains/evm/signer/gas.go Show resolved Hide resolved
zetaclient/chains/evm/signer/gas.go Show resolved Hide resolved
zetaclient/chains/evm/signer/gas.go Outdated Show resolved Hide resolved
@swift1337
Copy link
Contributor Author

The protocol still pay the gas with the formula gasPrice*gasLimit

To maintain solvency, we need to ensure the fee paid in the protocol is alway higher or equal to the actual fee paid by the TSS

Do we ensure this invarient is maintained?

@ws4charlie what do you think? As of now:

  • cctx.gasPrice = evm.SuggestGasPrice()
  • cctx.priorityFee = evm.SuggestGasTipCap() || zero
  • but based on your suggestion, for EIP-1559, maxGasFeeCap = 2*baseFee +priorityFee = 2*(gasPrice - priorityFee) + priorityFee = 2*gasPrice - priorityFee, so technically indeed maxGasFeeCap might be greater than gasPrice.

Doesn't ETH RPC take London fork into account when executing SuggestGasPrice() aka eth_gasPrice?

Copy link
Contributor

@ws4charlie ws4charlie left a comment

Choose a reason for hiding this comment

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

lgtm

@ws4charlie
Copy link
Contributor

The protocol still pay the gas with the formula gasPrice*gasLimit
To maintain solvency, we need to ensure the fee paid in the protocol is alway higher or equal to the actual fee paid by the TSS
Do we ensure this invarient is maintained?

@ws4charlie what do you think? As of now:

  • cctx.gasPrice = evm.SuggestGasPrice()
  • cctx.priorityFee = evm.SuggestGasTipCap() || zero
  • but based on your suggestion, for EIP-1559, maxGasFeeCap = 2*baseFee +priorityFee = 2*(gasPrice - priorityFee) + priorityFee = 2*gasPrice - priorityFee, so technically indeed maxGasFeeCap might be greater than gasPrice.

Doesn't ETH RPC take London fork into account when executing SuggestGasPrice() aka eth_gasPrice?

I think maxGasFeeCap can be > gasPrice. gasPrice equals to baseFee+priorityFee , while the maxGasFeeCap equals to baseFee*2+priorityFee to keep the transaction marketable for the next 6 blocks.

The maxGasFeeCap servers as an upper limit. The final gas price the tx to be charged is usually lower than maxGasFeeCap.

@swift1337 swift1337 enabled auto-merge August 6, 2024 16:43
@swift1337 swift1337 added this pull request to the merge queue Aug 6, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Aug 6, 2024
@swift1337 swift1337 enabled auto-merge August 6, 2024 17:11
@swift1337 swift1337 added this pull request to the merge queue Aug 6, 2024
Merged via the queue into develop with commit b56e758 Aug 6, 2024
28 checks passed
@swift1337 swift1337 deleted the feat/zetaclient/eip-1559 branch August 6, 2024 17:43
@xyhaxi
Copy link

xyhaxi commented Aug 9, 2024

Hello, buddy, I want to know how to contact you. I am interested in the oxygen you developed, but I don’t understand some of the installation steps. Can I ask you for help?

gartnera pushed a commit that referenced this pull request Aug 15, 2024
* Add Gas struct

* Add EIP-1559 fees

* Update changelog

* Add test cases for legacy vs dynamicFee txs

* Fix typo; Add E2E coverage

* Address PR comments

* Address PR comments

* Use gasFeeCap formula

* Revert "Use gasFeeCap formula"

This reverts commit 2260925.

* Address PR comments

* Fix e2e upgrade tests
gartnera pushed a commit that referenced this pull request Aug 15, 2024
* Add Gas struct

* Add EIP-1559 fees

* Update changelog

* Add test cases for legacy vs dynamicFee txs

* Fix typo; Add E2E coverage

* Address PR comments

* Address PR comments

* Use gasFeeCap formula

* Revert "Use gasFeeCap formula"

This reverts commit 2260925.

* Address PR comments

* Fix e2e upgrade tests
gartnera pushed a commit that referenced this pull request Aug 15, 2024
* Add Gas struct

* Add EIP-1559 fees

* Update changelog

* Add test cases for legacy vs dynamicFee txs

* Fix typo; Add E2E coverage

* Address PR comments

* Address PR comments

* Use gasFeeCap formula

* Revert "Use gasFeeCap formula"

This reverts commit 2260925.

* Address PR comments

* Fix e2e upgrade tests
gartnera added a commit that referenced this pull request Aug 16, 2024
* feat: parse inscription like witness data (#2524)

* parse inscription like witness data

* more comment

* remove unused code

* Update zetaclient/chains/bitcoin/tx_script.go

Co-authored-by: Dmitry S <[email protected]>

* Update zetaclient/chains/bitcoin/observer/inbound.go

Co-authored-by: Dmitry S <[email protected]>

* Update zetaclient/chains/bitcoin/tx_script.go

Co-authored-by: Dmitry S <[email protected]>

* Update zetaclient/chains/bitcoin/tx_script.go

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* pull origin

* Update zetaclient/chains/bitcoin/observer/inbound.go

Co-authored-by: Dmitry S <[email protected]>

* review feedbacks

* update review feedbacks

* update make generate

* fix linter

* remove over flow

* Update zetaclient/chains/bitcoin/observer/inbound.go

Co-authored-by: Francisco de Borja Aranda Castillejo <[email protected]>

* Update zetaclient/chains/bitcoin/tokenizer.go

Co-authored-by: Francisco de Borja Aranda Castillejo <[email protected]>

* Update zetaclient/chains/bitcoin/tokenizer.go

Co-authored-by: Francisco de Borja Aranda Castillejo <[email protected]>

* Update zetaclient/chains/bitcoin/tokenizer.go

Co-authored-by: Francisco de Borja Aranda Castillejo <[email protected]>

* Update zetaclient/chains/bitcoin/tokenizer.go

Co-authored-by: Francisco de Borja Aranda Castillejo <[email protected]>

* update review feedback

* update code commnet

* update comment

* more comments

* Update changelog.md

---------

Co-authored-by: Dmitry S <[email protected]>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: Francisco de Borja Aranda Castillejo <[email protected]>

fix version

* feat: detect memo in btc txn from OP_RETURN and inscription (#2533)

* parse inscription like witness data

* more comment

* remove unused code

* parse inscription

* Update zetaclient/chains/bitcoin/tx_script.go

Co-authored-by: Dmitry S <[email protected]>

* Update zetaclient/chains/bitcoin/observer/inbound.go

Co-authored-by: Dmitry S <[email protected]>

* Update zetaclient/chains/bitcoin/tx_script.go

Co-authored-by: Dmitry S <[email protected]>

* Update zetaclient/chains/bitcoin/tx_script.go

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* pull origin

* Update zetaclient/chains/bitcoin/observer/inbound.go

Co-authored-by: Dmitry S <[email protected]>

* review feedbacks

* update review feedbacks

* add mainnet txn

* Update zetaclient/chains/bitcoin/tx_script.go

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* parse inscription like witness data

* more comment

* remove unused code

* Update zetaclient/chains/bitcoin/tx_script.go

Co-authored-by: Dmitry S <[email protected]>

* Update zetaclient/chains/bitcoin/observer/inbound.go

Co-authored-by: Dmitry S <[email protected]>

* Update zetaclient/chains/bitcoin/tx_script.go

Co-authored-by: Dmitry S <[email protected]>

* Update zetaclient/chains/bitcoin/tx_script.go

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* pull origin

* Update zetaclient/chains/bitcoin/observer/inbound.go

Co-authored-by: Dmitry S <[email protected]>

* review feedbacks

* update review feedbacks

* update make generate

* fix linter

* remove over flow

* Update zetaclient/chains/bitcoin/observer/inbound.go

Co-authored-by: Francisco de Borja Aranda Castillejo <[email protected]>

* Update zetaclient/chains/bitcoin/tokenizer.go

Co-authored-by: Francisco de Borja Aranda Castillejo <[email protected]>

* Update zetaclient/chains/bitcoin/tokenizer.go

Co-authored-by: Francisco de Borja Aranda Castillejo <[email protected]>

* Update zetaclient/chains/bitcoin/tokenizer.go

Co-authored-by: Francisco de Borja Aranda Castillejo <[email protected]>

* Update zetaclient/chains/bitcoin/tokenizer.go

Co-authored-by: Francisco de Borja Aranda Castillejo <[email protected]>

* update review feedback

* update code commnet

* update comment

* more comments

* Update changelog.md

* Update zetaclient/chains/bitcoin/observer/inbound.go

Co-authored-by: Francisco de Borja Aranda Castillejo <[email protected]>

* Update zetaclient/chains/bitcoin/observer/inbound.go

Co-authored-by: Francisco de Borja Aranda Castillejo <[email protected]>

* clean up

* format code

---------

Co-authored-by: Dmitry S <[email protected]>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: Francisco de Borja Aranda Castillejo <[email protected]>

* refactor(zetaclient)!: improve AppContext (#2568)

* Implement chain registry

* Rewrite test-cases for AppContext

* Drop `supplychecker`

* Refactor app ctx Update worker

* Refactor orchestrator

* Refactor observer&signer; DROP postBlockHeaders

* Fix test cases [1]

* Update changelog

* Allow Zeta Chain in appContext; address PR comments [1]

* Fix app context update

* Check for `chain.IsZeta()`

* Add AppContext.FilterChains

* Fix test cases [2]

* Fix test cases [3]

* Address PR comments [1]

* Address PR comments [2]

* Add tests for `slices`

* Fix e2e tests [1]

* Fix e2e tests [2]

* Resolve conflicts, converge codebase between PRs

* Add lodash; remove slices pkg

* Address PR comments

* Minor logging fix

* Address PR comments

tmp

* feat(zetaclient): add generic rpc metrics (#2597)

* feat(zetaclient): add generic rpc metrics

* feedback

* changelog

* fmt

* fix(zetaclient): use name in pending tx metric (#2642)

* feat(pkg): add `ticker` package (#2617)

* Add `pkg/ticker`

* Sample ticker usage in evm observer

* Change naming

* Address PR comments

* Address PR comments

* feat(zetaclient)!: Add support for EIP-1559 gas fees (#2634)

* Add Gas struct

* Add EIP-1559 fees

* Update changelog

* Add test cases for legacy vs dynamicFee txs

* Fix typo; Add E2E coverage

* Address PR comments

* Address PR comments

* Use gasFeeCap formula

* Revert "Use gasFeeCap formula"

This reverts commit 2260925.

* Address PR comments

* Fix e2e upgrade tests

* fix: adjust evm outbound tracker reporter to avoid submitting invalid hashes (#2628)

* refactor and fix evm outbound tracker reporter to avoid invalid hashes; print log when outbound tracker is full of invalid hashes

* add changelog entry

* used predefined log fields

* remove repeated fields information from log message; Devops team would configure Datadog to show the fields

* remove redundant fields in log message; unified logs

* remove pending transaction map from observer; the outbound tracker reporter will no longer report pending hash

* use bg.Work() to launch outbound tracker reporter goroutines

* bring the checking EnsureNoTrackers() back

* add more rationale to EVM outbound tracker submission

* sync observer and signers without wait on startup

* try fixing tss migration E2E failure by increase timeout

* feat: Solana relayer (fee payer) key importer, encryption and decryption (#2673)

* configure observer relayer key for Solana; remove hardcoded solana test key from zetaclient code

* implementation of relayer key importer, encryption and decryption

* integrate relayer key into E2E and Solana signer

* add relayer_key_balance metrics and unit tests

* use TrimSpace to trim password

* add changelog entry

* use relayer account array in E2E config; a few renaming; add private key validation when importing

* fix linter

* remove GetNetworkName method for simplification

* added PromptPassword method to prompt single password

* use network name as map index to store relayer key passwords

* moved relayer passwords to chain registry

* airdrop SOL token only if solana local node is available

---------

Co-authored-by: Lucas Bertrand <[email protected]>

* ci: Set Docker Workflow to use Go 1.22 (#2722)

* Set go 1.22.2

* Set go 1.22.2

* Set go 1.22

* Set go 1.22

* Refactor contrib/rpc and contrib/docker-scripts to use snapshots API (#2724)

Co-authored-by: Julian Rubino <[email protected]>

---------

Co-authored-by: dev-bitSmiley <[email protected]>
Co-authored-by: Dmitry S <[email protected]>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: Francisco de Borja Aranda Castillejo <[email protected]>
Co-authored-by: Charlie Chen <[email protected]>
Co-authored-by: Lucas Bertrand <[email protected]>
Co-authored-by: Charlie <[email protected]>
Co-authored-by: Julian Rubino <[email protected]>
Co-authored-by: Julian Rubino <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ADMIN_TESTS Run make start-admin-tests UPGRADE_TESTS Run make start-upgrade-tests zetaclient Issues related to ZetaClient
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support EIP-1559 transactions with ZetaClient
4 participants