[Utility] Refactor and rename some existing functions related to handling transactions

persistence/block.go

@@ -18,12 +18,12 @@ func (p *persistenceModule) TransactionExists(transactionHash string) (bool, err
 	res, err := p.txIndexer.GetByHash(hash)
 	if res == nil {
 		// check for not found
-		if err != nil && err.Error() == kvstore.BadgerKeyNotFoundError {
+		if err != nil && err.Error() == kvstore.KeyNotFoundError {
 			return false, nil
 		}
 		return false, err
 	}
-	return true, err
+	return true, nil
 }
 
 func (p *PostgresContext) GetMinimumBlockHeight() (latestHeight uint64, err error) {

persistence/docs/CHANGELOG.md

@@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [0.0.0.56] - 2023-05-31
+
+- Renamed an error used by Badger / KVStore
+- Improve comments used for mock generation
+
 ## [0.0.0.55] - 2023-06-01
 
 - Integrate lazy loading SMT (release v0.5.0) into V1

persistence/indexer/indexer.go

@@ -1,7 +1,7 @@
-// TECHDEBT(andrew): Move this out of shared and alongside the mempool.
-
 package indexer
 
+//go:generate mockgen -package=mock_types -destination=../types/mocks/indexer_mock.go github.com/pokt-network/pocket/persistence/indexer TxIndexer
+
 import (
 	"encoding/hex"
 	"fmt"
@@ -11,8 +11,6 @@ import (
 	coreTypes "github.com/pokt-network/pocket/shared/core/types"
 )
 
-// Interface
-
 // `TxIndexer` interface defines methods to index and query transactions.
 type TxIndexer interface {
 	// `Index` analyzes, indexes and stores a single transaction result.

persistence/kvstore/kvstore.go

@@ -25,7 +25,7 @@ type KVStore interface {
 }
 
 const (
-	BadgerKeyNotFoundError = "Key not found"
+	KeyNotFoundError = "Key not found"
 )
 
 var (

rpc/doc/CHANGELOG.md

@@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [0.0.0.20] - 2023-05-30
+
+- Add a few clarifying comments
+- Use `utilityModule.GetIndexedTransaction` instead of `utilityUnitOfWork.HydrateTxId` which only retrieves indexed transactions instead of applying the underlying business logic
+
 ## [0.0.0.19] - 2023-05-24
 
 - Updates rpc handlers to use updated BlockStore interface

rpc/handlers.go

@@ -30,10 +30,12 @@ func (s *rpcServer) PostV1ClientBroadcastTxSync(ctx echo.Context) error {
 		return ctx.String(http.StatusBadRequest, "cannot decode tx bytes")
 	}
 
+	// Validate the transaction and add it to the mempool
 	if err := s.GetBus().GetUtilityModule().HandleTransaction(txBz); err != nil {
 		return ctx.String(http.StatusInternalServerError, err.Error())
 	}
 
+	// Broadcast the transaction to the rest of the network if it passed the basic validation above
 	if err := s.broadcastMessage(txBz); err != nil {
 		return ctx.String(http.StatusInternalServerError, err.Error())
 	}

rpc/utils.go

@@ -360,21 +360,15 @@ func (s *rpcServer) calculateMessageFeeForActor(actorType coreTypes.ActorType, m
 }
 
 // txProtoBytesToRPCIdxTxs converts a slice of serialised Transaction protobufs to a slice of RPC IdxTxs
+// ADDTEST: Add an E2E test (using the gherkin suite) that leverages this codepath
 func (s *rpcServer) txProtoBytesToRPCIdxTxs(txProtoBytes [][]byte) ([]IndexedTransaction, error) {
-	currentHeight := s.GetBus().GetConsensusModule().CurrentHeight()
-	uow, err := s.GetBus().GetUtilityModule().NewUnitOfWork(int64(currentHeight))
-	if err != nil {
-		return nil, err
-	}
-	defer uow.Release() //nolint:errcheck // We only need to make sure the UOW is released
-
 	txs := make([]IndexedTransaction, 0)
-	for idx, txBz := range txProtoBytes {
+	for _, txBz := range txProtoBytes {
 		tx := new(coreTypes.Transaction)
 		if err := codec.GetCodec().Unmarshal(txBz, tx); err != nil {
 			return nil, err
 		}
-		idxTx, er := uow.HydrateIdxTx(tx, idx)
+		idxTx, er := s.GetBus().GetUtilityModule().GetIndexedTransaction(txBz)
 		if er != nil {
 			return nil, er
 		}

shared/core/types/error.go

@@ -38,7 +38,7 @@ func NewError(code Code, msg string) Error {
 	}
 }
 
-// NextCode: 135
+// NextCode: 138
 type Code float64 // CONSIDERATION: Should these be a proto enum or a golang iota?
 
 //nolint:gosec // G101 - Not hard-coded credentials
@@ -53,6 +53,7 @@ const (
 	CodeNewPublicKeyFromBytesError       Code = 8
 	CodeNewAddressFromBytesError         Code = 9
 	CodeSignatureVerificationFailedError Code = 10
+	CodeRetrievingSignableBytesError     Code = 137
 	CodeHexDecodeFromStringError         Code = 11
 	CodeInvalidHashLengthError           Code = 12
 	CodeEmptyNetworkIDError              Code = 13
@@ -74,6 +75,7 @@ const (
 	CodeEmptyNonceError                  Code = 30
 	CodeEmptyPublicKeyError              Code = 31
 	CodeEmptySignatureError              Code = 32
+	CodeEmptySignatureStructureError     Code = 136
 	CodeDuplicateTransactionError        Code = 35
 	CodeTransactionSignError             Code = 36
 	CodeGetAllValidatorsError            Code = 37
@@ -137,6 +139,7 @@ const (
 	CodeEmptyParamValueError              Code = 93
 	CodeGetOutputAddressError             Code = 94
 	CodeTransactionAlreadyCommittedError  Code = 95
+	CodeTransactionNotCommittedError      Code = 135
 	CodeInitGenesisParamsError            Code = 96
 	CodeGetAllFishermenError              Code = 97
 	CodeGetAllServicersError              Code = 98
@@ -226,11 +229,13 @@ const (
 	InvalidNonceError                 = "the nonce field is invalid; cannot be converted to big.Int"
 	NewPublicKeyFromBytesError        = "unable to convert the raw bytes to a valid public key"
 	SignatureVerificationFailedError  = "the public key / signature combination is not valid for the msg"
+	RetrievingSignableBytesError      = "error retrieving signable bytes"
 	ProtoFromAnyError                 = "an error occurred getting the structure from the protobuf any"
 	NewFeeFromStringError             = "the fee string is unable to be converted to a valid base 10 number"
 	EmptyNonceError                   = "the nonce in the transaction is empty"
 	EmptyPublicKeyError               = "the public key field is empty"
 	EmptySignatureError               = "the signature field is empty"
+	EmptySignatureStructureError      = "the signature structure is empty"
 	TransactionSignError              = "an error occurred signing the transaction"
 	InterfaceConversionError          = "an error occurred converting the interface to an expected type: "
 	SetStatusPausedBeforeError        = "an error occurred setting the actor status that were paused before"
@@ -249,6 +254,7 @@ const (
 	GetOutputAddressError             = "an error occurred getting the output address using operator"
 	GetHeightError                    = "an error occurred when getting the height from the store"
 	TransactionAlreadyCommittedError  = "the transaction is already committed"
+	TransactionNotCommittedError      = "the transaction is not committed"
 	NewSavePointError                 = "an error occurred creating the save point"
 	RollbackSavePointError            = "an error occurred rolling back to save point"
 	NewPersistenceContextError        = "an error occurred creating the persistence context"
@@ -596,10 +602,18 @@ func ErrEmptySignature() Error {
 	return NewError(CodeEmptySignatureError, EmptySignatureError)
 }
 
+func ErrEmptySignatureStructure() Error {
+	return NewError(CodeEmptySignatureStructureError, EmptySignatureStructureError)
+}
+
 func ErrSignatureVerificationFailed() Error {
 	return NewError(CodeSignatureVerificationFailedError, SignatureVerificationFailedError)
 }
 
+func ErrRetrievingSignableBytes(err error) Error {
+	return NewError(CodeRetrievingSignableBytesError, fmt.Sprintf("%s: %s", RetrievingSignableBytesError, err.Error()))
+}
+
 func ErrDecodeMessage(err error) Error {
 	return NewError(CodeDecodeMessageError, fmt.Sprintf("%s: %s", DecodeMessageError, err.Error()))
 }
@@ -612,6 +626,10 @@ func ErrTransactionAlreadyCommitted() Error {
 	return NewError(CodeTransactionAlreadyCommittedError, TransactionAlreadyCommittedError)
 }
 
+func ErrTransactionNotCommitted() Error {
+	return NewError(CodeTransactionNotCommittedError, TransactionNotCommittedError)
+}
+
 func ErrTransactionSign(err error) Error {
 	return NewError(CodeTransactionSignError, fmt.Sprintf("%s: %s", TransactionSignError, err.Error()))
 }

shared/core/types/signature.go

@@ -1,13 +1,11 @@
 package types
 
-import "fmt"
-
 func (s *Signature) ValidateBasic() error {
 	if s.Signature == nil {
-		return fmt.Errorf("signature cannot be empty")
+		return ErrEmptySignature()
 	}
 	if s.PublicKey == nil {
-		return fmt.Errorf("public key cannot be empty")
+		return ErrEmptyPublicKey()
 	}
 	return nil
 }

shared/core/types/transaction.go

@@ -2,7 +2,6 @@ package types
 
 import (
 	"bytes"
-	"fmt"
 
 	"github.com/pokt-network/pocket/shared/codec"
 	"github.com/pokt-network/pocket/shared/crypto"
@@ -40,12 +39,12 @@ type ITransaction interface {
 func (tx *Transaction) ValidateBasic() error {
 	// Nonce cannot be empty to avoid transaction replays
 	if tx.Nonce == "" {
-		return fmt.Errorf("nonce cannot be empty") // ErrEmptyNonce
+		return ErrEmptyNonce()
 	}
 
 	// Is there a signature we can verify?
 	if tx.Signature == nil {
-		return fmt.Errorf("signature cannot be empty") // ErrEmptySignature
+		return ErrEmptySignatureStructure()
 	}
 	if err := tx.Signature.ValidateBasic(); err != nil {
 		return err
@@ -54,21 +53,21 @@ func (tx *Transaction) ValidateBasic() error {
 	// Does the transaction have a valid key?
 	publicKey, err := crypto.NewPublicKeyFromBytes(tx.Signature.PublicKey)
 	if err != nil {
-		return err // ErrEmptyPublicKey or ErrNewPublicKeyFromBytes
+		return ErrNewPublicKeyFromBytes(err)
 	}
 
 	// Is there a valid msg that can be decoded?
 	if _, err := tx.GetMessage(); err != nil {
-		return err // ? ErrBadMessage
+		return ErrDecodeMessage(err)
 	}
 
 	signBytes, err := tx.SignableBytes()
 	if err != nil {
-		return err // ? ErrBadSignature
+		return ErrRetrievingSignableBytes(err)
 	}
 
 	if ok := publicKey.Verify(signBytes, tx.Signature.Signature); !ok {
-		return fmt.Errorf("signature verification failed") // ErrSignatureVerificationFailed
+		return ErrSignatureVerificationFailed()
 	}
 
 	return nil

shared/modules/doc/CHANGELOG.md

@@ -7,6 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [0.0.0.13] - 2023-05-30
+
+- Added `GetIndexedTransaction` to the `UtilityModule` interface to be able to retrieve an indexed transaction without running the underlying business logic
+- Renamed `HydrateIdxTx` to `HandleTransaction` in the `UtilityUnitOfWork`interface so its more descriptive of what the function does
+- Renamed `anteHandleMessage` to `basicValidateTransaction`
+- Split the logic in `basicValidateTransaction` into multiple smaller functions for readability and so adding new business logic will be clearer
+
 ## [0.0.0.12] - 2023-05-16
 
 - Updates the PersistenceModule interface to return a BlockStore instead of KVStore directly
@@ -15,7 +22,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - `Consensus` - Updated debug interface functions: added `PushStateSyncMetadataResponse()`, removed `SetAggregatedStateSyncMetadata()` and `GetAggregatedStateSyncMetadataMaxHeight()`
 
-
 ## [0.0.0.10] - 2023-03-30
 
 - `Consensus` - improved documentation for supporting interfaces

shared/modules/utility_module.go

@@ -20,8 +20,12 @@ type UtilityModule interface {
 	NewUnitOfWork(height int64) (UtilityUnitOfWork, error)
 
 	// HandleTransaction does basic `Transaction` validation & adds it to the utility's module mempool if valid
+	// if it's valid. It does not process the business logic of the underlying message; see UtilityUnitOfWork.HandleTransaction.
 	HandleTransaction(tx []byte) error
 
+	// GetIndexedTransaction returns the indexed transaction if it is available in this node's view of the world state.
+	GetIndexedTransaction(tx []byte) (*coreTypes.IndexedTransaction, error)
+
 	// HandleRelay process the relay to the specified chain if this node is a servicer
 	HandleRelay(relay *coreTypes.Relay) (*coreTypes.RelayResponse, error) // TODO: Implement this
 
@@ -63,8 +67,9 @@ type UtilityUnitOfWork interface {
 	// TODO: Investigate a way to potentially simplify the interface by removing this function.
 	SetProposalBlock(blockHash string, proposerAddr []byte, txs [][]byte) error
 
-	// HydrateIdxTx hydrates a Transaction structure, with its index in the block returning a IndexedTransaction structure
-	HydrateIdxTx(tx *coreTypes.Transaction, index int) (*coreTypes.IndexedTransaction, coreTypes.Error)
+	// HandleTransaction validates the transaction, processes the business logic of the underlying message
+	// given the index in the current unit of work, and returns the resulting `IndexedTransaction` struct.
+	HandleTransaction(tx *coreTypes.Transaction, index int) (*coreTypes.IndexedTransaction, coreTypes.Error)
 
 	// ApplyBlock applies the context's in-memory proposed state (i.e. the txs in this context).
 	// Only intended to be used by the block verifiers (i.e. replicas).

utility/doc/CHANGELOG.md

@@ -7,13 +7,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [0.0.0.42] - 2023-05-30
+
+- Implemented `GetIndexedTransaction`
+- Add some comments and rename a few functions throughout to have things be more self-explanatory
+- Moved over all the business logic related to getting "signer candidates" into its own file for readability purposes
+
 ## [0.0.0.41] - 2023-05-30
 
 - Added the E2E Feature Path Template to document, present and explain how to go about defining and scoping utility level features
 
 ## [0.0.0.40] - 2023-05-04
 
-- Expose `HydrateIdxTx` for use outside of utility
+- Expose `HandleTransaction` for use outside of utility
 - Add placeholder functions `HandleRelay()` and `HandleChallenge()`
 - Moved `utility/types/errors.go` into `shared/core/types` and updated their usage accordingly
 

utility/transaction.go

@@ -1,6 +1,9 @@
 package utility
 
 import (
+	"encoding/hex"
+
+	"github.com/pokt-network/pocket/persistence/kvstore"
 	"github.com/pokt-network/pocket/shared/codec"
 	coreTypes "github.com/pokt-network/pocket/shared/core/types"
 )
@@ -35,3 +38,25 @@ func (u *utilityModule) HandleTransaction(txProtoBytes []byte) error {
 	// Store the tx in the mempool
 	return u.mempool.AddTx(txProtoBytes)
 }
+
+// GetIndexedTransaction implements the exposed functionality of the shared utilityModule interface.
+func (u *utilityModule) GetIndexedTransaction(txProtoBytes []byte) (*coreTypes.IndexedTransaction, error) {
+	txHash := coreTypes.TxHash(txProtoBytes)
+	persistenceModule := u.GetBus().GetPersistenceModule()
+
+	// TECHDEBT: Note the inconsistency between referencing tx hash as a string vs. byte slice in different places. Need to pick
+	// one and consolidate throughout the codebase
+	hash, err := hex.DecodeString(txHash)
+	if err != nil {
+		return nil, err
+	}
+	idTx, err := persistenceModule.GetTxIndexer().GetByHash(hash)
+	if err != nil {
+		if err.Error() == kvstore.KeyNotFoundError {
+			return nil, coreTypes.ErrTransactionNotCommitted()
+		}
+		return nil, err
+	}
+
+	return idTx, nil
+}