improve error handling (#2274)

* improve error handling

* fix

* fix

* fix

go/common/crosschain.go

@@ -30,7 +30,7 @@ func (hashes CrossChainRootHashes) ToHexString() string {
 	return strings.Join(hexStrings, ",")
 }
 
-func (bundle ExtCrossChainBundle) HashPacked() common.Hash {
+func (bundle ExtCrossChainBundle) HashPacked() (common.Hash, error) {
 	uint256type, _ := abi.NewType("uint256", "", nil)
 	bytes32type, _ := abi.NewType("bytes32", "", nil)
 	bytesТype, _ := abi.NewType("bytes[]", "", nil)
@@ -52,9 +52,9 @@ func (bundle ExtCrossChainBundle) HashPacked() common.Hash {
 
 	bytes, err := args.Pack(bundle.LastBatchHash, bundle.L1BlockHash, bundle.L1BlockNum, bundle.CrossChainRootHashes)
 	if err != nil {
-		panic(err)
+		return common.Hash{}, err
 	}
 
 	hash := crypto.Keccak256Hash(bytes)
-	return hash
+	return hash, nil
 }

go/enclave/components/batch_executor.go

@@ -395,7 +395,11 @@ func (executor *batchExecutor) executeExistingBatch(ec *BatchExecutionContext) e
 
 func (executor *batchExecutor) readXChainMessages(ec *BatchExecutionContext) error {
 	if ec.SequencerNo.Int64() > int64(common.L2SysContractGenesisSeqNo) {
-		ec.xChainMsgs, ec.xChainValueMsgs = executor.crossChainProcessors.Local.RetrieveInboundMessages(ec.ctx, ec.parentL1Block, ec.l1block)
+		var err error
+		ec.xChainMsgs, ec.xChainValueMsgs, err = executor.crossChainProcessors.Local.RetrieveInboundMessages(ec.ctx, ec.parentL1Block, ec.l1block)
+		if err != nil {
+			return err
+		}
 	}
 	return nil
 }
@@ -405,7 +409,10 @@ func (executor *batchExecutor) execXChainMessages(ec *BatchExecutionContext) err
 		return err
 	}
 
-	crossChainTransactions := executor.crossChainProcessors.Local.CreateSyntheticTransactions(ec.ctx, ec.xChainMsgs, ec.xChainValueMsgs, ec.stateDB)
+	crossChainTransactions, err := executor.crossChainProcessors.Local.CreateSyntheticTransactions(ec.ctx, ec.xChainMsgs, ec.xChainValueMsgs, ec.stateDB)
+	if err != nil {
+		return err
+	}
 	executor.crossChainProcessors.Local.ExecuteValueTransfers(ec.ctx, ec.xChainValueMsgs, ec.stateDB)
 	xchainTxs := make(common.L2PricedTransactions, 0)
 	for _, xTx := range crossChainTransactions {
@@ -667,13 +674,19 @@ func (executor *batchExecutor) populateOutboundCrossChainData(ctx context.Contex
 
 	hasMessages := false
 	if len(valueTransferMessages) > 0 {
-		transfers := crosschain.ValueTransfers(valueTransferMessages).ForMerkleTree()
+		transfers, err := crosschain.ValueTransfers(valueTransferMessages).ForMerkleTree()
+		if err != nil {
+			return err
+		}
 		xchainTree = append(xchainTree, transfers...)
 		hasMessages = true
 	}
 
 	if len(crossChainMessages) > 0 {
-		messages := crosschain.MessageStructs(crossChainMessages).ForMerkleTree()
+		messages, err := crosschain.MessageStructs(crossChainMessages).ForMerkleTree()
+		if err != nil {
+			return fmt.Errorf("could not create cross chain tree. Cause: %w", err)
+		}
 		xchainTree = append(xchainTree, messages...)
 		hasMessages = true
 	}
@@ -688,7 +701,7 @@ func (executor *batchExecutor) populateOutboundCrossChainData(ctx context.Contex
 
 		encodedTree, err := json.Marshal(xchainTree)
 		if err != nil {
-			panic(err) // todo: figure out what to do
+			return fmt.Errorf("could not marshal cross chain tree. Cause: %w", err)
 		}
 
 		batch.Header.CrossChainTree = encodedTree

go/enclave/components/rollup_compression.go

@@ -384,7 +384,8 @@ func (rc *RollupCompression) calculateL1HeightsFromDeltas(calldataRollupHeader *
 			}
 			value := l1Delta.Int64() + int64(prevHeight)
 			if value < 0 {
-				rc.logger.Crit("Should not have a negative height")
+				rc.logger.Error("Should not have a negative height")
+				return nil, errors.New("negative height")
 			}
 			l1Heights = append(l1Heights, uint64(value))
 			prevHeight = uint64(value)

go/enclave/components/rollup_consumer.go

@@ -173,7 +173,10 @@ func (rc *rollupConsumerImpl) extractAndVerifyRollups(processed *common.Processe
 	txsSeen := make(map[gethcommon.Hash]bool)
 
 	for i, tx := range rollupTxs {
-		t := rc.MgmtContractLib.DecodeTx(tx.Transaction)
+		t, err := rc.MgmtContractLib.DecodeTx(tx.Transaction)
+		if err != nil {
+			rc.logger.Warn(fmt.Sprintf("could not decode tx at index %d. Cause: %s", i, err))
+		}
 		if t == nil {
 			continue
 		}

go/enclave/components/shared_secret_process.go

@@ -41,7 +41,11 @@ func (ssp *SharedSecretProcessor) ProcessNetworkSecretMsgs(ctx context.Context,
 
 	// process initialize secret events
 	for _, txData := range processed.GetEvents(common.InitialiseSecretTx) {
-		t := ssp.mgmtContractLib.DecodeTx(txData.Transaction)
+		t, err := ssp.mgmtContractLib.DecodeTx(txData.Transaction)
+		if err != nil {
+			ssp.logger.Warn("Could not decode transaction", log.ErrKey, err)
+			continue
+		}
 		initSecretTx, ok := t.(*common.L1InitializeSecretTx)
 		if !ok {
 			continue
@@ -60,7 +64,11 @@ func (ssp *SharedSecretProcessor) ProcessNetworkSecretMsgs(ctx context.Context,
 
 	// process secret requests
 	for _, txData := range processed.GetEvents(common.SecretRequestTx) {
-		t := ssp.mgmtContractLib.DecodeTx(txData.Transaction)
+		t, err := ssp.mgmtContractLib.DecodeTx(txData.Transaction)
+		if err != nil {
+			ssp.logger.Warn("Could not decode transaction", log.ErrKey, err)
+			continue
+		}
 		scrtReqTx, ok := t.(*common.L1RequestSecretTx)
 		if !ok {
 			continue

go/enclave/crosschain/block_message_extractor.go

@@ -56,7 +56,7 @@ func (m *blockMessageExtractor) StoreCrossChainValueTransfers(ctx context.Contex
 
 	err := m.storage.StoreValueTransfers(ctx, block.Hash(), transfers)
 	if err != nil {
-		m.logger.Crit("Unable to store the transfers", log.ErrKey, err)
+		m.logger.Error("Unable to store the transfers", log.ErrKey, err)
 		return err
 	}
 
@@ -88,7 +88,7 @@ func (m *blockMessageExtractor) StoreCrossChainMessages(ctx context.Context, blo
 		m.logger.Info(fmt.Sprintf("Storing %d messages for block", len(messages)), log.BlockHashKey, block.Hash())
 		err := m.storage.StoreL1Messages(ctx, block.Hash(), messages)
 		if err != nil {
-			m.logger.Crit("Unable to store the messages", log.ErrKey, err)
+			m.logger.Error("Unable to store the messages", log.ErrKey, err)
 			return err
 		}
 	}

go/enclave/crosschain/common.go

@@ -205,20 +205,23 @@ func (ms MessageStructs) EncodeIndex(index int, w *bytes.Buffer) {
 	}
 }
 
-func (ms MessageStructs) ForMerkleTree() [][]interface{} {
+func (ms MessageStructs) ForMerkleTree() ([][]interface{}, error) {
 	values := make([][]interface{}, 0)
 	for idx := range ms {
-		hashedVal := ms.HashPacked(idx)
+		hashedVal, err := ms.HashPacked(idx)
+		if err != nil {
+			return nil, err
+		}
 		val := []interface{}{
 			"m",
 			hashedVal,
 		}
 		values = append(values, val)
 	}
-	return values
+	return values, nil
 }
 
-func (ms MessageStructs) HashPacked(index int) gethcommon.Hash {
+func (ms MessageStructs) HashPacked(index int) (gethcommon.Hash, error) {
 	messageStruct := ms[index]
 
 	addrType, _ := abi.NewType("address", "", nil)
@@ -247,13 +250,12 @@ func (ms MessageStructs) HashPacked(index int) gethcommon.Hash {
 		},
 	}
 
-	// todo @siliev: err
 	packed, err := args.Pack(messageStruct.Sender, messageStruct.Sequence, messageStruct.Nonce, messageStruct.Topic, messageStruct.Payload, messageStruct.ConsistencyLevel)
 	if err != nil {
-		panic(err)
+		return gethcommon.Hash{}, fmt.Errorf("unable to pack message struct: %w", err)
 	}
 	hash := crypto.Keccak256Hash(packed)
-	return hash
+	return hash, nil
 }
 
 type ValueTransfers []common.ValueTransferEvent
@@ -269,20 +271,23 @@ func (vt ValueTransfers) EncodeIndex(index int, w *bytes.Buffer) {
 	}
 }
 
-func (vt ValueTransfers) ForMerkleTree() [][]interface{} {
+func (vt ValueTransfers) ForMerkleTree() ([][]interface{}, error) {
 	values := make([][]interface{}, 0)
 	for idx := range vt {
-		hashedVal := vt.HashPacked(idx)
+		hashedVal, err := vt.HashPacked(idx)
+		if err != nil {
+			return nil, err
+		}
 		val := []interface{}{
 			"v", // [v, "0xblabla"]
 			hashedVal,
 		}
 		values = append(values, val)
 	}
-	return values
+	return values, nil
 }
 
-func (vt ValueTransfers) HashPacked(index int) gethcommon.Hash {
+func (vt ValueTransfers) HashPacked(index int) (gethcommon.Hash, error) {
 	valueTransfer := vt[index]
 
 	uint256Type, _ := abi.NewType("uint256", "", nil)
@@ -306,11 +311,11 @@ func (vt ValueTransfers) HashPacked(index int) gethcommon.Hash {
 
 	bytes, err := args.Pack(valueTransfer.Sender, valueTransfer.Receiver, valueTransfer.Amount, valueTransfer.Sequence)
 	if err != nil {
-		panic(err)
+		return gethcommon.Hash{}, fmt.Errorf("unable to pack value transfer: %w", err)
 	}
 
 	hash := crypto.Keccak256Hash(bytes)
-	return hash
+	return hash, nil
 }
 
 var CrossChainEncodings = []string{smt.SOL_STRING, smt.SOL_BYTES32}

go/enclave/crosschain/interfaces.go

@@ -42,11 +42,11 @@ type Manager interface {
 
 	ExtractOutboundTransfers(ctx context.Context, receipts common.L2Receipts) (common.ValueTransferEvents, error)
 
-	CreateSyntheticTransactions(ctx context.Context, messages common.CrossChainMessages, transfers common.ValueTransferEvents, stateDB *state.StateDB) common.L2Transactions
+	CreateSyntheticTransactions(ctx context.Context, messages common.CrossChainMessages, transfers common.ValueTransferEvents, stateDB *state.StateDB) (common.L2Transactions, error)
 
 	ExecuteValueTransfers(ctx context.Context, transfers common.ValueTransferEvents, stateDB *state.StateDB)
 
-	RetrieveInboundMessages(ctx context.Context, fromBlock *types.Header, toBlock *types.Header) (common.CrossChainMessages, common.ValueTransferEvents)
+	RetrieveInboundMessages(ctx context.Context, fromBlock *types.Header, toBlock *types.Header) (common.CrossChainMessages, common.ValueTransferEvents, error)
 
 	system.SystemContractsInitializable
 }

go/enclave/crosschain/message_bus_manager.go

@@ -94,7 +94,7 @@ func (m *MessageBusManager) ExtractOutboundTransfers(_ context.Context, receipts
 // todo (@stefan) - fix ordering of messages, currently it is irrelevant.
 // todo (@stefan) - do not extract messages below their consistency level. Irrelevant security wise.
 // todo (@stefan) - surface errors
-func (m *MessageBusManager) RetrieveInboundMessages(ctx context.Context, fromBlock *types.Header, toBlock *types.Header) (common.CrossChainMessages, common.ValueTransferEvents) {
+func (m *MessageBusManager) RetrieveInboundMessages(ctx context.Context, fromBlock *types.Header, toBlock *types.Header) (common.CrossChainMessages, common.ValueTransferEvents, error) {
 	messages := make(common.CrossChainMessages, 0)
 	transfers := make(common.ValueTransferEvents, 0)
 
@@ -114,24 +114,24 @@ func (m *MessageBusManager) RetrieveInboundMessages(ctx context.Context, fromBlo
 
 		messagesForBlock, err := m.storage.GetL1Messages(ctx, b.Hash())
 		if err != nil {
-			m.logger.Crit("Reading the key for the block failed with uncommon reason.", log.ErrKey, err)
+			return nil, nil, fmt.Errorf("reading the key for the block failed with uncommon reason: %w", err)
 		}
 
 		transfersForBlock, err := m.storage.GetL1Transfers(ctx, b.Hash())
 		if err != nil {
-			m.logger.Crit("Unable to get L1 transfers for block that should be there.", log.ErrKey, err)
+			return nil, nil, fmt.Errorf("unable to get L1 transfers for block that should be there %w", err)
 		}
 
 		messages = append(messages, messagesForBlock...) // Ordering here might work in POBI, but might be weird for fast finality
 		transfers = append(transfers, transfersForBlock...)
 
 		// No deposits before genesis.
 		if b.Number.Uint64() < height {
-			m.logger.Crit("block height is less than genesis height")
+			return nil, nil, fmt.Errorf("block height is less than genesis height")
 		}
 		p, err := m.storage.FetchBlock(ctx, b.ParentHash)
 		if err != nil {
-			m.logger.Crit("Synthetic transactions can't be processed because the rollups are not on the same Ethereum fork")
+			return nil, nil, fmt.Errorf("synthetic transactions can't be processed because the rollups are not on the same Ethereum fork")
 		}
 		b = p
 	}
@@ -142,7 +142,7 @@ func (m *MessageBusManager) RetrieveInboundMessages(ctx context.Context, fromBlo
 	}
 	logf(fmt.Sprintf("Extracted cross chain messages for block height %d ->%d", fromBlock.Number.Uint64(), toBlock.Number.Uint64()), "no_msgs", len(messages), "no_value_transfers", len(transfers))
 
-	return messages, transfers
+	return messages, transfers, nil
 }
 
 const BalanceIncreaseXChainValueTransfer tracing.BalanceChangeReason = 110
@@ -155,9 +155,9 @@ func (m *MessageBusManager) ExecuteValueTransfers(ctx context.Context, transfers
 }
 
 // CreateSyntheticTransactions - generates transactions that the enclave should execute internally for the messages.
-func (m *MessageBusManager) CreateSyntheticTransactions(ctx context.Context, messages common.CrossChainMessages, transfers common.ValueTransferEvents, rollupState *state.StateDB) common.L2Transactions {
+func (m *MessageBusManager) CreateSyntheticTransactions(ctx context.Context, messages common.CrossChainMessages, transfers common.ValueTransferEvents, rollupState *state.StateDB) (common.L2Transactions, error) {
 	if len(messages) == 0 && len(transfers) == 0 {
-		return make(common.L2Transactions, 0)
+		return make(common.L2Transactions, 0), nil
 	}
 
 	if m.messageBusAddress == nil {
@@ -173,11 +173,7 @@ func (m *MessageBusManager) CreateSyntheticTransactions(ctx context.Context, mes
 		delayInBlocks := big.NewInt(int64(message.ConsistencyLevel))
 		data, err := MessageBusABI.Pack("storeCrossChainMessage", message, delayInBlocks)
 		if err != nil {
-			m.logger.Crit("Failed packing storeCrossChainMessage message!")
-			return syntheticTransactions
-
-			// todo (@stefan) - return error
-			// return nil, fmt.Errorf("failed packing submitOutOfNetworkMessage %w", err)
+			return nil, fmt.Errorf("failed packing storeCrossChainMessage %w", err)
 		}
 
 		tx := &types.LegacyTx{
@@ -198,8 +194,7 @@ func (m *MessageBusManager) CreateSyntheticTransactions(ctx context.Context, mes
 	for idx, transfer := range transfers {
 		data, err := MessageBusABI.Pack("notifyDeposit", transfer.Receiver, transfer.Amount)
 		if err != nil {
-			m.logger.Crit("Failed packing notifyDeposit message!")
-			return syntheticTransactions
+			return nil, fmt.Errorf("failed packing notifyDeposit %w", err)
 		}
 
 		tx := &types.LegacyTx{
@@ -213,5 +208,5 @@ func (m *MessageBusManager) CreateSyntheticTransactions(ctx context.Context, mes
 		syntheticTransactions = append(syntheticTransactions, types.NewTx(tx))
 	}
 
-	return syntheticTransactions
+	return syntheticTransactions, nil
 }

go/enclave/enclave_admin_service.go

@@ -308,7 +308,12 @@ func (e *enclaveAdminService) ExportCrossChainData(ctx context.Context, fromSeqN
 		return nil, err
 	}
 
-	sig, err := e.enclaveKeyService.Sign(bundle.HashPacked())
+	bytes, err := bundle.HashPacked()
+	if err != nil {
+		return nil, err
+	}
+
+	sig, err := e.enclaveKeyService.Sign(bytes)
 	if err != nil {
 		return nil, err
 	}

go/enclave/rpc/vk_utils.go

@@ -104,7 +104,7 @@ func HandleEncryptedRPC(ctx context.Context,
 	case rpc.ERPCGetPersonalTransactions:
 		return withVKEncryption(ctx, encManager, decodedRequest, vk, GetPersonalTransactionsValidate, GetPersonalTransactionsExecute)
 	default:
-		panic(fmt.Sprintf("unsupported method %s", decodedRequest.Method))
+		return nil, fmt.Errorf("unsupported method %s", decodedRequest.Method)
 	}
 }
 

go/enclave/system/contracts.go

@@ -3,6 +3,8 @@ package system
 import (
 	"fmt"
 
+	"github.com/ten-protocol/go-ten/go/common/log"
+
 	gethcommon "github.com/ethereum/go-ethereum/common"
 	gethlog "github.com/ethereum/go-ethereum/log"
 
@@ -32,7 +34,7 @@ func SystemDeployerInitTransaction(logger gethlog.Logger, eoaOwner gethcommon.Ad
 	abi, _ := SystemDeployer.SystemDeployerMetaData.GetAbi()
 	args, err := abi.Constructor.Inputs.Pack(eoaOwner)
 	if err != nil {
-		panic(err) // This error is fatal. If the system contracts can't be initialized the network cannot bootstrap.
+		logger.Crit("This error is fatal. If the system contracts can't be initialized the network cannot bootstrap.", log.ErrKey, err)
 	}
 
 	bytecode := gethcommon.FromHex(SystemDeployer.SystemDeployerMetaData.Bin)