-
Notifications
You must be signed in to change notification settings - Fork 16
feat: support eip1559 when speeding up transactions #644
feat: support eip1559 when speeding up transactions #644
Conversation
WalkthroughThe codebase has been updated to better handle Ethereum transactions, specifically by introducing new functions to manage the resubmission of transactions with increased gas fees. This includes separate handling for EIP-1559 dynamic fee transactions and legacy transactions, ensuring compatibility with the Ethereum network's evolving gas pricing mechanisms. Changes
TipsChat with CodeRabbit Bot (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (1)
- relayer/relayer.go (4 hunks)
Additional comments: 1
relayer/relayer.go (1)
- 470-514: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [473-543]
The summary states that the new functions for creating and converting transactions are exported entities, but in the code, these functions (
createSpeededUpDynamicTransaction
,createSpeededUpLegacyTransaction
,toDynamicTransaction
, andtoLegacyTransaction
) are unexported (lowercase). If these functions are intended to be used by other packages, they should be exported (uppercase).
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #644 +/- ##
=========================================
+ Coverage 0 24.64% +24.64%
=========================================
Files 0 29 +29
Lines 0 3201 +3201
=========================================
+ Hits 0 789 +789
- Misses 0 2317 +2317
- Partials 0 95 +95 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 👍
var rawTx *coregethtypes.Transaction | ||
if tx.GasPrice() != nil { | ||
rawTx, err = createSpeededUpLegacyTransaction(ctx, ethClient, newTx) | ||
if err != nil { | ||
return err | ||
} | ||
if rawTx.GasPrice().Cmp(newTx.GasPrice()) <= 0 { | ||
// no need to resend the transaction if the suggested gas price is lower than the original one | ||
continue | ||
} | ||
} else if tx.GasTipCap() != nil && tx.GasFeeCap() != nil { | ||
rawTx, err = createSpeededUpDynamicTransaction(ctx, ethClient, newTx) | ||
if err != nil { | ||
return err | ||
} | ||
if rawTx.GasFeeCap().Cmp(newTx.GasFeeCap()) <= 0 { | ||
// no need to resend the transaction if the suggested gas price is lower than the original one | ||
continue | ||
} | ||
} else { | ||
// Only query for basefee if gasPrice not specified | ||
if head, errHead := ethClient.HeaderByNumber(ctx, nil); errHead != nil { | ||
return errHead | ||
} else if head.BaseFee != nil { | ||
rawTx, err = createSpeededUpDynamicTransaction(ctx, ethClient, newTx) | ||
if err != nil { | ||
return err | ||
} | ||
if rawTx.GasFeeCap().Cmp(newTx.GasFeeCap()) <= 0 { | ||
// no need to resend the transaction if the suggested gas price is lower than the original one | ||
continue | ||
} | ||
} else { | ||
// Chain is not London ready -> use legacy transaction | ||
rawTx, err = createSpeededUpLegacyTransaction(ctx, ethClient, newTx) | ||
if err != nil { | ||
return err | ||
} | ||
if rawTx.GasPrice().Cmp(newTx.GasPrice()) <= 0 { | ||
// no need to resend the transaction if the suggested gas price is lower than the original one | ||
continue | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think its necessarily worth changing, but it might be a good exercise to try and refactor this. Submitting a transaction is always really frustrating and some elaborate logic is unavoidable, I also haven't tried to so it might not be possible. However, my gut instinct is if there are this many if, if else, else, and continue statements there usually a way to rewrite it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good idea, but I think that's the best we could do because we have multiple chain types so we need to speed up the transaction according to its type. This implementation is inspired from the geth implementation.
Overview
Previously, we only supported speeding up legacy transactions. With this, we can also speed up EIP1559 ones.
Checklist
Summary by CodeRabbit
New Features
Improvements
waitForTransactionAndRetryIfNeeded
function to handle different transaction types more effectively.