babylonchain / babylon-finality-gadget

3 stars 3 forks source link

fix: check BTC staking activated instead of using total voting power as the indicator #59

Closed bap2pecs closed 2 months ago

bap2pecs commented 3 months ago

Context

38 was added b/c we found that the OP chain will get stuck before the first delegation becomes active. The reason why it will get stuck is when CriticalErrorEvent is emitted, op-e2e/actions/l2_verifier.go will detect it and panic:

func (s *L2Verifier) OnEvent(ev rollup.Event) {
    switch x := ev.(type) {
    ...
    case rollup.CriticalErrorEvent:
        panic(fmt.Errorf("derivation failed critically: %w", x.Err))
    ...
}

but per https://github.com/babylonchain/babylon-finality-gadget/pull/38#issuecomment-2231526118, this creates a security vuln

instead, we should query the timestamp when BTC staking protocol is activated (example)

https://github.com/babylonchain/finality-provider/issues/432 is also related

Tasks

bap2pecs commented 3 months ago

per my discussoin w @SebastianElvis, we don't have ActivatedHeight API in the consumer query client is by design

as Babylon is not responsible for monitoring the activated status of BTC staking of other consumers. Rather, consumer is repsonsible for defining "activated" for itself (the definition can be different from Babylon) and monitoring its activation status by itself

As an example, Babylon contract follows the same def as Babylon and is handled here https://github.com/babylonchain/babylon-contract/blob/main/contracts/btc-staking/src/contract.rs#L205-L218

so we should modify the task list accordingly cc @lesterli

bap2pecs commented 2 months ago

Kamino closed and cloned this issue to babylonlabs-io/babylon-finality-gadget