Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: update tryFinalize() to check finality on each L2 block #23

Merged
merged 19 commits into from
Jul 26, 2024
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ go 1.21

require (
github.com/andybalholm/brotli v1.1.0
github.com/babylonchain/babylon-finality-gadget v0.1.3-alpha.0.20240716025522-a7f8cd19f44f
github.com/babylonchain/babylon-finality-gadget v0.1.3-alpha.0.20240725201932-fe0411e65930
github.com/btcsuite/btcd v0.24.2
github.com/btcsuite/btcd/chaincfg/chainhash v1.1.0
github.com/cockroachdb/pebble v1.1.0
Expand Down Expand Up @@ -42,6 +42,7 @@ require (
github.com/prometheus/client_golang v1.19.1
github.com/stretchr/testify v1.9.0
github.com/urfave/cli/v2 v2.27.1
go.uber.org/mock v0.4.0
golang.org/x/crypto v0.24.0
golang.org/x/exp v0.0.0-20240506185415-9bf2ced13842
golang.org/x/sync v0.7.0
Expand Down Expand Up @@ -350,7 +351,6 @@ require (
go.uber.org/automaxprocs v1.5.2 // indirect
go.uber.org/dig v1.17.1 // indirect
go.uber.org/fx v1.21.1 // indirect
go.uber.org/mock v0.4.0 // indirect
go.uber.org/multierr v1.11.0 // indirect
go.uber.org/zap v1.27.0 // indirect
golang.org/x/mod v0.17.0 // indirect
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -292,8 +292,8 @@ github.com/aws/aws-sdk-go v1.44.122/go.mod h1:y4AeaBuwd2Lk+GepC1E9v0qOiTws0MIWAX
github.com/aws/aws-sdk-go v1.44.312 h1:llrElfzeqG/YOLFFKjg1xNpZCFJ2xraIi3PqSuP+95k=
github.com/aws/aws-sdk-go v1.44.312/go.mod h1:aVsgQcEevwlmQ7qHE9I3h+dtQgpqhFB+i8Phjh7fkwI=
github.com/aws/aws-sdk-go-v2 v0.18.0/go.mod h1:JWVYvqSMppoMJC0x5wdwiImzgXTI9FuZwxzkQq9wy+g=
github.com/babylonchain/babylon-finality-gadget v0.1.3-alpha.0.20240716025522-a7f8cd19f44f h1:FykJmDmmw5CypzzNBskqDugs9HvIXpJ+vxa0J0jzPlA=
github.com/babylonchain/babylon-finality-gadget v0.1.3-alpha.0.20240716025522-a7f8cd19f44f/go.mod h1:UkRb1walvNDdEF4fcJVz+M1t7Ok62PkayI40BtxYOGI=
github.com/babylonchain/babylon-finality-gadget v0.1.3-alpha.0.20240725201932-fe0411e65930 h1:AEhnrx3+H30mTwGDYSwNHP/DbbH1+QgHU5KC8TW6+oY=
github.com/babylonchain/babylon-finality-gadget v0.1.3-alpha.0.20240725201932-fe0411e65930/go.mod h1:RG4+gysH91XhqxvmJf2uDtLtFHNZNW3Jg8BkjKdnGvA=
github.com/babylonchain/babylon-private v0.8.6-0.20240705135310-e91ff7f60ead h1:LyyrFtdSbx0a5ZLHA/qMNMjAxZltfnNJgj5+uEgqZdI=
github.com/babylonchain/babylon-private v0.8.6-0.20240705135310-e91ff7f60ead/go.mod h1:Tdi+29Y+DzCaaz0V0sBRXF1tXXLnH6JHJsOG8uktWLU=
github.com/benbjohnson/clock v1.1.0/go.mod h1:J11/hYXuz8f4ySSvYwY0FKfm+ezbsZBKZxNJlLklBHA=
Expand Down
31 changes: 18 additions & 13 deletions op-node/rollup/finality/finalizer.go
Original file line number Diff line number Diff line change
Expand Up @@ -245,11 +245,13 @@ func (fi *Finalizer) tryFinalize() {
// go through the latest inclusion data, and find the last L2 block that was derived from a finalized L1 block
for _, fd := range fi.finalityData {
if fd.L2Block.Number > finalizedL2.Number && fd.L1Block.Number <= fi.finalizedL1.Number {
shouldContinue, err := tryFinalizeOnConsecutiveBlockQuorom(fd, fi, &finalizedL2, &finalizedDerivedFrom)
lastFinalizedBlockNumber, err := tryFinalizeOnConsecutiveBlockQuorom(fd, fi, &finalizedL2, &finalizedDerivedFrom)
bap2pecs marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return
Copy link
Collaborator

Choose a reason for hiding this comment

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

thinking deeper on it. "return on err" is inappropriate now. if you see the below logic where if finalizedDerivedFrom != (eth.BlockID{}) { is checked, whenever finalizedDerivedFrom is set, we have to do some sanity checks.

so we need to:

  • remove the "return on error" logic
  • findLastFinalizedL2BlockWithConsecutiveQuorom doesn't need to return hasErr now

Copy link
Collaborator

Choose a reason for hiding this comment

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

fixed in b28870d

}
if !shouldContinue {
// some blocks in the queried range is not BTC finalized, stop iterating to honor consecutive quorom
// all queried blocks are BTC finalized, continue iterating
bap2pecs marked this conversation as resolved.
Show resolved Hide resolved
if lastFinalizedBlockNumber == nil || *lastFinalizedBlockNumber != fd.L2Block.Number {
break
}
}
Expand Down Expand Up @@ -307,7 +309,7 @@ func tryFinalizeOnConsecutiveBlockQuorom(
fi *Finalizer,
finalizedL2 *eth.L2BlockRef,
finalizedDerivedFrom *eth.BlockID,
) (bool, error) {
) (*uint64, error) {
blockCount := int(fd.L2Block.Number - finalizedL2.Number)
l2Blocks := make(map[uint64]eth.L2BlockRef)
queryBlocks := make([]*cwclient.L2Block, blockCount)
Expand All @@ -323,7 +325,7 @@ func tryFinalizeOnConsecutiveBlockQuorom(
blockNumber,
err,
)})
return false, err
return nil, err
}
l2Blocks[blockNumber] = l2Block

Expand All @@ -345,13 +347,22 @@ func tryFinalizeOnConsecutiveBlockQuorom(
// TODO: we shouldn't skip on no voting power.
// https://github.com/babylonchain/babylon-finality-gadget/issues/59
if err != nil && !errors.Is(err, sdkclient.ErrNoFpHasVotingPower) {
bap2pecs marked this conversation as resolved.
Show resolved Hide resolved
if lastFinalizedBlockNumber != nil {
// set finalized block(s)
finalizedL2Block := l2Blocks[*lastFinalizedBlockNumber]
fi.log.Debug("set finalized block", "l2_block", finalizedL2Block)
*finalizedL2 = finalizedL2Block
*finalizedDerivedFrom = fd.L1Block

return lastFinalizedBlockNumber, nil
bap2pecs marked this conversation as resolved.
Show resolved Hide resolved
}
fi.emitter.Emit(rollup.CriticalErrorEvent{Err: fmt.Errorf(
"failed to check if block %d to %d is finalized on Babylon: %w",
finalizedL2.Number+1,
fd.L2Block.Number,
err,
)})
return false, err
return nil, err
}

// stop iterating to honor consecutive quorom
Expand All @@ -361,7 +372,7 @@ func tryFinalizeOnConsecutiveBlockQuorom(
"l2_block_start", finalizedL2.Number+1,
"l2_block_end", fd.L2Block.Number,
)
return false, nil
return nil, nil
}

// set finalized block(s)
Expand All @@ -370,13 +381,7 @@ func tryFinalizeOnConsecutiveBlockQuorom(
*finalizedL2 = finalizedL2Block
*finalizedDerivedFrom = fd.L1Block

// some blocks in the queried range is not BTC finalized, stop iterating to honor consecutive quorom
if finalizedL2.Number != fd.L2Block.Number {
return false, nil
}

// all queried blocks are BTC finalized, continue iterating
return true, nil
return lastFinalizedBlockNumber, nil
}

// onDerivedSafeBlock buffers the L1 block the safe head was fully derived from,
Expand Down
Loading