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

validation: decouple GasToken tests #803

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 0 additions & 90 deletions validation/gas-token_test.go

This file was deleted.

104 changes: 104 additions & 0 deletions validation/gas_token.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
package validation

import (
"context"
"errors"
"fmt"
"strings"
"testing"

"github.com/ethereum-optimism/superchain-registry/superchain"
"github.com/ethereum/go-ethereum"
"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/crypto"
"github.com/ethereum/go-ethereum/ethclient"
"github.com/stretchr/testify/require"
)

func testGasToken(t *testing.T, chain *superchain.ChainConfig) {
l1Client, err := ethclient.Dial(superchain.Superchains[chain.Superchain].Config.L1.PublicRPC)
require.NoError(t, err, "Failed to connect to the Ethereum client at RPC url %s", chain.PublicRPC)
defer l1Client.Close()

err = CheckGasToken(chain, l1Client)
require.NoError(t, err)
}

func CheckGasToken(chain *superchain.ChainConfig, l1Client EthClient) error {
bitwiseguy marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

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

Did we add a unit test for this yet? That is what I would expect in gas_token_test.go.

l2Client, err := ethclient.Dial(chain.PublicRPC)
bitwiseguy marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return err
}
defer l2Client.Close()

weth9PredeployAddress := superchain.MustHexToAddress("0x4200000000000000000000000000000000000006")
want := "0000000000000000000000000000000000000000000000000000000000000020" + // offset
"000000000000000000000000000000000000000000000000000000000000000d" + // length
"5772617070656420457468657200000000000000000000000000000000000000" // "Wrapped Ether" padded to 32 bytes
gotName, err := getHexString("name()", weth9PredeployAddress, l2Client)
if err != nil {
return err
}
if want != gotName {
return fmt.Errorf("predeploy WETH9.name(): want=%s, got=%s", want, gotName)
}

l1BlockPredeployAddress := superchain.MustHexToAddress("0x4200000000000000000000000000000000000015")
isCustomGasToken, err := getBool("isCustomGasToken()", l1BlockPredeployAddress, l2Client)
if err != nil && !strings.Contains(err.Error(), "execution reverted") {
// Pre: reverting is acceptable
return err
} else {
// Post: must be set to false
if isCustomGasToken {
return fmt.Errorf("L1Block.isCustomGasToken() must return false")
}
}

isCustomGasToken, err = getBool("isCustomGasToken()", superchain.Addresses[chain.ChainID].SystemConfigProxy, l2Client)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't this use an l1Client?

I'd like us to revisit the "reverting is acceptable" case b/c it seems like it could be hiding many skeletons like this one. Can we double check it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yea this should be an l1Client. Good catch. What are the pre/post comments referring to? Pre-what? Those comments were just copied from existing code

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think what this is trying to do is to cope with deployments of the SystemConfig which don't expose the isCustomGasToken() method. This would indicate they don't support custom gas tokens and therefore should automatically pass the test. A better approach would be to read the semver here, and unless it is greater than or equal to some version (which we would need to look up or ask someone for) we don't do the rest of the check.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe there is a semver for the SystemConfigProxy we could check?

If its < a certain value, we don't make the following calls at all (because the method doesn't exist?):

  • SystemConfigProxy.isCustomGasToken()
  • L1BlockPredeploy.isCustomGasToken()

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Discussed some findings on semver in this discord thread. Not sure how to proceed at the moment. The isCustomGasToken() method seems to be missing on the latest standard version of the SystemConfig contract

if err != nil && !strings.Contains(err.Error(), "execution reverted") {
// Pre: reverting is acceptable
return err
} else {
// Post: must be set to false
if isCustomGasToken {
return fmt.Errorf("SystemConfigProxy.isCustomGasToken() must return false")
}
}
return nil
}

func getBytes(method string, contractAddress superchain.Address, client EthClient) ([]byte, error) {
bitwiseguy marked this conversation as resolved.
Show resolved Hide resolved
addr := (common.Address(contractAddress))
callMsg := ethereum.CallMsg{
To: &addr,
Data: crypto.Keccak256([]byte(method))[:4],
}

callContract := func(msg ethereum.CallMsg) ([]byte, error) {
return client.CallContract(context.Background(), msg, nil)
}

return Retry(callContract)(callMsg)
}

func getHexString(method string, contractAddress superchain.Address, client EthClient) (string, error) {
bitwiseguy marked this conversation as resolved.
Show resolved Hide resolved
result, err := getBytes(method, contractAddress, client)
return common.Bytes2Hex(result), err
}

func getBool(method string, contractAddress superchain.Address, client EthClient) (bool, error) {
result, err := getBytes(method, contractAddress, client)
if err != nil {
return false, err
}

switch common.HexToHash(string(result)) {
case common.Hash{1}:
return true, nil
case common.Hash{}:
return false, nil
default:
return false, errors.New("unexpected non-bool return value")
}
}
97 changes: 97 additions & 0 deletions validation/gas_token_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
package validation

import (
"errors"
"testing"

"github.com/ethereum-optimism/superchain-registry/superchain"
"github.com/ethereum-optimism/superchain-registry/validation/testutils"
"github.com/stretchr/testify/require"
)

