feature: block tag support(safe&finalize) #857
Conversation
📝 WalkthroughWalkthroughAdds a new BlockTag service and configuration to compute and track safe and finalized L2 block tags from L1 batch commitments, integrates the service into the sequencer lifecycle, extends the retryable RPC client to notify Geth of tag updates, and introduces related CLI flags and a mainnet rollup address constant. Changes
Sequence DiagramsequenceDiagram
participant Poller as BlockTagService
participant L1 as L1 Node
participant Rollup as Rollup Contract
participant L2 as L2 Node
participant Geth as Geth (RPC)
loop Periodic Poll
Poller->>L1: Get L1 head & confirmations
L1-->>Poller: L1 block info
Poller->>Rollup: Query last committed batch at L1 block
Rollup-->>Poller: Batch index & state root
Poller->>L2: Resolve L2 block for batch (binary/forward search)
L2-->>Poller: L2 block header & state root
Note over Poller: Validate state roots (L1 ↔ L2)
alt Tags changed
Poller->>Geth: SetBlockTags(safeHash, finalizedHash)
Geth-->>Poller: ACK
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧹 Recent nitpick comments
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (3)
🧰 Additional context used🧬 Code graph analysis (2)node/blocktag/service.go (4)
node/cmd/node/main.go (2)
🪛 GitHub Actions: Tx-submitternode/types/retryable_client.go[error] 227-227: Build failed: rc.authClient.SetBlockTags undefined (type *authclient.Client has no field or method SetBlockTags) ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
🔇 Additional comments (6)
✏️ Tip: You can disable this entire section by setting 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: 3
🤖 Fix all issues with AI agents
In `@node/blocktag/service.go`:
- Around line 63-101: In NewBlockTagService, if bindings.NewRollup fails the
previously created l1Client is leaked; ensure l1Client is closed before
returning the error (e.g., close l1Client when rollup creation returns an error
or use a deferred cleanup tied to the rollup creation step) so that l1Client is
not left open on failure.
- Around line 292-335: findCompletedBatchForL2Block can recurse indefinitely
when binarySearchBatch yields an index but forward scanning finds no completed
batch; replace the recursive reset pattern with a single retry loop or a boolean
"cacheReset" flag: when resultIdx == 0, if we haven't already reset the cache,
call setCachedBatchIndex(tagType, 0), set the flag, recompute startIdx via
binarySearchBatch and retry the forward scan once; if it still yields no result,
return an error instead of recursing. Update logic around getCachedBatchIndex,
binarySearchBatch, setCachedBatchIndex, and the forward scan to respect the
single-retry flag.
In `@node/cmd/node/main.go`:
- Around line 147-159: The BlockTagService startup assumes L1NodeAddr and
RollupContractAddress are present but the CLI flags L1NodeAddr and
RollupContractAddress are currently optional; either make the flags required or
skip initializing the service when they're absent. To fix, either (A) update the
flag definitions for L1NodeAddr and RollupContractAddress in node/flags/flags.go
to include Required: true so NewBlockTagService can always rely on them, or (B)
modify the BlockTagService initialization in main.go (the blockTagConfig /
blockTagSvc / NewBlockTagService code path) to check the parsed flag values and
if either L1NodeAddr or RollupContractAddress is empty, log a warning and do not
call blocktag.NewBlockTagService (skip Start()), making the service optional and
avoiding the runtime error.
🧹 Nitpick comments (2)
node/blocktag/config.go (1)
36-39: Consider exposingSafeConfirmationsandPollIntervalvia CLI flags.Currently,
SetCliContextonly readsL1AddrandRollupAddressfrom CLI flags. Users cannot overrideSafeConfirmationsorPollIntervalwithout code changes.Also,
common.HexToAddresssilently returns a zero address for invalid input. WhileNewBlockTagServicevalidates this, consider adding early validation here or logging a warning for better debuggability.♻️ Suggested enhancement
+import ( + "fmt" + ... +) + +// Add new flags in node/flags/flags.go: +// BlockTagSafeConfirmations = cli.Uint64Flag{...} +// BlockTagPollInterval = cli.DurationFlag{...} func (c *Config) SetCliContext(ctx *cli.Context) error { c.L1Addr = ctx.GlobalString(flags.L1NodeAddr.Name) - c.RollupAddress = common.HexToAddress(ctx.GlobalString(flags.RollupContractAddress.Name)) + rollupAddrStr := ctx.GlobalString(flags.RollupContractAddress.Name) + if rollupAddrStr != "" && !common.IsHexAddress(rollupAddrStr) { + return fmt.Errorf("invalid rollup address: %s", rollupAddrStr) + } + c.RollupAddress = common.HexToAddress(rollupAddrStr) + // Optionally read SafeConfirmations and PollInterval from flags return nil }node/blocktag/service.go (1)
154-197:updateBlockTagsreturns nil even when errors occur.The function logs errors for
getL2BlockForTagandnotifyGethbut always returnsnil. This makes it harder for callers to know if updates succeeded. Consider returning an error or using a multi-error pattern if partial success is acceptable.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
node/blocktag/config.gonode/blocktag/service.gonode/cmd/node/main.gonode/types/retryable_client.go
🧰 Additional context used
🧬 Code graph analysis (3)
node/blocktag/config.go (2)
oracle/flags/flags.go (1)
RollupAddress(35-40)node/flags/flags.go (2)
L1NodeAddr(63-67)RollupContractAddress(187-191)
node/blocktag/service.go (4)
tx-submitter/iface/client.go (1)
Client(14-23)node/types/retryable_client.go (1)
RetryableClient(29-34)node/blocktag/config.go (1)
Config(20-25)oracle/flags/flags.go (1)
RollupAddress(35-40)
node/cmd/node/main.go (5)
node/core/executor.go (1)
Executor(31-61)node/sync/syncer.go (1)
Syncer(15-29)node/derivation/derivation.go (1)
Derivation(39-67)node/blocktag/service.go (2)
BlockTagService(33-61)NewBlockTagService(64-101)node/blocktag/config.go (1)
DefaultConfig(28-33)
🪛 GitHub Actions: Tx-submitter
node/types/retryable_client.go
[error] 227-227: rc.authClient.SetBlockTags undefined (type *authclient.Client has no field or method SetBlockTags)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: test
- GitHub Check: check
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (rust)
🔇 Additional comments (7)
node/cmd/node/main.go (1)
186-188: LGTM!Shutdown handling correctly checks for nil before stopping the service.
node/blocktag/service.go (5)
103-118: LGTM!Initialization failure is logged but doesn't block startup, which is reasonable for a non-critical service. The polling loop will retry on subsequent ticks.
120-127: LGTM!Stop cleanly cancels context, waits for the loop to exit via the
stopchannel, and closes the L1 client.
269-290: State root validation logic looks correct.The comparison between L1 committed state root and L2 block header root is a good safety check to detect mismatches.
411-435: LGTM!The notification logic correctly skips redundant RPC calls when tags haven't changed and updates the last-notified hashes only after successful notification.
375-393: No action needed —rpc.FinalizedBlockNumberconversion is compatible and follows an established pattern.The same pattern of converting
rpc.FinalizedBlockNumber(a negative constant) tobig.Intis already successfully used elsewhere in the codebase (node/common/layer1.go:24), where it's passed toHeaderByNumber()without any compatibility issues. This confirms the conversion works correctly with go-ethereum's RPC layer andCallOpts.BlockNumber.node/types/retryable_client.go (1)
225-240: TheSetBlockTagsmethod exists onauthclient.Clientand the code compiles successfully. A recent commit (fd7ab3e feature: block tag support(safe&finalize)) explicitly added this functionality to the morph-l2/go-ethereum fork. All methods called onrc.authClientfollow the identical retry pattern shown here, and the method is actively used innode/blocktag/service.go.Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| // NewBlockTagService creates a new BlockTagService | ||
| func NewBlockTagService( | ||
| ctx context.Context, | ||
| l2Client *types.RetryableClient, | ||
| config *Config, | ||
| logger tmlog.Logger, | ||
| ) (*BlockTagService, error) { | ||
| if config.L1Addr == "" { | ||
| return nil, fmt.Errorf("L1 RPC address is required") | ||
| } | ||
| if config.RollupAddress == (common.Address{}) { | ||
| return nil, fmt.Errorf("Rollup contract address is required") | ||
| } | ||
|
|
||
| l1Client, err := ethclient.Dial(config.L1Addr) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to connect to L1: %w", err) | ||
| } | ||
|
|
||
| rollup, err := bindings.NewRollup(config.RollupAddress, l1Client) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to create rollup binding: %w", err) | ||
| } | ||
|
|
||
| ctx, cancel := context.WithCancel(ctx) | ||
|
|
||
| return &BlockTagService{ | ||
| ctx: ctx, | ||
| cancel: cancel, | ||
| l1Client: l1Client, | ||
| l2Client: l2Client, | ||
| rollup: rollup, | ||
| rollupAddress: config.RollupAddress, | ||
| safeConfirmations: config.SafeConfirmations, | ||
| pollInterval: config.PollInterval, | ||
| logger: logger.With("module", "blocktag"), | ||
| stop: make(chan struct{}), | ||
| }, nil | ||
| } |
There was a problem hiding this comment.
Potential resource leak: L1 client not closed on subsequent error.
If bindings.NewRollup fails at line 82-85, the l1Client created at line 77 is not closed.
🔧 Suggested fix
l1Client, err := ethclient.Dial(config.L1Addr)
if err != nil {
return nil, fmt.Errorf("failed to connect to L1: %w", err)
}
rollup, err := bindings.NewRollup(config.RollupAddress, l1Client)
if err != nil {
+ l1Client.Close()
return nil, fmt.Errorf("failed to create rollup binding: %w", err)
}📝 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.
| // NewBlockTagService creates a new BlockTagService | |
| func NewBlockTagService( | |
| ctx context.Context, | |
| l2Client *types.RetryableClient, | |
| config *Config, | |
| logger tmlog.Logger, | |
| ) (*BlockTagService, error) { | |
| if config.L1Addr == "" { | |
| return nil, fmt.Errorf("L1 RPC address is required") | |
| } | |
| if config.RollupAddress == (common.Address{}) { | |
| return nil, fmt.Errorf("Rollup contract address is required") | |
| } | |
| l1Client, err := ethclient.Dial(config.L1Addr) | |
| if err != nil { | |
| return nil, fmt.Errorf("failed to connect to L1: %w", err) | |
| } | |
| rollup, err := bindings.NewRollup(config.RollupAddress, l1Client) | |
| if err != nil { | |
| return nil, fmt.Errorf("failed to create rollup binding: %w", err) | |
| } | |
| ctx, cancel := context.WithCancel(ctx) | |
| return &BlockTagService{ | |
| ctx: ctx, | |
| cancel: cancel, | |
| l1Client: l1Client, | |
| l2Client: l2Client, | |
| rollup: rollup, | |
| rollupAddress: config.RollupAddress, | |
| safeConfirmations: config.SafeConfirmations, | |
| pollInterval: config.PollInterval, | |
| logger: logger.With("module", "blocktag"), | |
| stop: make(chan struct{}), | |
| }, nil | |
| } | |
| // NewBlockTagService creates a new BlockTagService | |
| func NewBlockTagService( | |
| ctx context.Context, | |
| l2Client *types.RetryableClient, | |
| config *Config, | |
| logger tmlog.Logger, | |
| ) (*BlockTagService, error) { | |
| if config.L1Addr == "" { | |
| return nil, fmt.Errorf("L1 RPC address is required") | |
| } | |
| if config.RollupAddress == (common.Address{}) { | |
| return nil, fmt.Errorf("Rollup contract address is required") | |
| } | |
| l1Client, err := ethclient.Dial(config.L1Addr) | |
| if err != nil { | |
| return nil, fmt.Errorf("failed to connect to L1: %w", err) | |
| } | |
| rollup, err := bindings.NewRollup(config.RollupAddress, l1Client) | |
| if err != nil { | |
| l1Client.Close() | |
| return nil, fmt.Errorf("failed to create rollup binding: %w", err) | |
| } | |
| ctx, cancel := context.WithCancel(ctx) | |
| return &BlockTagService{ | |
| ctx: ctx, | |
| cancel: cancel, | |
| l1Client: l1Client, | |
| l2Client: l2Client, | |
| rollup: rollup, | |
| rollupAddress: config.RollupAddress, | |
| safeConfirmations: config.SafeConfirmations, | |
| pollInterval: config.PollInterval, | |
| logger: logger.With("module", "blocktag"), | |
| stop: make(chan struct{}), | |
| }, nil | |
| } |
🤖 Prompt for AI Agents
In `@node/blocktag/service.go` around lines 63 - 101, In NewBlockTagService, if
bindings.NewRollup fails the previously created l1Client is leaked; ensure
l1Client is closed before returning the error (e.g., close l1Client when rollup
creation returns an error or use a deferred cleanup tied to the rollup creation
step) so that l1Client is not left open on failure.
tomatoishealthy
force-pushed
the
feat/block_tag
branch
2 times, most recently
from
268bb59 to
68ac8b6
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@node/blocktag/service.go`:
- Around line 177-181: The current logic can overwrite the safe block when
getL2BlockForTag(TagTypeSafe, ...) fails because safeBlockNum stays 0; modify
the flow in the function that calls getL2BlockForTag to track whether the safe
block was successfully retrieved (e.g., a boolean safeFound or initialize
safeBlockNum to -1) and only perform the comparison/assignment to safeBlockHash
and call s.setSafeL2Block(...) if the safe retrieval succeeded; reference
getL2BlockForTag(TagTypeSafe,...), safeBlockNum, finalizedBlockNum,
safeBlockHash and s.setSafeL2Block to locate where to add the check and guard
the finalize-vs-safe overwrite.
♻️ Duplicate comments (1)
node/blocktag/service.go (1)
82-85: Resource leak:l1Clientnot closed onNewRollupfailure.If
bindings.NewRollupfails, thel1Clientcreated at line 77 remains open.🔧 Suggested fix
rollup, err := bindings.NewRollup(config.RollupAddress, l1Client) if err != nil { + l1Client.Close() return nil, fmt.Errorf("failed to create rollup binding: %w", err) }
🧹 Nitpick comments (3)
node/blocktag/config.go (1)
38-54: Consider:--mainnetflag takes precedence over explicit--derivation.rollupAddress.The current logic prioritizes
--mainnetover an explicitly provided rollup address. If both flags are set, the explicit address is silently ignored. This may be intentional, but could surprise users.Also,
PollIntervalhas no CLI flag to override the default—consider adding one for operational flexibility if needed.♻️ Optional: Warn or error if both flags are set
func (c *Config) SetCliContext(ctx *cli.Context) error { c.L1Addr = ctx.GlobalString(flags.L1NodeAddr.Name) // Determine RollupAddress: use explicit flag, or mainnet default, or error - if ctx.GlobalBool(flags.MainnetFlag.Name) { + isMainnet := ctx.GlobalBool(flags.MainnetFlag.Name) + hasExplicitRollup := ctx.GlobalIsSet(flags.RollupContractAddress.Name) + + if isMainnet && hasExplicitRollup { + // Log warning or return error about conflicting flags + } + + if isMainnet { c.RollupAddress = node.MainnetRollupContractAddress - } else if ctx.GlobalIsSet(flags.RollupContractAddress.Name) { + } else if hasExplicitRollup { c.RollupAddress = common.HexToAddress(ctx.GlobalString(flags.RollupContractAddress.Name)) } else {node/blocktag/service.go (2)
273-277: Consider usingErrorlog level for failed L1 query.Failing to retrieve
LastFinalizedBatchIndexis an unexpected condition that could mask state root mismatches. UsingInfolevel may cause this to go unnoticed in production.- s.logger.Info("Failed to get last finalized batch index, skipping state root validation", "error", err) + s.logger.Error("Failed to get last finalized batch index, skipping state root validation", "error", err)
385-388: Consider logging errors during binary search.Silently returning on RPC errors could mask intermittent L1 connectivity issues or contract call failures.
batchData, err := s.rollup.BatchDataStore(nil, big.NewInt(int64(mid))) if err != nil { + s.logger.Debug("Binary search batch lookup failed", "batchIndex", mid, "error", err) return result // Return best result so far on error }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
node/blocktag/config.gonode/blocktag/service.gonode/cmd/node/main.gonode/core/config.gonode/flags/flags.gonode/types/retryable_client.go
🧰 Additional context used
🧬 Code graph analysis (3)
node/cmd/node/main.go (2)
node/blocktag/service.go (2)
BlockTagService(33-61)NewBlockTagService(64-101)node/blocktag/config.go (1)
DefaultConfig(30-35)
node/blocktag/service.go (2)
node/types/retryable_client.go (1)
RetryableClient(29-34)node/blocktag/config.go (1)
Config(22-27)
node/blocktag/config.go (3)
oracle/flags/flags.go (1)
RollupAddress(35-40)node/core/config.go (1)
MainnetRollupContractAddress(30-30)node/flags/flags.go (4)
L1NodeAddr(63-68)MainnetFlag(238-241)RollupContractAddress(188-192)BlockTagSafeConfirmations(225-230)
🪛 GitHub Actions: Tx-submitter
node/types/retryable_client.go
[error] 227-227: Build error during 'make build': rc.authClient.SetBlockTags undefined (type *authclient.Client has no field or method SetBlockTags).
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: check
- GitHub Check: check
- GitHub Check: test
- GitHub Check: Analyze (rust)
- GitHub Check: Analyze (go)
🔇 Additional comments (15)
node/core/config.go (1)
28-31: LGTM!The mainnet rollup contract address constant is properly added, following the existing naming convention for mainnet-specific values. This provides a centralized reference for the BlockTag service configuration.
node/flags/flags.go (3)
63-68: LGTM!Making
L1NodeAddrrequired addresses the flag inconsistency issue whereBlockTagServicerequires the L1 RPC address but it was previously optional.
224-230: LGTM!The new
BlockTagSafeConfirmationsflag is properly defined with a sensible default value (10) that matchesDefaultSafeConfirmationsin the blocktag config.
350-352: LGTM!The flag is correctly wired into the global
Flagsslice.node/cmd/node/main.go (2)
186-188: LGTM!Proper nil-check before stopping the service ensures graceful shutdown regardless of which code path was taken during startup.
147-159: BlockTagService integration is correctly structured with no blocking dependencies.The service lifecycle management is correct:
- Configuration is populated from CLI context with proper error handling for required fields
- Service is created and started with error propagation
- Shutdown path properly checks for nil before stopping
The
SetBlockTagsmethod is implemented inRetryableClientand fully functional. The code builds successfully and should execute without issues. TheRollupContractAddressvalidation is appropriately handled byblocktag.Config.SetCliContext—either--mainnetor--derivation.rollupAddressmust be provided, with a clear error message if neither is set.Note:
L1NodeAddris not validated in the config and can be empty, though this will fail naturally at RPC connection time.node/blocktag/config.go (1)
14-27: LGTM!Constants and struct are well-defined. The defaults (10 confirmations, 10s poll interval) are reasonable starting points.
node/types/retryable_client.go (1)
225-240: Unable to verify the critical build failure claim from the codebase. TheSetBlockTagsmethod implementation follows the identical retry pattern as all 10 other methods inRetryableClient(AssembleL2Block, ValidateL2Block, NewL2Block, NewSafeL2Block, CommitBatch, AppendBlsSignature, BlockNumber, HeaderByNumber, CallContract, CodeAt), and the method is already being called innode/blocktag/service.go(line 453).The external
authclient.Clienttype comes fromgithub.com/morph-l2/go-ethereum v1.10.14-0.20251219060125-03910bc750a2as specified innode/go.mod. The go-ethereum submodule in this repository is empty in the sandbox, which prevents direct verification of the method definition. Confirm whether the actual build pipeline reports this error and, if so, check the go-ethereum fork repository directly to determine if theSetBlockTagsmethod exists onauthclient.Client.node/blocktag/service.go (7)
103-127: LGTM!The Start/Stop lifecycle management is well-implemented with proper context cancellation and graceful shutdown via the stop channel synchronization.
135-152: LGTM!Clean polling loop implementation with proper context cancellation handling and ticker cleanup.
199-267: LGTM!Well-structured method with clear separation of safe vs finalized tag handling, appropriate validation, and good logging.
316-361: LGTM!The recursion depth limit effectively prevents the infinite loop issue. The caching strategy with fallback to binary search is a reasonable optimization.
401-419: LGTM!The block number handling correctly distinguishes between the special
FinalizedBlockNumberconstant and actual block heights.
421-435: LGTM!Clean change-detection pattern that avoids redundant logging.
437-461: LGTM!Well-implemented notification deduplication that avoids redundant RPC calls and correctly handles the empty state.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| // If finalized > safe, update safe to finalized (finalized is a stronger state) | ||
| if finalizedBlockNum > safeBlockNum { | ||
| safeBlockHash = finalizedBlockHash | ||
| s.setSafeL2Block(safeBlockHash) | ||
| } |
There was a problem hiding this comment.
Logic flaw when safe block retrieval fails.
When getL2BlockForTag(TagTypeSafe, ...) returns an error (line 166), safeBlockNum remains 0. If finalized retrieval succeeds with any positive block number, the condition finalizedBlockNum > safeBlockNum at line 178 will always be true, incorrectly overwriting the safe block even though the actual safe block wasn't determined.
🔧 Suggested fix
var safeBlockNum uint64
var safeBlockHash common.Hash
+ var safeBlockValid bool
// Update safe block
safeBlockNum, safeBlockHash, err = s.getL2BlockForTag(TagTypeSafe, l2Head)
if err != nil {
s.logger.Error("Failed to get safe L2 block", "error", err)
} else if safeBlockHash != (common.Hash{}) {
s.setSafeL2Block(safeBlockHash)
+ safeBlockValid = true
}
// Update finalized block
finalizedBlockNum, finalizedBlockHash, err := s.getL2BlockForTag(TagTypeFinalized, l2Head)
if err != nil {
s.logger.Error("Failed to get finalized L2 block", "error", err)
} else if finalizedBlockHash != (common.Hash{}) {
// If finalized > safe, update safe to finalized (finalized is a stronger state)
- if finalizedBlockNum > safeBlockNum {
+ if safeBlockValid && finalizedBlockNum > safeBlockNum {
safeBlockHash = finalizedBlockHash
s.setSafeL2Block(safeBlockHash)
}
s.setFinalizedL2Block(finalizedBlockHash)
}📝 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 finalized > safe, update safe to finalized (finalized is a stronger state) | |
| if finalizedBlockNum > safeBlockNum { | |
| safeBlockHash = finalizedBlockHash | |
| s.setSafeL2Block(safeBlockHash) | |
| } | |
| var safeBlockNum uint64 | |
| var safeBlockHash common.Hash | |
| var safeBlockValid bool | |
| // Update safe block | |
| safeBlockNum, safeBlockHash, err = s.getL2BlockForTag(TagTypeSafe, l2Head) | |
| if err != nil { | |
| s.logger.Error("Failed to get safe L2 block", "error", err) | |
| } else if safeBlockHash != (common.Hash{}) { | |
| s.setSafeL2Block(safeBlockHash) | |
| safeBlockValid = true | |
| } | |
| // Update finalized block | |
| finalizedBlockNum, finalizedBlockHash, err := s.getL2BlockForTag(TagTypeFinalized, l2Head) | |
| if err != nil { | |
| s.logger.Error("Failed to get finalized L2 block", "error", err) | |
| } else if finalizedBlockHash != (common.Hash{}) { | |
| // If finalized > safe, update safe to finalized (finalized is a stronger state) | |
| if safeBlockValid && finalizedBlockNum > safeBlockNum { | |
| safeBlockHash = finalizedBlockHash | |
| s.setSafeL2Block(safeBlockHash) | |
| } | |
| s.setFinalizedL2Block(finalizedBlockHash) | |
| } |
🤖 Prompt for AI Agents
In `@node/blocktag/service.go` around lines 177 - 181, The current logic can
overwrite the safe block when getL2BlockForTag(TagTypeSafe, ...) fails because
safeBlockNum stays 0; modify the flow in the function that calls
getL2BlockForTag to track whether the safe block was successfully retrieved
(e.g., a boolean safeFound or initialize safeBlockNum to -1) and only perform
the comparison/assignment to safeBlockHash and call s.setSafeL2Block(...) if the
safe retrieval succeeded; reference getL2BlockForTag(TagTypeSafe,...),
safeBlockNum, finalizedBlockNum, safeBlockHash and s.setSafeL2Block to locate
where to add the check and guard the finalize-vs-safe overwrite.
68ac8b6 to
d485931
Compare
Summary by CodeRabbit
New Features
Configuration
✏️ Tip: You can customize this high-level summary in your review settings.