feat: forward txs to sequencer #1208
Conversation
WalkthroughThis update introduces three new CLI flags for controlling transaction gossip broadcasting, receiving, and sequencer HTTP endpoint configuration. Corresponding fields are added to configuration structs and wiring logic. The transaction submission flow is extended to optionally forward transactions to a remote sequencer and conditionally bypass the local transaction pool based on these flags. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant Node
participant EthAPIBackend
participant SequencerRPC
participant TxPool
User->>CLI: Start node with gossip flags
CLI->>Node: Initialize with config
Node->>EthAPIBackend: Pass gossip flags and sequencer endpoint
User->>EthAPIBackend: SendTx(signedTx)
EthAPIBackend->>EthAPIBackend: Check tx type
alt BlobTxType
EthAPIBackend-->>User: Return error
else sequencerRPC configured
EthAPIBackend->>SequencerRPC: Forward tx via eth_sendRawTransaction
alt disableTxPool
EthAPIBackend-->>User: Return (do not add to local pool)
else
EthAPIBackend->>TxPool: AddLocal(tx)
alt AddLocal fails but sequencer ok
EthAPIBackend-->>User: Return success (log warning)
else
EthAPIBackend-->>User: Return result/error
end
end
else
EthAPIBackend->>TxPool: AddLocal(tx)
EthAPIBackend-->>User: Return result/error
end
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches
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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
eth/handler.go (1)
442-444: Fix potential nil pointer dereference in Stop method.The Stop method unconditionally calls
h.txsSub.Unsubscribe(), buth.txsSubwill be nil whendisableTxBroadcastis true since the subscription is not created in that case.Add a nil check to prevent panic:
func (h *handler) Stop() { - h.txsSub.Unsubscribe() // quits txBroadcastLoop + if h.txsSub != nil { + h.txsSub.Unsubscribe() // quits txBroadcastLoop + } h.minedBlockSub.Unsubscribe() // quits blockBroadcastLoop
🧹 Nitpick comments (1)
eth/backend.go (1)
311-319: Consider making the connection timeout configurable.The hardcoded 5-second timeout for the sequencer RPC connection may be insufficient for high-latency networks or during network congestion.
Consider adding a configuration option for the timeout:
- ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + timeout := 5 * time.Second + if config.TxGossipSequencerTimeout > 0 { + timeout = config.TxGossipSequencerTimeout + } + ctx, cancel := context.WithTimeout(context.Background(), timeout)Additionally, consider validating the HTTP endpoint format before attempting the connection to provide better error messages.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
cmd/geth/main.go(1 hunks)cmd/geth/usage.go(1 hunks)cmd/utils/flags.go(2 hunks)eth/api_backend.go(4 hunks)eth/backend.go(5 hunks)eth/ethconfig/config.go(1 hunks)eth/handler.go(4 hunks)eth/handler_eth.go(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: test
- GitHub Check: check
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (go)
🔇 Additional comments (16)
eth/ethconfig/config.go (1)
234-236: LGTM! Clean configuration additions.The new configuration fields for transaction gossip control are well-named, properly typed, and appropriately placed in the Config struct.
eth/handler_eth.go (1)
59-61: LGTM! Correct implementation of transaction receiving control.The early return when
disableTxReceivingis true ensures that transaction receiving can be completely disabled, which aligns with the PR objective.cmd/geth/main.go (1)
179-181: LGTM! Proper CLI flag integration.The three transaction gossip control flags are correctly added to the node configuration flags, making them available as command-line options.
eth/handler.go (4)
97-98: LGTM! Clean configuration structure additions.The new fields for controlling transaction gossip behavior are properly added to the handler configuration.
138-139: LGTM! Proper field additions to handler struct.The transaction gossip control fields are correctly added to the handler struct.
159-160: LGTM! Correct field initialization.The configuration fields are properly initialized in the handler constructor.
425-430: LGTM! Proper conditional initialization of transaction broadcast.The transaction broadcast loop initialization is correctly guarded by the
disableTxBroadcastflag, preventing unnecessary resource allocation when broadcasting is disabled.eth/backend.go (4)
112-114: LGTM: Sequencer RPC service field addition.The new
seqRPCServicefield is properly typed and documented. The placement within the "Scroll additions" section is appropriate.
289-290: LGTM: Handler configuration for transaction gossip control.The new configuration flags for disabling transaction broadcast and receiving are properly integrated into the handler configuration.
300-300: LGTM: EthAPIBackend initialization with transaction gossip flag.The addition of
config.TxGossipReceivingDisabledparameter properly integrates the new transaction pool control functionality.
693-695: LGTM: Proper resource cleanup for sequencer RPC service.The cleanup logic correctly checks for nil and closes the RPC client to prevent resource leaks.
eth/api_backend.go (3)
28-28: LGTM: Required imports for sequencer functionality.The added imports for
hexutilandlogare necessary for transaction encoding and logging in the new sequencer forwarding logic.Also applies to: 39-39
49-49: LGTM: Transaction pool control field addition.The
disableTxPoolfield provides clear control over local transaction pool behavior.
295-298: LGTM: Clean encapsulation of local transaction pool logic.The
sendTxhelper method properly encapsulates the transaction pool addition logic with appropriate validation.cmd/utils/flags.go (2)
896-909: LGTM: Well-structured flag definitions.The three new transaction gossip flags are properly defined with clear names, appropriate types, and descriptive usage strings. The flag naming follows the established conventions in the codebase.
1807-1814: LGTM: Correct flag-to-config mapping with appropriate logging.The boolean flag assignments are implemented correctly, following the established pattern of other flags in this function. The informational logging provides useful visibility into the configuration state.
yiweichi
changed the title
1. Purpose or design rationale of this PR
This PR add feature to enable RPC node forward transactions directly to sequencer.
txgossip.sequencerhttp: the sequencer's http rpc endpoint2. PR title
Your PR title must follow conventional commits (as we are doing squash merge for each PR), so it must start with one of the following types:
3. Deployment tag versioning
Has the version in
params/version.gobeen updated?4. Breaking change label
Does this PR have the
breaking-changelabel?Summary by CodeRabbit
New Features
Bug Fixes
Chores