func TestGetBytes_Success(t *testing.T) {
t.Parallel()
mockClient := &testutils.MockEthClient{
Responses: map[string][]byte{
string(testutils.MethodID("myMethod()")): {0xde, 0xad, 0xbe, 0xef},
},
bitwiseguy marked this conversation as resolved.
Show resolved Hide resolved
}
contractAddress := superchain.MustHexToAddress("0x1234567890abcdef1234567890abcdef12345678")
result, err := getBytes("myMethod()", contractAddress, mockClient)
require.NoError(t, err)
require.Equal(t, []byte{0xde, 0xad, 0xbe, 0xef}, result)
bitwiseguy marked this conversation as resolved.
Show resolved Hide resolved
}

func TestGetBytes_Error(t *testing.T) {
t.Parallel()
mockClient := &testutils.MockEthClient{
Err: errors.New("some call error"),
}
contractAddress := superchain.MustHexToAddress("0x1234567890abcdef1234567890abcdef12345678")
_, err := getBytes("failingMethod()", contractAddress, mockClient)
require.Error(t, err)
require.Contains(t, err.Error(), "some call error")
}

func TestGetHexString_Success(t *testing.T) {
t.Parallel()
mockClient := &testutils.MockEthClient{
Responses: map[string][]byte{
string(testutils.MethodID("hexMethod()")): {0x00, 0x11, 0x22, 0x33},
},
}
contractAddress := superchain.MustHexToAddress("0xabcdef7890abcdef1234567890abcdef12345678")
hexVal, err := getHexString("hexMethod()", contractAddress, mockClient)
require.NoError(t, err)
require.Equal(t, "00112233", hexVal)
}

func TestGetHexString_Error(t *testing.T) {
t.Parallel()
mockClient := &testutils.MockEthClient{
Err: errors.New("getHexString error"),
}
contractAddress := superchain.MustHexToAddress("0x1234567890abcdef1234567890abcdef12345678")
bitwiseguy marked this conversation as resolved.
Show resolved Hide resolved
_, err := getHexString("hexMethod()", contractAddress, mockClient)
require.Error(t, err)
require.Contains(t, err.Error(), "getHexString error")
}

func TestGetBool_True(t *testing.T) {
t.Parallel()
mockClient := &testutils.MockEthClient{
Responses: map[string][]byte{
string(testutils.MethodID("boolMethod()")): []byte("0x0100000000000000000000000000000000000000000000000000000000000000"),
},
}
contractAddress := superchain.MustHexToAddress("0x1111117890abcdef1234567890abcdef12345678")
val, err := getBool("boolMethod()", contractAddress, mockClient)
require.NoError(t, err)
require.True(t, val)
}

func TestGetBool_False(t *testing.T) {
t.Parallel()
mockClient := &testutils.MockEthClient{
Responses: map[string][]byte{
string(testutils.MethodID("boolMethod()")): []byte(""),
},
}
contractAddress := superchain.MustHexToAddress("0x2222227890abcdef1234567890abcdef12345678")
val, err := getBool("boolMethod()", contractAddress, mockClient)
require.NoError(t, err)
require.False(t, val)
}

func TestGetBool_ErrorUnexpectedValue(t *testing.T) {
t.Parallel()
mockClient := &testutils.MockEthClient{
Responses: map[string][]byte{
string(testutils.MethodID("boolMethod()")): []byte("0xabcdef"),
},
}
contractAddress := superchain.MustHexToAddress("0x2222227890abcdef1234567890abcdef12345678")
_, err := getBool("boolMethod()", contractAddress, mockClient)
require.Error(t, err)
require.Contains(t, err.Error(), "unexpected non-bool return value")
}
29 changes: 29 additions & 0 deletions validation/testutils/mock_ethclient.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package testutils

import (
"context"
"math/big"

"github.com/ethereum/go-ethereum"
"github.com/ethereum/go-ethereum/crypto"
)

// MockEthClient is a mock that implements EthCaller for testing.
type MockEthClient struct {
// Setup response maps keyed by method 4-byte signature + address, or
// just by method signature if that's simpler for your tests.
Responses map[string][]byte
Err error
}
bitwiseguy marked this conversation as resolved.
Show resolved Hide resolved

func (m *MockEthClient) CallContract(ctx context.Context, msg ethereum.CallMsg, blockNumber *big.Int) ([]byte, error) {
if m.Err != nil {
return nil, m.Err
}
return m.Responses[string(msg.Data)], nil
}

// Helper to get the 4-byte method ID (like what getBytes does internally)
func MethodID(method string) []byte {
return crypto.Keccak256([]byte(method))[:4]
}
5 changes: 5 additions & 0 deletions validation/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,14 @@ import (

"github.com/ethereum-optimism/optimism/op-service/retry"
"github.com/ethereum-optimism/superchain-registry/superchain"
"github.com/ethereum/go-ethereum"
"github.com/stretchr/testify/assert"
)

type EthClient interface {
CallContract(ctx context.Context, msg ethereum.CallMsg, blockNumber *big.Int) ([]byte, error)
}

// isBigIntWithinBounds returns true if actual is within bounds, where the bounds are [lower bound, upper bound] and are inclusive.
var isBigIntWithinBounds = func(actual *big.Int, bounds [2]*big.Int) bool {
if (bounds[1].Cmp(bounds[0])) < 0 {
Expand Down