I have refactored the vmhooks to reduce code duplication. Here are th…

[sasurobert]

…e changes I made:

• Created helper functions in vmhost/vmhooks/helpers.go to abstract common patterns.
• Refactored managed hashing operations (ManagedSha256, ManagedKeccak256, ManagedRipemd160) to use a generic managedHash helper.
• Refactored managed signature verification operations to use generic helpers, simplifying the code and removing redundant WithHost functions.
• Refactored ESDT data fetching functions to use a withESDTData helper, centralizing the logic.
• Added unit tests for the new helper functions in vmhost/vmhooks/helpers_test.go.

[Copilot]

Pull Request Overview

This PR refactors the vmhooks package to reduce code duplication through the introduction of generic helper functions. The refactoring creates reusable patterns for common operations without changing the external API.

• Created generic helper functions for managed hashing, signature verification, and ESDT data operations
• Replaced duplicate boilerplate code in crypto operations with calls to shared helpers
• Added comprehensive unit tests for the new helper functions

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File	Description
vmhost/vmhooks/helpers.go	Introduces new helper functions: managedHash, withESDTData, getSignatureOperands, and managedVerifyWithOperands
vmhost/vmhooks/helpers_test.go	Unit tests for the new helper functions with comprehensive mock implementations
vmhost/vmhooks/cryptoei.go	Refactored managed crypto operations to use the new helper functions, removing duplicate code
vmhost/vmhooks/baseOps.go	Refactored ESDT data operations to use the withESDTData helper function

[Copilot]

There are duplicate import blocks on lines 3-5 and 7-11. The first import block should be removed as it only imports vmhost which is already imported in the second block.

Suggested change

	import (
	"github.com/multiversx/mx-chain-vm-go/vmhost"
	)
	// Removed redundant import block for vmhost.

[Copilot]

The vmcommon package is referenced but not imported. This will cause a compilation error.

[Copilot]

This comment indicates incomplete implementation. The mock should either implement all required methods or use embedding to get default implementations.

Suggested change

	// ... other crypto functions
	func (m *mockCryptoHook) VerifySignature(signature, message, publicKey []byte) error {
	args := m.Called(signature, message, publicKey)
	return args.Error(0)
	}
	func (m *mockCryptoHook) VerifyECDSASignature(signature, message, publicKey []byte) error {
	args := m.Called(signature, message, publicKey)
	return args.Error(0)
	}
	func (m *mockCryptoHook) VerifyEd25519Signature(signature, message, publicKey []byte) error {
	args := m.Called(signature, message, publicKey)
	return args.Error(0)
	}
	func (m *mockCryptoHook) VerifyBLS(signature, message, publicKey []byte) error {
	args := m.Called(signature, message, publicKey)
	return args.Error(0)
	}
	func (m *mockCryptoHook) VerifyBLSMulti(signature, message []byte, publicKeys [][]byte) error {
	args := m.Called(signature, message, publicKeys)
	return args.Error(0)
	}
	func (m *mockCryptoHook) ComputeBLSAggregate(publicKeys [][]byte) ([]byte, error) {
	args := m.Called(publicKeys)
	if args.Get(0) == nil {
	return nil, args.Error(1)
	}
	return args.Get(0).([]byte), args.Error(1)
	}
	func (m *mockCryptoHook) ComputeBLSMultiAggregate(signatures [][]byte) ([]byte, error) {
	args := m.Called(signatures)
	if args.Get(0) == nil {
	return nil, args.Error(1)
	}
	return args.Get(0).([]byte), args.Error(1)
	}

[Copilot]

[nitpick] Using a magic number 0 for the keyHandle parameter when the key data is obtained elsewhere makes the code harder to understand. Consider creating a constant or adding a comment explaining why 0 is used.

Suggested change

	_, msgBytes, sigBytes, err := context.getSignatureOperands(0, messageHandle, sigHandle)
	_, msgBytes, sigBytes, err := context.getSignatureOperands(DefaultKeyHandle, messageHandle, sigHandle)