From 61f0901e97b6873049a7e918ade3be189fa13e5b Mon Sep 17 00:00:00 2001 From: Goran Rojovic Date: Mon, 20 Nov 2023 14:07:03 +0100 Subject: [PATCH 01/16] UTs --- rpc/rpc.go | 20 ++- rpc/rpc_test.go | 433 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 445 insertions(+), 8 deletions(-) create mode 100644 rpc/rpc_test.go diff --git a/rpc/rpc.go b/rpc/rpc.go index 4d5cd74..5d7ff51 100644 --- a/rpc/rpc.go +++ b/rpc/rpc.go @@ -103,7 +103,7 @@ func (i *InteropEndpoints) SendTx(signedTx tx.SignedTx) (interface{}, types.Erro if batch.StateRoot != signedTx.Tx.ZKP.NewStateRoot || batch.LocalExitRoot != signedTx.Tx.ZKP.NewLocalExitRoot { return "0x0", types.NewRPCError(types.DefaultErrorCode, fmt.Sprintf( - "Missmatch detected, expected local exit root: %s actual: %s. expected state root: %s actual: %s", + "Mismatch detected, expected local exit root: %s actual: %s. expected state root: %s actual: %s", signedTx.Tx.ZKP.NewLocalExitRoot.Hex(), batch.LocalExitRoot.Hex(), signedTx.Tx.ZKP.NewStateRoot.Hex(), @@ -135,22 +135,26 @@ func (i *InteropEndpoints) GetTxStatus(hash common.Hash) (result interface{}, er dbTx, innerErr := i.db.BeginStateTransaction(ctx) if innerErr != nil { result = "0x0" - err = types.NewRPCError(types.DefaultErrorCode, fmt.Sprintf("failed to begin dbTx, error: %s", err)) - } + err = types.NewRPCError(types.DefaultErrorCode, fmt.Sprintf("failed to begin dbTx, error: %s", innerErr)) - res, innerErr := i.ethTxManager.Result(ctx, ethTxManOwner, hash.Hex(), dbTx) - if innerErr != nil { - result = "0x0" - err = types.NewRPCError(types.DefaultErrorCode, fmt.Sprintf("failed to get tx, error: %s", err)) + return } defer func() { if innerErr := dbTx.Rollback(ctx); innerErr != nil { result = "0x0" - err = types.NewRPCError(types.DefaultErrorCode, fmt.Sprintf("failed to rollback dbTx, error: %s", err)) + err = types.NewRPCError(types.DefaultErrorCode, fmt.Sprintf("failed to rollback dbTx, error: %s", innerErr)) } }() + res, innerErr := i.ethTxManager.Result(ctx, ethTxManOwner, hash.Hex(), dbTx) + if innerErr != nil { + result = "0x0" + err = types.NewRPCError(types.DefaultErrorCode, fmt.Sprintf("failed to get tx, error: %s", innerErr)) + + return + } + result = res.Status.String() return result, err diff --git a/rpc/rpc_test.go b/rpc/rpc_test.go new file mode 100644 index 0000000..b5b5768 --- /dev/null +++ b/rpc/rpc_test.go @@ -0,0 +1,433 @@ +package rpc + +import ( + "context" + "errors" + "math/big" + "testing" + + "github.com/0xPolygon/beethoven/tx" + "github.com/0xPolygon/cdk-validium-node/ethtxmanager" + validiumTypes "github.com/0xPolygon/cdk-validium-node/jsonrpc/types" + "github.com/ethereum/go-ethereum" + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/core/types" + "github.com/ethereum/go-ethereum/crypto" + "github.com/jackc/pgconn" + "github.com/jackc/pgx/v4" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +var _ EthermanInterface = (*ethermanMock)(nil) + +type ethermanMock struct { + mock.Mock +} + +func (e *ethermanMock) GetSequencerAddr(l1Contract common.Address) (common.Address, error) { + args := e.Called(l1Contract) + + return args.Get(0).(common.Address), args.Error(1) //nolint:forcetypeassert +} + +func (e *ethermanMock) BuildTrustedVerifyBatchesTxData(lastVerifiedBatch, + newVerifiedBatch uint64, proof tx.ZKP) (data []byte, err error) { + args := e.Called(lastVerifiedBatch, newVerifiedBatch, proof) + + return args.Get(0).([]byte), args.Error(1) //nolint:forcetypeassert +} + +func (e *ethermanMock) CallContract(ctx context.Context, call ethereum.CallMsg, + blockNumber *big.Int) ([]byte, error) { + args := e.Called(ctx, call, blockNumber) + + return args.Get(0).([]byte), args.Error(1) //nolint:forcetypeassert +} + +var _ DBInterface = (*dbMock)(nil) + +type dbMock struct { + mock.Mock +} + +func (db *dbMock) BeginStateTransaction(ctx context.Context) (pgx.Tx, error) { + args := db.Called(ctx) + + tx, ok := args.Get(0).(pgx.Tx) + if !ok { + return nil, args.Error(1) + } + + return tx, args.Error(1) +} + +var _ EthTxManager = (*ethTxManagerMock)(nil) + +type ethTxManagerMock struct { + mock.Mock +} + +func (e *ethTxManagerMock) Add(ctx context.Context, owner, id string, + from common.Address, to *common.Address, value *big.Int, data []byte, dbTx pgx.Tx) error { + e.Called(ctx, owner, id, from, to, value, data, dbTx) + + return nil +} + +func (e *ethTxManagerMock) Result(ctx context.Context, owner, + id string, dbTx pgx.Tx) (ethtxmanager.MonitoredTxResult, error) { + args := e.Called(ctx, owner, id, dbTx) + + return args.Get(0).(ethtxmanager.MonitoredTxResult), args.Error(1) //nolint:forcetypeassert +} + +func (e *ethTxManagerMock) ResultsByStatus(ctx context.Context, owner string, + statuses []ethtxmanager.MonitoredTxStatus, dbTx pgx.Tx) ([]ethtxmanager.MonitoredTxResult, error) { + e.Called(ctx, owner, statuses, dbTx) + + return nil, nil +} + +func (e *ethTxManagerMock) ProcessPendingMonitoredTxs(ctx context.Context, owner string, + failedResultHandler ethtxmanager.ResultHandler, dbTx pgx.Tx) { + e.Called(ctx, owner, failedResultHandler, dbTx) +} + +var _ pgx.Tx = (*txMock)(nil) + +type txMock struct { + mock.Mock +} + +func (tx *txMock) Begin(ctx context.Context) (pgx.Tx, error) { + return nil, nil +} + +func (tx *txMock) BeginFunc(ctx context.Context, f func(pgx.Tx) error) (err error) { + return nil +} + +func (tx *txMock) Commit(ctx context.Context) error { + return nil +} + +func (tx *txMock) Rollback(ctx context.Context) error { + args := tx.Called(ctx) + + return args.Error(0) +} + +func (tx *txMock) CopyFrom(ctx context.Context, tableName pgx.Identifier, + columnNames []string, rowSrc pgx.CopyFromSource) (int64, error) { + return 0, nil +} + +func (tx *txMock) SendBatch(ctx context.Context, b *pgx.Batch) pgx.BatchResults { + return nil +} + +func (tx *txMock) LargeObjects() pgx.LargeObjects { + return pgx.LargeObjects{} +} + +func (tx *txMock) Prepare(ctx context.Context, name, sql string) (*pgconn.StatementDescription, error) { + return nil, nil +} + +func (tx *txMock) Exec(ctx context.Context, sql string, arguments ...interface{}) (commandTag pgconn.CommandTag, err error) { + return nil, nil +} + +func (tx *txMock) Query(ctx context.Context, sql string, args ...interface{}) (pgx.Rows, error) { + return nil, nil +} + +func (tx *txMock) QueryRow(ctx context.Context, sql string, args ...interface{}) pgx.Row { + return nil +} + +func (tx *txMock) QueryFunc(ctx context.Context, sql string, + args []interface{}, scans []interface{}, f func(pgx.QueryFuncRow) error) (pgconn.CommandTag, error) { + return nil, nil +} + +func (tx *txMock) Conn() *pgx.Conn { + return nil +} + +func TestInteropEndpoints_GetTxStatus(t *testing.T) { + t.Parallel() + + t.Run("BeginStateTransaction returns an error", func(t *testing.T) { + t.Parallel() + + dbMock := new(dbMock) + dbMock.On("BeginStateTransaction", mock.Anything).Return(nil, errors.New("error")).Once() + + i := NewInteropEndpoints( + common.HexToAddress("0xadmin"), + dbMock, + new(ethermanMock), + nil, + new(ethTxManagerMock), + ) + + result, err := i.GetTxStatus(common.HexToHash("0xsomeTxHash")) + + require.Equal(t, "0x0", result) + require.ErrorContains(t, err, "failed to begin dbTx") + + dbMock.AssertExpectations(t) + }) + + t.Run("failed to get tx", func(t *testing.T) { + t.Parallel() + + txHash := common.HexToHash("0xsomeTxHash") + + txMock := new(txMock) + txMock.On("Rollback", mock.Anything).Return(nil).Once() + + dbMock := new(dbMock) + dbMock.On("BeginStateTransaction", mock.Anything).Return(txMock, nil).Once() + + txManagerMock := new(ethTxManagerMock) + txManagerMock.On("Result", mock.Anything, ethTxManOwner, txHash.Hex(), txMock). + Return(ethtxmanager.MonitoredTxResult{}, errors.New("error")).Once() + + i := NewInteropEndpoints( + common.HexToAddress("0xadmin"), + dbMock, + new(ethermanMock), + nil, + txManagerMock, + ) + + result, err := i.GetTxStatus(txHash) + + require.Equal(t, "0x0", result) + require.ErrorContains(t, err, "failed to get tx") + + dbMock.AssertExpectations(t) + txMock.AssertExpectations(t) + txManagerMock.AssertExpectations(t) + }) + + t.Run("happy path", func(t *testing.T) { + t.Parallel() + + to := common.HexToAddress("0xreceiver") + txHash := common.HexToHash("0xsomeTxHash") + result := ethtxmanager.MonitoredTxResult{ + ID: "1", + Status: ethtxmanager.MonitoredTxStatusConfirmed, + Txs: map[common.Hash]ethtxmanager.TxResult{ + txHash: { + Tx: types.NewTransaction(1, to, big.NewInt(100_000), 21000, big.NewInt(10_000), nil), + }, + }, + } + + txMock := new(txMock) + txMock.On("Rollback", mock.Anything).Return(nil).Once() + + dbMock := new(dbMock) + dbMock.On("BeginStateTransaction", mock.Anything).Return(txMock, nil).Once() + + txManagerMock := new(ethTxManagerMock) + txManagerMock.On("Result", mock.Anything, ethTxManOwner, txHash.Hex(), txMock). + Return(result, nil).Once() + + i := NewInteropEndpoints( + common.HexToAddress("0xadmin"), + dbMock, + new(ethermanMock), + nil, + txManagerMock, + ) + + status, err := i.GetTxStatus(txHash) + + require.NoError(t, err) + require.Equal(t, "confirmed", status) + + dbMock.AssertExpectations(t) + txMock.AssertExpectations(t) + txManagerMock.AssertExpectations(t) + }) +} + +func TestInteropEndpoints_SendTx(t *testing.T) { + t.Parallel() + + t.Run("don't have given contract in map", func(t *testing.T) { + t.Parallel() + + i := NewInteropEndpoints(common.HexToAddress("0xadmin"), nil, nil, make(FullNodeRPCs), nil) + + result, err := i.SendTx(tx.SignedTx{ + Tx: tx.Tx{ + L1Contract: common.HexToAddress("0xnonExistingContract"), + }, + }) + + require.Equal(t, "0x0", result) + require.ErrorContains(t, err, "there is no RPC registered") + }) + + t.Run("could not build verified ZKP tx data", func(t *testing.T) { + t.Parallel() + + fullNodeRPCs := FullNodeRPCs{ + common.BytesToAddress([]byte{1, 2, 3, 4}): "someRPC", + } + tnx := tx.Tx{ + L1Contract: common.BytesToAddress([]byte{1, 2, 3, 4}), + LastVerifiedBatch: validiumTypes.ArgUint64(1), + NewVerifiedBatch: *validiumTypes.ArgUint64Ptr(2), + } + + ethermanMock := new(ethermanMock) + ethermanMock.On("BuildTrustedVerifyBatchesTxData", + uint64(tnx.LastVerifiedBatch), uint64(tnx.NewVerifiedBatch), mock.Anything). + Return([]byte{}, errors.New("error")).Once() + + i := NewInteropEndpoints(common.HexToAddress("0xadmin"), nil, ethermanMock, fullNodeRPCs, nil) + + result, err := i.SendTx(tx.SignedTx{Tx: tnx}) + + require.Equal(t, "0x0", result) + require.ErrorContains(t, err, "failed to build verify ZKP tx") + + ethermanMock.AssertExpectations(t) + }) + + t.Run("could not verified ZKP", func(t *testing.T) { + t.Parallel() + + fullNodeRPCs := FullNodeRPCs{ + common.BytesToAddress([]byte{1, 2, 3, 4}): "someRPC", + } + tnx := tx.Tx{ + L1Contract: common.BytesToAddress([]byte{1, 2, 3, 4}), + LastVerifiedBatch: validiumTypes.ArgUint64(1), + NewVerifiedBatch: *validiumTypes.ArgUint64Ptr(2), + } + + ethermanMock := new(ethermanMock) + ethermanMock.On("BuildTrustedVerifyBatchesTxData", + uint64(tnx.LastVerifiedBatch), uint64(tnx.NewVerifiedBatch), mock.Anything). + Return([]byte{1, 2}, nil).Once() + ethermanMock.On("CallContract", mock.Anything, mock.Anything, mock.Anything). + Return([]byte{}, errors.New("error")).Once() + + i := NewInteropEndpoints(common.HexToAddress("0xadmin"), nil, ethermanMock, fullNodeRPCs, nil) + + result, err := i.SendTx(tx.SignedTx{Tx: tnx}) + + require.Equal(t, "0x0", result) + require.ErrorContains(t, err, "failed to call verify ZKP response") + + ethermanMock.AssertExpectations(t) + }) + + t.Run("could not get signer", func(t *testing.T) { + t.Parallel() + + fullNodeRPCs := FullNodeRPCs{ + common.BytesToAddress([]byte{1, 2, 3, 4}): "someRPC", + } + tnx := tx.Tx{ + L1Contract: common.BytesToAddress([]byte{1, 2, 3, 4}), + LastVerifiedBatch: validiumTypes.ArgUint64(1), + NewVerifiedBatch: *validiumTypes.ArgUint64Ptr(2), + } + + ethermanMock := new(ethermanMock) + ethermanMock.On("BuildTrustedVerifyBatchesTxData", + uint64(tnx.LastVerifiedBatch), uint64(tnx.NewVerifiedBatch), mock.Anything). + Return([]byte{1, 2}, nil).Once() + ethermanMock.On("CallContract", mock.Anything, mock.Anything, mock.Anything). + Return([]byte{1, 2}, nil).Once() + + i := NewInteropEndpoints(common.HexToAddress("0xadmin"), nil, ethermanMock, fullNodeRPCs, nil) + + result, err := i.SendTx(tx.SignedTx{Tx: tnx}) + + require.Equal(t, "0x0", result) + require.ErrorContains(t, err, "failed to get signer") + + ethermanMock.AssertExpectations(t) + }) + + t.Run("failed to get admin from L1", func(t *testing.T) { + t.Parallel() + + privateKey, err := crypto.GenerateKey() + require.NoError(t, err) + + fullNodeRPCs := FullNodeRPCs{ + common.BytesToAddress([]byte{1, 2, 3, 4}): "someRPC", + } + tr := tx.Tx{ + L1Contract: common.BytesToAddress([]byte{1, 2, 3, 4}), + LastVerifiedBatch: validiumTypes.ArgUint64(1), + NewVerifiedBatch: *validiumTypes.ArgUint64Ptr(2), + } + signedTx, err := tr.Sign(privateKey) + require.NoError(t, err) + + ethermanMock := new(ethermanMock) + ethermanMock.On("BuildTrustedVerifyBatchesTxData", + uint64(tr.LastVerifiedBatch), uint64(tr.NewVerifiedBatch), mock.Anything). + Return([]byte{1, 2}, nil).Once() + ethermanMock.On("CallContract", mock.Anything, mock.Anything, mock.Anything). + Return([]byte{1, 2}, nil).Once() + ethermanMock.On("GetSequencerAddr", tr.L1Contract).Return(common.Address{}, errors.New("error")).Once() + + i := NewInteropEndpoints(common.HexToAddress("0xadmin"), nil, ethermanMock, fullNodeRPCs, nil) + + result, err := i.SendTx(*signedTx) + + require.Equal(t, "0x0", result) + require.ErrorContains(t, err, "failed to get admin from L1") + + ethermanMock.AssertExpectations(t) + }) + + t.Run("unexpected signer", func(t *testing.T) { + t.Parallel() + + privateKey, err := crypto.GenerateKey() + require.NoError(t, err) + + fullNodeRPCs := FullNodeRPCs{ + common.BytesToAddress([]byte{1, 2, 3, 4}): "someRPC", + } + tr := tx.Tx{ + L1Contract: common.BytesToAddress([]byte{1, 2, 3, 4}), + LastVerifiedBatch: validiumTypes.ArgUint64(1), + NewVerifiedBatch: *validiumTypes.ArgUint64Ptr(2), + } + signedTx, err := tr.Sign(privateKey) + require.NoError(t, err) + + ethermanMock := new(ethermanMock) + ethermanMock.On("BuildTrustedVerifyBatchesTxData", + uint64(tr.LastVerifiedBatch), uint64(tr.NewVerifiedBatch), mock.Anything). + Return([]byte{1, 2}, nil).Once() + ethermanMock.On("CallContract", mock.Anything, mock.Anything, mock.Anything). + Return([]byte{1, 2}, nil).Once() + ethermanMock.On("GetSequencerAddr", tr.L1Contract).Return(common.BytesToAddress([]byte{2, 3, 4}), nil).Once() + + i := NewInteropEndpoints(common.HexToAddress("0xadmin"), nil, ethermanMock, fullNodeRPCs, nil) + + result, err := i.SendTx(*signedTx) + + require.Equal(t, "0x0", result) + require.ErrorContains(t, err, "unexpected signer") + + ethermanMock.AssertExpectations(t) + }) +} From c8aba6d56c75024385844ccedb8bfc17467c2dc4 Mon Sep 17 00:00:00 2001 From: Goran Rojovic Date: Fri, 1 Dec 2023 10:44:09 +0100 Subject: [PATCH 02/16] SonarQube fixes --- rpc/rpc_test.go | 209 +++++++++++++++++++----------------------------- 1 file changed, 82 insertions(+), 127 deletions(-) diff --git a/rpc/rpc_test.go b/rpc/rpc_test.go index b5b5768..ec66eac 100644 --- a/rpc/rpc_test.go +++ b/rpc/rpc_test.go @@ -156,7 +156,7 @@ func (tx *txMock) Conn() *pgx.Conn { return nil } -func TestInteropEndpoints_GetTxStatus(t *testing.T) { +func TestInteropEndpointsGetTxStatus(t *testing.T) { t.Parallel() t.Run("BeginStateTransaction returns an error", func(t *testing.T) { @@ -258,27 +258,12 @@ func TestInteropEndpoints_GetTxStatus(t *testing.T) { }) } -func TestInteropEndpoints_SendTx(t *testing.T) { +func TestInteropEndpointsSendTx(t *testing.T) { t.Parallel() - t.Run("don't have given contract in map", func(t *testing.T) { - t.Parallel() - - i := NewInteropEndpoints(common.HexToAddress("0xadmin"), nil, nil, make(FullNodeRPCs), nil) - - result, err := i.SendTx(tx.SignedTx{ - Tx: tx.Tx{ - L1Contract: common.HexToAddress("0xnonExistingContract"), - }, - }) - - require.Equal(t, "0x0", result) - require.ErrorContains(t, err, "there is no RPC registered") - }) - - t.Run("could not build verified ZKP tx data", func(t *testing.T) { - t.Parallel() - + testWithError := func(ethermanMockFn func(tx.Tx) *ethermanMock, + shouldSignTx bool, + expectedError string) { fullNodeRPCs := FullNodeRPCs{ common.BytesToAddress([]byte{1, 2, 3, 4}): "someRPC", } @@ -288,146 +273,116 @@ func TestInteropEndpoints_SendTx(t *testing.T) { NewVerifiedBatch: *validiumTypes.ArgUint64Ptr(2), } - ethermanMock := new(ethermanMock) - ethermanMock.On("BuildTrustedVerifyBatchesTxData", - uint64(tnx.LastVerifiedBatch), uint64(tnx.NewVerifiedBatch), mock.Anything). - Return([]byte{}, errors.New("error")).Once() - - i := NewInteropEndpoints(common.HexToAddress("0xadmin"), nil, ethermanMock, fullNodeRPCs, nil) + signedTx := &tx.SignedTx{Tx: tnx} + if shouldSignTx { + privateKey, err := crypto.GenerateKey() + require.NoError(t, err) - result, err := i.SendTx(tx.SignedTx{Tx: tnx}) + stx, err := tnx.Sign(privateKey) + require.NoError(t, err) - require.Equal(t, "0x0", result) - require.ErrorContains(t, err, "failed to build verify ZKP tx") - - ethermanMock.AssertExpectations(t) - }) - - t.Run("could not verified ZKP", func(t *testing.T) { - t.Parallel() - - fullNodeRPCs := FullNodeRPCs{ - common.BytesToAddress([]byte{1, 2, 3, 4}): "someRPC", - } - tnx := tx.Tx{ - L1Contract: common.BytesToAddress([]byte{1, 2, 3, 4}), - LastVerifiedBatch: validiumTypes.ArgUint64(1), - NewVerifiedBatch: *validiumTypes.ArgUint64Ptr(2), + signedTx = stx } - ethermanMock := new(ethermanMock) - ethermanMock.On("BuildTrustedVerifyBatchesTxData", - uint64(tnx.LastVerifiedBatch), uint64(tnx.NewVerifiedBatch), mock.Anything). - Return([]byte{1, 2}, nil).Once() - ethermanMock.On("CallContract", mock.Anything, mock.Anything, mock.Anything). - Return([]byte{}, errors.New("error")).Once() + ethermanMock := ethermanMockFn(tnx) i := NewInteropEndpoints(common.HexToAddress("0xadmin"), nil, ethermanMock, fullNodeRPCs, nil) - result, err := i.SendTx(tx.SignedTx{Tx: tnx}) + result, err := i.SendTx(*signedTx) require.Equal(t, "0x0", result) - require.ErrorContains(t, err, "failed to call verify ZKP response") + require.ErrorContains(t, err, expectedError) ethermanMock.AssertExpectations(t) - }) + } - t.Run("could not get signer", func(t *testing.T) { + t.Run("don't have given contract in map", func(t *testing.T) { t.Parallel() - fullNodeRPCs := FullNodeRPCs{ - common.BytesToAddress([]byte{1, 2, 3, 4}): "someRPC", - } - tnx := tx.Tx{ - L1Contract: common.BytesToAddress([]byte{1, 2, 3, 4}), - LastVerifiedBatch: validiumTypes.ArgUint64(1), - NewVerifiedBatch: *validiumTypes.ArgUint64Ptr(2), - } - - ethermanMock := new(ethermanMock) - ethermanMock.On("BuildTrustedVerifyBatchesTxData", - uint64(tnx.LastVerifiedBatch), uint64(tnx.NewVerifiedBatch), mock.Anything). - Return([]byte{1, 2}, nil).Once() - ethermanMock.On("CallContract", mock.Anything, mock.Anything, mock.Anything). - Return([]byte{1, 2}, nil).Once() - - i := NewInteropEndpoints(common.HexToAddress("0xadmin"), nil, ethermanMock, fullNodeRPCs, nil) + i := NewInteropEndpoints(common.HexToAddress("0xadmin"), nil, nil, make(FullNodeRPCs), nil) - result, err := i.SendTx(tx.SignedTx{Tx: tnx}) + result, err := i.SendTx(tx.SignedTx{ + Tx: tx.Tx{ + L1Contract: common.HexToAddress("0xnonExistingContract"), + }, + }) require.Equal(t, "0x0", result) - require.ErrorContains(t, err, "failed to get signer") - - ethermanMock.AssertExpectations(t) + require.ErrorContains(t, err, "there is no RPC registered") }) - t.Run("failed to get admin from L1", func(t *testing.T) { + t.Run("could not build verified ZKP tx data", func(t *testing.T) { t.Parallel() - privateKey, err := crypto.GenerateKey() - require.NoError(t, err) - - fullNodeRPCs := FullNodeRPCs{ - common.BytesToAddress([]byte{1, 2, 3, 4}): "someRPC", - } - tr := tx.Tx{ - L1Contract: common.BytesToAddress([]byte{1, 2, 3, 4}), - LastVerifiedBatch: validiumTypes.ArgUint64(1), - NewVerifiedBatch: *validiumTypes.ArgUint64Ptr(2), - } - signedTx, err := tr.Sign(privateKey) - require.NoError(t, err) - - ethermanMock := new(ethermanMock) - ethermanMock.On("BuildTrustedVerifyBatchesTxData", - uint64(tr.LastVerifiedBatch), uint64(tr.NewVerifiedBatch), mock.Anything). - Return([]byte{1, 2}, nil).Once() - ethermanMock.On("CallContract", mock.Anything, mock.Anything, mock.Anything). - Return([]byte{1, 2}, nil).Once() - ethermanMock.On("GetSequencerAddr", tr.L1Contract).Return(common.Address{}, errors.New("error")).Once() + testWithError(func(tnx tx.Tx) *ethermanMock { + ethermanMock := new(ethermanMock) + ethermanMock.On("BuildTrustedVerifyBatchesTxData", + uint64(tnx.LastVerifiedBatch), uint64(tnx.NewVerifiedBatch), mock.Anything). + Return([]byte{}, errors.New("error")).Once() - i := NewInteropEndpoints(common.HexToAddress("0xadmin"), nil, ethermanMock, fullNodeRPCs, nil) + return ethermanMock + }, false, "failed to build verify ZKP tx") + }) - result, err := i.SendTx(*signedTx) + t.Run("could not verified ZKP", func(t *testing.T) { + t.Parallel() - require.Equal(t, "0x0", result) - require.ErrorContains(t, err, "failed to get admin from L1") + testWithError(func(tnx tx.Tx) *ethermanMock { + ethermanMock := new(ethermanMock) + ethermanMock.On("BuildTrustedVerifyBatchesTxData", + uint64(tnx.LastVerifiedBatch), uint64(tnx.NewVerifiedBatch), mock.Anything). + Return([]byte{1, 2}, nil).Once() + ethermanMock.On("CallContract", mock.Anything, mock.Anything, mock.Anything). + Return([]byte{}, errors.New("error")).Once() - ethermanMock.AssertExpectations(t) + return ethermanMock + }, false, "failed to call verify ZKP response") }) - t.Run("unexpected signer", func(t *testing.T) { + t.Run("could not get signer", func(t *testing.T) { t.Parallel() - privateKey, err := crypto.GenerateKey() - require.NoError(t, err) + testWithError(func(tnx tx.Tx) *ethermanMock { + ethermanMock := new(ethermanMock) + ethermanMock.On("BuildTrustedVerifyBatchesTxData", + uint64(tnx.LastVerifiedBatch), uint64(tnx.NewVerifiedBatch), mock.Anything). + Return([]byte{1, 2}, nil).Once() + ethermanMock.On("CallContract", mock.Anything, mock.Anything, mock.Anything). + Return([]byte{1, 2}, nil).Once() - fullNodeRPCs := FullNodeRPCs{ - common.BytesToAddress([]byte{1, 2, 3, 4}): "someRPC", - } - tr := tx.Tx{ - L1Contract: common.BytesToAddress([]byte{1, 2, 3, 4}), - LastVerifiedBatch: validiumTypes.ArgUint64(1), - NewVerifiedBatch: *validiumTypes.ArgUint64Ptr(2), - } - signedTx, err := tr.Sign(privateKey) - require.NoError(t, err) - - ethermanMock := new(ethermanMock) - ethermanMock.On("BuildTrustedVerifyBatchesTxData", - uint64(tr.LastVerifiedBatch), uint64(tr.NewVerifiedBatch), mock.Anything). - Return([]byte{1, 2}, nil).Once() - ethermanMock.On("CallContract", mock.Anything, mock.Anything, mock.Anything). - Return([]byte{1, 2}, nil).Once() - ethermanMock.On("GetSequencerAddr", tr.L1Contract).Return(common.BytesToAddress([]byte{2, 3, 4}), nil).Once() + return ethermanMock + }, false, "failed to get signer") + }) - i := NewInteropEndpoints(common.HexToAddress("0xadmin"), nil, ethermanMock, fullNodeRPCs, nil) + t.Run("failed to get admin from L1", func(t *testing.T) { + t.Parallel() - result, err := i.SendTx(*signedTx) + testWithError(func(tnx tx.Tx) *ethermanMock { + ethermanMock := new(ethermanMock) + ethermanMock.On("BuildTrustedVerifyBatchesTxData", + uint64(tnx.LastVerifiedBatch), uint64(tnx.NewVerifiedBatch), mock.Anything). + Return([]byte{1, 2}, nil).Once() + ethermanMock.On("CallContract", mock.Anything, mock.Anything, mock.Anything). + Return([]byte{1, 2}, nil).Once() + ethermanMock.On("GetSequencerAddr", tnx.L1Contract).Return(common.Address{}, errors.New("error")).Once() + + return ethermanMock + }, true, "failed to get admin from L1") + }) - require.Equal(t, "0x0", result) - require.ErrorContains(t, err, "unexpected signer") + t.Run("unexpected signer", func(t *testing.T) { + t.Parallel() - ethermanMock.AssertExpectations(t) + testWithError(func(tnx tx.Tx) *ethermanMock { + ethermanMock := new(ethermanMock) + ethermanMock.On("BuildTrustedVerifyBatchesTxData", + uint64(tnx.LastVerifiedBatch), uint64(tnx.NewVerifiedBatch), mock.Anything). + Return([]byte{1, 2}, nil).Once() + ethermanMock.On("CallContract", mock.Anything, mock.Anything, mock.Anything). + Return([]byte{1, 2}, nil).Once() + ethermanMock.On("GetSequencerAddr", tnx.L1Contract).Return(common.BytesToAddress([]byte{2, 3, 4}), nil).Once() + + return ethermanMock + }, true, "unexpected signer") }) } From 40f04dc76e4879b7fcac1e2ee769db27dfc518c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Negovanovi=C4=87?= Date: Fri, 1 Dec 2023 12:28:51 +0100 Subject: [PATCH 03/16] Fix go mod --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index bd60642..d87545c 100644 --- a/go.mod +++ b/go.mod @@ -8,6 +8,7 @@ require ( github.com/ethereum/go-ethereum v1.12.0 github.com/gobuffalo/packr/v2 v2.8.3 github.com/jackc/pgx/v4 v4.18.1 + github.com/jackc/pgconn v1.14.1 github.com/mitchellh/mapstructure v1.5.0 github.com/prometheus/client_golang v1.17.0 github.com/rubenv/sql-migrate v1.5.2 @@ -64,7 +65,6 @@ require ( github.com/iden3/go-iden3-crypto v0.0.15 // indirect github.com/invopop/jsonschema v0.7.0 // indirect github.com/jackc/chunkreader/v2 v2.0.1 // indirect - github.com/jackc/pgconn v1.14.1 // indirect github.com/jackc/pgio v1.0.0 // indirect github.com/jackc/pgpassfile v1.0.0 // indirect github.com/jackc/pgproto3/v2 v2.3.2 // indirect From 4ff3f5292931940e338c265d4f455f87ac41701d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Negovanovi=C4=87?= Date: Fri, 1 Dec 2023 12:34:02 +0100 Subject: [PATCH 04/16] Introduce unit tests execution workflow --- .github/workflows/unit-test.yml | 38 +++++++++++++++++++++++++++++++++ Makefile | 6 +++++- 2 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/unit-test.yml diff --git a/.github/workflows/unit-test.yml b/.github/workflows/unit-test.yml new file mode 100644 index 0000000..6f29f21 --- /dev/null +++ b/.github/workflows/unit-test.yml @@ -0,0 +1,38 @@ +name: Unit Tests +on: + push: + branches: + - main + pull_request: + workflow_dispatch: + workflow_call: + outputs: + workflow_output: + description: "Unit tests output" + value: ${{ jobs.go_test.outputs.test_output_failure }} + +jobs: + go_test: + name: Execution + runs-on: ubuntu-latest + outputs: + test_output_failure: ${{ steps.run_tests_failure.outputs.test_output }} + steps: + - name: Setup Go + uses: actions/setup-go@v3 + with: + go-version: 1.21.x + + - name: Checkout Code + uses: actions/checkout@v3 + with: + submodules: recursive + fetch-depth: 0 # Shallow clones should be disabled for a better relevancy of analysis + + - name: Run Go Test + run: make unit-tests + + - name: Run Go Test Failed + if: failure() + id: run_tests_failure + run: echo "test_output=false" >> $GITHUB_OUTPUT diff --git a/Makefile b/Makefile index dbfc22f..7838eef 100644 --- a/Makefile +++ b/Makefile @@ -30,6 +30,8 @@ stop-docker: check-docker destroy-docker: check-docker install-linter: check-go check-curl lint: check-go +unit-tests: check-go +e2e-tests: check-go ARCH := $(shell uname -m) @@ -103,4 +105,6 @@ help: ## Prints the help .PHONY: e2e-tests e2e-tests: ## Runs E2E tests - go test -v -timeout=30m github.com/0xPolygon/beethoven/test +.PHONY: unit-tests +unit-tests: ## Runs unit tests + go test $(go list ./... | grep -v ./test) -v -timeout=5m From 7c0d9cb7d650eb9bd027d2a7df1508b1fdedc0e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Negovanovi=C4=87?= Date: Fri, 1 Dec 2023 12:44:34 +0100 Subject: [PATCH 05/16] Fix unit tests execution --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 7838eef..9d0bcd7 100644 --- a/Makefile +++ b/Makefile @@ -107,4 +107,4 @@ help: ## Prints the help e2e-tests: ## Runs E2E tests .PHONY: unit-tests unit-tests: ## Runs unit tests - go test $(go list ./... | grep -v ./test) -v -timeout=5m + go test -v `go list ./... | grep -v test` From f74688f9f9e43c1315b5f780d5ca4ddaa8a6c733 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Negovanovi=C4=87?= Date: Fri, 1 Dec 2023 12:47:22 +0100 Subject: [PATCH 06/16] Provide coverage, race, shuffle and timeout flags --- .gitignore | 3 ++- Makefile | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index 6c3a496..157cc48 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,3 @@ dist -docker/data \ No newline at end of file +docker/data +coverage.out \ No newline at end of file diff --git a/Makefile b/Makefile index 9d0bcd7..fa9bfe9 100644 --- a/Makefile +++ b/Makefile @@ -107,4 +107,4 @@ help: ## Prints the help e2e-tests: ## Runs E2E tests .PHONY: unit-tests unit-tests: ## Runs unit tests - go test -v `go list ./... | grep -v test` + go test -v -timeout=5m -race -shuffle=on -coverprofile coverage.out `go list ./... | grep -v test` From a750ba53a7c2289a2ed453853818f34cb90b40e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Negovanovi=C4=87?= Date: Fri, 1 Dec 2023 12:49:28 +0100 Subject: [PATCH 07/16] Minor changes in e2e workflow --- .github/workflows/{e2e.yaml => e2e.yml} | 1 + .gitignore | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) rename .github/workflows/{e2e.yaml => e2e.yml} (97%) diff --git a/.github/workflows/e2e.yaml b/.github/workflows/e2e.yml similarity index 97% rename from .github/workflows/e2e.yaml rename to .github/workflows/e2e.yml index d1341c2..844cc72 100644 --- a/.github/workflows/e2e.yaml +++ b/.github/workflows/e2e.yml @@ -13,6 +13,7 @@ on: # yamllint disable-line rule:truthy jobs: execution: + name: "Execution" runs-on: ubuntu-latest env: CI_VERBOSE: true diff --git a/.gitignore b/.gitignore index 157cc48..5aecbe5 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,3 @@ dist docker/data -coverage.out \ No newline at end of file +coverage.out From 97a0fe7d58551b3c8c03b1a1259011637bd27e00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Negovanovi=C4=87?= Date: Fri, 1 Dec 2023 13:11:40 +0100 Subject: [PATCH 08/16] Format Makefile --- Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile b/Makefile index fa9bfe9..7e1b7ac 100644 --- a/Makefile +++ b/Makefile @@ -105,6 +105,7 @@ help: ## Prints the help .PHONY: e2e-tests e2e-tests: ## Runs E2E tests + .PHONY: unit-tests unit-tests: ## Runs unit tests go test -v -timeout=5m -race -shuffle=on -coverprofile coverage.out `go list ./... | grep -v test` From 3a2fd088882189871b53773ed79367f57060cb48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Negovanovi=C4=87?= Date: Fri, 1 Dec 2023 13:13:09 +0100 Subject: [PATCH 09/16] Rebase fix --- Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile b/Makefile index 7e1b7ac..e1303d5 100644 --- a/Makefile +++ b/Makefile @@ -105,6 +105,7 @@ help: ## Prints the help .PHONY: e2e-tests e2e-tests: ## Runs E2E tests + go test -v -timeout=30m github.com/0xPolygon/beethoven/test .PHONY: unit-tests unit-tests: ## Runs unit tests From 3eff1a98b93226cfb5e4471f9b3301c75e670fbe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Negovanovi=C4=87?= Date: Fri, 1 Dec 2023 13:46:30 +0100 Subject: [PATCH 10/16] Add ResolveAddr test cases --- cmd/main.go | 2 +- {pkg/network => network}/network.go | 0 network/network_test.go | 59 +++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 1 deletion(-) rename {pkg/network => network}/network.go (100%) create mode 100644 network/network_test.go diff --git a/cmd/main.go b/cmd/main.go index 48c241f..86c1e9d 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -26,7 +26,7 @@ import ( "github.com/0xPolygon/beethoven/config" "github.com/0xPolygon/beethoven/db" "github.com/0xPolygon/beethoven/etherman" - "github.com/0xPolygon/beethoven/pkg/network" + "github.com/0xPolygon/beethoven/network" "github.com/0xPolygon/beethoven/rpc" ) diff --git a/pkg/network/network.go b/network/network.go similarity index 100% rename from pkg/network/network.go rename to network/network.go diff --git a/network/network_test.go b/network/network_test.go new file mode 100644 index 0000000..2be57ce --- /dev/null +++ b/network/network_test.go @@ -0,0 +1,59 @@ +package network + +import ( + "fmt" + "net" + "testing" + + "github.com/stretchr/testify/require" +) + +func Test_ResolveAddr(t *testing.T) { + t.Parallel() + + tcpAddrBuilder := func(t *testing.T, address string) *net.TCPAddr { + tcpAddr, err := net.ResolveTCPAddr("", address) + require.NoError(t, err) + + return tcpAddr + } + + cases := []struct { + name string + address string + defaultIP string + errMsg string + }{ + { + name: "incorrect address", + address: "Foo Bar", + errMsg: "failed to parse addr", + }, + { + name: "only port provided", + address: ":8080", + defaultIP: "127.0.0.1", + }, + { + name: "both address and port provided", + address: "255.0.255.0:8080", + defaultIP: "", + }, + } + + for _, c := range cases { + c := c + t.Run(c.name, func(t *testing.T) { + t.Parallel() + + ipAddr, err := ResolveAddr(c.address, c.defaultIP) + if c.errMsg != "" { + require.ErrorContains(t, err, c.errMsg) + } else { + require.NoError(t, err) + expectedIPAddr := tcpAddrBuilder(t, fmt.Sprintf("%s%s", c.defaultIP, c.address)) + require.Equal(t, expectedIPAddr, ipAddr) + } + }) + } +} From b1d7af707d4c364b3a48ec5f8483bafebe84526e Mon Sep 17 00:00:00 2001 From: Goran Rojovic Date: Fri, 1 Dec 2023 14:51:10 +0100 Subject: [PATCH 11/16] Cover all rpc.go file with UTs --- rpc/interfaces.go | 4 + rpc/rpc.go | 32 ++-- rpc/rpc_test.go | 400 +++++++++++++++++++++++++++++++++++++--------- 3 files changed, 348 insertions(+), 88 deletions(-) diff --git a/rpc/interfaces.go b/rpc/interfaces.go index 7a552fa..112bf16 100644 --- a/rpc/interfaces.go +++ b/rpc/interfaces.go @@ -33,3 +33,7 @@ type EthTxManager interface { type ZkEVMClientInterface interface { BatchByNumber(ctx context.Context, number *big.Int) (*types.Batch, error) } + +type ZkEVMClientClientCreator interface { + NewClient(rpc string) ZkEVMClientInterface +} diff --git a/rpc/rpc.go b/rpc/rpc.go index 5d7ff51..b317f7e 100644 --- a/rpc/rpc.go +++ b/rpc/rpc.go @@ -21,13 +21,22 @@ const ( type FullNodeRPCs map[common.Address]string +var _ ZkEVMClientClientCreator = (*zkEVMClientCreator)(nil) + +type zkEVMClientCreator struct{} + +func (zc *zkEVMClientCreator) NewClient(rpc string) ZkEVMClientInterface { + return client.NewClient(rpc) +} + // InteropEndpoints contains implementations for the "interop" RPC endpoints type InteropEndpoints struct { - db DBInterface - etherman EthermanInterface - interopAdminAddr common.Address - fullNodeRPCs FullNodeRPCs - ethTxManager EthTxManager + db DBInterface + etherman EthermanInterface + interopAdminAddr common.Address + fullNodeRPCs FullNodeRPCs + ethTxManager EthTxManager + zkEVMClientCreator ZkEVMClientClientCreator } // NewInteropEndpoints returns InteropEndpoints @@ -39,11 +48,12 @@ func NewInteropEndpoints( ethTxManager EthTxManager, ) *InteropEndpoints { return &InteropEndpoints{ - db: db, - interopAdminAddr: interopAdminAddr, - etherman: etherman, - fullNodeRPCs: fullNodeRPCs, - ethTxManager: ethTxManager, + db: db, + interopAdminAddr: interopAdminAddr, + etherman: etherman, + fullNodeRPCs: fullNodeRPCs, + ethTxManager: ethTxManager, + zkEVMClientCreator: &zkEVMClientCreator{}, } } @@ -92,7 +102,7 @@ func (i *InteropEndpoints) SendTx(signedTx tx.SignedTx) (interface{}, types.Erro // Check expected root vs root from the managed full node // TODO: go stateless, depends on https://github.com/0xPolygonHermez/zkevm-prover/issues/581 // when this happens we should go async from here, since processing all the batches could take a lot of time - zkEVMClient := client.NewClient(i.fullNodeRPCs[signedTx.Tx.L1Contract]) + zkEVMClient := i.zkEVMClientCreator.NewClient(i.fullNodeRPCs[signedTx.Tx.L1Contract]) batch, err := zkEVMClient.BatchByNumber( ctx, big.NewInt(int64(signedTx.Tx.NewVerifiedBatch)), diff --git a/rpc/rpc_test.go b/rpc/rpc_test.go index ec66eac..64a4d6a 100644 --- a/rpc/rpc_test.go +++ b/rpc/rpc_test.go @@ -70,9 +70,9 @@ type ethTxManagerMock struct { func (e *ethTxManagerMock) Add(ctx context.Context, owner, id string, from common.Address, to *common.Address, value *big.Int, data []byte, dbTx pgx.Tx) error { - e.Called(ctx, owner, id, from, to, value, data, dbTx) + args := e.Called(ctx, owner, id, from, to, value, data, dbTx) - return nil + return args.Error(0) } func (e *ethTxManagerMock) Result(ctx context.Context, owner, @@ -109,7 +109,9 @@ func (tx *txMock) BeginFunc(ctx context.Context, f func(pgx.Tx) error) (err erro } func (tx *txMock) Commit(ctx context.Context) error { - return nil + args := tx.Called(ctx) + + return args.Error(0) } func (tx *txMock) Rollback(ctx context.Context) error { @@ -156,6 +158,35 @@ func (tx *txMock) Conn() *pgx.Conn { return nil } +var _ ZkEVMClientInterface = (*zkEVMClientMock)(nil) + +type zkEVMClientMock struct { + mock.Mock +} + +func (zkc *zkEVMClientMock) BatchByNumber(ctx context.Context, number *big.Int) (*validiumTypes.Batch, error) { + args := zkc.Called(ctx, number) + + batch, ok := args.Get(0).(*validiumTypes.Batch) + if !ok { + return nil, args.Error(1) + } + + return batch, args.Error(1) +} + +var _ ZkEVMClientClientCreator = (*zkEVMClientCreatorMock)(nil) + +type zkEVMClientCreatorMock struct { + mock.Mock +} + +func (zc *zkEVMClientCreatorMock) NewClient(rpc string) ZkEVMClientInterface { + args := zc.Called(rpc) + + return args.Get(0).(ZkEVMClientInterface) //nolint:forcetypeassert +} + func TestInteropEndpointsGetTxStatus(t *testing.T) { t.Parallel() @@ -261,9 +292,23 @@ func TestInteropEndpointsGetTxStatus(t *testing.T) { func TestInteropEndpointsSendTx(t *testing.T) { t.Parallel() - testWithError := func(ethermanMockFn func(tx.Tx) *ethermanMock, - shouldSignTx bool, - expectedError string) { + type testConfig struct { + isL1ContractInMap bool + canBuildZKProof bool + isZKProofValid bool + isTxSigned bool + isAdminRetrieved bool + isSignerValid bool + canGetBatch bool + isBatchValid bool + isDbTxOpen bool + isTxAddedToEthTxMan bool + isTxCommitted bool + + expectedError string + } + + testFn := func(cfg testConfig) { fullNodeRPCs := FullNodeRPCs{ common.BytesToAddress([]byte{1, 2, 3, 4}): "someRPC", } @@ -271,118 +316,319 @@ func TestInteropEndpointsSendTx(t *testing.T) { L1Contract: common.BytesToAddress([]byte{1, 2, 3, 4}), LastVerifiedBatch: validiumTypes.ArgUint64(1), NewVerifiedBatch: *validiumTypes.ArgUint64Ptr(2), + ZKP: tx.ZKP{ + NewStateRoot: common.BigToHash(big.NewInt(11)), + NewLocalExitRoot: common.BigToHash(big.NewInt(11)), + }, } - signedTx := &tx.SignedTx{Tx: tnx} - if shouldSignTx { - privateKey, err := crypto.GenerateKey() - require.NoError(t, err) + ethermanMock := new(ethermanMock) + zkEVMClientCreatorMock := new(zkEVMClientCreatorMock) + zkEVMClientMock := new(zkEVMClientMock) + dbMock := new(dbMock) + txMock := new(txMock) + ethTxManagerMock := new(ethTxManagerMock) + + executeTestFn := func() { + i := NewInteropEndpoints(common.HexToAddress("0xadmin"), dbMock, ethermanMock, fullNodeRPCs, ethTxManagerMock) + i.zkEVMClientCreator = zkEVMClientCreatorMock + + result, err := i.SendTx(*signedTx) + + if cfg.expectedError != "" { + require.Equal(t, "0x0", result) + require.ErrorContains(t, err, cfg.expectedError) + } else { + require.NoError(t, err) + require.Equal(t, signedTx.Tx.Hash(), result) + } + + ethermanMock.AssertExpectations(t) + zkEVMClientCreatorMock.AssertExpectations(t) + zkEVMClientMock.AssertExpectations(t) + dbMock.AssertExpectations(t) + txMock.AssertExpectations(t) + ethTxManagerMock.AssertExpectations(t) + } - stx, err := tnx.Sign(privateKey) - require.NoError(t, err) + if !cfg.isL1ContractInMap { + fullNodeRPCs = FullNodeRPCs{} + executeTestFn() - signedTx = stx + return } - ethermanMock := ethermanMockFn(tnx) + if !cfg.canBuildZKProof { + ethermanMock.On("BuildTrustedVerifyBatchesTxData", + uint64(tnx.LastVerifiedBatch), uint64(tnx.NewVerifiedBatch), mock.Anything). + Return([]byte{}, errors.New("error")).Once() + executeTestFn() + + return + } - i := NewInteropEndpoints(common.HexToAddress("0xadmin"), nil, ethermanMock, fullNodeRPCs, nil) + ethermanMock.On("BuildTrustedVerifyBatchesTxData", + uint64(tnx.LastVerifiedBatch), uint64(tnx.NewVerifiedBatch), mock.Anything). + Return([]byte{1, 2}, nil).Once() - result, err := i.SendTx(*signedTx) + if !cfg.isZKProofValid { + ethermanMock.On("CallContract", mock.Anything, mock.Anything, mock.Anything). + Return([]byte{}, errors.New("error")).Once() + executeTestFn() - require.Equal(t, "0x0", result) - require.ErrorContains(t, err, expectedError) + return + } + + ethermanMock.On("CallContract", mock.Anything, mock.Anything, mock.Anything). + Return([]byte{1, 2}, nil).Once() + + if !cfg.isTxSigned { + executeTestFn() + + return + } + + privateKey, err := crypto.GenerateKey() + require.NoError(t, err) + + stx, err := tnx.Sign(privateKey) + require.NoError(t, err) + + signedTx = stx + + if !cfg.isAdminRetrieved { + ethermanMock.On("GetSequencerAddr", tnx.L1Contract).Return(common.Address{}, errors.New("error")).Once() + executeTestFn() + + return + } + + if !cfg.isSignerValid { + ethermanMock.On("GetSequencerAddr", tnx.L1Contract).Return(common.BytesToAddress([]byte{1, 2, 3, 4}), nil).Once() + executeTestFn() + + return + } + + ethermanMock.On("GetSequencerAddr", tnx.L1Contract).Return(crypto.PubkeyToAddress(privateKey.PublicKey), nil).Once() + zkEVMClientCreatorMock.On("NewClient", mock.Anything).Return(zkEVMClientMock) + + if !cfg.canGetBatch { + zkEVMClientMock.On("BatchByNumber", mock.Anything, big.NewInt(int64(signedTx.Tx.NewVerifiedBatch))).Return( + nil, errors.New("error"), + ).Once() + executeTestFn() + + return + } + + if !cfg.isBatchValid { + zkEVMClientMock.On("BatchByNumber", mock.Anything, big.NewInt(int64(signedTx.Tx.NewVerifiedBatch))).Return( + &validiumTypes.Batch{ + StateRoot: common.BigToHash(big.NewInt(12)), + }, nil, + ).Once() + executeTestFn() + + return + } + + zkEVMClientMock.On("BatchByNumber", mock.Anything, big.NewInt(int64(signedTx.Tx.NewVerifiedBatch))).Return( + &validiumTypes.Batch{ + StateRoot: common.BigToHash(big.NewInt(11)), + LocalExitRoot: common.BigToHash(big.NewInt(11)), + }, nil, + ).Once() + + if !cfg.isDbTxOpen { + dbMock.On("BeginStateTransaction", mock.Anything).Return(nil, errors.New("error")).Once() + executeTestFn() + + return + } + + dbMock.On("BeginStateTransaction", mock.Anything).Return(txMock, nil).Once() + + if !cfg.isTxAddedToEthTxMan { + ethTxManagerMock.On("Add", mock.Anything, ethTxManOwner, signedTx.Tx.Hash().Hex(), mock.Anything, + mock.Anything, mock.Anything, mock.Anything, txMock).Return(errors.New("error")).Once() + txMock.On("Rollback", mock.Anything).Return(nil).Once() + executeTestFn() - ethermanMock.AssertExpectations(t) + return + } + + ethTxManagerMock.On("Add", mock.Anything, ethTxManOwner, signedTx.Tx.Hash().Hex(), mock.Anything, + mock.Anything, mock.Anything, mock.Anything, txMock).Return(nil).Once() + + if !cfg.isTxCommitted { + txMock.On("Commit", mock.Anything).Return(errors.New("error")).Once() + executeTestFn() + + return + } + + txMock.On("Commit", mock.Anything).Return(nil).Once() + executeTestFn() } t.Run("don't have given contract in map", func(t *testing.T) { t.Parallel() - i := NewInteropEndpoints(common.HexToAddress("0xadmin"), nil, nil, make(FullNodeRPCs), nil) - - result, err := i.SendTx(tx.SignedTx{ - Tx: tx.Tx{ - L1Contract: common.HexToAddress("0xnonExistingContract"), - }, + testFn(testConfig{ + isL1ContractInMap: false, + expectedError: "there is no RPC registered", }) - - require.Equal(t, "0x0", result) - require.ErrorContains(t, err, "there is no RPC registered") }) t.Run("could not build verified ZKP tx data", func(t *testing.T) { t.Parallel() - testWithError(func(tnx tx.Tx) *ethermanMock { - ethermanMock := new(ethermanMock) - ethermanMock.On("BuildTrustedVerifyBatchesTxData", - uint64(tnx.LastVerifiedBatch), uint64(tnx.NewVerifiedBatch), mock.Anything). - Return([]byte{}, errors.New("error")).Once() - - return ethermanMock - }, false, "failed to build verify ZKP tx") + testFn(testConfig{ + isL1ContractInMap: true, + canBuildZKProof: false, + expectedError: "failed to build verify ZKP tx", + }) }) t.Run("could not verified ZKP", func(t *testing.T) { t.Parallel() - testWithError(func(tnx tx.Tx) *ethermanMock { - ethermanMock := new(ethermanMock) - ethermanMock.On("BuildTrustedVerifyBatchesTxData", - uint64(tnx.LastVerifiedBatch), uint64(tnx.NewVerifiedBatch), mock.Anything). - Return([]byte{1, 2}, nil).Once() - ethermanMock.On("CallContract", mock.Anything, mock.Anything, mock.Anything). - Return([]byte{}, errors.New("error")).Once() - - return ethermanMock - }, false, "failed to call verify ZKP response") + testFn(testConfig{ + isL1ContractInMap: true, + canBuildZKProof: true, + isZKProofValid: false, + expectedError: "failed to call verify ZKP response", + }) }) t.Run("could not get signer", func(t *testing.T) { t.Parallel() - testWithError(func(tnx tx.Tx) *ethermanMock { - ethermanMock := new(ethermanMock) - ethermanMock.On("BuildTrustedVerifyBatchesTxData", - uint64(tnx.LastVerifiedBatch), uint64(tnx.NewVerifiedBatch), mock.Anything). - Return([]byte{1, 2}, nil).Once() - ethermanMock.On("CallContract", mock.Anything, mock.Anything, mock.Anything). - Return([]byte{1, 2}, nil).Once() - - return ethermanMock - }, false, "failed to get signer") + testFn(testConfig{ + isL1ContractInMap: true, + canBuildZKProof: true, + isZKProofValid: true, + isTxSigned: false, + expectedError: "failed to get signer", + }) }) t.Run("failed to get admin from L1", func(t *testing.T) { t.Parallel() - testWithError(func(tnx tx.Tx) *ethermanMock { - ethermanMock := new(ethermanMock) - ethermanMock.On("BuildTrustedVerifyBatchesTxData", - uint64(tnx.LastVerifiedBatch), uint64(tnx.NewVerifiedBatch), mock.Anything). - Return([]byte{1, 2}, nil).Once() - ethermanMock.On("CallContract", mock.Anything, mock.Anything, mock.Anything). - Return([]byte{1, 2}, nil).Once() - ethermanMock.On("GetSequencerAddr", tnx.L1Contract).Return(common.Address{}, errors.New("error")).Once() - - return ethermanMock - }, true, "failed to get admin from L1") + testFn(testConfig{ + isL1ContractInMap: true, + canBuildZKProof: true, + isZKProofValid: true, + isTxSigned: true, + isAdminRetrieved: false, + expectedError: "failed to get admin from L1", + }) }) t.Run("unexpected signer", func(t *testing.T) { t.Parallel() - testWithError(func(tnx tx.Tx) *ethermanMock { - ethermanMock := new(ethermanMock) - ethermanMock.On("BuildTrustedVerifyBatchesTxData", - uint64(tnx.LastVerifiedBatch), uint64(tnx.NewVerifiedBatch), mock.Anything). - Return([]byte{1, 2}, nil).Once() - ethermanMock.On("CallContract", mock.Anything, mock.Anything, mock.Anything). - Return([]byte{1, 2}, nil).Once() - ethermanMock.On("GetSequencerAddr", tnx.L1Contract).Return(common.BytesToAddress([]byte{2, 3, 4}), nil).Once() + testFn(testConfig{ + isL1ContractInMap: true, + canBuildZKProof: true, + isZKProofValid: true, + isTxSigned: true, + isAdminRetrieved: true, + isSignerValid: false, + expectedError: "unexpected signer", + }) + }) - return ethermanMock - }, true, "unexpected signer") + t.Run("error on batch retrieval", func(t *testing.T) { + testFn(testConfig{ + isL1ContractInMap: true, + canBuildZKProof: true, + isZKProofValid: true, + isTxSigned: true, + isAdminRetrieved: true, + isSignerValid: true, + canGetBatch: false, + expectedError: "failed to get batch from our node", + }) + }) + + t.Run("unexpected batch", func(t *testing.T) { + testFn(testConfig{ + isL1ContractInMap: true, + canBuildZKProof: true, + isZKProofValid: true, + isTxSigned: true, + isAdminRetrieved: true, + isSignerValid: true, + canGetBatch: true, + isBatchValid: false, + expectedError: "Mismatch detected", + }) + }) + + t.Run("failed to begin dbTx", func(t *testing.T) { + testFn(testConfig{ + isL1ContractInMap: true, + canBuildZKProof: true, + isZKProofValid: true, + isTxSigned: true, + isAdminRetrieved: true, + isSignerValid: true, + canGetBatch: true, + isBatchValid: true, + isDbTxOpen: false, + expectedError: "failed to begin dbTx", + }) + }) + + t.Run("failed to add tx to ethTxMan", func(t *testing.T) { + testFn(testConfig{ + isL1ContractInMap: true, + canBuildZKProof: true, + isZKProofValid: true, + isTxSigned: true, + isAdminRetrieved: true, + isSignerValid: true, + canGetBatch: true, + isBatchValid: true, + isDbTxOpen: true, + isTxAddedToEthTxMan: false, + expectedError: "failed to add tx to ethTxMan", + }) + }) + + t.Run("failed to commit tx", func(t *testing.T) { + testFn(testConfig{ + isL1ContractInMap: true, + canBuildZKProof: true, + isZKProofValid: true, + isTxSigned: true, + isAdminRetrieved: true, + isSignerValid: true, + canGetBatch: true, + isBatchValid: true, + isDbTxOpen: true, + isTxAddedToEthTxMan: true, + isTxCommitted: false, + expectedError: "failed to commit dbTx", + }) + }) + + t.Run("happy path", func(t *testing.T) { + testFn(testConfig{ + isL1ContractInMap: true, + canBuildZKProof: true, + isZKProofValid: true, + isTxSigned: true, + isAdminRetrieved: true, + isSignerValid: true, + canGetBatch: true, + isBatchValid: true, + isDbTxOpen: true, + isTxAddedToEthTxMan: true, + isTxCommitted: true, + }) }) } From d764849bef5def2afeba6a607ede068e6eaca569 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Negovanovi=C4=87?= Date: Fri, 1 Dec 2023 15:05:26 +0100 Subject: [PATCH 12/16] Fix SonarCloud issue --- network/network_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/network/network_test.go b/network/network_test.go index 2be57ce..946133d 100644 --- a/network/network_test.go +++ b/network/network_test.go @@ -8,7 +8,7 @@ import ( "github.com/stretchr/testify/require" ) -func Test_ResolveAddr(t *testing.T) { +func TestResolveAddr(t *testing.T) { t.Parallel() tcpAddrBuilder := func(t *testing.T, address string) *net.TCPAddr { @@ -36,7 +36,7 @@ func Test_ResolveAddr(t *testing.T) { }, { name: "both address and port provided", - address: "255.0.255.0:8080", + address: "0.0.0.0:9000", defaultIP: "", }, } From ab3b0122b9dc1a4a8d545087fff7515147e52b8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Negovanovi=C4=87?= Date: Mon, 4 Dec 2023 13:24:59 +0100 Subject: [PATCH 13/16] Add SonarCloud code coverage report --- sonar-project.properties | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 sonar-project.properties diff --git a/sonar-project.properties b/sonar-project.properties new file mode 100644 index 0000000..099753f --- /dev/null +++ b/sonar-project.properties @@ -0,0 +1,35 @@ + +# ===================================================== +# Standard properties +# ===================================================== + +sonar.projectKey=0xPolygon_beethoven +sonar.projectName=Beethoven +sonar.organization=0xpolygon +# sonar.projectVersion=1.0 + +sonar.sources=. +sonar.exclusions=**/*_test.go,**/vendor/** + +sonar.tests=. +sonar.test.inclusions=**/*_test.go +sonar.test.exclusions=**/vendor/** + +# ===================================================== +# Meta-data for the project +# ===================================================== + +sonar.links.homepage=https://github.com/0xPolygon/beethoven +sonar.links.ci=https://github.com/0xPolygon/beethoven/actions +sonar.links.scm=https://github.com/0xPolygon/beethoven +sonar.links.issue=https://github.com/0xPolygon/beethoven/issues + +# ===================================================== +# Properties specific to Go +# ===================================================== + +# sonar.go.gometalinter.reportPaths=gometalinter-report.out +#sonar.go.govet.reportPaths=govet-report.out +#sonar.go.golint.reportPaths=golint-report.out +sonar.go.tests.reportPaths=report.json +sonar.go.coverage.reportPaths=coverage.out \ No newline at end of file From 8b50f4c6fb58a64f91586d82389d952c9a73725e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Negovanovi=C4=87?= Date: Mon, 4 Dec 2023 15:46:14 +0100 Subject: [PATCH 14/16] Address comments --- .github/workflows/unit-test.yml | 8 ++++++++ sonar-project.properties | 4 ---- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/.github/workflows/unit-test.yml b/.github/workflows/unit-test.yml index 6f29f21..ab4af16 100644 --- a/.github/workflows/unit-test.yml +++ b/.github/workflows/unit-test.yml @@ -36,3 +36,11 @@ jobs: if: failure() id: run_tests_failure run: echo "test_output=false" >> $GITHUB_OUTPUT + + - name: SonarCloud Scan + if: ${{ env.HAVE_SONAR_TOKEN == 'true' }} + uses: SonarSource/sonarcloud-github-action@master + env: + HAVE_SONAR_TOKEN: ${{ secrets.SONAR_TOKEN != '' }} + GITHUB_TOKEN: ${{secrets.GITHUB_TOKEN }} + SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} diff --git a/sonar-project.properties b/sonar-project.properties index 099753f..8a8fd29 100644 --- a/sonar-project.properties +++ b/sonar-project.properties @@ -28,8 +28,4 @@ sonar.links.issue=https://github.com/0xPolygon/beethoven/issues # Properties specific to Go # ===================================================== -# sonar.go.gometalinter.reportPaths=gometalinter-report.out -#sonar.go.govet.reportPaths=govet-report.out -#sonar.go.golint.reportPaths=golint-report.out -sonar.go.tests.reportPaths=report.json sonar.go.coverage.reportPaths=coverage.out \ No newline at end of file From 7fb3436ff34f5bb0b07a90ea16581eedc0720e28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Negovanovi=C4=87?= Date: Tue, 5 Dec 2023 11:04:55 +0100 Subject: [PATCH 15/16] Format and remove commented out code --- .github/workflows/unit-test.yml | 10 +++++----- sonar-project.properties | 1 - 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/.github/workflows/unit-test.yml b/.github/workflows/unit-test.yml index ab4af16..db52f8d 100644 --- a/.github/workflows/unit-test.yml +++ b/.github/workflows/unit-test.yml @@ -2,14 +2,14 @@ name: Unit Tests on: push: branches: - - main + - main pull_request: workflow_dispatch: workflow_call: outputs: - workflow_output: - description: "Unit tests output" - value: ${{ jobs.go_test.outputs.test_output_failure }} + workflow_output: + description: "Unit tests output" + value: ${{ jobs.go_test.outputs.test_output_failure }} jobs: go_test: @@ -27,7 +27,7 @@ jobs: uses: actions/checkout@v3 with: submodules: recursive - fetch-depth: 0 # Shallow clones should be disabled for a better relevancy of analysis + fetch-depth: 0 # Shallow clones should be disabled for a better relevancy of analysis - name: Run Go Test run: make unit-tests diff --git a/sonar-project.properties b/sonar-project.properties index 8a8fd29..f20650b 100644 --- a/sonar-project.properties +++ b/sonar-project.properties @@ -6,7 +6,6 @@ sonar.projectKey=0xPolygon_beethoven sonar.projectName=Beethoven sonar.organization=0xpolygon -# sonar.projectVersion=1.0 sonar.sources=. sonar.exclusions=**/*_test.go,**/vendor/** From 4ccd91a5bb7f4f4d83c4be024f838ca9ca5ffaec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Negovanovi=C4=87?= Date: Wed, 6 Dec 2023 15:04:53 +0100 Subject: [PATCH 16/16] Try to enable code smells check --- sonar-project.properties | 1 + 1 file changed, 1 insertion(+) diff --git a/sonar-project.properties b/sonar-project.properties index f20650b..1e052fa 100644 --- a/sonar-project.properties +++ b/sonar-project.properties @@ -13,6 +13,7 @@ sonar.exclusions=**/*_test.go,**/vendor/** sonar.tests=. sonar.test.inclusions=**/*_test.go sonar.test.exclusions=**/vendor/** +sonar.issue.enforceSemantic=true # ===================================================== # Meta-data for the project