Merge branch 'UlyanaAndrukhiv/6497-refactor-executionNodesForBlockID'…

… of github.com:The-K-R-O-K/flow-go into UlyanaAndrukhiv/6413-backfill-tx-error-messages

README.md

@@ -54,7 +54,7 @@ The following table lists all work streams and links to their home directory and
 ## Installation
 
 - Clone this repository
-- Install [Go](https://golang.org/doc/install) (Flow supports Go 1.18 and later)
+- Install [Go](https://golang.org/doc/install) (Flow requires Go 1.22 and later)
 - Install [Docker](https://docs.docker.com/get-docker/), which is used for running a local network and integration tests
 - Make sure the [`GOPATH`](https://golang.org/cmd/go/#hdr-GOPATH_environment_variable) and `GOBIN` environment variables
   are set, and `GOBIN` is added to your path:

access/errors.go

@@ -12,6 +12,10 @@ import (
 // ErrUnknownReferenceBlock indicates that a transaction references an unknown block.
 var ErrUnknownReferenceBlock = errors.New("unknown reference block")
 
+// IndexReporterNotInitialized is returned when indexReporter is nil because
+// execution data syncing and indexing is disabled
+var IndexReporterNotInitialized = errors.New("index reported not initialized")
+
 // IncompleteTransactionError indicates that a transaction is missing one or more required fields.
 type IncompleteTransactionError struct {
 	MissingFields []string
@@ -115,3 +119,14 @@ func IsInsufficientBalanceError(err error) bool {
 	var balanceError InsufficientBalanceError
 	return errors.As(err, &balanceError)
 }
+
+// IndexedHeightFarBehindError indicates that a node is far behind on indexing.
+type IndexedHeightFarBehindError struct {
+	SealedHeight  uint64
+	IndexedHeight uint64
+}
+
+func (e IndexedHeightFarBehindError) Error() string {
+	return fmt.Sprintf("the difference between the latest sealed height (%d) and indexed height (%d) exceeds the maximum gap allowed",
+		e.SealedHeight, e.IndexedHeight)
+}

access/validator.go

@@ -20,22 +20,32 @@ import (
 	"github.com/onflow/flow-go/module"
 	"github.com/onflow/flow-go/module/execution"
 	"github.com/onflow/flow-go/module/metrics"
+	"github.com/onflow/flow-go/module/state_synchronization"
 	"github.com/onflow/flow-go/state"
 	"github.com/onflow/flow-go/state/protocol"
 )
 
+// DefaultSealedIndexedHeightThreshold is the default number of blocks between sealed and indexed height
+// this sets a limit on how far into the past the payer validator will allow for checking the payer's balance.
+const DefaultSealedIndexedHeightThreshold = 30
+
 type Blocks interface {
 	HeaderByID(id flow.Identifier) (*flow.Header, error)
 	FinalizedHeader() (*flow.Header, error)
 	SealedHeader() (*flow.Header, error)
+	IndexedHeight() (uint64, error)
 }
 
 type ProtocolStateBlocks struct {
-	state protocol.State
+	state         protocol.State
+	indexReporter state_synchronization.IndexReporter
 }
 
-func NewProtocolStateBlocks(state protocol.State) *ProtocolStateBlocks {
-	return &ProtocolStateBlocks{state: state}
+func NewProtocolStateBlocks(state protocol.State, indexReporter state_synchronization.IndexReporter) *ProtocolStateBlocks {
+	return &ProtocolStateBlocks{
+		state:         state,
+		indexReporter: indexReporter,
+	}
 }
 
 func (b *ProtocolStateBlocks) HeaderByID(id flow.Identifier) (*flow.Header, error) {
@@ -56,7 +66,19 @@ func (b *ProtocolStateBlocks) FinalizedHeader() (*flow.Header, error) {
 }
 
 func (b *ProtocolStateBlocks) SealedHeader() (*flow.Header, error) {
+
 	return b.state.Sealed().Head()
+
+}
+
+// IndexedHeight returns the highest indexed height by calling corresponding function of indexReporter.
+// Expected errors during normal operation:
+// - access.IndexReporterNotInitialized - indexed reporter was not initialized.
+func (b *ProtocolStateBlocks) IndexedHeight() (uint64, error) {
+	if b.indexReporter != nil {
+		return b.indexReporter.HighestIndexedHeight()
+	}
+	return 0, IndexReporterNotInitialized
 }
 
 // RateLimiter is an interface for checking if an address is rate limited.
@@ -78,6 +100,59 @@ func (l *NoopLimiter) IsRateLimited(address flow.Address) bool {
 	return false
 }
 
+// PayerBalanceMode represents the mode for checking the payer's balance
+// when validating transactions. It controls whether and how the balance
+// check is performed during transaction validation.
+//
+// There are few modes available:
+//
+//   - `Disabled` - Balance checking is completely disabled. No checks are
+//     performed to verify if the payer has sufficient balance to cover the
+//     transaction fees.
+//   - `WarnCheck` - Balance is checked, and a warning is logged if the payer
+//     does not have enough balance. The transaction is still accepted and
+//     processed regardless of the check result.
+//   - `EnforceCheck` - Balance is checked, and the transaction is rejected if
+//     the payer does not have sufficient balance to cover the transaction fees.
+type PayerBalanceMode int
+
+const (
+	// Disabled indicates that payer balance checking is turned off.
+	Disabled PayerBalanceMode = iota
+
+	// WarnCheck logs a warning if the payer's balance is insufficient, but does not prevent the transaction from being accepted.
+	WarnCheck
+
+	// EnforceCheck prevents the transaction from being accepted if the payer's balance is insufficient to cover transaction fees.
+	EnforceCheck
+)
+
+func ParsePayerBalanceMode(s string) (PayerBalanceMode, error) {
+	switch s {
+	case Disabled.String():
+		return Disabled, nil
+	case WarnCheck.String():
+		return WarnCheck, nil
+	case EnforceCheck.String():
+		return EnforceCheck, nil
+	default:
+		return 0, errors.New("invalid payer balance mode")
+	}
+}
+
+func (m PayerBalanceMode) String() string {
+	switch m {
+	case Disabled:
+		return "disabled"
+	case WarnCheck:
+		return "warn"
+	case EnforceCheck:
+		return "enforce"
+	default:
+		return ""
+	}
+}
+
 type TransactionValidationOptions struct {
 	Expiry                       uint
 	ExpiryBuffer                 uint
@@ -87,7 +162,7 @@ type TransactionValidationOptions struct {
 	CheckScriptsParse            bool
 	MaxTransactionByteSize       uint64
 	MaxCollectionByteSize        uint64
-	CheckPayerBalance            bool
+	CheckPayerBalanceMode        PayerBalanceMode
 }
 
 type ValidationStep struct {
@@ -115,7 +190,7 @@ func NewTransactionValidator(
 	options TransactionValidationOptions,
 	executor execution.ScriptExecutor,
 ) (*TransactionValidator, error) {
-	if options.CheckPayerBalance && executor == nil {
+	if options.CheckPayerBalanceMode != Disabled && executor == nil {
 		return nil, errors.New("transaction validator cannot use checkPayerBalance with nil executor")
 	}
 
@@ -193,7 +268,11 @@ func (v *TransactionValidator) Validate(ctx context.Context, tx *flow.Transactio
 		// prevent the transaction from proceeding.
 		if IsInsufficientBalanceError(err) {
 			v.transactionValidationMetrics.TransactionValidationFailed(metrics.InsufficientBalance)
-			return err
+
+			if v.options.CheckPayerBalanceMode == EnforceCheck {
+				log.Warn().Err(err).Str("transactionID", tx.ID().String()).Str("payerAddress", tx.Payer.String()).Msg("enforce check error")
+				return err
+			}
 		}
 
 		// log and ignore all other errors
@@ -398,7 +477,7 @@ func (v *TransactionValidator) checkSignatureFormat(tx *flow.TransactionBody) er
 }
 
 func (v *TransactionValidator) checkSufficientBalanceToPayForTransaction(ctx context.Context, tx *flow.TransactionBody) error {
-	if !v.options.CheckPayerBalance {
+	if v.options.CheckPayerBalanceMode == Disabled {
 		return nil
 	}
 
@@ -407,6 +486,19 @@ func (v *TransactionValidator) checkSufficientBalanceToPayForTransaction(ctx con
 		return fmt.Errorf("could not fetch block header: %w", err)
 	}
 
+	indexedHeight, err := v.blocks.IndexedHeight()
+	if err != nil {
+		return fmt.Errorf("could not get indexed height: %w", err)
+	}
+
+	// we use latest indexed block to get the most up-to-date state data available for executing scripts.
+	// check here to make sure indexing is within an acceptable tolerance of sealing to avoid issues
+	// if indexing falls behind
+	sealedHeight := header.Height
+	if indexedHeight < sealedHeight-DefaultSealedIndexedHeightThreshold {
+		return IndexedHeightFarBehindError{SealedHeight: sealedHeight, IndexedHeight: indexedHeight}
+	}
+
 	payerAddress := cadence.NewAddress(tx.Payer)
 	inclusionEffort := cadence.UFix64(tx.InclusionEffort())
 	gasLimit := cadence.UFix64(tx.GasLimit)
@@ -416,7 +508,7 @@ func (v *TransactionValidator) checkSufficientBalanceToPayForTransaction(ctx con
 		return fmt.Errorf("failed to encode cadence args for script executor: %w", err)
 	}
 
-	result, err := v.scriptExecutor.ExecuteAtBlockHeight(ctx, v.verifyPayerBalanceScript, args, header.Height)
+	result, err := v.scriptExecutor.ExecuteAtBlockHeight(ctx, v.verifyPayerBalanceScript, args, indexedHeight)
 	if err != nil {
 		return fmt.Errorf("script finished with error: %w", err)
 	}
@@ -432,7 +524,7 @@ func (v *TransactionValidator) checkSufficientBalanceToPayForTransaction(ctx con
 	}
 
 	// return no error if payer has sufficient balance
-	if bool(canExecuteTransaction) {
+	if canExecuteTransaction {
 		return nil
 	}
 

access/validator_test.go

@@ -58,7 +58,7 @@ func (s *TransactionValidatorSuite) SetupTest() {
 
 	s.chain = flow.Testnet.Chain()
 	s.validatorOptions = access.TransactionValidationOptions{
-		CheckPayerBalance:      true,
+		CheckPayerBalanceMode:  access.EnforceCheck,
 		MaxTransactionByteSize: flow.DefaultMaxTransactionByteSize,
 		MaxCollectionByteSize:  flow.DefaultMaxCollectionByteSize,
 	}
@@ -88,6 +88,10 @@ func (s *TransactionValidatorSuite) TestTransactionValidator_ScriptExecutorInter
 	scriptExecutor := execmock.NewScriptExecutor(s.T())
 	assert.NotNil(s.T(), scriptExecutor)
 
+	s.blocks.
+		On("IndexedHeight").
+		Return(s.header.Height, nil)
+
 	scriptExecutor.
 		On("ExecuteAtBlockHeight", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
 		Return(nil, errors.New("script executor internal error")).
@@ -115,6 +119,10 @@ func (s *TransactionValidatorSuite) TestTransactionValidator_SufficientBalance()
 	actualResponse, err := jsoncdc.Encode(actualResponseValue)
 	assert.NoError(s.T(), err)
 
+	s.blocks.
+		On("IndexedHeight").
+		Return(s.header.Height, nil)
+
 	scriptExecutor.
 		On("ExecuteAtBlockHeight", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
 		Return(actualResponse, nil).
@@ -142,27 +150,61 @@ func (s *TransactionValidatorSuite) TestTransactionValidator_InsufficientBalance
 	actualResponse, err := jsoncdc.Encode(actualResponseValue)
 	assert.NoError(s.T(), err)
 
+	s.blocks.
+		On("IndexedHeight").
+		Return(s.header.Height, nil)
+
 	scriptExecutor.
 		On("ExecuteAtBlockHeight", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
-		Return(actualResponse, nil).
-		Once()
+		Return(actualResponse, nil).Twice()
 
 	actualAccountResponse, err := unittest.AccountFixture()
 	assert.NoError(s.T(), err)
 	assert.NotNil(s.T(), actualAccountResponse)
 
+	validateTx := func() error {
+		txBody := unittest.TransactionBodyFixture()
+		validator, err := access.NewTransactionValidator(s.blocks, s.chain, s.metrics, s.validatorOptions, scriptExecutor)
+		assert.NoError(s.T(), err)
+		assert.NotNil(s.T(), validator)
+
+		return validator.Validate(context.Background(), &txBody)
+	}
+
+	s.Run("with enforce check", func() {
+		err := validateTx()
+
+		expectedError := access.InsufficientBalanceError{
+			Payer:           unittest.AddressFixture(),
+			RequiredBalance: requiredBalance,
+		}
+		assert.ErrorIs(s.T(), err, expectedError)
+	})
+
+	s.Run("with warn check", func() {
+		s.validatorOptions.CheckPayerBalanceMode = access.WarnCheck
+		err := validateTx()
+		assert.NoError(s.T(), err)
+	})
+}
+
+func (s *TransactionValidatorSuite) TestTransactionValidator_SealedIndexedHeightThresholdLimit() {
+	scriptExecutor := execmock.NewScriptExecutor(s.T())
+
+	// setting indexed height to be behind of sealed by bigger number than allowed(DefaultSealedIndexedHeightThreshold)
+	indexedHeight := s.header.Height - 40
+
+	s.blocks.
+		On("IndexedHeight").
+		Return(indexedHeight, nil)
+
 	validator, err := access.NewTransactionValidator(s.blocks, s.chain, s.metrics, s.validatorOptions, scriptExecutor)
 	assert.NoError(s.T(), err)
 	assert.NotNil(s.T(), validator)
 
 	txBody := unittest.TransactionBodyFixture()
 
-	expectedError := access.InsufficientBalanceError{
-		Payer:           unittest.AddressFixture(),
-		RequiredBalance: requiredBalance,
-	}
-
-	actualErr := validator.Validate(context.Background(), &txBody)
+	err = validator.Validate(context.Background(), &txBody)
+	assert.NoError(s.T(), err)
 
-	assert.ErrorIs(s.T(), actualErr, expectedError)
 }