From 27f97044032c1a841fe0644046a81599d598759c Mon Sep 17 00:00:00 2001 From: Maurelian Date: Wed, 23 Oct 2024 14:08:32 -0400 Subject: [PATCH 1/5] chore: Remove unused script (#12597) It has been replaced with a go script. https://github.com/ethereum-optimism/optimism/blob/rm/check-ifaces-sh/packages/contracts-bedrock/scripts/checks/interfaces/main.go#L1 --- .../scripts/checks/check-interfaces.sh | 286 ------------------ 1 file changed, 286 deletions(-) delete mode 100755 packages/contracts-bedrock/scripts/checks/check-interfaces.sh diff --git a/packages/contracts-bedrock/scripts/checks/check-interfaces.sh b/packages/contracts-bedrock/scripts/checks/check-interfaces.sh deleted file mode 100755 index a2cda470d2a8..000000000000 --- a/packages/contracts-bedrock/scripts/checks/check-interfaces.sh +++ /dev/null @@ -1,286 +0,0 @@ -#!/usr/bin/env bash -set -euo pipefail - -# Warn users of Mac OSX who have not ever upgraded bash from the default that they may experience -# performance issues. -if [ "${BASH_VERSINFO[0]}" -lt 5 ]; then - echo "WARNING: your bash installation is very old, and may cause this script to run extremely slowly. Please upgrade bash to at least version 5 if you have performance issues." -fi - -# This script checks for ABI consistency between interfaces and their corresponding contracts. -# It compares the ABIs of interfaces (files starting with 'I') with their implementation contracts, -# excluding certain predefined files. Constructors are expected to be represented in interfaces by a -# pseudo-constructor function `__constructor__(...)` with arguments the same as the contract's constructor. -# The script reports any differences found and exits with an error if inconsistencies are detected. -# NOTE: Script is fast enough but could be parallelized if necessary. - -# Parse flags -no_diff=false -if [[ "${1:-}" == "--no-diff" ]]; then - no_diff=true -fi - -# Grab the directory of the contracts-bedrock package -SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd ) -CONTRACTS_BASE=$(dirname "$(dirname "$SCRIPT_DIR")") - -# Define contracts that should be excluded from the check -EXCLUDE_CONTRACTS=( - # External dependencies - "IERC20" - "IERC721" - "IERC721Enumerable" - "IERC721Upgradeable" - "IERC721Metadata" - "IERC165" - "IERC165Upgradeable" - "ERC721TokenReceiver" - "ERC1155TokenReceiver" - "ERC777TokensRecipient" - "Guard" - "IProxy" - "Vm" - "VmSafe" - "IMulticall3" - "IERC721TokenReceiver" - "IProxyCreationCallback" - "IBeacon" - - # EAS - "IEAS" - "ISchemaResolver" - "ISchemaRegistry" - - # TODO: Interfaces that need to be fixed are below this line - # ---------------------------------------------------------- - - # Inlined interface, needs to be replaced. - "IInitializable" - - # Missing various functions. - "IPreimageOracle" - "ILegacyMintableERC20" - "IOptimismMintableERC20" - "IOptimismMintableERC721" - - # Doesn't start with "I" - "KontrolCheatsBase" - - # Currently inherit from interface, needs to be fixed. - "IWETH" - "IDelayedWETH" - "ISuperchainWETH" - "IL2ToL2CrossDomainMessenger" - "ICrossL2Inbox" - "ISystemConfigInterop" - - # Enums need to be normalized - "ISequencerFeeVault" - "IBaseFeeVault" - "IL1FeeVault" - "IFeeVault" - - # Solidity complains about receive but contract doens't have it. - "IResolvedDelegateProxy" -) - -# Find all JSON files in the forge-artifacts folder -JSON_FILES=$(find "$CONTRACTS_BASE/forge-artifacts" -type f -name "*.json") - -# Initialize a flag to track if any issues are detected -issues_detected=false - -# Create a temporary file to store files that have already been reported -REPORTED_INTERFACES_FILE=$(mktemp) - -# Clean up the temporary file on exit -cleanup() { - rm -f "$REPORTED_INTERFACES_FILE" -} - -# Trap exit and error signals and call cleanup function -trap cleanup EXIT ERR - -# Check if a contract is excluded -is_excluded() { - for exclude in "${EXCLUDE_CONTRACTS[@]}"; do - if [[ "$exclude" == "$1" ]]; then - return 0 - fi - done - return 1 -} - -# Iterate over all JSON files -for interface_file in $JSON_FILES; do - # Grab the contract name from the file name - contract_name=$(basename "$interface_file" .json | cut -d '.' -f 1) - - # Extract all contract definitions in a single pass - contract_definitions=$(jq -r '.ast.nodes[] | select(.nodeType == "ContractDefinition") | "\(.contractKind),\(.name)"' "$interface_file") - - # Continue if no contract definitions are found - # Can happen in Solidity files that don't declare any contracts/interfaces - if [ -z "$contract_definitions" ]; then - continue - fi - - # Iterate over the found contract definitions and figure out which one - # matches the file name. We do this so that we can figure out if this is an - # interface or a contract based on the contract kind. - found=false - contract_temp="" - contract_kind="" - for definition in $contract_definitions; do - IFS=',' read -r contract_kind contract_temp <<< "$definition" - if [[ "$contract_name" == "$contract_temp" ]]; then - found=true - break - fi - done - - # Continue if no matching contract name is found. Can happen in Solidity - # files where no contracts/interfaces are defined with the same name as the - # file. Still OK because a separate artifact *will* be generated for the - # specific contract/interface. - if [ "$found" = false ]; then - continue - fi - - # If contract kind is not "interface", skip the file - if [ "$contract_kind" != "interface" ]; then - continue - fi - - # If contract name does not start with an "I", throw an error - if [[ "$contract_name" != I* ]]; then - if ! grep -q "^$contract_name$" "$REPORTED_INTERFACES_FILE"; then - echo "$contract_name" >> "$REPORTED_INTERFACES_FILE" - if ! is_excluded "$contract_name"; then - echo "Issue found in ABI for interface $contract_name from file $interface_file." - echo "Interface $contract_name does not start with 'I'." - issues_detected=true - fi - fi - continue - fi - - # Extract contract semver - contract_semver=$(jq -r '.ast.nodes[] | select(.nodeType == "PragmaDirective") | .literals | join("")' "$interface_file") - - # If semver is not exactly "solidity^0.8.0", throw an error - if [ "$contract_semver" != "solidity^0.8.0" ]; then - if ! grep -q "^$contract_name$" "$REPORTED_INTERFACES_FILE"; then - echo "$contract_name" >> "$REPORTED_INTERFACES_FILE" - if ! is_excluded "$contract_name"; then - echo "Issue found in ABI for interface $contract_name from file $interface_file." - echo "Interface $contract_name does not have correct compiler version (MUST be exactly solidity ^0.8.0)." - issues_detected=true - fi - fi - continue - fi - - # Construct the corresponding contract name by removing the leading "I" - contract_basename=${contract_name:1} - corresponding_contract_file="$CONTRACTS_BASE/forge-artifacts/$contract_basename.sol/$contract_basename.json" - - # Skip the file if the corresponding contract file does not exist - if [ ! -f "$corresponding_contract_file" ]; then - continue - fi - - # Extract and compare ABIs excluding constructors - interface_abi=$(jq '[.abi[]]' < "$interface_file") - contract_abi=$(jq '[.abi[]]' < "$corresponding_contract_file") - - # Function to normalize ABI by replacing interface name with contract name. - # Base contracts aren't allowed to inherit from their interfaces in order - # to guarantee a 1:1 match between interfaces and contracts. This means - # that the interface will redefine types in the base contract. We normalize - # the ABI as if the interface and contract are the same name. - normalize_abi() { - # Here we just remove the leading "I" from any contract, enum, or - # struct type. It's not beautiful but it's good enough for now. It - # would miss certain edge cases like if an interface really is using - # the contract type instead of the interface type but that's unlikely - # to happen in practice and should be an easy fix if it does. - local abi="$1" - - # Remove the leading "I" from types. - abi="${abi//\"internalType\": \"contract I/\"internalType\": \"contract }" - abi="${abi//\"internalType\": \"enum I/\"internalType\": \"enum }" - abi="${abi//\"internalType\": \"struct I/\"internalType\": \"struct }" - - # Handle translating pseudo-constructors. - abi=$(echo "$abi" | jq 'map(if .type == "function" and .name == "__constructor__" then .type = "constructor" | del(.name) | del(.outputs) else . end)') - - echo "$abi" - } - - # Normalize the ABIs - normalized_interface_abi=$(normalize_abi "$interface_abi") - normalized_contract_abi=$(normalize_abi "$contract_abi") - - # Check if the contract ABI has no constructor but the interface is missing __constructor__ - contract_has_constructor=$(echo "$normalized_contract_abi" | jq 'any(.[]; .type == "constructor")') - interface_has_default_pseudo_constructor=$(echo "$normalized_interface_abi" | jq 'any(.[]; .type == "constructor" and .inputs == [])') - - # If any contract has no constructor and its corresponding interface also does not have one, flag it as a detected issue - if [ "$contract_has_constructor" = false ] && [ "$interface_has_default_pseudo_constructor" = false ]; then - if ! grep -q "^$contract_name$" "$REPORTED_INTERFACES_FILE"; then - echo "$contract_name" >> "$REPORTED_INTERFACES_FILE" - if ! is_excluded "$contract_name"; then - echo "Issue found in ABI for interface $contract_name from file $interface_file." - echo "Interface $contract_name must have a function named '__constructor__' as the corresponding contract has no constructor in its ABI." - issues_detected=true - fi - fi - continue - fi - - # removes the pseudo constructor json entry from the interface files where the corresponding contract file has no constructor - # this is to ensure it is not flagged as a diff in the next step below - if [ "$contract_has_constructor" = false ] && [ "$interface_has_default_pseudo_constructor" ]; then - normalized_interface_abi=$(echo "$normalized_interface_abi" | jq 'map(select(.type != "constructor"))') - fi - - # Use jq to compare the ABIs - if ! diff_result=$(diff -u <(echo "$normalized_interface_abi" | jq 'sort') <(echo "$normalized_contract_abi" | jq 'sort')); then - if ! grep -q "^$contract_name$" "$REPORTED_INTERFACES_FILE"; then - echo "$contract_name" >> "$REPORTED_INTERFACES_FILE" - if ! is_excluded "$contract_name"; then - echo "Issue found in ABI for interface $contract_name from file $interface_file." - echo "Differences found in ABI between interface $contract_name and actual contract $contract_basename." - if [ "$no_diff" = false ]; then - echo "$diff_result" - fi - issues_detected=true - fi - fi - continue - fi -done - -# Check for unnecessary exclusions -for exclude_item in "${EXCLUDE_CONTRACTS[@]}"; do - if ! grep -q "^$exclude_item$" "$REPORTED_INTERFACES_FILE"; then - echo "Warning: $exclude_item is in the exclude list but WAS NOT reported as an issue. It" - echo "may be unnecessary in the EXCLUDE_CONTRACTS list, but you MUST verify this before" - echo "removing it by performing a clean and full build before re-running this script." - fi -done - -# Fail the script if any issues were detected -if [ "$issues_detected" = true ]; then - echo "Issues were detected while validating interface files." - echo "If the interface is an external dependency or should otherwise be excluded from this" - echo "check, add the interface name to the EXCLUDE_CONTRACTS list in the script. This will prevent" - echo "the script from comparing it against a corresponding contract." - echo "IMPORTANT: Interface files are NOT yet generated automatically. You must fix any" - echo "listed discrepancies manually by updating the specified interface file. Automated" - echo "interface generation is dependent on a few Forge bug fixes." - exit 1 -else - exit 0 -fi From d67991a3fc4f0a755cb507a33bca0b6c111c83fd Mon Sep 17 00:00:00 2001 From: Adrian Sutton Date: Thu, 24 Oct 2024 04:23:18 +1000 Subject: [PATCH 2/5] op-dispute-mon: Use game data from previous update cycle if update fails (#12481) --- op-dispute-mon/metrics/metrics.go | 15 +++- op-dispute-mon/metrics/noop.go | 2 + op-dispute-mon/mon/extract/extractor.go | 22 ++++-- op-dispute-mon/mon/extract/extractor_test.go | 78 ++++++++++++++++---- op-dispute-mon/mon/monitor.go | 35 +++------ op-dispute-mon/mon/monitor_test.go | 75 +++++-------------- op-dispute-mon/mon/service.go | 17 ++--- op-dispute-mon/mon/types/types.go | 1 + op-dispute-mon/mon/update_times.go | 34 +++++++++ op-dispute-mon/mon/update_times_test.go | 43 +++++++++++ 10 files changed, 208 insertions(+), 114 deletions(-) create mode 100644 op-dispute-mon/mon/update_times.go create mode 100644 op-dispute-mon/mon/update_times_test.go diff --git a/op-dispute-mon/metrics/metrics.go b/op-dispute-mon/metrics/metrics.go index b76c23c4ca26..aa8cd9947905 100644 --- a/op-dispute-mon/metrics/metrics.go +++ b/op-dispute-mon/metrics/metrics.go @@ -183,6 +183,8 @@ type Metricer interface { RecordL2Challenges(agreement bool, count int) + RecordOldestGameUpdateTime(t time.Time) + caching.Metrics contractMetrics.ContractMetricer } @@ -215,7 +217,8 @@ type Metrics struct { credits prometheus.GaugeVec honestWithdrawableAmounts prometheus.GaugeVec - lastOutputFetch prometheus.Gauge + lastOutputFetch prometheus.Gauge + oldestGameUpdateTime prometheus.Gauge gamesAgreement prometheus.GaugeVec latestValidProposalL2Block prometheus.Gauge @@ -269,6 +272,12 @@ func NewMetrics() *Metrics { Name: "last_output_fetch", Help: "Timestamp of the last output fetch", }), + oldestGameUpdateTime: factory.NewGauge(prometheus.GaugeOpts{ + Namespace: Namespace, + Name: "oldest_game_update_time", + Help: "Timestamp the least recently updated game " + + "or the time of the last update cycle if there were no games in the monitoring window", + }), honestActorClaims: *factory.NewGaugeVec(prometheus.GaugeOpts{ Namespace: Namespace, Name: "honest_actor_claims", @@ -499,6 +508,10 @@ func (m *Metrics) RecordOutputFetchTime(timestamp float64) { m.lastOutputFetch.Set(timestamp) } +func (m *Metrics) RecordOldestGameUpdateTime(t time.Time) { + m.oldestGameUpdateTime.Set(float64(t.Unix())) +} + func (m *Metrics) RecordGameAgreement(status GameAgreementStatus, count int) { m.gamesAgreement.WithLabelValues(labelValuesFor(status)...).Set(float64(count)) } diff --git a/op-dispute-mon/metrics/noop.go b/op-dispute-mon/metrics/noop.go index 6b4982ff26ba..4459fdd9c1fb 100644 --- a/op-dispute-mon/metrics/noop.go +++ b/op-dispute-mon/metrics/noop.go @@ -36,6 +36,8 @@ func (*NoopMetricsImpl) RecordWithdrawalRequests(_ common.Address, _ bool, _ int func (*NoopMetricsImpl) RecordOutputFetchTime(_ float64) {} +func (*NoopMetricsImpl) RecordOldestGameUpdateTime(_ time.Time) {} + func (*NoopMetricsImpl) RecordGameAgreement(_ GameAgreementStatus, _ int) {} func (*NoopMetricsImpl) RecordLatestValidProposalL2Block(_ uint64) {} diff --git a/op-dispute-mon/mon/extract/extractor.go b/op-dispute-mon/mon/extract/extractor.go index d19cab340b66..c40b31ccb6c5 100644 --- a/op-dispute-mon/mon/extract/extractor.go +++ b/op-dispute-mon/mon/extract/extractor.go @@ -9,9 +9,11 @@ import ( gameTypes "github.com/ethereum-optimism/optimism/op-challenger/game/types" monTypes "github.com/ethereum-optimism/optimism/op-dispute-mon/mon/types" + "github.com/ethereum-optimism/optimism/op-service/clock" "github.com/ethereum-optimism/optimism/op-service/sources/batching/rpcblock" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/log" + "golang.org/x/exp/maps" ) var ( @@ -29,20 +31,23 @@ type Enricher interface { type Extractor struct { logger log.Logger + clock clock.Clock createContract CreateGameCaller fetchGames FactoryGameFetcher maxConcurrency int enrichers []Enricher ignoredGames map[common.Address]bool + latestGameData map[common.Address]*monTypes.EnrichedGameData } -func NewExtractor(logger log.Logger, creator CreateGameCaller, fetchGames FactoryGameFetcher, ignoredGames []common.Address, maxConcurrency uint, enrichers ...Enricher) *Extractor { +func NewExtractor(logger log.Logger, cl clock.Clock, creator CreateGameCaller, fetchGames FactoryGameFetcher, ignoredGames []common.Address, maxConcurrency uint, enrichers ...Enricher) *Extractor { ignored := make(map[common.Address]bool) for _, game := range ignoredGames { ignored[game] = true } return &Extractor{ logger: logger, + clock: cl, createContract: creator, fetchGames: fetchGames, maxConcurrency: int(maxConcurrency), @@ -61,7 +66,6 @@ func (e *Extractor) Extract(ctx context.Context, blockHash common.Hash, minTimes } func (e *Extractor) enrichGames(ctx context.Context, blockHash common.Hash, games []gameTypes.GameMetadata) ([]*monTypes.EnrichedGameData, int, int) { - var enrichedGames []*monTypes.EnrichedGameData var ignored atomic.Int32 var failed atomic.Int32 @@ -101,8 +105,14 @@ func (e *Extractor) enrichGames(ctx context.Context, blockHash common.Hash, game }() } - // Push each game into the channel + // Create a new store for game data. This ensures any games no longer in the monitoring set are dropped. + updatedGameData := make(map[common.Address]*monTypes.EnrichedGameData) + // Push each game into the channel and store the latest cached game data as a default if fetching fails for _, game := range games { + previousData := e.latestGameData[game.Proxy] + if previousData != nil { + updatedGameData[game.Proxy] = previousData + } gameCh <- game } close(gameCh) @@ -112,9 +122,10 @@ func (e *Extractor) enrichGames(ctx context.Context, blockHash common.Hash, game // Read the results for enrichedGame := range enrichedCh { - enrichedGames = append(enrichedGames, enrichedGame) + updatedGameData[enrichedGame.Proxy] = enrichedGame } - return enrichedGames, int(ignored.Load()), int(failed.Load()) + e.latestGameData = updatedGameData + return maps.Values(updatedGameData), int(ignored.Load()), int(failed.Load()) } func (e *Extractor) enrichGame(ctx context.Context, blockHash common.Hash, game gameTypes.GameMetadata) (*monTypes.EnrichedGameData, error) { @@ -138,6 +149,7 @@ func (e *Extractor) enrichGame(ctx context.Context, blockHash common.Hash, game enrichedClaims[i] = monTypes.EnrichedClaim{Claim: claim} } enrichedGame := &monTypes.EnrichedGameData{ + LastUpdateTime: e.clock.Now(), GameMetadata: game, L1Head: meta.L1Head, L2BlockNumber: meta.L2BlockNum, diff --git a/op-dispute-mon/mon/extract/extractor_test.go b/op-dispute-mon/mon/extract/extractor_test.go index 361aeb57420e..ee4b8a63ee25 100644 --- a/op-dispute-mon/mon/extract/extractor_test.go +++ b/op-dispute-mon/mon/extract/extractor_test.go @@ -9,6 +9,7 @@ import ( "github.com/ethereum-optimism/optimism/op-challenger/game/fault/contracts" monTypes "github.com/ethereum-optimism/optimism/op-dispute-mon/mon/types" + "github.com/ethereum-optimism/optimism/op-service/clock" "github.com/ethereum-optimism/optimism/op-service/sources/batching/rpcblock" "github.com/stretchr/testify/require" @@ -26,7 +27,7 @@ var ( func TestExtractor_Extract(t *testing.T) { t.Run("FetchGamesError", func(t *testing.T) { - extractor, _, games, _ := setupExtractorTest(t) + extractor, _, games, _, _ := setupExtractorTest(t) games.err = errors.New("boom") _, _, _, err := extractor.Extract(context.Background(), common.Hash{}, 0) require.ErrorIs(t, err, games.err) @@ -34,7 +35,7 @@ func TestExtractor_Extract(t *testing.T) { }) t.Run("CreateGameErrorLog", func(t *testing.T) { - extractor, creator, games, logs := setupExtractorTest(t) + extractor, creator, games, logs, _ := setupExtractorTest(t) games.games = []gameTypes.GameMetadata{{}} creator.err = errors.New("boom") enriched, ignored, failed, err := extractor.Extract(context.Background(), common.Hash{}, 0) @@ -50,7 +51,7 @@ func TestExtractor_Extract(t *testing.T) { }) t.Run("MetadataFetchErrorLog", func(t *testing.T) { - extractor, creator, games, logs := setupExtractorTest(t) + extractor, creator, games, logs, _ := setupExtractorTest(t) games.games = []gameTypes.GameMetadata{{}} creator.caller.metadataErr = errors.New("boom") enriched, ignored, failed, err := extractor.Extract(context.Background(), common.Hash{}, 0) @@ -66,7 +67,7 @@ func TestExtractor_Extract(t *testing.T) { }) t.Run("ClaimsFetchErrorLog", func(t *testing.T) { - extractor, creator, games, logs := setupExtractorTest(t) + extractor, creator, games, logs, _ := setupExtractorTest(t) games.games = []gameTypes.GameMetadata{{}} creator.caller.claimsErr = errors.New("boom") enriched, ignored, failed, err := extractor.Extract(context.Background(), common.Hash{}, 0) @@ -82,7 +83,7 @@ func TestExtractor_Extract(t *testing.T) { }) t.Run("Success", func(t *testing.T) { - extractor, creator, games, _ := setupExtractorTest(t) + extractor, creator, games, _, _ := setupExtractorTest(t) games.games = []gameTypes.GameMetadata{{}} enriched, ignored, failed, err := extractor.Extract(context.Background(), common.Hash{}, 0) require.NoError(t, err) @@ -97,7 +98,7 @@ func TestExtractor_Extract(t *testing.T) { t.Run("EnricherFails", func(t *testing.T) { enricher := &mockEnricher{err: errors.New("whoops")} - extractor, _, games, logs := setupExtractorTest(t, enricher) + extractor, _, games, logs, _ := setupExtractorTest(t, enricher) games.games = []gameTypes.GameMetadata{{}} enriched, ignored, failed, err := extractor.Extract(context.Background(), common.Hash{}, 0) require.NoError(t, err) @@ -110,7 +111,7 @@ func TestExtractor_Extract(t *testing.T) { t.Run("EnricherSuccess", func(t *testing.T) { enricher := &mockEnricher{} - extractor, _, games, _ := setupExtractorTest(t, enricher) + extractor, _, games, _, _ := setupExtractorTest(t, enricher) games.games = []gameTypes.GameMetadata{{}} enriched, ignored, failed, err := extractor.Extract(context.Background(), common.Hash{}, 0) require.NoError(t, err) @@ -123,8 +124,8 @@ func TestExtractor_Extract(t *testing.T) { t.Run("MultipleEnrichersMultipleGames", func(t *testing.T) { enricher1 := &mockEnricher{} enricher2 := &mockEnricher{} - extractor, _, games, _ := setupExtractorTest(t, enricher1, enricher2) - games.games = []gameTypes.GameMetadata{{}, {}} + extractor, _, games, _, _ := setupExtractorTest(t, enricher1, enricher2) + games.games = []gameTypes.GameMetadata{{Proxy: common.Address{0xaa}}, {Proxy: common.Address{0xbb}}} enriched, ignored, failed, err := extractor.Extract(context.Background(), common.Hash{}, 0) require.NoError(t, err) require.Zero(t, ignored) @@ -136,7 +137,7 @@ func TestExtractor_Extract(t *testing.T) { t.Run("IgnoreGames", func(t *testing.T) { enricher1 := &mockEnricher{} - extractor, _, games, logs := setupExtractorTest(t, enricher1) + extractor, _, games, logs, _ := setupExtractorTest(t, enricher1) // Two games, one of which is ignored games.games = []gameTypes.GameMetadata{{Proxy: ignoredGames[0]}, {Proxy: common.Address{0xaa}}} enriched, ignored, failed, err := extractor.Extract(context.Background(), common.Hash{}, 0) @@ -152,6 +153,47 @@ func TestExtractor_Extract(t *testing.T) { testlog.NewMessageFilter("Ignoring game"), testlog.NewAttributesFilter("game", ignoredGames[0].Hex()))) }) + + t.Run("UseCachedValueOnFailure", func(t *testing.T) { + enricher := &mockEnricher{} + extractor, _, games, _, cl := setupExtractorTest(t, enricher) + gameA := common.Address{0xaa} + gameB := common.Address{0xbb} + games.games = []gameTypes.GameMetadata{{Proxy: gameA}, {Proxy: gameB}} + + // First fetch succeeds and the results should be cached + enriched, ignored, failed, err := extractor.Extract(context.Background(), common.Hash{}, 0) + require.NoError(t, err) + require.Zero(t, ignored) + require.Zero(t, failed) + require.Len(t, enriched, 2) + require.Equal(t, 2, enricher.calls) + firstUpdateTime := cl.Now() + require.Equal(t, firstUpdateTime, enriched[0].LastUpdateTime) + require.Equal(t, firstUpdateTime, enriched[1].LastUpdateTime) + + cl.AdvanceTime(2 * time.Minute) + secondUpdateTime := cl.Now() + enricher.action = func(game *monTypes.EnrichedGameData) error { + if game.Proxy == gameA { + return errors.New("boom") + } + // Updated games will have a different status + game.Status = gameTypes.GameStatusChallengerWon + return nil + } + // Second fetch fails for one of the two games, it's cached value should be used. + enriched, ignored, failed, err = extractor.Extract(context.Background(), common.Hash{}, 0) + require.NoError(t, err) + require.Zero(t, ignored) + require.Equal(t, 1, failed) + require.Len(t, enriched, 2) + require.Equal(t, 4, enricher.calls) + require.Equal(t, enriched[0].Status, gameTypes.GameStatusInProgress) // Uses cached value from game A + require.Equal(t, enriched[1].Status, gameTypes.GameStatusChallengerWon) // Updates game B + require.Equal(t, firstUpdateTime, enriched[0].LastUpdateTime) + require.Equal(t, secondUpdateTime, enriched[1].LastUpdateTime) + }) } func verifyLogs(t *testing.T, logs *testlog.CapturingHandler, createErr, metadataErr, claimsErr, durationErr int) { @@ -170,20 +212,22 @@ func verifyLogs(t *testing.T, logs *testlog.CapturingHandler, createErr, metadat require.Len(t, l, durationErr) } -func setupExtractorTest(t *testing.T, enrichers ...Enricher) (*Extractor, *mockGameCallerCreator, *mockGameFetcher, *testlog.CapturingHandler) { +func setupExtractorTest(t *testing.T, enrichers ...Enricher) (*Extractor, *mockGameCallerCreator, *mockGameFetcher, *testlog.CapturingHandler, *clock.DeterministicClock) { logger, capturedLogs := testlog.CaptureLogger(t, log.LvlDebug) games := &mockGameFetcher{} caller := &mockGameCaller{rootClaim: mockRootClaim} creator := &mockGameCallerCreator{caller: caller} + cl := clock.NewDeterministicClock(time.Unix(48294294, 58)) extractor := NewExtractor( logger, + cl, creator.CreateGameCaller, games.FetchGames, ignoredGames, 5, enrichers..., ) - return extractor, creator, games, capturedLogs + return extractor, creator, games, capturedLogs, cl } type mockGameFetcher struct { @@ -311,11 +355,15 @@ func (m *mockGameCaller) IsResolved(_ context.Context, _ rpcblock.Block, claims } type mockEnricher struct { - err error - calls int + err error + calls int + action func(game *monTypes.EnrichedGameData) error } -func (m *mockEnricher) Enrich(_ context.Context, _ rpcblock.Block, _ GameCaller, _ *monTypes.EnrichedGameData) error { +func (m *mockEnricher) Enrich(_ context.Context, _ rpcblock.Block, _ GameCaller, game *monTypes.EnrichedGameData) error { m.calls++ + if m.action != nil { + return m.action(game) + } return m.err } diff --git a/op-dispute-mon/mon/monitor.go b/op-dispute-mon/mon/monitor.go index 039255608c73..5f7764adccbd 100644 --- a/op-dispute-mon/mon/monitor.go +++ b/op-dispute-mon/mon/monitor.go @@ -14,8 +14,6 @@ import ( ) type ForecastResolution func(games []*types.EnrichedGameData, ignoredCount, failedCount int) -type Bonds func(games []*types.EnrichedGameData) -type Resolutions func(games []*types.EnrichedGameData) type Monitor func(games []*types.EnrichedGameData) type BlockHashFetcher func(ctx context.Context, number *big.Int) (common.Hash, error) type BlockNumberFetcher func(ctx context.Context) (uint64, error) @@ -38,11 +36,7 @@ type gameMonitor struct { monitorInterval time.Duration forecast ForecastResolution - bonds Bonds - resolutions Resolutions - claims Monitor - withdrawals Monitor - l2Challenges Monitor + monitors []Monitor extract Extract fetchBlockHash BlockHashFetcher fetchBlockNumber BlockNumberFetcher @@ -55,16 +49,11 @@ func newGameMonitor( metrics MonitorMetrics, monitorInterval time.Duration, gameWindow time.Duration, - forecast ForecastResolution, - bonds Bonds, - resolutions Resolutions, - claims Monitor, - withdrawals Monitor, - l2Challenges Monitor, - extract Extract, - fetchBlockNumber BlockNumberFetcher, fetchBlockHash BlockHashFetcher, -) *gameMonitor { + fetchBlockNumber BlockNumberFetcher, + extract Extract, + forecast ForecastResolution, + monitors ...Monitor) *gameMonitor { return &gameMonitor{ logger: logger, clock: cl, @@ -74,11 +63,7 @@ func newGameMonitor( monitorInterval: monitorInterval, gameWindow: gameWindow, forecast: forecast, - bonds: bonds, - resolutions: resolutions, - claims: claims, - withdrawals: withdrawals, - l2Challenges: l2Challenges, + monitors: monitors, extract: extract, fetchBlockNumber: fetchBlockNumber, fetchBlockHash: fetchBlockHash, @@ -101,12 +86,10 @@ func (m *gameMonitor) monitorGames() error { if err != nil { return fmt.Errorf("failed to load games: %w", err) } - m.resolutions(enrichedGames) m.forecast(enrichedGames, ignored, failed) - m.bonds(enrichedGames) - m.claims(enrichedGames) - m.withdrawals(enrichedGames) - m.l2Challenges(enrichedGames) + for _, monitor := range m.monitors { + monitor(enrichedGames) + } timeTaken := m.clock.Since(start) m.metrics.RecordMonitorDuration(timeTaken) m.logger.Info("Completed monitoring update", "blockNumber", blockNumber, "blockHash", blockHash, "duration", timeTaken, "games", len(enrichedGames), "ignored", ignored, "failed", failed) diff --git a/op-dispute-mon/mon/monitor_test.go b/op-dispute-mon/mon/monitor_test.go index 180c29bb26ce..1181c57b4ecb 100644 --- a/op-dispute-mon/mon/monitor_test.go +++ b/op-dispute-mon/mon/monitor_test.go @@ -25,7 +25,7 @@ func TestMonitor_MonitorGames(t *testing.T) { t.Parallel() t.Run("FailedFetchBlocknumber", func(t *testing.T) { - monitor, _, _, _, _, _, _, _ := setupMonitorTest(t) + monitor, _, _, _ := setupMonitorTest(t) boom := errors.New("boom") monitor.fetchBlockNumber = func(ctx context.Context) (uint64, error) { return 0, boom @@ -35,7 +35,7 @@ func TestMonitor_MonitorGames(t *testing.T) { }) t.Run("FailedFetchBlockHash", func(t *testing.T) { - monitor, _, _, _, _, _, _, _ := setupMonitorTest(t) + monitor, _, _, _ := setupMonitorTest(t) boom := errors.New("boom") monitor.fetchBlockHash = func(ctx context.Context, number *big.Int) (common.Hash, error) { return common.Hash{}, boom @@ -45,29 +45,25 @@ func TestMonitor_MonitorGames(t *testing.T) { }) t.Run("MonitorsWithNoGames", func(t *testing.T) { - monitor, factory, forecast, bonds, withdrawals, resolutions, claims, l2Challenges := setupMonitorTest(t) + monitor, factory, forecast, monitors := setupMonitorTest(t) factory.games = []*monTypes.EnrichedGameData{} err := monitor.monitorGames() require.NoError(t, err) require.Equal(t, 1, forecast.calls) - require.Equal(t, 1, bonds.calls) - require.Equal(t, 1, resolutions.calls) - require.Equal(t, 1, claims.calls) - require.Equal(t, 1, withdrawals.calls) - require.Equal(t, 1, l2Challenges.calls) + for _, m := range monitors { + require.Equal(t, 1, m.calls) + } }) t.Run("MonitorsMultipleGames", func(t *testing.T) { - monitor, factory, forecast, bonds, withdrawals, resolutions, claims, l2Challenges := setupMonitorTest(t) + monitor, factory, forecast, monitors := setupMonitorTest(t) factory.games = []*monTypes.EnrichedGameData{{}, {}, {}} err := monitor.monitorGames() require.NoError(t, err) require.Equal(t, 1, forecast.calls) - require.Equal(t, 1, bonds.calls) - require.Equal(t, 1, resolutions.calls) - require.Equal(t, 1, claims.calls) - require.Equal(t, 1, withdrawals.calls) - require.Equal(t, 1, l2Challenges.calls) + for _, m := range monitors { + require.Equal(t, 1, m.calls) + } }) } @@ -75,7 +71,7 @@ func TestMonitor_StartMonitoring(t *testing.T) { t.Run("MonitorsGames", func(t *testing.T) { addr1 := common.Address{0xaa} addr2 := common.Address{0xbb} - monitor, factory, forecaster, _, _, _, _, _ := setupMonitorTest(t) + monitor, factory, forecaster, _ := setupMonitorTest(t) factory.games = []*monTypes.EnrichedGameData{newEnrichedGameData(addr1, 9999), newEnrichedGameData(addr2, 9999)} factory.maxSuccess = len(factory.games) // Only allow two successful fetches @@ -88,7 +84,7 @@ func TestMonitor_StartMonitoring(t *testing.T) { }) t.Run("FailsToFetchGames", func(t *testing.T) { - monitor, factory, forecaster, _, _, _, _, _ := setupMonitorTest(t) + monitor, factory, forecaster, _ := setupMonitorTest(t) factory.fetchErr = errors.New("boom") monitor.StartMonitoring() @@ -110,7 +106,7 @@ func newEnrichedGameData(proxy common.Address, timestamp uint64) *monTypes.Enric } } -func setupMonitorTest(t *testing.T) (*gameMonitor, *mockExtractor, *mockForecast, *mockBonds, *mockMonitor, *mockResolutionMonitor, *mockMonitor, *mockMonitor) { +func setupMonitorTest(t *testing.T) (*gameMonitor, *mockExtractor, *mockForecast, []*mockMonitor) { logger := testlog.Logger(t, log.LvlDebug) fetchBlockNum := func(ctx context.Context) (uint64, error) { return 1, nil @@ -123,37 +119,12 @@ func setupMonitorTest(t *testing.T) (*gameMonitor, *mockExtractor, *mockForecast cl.Start() extractor := &mockExtractor{} forecast := &mockForecast{} - bonds := &mockBonds{} - resolutions := &mockResolutionMonitor{} - claims := &mockMonitor{} - withdrawals := &mockMonitor{} - l2Challenges := &mockMonitor{} - monitor := newGameMonitor( - context.Background(), - logger, - cl, - metrics.NoopMetrics, - monitorInterval, - 10*time.Second, - forecast.Forecast, - bonds.CheckBonds, - resolutions.CheckResolutions, - claims.Check, - withdrawals.Check, - l2Challenges.Check, - extractor.Extract, - fetchBlockNum, - fetchBlockHash, - ) - return monitor, extractor, forecast, bonds, withdrawals, resolutions, claims, l2Challenges -} - -type mockResolutionMonitor struct { - calls int -} - -func (m *mockResolutionMonitor) CheckResolutions(games []*monTypes.EnrichedGameData) { - m.calls++ + monitor1 := &mockMonitor{} + monitor2 := &mockMonitor{} + monitor3 := &mockMonitor{} + monitor := newGameMonitor(context.Background(), logger, cl, metrics.NoopMetrics, monitorInterval, 10*time.Second, fetchBlockHash, fetchBlockNum, + extractor.Extract, forecast.Forecast, monitor1.Check, monitor2.Check, monitor3.Check) + return monitor, extractor, forecast, []*mockMonitor{monitor1, monitor2, monitor3} } type mockMonitor struct { @@ -172,14 +143,6 @@ func (m *mockForecast) Forecast(_ []*monTypes.EnrichedGameData, _, _ int) { m.calls++ } -type mockBonds struct { - calls int -} - -func (m *mockBonds) CheckBonds(_ []*monTypes.EnrichedGameData) { - m.calls++ -} - type mockExtractor struct { fetchErr error calls int diff --git a/op-dispute-mon/mon/service.go b/op-dispute-mon/mon/service.go index 8ce97b7b8dd9..083e391b9111 100644 --- a/op-dispute-mon/mon/service.go +++ b/op-dispute-mon/mon/service.go @@ -126,6 +126,7 @@ func (s *Service) initGameCallerCreator() { func (s *Service) initExtractor(cfg *config.Config) { s.extractor = extract.NewExtractor( s.logger, + s.cl, s.game.CreateContract, s.factoryContract.GetGamesAtOrAfter, cfg.IgnoredGames, @@ -217,23 +218,17 @@ func (s *Service) initMonitor(ctx context.Context, cfg *config.Config) { return block.Hash(), nil } l2ChallengesMonitor := NewL2ChallengesMonitor(s.logger, s.metrics) - s.monitor = newGameMonitor( - ctx, - s.logger, - s.cl, - s.metrics, - cfg.MonitorInterval, - cfg.GameWindow, + updateTimeMonitor := NewUpdateTimeMonitor(s.cl, s.metrics) + s.monitor = newGameMonitor(ctx, s.logger, s.cl, s.metrics, cfg.MonitorInterval, cfg.GameWindow, blockHashFetcher, + s.l1Client.BlockNumber, + s.extractor.Extract, s.forecast.Forecast, s.bonds.CheckBonds, s.resolutions.CheckResolutions, s.claims.CheckClaims, s.withdrawals.CheckWithdrawals, l2ChallengesMonitor.CheckL2Challenges, - s.extractor.Extract, - s.l1Client.BlockNumber, - blockHashFetcher, - ) + updateTimeMonitor.CheckUpdateTimes) } func (s *Service) Start(ctx context.Context) error { diff --git a/op-dispute-mon/mon/types/types.go b/op-dispute-mon/mon/types/types.go index dff06dab90bf..774ad3908cf7 100644 --- a/op-dispute-mon/mon/types/types.go +++ b/op-dispute-mon/mon/types/types.go @@ -18,6 +18,7 @@ type EnrichedClaim struct { type EnrichedGameData struct { types.GameMetadata + LastUpdateTime time.Time L1Head common.Hash L1HeadNum uint64 L2BlockNumber uint64 diff --git a/op-dispute-mon/mon/update_times.go b/op-dispute-mon/mon/update_times.go new file mode 100644 index 000000000000..3482775757b0 --- /dev/null +++ b/op-dispute-mon/mon/update_times.go @@ -0,0 +1,34 @@ +package mon + +import ( + "time" + + "github.com/ethereum-optimism/optimism/op-dispute-mon/mon/types" + "github.com/ethereum-optimism/optimism/op-service/clock" +) + +type UpdateTimeMetrics interface { + RecordOldestGameUpdateTime(t time.Time) +} + +type UpdateTimeMonitor struct { + metrics UpdateTimeMetrics + clock clock.Clock +} + +func NewUpdateTimeMonitor(cl clock.Clock, metrics UpdateTimeMetrics) *UpdateTimeMonitor { + return &UpdateTimeMonitor{clock: cl, metrics: metrics} +} + +func (m *UpdateTimeMonitor) CheckUpdateTimes(games []*types.EnrichedGameData) { + // Report the current time if there are no games + // Otherwise the last update time would drop to 0 when there are no games, making it appear there were errors + earliest := m.clock.Now() + + for _, game := range games { + if game.LastUpdateTime.Before(earliest) { + earliest = game.LastUpdateTime + } + } + m.metrics.RecordOldestGameUpdateTime(earliest) +} diff --git a/op-dispute-mon/mon/update_times_test.go b/op-dispute-mon/mon/update_times_test.go new file mode 100644 index 000000000000..a16da62b3c2e --- /dev/null +++ b/op-dispute-mon/mon/update_times_test.go @@ -0,0 +1,43 @@ +package mon + +import ( + "testing" + "time" + + "github.com/ethereum-optimism/optimism/op-dispute-mon/mon/types" + "github.com/ethereum-optimism/optimism/op-service/clock" + "github.com/stretchr/testify/require" +) + +func TestUpdateTimeMonitor_NoGames(t *testing.T) { + m := &mockUpdateTimeMetrics{} + cl := clock.NewDeterministicClock(time.UnixMilli(45892)) + monitor := NewUpdateTimeMonitor(cl, m) + monitor.CheckUpdateTimes(nil) + require.Equal(t, cl.Now(), m.oldestUpdateTime) + + cl.AdvanceTime(time.Minute) + monitor.CheckUpdateTimes([]*types.EnrichedGameData{}) + require.Equal(t, cl.Now(), m.oldestUpdateTime) +} + +func TestUpdateTimeMonitor_ReportsOldestUpdateTime(t *testing.T) { + m := &mockUpdateTimeMetrics{} + cl := clock.NewDeterministicClock(time.UnixMilli(45892)) + monitor := NewUpdateTimeMonitor(cl, m) + monitor.CheckUpdateTimes([]*types.EnrichedGameData{ + {LastUpdateTime: time.UnixMilli(4)}, + {LastUpdateTime: time.UnixMilli(3)}, + {LastUpdateTime: time.UnixMilli(7)}, + {LastUpdateTime: time.UnixMilli(9)}, + }) + require.Equal(t, time.UnixMilli(3), m.oldestUpdateTime) +} + +type mockUpdateTimeMetrics struct { + oldestUpdateTime time.Time +} + +func (m *mockUpdateTimeMetrics) RecordOldestGameUpdateTime(t time.Time) { + m.oldestUpdateTime = t +} From 20e35fd8540d2868aa3d5f0806da18c4e6bf16e7 Mon Sep 17 00:00:00 2001 From: John Chase <68833933+joohhnnn@users.noreply.github.com> Date: Thu, 24 Oct 2024 04:06:27 +0800 Subject: [PATCH 3/5] MTCannon: add WakeupTraversal_WithExitedThreads test (#12551) * add WakeupTraversal_WithExitedThreads * In the TestEVM_WakeupTraversal_WithExitedThreads test, explicitly set the current active thread to exited state (activeThread.Exited = true) to test the wakeup traversal behavior when the active thread has exited && Modified test cases by resetting the active thread's index from exitedThreadIdx to avoid duplicate settings and confusion. * setting Futex fields with varying values. --- .../mipsevm/tests/evm_multithreaded_test.go | 74 +++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/cannon/mipsevm/tests/evm_multithreaded_test.go b/cannon/mipsevm/tests/evm_multithreaded_test.go index 813e307849db..6c297ea946d0 100644 --- a/cannon/mipsevm/tests/evm_multithreaded_test.go +++ b/cannon/mipsevm/tests/evm_multithreaded_test.go @@ -1495,6 +1495,80 @@ func TestEVM_WakeupTraversal_Full(t *testing.T) { } } +func TestEVM_WakeupTraversal_WithExitedThreads(t *testing.T) { + var tracer *tracing.Hooks + addr := Word(0x1234) + wakeupVal := Word(0x999) + cases := []struct { + name string + wakeupAddr Word + futexAddr Word + targetVal Word + traverseRight bool + activeStackSize int + otherStackSize int + exitedThreadIdx []int + shouldClearWakeup bool + shouldPreempt bool + activeThreadFutexAddr Word + activeThreadFutexVal Word + }{ + {name: "Wakeable thread exists among exited threads", wakeupAddr: addr, futexAddr: addr, targetVal: wakeupVal + 1, traverseRight: false, activeStackSize: 3, otherStackSize: 1, exitedThreadIdx: []int{2}, shouldClearWakeup: false, shouldPreempt: true, activeThreadFutexAddr: addr + 8, activeThreadFutexVal: wakeupVal + 2}, + {name: "All threads exited", wakeupAddr: addr, futexAddr: addr, targetVal: wakeupVal, traverseRight: false, activeStackSize: 3, otherStackSize: 0, exitedThreadIdx: []int{1, 2}, shouldClearWakeup: false, shouldPreempt: true, activeThreadFutexAddr: addr + 16, activeThreadFutexVal: wakeupVal + 3}, + {name: "Exited threads, no matching futex", wakeupAddr: addr, futexAddr: addr + 4, targetVal: wakeupVal, traverseRight: false, activeStackSize: 2, otherStackSize: 1, exitedThreadIdx: []int{}, shouldClearWakeup: false, shouldPreempt: true, activeThreadFutexAddr: addr + 24, activeThreadFutexVal: wakeupVal + 4}, + {name: "Matching addr, not wakeable, with exited threads", wakeupAddr: addr, futexAddr: addr, targetVal: wakeupVal, traverseRight: true, activeStackSize: 3, otherStackSize: 0, exitedThreadIdx: []int{1}, shouldClearWakeup: false, shouldPreempt: true, activeThreadFutexAddr: addr + 32, activeThreadFutexVal: wakeupVal + 5}, + {name: "Non-waiting threads with exited threads", wakeupAddr: addr, futexAddr: exec.FutexEmptyAddr, targetVal: 0, traverseRight: false, activeStackSize: 2, otherStackSize: 1, exitedThreadIdx: []int{}, shouldClearWakeup: false, shouldPreempt: true, activeThreadFutexAddr: addr + 40, activeThreadFutexVal: wakeupVal + 6}, + } + + for i, c := range cases { + t.Run(c.name, func(t *testing.T) { + goVm, state, contracts := setup(t, i*1000, nil) + mttestutil.SetupThreads(int64(i*5000), state, c.traverseRight, c.activeStackSize, c.otherStackSize) + step := state.Step + + state.Wakeup = c.wakeupAddr + state.GetMemory().SetWord(c.wakeupAddr&arch.AddressMask, wakeupVal) + + threads := mttestutil.GetAllThreads(state) + for idx, thread := range threads { + if slices.Contains(c.exitedThreadIdx, idx) { + thread.Exited = true + } else { + thread.FutexAddr = c.futexAddr + thread.FutexVal = c.targetVal + thread.FutexTimeoutStep = exec.FutexNoTimeout + } + } + + activeThread := state.GetCurrentThread() + activeThread.Exited = true + + activeThread.FutexAddr = c.activeThreadFutexAddr + activeThread.FutexVal = c.activeThreadFutexVal + activeThread.FutexTimeoutStep = exec.FutexNoTimeout + + expected := mttestutil.NewExpectedMTState(state) + expected.Step += 1 + + if c.shouldClearWakeup { + expected.Wakeup = exec.FutexEmptyAddr + } + if c.shouldPreempt { + // Just preempt the current thread + expected.ExpectPreemption(state) + } + + // State transition + var err error + var stepWitness *mipsevm.StepWitness + stepWitness, err = goVm.Step(true) + require.NoError(t, err) + expected.Validate(t, state) + testutil.ValidateEVM(t, stepWitness, step, goVm, multithreaded.GetStateHashFn(), contracts, tracer) + }) + } +} + func TestEVM_SchedQuantumThreshold(t *testing.T) { var tracer *tracing.Hooks cases := []struct { From 358dd3f7d9d94cdbdd92f56c5544dc5078379d16 Mon Sep 17 00:00:00 2001 From: AgusDuha <81362284+agusduha@users.noreply.github.com> Date: Wed, 23 Oct 2024 17:18:21 -0300 Subject: [PATCH 4/5] refactor: remove superchain erc20 modifier (#12566) * fix: remove superchain erc20 modifier (#111) * fix: remove superchain erc20 modifier --------- Co-authored-by: Disco <131301107+0xDiscotech@users.noreply.github.com> Co-authored-by: 0xng Co-authored-by: 0xParticle Co-authored-by: gotzenx <78360669+gotzenx@users.noreply.github.com> * fix: pre pr --------- Co-authored-by: Disco <131301107+0xDiscotech@users.noreply.github.com> Co-authored-by: 0xng Co-authored-by: 0xParticle Co-authored-by: gotzenx <78360669+gotzenx@users.noreply.github.com> --- packages/contracts-bedrock/semver-lock.json | 2 +- .../src/L2/SuperchainERC20.sol | 18 ++++++++---------- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/packages/contracts-bedrock/semver-lock.json b/packages/contracts-bedrock/semver-lock.json index e73dd3b4527e..433b6ce92a16 100644 --- a/packages/contracts-bedrock/semver-lock.json +++ b/packages/contracts-bedrock/semver-lock.json @@ -125,7 +125,7 @@ }, "src/L2/SuperchainERC20.sol": { "initCodeHash": "0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470", - "sourceCodeHash": "0x6a384ccfb6f2f7316c1b33873a1630b5179e52475951d31771656e06d2b11519" + "sourceCodeHash": "0x45b9afdc9e52c27f192673388868a803f54d8f0bffe9defd81d584642e282b6b" }, "src/L2/SuperchainTokenBridge.sol": { "initCodeHash": "0xef7590c30630a75f105384e339e52758569c25a5aa0a5934c521e004b8f86220", diff --git a/packages/contracts-bedrock/src/L2/SuperchainERC20.sol b/packages/contracts-bedrock/src/L2/SuperchainERC20.sol index d723ed8d992b..a824dd3f9018 100644 --- a/packages/contracts-bedrock/src/L2/SuperchainERC20.sol +++ b/packages/contracts-bedrock/src/L2/SuperchainERC20.sol @@ -12,22 +12,18 @@ import { Unauthorized } from "src/libraries/errors/CommonErrors.sol"; /// bridging to make it fungible across the Superchain. This construction allows the SuperchainTokenBridge to /// burn and mint tokens. abstract contract SuperchainERC20 is ERC20, ICrosschainERC20, ISemver { - /// @notice A modifier that only allows the SuperchainTokenBridge to call - modifier onlySuperchainTokenBridge() { - if (msg.sender != Predeploys.SUPERCHAIN_TOKEN_BRIDGE) revert Unauthorized(); - _; - } - /// @notice Semantic version. - /// @custom:semver 1.0.0-beta.2 + /// @custom:semver 1.0.0-beta.3 function version() external view virtual returns (string memory) { - return "1.0.0-beta.2"; + return "1.0.0-beta.3"; } /// @notice Allows the SuperchainTokenBridge to mint tokens. /// @param _to Address to mint tokens to. /// @param _amount Amount of tokens to mint. - function crosschainMint(address _to, uint256 _amount) external onlySuperchainTokenBridge { + function crosschainMint(address _to, uint256 _amount) external { + if (msg.sender != Predeploys.SUPERCHAIN_TOKEN_BRIDGE) revert Unauthorized(); + _mint(_to, _amount); emit CrosschainMinted(_to, _amount); @@ -36,7 +32,9 @@ abstract contract SuperchainERC20 is ERC20, ICrosschainERC20, ISemver { /// @notice Allows the SuperchainTokenBridge to burn tokens. /// @param _from Address to burn tokens from. /// @param _amount Amount of tokens to burn. - function crosschainBurn(address _from, uint256 _amount) external onlySuperchainTokenBridge { + function crosschainBurn(address _from, uint256 _amount) external { + if (msg.sender != Predeploys.SUPERCHAIN_TOKEN_BRIDGE) revert Unauthorized(); + _burn(_from, _amount); emit CrosschainBurnt(_from, _amount); From ca076e8a1f5e4e23e3cb04b7143fba469b5675dd Mon Sep 17 00:00:00 2001 From: agusduha Date: Wed, 23 Oct 2024 17:48:48 -0300 Subject: [PATCH 5/5] fix: pre pr --- packages/contracts-bedrock/semver-lock.json | 4 ++-- packages/contracts-bedrock/src/L2/SuperchainERC20.sol | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/contracts-bedrock/semver-lock.json b/packages/contracts-bedrock/semver-lock.json index d92063a7d647..45944697945e 100644 --- a/packages/contracts-bedrock/semver-lock.json +++ b/packages/contracts-bedrock/semver-lock.json @@ -125,7 +125,7 @@ }, "src/L2/SuperchainERC20.sol": { "initCodeHash": "0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470", - "sourceCodeHash": "0xceb4c9c406a0f9d66bf6f954f35f95ca5664c834f2a9f92657b6ae4d6981286e" + "sourceCodeHash": "0xba47f404e66e010ce417410476f26c704f2be4ce584cb79210bc5536a82ddb1f" }, "src/L2/SuperchainTokenBridge.sol": { "initCodeHash": "0xef7590c30630a75f105384e339e52758569c25a5aa0a5934c521e004b8f86220", @@ -235,4 +235,4 @@ "initCodeHash": "0x06ae2c0b39c215b7fa450d382916ce6f5c6f9f2d630e572db6b72d688255b3fd", "sourceCodeHash": "0xa014d9c992f439dee8221e065828c3326ca2c4f5db0e83431c64c20f7e51ec14" } -} +} \ No newline at end of file diff --git a/packages/contracts-bedrock/src/L2/SuperchainERC20.sol b/packages/contracts-bedrock/src/L2/SuperchainERC20.sol index f8cc8e831901..f18f91ad2413 100644 --- a/packages/contracts-bedrock/src/L2/SuperchainERC20.sol +++ b/packages/contracts-bedrock/src/L2/SuperchainERC20.sol @@ -13,9 +13,9 @@ import { Unauthorized } from "src/libraries/errors/CommonErrors.sol"; /// burn and mint tokens. abstract contract SuperchainERC20 is ERC20, ICrosschainERC20, ISemver { /// @notice Semantic version. - /// @custom:semver 1.0.0-beta.3 + /// @custom:semver 1.0.0-beta.4 function version() external view virtual returns (string memory) { - return "1.0.0-beta.3"; + return "1.0.0-beta.4"; } /// @notice Allows the SuperchainTokenBridge to mint tokens.