Skip to content

Commit

Permalink
85 faultproof withdrawals does not report connection errors metrics c…
Browse files Browse the repository at this point in the history
…orrectly (#90)

* fixed bug, of not setting correct number of nodeconnection errors on metrics

* fixed bugs and added features. Still missing proper metrics

* fixing metrics

* fixing metrics

* fixes

* completing changes

* Update op-monitorism/faultproof_withdrawals/validator/optimism_portal2_helper.go

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* fixing review issues

* fixing review issues

* fixing review issues

* fixing review issues

* fixing review issues

* fixing review issues

* Update op-monitorism/faultproof_withdrawals/monitor_live_sepolia_test.go

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* Update op-monitorism/faultproof_withdrawals/monitor_live_sepolia_test.go

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* fixing review issues

* Update op-monitorism/faultproof_withdrawals/monitor_live_mainnet_test.go

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* fixing review issues

* fixing review issues

* Apply suggestions from code review

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* fixing review issues

* fixing review issues

* fixing tests

* fixing libraries

* fixing minor issues

* Update state.go

Adding extra comment description

* fixing comment

* fxing one piece of code

* cleaning up and fixing documentation

---------

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
  • Loading branch information
raffaele-oplabs and coderabbitai[bot] authored Oct 10, 2024
1 parent b2fe04b commit 88bb56c
Show file tree
Hide file tree
Showing 15 changed files with 709 additions and 426 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ This component is designed to work exclusively with chains that are already util
This is a new version of the deprecated [chain-mon faultproof-wd-mon](https://github.com/ethereum-optimism/optimism/tree/chain-mon/v1.2.1/packages/chain-mon/src/faultproof-wd-mon).
For detailed information on how the component works and the algorithms used, please refer to the component README.

| `op-monitorism/faultproof-withdrawals` | [README](https://github.com/ethereum-optimism/monitorism/blob/main/op-monitorism/faultproof-withdrawals/README.md) |
| `op-monitorism/faultproof-withdrawals` | [README](https://github.com/ethereum-optimism/monitorism/blob/main/op-monitorism/faultproof_withdrawals/README.md) |
| ----------------------- | --------------------------------------------------------------------------------------------------- |


Expand Down
146 changes: 0 additions & 146 deletions op-monitorism/README.md

This file was deleted.

9 changes: 9 additions & 0 deletions op-monitorism/faultproof_withdrawals/.env.op.mainnet.example
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
FAULTPROOF_WITHDRAWAL_MON_L1_GETH_URL="<geth-url>"
FAULTPROOF_WITHDRAWAL_MON_L2_OP_NODE_URL="<op-geth-node>"
FAULTPROOF_WITHDRAWAL_MON_L2_OP_GETH_URL="<op-geth-url>"
FAULTPROOF_WITHDRAWAL_MON_OPTIMISM_PORTAL="0xbEb5Fc579115071764c7423A4f12eDde41f106Ed" # This is the address of the Optimism portal contract, this should be for the chain you are monitoring
FAULTPROOF_WITHDRAWAL_MON_START_BLOCK_HEIGHT=20872390 # This is the block height from which the monitoring will start, decide at which block height you want to start monitoring
FAULTPROOF_WITHDRAWAL_MON_EVENT_BLOCK_RANGE=1000 # This is the range of blocks to be monitored
MONITORISM_LOOP_INTERVAL_MSEC=100 # This is the interval in milliseconds for the monitoring loop
MONITORISM_METRICS_PORT=7300 # This is the port on which the metrics server will run
MONITORISM_METRICS_ENABLED=true # This is the flag to enable/disable the metrics server
2 changes: 0 additions & 2 deletions op-monitorism/faultproof_withdrawals/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ or
cd ../
go run ./cmd/monitorism faultproof_withdrawals --metrics.enabled --metrics.port 7300
```
## Available Metrics and Meaning


# Cli options

Expand Down
84 changes: 32 additions & 52 deletions op-monitorism/faultproof_withdrawals/monitor.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/ethereum-optimism/monitorism/op-monitorism/faultproof_withdrawals/validator"
"github.com/ethereum-optimism/optimism/op-service/metrics"

"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/ethclient"
"github.com/ethereum/go-ethereum/log"
)
Expand Down Expand Up @@ -123,14 +124,14 @@ func NewMonitor(ctx context.Context, log log.Logger, m metrics.Factory, cfg CLIC
startingL1BlockHeight = uint64(cfg.StartingL1BlockHeight)
}

state, err := NewState(log, startingL1BlockHeight, latestL1Height)
state, err := NewState(log, startingL1BlockHeight, latestL1Height, ret.withdrawalValidator.GetLatestL2Height())
if err != nil {
return nil, fmt.Errorf("failed to create state: %w", err)
}
ret.state = *state

// log state and metrics
ret.state.LogState(ret.log)
ret.state.LogState()
ret.metrics.UpdateMetricsFromState(&ret.state)

return ret, nil
Expand Down Expand Up @@ -228,6 +229,10 @@ func (m *Monitor) GetMaxBlock() (uint64, error) {
// Run executes the main monitoring loop.
// It retrieves new events, processes them, and updates the state accordingly.
func (m *Monitor) Run(ctx context.Context) {
// Defer the update function
defer m.metrics.UpdateMetricsFromState(&m.state)
defer m.state.LogState()

start := m.state.nextL1Height

stop, err := m.GetMaxBlock()
Expand All @@ -238,18 +243,16 @@ func (m *Monitor) Run(ctx context.Context) {
}

// review previous invalidProposalWithdrawalsEvents
invalidProposalWithdrawalsEvents, err := m.ConsumeEvents(m.state.invalidProposalWithdrawalsEvents)
err = m.ConsumeEvents(m.state.potentialAttackOnInProgressGames)
if err != nil {
m.state.nodeConnectionFailures++
m.log.Error("failed to consume events", "error", err)
return
}

// update state
m.state.invalidProposalWithdrawalsEvents = *invalidProposalWithdrawalsEvents

// get new events

newEvents, err := m.withdrawalValidator.GetEnrichedWithdrawalsEvents(start, &stop)
m.log.Info("getting enriched withdrawal events", "start", fmt.Sprintf("%d", start), "stop", fmt.Sprintf("%d", stop))
newEvents, err := m.withdrawalValidator.GetEnrichedWithdrawalsEventsMap(start, &stop)
if err != nil {
if start >= stop {
m.log.Info("no new events to process", "start", start, "stop", stop)
Expand All @@ -262,99 +265,76 @@ func (m *Monitor) Run(ctx context.Context) {
}
return
}
newInvalidProposalWithdrawalsEvents, err := m.ConsumeEvents(newEvents)

err = m.ConsumeEvents(newEvents)
if err != nil {
m.state.nodeConnectionFailures++
m.log.Error("failed to consume events", "error", err)
return
}

// update state
if len(*newInvalidProposalWithdrawalsEvents) > 0 && newInvalidProposalWithdrawalsEvents != nil {
m.state.invalidProposalWithdrawalsEvents = append(m.state.invalidProposalWithdrawalsEvents, *newInvalidProposalWithdrawalsEvents...)
}

// update state
m.state.nextL1Height = stop

// log state and metrics
m.state.LogState(m.log)
m.metrics.UpdateMetricsFromState(&m.state)
}

// ConsumeEvents processes a slice of enriched withdrawal events and updates their states.
// It returns any events detected during the consumption that requires to be re-analysed again at a later stage (when the event referenced DisputeGame completes).
func (m *Monitor) ConsumeEvents(enrichedWithdrawalEvent []validator.EnrichedProvenWithdrawalEvent) (*[]validator.EnrichedProvenWithdrawalEvent, error) {
var newForgeriesGameInProgressEvent []validator.EnrichedProvenWithdrawalEvent = make([]validator.EnrichedProvenWithdrawalEvent, 0)
for _, enrichedWithdrawalEvent := range enrichedWithdrawalEvent {
func (m *Monitor) ConsumeEvents(enrichedWithdrawalEvents map[common.Hash]validator.EnrichedProvenWithdrawalEvent) error {
for _, enrichedWithdrawalEvent := range enrichedWithdrawalEvents {
m.log.Info("processing withdrawal event", "event", &enrichedWithdrawalEvent)
err := m.withdrawalValidator.UpdateEnrichedWithdrawalEvent(&enrichedWithdrawalEvent)
//upgrade state to the latest L2 height after the event is processed
m.state.latestL2Height = m.withdrawalValidator.GetLatestL2Height()
if err != nil {
m.state.nodeConnectionFailures++
m.log.Error("failed to update enriched withdrawal event", "error", err)
return nil, err
return err
}

consumedEvent, err := m.ConsumeEvent(enrichedWithdrawalEvent)
err = m.ConsumeEvent(enrichedWithdrawalEvent)
if err != nil {
m.log.Error("failed to consume event", "error", err)
return nil, err
} else if !consumedEvent {
newForgeriesGameInProgressEvent = append(newForgeriesGameInProgressEvent, enrichedWithdrawalEvent)
return err
}
}

return &newForgeriesGameInProgressEvent, nil
return nil
}

// ConsumeEvent processes a single enriched withdrawal event.
// It logs the event details and checks for any forgery detection.
func (m *Monitor) ConsumeEvent(enrichedWithdrawalEvent validator.EnrichedProvenWithdrawalEvent) (bool, error) {
m.log.Info("processing withdrawal event", "event", enrichedWithdrawalEvent.Event)
func (m *Monitor) ConsumeEvent(enrichedWithdrawalEvent validator.EnrichedProvenWithdrawalEvent) error {
if enrichedWithdrawalEvent.DisputeGame.DisputeGameData.L2ChainID.Cmp(m.l2ChainID) != 0 {
m.log.Error("l2ChainID mismatch", "expected", fmt.Sprintf("%d", m.l2ChainID), "got", fmt.Sprintf("%d", enrichedWithdrawalEvent.DisputeGame.DisputeGameData.L2ChainID))
}
valid, err := m.withdrawalValidator.IsWithdrawalEventValid(&enrichedWithdrawalEvent)
if err != nil {
m.state.nodeConnectionFailures++
m.log.Error("failed to check if forgery detected", "error", err)
return false, err
return err
}
eventConsumed := false

if !valid {
m.state.numberOfInvalidWithdrawals++
if !enrichedWithdrawalEvent.Blacklisted {
if enrichedWithdrawalEvent.DisputeGame.DisputeGameData.Status == validator.CHALLENGER_WINS {
m.log.Warn("WITHDRAWAL: is NOT valid, but the game is correctly resolved", "enrichedWithdrawalEvent", enrichedWithdrawalEvent)
m.state.withdrawalsValidated++
eventConsumed = true
m.state.IncrementSuspiciousEventsOnChallengerWinsGames(enrichedWithdrawalEvent)
} else if enrichedWithdrawalEvent.DisputeGame.DisputeGameData.Status == validator.DEFENDER_WINS {
m.log.Error("WITHDRAWAL: is NOT valid, forgery detected", "enrichedWithdrawalEvent", enrichedWithdrawalEvent)
m.state.numberOfDetectedForgery++
// add to forgeries
m.state.forgeriesWithdrawalsEvents = append(m.state.forgeriesWithdrawalsEvents, enrichedWithdrawalEvent)
eventConsumed = true
m.state.IncrementPotentialAttackOnDefenderWinsGames(enrichedWithdrawalEvent)
} else if enrichedWithdrawalEvent.DisputeGame.DisputeGameData.Status == validator.IN_PROGRESS {
m.log.Warn("WITHDRAWAL: is NOT valid, game is still in progress.", "enrichedWithdrawalEvent", enrichedWithdrawalEvent)
m.state.IncrementPotentialAttackOnInProgressGames(enrichedWithdrawalEvent)
// add to events to be re-processed
eventConsumed = false
} else {
m.log.Error("WITHDRAWAL: is NOT valid, game status is unknown. UNKNOWN STATE SHOULD NEVER HAPPEN", "enrichedWithdrawalEvent", enrichedWithdrawalEvent)
eventConsumed = false
}

} else {
m.log.Warn("WITHDRAWAL: is NOT valid, but game is blacklisted", "enrichedWithdrawalEvent", enrichedWithdrawalEvent)
m.state.withdrawalsValidated++
eventConsumed = true
m.state.IncrementSuspiciousEventsOnChallengerWinsGames(enrichedWithdrawalEvent)
}
} else {
m.log.Info("WITHDRAWAL: is valid", "enrichedWithdrawalEvent", enrichedWithdrawalEvent)
m.state.withdrawalsValidated++
eventConsumed = true
m.state.IncrementWithdrawalsValidated(enrichedWithdrawalEvent)
}
m.state.processedProvenWithdrawalsExtension1Events++
m.state.eventsProcessed++
m.metrics.UpdateMetricsFromState(&m.state)
return eventConsumed, nil
return nil
}

// Close gracefully shuts down the Monitor by closing the Geth clients.
Expand Down
Loading

0 comments on commit 88bb56c

Please sign in to comment.