Conversation
curryxbo
requested review from
tomatoishealthy
and removed request for
a team
📝 WalkthroughWalkthroughAdds optional fixed gas fee and gas tip caps (CLI flags → config → L2Client). L2Client stores caps and exposes accessors; signing and tx-manager logic use configured caps (clamping dynamic suggestions) or fall back to dynamic tip/base-fee computations. (≤50 words) Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant L2Client
participant EthNode as EthereumNode
participant Signer
participant TxManager
Caller->>L2Client: Request tx opts (GetOpts)
alt Fixed caps configured
L2Client->>L2Client: Return opts with GasTipCap & GasFeeCap (fixed)
else Dynamic caps
L2Client->>EthNode: SuggestGasTipCap
EthNode-->>L2Client: tipCap
L2Client->>EthNode: HeaderByNumber("latest")
EthNode-->>L2Client: head (BaseFee?)
alt head.BaseFee present
L2Client->>L2Client: feeCap = tipCap + 2*BaseFee
else
L2Client->>L2Client: feeCap = tipCap
end
L2Client->>Caller: return opts with dynamic caps
end
Caller->>TxManager: Send transaction (local sign path)
TxManager->>TxManager: applyGasCaps(auth) — fetch tip/header, clamp to configured caps
TxManager->>Signer: EstimateGas + Sign using auth with GasTipCap/GasFeeCap
Signer->>EthNode: SendRawTransaction
EthNode-->>Signer: TxHash
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@token-price-oracle/client/l2_client.go`:
- Around line 55-63: The code currently converts uint64 cfg.GasFeeCap and
cfg.GasTipCap to signed int64 via big.NewInt(int64(...)) which can overflow;
update the assignment for l2Client.gasFeeCap and l2Client.gasTipCap to construct
unsigned big.Ints (use new(big.Int).SetUint64(cfg.GasFeeCap) and
new(big.Int).SetUint64(cfg.GasTipCap)) and keep the existing log.Info calls (log
the original uint64 values) so the gas caps are correctly represented as
unsigned big.Ints rather than risking negative values from int64 overflow.
🧹 Nitpick comments (3)
token-price-oracle/config/config.go (1)
88-91: Consider validating thatGasFeeCap >= GasTipCapwhen both are set.EIP-1559 requires
gasFeeCap >= gasTipCap. When both fixed values are provided, an invalid combination (e.g.,GasTipCap > GasFeeCap) would produce a transaction that gets rejected at submission time with a potentially confusing error. A quick check inLoadConfigwould surface this early.Suggested validation in LoadConfig (after line 120)
// Add after loading GasFeeCap and GasTipCap (around line 121) if cfg.GasFeeCap > 0 && cfg.GasTipCap > 0 && cfg.GasTipCap > cfg.GasFeeCap { return nil, fmt.Errorf("gas tip cap (%d) must not exceed gas fee cap (%d)", cfg.GasTipCap, cfg.GasFeeCap) }token-price-oracle/client/l2_client.go (2)
156-162: Shared*big.Intreference assigned toopts— callers could inadvertently mutate internal state.
c.gasFeeCapis assigned directly toopts.GasFeeCapwithout copying. If any downstream code (e.g., thebindpackage or gas estimation) mutates the*big.Intin-place, it would corrupt the fixed cap for all subsequent transactions. This is a defensive concern given current callers don't appear to mutate, but worth protecting.Suggested defensive copy
if c.gasFeeCap != nil { - opts.GasFeeCap = c.gasFeeCap + opts.GasFeeCap = new(big.Int).Set(c.gasFeeCap) } if c.gasTipCap != nil { - opts.GasTipCap = c.gasTipCap + opts.GasTipCap = new(big.Int).Set(c.gasTipCap) }
190-199: Same shared-pointer concern applies to the accessors.
GetFixedGasFeeCap()andGetFixedGasTipCap()return the internal*big.Intdirectly. Ifsign.go(or a future caller) mutates the returned value, it corrupts the L2Client's state. Returning a copy would be safer.Suggested defensive copy
func (c *L2Client) GetFixedGasFeeCap() *big.Int { + if c.gasFeeCap == nil { + return nil + } - return c.gasFeeCap + return new(big.Int).Set(c.gasFeeCap) } func (c *L2Client) GetFixedGasTipCap() *big.Int { + if c.gasTipCap == nil { + return nil + } - return c.gasTipCap + return new(big.Int).Set(c.gasTipCap) }
…e min of dynamic and configured value)
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
token-price-oracle/client/sign.go (1)
99-134:⚠️ Potential issue | 🔴 CriticalClamping tip and fee cap independently can violate the EIP-1559 invariant
gasFeeCap >= gasTipCap.Tip is clamped first (lines 104-108), then
gasFeeCapis computed from clamped tip and later independently clamped (lines 129-133). If onlygas-fee-capis configured (withoutgas-tip-cap), a high dynamic tip can exceed the clampedgasFeeCap, producing an invalid transaction.Example: dynamic tip = 100 gwei, baseFee = 50 gwei → computed gasFeeCap = 200 gwei. If
gas-fee-capis set to 80 gwei butgas-tip-capis unset →gasFeeCap = 80,tip = 100→ gasFeeCap < tip, transaction rejected.Add a post-clamping guard to enforce the invariant:
Proposed fix — clamp tip to gasFeeCap after all caps are applied
// Apply gas fee cap limit if configured if maxFeeCap := client.GetFixedGasFeeCap(); maxFeeCap != nil { if gasFeeCap.Cmp(maxFeeCap) > 0 { log.Debug("Applying gas fee cap limit", "dynamic", gasFeeCap, "cap", maxFeeCap) gasFeeCap = maxFeeCap } } + + // Ensure EIP-1559 invariant: gasFeeCap >= gasTipCap + if gasFeeCap.Cmp(tip) < 0 { + log.Warn("Gas fee cap is lower than tip cap, clamping tip to fee cap", "gasFeeCap", gasFeeCap, "tip", tip) + tip = new(big.Int).Set(gasFeeCap) + }
🤖 Fix all issues with AI agents
In `@token-price-oracle/config/config.go`:
- Around line 119-127: When reading flags in config.go for flags.GasFeeCapFlag
and flags.GasTipCapFlag, validate and reject zero values and enforce GasFeeCap
>= GasTipCap: when ctx.IsSet(flags.GasFeeCapFlag.Name) or
ctx.IsSet(flags.GasTipCapFlag.Name) read the uint64 into a local v, return an
error if v == 0 (do not assign to cfg.GasFeeCap/cfg.GasTipCap), and only assign
the pointers after validation; if both caps are provided, check that
cfg.GasFeeCap >= cfg.GasTipCap and return an error if not (this prevents the
invalid state that breaks the EIP-1559 invariant and avoids the clamping issue
in sign.go).
🧹 Nitpick comments (1)
token-price-oracle/client/sign.go (1)
104-108: Potential aliasing with internal client state in gas cap assignments.The
GetFixedGasTipCap()andGetFixedGasFeeCap()methods return direct references to the client's internal*big.Intfields (gasTipCapandgasFeeCap). This creates aliasing when assigned viatip = maxTip(line 104) andgasFeeCap = maxFeeCap(line 131), where local variables reference the client's state.While the current code only reads these variables after assignment (no in-place mutations), this pattern is fragile and may lead to subtle bugs if future changes add arithmetic operations on
tiporgasFeeCapafter these assignments.Use defensive copies to prevent aliasing:
Line 104
if maxTip := client.GetFixedGasTipCap(); maxTip != nil { if tip.Cmp(maxTip) > 0 { log.Debug("Applying gas tip cap limit", "dynamic", tip, "cap", maxTip) - tip = maxTip + tip = new(big.Int).Set(maxTip) }Line 131
if maxFeeCap := client.GetFixedGasFeeCap(); maxFeeCap != nil { if gasFeeCap.Cmp(maxFeeCap) > 0 { log.Debug("Applying gas fee cap limit", "dynamic", gasFeeCap, "cap", maxFeeCap) - gasFeeCap = maxFeeCap + gasFeeCap = new(big.Int).Set(maxFeeCap) }
| // Gas fee caps (only set if flag is explicitly provided) | ||
| if ctx.IsSet(flags.GasFeeCapFlag.Name) { | ||
| v := ctx.Uint64(flags.GasFeeCapFlag.Name) | ||
| cfg.GasFeeCap = &v | ||
| } | ||
| if ctx.IsSet(flags.GasTipCapFlag.Name) { | ||
| v := ctx.Uint64(flags.GasTipCapFlag.Name) | ||
| cfg.GasTipCap = &v | ||
| } |
There was a problem hiding this comment.
Add validation: reject zero values and enforce GasFeeCap >= GasTipCap.
Two gaps:
- A user can explicitly pass
--gas-fee-cap=0or--gas-tip-cap=0, producing a cap of 0 wei. This would make every transaction fail (zero fee cap is invalid for EIP-1559). - When both caps are set and
GasFeeCap < GasTipCap, the clamping insign.gocan producegasFeeCap < gasTipCap, violating the EIP-1559 invariant and causing transaction rejection.
Both are easy to catch here at config load time.
Proposed fix
// Gas fee caps (only set if flag is explicitly provided)
if ctx.IsSet(flags.GasFeeCapFlag.Name) {
v := ctx.Uint64(flags.GasFeeCapFlag.Name)
+ if v == 0 {
+ return nil, fmt.Errorf("gas-fee-cap must be greater than 0 when set")
+ }
cfg.GasFeeCap = &v
}
if ctx.IsSet(flags.GasTipCapFlag.Name) {
v := ctx.Uint64(flags.GasTipCapFlag.Name)
+ if v == 0 {
+ return nil, fmt.Errorf("gas-tip-cap must be greater than 0 when set")
+ }
cfg.GasTipCap = &v
}
+ if cfg.GasFeeCap != nil && cfg.GasTipCap != nil && *cfg.GasFeeCap < *cfg.GasTipCap {
+ return nil, fmt.Errorf("gas-fee-cap (%d) must be >= gas-tip-cap (%d)", *cfg.GasFeeCap, *cfg.GasTipCap)
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Gas fee caps (only set if flag is explicitly provided) | |
| if ctx.IsSet(flags.GasFeeCapFlag.Name) { | |
| v := ctx.Uint64(flags.GasFeeCapFlag.Name) | |
| cfg.GasFeeCap = &v | |
| } | |
| if ctx.IsSet(flags.GasTipCapFlag.Name) { | |
| v := ctx.Uint64(flags.GasTipCapFlag.Name) | |
| cfg.GasTipCap = &v | |
| } | |
| // Gas fee caps (only set if flag is explicitly provided) | |
| if ctx.IsSet(flags.GasFeeCapFlag.Name) { | |
| v := ctx.Uint64(flags.GasFeeCapFlag.Name) | |
| if v == 0 { | |
| return nil, fmt.Errorf("gas-fee-cap must be greater than 0 when set") | |
| } | |
| cfg.GasFeeCap = &v | |
| } | |
| if ctx.IsSet(flags.GasTipCapFlag.Name) { | |
| v := ctx.Uint64(flags.GasTipCapFlag.Name) | |
| if v == 0 { | |
| return nil, fmt.Errorf("gas-tip-cap must be greater than 0 when set") | |
| } | |
| cfg.GasTipCap = &v | |
| } | |
| if cfg.GasFeeCap != nil && cfg.GasTipCap != nil && *cfg.GasFeeCap < *cfg.GasTipCap { | |
| return nil, fmt.Errorf("gas-fee-cap (%d) must be >= gas-tip-cap (%d)", *cfg.GasFeeCap, *cfg.GasTipCap) | |
| } |
🤖 Prompt for AI Agents
In `@token-price-oracle/config/config.go` around lines 119 - 127, When reading
flags in config.go for flags.GasFeeCapFlag and flags.GasTipCapFlag, validate and
reject zero values and enforce GasFeeCap >= GasTipCap: when
ctx.IsSet(flags.GasFeeCapFlag.Name) or ctx.IsSet(flags.GasTipCapFlag.Name) read
the uint64 into a local v, return an error if v == 0 (do not assign to
cfg.GasFeeCap/cfg.GasTipCap), and only assign the pointers after validation; if
both caps are provided, check that cfg.GasFeeCap >= cfg.GasTipCap and return an
error if not (this prevents the invalid state that breaks the EIP-1559 invariant
and avoids the clamping issue in sign.go).
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@token-price-oracle/client/sign.go`:
- Around line 104-109: The code assigns tip = maxTip which aliases L2Client's
internal *big.Int and risks shared-state corruption; in the block where
client.GetMaxGasTipCap() is non-nil and tip.Cmp(maxTip) > 0, replace the direct
assignment with a defensive copy (e.g., tip = new(big.Int).Set(maxTip)) so tip
no longer points to the client-owned gasTipCap; update the logic in sign.go
where tip is compared/assigned to use the copy.
- Around line 129-134: The gasFeeCap assignment currently aliases the pointer
(gasFeeCap = maxFeeCap) and doesn't ensure gasTipCap ≤ gasFeeCap; change to copy
the value from client.GetMaxGasFeeCap() into a new big.Int (so gasFeeCap is
independent) and after clamping gasFeeCap, enforce the invariant by clamping
gasTipCap (or tip) to be ≤ gasFeeCap (i.e., if gasTipCap.Cmp(gasFeeCap) > 0 then
set gasTipCap to a copy of gasFeeCap); apply the same pointer-copy +
tip-clamping logic in tx_manager.go's applyGasCaps to avoid creating invalid
EIP‑1559 transactions (reference symbols: gasFeeCap, maxFeeCap, gasTipCap, tip,
client.GetMaxGasFeeCap(), applyGasCaps).
In `@token-price-oracle/updater/tx_manager.go`:
- Around line 202-252: The applyGasCaps function currently can leave tip >
gasFeeCap after clamping maxFeeCap; after you set gasFeeCap (and after the
maxFeeCap clamp) add the same post-clamp invariant enforcement as in sign.go:
ensure tip ≤ gasFeeCap by comparing tip (or auth.GasTipCap) with gasFeeCap and,
if tip.Cmp(gasFeeCap) > 0, set tip = new(big.Int).Set(gasFeeCap) and update
auth.GasTipCap accordingly before assigning auth.GasFeeCap and returning;
reference applyGasCaps, tip, gasFeeCap, maxFeeCap, auth.GasTipCap and
auth.GasFeeCap when implementing.
🧹 Nitpick comments (2)
token-price-oracle/client/l2_client.go (1)
183-191: Accessors return internal*big.Int— callers can mutate shared state.
GetMaxGasFeeCap()andGetMaxGasTipCap()return the internal pointer directly. Any caller that performs in-place operations (e.g.,result.Mul(...)) would silently corruptL2Client's state.tx_manager.gocopies before use, butsign.godoes not (see separate comment there). A defensive copy here would eliminate the class of bugs entirely.♻️ Suggested defensive copy
// GetMaxGasFeeCap returns the max gas fee cap (nil if not configured) func (c *L2Client) GetMaxGasFeeCap() *big.Int { + if c.gasFeeCap == nil { + return nil + } - return c.gasFeeCap + return new(big.Int).Set(c.gasFeeCap) } // GetMaxGasTipCap returns the max gas tip cap (nil if not configured) func (c *L2Client) GetMaxGasTipCap() *big.Int { + if c.gasTipCap == nil { + return nil + } - return c.gasTipCap + return new(big.Int).Set(c.gasTipCap) }token-price-oracle/client/sign.go (1)
85-172: Gas cap logic is duplicated betweenCreateAndSignTxandTxManager.applyGasCaps.The tip-fetch → clamp → header-fetch → feeCap-compute → clamp sequence is nearly identical in both
sign.go:CreateAndSignTx(lines 99-134) andtx_manager.go:applyGasCaps(lines 204-251). This duplication means any fix (e.g., thegasTipCap ≤ gasFeeCapinvariant) must be applied in two places. Consider extracting a shared helper, e.g.,computeGasCaps(ctx, client) (tip, feeCap *big.Int, err error), and calling it from both paths.
| if maxTip := client.GetMaxGasTipCap(); maxTip != nil { | ||
| if tip.Cmp(maxTip) > 0 { | ||
| log.Debug("Applying gas tip cap limit", "dynamic", tip, "cap", maxTip) | ||
| tip = maxTip | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing defensive copy — tip aliases L2Client's internal gasTipCap.
When the cap is applied, tip = maxTip makes tip point to the same *big.Int stored inside L2Client. Any future in-place mutation of tip would corrupt the shared state. The parallel code in tx_manager.go (line 222) correctly uses new(big.Int).Set(maxTipCap).
Proposed fix
if maxTip := client.GetMaxGasTipCap(); maxTip != nil {
if tip.Cmp(maxTip) > 0 {
log.Debug("Applying gas tip cap limit", "dynamic", tip, "cap", maxTip)
- tip = maxTip
+ tip = new(big.Int).Set(maxTip)
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if maxTip := client.GetMaxGasTipCap(); maxTip != nil { | |
| if tip.Cmp(maxTip) > 0 { | |
| log.Debug("Applying gas tip cap limit", "dynamic", tip, "cap", maxTip) | |
| tip = maxTip | |
| } | |
| } | |
| if maxTip := client.GetMaxGasTipCap(); maxTip != nil { | |
| if tip.Cmp(maxTip) > 0 { | |
| log.Debug("Applying gas tip cap limit", "dynamic", tip, "cap", maxTip) | |
| tip = new(big.Int).Set(maxTip) | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@token-price-oracle/client/sign.go` around lines 104 - 109, The code assigns
tip = maxTip which aliases L2Client's internal *big.Int and risks shared-state
corruption; in the block where client.GetMaxGasTipCap() is non-nil and
tip.Cmp(maxTip) > 0, replace the direct assignment with a defensive copy (e.g.,
tip = new(big.Int).Set(maxTip)) so tip no longer points to the client-owned
gasTipCap; update the logic in sign.go where tip is compared/assigned to use the
copy.
| if maxFeeCap := client.GetMaxGasFeeCap(); maxFeeCap != nil { | ||
| if gasFeeCap.Cmp(maxFeeCap) > 0 { | ||
| log.Debug("Applying gas fee cap limit", "dynamic", gasFeeCap, "cap", maxFeeCap) | ||
| gasFeeCap = maxFeeCap | ||
| } | ||
| } |
There was a problem hiding this comment.
Same aliasing issue for gasFeeCap; also missing gasTipCap ≤ gasFeeCap invariant.
-
gasFeeCap = maxFeeCapaliases the internal pointer (same issue astipabove). -
More importantly: after clamping
gasFeeCaptomaxFeeCap, there is no guarantee thattip ≤ gasFeeCap. IfmaxFeeCapis configured butmaxTipCapis not (or is higher), a high suggested tip will exceed the capped fee cap, producing an invalid EIP-1559 transaction (gasTipCap > gasFeeCap). The same issue exists intx_manager.goapplyGasCaps.Example:
baseFee=10, suggestedtip=20,maxFeeCap=15, nomaxTipCap→gasFeeCapclamped to 15, buttipstays at 20. Node rejects the tx.
Proposed fix (apply to both sign.go and tx_manager.go)
// Apply gas fee cap limit if configured
if maxFeeCap := client.GetMaxGasFeeCap(); maxFeeCap != nil {
if gasFeeCap.Cmp(maxFeeCap) > 0 {
log.Debug("Applying gas fee cap limit", "dynamic", gasFeeCap, "cap", maxFeeCap)
- gasFeeCap = maxFeeCap
+ gasFeeCap = new(big.Int).Set(maxFeeCap)
}
}
+
+ // Ensure gasTipCap <= gasFeeCap (EIP-1559 invariant)
+ if tip.Cmp(gasFeeCap) > 0 {
+ log.Debug("Clamping tip to gasFeeCap", "tip", tip, "gasFeeCap", gasFeeCap)
+ tip = new(big.Int).Set(gasFeeCap)
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if maxFeeCap := client.GetMaxGasFeeCap(); maxFeeCap != nil { | |
| if gasFeeCap.Cmp(maxFeeCap) > 0 { | |
| log.Debug("Applying gas fee cap limit", "dynamic", gasFeeCap, "cap", maxFeeCap) | |
| gasFeeCap = maxFeeCap | |
| } | |
| } | |
| if maxFeeCap := client.GetMaxGasFeeCap(); maxFeeCap != nil { | |
| if gasFeeCap.Cmp(maxFeeCap) > 0 { | |
| log.Debug("Applying gas fee cap limit", "dynamic", gasFeeCap, "cap", maxFeeCap) | |
| gasFeeCap = new(big.Int).Set(maxFeeCap) | |
| } | |
| } | |
| // Ensure gasTipCap <= gasFeeCap (EIP-1559 invariant) | |
| if tip.Cmp(gasFeeCap) > 0 { | |
| log.Debug("Clamping tip to gasFeeCap", "tip", tip, "gasFeeCap", gasFeeCap) | |
| tip = new(big.Int).Set(gasFeeCap) | |
| } |
🤖 Prompt for AI Agents
In `@token-price-oracle/client/sign.go` around lines 129 - 134, The gasFeeCap
assignment currently aliases the pointer (gasFeeCap = maxFeeCap) and doesn't
ensure gasTipCap ≤ gasFeeCap; change to copy the value from
client.GetMaxGasFeeCap() into a new big.Int (so gasFeeCap is independent) and
after clamping gasFeeCap, enforce the invariant by clamping gasTipCap (or tip)
to be ≤ gasFeeCap (i.e., if gasTipCap.Cmp(gasFeeCap) > 0 then set gasTipCap to a
copy of gasFeeCap); apply the same pointer-copy + tip-clamping logic in
tx_manager.go's applyGasCaps to avoid creating invalid EIP‑1559 transactions
(reference symbols: gasFeeCap, maxFeeCap, gasTipCap, tip,
client.GetMaxGasFeeCap(), applyGasCaps).
| // applyGasCaps applies configured gas caps as upper limits to dynamic gas prices | ||
| // This ensures consistent behavior between local sign and external sign | ||
| func (m *TxManager) applyGasCaps(ctx context.Context, auth *bind.TransactOpts) error { | ||
| maxTipCap := m.l2Client.GetMaxGasTipCap() | ||
| maxFeeCap := m.l2Client.GetMaxGasFeeCap() | ||
|
|
||
| // If no caps configured, let bind package handle gas pricing dynamically | ||
| if maxTipCap == nil && maxFeeCap == nil { | ||
| return nil | ||
| } | ||
|
|
||
| // Get dynamic gas tip cap | ||
| tip, err := m.l2Client.GetClient().SuggestGasTipCap(ctx) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to get gas tip cap: %w", err) | ||
| } | ||
|
|
||
| // Apply tip cap limit if configured | ||
| if maxTipCap != nil && tip.Cmp(maxTipCap) > 0 { | ||
| log.Debug("Applying gas tip cap limit", "dynamic", tip, "cap", maxTipCap) | ||
| tip = new(big.Int).Set(maxTipCap) | ||
| } | ||
| auth.GasTipCap = tip | ||
|
|
||
| // Get base fee from latest block | ||
| head, err := m.l2Client.GetClient().HeaderByNumber(ctx, nil) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to get block header: %w", err) | ||
| } | ||
|
|
||
| // Calculate dynamic gas fee cap | ||
| var gasFeeCap *big.Int | ||
| if head.BaseFee != nil { | ||
| gasFeeCap = new(big.Int).Add( | ||
| tip, | ||
| new(big.Int).Mul(head.BaseFee, big.NewInt(2)), | ||
| ) | ||
| } else { | ||
| gasFeeCap = new(big.Int).Set(tip) | ||
| } | ||
|
|
||
| // Apply fee cap limit if configured | ||
| if maxFeeCap != nil && gasFeeCap.Cmp(maxFeeCap) > 0 { | ||
| log.Debug("Applying gas fee cap limit", "dynamic", gasFeeCap, "cap", maxFeeCap) | ||
| gasFeeCap = new(big.Int).Set(maxFeeCap) | ||
| } | ||
| auth.GasFeeCap = gasFeeCap | ||
|
|
||
| log.Debug("Gas caps applied", "tipCap", auth.GasTipCap, "feeCap", auth.GasFeeCap) | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Same missing gasTipCap ≤ gasFeeCap invariant as sign.go.
After clamping gasFeeCap (line 244-247), tip can still exceed gasFeeCap when only maxFeeCap is configured. This produces an invalid EIP-1559 transaction. See the detailed analysis and fix in the sign.go review — the same post-clamp check needs to be added here (after line 248):
auth.GasFeeCap = gasFeeCap
+
+ // Ensure gasTipCap <= gasFeeCap (EIP-1559 invariant)
+ if auth.GasTipCap.Cmp(auth.GasFeeCap) > 0 {
+ log.Debug("Clamping tip to gasFeeCap", "tip", auth.GasTipCap, "gasFeeCap", auth.GasFeeCap)
+ auth.GasTipCap = new(big.Int).Set(auth.GasFeeCap)
+ }🤖 Prompt for AI Agents
In `@token-price-oracle/updater/tx_manager.go` around lines 202 - 252, The
applyGasCaps function currently can leave tip > gasFeeCap after clamping
maxFeeCap; after you set gasFeeCap (and after the maxFeeCap clamp) add the same
post-clamp invariant enforcement as in sign.go: ensure tip ≤ gasFeeCap by
comparing tip (or auth.GasTipCap) with gasFeeCap and, if tip.Cmp(gasFeeCap) > 0,
set tip = new(big.Int).Set(gasFeeCap) and update auth.GasTipCap accordingly
before assigning auth.GasFeeCap and returning; reference applyGasCaps, tip,
gasFeeCap, maxFeeCap, auth.GasTipCap and auth.GasFeeCap when implementing.
2 participants
…guration
Summary by CodeRabbit
New Features
Improvements