Refactor: Refactored p2p2core adapater

.golangci.yaml

@@ -46,6 +46,8 @@ linters:
               desc: use github.com/NethermindEth/juno/utils for logging
     dupl:
       threshold: 100
+    exhaustruct:
+      allow-empty-returns: true
     funlen:
       lines: 120
       statements: 50

adapters/p2p2core/class.go

@@ -14,58 +14,50 @@ import (
 	"github.com/starknet-io/starknet-p2pspecs/p2p/proto/class"
 )
 
-func AdaptSierraClass(cairo1 *class.Cairo1Class) (core.SierraClass, error) {
-	abiHash := crypto.StarknetKeccak([]byte(cairo1.Abi))
+func AdaptSierraClass(sierraClass *class.Cairo1Class) (core.SierraClass, error) {
+	abiHash := crypto.StarknetKeccak([]byte(sierraClass.Abi))
 
-	program := utils.Map(cairo1.Program, AdaptFelt)
-	compiled, err := createCompiledClass(cairo1)
+	program := utils.Map(sierraClass.Program, AdaptFelt)
+
+	casmClass, err := compileToCasm(sierraClass)
 	if err != nil {
 		return core.SierraClass{}, fmt.Errorf("invalid compiled class: %w", err)
 	}
 
-	adaptEP := func(points []*class.SierraEntryPoint) []core.SierraEntryPoint {
-		// usage of NonNilSlice is essential because relevant core class fields are non nil
-		return utils.Map(utils.NonNilSlice(points), adaptSierra)
-	}
-
-	entryPoints := cairo1.EntryPoints
+	entryPoints := sierraClass.EntryPoints
 	return core.SierraClass{
-		Abi:     cairo1.Abi,
+		Abi:     sierraClass.Abi,
 		AbiHash: abiHash,
 		EntryPoints: struct {
 			Constructor []core.SierraEntryPoint
 			External    []core.SierraEntryPoint
 			L1Handler   []core.SierraEntryPoint
 		}{
-			Constructor: adaptEP(entryPoints.Constructors),
-			External:    adaptEP(entryPoints.Externals),
-			L1Handler:   adaptEP(entryPoints.L1Handlers),
+			Constructor: adaptSierraEntryPoints(entryPoints.Constructors),
+			External:    adaptSierraEntryPoints(entryPoints.Externals),
+			L1Handler:   adaptSierraEntryPoints(entryPoints.L1Handlers),
 		},
 		Program:         program,
 		ProgramHash:     crypto.PoseidonArray(program...),
-		SemanticVersion: cairo1.ContractClassVersion,
-		Compiled:        compiled,
+		SemanticVersion: sierraClass.ContractClassVersion,
+		Compiled:        &casmClass,
 	}, nil
 }
 
-func AdaptClass(cls *class.Class) (core.ClassDefinition, error) {
+func AdaptClassDefinition(cls *class.Class) (core.ClassDefinition, error) {
 	if cls == nil {
 		return nil, nil
 	}
 
 	switch cls := cls.Class.(type) {
 	case *class.Class_Cairo0:
-		adaptEP := func(points []*class.EntryPoint) []core.DeprecatedEntryPoint {
-			// usage of NonNilSlice is essential because relevant core class fields are non nil
-			return utils.Map(utils.NonNilSlice(points), adaptEntryPoint)
-		}
 
 		deprecatedCairo := cls.Cairo0
 		return &core.DeprecatedCairoClass{
 			Abi:          json.RawMessage(deprecatedCairo.Abi),
-			Externals:    adaptEP(deprecatedCairo.Externals),
-			L1Handlers:   adaptEP(deprecatedCairo.L1Handlers),
-			Constructors: adaptEP(deprecatedCairo.Constructors),
+			Externals:    adaptDeprecatedEntryPoints(deprecatedCairo.Externals),
+			L1Handlers:   adaptDeprecatedEntryPoints(deprecatedCairo.L1Handlers),
+			Constructors: adaptDeprecatedEntryPoints(deprecatedCairo.Constructors),
 			Program:      deprecatedCairo.Program,
 		}, nil
 	case *class.Class_Cairo1:
@@ -76,37 +68,50 @@ func AdaptClass(cls *class.Class) (core.ClassDefinition, error) {
 	}
 }
 
-func adaptSierra(e *class.SierraEntryPoint) core.SierraEntryPoint {
-	return core.SierraEntryPoint{
-		Index:    e.Index,
-		Selector: AdaptFelt(e.Selector),
+func adaptSierraEntryPoints(points []*class.SierraEntryPoint) []core.SierraEntryPoint {
+	sierraEntryPoints := make([]core.SierraEntryPoint, len(points))
+	for i := range points {
+		sierraEntryPoints[i] = core.SierraEntryPoint{
+			Index: points[i].Index,
+			// todo(rdr): look for a way to get rid of this AdaptFelt or change it
+			Selector: AdaptFelt(points[i].Selector),
+		}
 	}
+	return sierraEntryPoints
 }
 
-func adaptEntryPoint(e *class.EntryPoint) core.DeprecatedEntryPoint {
-	return core.DeprecatedEntryPoint{
-		Selector: AdaptFelt(e.Selector),
-		Offset:   new(felt.Felt).SetUint64(e.Offset),
+func adaptDeprecatedEntryPoints(points []*class.EntryPoint) []core.DeprecatedEntryPoint {
+	deprecatedEntryPoints := make([]core.DeprecatedEntryPoint, len(points))
+	for i := range points {
+		deprecatedEntryPoints[i] = core.DeprecatedEntryPoint{
+			// todo(rdr): look for a way to get rid of this AdaptFelt or change it
+			Selector: AdaptFelt(points[i].Selector),
+			// todo(rdr): wHy do we store this as a felt instead of as a uint64
+			Offset: felt.NewFromUint64[felt.Felt](points[i].Offset),
+		}
 	}
+	return deprecatedEntryPoints
 }
 
-func createCompiledClass(cairo1 *class.Cairo1Class) (*core.CasmClass, error) {
-	if cairo1 == nil {
-		return nil, nil
-	}
+// todo(rdr): There is a p2p to starknet conversion here
+//            Can this whole code be differnet, Like adapting from p2p to core and from core to sn
+//            Or write a dedicated adapter. I think this option might be the best one
 
+func compileToCasm(cairo1 *class.Cairo1Class) (core.CasmClass, error) {
+	// todo(rdr): write a dedicatd function for this
 	adapt := func(ep *class.SierraEntryPoint) starknet.SierraEntryPoint {
 		return starknet.SierraEntryPoint{
 			Index:    ep.Index,
 			Selector: AdaptFelt(ep.Selector),
 		}
 	}
 	ep := cairo1.EntryPoints
+
 	def := &starknet.SierraClass{
 		Abi: cairo1.Abi,
 		EntryPoints: starknet.SierraEntryPoints{
-			// WARNING: usage of utils.NonNilSlice is essential, otherwise compilation will finish with errors
-			// todo move NonNilSlice to Compile ?
+			// todo(rdr): remove the this functional programming which doesn't pair well
+			//            with go (both style and performance wise)
 			Constructor: utils.Map(utils.NonNilSlice(ep.Constructors), adapt),
 			External:    utils.Map(utils.NonNilSlice(ep.Externals), adapt),
 			L1Handler:   utils.Map(utils.NonNilSlice(ep.L1Handlers), adapt),
@@ -117,8 +122,8 @@ func createCompiledClass(cairo1 *class.Cairo1Class) (*core.CasmClass, error) {
 
 	compiledClass, err := compiler.Compile(def)
 	if err != nil {
-		return nil, err
+		return core.CasmClass{}, err
 	}
 
-	return sn2core.AdaptCompiledClass(compiledClass)
+	return sn2core.AdaptCasmClass(compiledClass)
 }

adapters/p2p2core/state.go

@@ -24,7 +24,7 @@ func AdaptStateDiff(
 	)
 
 	for _, cls := range classes {
-		class, err := AdaptClass(cls)
+		class, err := AdaptClassDefinition(cls)
 		if err != nil {
 			return nil, fmt.Errorf("unsupported class: %w", err)
 		}

adapters/p2p2core/transaction.go

@@ -27,10 +27,17 @@ func AdaptDeclareV3WithClass(
 
 	declareCommon, err := AdaptDeclareV3TxnCommon(tx.Common, classHash, txnHash)
 	if err != nil {
-		return nil, nil, fmt.Errorf("failed to adapt declare v3 transaction common: %w", err)
+		return nil,
+			nil,
+			fmt.Errorf("failed to adapt declare v3 transaction common: %w", err)
 	}
-	if *class.Compiled.Hash() != *declareCommon.CompiledClassHash {
-		err := fmt.Errorf("compiled class hash mismatch: expected %s, got %s", class.Compiled.Hash(), declareCommon.CompiledClassHash)
+	compiledClassHash := class.Compiled.Hash()
+	if !compiledClassHash.Equal(declareCommon.CompiledClassHash) {
+		err := fmt.Errorf(
+			"compiled class hash mismatch: expected %s, got %s",
+			compiledClassHash,
+			declareCommon.CompiledClassHash,
+		)
 		return nil, nil, err
 	}
 

adapters/sn2core/sn2core.go

@@ -13,25 +13,28 @@ import (
 	"github.com/ethereum/go-ethereum/common"
 )
 
-func AdaptBlock(response *starknet.Block, sig *starknet.Signature) (*core.Block, error) {
-	if response == nil {
+// todo(rdr): this monoscript sn2core module is a bit messy. Refactor it into several files
+
+// todo(rdr): this should return `core.Block` by value
+func AdaptBlock(snBlock *starknet.Block, sig *starknet.Signature) (*core.Block, error) {
+	if snBlock == nil {
 		return nil, errors.New("nil client block")
 	}
 
-	txns := make([]core.Transaction, len(response.Transactions))
-	for i := range response.Transactions {
+	txns := make([]core.Transaction, len(snBlock.Transactions))
+	for i := range snBlock.Transactions {
 		var err error
-		txns[i], err = AdaptTransaction(response.Transactions[i])
+		txns[i], err = AdaptTransaction(snBlock.Transactions[i])
 		if err != nil {
 			return nil, err
 		}
 	}
 
-	receipts := make([]*core.TransactionReceipt, len(response.Receipts))
+	receipts := make([]*core.TransactionReceipt, len(snBlock.Receipts))
 	eventCount := uint64(0)
-	for i := range response.Receipts {
-		receipts[i] = AdaptTransactionReceipt(response.Receipts[i])
-		eventCount += uint64(len(response.Receipts[i].Events))
+	for i := range snBlock.Receipts {
+		receipts[i] = AdaptTransactionReceipt(snBlock.Receipts[i])
+		eventCount += uint64(len(snBlock.Receipts[i].Events))
 	}
 
 	sigs := [][]*felt.Felt{}
@@ -40,30 +43,33 @@ func AdaptBlock(response *starknet.Block, sig *starknet.Signature) (*core.Block,
 	}
 
 	return &core.Block{
+		// todo(rdr): core.Header should probably mostly be a value and not a reference
 		Header: &core.Header{
-			Hash:             response.Hash,
-			ParentHash:       response.ParentHash,
-			Number:           response.Number,
-			GlobalStateRoot:  response.StateRoot,
-			Timestamp:        response.Timestamp,
-			ProtocolVersion:  response.Version,
-			SequencerAddress: response.SequencerAddress,
-			TransactionCount: uint64(len(response.Transactions)),
+			Hash:             snBlock.Hash,
+			ParentHash:       snBlock.ParentHash,
+			Number:           snBlock.Number,
+			GlobalStateRoot:  snBlock.StateRoot,
+			Timestamp:        snBlock.Timestamp,
+			ProtocolVersion:  snBlock.Version,
+			SequencerAddress: snBlock.SequencerAddress,
+			TransactionCount: uint64(len(snBlock.Transactions)),
 			EventCount:       eventCount,
 			EventsBloom:      core.EventsBloom(receipts),
-			L1GasPriceETH:    response.L1GasPriceETH(),
-			L1GasPriceSTRK:   response.L1GasPriceSTRK(),
-			L1DAMode:         core.L1DAMode(response.L1DAMode),
-			L1DataGasPrice:   (*core.GasPrice)(response.L1DataGasPrice),
-			L2GasPrice:       (*core.GasPrice)(response.L2GasPrice),
+			L1GasPriceETH:    snBlock.L1GasPriceETH(),
+			L1GasPriceSTRK:   snBlock.L1GasPriceSTRK(),
+			L1DAMode:         core.L1DAMode(snBlock.L1DAMode),
+			L1DataGasPrice:   (*core.GasPrice)(snBlock.L1DataGasPrice),
+			L2GasPrice:       (*core.GasPrice)(snBlock.L2GasPrice),
 			Signatures:       sigs,
 		},
 		Transactions: txns,
 		Receipts:     receipts,
 	}, nil
 }
 
+// todo(rdr): this should return `core.TransactionReceipt` by value
 func AdaptTransactionReceipt(response *starknet.TransactionReceipt) *core.TransactionReceipt {
+	// todo(rdr): Does it makes sense to check for nil here?
 	if response == nil {
 		return nil
 	}
@@ -84,6 +90,7 @@ func AdaptTransactionReceipt(response *starknet.TransactionReceipt) *core.Transa
 	}
 }
 
+// todo(rdr): this should return `core.GasConsumed` by value
 func adaptGasConsumed(response *starknet.GasConsumed) *core.GasConsumed {
 	if response == nil {
 		return nil
@@ -96,6 +103,7 @@ func adaptGasConsumed(response *starknet.GasConsumed) *core.GasConsumed {
 	}
 }
 
+// todo(rdr): this should return `core.Event` by value
 func AdaptEvent(response *starknet.Event) *core.Event {
 	if response == nil {
 		return nil
@@ -108,6 +116,7 @@ func AdaptEvent(response *starknet.Event) *core.Event {
 	}
 }
 
+// todo(rdr): this should return `core.Event` by value
 func AdaptExecutionResources(response *starknet.ExecutionResources) *core.ExecutionResources {
 	if response == nil {
 		return nil
@@ -122,6 +131,7 @@ func AdaptExecutionResources(response *starknet.ExecutionResources) *core.Execut
 	}
 }
 
+// todo(rdr): this should return `core.L1toL2Message` by value
 func AdaptL1ToL2Message(response *starknet.L1ToL2Message) *core.L1ToL2Message {
 	if response == nil {
 		return nil
@@ -136,6 +146,7 @@ func AdaptL1ToL2Message(response *starknet.L1ToL2Message) *core.L1ToL2Message {
 	}
 }
 
+// todo(rdr): this should return `core.L2toL1Message` by value
 func AdaptL2ToL1Message(response *starknet.L2ToL1Message) *core.L2ToL1Message {
 	if response == nil {
 		return nil
@@ -316,7 +327,7 @@ func AdaptSierraClass(
 		return nil, errors.New("sierra program size is too small")
 	}
 
-	coreCompiledClass, err := AdaptCompiledClass(compiledClass)
+	casmClass, err := AdaptCasmClass(compiledClass)
 	if err != nil {
 		return nil, err
 	}
@@ -329,36 +340,41 @@ func AdaptSierraClass(
 		Abi:     response.Abi,
 		AbiHash: crypto.StarknetKeccak([]byte(response.Abi)),
 
-		Compiled: coreCompiledClass,
+		Compiled: &casmClass,
 
 		EntryPoints: adaptSierraEntrypoints(&response.EntryPoints),
 	}, nil
 }
 
-func AdaptCompiledClass(compiledClass *starknet.CasmClass) (*core.CasmClass, error) {
-	if compiledClass == nil {
-		return nil, nil
+func AdaptCasmClass(starknetCasm *starknet.CasmClass) (core.CasmClass, error) {
+	if starknetCasm == nil {
+		return core.CasmClass{}, nil
 	}
 
 	var casm core.CasmClass
-	casm.Bytecode = compiledClass.Bytecode
-	casm.PythonicHints = compiledClass.PythonicHints
-	casm.CompilerVersion = compiledClass.CompilerVersion
-	casm.Hints = compiledClass.Hints
-	casm.BytecodeSegmentLengths = AdaptSegmentLengths(compiledClass.BytecodeSegmentLengths)
+	casm.Bytecode = starknetCasm.Bytecode
+	casm.PythonicHints = starknetCasm.PythonicHints
+	casm.CompilerVersion = starknetCasm.CompilerVersion
+	casm.Hints = starknetCasm.Hints
+	casm.BytecodeSegmentLengths = AdaptSegmentLengths(starknetCasm.BytecodeSegmentLengths)
 
 	var ok bool
-	casm.Prime, ok = new(big.Int).SetString(compiledClass.Prime, 0)
+
+	// todo(rdr): the Prime in `starknetCasm` should be an felt.Felt I believe
+	// todo(rdr): the Prime in `casm` should be a felt as well
+	casm.Prime, ok = new(big.Int).SetString(starknetCasm.Prime, 0)
 	if !ok {
-		return nil, fmt.Errorf("couldn't convert prime value to big.Int: %d", casm.Prime)
+		return core.CasmClass{},
+			fmt.Errorf("couldn't convert prime value to big.Int: %d", casm.Prime)
 	}
 
-	entryPoints := compiledClass.EntryPoints
+	entryPoints := starknetCasm.EntryPoints
+	// todo(rdr): get rid of this utils.Map
 	casm.External = utils.Map(entryPoints.External, adaptCompiledEntryPoint)
 	casm.L1Handler = utils.Map(entryPoints.L1Handler, adaptCompiledEntryPoint)
 	casm.Constructor = utils.Map(entryPoints.Constructor, adaptCompiledEntryPoint)
 
-	return &casm, nil
+	return casm, nil
 }
 
 func AdaptSegmentLengths(l starknet.SegmentLengths) core.SegmentLengths {
@@ -581,13 +597,18 @@ func AdaptPreConfirmedBlock(
 	return core.NewPreConfirmed(adaptedBlock, &stateUpdate, txStateDiffs, candidateTxs), nil
 }
 
+// todo(rdr): First remove this function or find a better name. Second: make sure the operation
+//	is safe. If it is always going to be something that fits in a uint64 why are we
+//	storing a felt.Felt in the first place
+
 func safeFeltToUint64(f *felt.Felt) uint64 {
 	if f != nil {
 		return f.Uint64()
 	}
 	return 0
 }
 
+// todo(rdr): rename this to `adaptCasmEntryPoint`
 func adaptCompiledEntryPoint(entryPoint starknet.CompiledEntryPoint) core.CasmEntryPoint {
 	return core.CasmEntryPoint{
 		Offset:   entryPoint.Offset,