Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 28 additions & 6 deletions token-price-oracle/client/l2_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ type L2Client struct {
opts *bind.TransactOpts
signer *Signer
externalSign bool
gasFeeCap *big.Int // Max gas fee cap (nil means no cap)
gasTipCap *big.Int // Max gas tip cap (nil means no cap)
}

// NewL2Client creates new L2 client
Expand Down Expand Up @@ -50,6 +52,16 @@ func NewL2Client(rpcURL string, cfg *config.Config) (*L2Client, error) {
externalSign: cfg.ExternalSign,
}

// Set gas fee caps if configured (used as max cap, not fixed value)
if cfg.GasFeeCap != nil {
l2Client.gasFeeCap = new(big.Int).SetUint64(*cfg.GasFeeCap)
log.Info("Using gas fee cap limit", "maxGasFeeCap", *cfg.GasFeeCap)
}
if cfg.GasTipCap != nil {
l2Client.gasTipCap = new(big.Int).SetUint64(*cfg.GasTipCap)
log.Info("Using gas tip cap limit", "maxGasTipCap", *cfg.GasTipCap)
}

if cfg.ExternalSign {
// External sign mode
rsaPriv, err := externalsign.ParseRsaPrivateKey(cfg.ExternalSignRsaPriv)
Expand Down Expand Up @@ -127,14 +139,14 @@ func (c *L2Client) GetClient() *ethclient.Client {

// GetOpts returns a copy of transaction options
// Returns a new instance to prevent concurrent modification
// Note: Gas caps are applied by TxManager.applyGasCaps(), not here
func (c *L2Client) GetOpts() *bind.TransactOpts {
// Return a copy to prevent shared state issues
return &bind.TransactOpts{
From: c.opts.From,
Nonce: c.opts.Nonce,
Signer: c.opts.Signer,
Value: c.opts.Value,
GasPrice: c.opts.GasPrice,
From: c.opts.From,
Nonce: c.opts.Nonce,
Signer: c.opts.Signer,
Value: c.opts.Value,
GasPrice: c.opts.GasPrice,
GasFeeCap: c.opts.GasFeeCap,
GasTipCap: c.opts.GasTipCap,
GasLimit: c.opts.GasLimit,
Expand Down Expand Up @@ -167,3 +179,13 @@ func (c *L2Client) GetSigner() *Signer {
func (c *L2Client) GetChainID() *big.Int {
return c.chainID
}

// GetMaxGasFeeCap returns the max gas fee cap (nil if not configured)
func (c *L2Client) GetMaxGasFeeCap() *big.Int {
return c.gasFeeCap
}

// GetMaxGasTipCap returns the max gas tip cap (nil if not configured)
func (c *L2Client) GetMaxGasTipCap() *big.Int {
return c.gasTipCap
}
17 changes: 16 additions & 1 deletion token-price-oracle/client/sign.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,18 +96,25 @@ func (s *Signer) CreateAndSignTx(
return nil, fmt.Errorf("failed to get nonce: %w", err)
}

// Get gas tip cap
// Get gas tip cap (dynamic, then apply cap if configured)
tip, err := client.GetClient().SuggestGasTipCap(ctx)
if err != nil {
return nil, fmt.Errorf("failed to get gas tip cap: %w", err)
}
if maxTip := client.GetMaxGasTipCap(); maxTip != nil {
if tip.Cmp(maxTip) > 0 {
log.Debug("Applying gas tip cap limit", "dynamic", tip, "cap", maxTip)
tip = maxTip
}
}
Comment on lines +104 to +109
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

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.

Suggested change
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.


// Get base fee from latest block
head, err := client.GetClient().HeaderByNumber(ctx, nil)
if err != nil {
return nil, 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(
Expand All @@ -118,6 +125,14 @@ func (s *Signer) CreateAndSignTx(
gasFeeCap = new(big.Int).Set(tip)
}

// 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
}
}
Comment on lines +129 to +134
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Same aliasing issue for gasFeeCap; also missing gasTipCap ≤ gasFeeCap invariant.

  1. gasFeeCap = maxFeeCap aliases the internal pointer (same issue as tip above).

  2. More importantly: after clamping gasFeeCap to maxFeeCap, there is no guarantee that tip ≤ gasFeeCap. If maxFeeCap is configured but maxTipCap is 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 in tx_manager.go applyGasCaps.

    Example: baseFee=10, suggested tip=20, maxFeeCap=15, no maxTipCapgasFeeCap clamped to 15, but tip stays 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.

Suggested change
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).


// Estimate gas
gas, err := client.GetClient().EstimateGas(ctx, ethereum.CallMsg{
From: from,
Expand Down
14 changes: 14 additions & 0 deletions token-price-oracle/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,10 @@ type Config struct {
LogFileMaxSize int
LogFileMaxAge int
LogCompress bool

// Gas fee caps (optional - if set, use as max cap)
GasFeeCap *uint64 // Max gas fee cap in wei (nil means no cap)
GasTipCap *uint64 // Max gas tip cap in wei (nil means no cap)
}

// LoadConfig loads configuration from cli.Context
Expand Down Expand Up @@ -112,6 +116,16 @@ func LoadConfig(ctx *cli.Context) (*Config, error) {
LogCompress: ctx.Bool(flags.LogCompressFlag.Name),
}

// 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
}
Comment on lines +119 to +127
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Add validation: reject zero values and enforce GasFeeCap >= GasTipCap.

Two gaps:

  1. A user can explicitly pass --gas-fee-cap=0 or --gas-tip-cap=0, producing a cap of 0 wei. This would make every transaction fail (zero fee cap is invalid for EIP-1559).
  2. When both caps are set and GasFeeCap < GasTipCap, the clamping in sign.go can produce gasFeeCap < 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.

Suggested change
// 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).


// Parse token registry address (optional)
cfg.L2TokenRegistryAddr = predeploys.L2TokenRegistryAddr

Expand Down
17 changes: 17 additions & 0 deletions token-price-oracle/flags/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,19 @@ var (
Usage: "The RSA private key for external sign",
EnvVar: prefixEnvVar("EXTERNAL_SIGN_RSA_PRIV"),
}

// Gas fee flags (optional - if set, use as max cap instead of dynamic)
GasFeeCapFlag = cli.Uint64Flag{
Name: "gas-fee-cap",
Usage: "Max gas fee cap in wei (if set, actual fee = min(dynamic, this value))",
EnvVar: prefixEnvVar("GAS_FEE_CAP"),
}

GasTipCapFlag = cli.Uint64Flag{
Name: "gas-tip-cap",
Usage: "Max gas tip cap in wei (if set, actual tip = min(dynamic, this value))",
EnvVar: prefixEnvVar("GAS_TIP_CAP"),
}
)

var requiredFlags = []cli.Flag{
Expand Down Expand Up @@ -210,6 +223,10 @@ var optionalFlags = []cli.Flag{
ExternalSignChainFlag,
ExternalSignUrlFlag,
ExternalSignRsaPrivFlag,

// Gas fee
GasFeeCapFlag,
GasTipCapFlag,
}

// Flags contains the list of configuration options available to the binary.
Expand Down
58 changes: 58 additions & 0 deletions token-price-oracle/updater/tx_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package updater
import (
"context"
"fmt"
"math/big"
"sync"
"time"

Expand Down Expand Up @@ -72,6 +73,11 @@ func (m *TxManager) sendWithLocalSign(ctx context.Context, txFunc func(*bind.Tra
auth := m.l2Client.GetOpts()
auth.Context = ctx

// Apply gas caps if configured (same logic as external sign)
if err := m.applyGasCaps(ctx, auth); err != nil {
return nil, fmt.Errorf("failed to apply gas caps: %w", err)
}

// First, estimate gas with GasLimit = 0
auth.GasLimit = 0
auth.NoSend = true
Expand Down Expand Up @@ -193,6 +199,58 @@ func (m *TxManager) sendWithExternalSign(ctx context.Context, txFunc func(*bind.
return receipt, nil
}

// 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
}
Comment on lines +202 to +252
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

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.


// waitForReceipt waits for a transaction receipt with timeout and custom polling interval
func (m *TxManager) waitForReceipt(ctx context.Context, txHash common.Hash, timeout, pollInterval time.Duration) (*types.Receipt, error) {
deadline := time.Now().Add(timeout)
Expand Down