opstackl2: temporarily disable Finality Gadget dependency #504
Conversation
There was a problem hiding this comment.
Pull Request Overview
A quick temporary change to disable reliance on the Finality Gadget and return a fixed activation height.
- Removed Finality Gadget gRPC client import and related logic in
QueryActivatedHeight - Replaced activation-height lookup with a forced return of 0 and a TODO reference
- Marked this change as temporary, with proper fix tracked in issue #503
Comments suppressed due to low confidence (2)
clientcontroller/opstackl2/consumer.go:449
- Consider expanding this TODO to include a brief description of why the temporary override is safe and what the eventual implementation should handle (e.g., timestamp mapping), to aid future maintainers.
// TODO(babylonlabs-io/finality-provider#503): Replace forced return with proper activation height logic without relying on finality gadget
clientcontroller/opstackl2/consumer.go:448
- Since the behavior of
QueryActivatedHeighthas changed to always return0, add or update unit tests to validate this temporary override and ensure downstream components handle it correctly.
func (cc *OPStackL2ConsumerController) QueryActivatedHeight() (uint64, error) {
|
|
||
| return l2BlockNumber, nil | ||
| // TODO(babylonlabs-io/finality-provider#503): Replace forced return with proper activation height logic without relying on finality gadget | ||
| return 0, nil |
There was a problem hiding this comment.
Using a magic number (0) here can be confusing; introduce a named constant (e.g., DefaultActivationHeight) to document that this is a placeholder.
| return 0, nil | |
| return DefaultActivationHeight, nil |
|
Given that none of the BSNs wants to rely on finality gadgets for getting the activated height, I recommend we proceed with this PR (i.e., always starting with 0 height) in order to get rid of the finality gadget dependency, and then add a config entry for setting an activated height in a subsequent PR. Wdyt @0xAleksaOpacic ? |
I still don't understand concept of activation hight. Can you write setnance or two just to make sure we are aligned before we make decision cc @SebastianElvis |
In Babylon chain, the activation height is the height since which the chain starts receiving BTC staking finality, i.e., submitting finality signatures are allowed and the chain will tally these finality signatures. In the finality provider implementation, it will only submit finality signatures since this height. The actviation height in the BSN context, however, depends on the implementation of the finality gadget. A proper design would be to defer the responsibility to set the right actviation height to individual finality providers. Wdyt? |
|
Fixed in #522 by hardcoding 0 as activation height. Will work on a proper fix in subsequent PR |
Description
This PR is a quick fix to unblock testing, as we cannot rely on the Finality Gadget being present in all setups.
Note: This is a temporary change and should not be merged. The proper fix is tracked in #503.