From b1d7af707d4c364b3a48ec5f8483bafebe84526e Mon Sep 17 00:00:00 2001 From: Goran Rojovic Date: Fri, 1 Dec 2023 14:51:10 +0100 Subject: [PATCH] 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, + }) }) }