Skip to content

Commit

Permalink
chore: adjust code review
Browse files Browse the repository at this point in the history
  • Loading branch information
kangsorang committed Nov 28, 2023
1 parent b4c423b commit f87b6f6
Show file tree
Hide file tree
Showing 15 changed files with 152 additions and 89 deletions.
3 changes: 1 addition & 2 deletions kroma-validator/l2_output_submitter.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import (
"github.com/ethereum/go-ethereum/log"

"github.com/ethereum-optimism/optimism/op-bindings/bindings"
"github.com/ethereum-optimism/optimism/op-node/rollup"
"github.com/ethereum-optimism/optimism/op-service/eth"
"github.com/ethereum-optimism/optimism/op-service/optsutils"
"github.com/ethereum-optimism/optimism/op-service/txmgr"
Expand Down Expand Up @@ -399,7 +398,7 @@ func (l *L2OutputSubmitter) FetchOutput(ctx context.Context, blockNumber *big.In
l.log.Error("failed to fetch output at ", "block number", blockNumber.Uint64(), " err", err)
return nil, err
}
if output.Version != rollup.L2OutputRootVersion(l.cfg.RollupConfig, l.cfg.RollupConfig.ComputeTimestamp(blockNumber.Uint64())) {
if output.Version != eth.OutputVersionV0 {
l.log.Error("l2 output version is not matched: %s", output.Version)
return nil, errors.New("mismatched l2 output version")
}
Expand Down
9 changes: 6 additions & 3 deletions op-e2e/actions/l2_verifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,7 @@ type L2API interface {
InfoAndTxsByHash(ctx context.Context, hash common.Hash) (eth.BlockInfo, types.Transactions, error)
// GetProof returns a proof of the account, it may return a nil result without error if the address was not found.
GetProof(ctx context.Context, address common.Address, storage []common.Hash, blockTag string) (*eth.AccountResult, error)
// [Kroma: START]
//OutputV0AtBlock(ctx context.Context, blockHash common.Hash) (*eth.OutputV0, error)
// [Kroma: END]
OutputV0AtBlock(ctx context.Context, blockHash common.Hash) (*eth.OutputV0, error)
}

func NewL2Verifier(t Testing, log log.Logger, l1 derive.L1Fetcher, eng L2API, cfg *rollup.Config, syncCfg *sync.Config) *L2Verifier {
Expand Down Expand Up @@ -105,6 +103,11 @@ type l2VerifierBackend struct {
verifier *L2Verifier
}

func (s *l2VerifierBackend) BlockRefWithStatus(ctx context.Context, num uint64) (eth.L2BlockRef, *eth.SyncStatus, error) {
ref, err := s.verifier.eng.L2BlockRefByNumber(ctx, num)
return ref, s.verifier.SyncStatus(), err
}

func (s *l2VerifierBackend) BlockRefsWithStatus(ctx context.Context, num uint64) (eth.L2BlockRef, eth.L2BlockRef, *eth.SyncStatus, error) {
ref, err := s.verifier.eng.L2BlockRefByNumber(ctx, num)
if err != nil {
Expand Down
18 changes: 10 additions & 8 deletions op-e2e/actions/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,6 @@ import (
"math/big"
"math/rand"

"github.com/ethereum-optimism/optimism/op-bindings/bindings"
"github.com/ethereum-optimism/optimism/op-bindings/predeploys"
"github.com/ethereum-optimism/optimism/op-e2e/e2eutils"
"github.com/ethereum-optimism/optimism/op-node/rollup"
"github.com/ethereum-optimism/optimism/op-node/rollup/derive"
"github.com/ethereum-optimism/optimism/op-node/withdrawals"
"github.com/ethereum/go-ethereum"
"github.com/ethereum/go-ethereum/accounts/abi/bind"
"github.com/ethereum/go-ethereum/common"
Expand All @@ -22,6 +16,14 @@ import (
"github.com/ethereum/go-ethereum/log"
"github.com/ethereum/go-ethereum/params"
"github.com/stretchr/testify/require"

"github.com/ethereum-optimism/optimism/op-bindings/bindings"
"github.com/ethereum-optimism/optimism/op-bindings/predeploys"
"github.com/ethereum-optimism/optimism/op-e2e/e2eutils"
"github.com/ethereum-optimism/optimism/op-node/rollup"
"github.com/ethereum-optimism/optimism/op-node/rollup/derive"
"github.com/ethereum-optimism/optimism/op-node/withdrawals"
"github.com/kroma-network/kroma/op-service/eth"
)

type L1Bindings struct {
Expand Down Expand Up @@ -426,7 +428,7 @@ func (s *CrossLayerUser) ProveWithdrawal(t Testing, l2TxHash common.Hash) common
require.NoError(t, err)
nextHeader, err := s.L2.env.EthCl.HeaderByNumber(t.Ctx(), new(big.Int).Add(l2OutputBlockNr, common.Big1))
require.NoError(t, err)
version := rollup.L2OutputRootVersion(s.rollupConfig, header.Time)
version := eth.OutputVersionV0
params, err := withdrawals.ProveWithdrawalParameters(t.Ctx(), version, s.L2.env.Bindings.ProofClient, s.L2.env.EthCl, s.lastL2WithdrawalTxHash, header, nextHeader, &s.L1.env.Bindings.L2OutputOracle.L2OutputOracleCaller)
require.NoError(t, err)

Expand Down Expand Up @@ -500,7 +502,7 @@ func (s *CrossLayerUser) CompleteWithdrawal(t Testing, l2TxHash common.Hash) com
require.NoError(t, err)
nextHeader, err := s.L2.env.EthCl.HeaderByNumber(t.Ctx(), new(big.Int).Add(l2OutputBlockNr, common.Big1))
require.NoError(t, err)
version := rollup.L2OutputRootVersion(s.rollupConfig, header.Time)
version := eth.OutputVersionV0
params, err := withdrawals.ProveWithdrawalParameters(t.Ctx(), version, s.L2.env.Bindings.ProofClient, s.L2.env.EthCl, s.lastL2WithdrawalTxHash, header, nextHeader, &s.L1.env.Bindings.L2OutputOracle.L2OutputOracleCaller)
require.NoError(t, err)

Expand Down
3 changes: 1 addition & 2 deletions op-e2e/system_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ import (
"github.com/ethereum-optimism/optimism/op-node/metrics"
rollupNode "github.com/ethereum-optimism/optimism/op-node/node"
"github.com/ethereum-optimism/optimism/op-node/p2p"
"github.com/ethereum-optimism/optimism/op-node/rollup"
"github.com/ethereum-optimism/optimism/op-node/rollup/derive"
"github.com/ethereum-optimism/optimism/op-node/rollup/driver"
"github.com/ethereum-optimism/optimism/op-service/client"
Expand Down Expand Up @@ -1083,7 +1082,7 @@ func TestWithdrawals(t *testing.T) {
startBalance, err = l1Client.BalanceAt(ctx, fromAddr, nil)
require.Nil(t, err)

version := rollup.L2OutputRootVersion(sys.RollupConfig, header.Time)
version := eth.OutputVersionV0
proveReceipt, finalizeReceipt := ProveAndFinalizeWithdrawal(t, version, cfg, l1Client, sys.EthInstances["verifier"], ethPrivKey, receipt)

// Verify balance after withdrawal
Expand Down
4 changes: 2 additions & 2 deletions op-e2e/system_tob_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ import (
"github.com/ethereum-optimism/optimism/op-e2e/e2eutils"
"github.com/ethereum-optimism/optimism/op-e2e/e2eutils/geth"
"github.com/ethereum-optimism/optimism/op-e2e/e2eutils/wait"
"github.com/ethereum-optimism/optimism/op-node/rollup"
"github.com/ethereum-optimism/optimism/op-node/withdrawals"
"github.com/ethereum-optimism/optimism/op-service/testutils/fuzzerutils"
"github.com/kroma-network/kroma/op-service/eth"
)

// TestGasPriceOracleFeeUpdates checks that the gas price oracle cannot be locked by mis-configuring parameters.
Expand Down Expand Up @@ -545,7 +545,7 @@ func TestMixedWithdrawalValidity(t *testing.T) {
receiptCl := ethclient.NewClient(rpcClient)

// Now create the withdrawal
version := rollup.L2OutputRootVersion(sys.RollupConfig, header.Time)
version := eth.OutputVersionV0
params, err := withdrawals.ProveWithdrawalParameters(context.Background(), version, proofCl, receiptCl, tx.Hash(), header, nextHeader, l2OutputOracle)
require.Nil(t, err)

Expand Down
6 changes: 3 additions & 3 deletions op-e2e/testdata/challenge_test_data.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import (
"github.com/ethereum/go-ethereum/core/types"

"github.com/ethereum-optimism/optimism/op-bindings/bindings"
"github.com/ethereum-optimism/optimism/op-service/eth"
"github.com/ethereum-optimism/optimism/op-node/rollup"
"github.com/ethereum-optimism/optimism/op-service/eth"
)

var (
Expand All @@ -37,7 +37,7 @@ var (

func SetPrevOutputResponse(s **eth.OutputResponse) error {
outputRoot, err := rollup.ComputeL2OutputRoot(&bindings.TypesOutputRootProof{
Version: rollup.V0,
Version: eth.OutputVersionV0,
StateRoot: common.HexToHash("0x2ecc9f95421c4f8c6acfd73a9983b021e79b381c9e80991b9b45da927c926c4f"),
MessagePasserStorageRoot: common.HexToHash("0x24f53397bd92b66fda812b6e1191a00b60fc8e304033518006cbeedcab7f2127"),
BlockHash: common.HexToHash("0x2d8d7264743ac0648b2b0fae0137cb0f77b2a952f5583a2cc6abf0c72f4f1b80"),
Expand Down Expand Up @@ -94,7 +94,7 @@ func SetPrevOutputResponse(s **eth.OutputResponse) error {

func SetTargetOutputResponse(s **eth.OutputResponse) error {
outputRoot, err := rollup.ComputeL2OutputRoot(&bindings.TypesOutputRootProof{
Version: rollup.V0,
Version: eth.OutputVersionV0,
StateRoot: common.HexToHash("0x1370c09d12e3aefefbe29bcecaa9a1adac759ee1b6657065f8e103f56b364037"),
MessagePasserStorageRoot: common.HexToHash("0x24f53397bd92b66fda812b6e1191a00b60fc8e304033518006cbeedcab7f2127"),
BlockHash: common.HexToHash("0x5cd3ba48964223516867ee8036fb0121c095d93a4301084f3fa37d811655d1e8"),
Expand Down
56 changes: 29 additions & 27 deletions op-node/node/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"

"github.com/ethereum/go-ethereum"
"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/common/hexutil"
"github.com/ethereum/go-ethereum/core/types"
Expand All @@ -16,30 +17,31 @@ import (
"github.com/ethereum-optimism/optimism/op-service/eth"
"github.com/ethereum-optimism/optimism/op-service/metrics"
"github.com/ethereum-optimism/optimism/op-service/rpc"
"github.com/ethereum/go-ethereum"
)

type l2EthClient interface {
InfoByHash(ctx context.Context, hash common.Hash) (eth.BlockInfo, error)
// GetProof returns a proof of the account, it may return a nil result without error if the address was not found.
// Optionally keys of the account storage trie can be specified to include with corresponding values in the proof.
GetProof(ctx context.Context, address common.Address, storage []common.Hash, blockTag string) (*eth.AccountResult, error)
OutputV0AtBlock(ctx context.Context, blockHash common.Hash) (*eth.OutputV0, error)

// [Kroma: START]
InfoAndTxsByHash(ctx context.Context, hash common.Hash) (eth.BlockInfo, types.Transactions, error)
//OutputV0AtBlock(ctx context.Context, blockHash common.Hash) (*eth.OutputV0, error)
// [Kroma: END]
}

type driverClient interface {
SyncStatus(ctx context.Context) (*eth.SyncStatus, error)
// [Kroma: START]
// BlockRefWithStatus(ctx context.Context, num uint64) (eth.L2BlockRef, *eth.SyncStatus, error)
BlockRefsWithStatus(ctx context.Context, num uint64) (eth.L2BlockRef, eth.L2BlockRef, *eth.SyncStatus, error)
// [Kroma: END]
BlockRefWithStatus(ctx context.Context, num uint64) (eth.L2BlockRef, *eth.SyncStatus, error)
ResetDerivationPipeline(context.Context) error
StartSequencer(ctx context.Context, blockHash common.Hash) error
StopSequencer(context.Context) (common.Hash, error)
SequencerActive(context.Context) (bool, error)

// [Kroma: START]
BlockRefsWithStatus(ctx context.Context, num uint64) (eth.L2BlockRef, eth.L2BlockRef, *eth.SyncStatus, error)
// [Kroma: END]
}

type adminAPI struct {
Expand Down Expand Up @@ -97,7 +99,7 @@ func NewNodeAPI(config *rollup.Config, l2Client l2EthClient, dr driverClient, lo
}

func (n *nodeAPI) OutputAtBlock(ctx context.Context, number hexutil.Uint64) (*eth.OutputResponse, error) {
recordDur := n.m.RecordRPCServerRequest("kroma_outputAtBlock")
recordDur := n.m.RecordRPCServerRequest("optimism_outputAtBlock")
defer recordDur()

output, err := n.fetchOutputAtBlock(ctx, number)
Expand All @@ -108,8 +110,26 @@ func (n *nodeAPI) OutputAtBlock(ctx context.Context, number hexutil.Uint64) (*et
return output, nil
}

func (n *nodeAPI) SyncStatus(ctx context.Context) (*eth.SyncStatus, error) {
recordDur := n.m.RecordRPCServerRequest("optimism_syncStatus")
defer recordDur()
return n.dr.SyncStatus(ctx)
}

func (n *nodeAPI) RollupConfig(_ context.Context) (*rollup.Config, error) {
recordDur := n.m.RecordRPCServerRequest("optimism_rollupConfig")
defer recordDur()
return n.config, nil
}

func (n *nodeAPI) Version(ctx context.Context) (string, error) {
recordDur := n.m.RecordRPCServerRequest("optimism_version")
defer recordDur()
return version.Version + "-" + version.Meta, nil
}

func (n *nodeAPI) OutputWithProofAtBlock(ctx context.Context, number hexutil.Uint64) (*eth.OutputResponse, error) {
recordDur := n.m.RecordRPCServerRequest("kroma_outputWithProofAtBlock")
recordDur := n.m.RecordRPCServerRequest("optimism_outputWithProofAtBlock")
defer recordDur()

output, err := n.fetchOutputAtBlock(ctx, number)
Expand Down Expand Up @@ -170,7 +190,7 @@ func (n *nodeAPI) fetchOutputAtBlock(ctx context.Context, number hexutil.Uint64)
return nil, fmt.Errorf("invalid withdrawal root hash, state root was %s: %w", head.Root(), err)
}

l2OutputRootVersion := rollup.V0 // current version is 0
l2OutputRootVersion := eth.OutputVersionV0 // current version is 0
l2OutputRoot, err := rollup.ComputeL2OutputRoot(&bindings.TypesOutputRootProof{
Version: l2OutputRootVersion,
StateRoot: head.Root(),
Expand All @@ -193,21 +213,3 @@ func (n *nodeAPI) fetchOutputAtBlock(ctx context.Context, number hexutil.Uint64)
Status: status,
}, nil
}

func (n *nodeAPI) SyncStatus(ctx context.Context) (*eth.SyncStatus, error) {
recordDur := n.m.RecordRPCServerRequest("kroma_syncStatus")
defer recordDur()
return n.dr.SyncStatus(ctx)
}

func (n *nodeAPI) RollupConfig(_ context.Context) (*rollup.Config, error) {
recordDur := n.m.RecordRPCServerRequest("kroma_rollupConfig")
defer recordDur()
return n.config, nil
}

func (n *nodeAPI) Version(ctx context.Context) (string, error) {
recordDur := n.m.RecordRPCServerRequest("kroma_version")
defer recordDur()
return version.Version + "-" + version.Meta, nil
}
4 changes: 4 additions & 0 deletions op-node/node/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,10 @@ type mockDriverClient struct {
func (c *mockDriverClient) ExpectBlockRefsWithStatus(num uint64, ref, nextRef eth.L2BlockRef, status *eth.SyncStatus, err error) {
c.Mock.On("BlockRefsWithStatus", num).Return(ref, nextRef, status, &err)
}
func (c *mockDriverClient) BlockRefWithStatus(ctx context.Context, num uint64) (eth.L2BlockRef, *eth.SyncStatus, error) {
m := c.Mock.MethodCalled("BlockRefWithStatus", num)
return m[0].(eth.L2BlockRef), m[1].(*eth.SyncStatus), *m[2].(*error)
}

func (c *mockDriverClient) BlockRefsWithStatus(ctx context.Context, num uint64) (eth.L2BlockRef, eth.L2BlockRef, *eth.SyncStatus, error) {
m := c.Mock.MethodCalled("BlockRefsWithStatus", num)
Expand Down
8 changes: 5 additions & 3 deletions op-node/p2p/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -343,9 +343,11 @@ func (s *SyncClient) mainLoop() {
s.onRangeRequest(ctx, req)
}()
case res := <-s.results:
ctx, cancel := context.WithTimeout(s.resCtx, maxResultProcessing)
s.onResult(ctx, res)
cancel()
func() {
ctx, cancel := context.WithTimeout(s.resCtx, maxResultProcessing)
defer cancel()
s.onResult(ctx, res)
}()
case check := <-s.inFlightChecks:
s.log.Info("Checking in flight", "num", check.num)
complete, ok := s.inFlight[check.num]
Expand Down
1 change: 1 addition & 0 deletions op-node/rollup/derive/system_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ var (
SystemConfigUpdateGasConfig = common.Hash{31: 1}
SystemConfigUpdateGasLimit = common.Hash{31: 2}
SystemConfigUpdateUnsafeBlockSigner = common.Hash{31: 3}

// [Kroma: START]
SystemConfigUpdateValidatorRewardScalar = common.Hash{31: 4}
// [Kroma: END]
Expand Down
44 changes: 30 additions & 14 deletions op-node/rollup/driver/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -500,31 +500,22 @@ func (s *Driver) SyncStatus(ctx context.Context) (*eth.SyncStatus, error) {
}
}

// [Kroma: START]
// BlockRefsWithStatus blocks the driver event loop and captures the syncing status,
// along with L2 blocks reference by number and number plus 1 consistent with that same status.
// BlockRefWithStatus blocks the driver event loop and captures the syncing status,
// along with an L2 block reference by number consistent with that same status.
// If the event loop is too busy and the context expires, a context error is returned.
func (s *Driver) BlockRefsWithStatus(ctx context.Context, num uint64) (eth.L2BlockRef, eth.L2BlockRef, *eth.SyncStatus, error) {
func (s *Driver) BlockRefWithStatus(ctx context.Context, num uint64) (eth.L2BlockRef, *eth.SyncStatus, error) {
wait := make(chan struct{})
select {
case s.stateReq <- wait:
nextRef := eth.L2BlockRef{}

resp := s.syncStatus()
ref, err := s.l2.L2BlockRefByNumber(ctx, num)
if err == nil {
nextRef, err = s.l2.L2BlockRefByNumber(ctx, num+1)
}

<-wait
return ref, nextRef, resp, err
return ref, resp, err
case <-ctx.Done():
return eth.L2BlockRef{}, eth.L2BlockRef{}, nil, ctx.Err()
return eth.L2BlockRef{}, nil, ctx.Err()
}
}

// [Kroma: END]

// deferJSONString helps avoid a JSON-encoding performance hit if the snapshot logger does not run
type deferJSONString struct {
x any
Expand Down Expand Up @@ -571,3 +562,28 @@ func (s *Driver) checkForGapInUnsafeQueue(ctx context.Context) error {
}
return nil
}

// [Kroma: START]
// BlockRefsWithStatus blocks the driver event loop and captures the syncing status,
// along with L2 blocks reference by number and number plus 1 consistent with that same status.
// If the event loop is too busy and the context expires, a context error is returned.
func (s *Driver) BlockRefsWithStatus(ctx context.Context, num uint64) (eth.L2BlockRef, eth.L2BlockRef, *eth.SyncStatus, error) {
wait := make(chan struct{})
select {
case s.stateReq <- wait:
nextRef := eth.L2BlockRef{}

resp := s.syncStatus()
ref, err := s.l2.L2BlockRefByNumber(ctx, num)
if err == nil {
nextRef, err = s.l2.L2BlockRefByNumber(ctx, num+1)
}

<-wait
return ref, nextRef, resp, err
case <-ctx.Done():
return eth.L2BlockRef{}, eth.L2BlockRef{}, nil, ctx.Err()
}
}

// [Kroma: END]
Loading

0 comments on commit f87b6f6

Please sign in to comment.